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()) +}