Commit `1285c70990`

1285c70990b55e35ba6ce0f7848cae5c6b46b0f4

parent: d4c806be81

Verified · cmc ci/build: success ci/test: success

cmc <hello@cleberg.net> · 2026-09-07 22:18 UTC

store: removing an org member drops their team memberships

RemoveOrgMember deleted the org_members row only. team add refuses
non-members, so the invariant was kept on the way in and not on the way
out: a removed member kept every grant their teams carried, and re-adding
them to the org restored nothing because nothing had gone.

The team_members rows for the org's teams now go in the same transaction.
Other orgs' teams are untouched.

Closes #196

e2e/orgremove_test.go added +72

		@@ -0,0 +1,72 @@
	1	package e2e
	2
	3	import (
	4	"os"
	5	"path/filepath"
	6	"strings"
	7	"testing"
	8	)
	9
	10	// Removing a member from an org ends the access they held through its
	11	// teams (#196). Before the fix, team_members rows survived removal, so a
	12	// former member kept pushing.
	13	func TestOrgMemberRemovalEndsTeamAccess(t *testing.T) {
	14	inst := startInstance(t)
	15	aliceKey := inst.newKey(t, "alice")
	16	bobKey := inst.newKey(t, "bob")
	17	inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub")
	18	inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub")
	19
	20	for _, args := range [][]string{
	21	{"org", "create", "acme"},
	22	{"org", "settings", "members-role", "acme", "none"},
	23	{"org", "members", "add", "acme", "bob"},
	24	{"repo", "create", "acme/widget", "--private"},
	25	{"org", "team", "create", "acme", "core"},
	26	{"org", "team", "add", "acme", "core", "bob"},
	27	{"org", "team", "grant", "acme", "core", "acme/widget", "write"},
	28	} {
	29	if _, errOut, code := inst.ssh(t, aliceKey, "", args...); code != 0 {
	30	t.Fatalf("%v: %s", args, errOut)
	31	}
	32	}
	33
	34	// bob pushes through the team grant.
	35	bobEnv := inst.gitEnv(bobKey)
	36	work := t.TempDir()
	37	mustGit(t, work, bobEnv, "clone", inst.sshURL("acme/widget"), "widget")
	38	dir := filepath.Join(work, "widget")
	39	if err := os.WriteFile(filepath.Join(dir, "README"), []byte("hi\n"), 0o644); err != nil {
	40	t.Fatal(err)
	41	}
	42	mustGit(t, dir, bobEnv, "checkout", "-q", "-b", "main")
	43	mustGit(t, dir, bobEnv, "add", "README")
	44	mustGit(t, dir, bobEnv, "commit", "-q", "-m", "one")
	45	mustGit(t, dir, bobEnv, "push", "-q", "origin", "main")
	46
	47	if _, errOut, code := inst.ssh(t, aliceKey, "", "org", "members", "remove", "acme", "bob"); code != 0 {
	48	t.Fatalf("members remove: %s", errOut)
	49	}
	50	out, _, _ := inst.ssh(t, aliceKey, "", "org", "team", "show", "acme", "core", "--json")
	51	if strings.Contains(out, `"bob"`) {
	52	t.Fatalf("team still lists the removed member: %s", out)
	53	}
	54	if err := os.WriteFile(filepath.Join(dir, "README"), []byte("again\n"), 0o644); err != nil {
	55	t.Fatal(err)
	56	}
	57	mustGit(t, dir, bobEnv, "commit", "-q", "-am", "two")
	58	if out, code := gitRun(t, dir, bobEnv, "push", "-q", "origin", "main"); code == 0 {
	59	t.Fatalf("removed member still pushes:\n%s", out)
	60	}
	61	if _, _, code := inst.ssh(t, bobKey, "", "repo", "show", "acme/widget"); code != 3 {
	62	t.Fatalf("removed member still sees the private repo: exit %d", code)
	63	}
	64
	65	// Re-adding to the org does not silently restore the team grant.
	66	if _, errOut, code := inst.ssh(t, aliceKey, "", "org", "members", "add", "acme", "bob"); code != 0 {
	67	t.Fatalf("members add: %s", errOut)
	68	}
	69	if _, _, code := inst.ssh(t, bobKey, "", "repo", "show", "acme/widget"); code != 3 {
	70	t.Fatalf("re-added member regained the team grant: exit %d", code)
	71	}
	72	}

