derive maintainer edit

routers/private/hook_pre_receive.go

@@ -51,20 +51,25 @@ type preReceiveContext struct {
 }
 
 // CanWriteCode returns true if pusher can write code
-func (ctx *preReceiveContext) CanWriteCode(ref git.RefName) bool {
+func (ctx *preReceiveContext) CanWriteCode(refFullName git.RefName) bool {
 	if !ctx.loadPusherAndPermission() {
 		return false
 	}
-	// Must not be cached: CanMaintainerWriteToBranch is evaluated against ctx.branchName, which
+	// The maintainer-edit grant is scoped to a single PR head branch, so it must be evaluated
-	// differs for each ref in a batch push. Caching the first result would let a per-branch
+	// against the exact branch being pushed and only for branch refs. Tags and other refs can
-	// maintainer-edit grant on one ref authorize writes to every other ref in the same push.
+	// never match a PR head branch, so they pass an empty name. Deriving this per ref (instead of
-	// FIXME: the ref can be a branch, a tag, or something else?
+	// from shared mutable state) prevents a per-branch grant from authorizing writes to other refs
-	return issues_model.CanMaintainerWriteToBranch(ctx, ctx.userPerm, branchName, ctx.user) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
+	// batched into the same push.
+	maintainerEditBranch := ""
+	if refFullName.IsBranch() {
+		maintainerEditBranch = refFullName.BranchName()
+	}
+	return issues_model.CanMaintainerWriteToBranch(ctx, ctx.userPerm, maintainerEditBranch, ctx.user) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
 }
 
 // AssertCanWriteCode returns true if pusher can write code
-func (ctx *preReceiveContext) AssertCanWriteCode(ref git.RefName) bool {
+func (ctx *preReceiveContext) AssertCanWriteCode(refFullName git.RefName) bool {
-	if !ctx.CanWriteCode(ref) {
+	if !ctx.CanWriteCode(refFullName) {
 		if ctx.Written() {
 			return false
 		}

routers/private/hook_pre_receive_test.go

@@ -10,6 +10,7 @@ import (
 	"gitea.dev/models/perm/access"
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/models/unittest"
+	"gitea.dev/modules/git"
 	"gitea.dev/services/contexttest"
 
 	"github.com/stretchr/testify/assert"
@@ -17,9 +18,9 @@ import (
 )
 
 // TestPreReceiveCanWriteCodePerBranch ensures the maintainer-edit write grant is evaluated against
-// the current branch on every call, instead of being cached from the first ref of a batch push.
+// the exact ref being pushed on every call, derived from that ref rather than shared mutable state.
 // Otherwise a per-branch grant (an open PR with "allow edits from maintainers") could be batched
-// together with a protected branch to escalate into full repository write.
+// together with a protected branch or a tag to escalate into full repository write.
 func TestPreReceiveCanWriteCodePerBranch(t *testing.T) {
 	require.NoError(t, unittest.PrepareTestDatabase())
 
@@ -57,10 +58,13 @@ func TestPreReceiveCanWriteCodePerBranch(t *testing.T) {
 	}
 
 	// The granted branch must be writable...
-	ctx.branchName = "granted-branch"
+	assert.True(t, ctx.CanWriteCode(git.RefNameFromBranch("granted-branch")))
-	assert.True(t, ctx.CanWriteCode())
 
 	// ...but another branch in the same push must NOT inherit that grant.
-	ctx.branchName = "master"
+	assert.False(t, ctx.CanWriteCode(git.RefNameFromBranch("master")))
-	assert.False(t, ctx.CanWriteCode())
+
+	// ...and a tag sharing the granted branch's name must NOT inherit it either: the grant is
+	// scoped to PR head branches, so a non-branch ref can never match it. (A tag ref already
+	// yields an empty branch name, so this guards the per-ref evaluation, not the IsBranch check.)
+	assert.False(t, ctx.CanWriteCode(git.RefNameFromTag("granted-branch")))
 }