diff --git a/services/pull/update.go b/services/pull/update.go index 462bbec55afa4..47a410032915a 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -100,78 +100,98 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model. return err } -// IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections -// update PR means send new commits to PR head branch from base branch -func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest, user *user_model.User) (mergeAllowed, rebaseAllowed bool, err error) { - if pull.Flow == issues_model.PullRequestFlowAGit { - return false, false, nil - } +// isUserAllowedToPushOrForcePushInRepoBranch checks whether user is allowed to push or force push in the given repo and branch +// it will check both user permission and branch protection rules +func isUserAllowedToPushOrForcePushInRepoBranch(ctx context.Context, user *user_model.User, repo *repo_model.Repository, branch string) (pushAllowed, rebaseAllowed bool, err error) { if user == nil { return false, false, nil } - headRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, user) + + // 1. check user push permission on head repository + repoPerm, err := access_model.GetUserRepoPermission(ctx, repo, user) if err != nil { if repo_model.IsErrUnitTypeNotExist(err) { return false, false, nil } return false, false, err } + pushAllowed = repoPerm.CanWrite(unit.TypeCode) + rebaseAllowed = pushAllowed - if err := pull.LoadBaseRepo(ctx); err != nil { + // 2. check branch protection whether user can push or force push + pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branch) + if err != nil { return false, false, err } - - // 1. check base repository's AllowRebaseUpdate configuration - // it is a config in base repo but controls the head (fork) repo's "Update" behavior - { - prBaseUnit, err := pull.BaseRepo.GetUnit(ctx, unit.TypePullRequests) - if repo_model.IsErrUnitTypeNotExist(err) { - return false, false, nil // the PR unit is disabled in base repo - } else if err != nil { - return false, false, fmt.Errorf("get base repo unit: %v", err) - } - rebaseAllowed = prBaseUnit.PullRequestsConfig().AllowRebaseUpdate + if pb != nil { // override previous results if there is a branch protection rule + pb.Repo = repo + pushAllowed = pb.CanUserPush(ctx, user) + rebaseAllowed = pb.CanUserForcePush(ctx, user) } + return pushAllowed, rebaseAllowed, nil +} - // 2. check head branch protection whether rebase is allowed, if pb not found then rebase depends on the above setting - { - pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.HeadRepoID, pull.HeadBranch) - if err != nil { - return false, false, err - } - // If branch protected, disable rebase unless user is whitelisted to force push (which extends regular push) - if pb != nil { - pb.Repo = pull.HeadRepo - rebaseAllowed = rebaseAllowed && pb.CanUserForcePush(ctx, user) - } +// IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections +// update PR means send new commits to PR head branch from base branch +func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest, user *user_model.User) (pushAllowed, rebaseAllowed bool, err error) { + if pull.Flow == issues_model.PullRequestFlowAGit { + return false, false, nil + } + if user == nil { + return false, false, nil } - // 3. check whether user has write access to head branch - baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, user) + // 1. check user push permission on head repository + pushAllowed, rebaseAllowed, err = isUserAllowedToPushOrForcePushInRepoBranch(ctx, user, pull.HeadRepo, pull.HeadBranch) if err != nil { return false, false, err } - mergeAllowed, err = isUserAllowedToMergeInRepoBranch(ctx, pull.HeadRepoID, pull.HeadBranch, headRepoPerm, user) - if err != nil { + // 2. check base repository's AllowRebaseUpdate configuration + // it is a config in base repo but controls the head (fork) repo's "Update" behavior + if err := pull.LoadBaseRepo(ctx); err != nil { return false, false, err } + prBaseUnit, err := pull.BaseRepo.GetUnit(ctx, unit.TypePullRequests) + if repo_model.IsErrUnitTypeNotExist(err) { + return false, false, nil // the PR unit is disabled in base repo means no update allowed + } else if err != nil { + return false, false, fmt.Errorf("get base repo unit: %v", err) + } + rebaseAllowed = rebaseAllowed && prBaseUnit.PullRequestsConfig().AllowRebaseUpdate - // 4. if the pull creator allows maintainer to edit, it means the write permissions of the head branch has been - // granted to the user with write permission of the base repository - if pull.AllowMaintainerEdit { - mergeAllowedMaintainer, err := isUserAllowedToMergeInRepoBranch(ctx, pull.BaseRepoID, pull.BaseBranch, baseRepoPerm, user) + // 3. if the pull creator allows maintainer to edit, we needs to check whether + // user is a maintainer and inherit pull request creator's permission + if pull.AllowMaintainerEdit && (!pushAllowed || !rebaseAllowed) { + baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, user) if err != nil { return false, false, err } - - mergeAllowed = mergeAllowed || mergeAllowedMaintainer + userAllowedToMergePR, err := isUserAllowedToMergeInRepoBranch(ctx, pull.BaseRepoID, pull.BaseBranch, baseRepoPerm, user) + if err != nil { + return false, false, err + } + if userAllowedToMergePR { // if user is maintainer, then it can inherit the poster's push/rebase permission + if err := pull.LoadIssue(ctx); err != nil { + return false, false, err + } + if err := pull.Issue.LoadPoster(ctx); err != nil { + return false, false, err + } + posterPushAllowed, posterRebaseAllowed, err := isUserAllowedToPushOrForcePushInRepoBranch(ctx, pull.Issue.Poster, pull.HeadRepo, pull.HeadBranch) + if err != nil { + return false, false, err + } + if !pushAllowed { + pushAllowed = posterPushAllowed + } + if !rebaseAllowed { + rebaseAllowed = posterRebaseAllowed + } + } } - // if merge is not allowed, rebase is also not allowed - rebaseAllowed = rebaseAllowed && mergeAllowed - - return mergeAllowed, rebaseAllowed, nil + return pushAllowed, rebaseAllowed, nil } func syncCommitDivergence(ctx context.Context, pr *issues_model.PullRequest) error { diff --git a/services/pull/update_test.go b/services/pull/update_test.go new file mode 100644 index 0000000000000..7b752fb24c506 --- /dev/null +++ b/services/pull/update_test.go @@ -0,0 +1,161 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" + issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/perm" + access_model "code.gitea.io/gitea/models/perm/access" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" +) + +func setRepoAllowRebaseUpdate(t *testing.T, repoID int64, allow bool) { + t.Helper() + + repoUnit := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoUnit{RepoID: repoID, Type: unit.TypePullRequests}) + repoUnit.PullRequestsConfig().AllowRebaseUpdate = allow + assert.NoError(t, repo_model.UpdateRepoUnit(t.Context(), repoUnit)) +} + +func TestIsUserAllowedToUpdateRespectsProtectedBranch(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pr.LoadHeadRepo(t.Context())) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + protectedBranch := &git_model.ProtectedBranch{ + RepoID: pr.HeadRepoID, + RuleName: pr.HeadBranch, + CanPush: false, + CanForcePush: false, + } + _, err := db.GetEngine(t.Context()).Insert(protectedBranch) + assert.NoError(t, err) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr, user) + assert.NoError(t, err) + assert.False(t, pushAllowed) + assert.False(t, rebaseAllowed) +} + +func TestIsUserAllowedToUpdateDisablesRebaseWhenConfigDisabled(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pr.LoadHeadRepo(t.Context())) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + setRepoAllowRebaseUpdate(t, pr.BaseRepoID, false) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr, user) + assert.NoError(t, err) + assert.True(t, pushAllowed) + assert.False(t, rebaseAllowed) +} + +func TestIsUserAllowedToUpdateReadOnlyAccessDenied(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pr.LoadHeadRepo(t.Context())) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + + assert.NoError(t, db.Insert(t.Context(), &repo_model.Collaboration{ + RepoID: pr.HeadRepoID, + UserID: user.ID, + Mode: perm.AccessModeRead, + })) + assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr.HeadRepo, user.ID)) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr, user) + assert.NoError(t, err) + assert.False(t, pushAllowed) + assert.False(t, rebaseAllowed) +} + +func TestIsUserAllowedToUpdateProtectedBranchAllowsPushWithoutRebase(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pr.LoadHeadRepo(t.Context())) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + protectedBranch := &git_model.ProtectedBranch{ + RepoID: pr.HeadRepoID, + RuleName: pr.HeadBranch, + CanPush: true, + CanForcePush: false, + } + _, err := db.GetEngine(t.Context()).Insert(protectedBranch) + assert.NoError(t, err) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr, user) + assert.NoError(t, err) + assert.True(t, pushAllowed) + assert.False(t, rebaseAllowed) +} + +func TestIsUserAllowedToUpdateMaintainerEditRespectsPosterPermissions(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) + pr.AllowMaintainerEdit = true + assert.NoError(t, pr.LoadHeadRepo(t.Context())) + assert.NoError(t, pr.LoadIssue(t.Context())) + assert.NoError(t, pr.Issue.LoadPoster(t.Context())) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 12}) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr, user) + assert.NoError(t, err) + assert.False(t, pushAllowed) + assert.False(t, rebaseAllowed) +} + +func TestIsUserAllowedToUpdateMaintainerEditInheritsPosterPermissions(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) + pr.AllowMaintainerEdit = true + assert.NoError(t, pr.LoadHeadRepo(t.Context())) + assert.NoError(t, pr.LoadIssue(t.Context())) + assert.NoError(t, pr.Issue.LoadPoster(t.Context())) + + protectedBranch := &git_model.ProtectedBranch{ + RepoID: pr.HeadRepoID, + RuleName: pr.HeadBranch, + CanPush: true, + CanForcePush: true, + } + _, err := db.GetEngine(t.Context()).Insert(protectedBranch) + assert.NoError(t, err) + + assert.NoError(t, db.Insert(t.Context(), &repo_model.Collaboration{ + RepoID: pr.HeadRepoID, + UserID: pr.Issue.Poster.ID, + Mode: perm.AccessModeWrite, + })) + assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr.HeadRepo, pr.Issue.Poster.ID)) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 12}) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr, user) + assert.NoError(t, err) + assert.True(t, pushAllowed) + assert.True(t, rebaseAllowed) +} diff --git a/tests/integration/pull_update_test.go b/tests/integration/pull_update_test.go index 46b3769b79ec0..a873d50561347 100644 --- a/tests/integration/pull_update_test.go +++ b/tests/integration/pull_update_test.go @@ -11,7 +11,6 @@ import ( "time" auth_model "code.gitea.io/gitea/models/auth" - git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" @@ -121,49 +120,6 @@ func TestAPIPullUpdateByRebase(t *testing.T) { }) } -func TestAPIPullUpdateByRebase2(t *testing.T) { - onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - // Create PR to test - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - org26 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 26}) - pr := createOutdatedPR(t, user, org26) - assert.NoError(t, pr.LoadBaseRepo(t.Context())) - assert.NoError(t, pr.LoadIssue(t.Context())) - - enableRepoAllowUpdateWithRebase(t, pr.BaseRepo.ID, false) - - session := loginUser(t, "user2") - token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - req := NewRequestf(t, "POST", "/api/v1/repos/%s/%s/pulls/%d/update?style=rebase", pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index). - AddTokenAuth(token) - session.MakeRequest(t, req, http.StatusForbidden) - - enableRepoAllowUpdateWithRebase(t, pr.BaseRepo.ID, true) - assert.NoError(t, pr.LoadHeadRepo(t.Context())) - - // add a protected branch rule to the head branch to block rebase - pb := git_model.ProtectedBranch{ - RepoID: pr.HeadRepo.ID, - RuleName: pr.HeadBranch, - CanPush: false, - CanForcePush: false, - } - err := git_model.UpdateProtectBranch(t.Context(), pr.HeadRepo, &pb, git_model.WhitelistOptions{}) - assert.NoError(t, err) - req = NewRequestf(t, "POST", "/api/v1/repos/%s/%s/pulls/%d/update?style=rebase", pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index). - AddTokenAuth(token) - session.MakeRequest(t, req, http.StatusForbidden) - - // remove the protected branch rule to allow rebase - err = git_model.DeleteProtectedBranch(t.Context(), pr.HeadRepo, pb.ID) - assert.NoError(t, err) - - req = NewRequestf(t, "POST", "/api/v1/repos/%s/%s/pulls/%d/update?style=rebase", pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index). - AddTokenAuth(token) - session.MakeRequest(t, req, http.StatusOK) - }) -} - func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_model.PullRequest { baseRepo, err := repo_service.CreateRepository(t.Context(), actor, actor, repo_service.CreateRepoOptions{ Name: "repo-pr-update",