mirror of
https://github.com/go-gitea/gitea.git
synced 2026-07-11 19:59:58 +00:00
Backport #38284 by @bircni Fixes #38278 ## Problem When branch protection matches the branch an Actions workflow pushes to, the runner's `git push` is rejected — even though the workflow token has `contents: write` and the same push performed with a PAT (write access) succeeds. Disabling protection or changing the pattern so it no longer matches makes the push work. ## Root cause In `preReceiveBranch` (`routers/private/hook_pre_receive.go`), the "can the doer push to this protected branch" check resolves the pusher with `user_model.GetUserByID(ctx, ctx.opts.UserID)`. For an Actions push the user ID is `-2` (the virtual `ActionsUserID`), which has no database row, so the lookup fails. Even past that, `CanUserPush` → `HasAccessUnit`/whitelist membership cannot evaluate a virtual user and returns `false`. As a result the Actions bot was rejected on every matching protected branch, despite the earlier `assertCanWriteRef` already confirming the token's code-write via `GetActionsUserRepoPermission`. This was inconsistent: a PAT with identical write access passed the exact same check. ## Fix Evaluate the Actions bot against its already-computed token permission instead of a user lookup, mirroring the existing `IsUserMergeWhitelisted` pattern: - Add `CanActionsUserPush` / `CanActionsUserForcePush` on `ProtectedBranch`, which take the precomputed `access_model.Permission`. - Allow the push when push is enabled, **no** push whitelist is enforced, and the token has code-write. - Keep the bot blocked when a whitelist is enforced — it cannot be added to one, so it must use a pull request. This preserves the whitelist as a real security boundary. Force-push, signed-commit and protected-file-path checks are untouched. Signed-off-by: bircni <bircni@icloud.com> Co-authored-by: bircni <bircni@icloud.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
@@ -31,9 +31,7 @@ import (
|
||||
type preReceiveContext struct {
|
||||
*gitea_context.PrivateContext
|
||||
|
||||
// loadedPusher indicates that where the following information are loaded
|
||||
loadedPusher bool
|
||||
user *user_model.User // it's the org user if a DeployKey is used
|
||||
user *user_model.User // the "pusher", it's the org user if a DeployKey is used
|
||||
userPerm access_model.Permission
|
||||
deployKeyAccessMode perm_model.AccessMode
|
||||
|
||||
@@ -53,10 +51,7 @@ type preReceiveContext struct {
|
||||
|
||||
func (ctx *preReceiveContext) canWriteCodeUnit() bool {
|
||||
if ctx.canWriteCodeUnitCached == nil {
|
||||
var canWrite bool
|
||||
if ctx.loadPusherAndPermission() {
|
||||
canWrite = ctx.userPerm.CanWrite(unit.TypeCode) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
|
||||
}
|
||||
canWrite := ctx.userPerm.CanWrite(unit.TypeCode) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
|
||||
ctx.canWriteCodeUnitCached = &canWrite
|
||||
}
|
||||
return *ctx.canWriteCodeUnitCached
|
||||
@@ -91,9 +86,6 @@ func (ctx *preReceiveContext) assertCanWriteRef(refFullName git.RefName) bool {
|
||||
// CanCreatePullRequest returns true if pusher can create pull requests
|
||||
func (ctx *preReceiveContext) CanCreatePullRequest() bool {
|
||||
if !ctx.checkedCanCreatePullRequest {
|
||||
if !ctx.loadPusherAndPermission() {
|
||||
return false
|
||||
}
|
||||
ctx.canCreatePullRequest = ctx.userPerm.CanRead(unit.TypePullRequests)
|
||||
ctx.checkedCanCreatePullRequest = true
|
||||
}
|
||||
@@ -124,6 +116,10 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) {
|
||||
opts: opts,
|
||||
}
|
||||
|
||||
if !ourCtx.loadPusherAndPermission() {
|
||||
return // if error occurs, loadPusherAndPermission had written the error response
|
||||
}
|
||||
|
||||
// Iterate across the provided old commit IDs
|
||||
for i := range opts.OldCommitIDs {
|
||||
oldCommitID := opts.OldCommitIDs[i]
|
||||
@@ -281,18 +277,10 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
|
||||
canPush = !changedProtectedfiles && protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys)
|
||||
}
|
||||
} else {
|
||||
user, err := user_model.GetUserByID(ctx, ctx.opts.UserID)
|
||||
if err != nil {
|
||||
log.Error("Unable to GetUserByID for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err)
|
||||
ctx.JSON(http.StatusInternalServerError, private.Response{
|
||||
Err: fmt.Sprintf("Unable to GetUserByID for commits from %s to %s: %v", oldCommitID, newCommitID, err),
|
||||
})
|
||||
return
|
||||
}
|
||||
if isForcePush {
|
||||
canPush = !changedProtectedfiles && protectBranch.CanUserForcePush(ctx, user)
|
||||
canPush = !changedProtectedfiles && protectBranch.CanUserForcePush(ctx, ctx.user)
|
||||
} else {
|
||||
canPush = !changedProtectedfiles && protectBranch.CanUserPush(ctx, user)
|
||||
canPush = !changedProtectedfiles && protectBranch.CanUserPush(ctx, ctx.user)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -354,12 +342,6 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
|
||||
return
|
||||
}
|
||||
|
||||
// although we should have called `loadPusherAndPermission` before, here we call it explicitly again because we need to access ctx.user below
|
||||
if !ctx.loadPusherAndPermission() {
|
||||
// if error occurs, loadPusherAndPermission had written the error response
|
||||
return
|
||||
}
|
||||
|
||||
// Now check if the user is allowed to merge PRs for this repository
|
||||
// Note: we can use ctx.perm and ctx.user directly as they will have been loaded above
|
||||
allowedMerge, err := pull_service.IsUserAllowedToMerge(ctx, pr, ctx.userPerm, ctx.user)
|
||||
@@ -499,10 +481,6 @@ func generateGitEnv(opts *private.HookOptions) (env []string) {
|
||||
|
||||
// loadPusherAndPermission returns false if an error occurs, and it writes the error response
|
||||
func (ctx *preReceiveContext) loadPusherAndPermission() bool {
|
||||
if ctx.loadedPusher {
|
||||
return true
|
||||
}
|
||||
|
||||
if ctx.opts.UserID == user_model.ActionsUserID {
|
||||
taskID := ctx.opts.ActionsTaskID
|
||||
ctx.user = user_model.NewActionsUserWithTaskID(taskID)
|
||||
@@ -555,7 +533,5 @@ func (ctx *preReceiveContext) loadPusherAndPermission() bool {
|
||||
}
|
||||
ctx.deployKeyAccessMode = deployKey.Mode
|
||||
}
|
||||
|
||||
ctx.loadedPusher = true
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -52,7 +52,6 @@ func TestPreReceiveCanWriteCodePerBranch(t *testing.T) {
|
||||
mockCtx, _ := contexttest.MockPrivateContext(t, "/")
|
||||
ctx := &preReceiveContext{
|
||||
PrivateContext: mockCtx,
|
||||
loadedPusher: true,
|
||||
user: maintainer,
|
||||
userPerm: headPerm,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user