12d3d1e fix(usecase): refuse repository settings changes to branch-limited tokens · ermite

M internal/app/e2e_branches_test.go +9 -0


@@ -10,6 +10,7 @@ import (
10	10		"connectrpc.com/connect"
11	11		"github.com/stretchr/testify/assert"
12	12		"github.com/stretchr/testify/require"
	13	+	"google.golang.org/protobuf/types/known/fieldmaskpb"
13	14
14	15		pb "bnema.dev/ermite-api/gen/ermite"
15	16		"bnema.dev/ermite/internal/domain"

@@ -121,6 +122,14 @@ func TestAC5BranchLimitedToken(t *testing.T) {
121	122		_, err = h.api(admin).repo.DeleteRepo(h.ctx, connect.NewRequest(&pb.DeleteRepoRequest{Repo: "via-api"}))
122	123		assert.Equal(t, "permission_denied", codeName(err))
123	124		assert.DirExists(t, h.repoDir("via-api"))
	125	+	// ... nor change its settings (default branch, visibility, ...).
	126	+	_, err = h.api(admin).repo.UpdateRepo(h.ctx, connect.NewRequest(&pb.UpdateRepoRequest{
	127	+	Repo: "via-api", Visibility: pb.Visibility_VISIBILITY_PUBLIC, UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"visibility"}},
	128	+	}))
	129	+	assert.Equal(t, "permission_denied", codeName(err))
	130	+	got, err := h.api(adminToken).repo.GetRepo(h.ctx, connect.NewRequest(&pb.GetRepoRequest{Repo: "via-api"}))
	131	+	require.NoError(t, err)
	132	+	assert.Equal(t, pb.Visibility_VISIBILITY_PRIVATE, got.Msg.GetRepo().GetVisibility())
124	133
125	134		// It never has owner rights, whatever its scopes.
126	135		all, _ := h.branchToken(domain.AllScopes(), []string{"agent/claude/"})


@@ -10,6 +10,7 @@ import (
10	"connectrpc.com/connect"	10		"connectrpc.com/connect"
11	"github.com/stretchr/testify/assert"	11		"github.com/stretchr/testify/assert"
12	"github.com/stretchr/testify/require"	12		"github.com/stretchr/testify/require"
		13	+	"google.golang.org/protobuf/types/known/fieldmaskpb"
13		14
14	pb "bnema.dev/ermite-api/gen/ermite"	15		pb "bnema.dev/ermite-api/gen/ermite"
15	"bnema.dev/ermite/internal/domain"	16		"bnema.dev/ermite/internal/domain"

@@ -121,6 +122,14 @@ func TestAC5BranchLimitedToken(t *testing.T) {
121	_, err = h.api(admin).repo.DeleteRepo(h.ctx, connect.NewRequest(&pb.DeleteRepoRequest{Repo: "via-api"}))	122		_, err = h.api(admin).repo.DeleteRepo(h.ctx, connect.NewRequest(&pb.DeleteRepoRequest{Repo: "via-api"}))
122	assert.Equal(t, "permission_denied", codeName(err))	123		assert.Equal(t, "permission_denied", codeName(err))
123	assert.DirExists(t, h.repoDir("via-api"))	124		assert.DirExists(t, h.repoDir("via-api"))
		125	+	// ... nor change its settings (default branch, visibility, ...).
		126	+	_, err = h.api(admin).repo.UpdateRepo(h.ctx, connect.NewRequest(&pb.UpdateRepoRequest{
		127	+	Repo: "via-api", Visibility: pb.Visibility_VISIBILITY_PUBLIC, UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"visibility"}},
		128	+	}))
		129	+	assert.Equal(t, "permission_denied", codeName(err))
		130	+	got, err := h.api(adminToken).repo.GetRepo(h.ctx, connect.NewRequest(&pb.GetRepoRequest{Repo: "via-api"}))
		131	+	require.NoError(t, err)
		132	+	assert.Equal(t, pb.Visibility_VISIBILITY_PRIVATE, got.Msg.GetRepo().GetVisibility())