internal/store/orgremove_test.go added +57

		@@ -0,0 +1,57 @@
	1	package store
	2
	3	import "testing"
	4
	5	// Removing an org member drops their team memberships in that org. Team
	6	// add requires membership, so a row that survived removal would grant a
	7	// non-member access through the team (#196).
	8	func TestRemoveOrgMemberDropsTeamRows(t *testing.T) {
	9	s := open(t)
	10	if err := s.MigrateUp(); err != nil {
	11	t.Fatal(err)
	12	}
	13	alice, err := s.CreateUser("alice", false)
	14	if err != nil {
	15	t.Fatal(err)
	16	}
	17	bob, err := s.CreateUser("bob", false)
	18	if err != nil {
	19	t.Fatal(err)
	20	}
	21	oid, err := s.CreateOrg("acme", alice)
	22	if err != nil {
	23	t.Fatal(err)
	24	}
	25	other, err := s.CreateOrg("other", alice)
	26	if err != nil {
	27	t.Fatal(err)
	28	}
	29	for _, o := range []int64{oid, other} {
	30	if err := s.SetOrgMember(o, bob, "member"); err != nil {
	31	t.Fatal(err)
	32	}
	33	}
	34	core, err := s.CreateTeam(oid, "core")
	35	if err != nil {
	36	t.Fatal(err)
	37	}
	38	elsewhere, err := s.CreateTeam(other, "core")
	39	if err != nil {
	40	t.Fatal(err)
	41	}
	42	for _, tm := range []int64{core, elsewhere} {
	43	if err := s.AddTeamMember(tm, bob); err != nil {
	44	t.Fatal(err)
	45	}
	46	}
	47	if err := s.RemoveOrgMember(oid, bob); err != nil {
	48	t.Fatal(err)
	49	}
	50	if m, _ := s.TeamMembers(core); len(m) != 0 {
	51	t.Fatalf("team rows survived org removal: %v", m)
	52	}
	53	// The other org's team is untouched.
	54	if m, _ := s.TeamMembers(elsewhere); len(m) != 1 \|\| m[0] != "bob" {
	55	t.Fatalf("removal reached another org's team: %v", m)
	56	}
	57	}

internal/store/orgs.go +8 −1

		@@ -143,7 +143,9 @@ func (s *Store) SetOrgMember(orgID, userID int64, role string) error {
143	143	return tx.Commit()
144	144	}
145	145
146		// RemoveOrgMember drops a member, refusing to remove the last admin.
	146	// RemoveOrgMember drops a member, refusing to remove the last admin. The
	147	// account's team memberships in the org go with it: team add requires
	148	// membership, so a row left behind would grant access to a non-member.
147	149	func (s *Store) RemoveOrgMember(orgID, userID int64) error {
148	150	tx, err := s.DB.Begin()
149	151	if err != nil {
		@@ -164,6 +166,11 @@ func (s *Store) RemoveOrgMember(orgID, userID int64) error {
164	166	if n, _ := res.RowsAffected(); n == 0 {
165	167	return ErrNotFound
166	168	}
	169	if _, err := tx.Exec(
	170	"DELETE FROM team_members WHERE user_id = ? AND team_id IN (SELECT id FROM teams WHERE org_id = ?)",
	171	userID, orgID); err != nil {
	172	return err
	173	}
167	174	return tx.Commit()
168	175	}
169	176

Commit 1285c70990

Commit `1285c70990`