mirror of
https://github.com/go-gitea/gitea.git
synced 2026-06-14 07:34:11 +00:00
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
This commit is contained in:
@@ -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
|
||||
|
||||
66
routers/private/hook_pre_receive_test.go
Normal file
66
routers/private/hook_pre_receive_test.go
Normal file
@@ -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())
|
||||
}
|
||||
Reference in New Issue
Block a user