124		133
125	// It never has owner rights, whatever its scopes.	134		// It never has owner rights, whatever its scopes.
126	all, _ := h.branchToken(domain.AllScopes(), []string{"agent/claude/"})	135		all, _ := h.branchToken(domain.AllScopes(), []string{"agent/claude/"})

M internal/usecase/access.go +7 -4


@@ -55,10 +55,13 @@ func canCreateRepo(p domain.Principal, name domain.RepoName) bool {
55	55		return canWrite(p, name) && len(p.BranchPrefixes()) == 0
56	56		}
57	57
58		-	// canDeleteRepo is the access rule for deleting a repository on top of
59		-	// adminAccess: no branch prefixes. Deleting a repository deletes every branch,
60		-	// those outside the prefixes of a limited token included.
61		-	func canDeleteRepo(p domain.Principal) bool {
	58	+	// canAdminRepo is the access rule for changing the settings of a repository
	59	+	// or deleting it, on top of adminAccess: no branch prefixes. A token limited to
	60	+	// some branches may not delete the repository (that deletes every branch, those
	61	+	// outside its prefixes included), nor change its default branch, visibility,
	62	+	// description, merge modes or failing-checks rule: these settings decide what
	63	+	// happens to branches the token may not write.
	64	+	func canAdminRepo(p domain.Principal) bool {
62	65		return len(p.BranchPrefixes()) == 0
63	66		}
64	67

 	return canWrite(p, name) && len(p.BranchPrefixes()) == 0
 }
