From f25811942cea35233299efdbdfeec40663d4807a Mon Sep 17 00:00:00 2001
From: Nicolas <bircni@icloud.com>
Date: Sat, 13 Jun 2026 18:37:03 +0200
Subject: [PATCH] fix: re-check branch write permission for every ref in a push

The pre-receive hook cached the result of CanWriteCode() after the first
ref in a batch push, but CanMaintainerWriteToBranch depends on the current
branch name. A user holding a per-branch maintainer-edit grant (an open PR
with "allow edits from maintainers") could batch that branch with protected
branches or tags and have the cached approval reused, escalating to full
repository write. Evaluate the permission fresh for every ref; the pusher
and base permission remain cached via loadPusherAndPermission.

Assisted-by: Claude:claude-opus-4-8
---
 routers/private/hook_pre_receive.go      | 16 +++---
 routers/private/hook_pre_receive_test.go | 66 ++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 10 deletions(-)
 create mode 100644 routers/private/hook_pre_receive_test.go

diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go
index 91a02b0f367..154a830a906 100644
--- a/routers/private/hook_pre_receive.go
+++ b/routers/private/hook_pre_receive.go
@@ -40,9 +40,6 @@ type preReceiveContext struct {
 	canCreatePullRequest        bool
 	checkedCanCreatePullRequest bool
 
-	canWriteCode        bool
-	checkedCanWriteCode bool
-
 	protectedTags    []*git_model.ProtectedTag
 	gotProtectedTags bool
 
@@ -55,14 +52,13 @@ type preReceiveContext struct {
 
 // CanWriteCode returns true if pusher can write code
 func (ctx *preReceiveContext) CanWriteCode() bool {
-	if !ctx.checkedCanWriteCode {
-		if !ctx.loadPusherAndPermission() {
-			return false
-		}
-		ctx.canWriteCode = issues_model.CanMaintainerWriteToBranch(ctx, ctx.userPerm, ctx.branchName, ctx.user) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
-		ctx.checkedCanWriteCode = true
+	if !ctx.loadPusherAndPermission() {
+		return false
 	}
-	return ctx.canWriteCode
+	// Must not be cached: CanMaintainerWriteToBranch is evaluated against ctx.branchName, which
+	// differs for each ref in a batch push. Caching the first result would let a per-branch
+	// maintainer-edit grant on one ref authorize writes to every other ref in the same push.
+	return issues_model.CanMaintainerWriteToBranch(ctx, ctx.userPerm, ctx.branchName, ctx.user) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
 }
 
 // AssertCanWriteCode returns true if pusher can write code
diff --git a/routers/private/hook_pre_receive_test.go b/routers/private/hook_pre_receive_test.go
new file mode 100644
index 00000000000..2ee64f2e403
--- /dev/null
+++ b/routers/private/hook_pre_receive_test.go
@@ -0,0 +1,66 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package private
+
+import (
+	"testing"
+
+	issues_model "gitea.dev/models/issues"
+	"gitea.dev/models/perm/access"
+	repo_model "gitea.dev/models/repo"
+	"gitea.dev/models/unittest"
+	"gitea.dev/services/contexttest"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// 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.
+// 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.
+func TestPreReceiveCanWriteCodePerBranch(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
+	headRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 11})
+	require.NoError(t, baseRepo.LoadOwner(t.Context()))
+	require.NoError(t, headRepo.LoadOwner(t.Context()))
+
+	// An open PR from the head repo owner, with maintainer edits allowed: this grants the base
+	// repo owner write access to exactly this head branch and nothing else.
+	pr := &issues_model.PullRequest{
+		Issue: &issues_model.Issue{
+			RepoID:   baseRepo.ID,
+			PosterID: headRepo.OwnerID,
+		},
+		HeadRepoID:          headRepo.ID,
+		BaseRepoID:          baseRepo.ID,
+		HeadBranch:          "granted-branch",
+		BaseBranch:          "master",
+		AllowMaintainerEdit: true,
+	}
+	require.NoError(t, issues_model.NewPullRequest(t.Context(), baseRepo, pr.Issue, nil, nil, pr))
+
+	// The pusher is the base repo owner (the maintainer) with only read access on the head repo.
+	maintainer := baseRepo.Owner
+	headPerm, err := access.GetIndividualUserRepoPermission(t.Context(), headRepo, maintainer)
+	require.NoError(t, err)
+
+	mockCtx, _ := contexttest.MockPrivateContext(t, "/")
+	ctx := &preReceiveContext{
+		PrivateContext: mockCtx,
+		loadedPusher:   true,
+		user:           maintainer,
+		userPerm:       headPerm,
+	}
+
+	// The granted branch must be writable...
+	ctx.branchName = "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())
+}