-// canDeleteRepo is the access rule for deleting a repository on top of
+// canAdminRepo is the access rule for changing the settings of a repository
-// adminAccess: no branch prefixes. Deleting a repository deletes every branch,
+// or deleting it, on top of adminAccess: no branch prefixes. A token limited to
-// those outside the prefixes of a limited token included.
+// some branches may not delete the repository (that deletes every branch, those
-func canDeleteRepo(p domain.Principal) bool {
+// outside its prefixes included), nor change its default branch, visibility,
+// description, merge modes or failing-checks rule: these settings decide what
+// happens to branches the token may not write.
+func canAdminRepo(p domain.Principal) bool {
 	return len(p.BranchPrefixes()) == 0
 }

M internal/usecase/repos_write.go +5 -2


@@ -46,7 +46,7 @@ func (r *Repos) View(ctx context.Context, repo domain.Repo) (ports.RepoView, err
46	46
47	47		// Update changes the description, the visibility, the default branch and the
48	48		// merge settings of a repository, and publishes RepoUpdated. The principal needs repo:admin on it
49		-	// (see adminAccess for what the others learn). At least one field must be set,
	49	+	// (see adminAccess for what the others learn) and no branch prefixes. At least one field must be set,
50	50		// else domain.ErrInvalid. A default branch that does not exist gives
51	51		// domain.ErrPreconditionFailed; nothing is written before every check passed.
52	52		// The metadata is written first, then HEAD, and RepoUpdated is published after

@@ -60,6 +60,9 @@ func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.Repo
60	60		if err := adminAccess(p, repo); err != nil {
61	61		return domain.Repo{}, err
62	62		}
	63	+	if !canAdminRepo(p) {
	64	+	return domain.Repo{}, fmt.Errorf("%w: a token limited to branch prefixes may not change repository settings", domain.ErrForbidden)
	65	+	}
63	66		before := repo
64	67		if params.Description == nil && params.Visibility == nil && params.DefaultBranch == nil &&
65	68		params.AllowedMergeModes == nil && params.FailingChecksBlock == nil {

@@ -184,7 +187,7 @@ func (r *Repos) Delete(ctx context.Context, p domain.Principal, name domain.Repo
184	187		if err := adminAccess(p, repo); err != nil {
185	188		return err
186	189		}
187		-	if !canDeleteRepo(p) {
	190	+	if !canAdminRepo(p) {
188	191		return fmt.Errorf("%w: a token limited to branch prefixes may not delete a repository", domain.ErrForbidden)
189	192		}
190	193		log := zerowrap.FromCtx(ctx)

 // Update changes the description, the visibility, the default branch and the
 // merge settings of a repository, and publishes RepoUpdated. The principal needs repo:admin on it
-// (see adminAccess for what the others learn). At least one field must be set,
+// (see adminAccess for what the others learn) and no branch prefixes. At least one field must be set,
 // else domain.ErrInvalid. A default branch that does not exist gives
 // domain.ErrPreconditionFailed; nothing is written before every check passed.
 // The metadata is written first, then HEAD, and RepoUpdated is published after
 	if err := adminAccess(p, repo); err != nil {
 		return domain.Repo{}, err
 	}
+	if !canAdminRepo(p) {
+		return domain.Repo{}, fmt.Errorf("%w: a token limited to branch prefixes may not change repository settings", domain.ErrForbidden)
+	}
 	before := repo
 	if params.Description == nil && params.Visibility == nil && params.DefaultBranch == nil &&
 		params.AllowedMergeModes == nil && params.FailingChecksBlock == nil {
 	if err := adminAccess(p, repo); err != nil {
 		return err
 	}
-	if !canDeleteRepo(p) {
+	if !canAdminRepo(p) {
 		return fmt.Errorf("%w: a token limited to branch prefixes may not delete a repository", domain.ErrForbidden)
 	}
 	log := zerowrap.FromCtx(ctx)

M internal/usecase/repos_write_test.go +18 -0


@@ -165,6 +165,24 @@ func TestUpdate(t *testing.T) {
165	165		})
166	166		}
167	167
	168	+	t.Run("a token limited to branches may not change any setting", func(t *testing.T) {
	169	+	t.Parallel()
	170	+	modes := []domain.MergeMode{domain.MergeModeSquash}
	171	+	block := true
	172	+	for name, params := range map[string]ports.UpdateRepoParams{
	173	+	"default branch": {DefaultBranch: ptr("dev")},
	174	+	"visibility": {Visibility: ptr(domain.VisibilityPublic)},
	175	+	"description": {Description: ptr("x")},
	176	+	"merge modes": {AllowedMergeModes: &modes},
	177	+	"checks rule": {FailingChecksBlock: &block},
	178	+	} {
	179	+	f := newFixture(t) // no HasBranch, no Update, no SetDefaultBranch, no Publish
	180	+	f.meta.EXPECT().Get(ctx, n).Return(priv, nil)
	181	+	_, err := f.repos.Update(ctx, branchLimited(domain.ScopeRepoAdmin), n, params)
	182	+	assert.ErrorIs(t, err, domain.ErrForbidden, name)
	183	+	}
	184	+	})
	185	+
168	186		t.Run("a token limited to branches may not delete a repository", func(t *testing.T) {
169	187		t.Parallel()
170	188		f := newFixture(t) // no Trash, no Delete


@@ -165,6 +165,24 @@ func TestUpdate(t *testing.T) {
165	})	165		})
166	}	166		}
167		167
		168	+	t.Run("a token limited to branches may not change any setting", func(t *testing.T) {
		169	+	t.Parallel()
		170	+	modes := []domain.MergeMode{domain.MergeModeSquash}
		171	+	block := true
		172	+	for name, params := range map[string]ports.UpdateRepoParams{
		173	+	"default branch": {DefaultBranch: ptr("dev")},
		174	+	"visibility": {Visibility: ptr(domain.VisibilityPublic)},
		175	+	"description": {Description: ptr("x")},
		176	+	"merge modes": {AllowedMergeModes: &modes},
		177	+	"checks rule": {FailingChecksBlock: &block},
		178	+	} {
		179	+	f := newFixture(t) // no HasBranch, no Update, no SetDefaultBranch, no Publish
		180	+	f.meta.EXPECT().Get(ctx, n).Return(priv, nil)
		181	+	_, err := f.repos.Update(ctx, branchLimited(domain.ScopeRepoAdmin), n, params)
		182	+	assert.ErrorIs(t, err, domain.ErrForbidden, name)
		183	+	}
		184	+	})
		185	+
168	t.Run("a token limited to branches may not delete a repository", func(t *testing.T) {	186		t.Run("a token limited to branches may not delete a repository", func(t *testing.T) {
169	t.Parallel()	187		t.Parallel()
170	f := newFixture(t) // no Trash, no Delete	188		f := newFixture(t) // no Trash, no Delete