diff --git a/services/pull/update.go b/services/pull/update.go index 462bbec55a..c9dbb34ef6 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -100,78 +100,105 @@ 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, forcePushAllowed 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 the given 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) + forcePushAllowed = pushAllowed + // 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 + } + if pb != nil { // override previous results if there is a branch protection rule + pb.Repo = repo + pushAllowed = pb.CanUserPush(ctx, user) + forcePushAllowed = pb.CanUserForcePush(ctx, user) + } + return pushAllowed, forcePushAllowed, nil +} + +// 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 user == nil { + return false, false, nil + } if err := pull.LoadBaseRepo(ctx); err != nil { return false, false, err } + if err := pull.LoadHeadRepo(ctx); err != nil { + return false, false, err + } - // 1. check base repository's AllowRebaseUpdate configuration + // 1. check whether pull request enabled. + 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) + } + + // 2. only support Github style pull request + if pull.Flow == issues_model.PullRequestFlowAGit { + return false, false, nil + } + + // 3. check user push permission on head repository + pushAllowed, rebaseAllowed, err = isUserAllowedToPushOrForcePushInRepoBranch(ctx, user, pull.HeadRepo, pull.HeadBranch) + if err != nil { + return false, false, err + } + + // 4. if the pull creator allows maintainer to edit, we need to check whether + // user is a maintainer (has permission to merge into base branch) and inherit pull request poster's permission + if pull.AllowMaintainerEdit && (!pushAllowed || !rebaseAllowed) { + baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, user) + if err != nil { + return false, false, err + } + userAllowedToMergePR, err := isUserAllowedToMergeInRepoBranch(ctx, pull.BaseRepoID, pull.BaseBranch, baseRepoPerm, user) + if err != nil { + return false, false, err + } + if userAllowedToMergePR { + // the user is maintainer (can merge PR), and this PR is allowed to be edited by maintainers, + // then the user should inherit the PR poster's push/rebase permission for the head branch + 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 + } + } + } + + // 5. 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 - } - - // 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) - } - } - - // 3. check whether user has write access to head branch - baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, user) - if err != nil { - return false, false, err - } - - mergeAllowed, err = isUserAllowedToMergeInRepoBranch(ctx, pull.HeadRepoID, pull.HeadBranch, headRepoPerm, user) - if err != nil { - return false, false, err - } - - // 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) - if err != nil { - return false, false, err - } - - mergeAllowed = mergeAllowed || mergeAllowedMaintainer - } - - // if merge is not allowed, rebase is also not allowed - rebaseAllowed = rebaseAllowed && mergeAllowed - - return mergeAllowed, rebaseAllowed, nil + return pushAllowed, rebaseAllowed && prBaseUnit.PullRequestsConfig().AllowRebaseUpdate, 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 0000000000..4b5772e35d --- /dev/null +++ b/services/pull/update_test.go @@ -0,0 +1,172 @@ +// 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" + "github.com/stretchr/testify/require" +) + +func TestIsUserAllowedToUpdate(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + setRepoAllowRebaseUpdate := func(t *testing.T, repoID int64, allow bool) { + repoUnit := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoUnit{RepoID: repoID, Type: unit.TypePullRequests}) + repoUnit.PullRequestsConfig().AllowRebaseUpdate = allow + require.NoError(t, repo_model.UpdateRepoUnit(t.Context(), repoUnit)) + } + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + t.Run("RespectsProtectedBranch", func(t *testing.T) { + pr2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + protectedBranch := &git_model.ProtectedBranch{ + RepoID: pr2.HeadRepoID, + RuleName: pr2.HeadBranch, + CanPush: false, + CanForcePush: false, + } + _, err := db.GetEngine(t.Context()).Insert(protectedBranch) + require.NoError(t, err) + defer db.DeleteByBean(t.Context(), protectedBranch) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr2, user2) + assert.NoError(t, err) + assert.False(t, pushAllowed) + assert.False(t, rebaseAllowed) + }) + + t.Run("DisallowRebaseWhenConfigDisabled", func(t *testing.T) { + pr2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + setRepoAllowRebaseUpdate(t, pr2.BaseRepoID, false) + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr2, user2) + assert.NoError(t, err) + assert.True(t, pushAllowed) + assert.False(t, rebaseAllowed) + }) + + t.Run("ReadOnlyAccessDenied", func(t *testing.T) { + pr2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + + collaboration := &repo_model.Collaboration{ + RepoID: pr2.HeadRepoID, + UserID: user4.ID, + Mode: perm.AccessModeRead, + } + require.NoError(t, db.Insert(t.Context(), collaboration)) + defer db.DeleteByBean(t.Context(), collaboration) + + require.NoError(t, pr2.LoadHeadRepo(t.Context())) + assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr2.HeadRepo, user4.ID)) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr2, user4) + assert.NoError(t, err) + assert.False(t, pushAllowed) + assert.False(t, rebaseAllowed) + }) + + t.Run("ProtectedBranchAllowsPushWithoutRebase", func(t *testing.T) { + pr2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + protectedBranch := &git_model.ProtectedBranch{ + RepoID: pr2.HeadRepoID, + RuleName: pr2.HeadBranch, + CanPush: true, + CanForcePush: false, + } + _, err := db.GetEngine(t.Context()).Insert(protectedBranch) + require.NoError(t, err) + defer db.DeleteByBean(t.Context(), protectedBranch) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr2, user2) + assert.NoError(t, err) + assert.True(t, pushAllowed) + assert.False(t, rebaseAllowed) + }) + + pr3Poster := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 12}) + + t.Run("MaintainerEditRespectsPosterPermissions", func(t *testing.T) { + pr3 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) + pr3.AllowMaintainerEdit = true + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr3, pr3Poster) + assert.NoError(t, err) + assert.False(t, pushAllowed) + assert.False(t, rebaseAllowed) + }) + + t.Run("MaintainerEditInheritsPosterPermissions", func(t *testing.T) { + pr3 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) + pr3.AllowMaintainerEdit = true + protectedBranch := &git_model.ProtectedBranch{ + RepoID: pr3.HeadRepoID, + RuleName: pr3.HeadBranch, + CanPush: true, + CanForcePush: true, + } + _, err := db.GetEngine(t.Context()).Insert(protectedBranch) + require.NoError(t, err) + defer db.DeleteByBean(t.Context(), protectedBranch) + + collaboration := &repo_model.Collaboration{ + RepoID: pr3.HeadRepoID, + UserID: pr3Poster.ID, + Mode: perm.AccessModeWrite, + } + require.NoError(t, db.Insert(t.Context(), collaboration)) + defer db.DeleteByBean(t.Context(), collaboration) + + require.NoError(t, pr3.LoadHeadRepo(t.Context())) + assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr3.HeadRepo, pr3Poster.ID)) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr3, pr3Poster) + assert.NoError(t, err) + assert.True(t, pushAllowed) + assert.True(t, rebaseAllowed) + }) + + t.Run("MaintainerEditInheritsPosterPermissionsRebaseDisabled", func(t *testing.T) { + pr3 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) + pr3.AllowMaintainerEdit = true + protectedBranch := &git_model.ProtectedBranch{ + RepoID: pr3.HeadRepoID, + RuleName: pr3.HeadBranch, + CanPush: true, + CanForcePush: true, + } + _, err := db.GetEngine(t.Context()).Insert(protectedBranch) + require.NoError(t, err) + defer db.DeleteByBean(t.Context(), protectedBranch) + + collaboration := &repo_model.Collaboration{ + RepoID: pr3.HeadRepoID, + UserID: pr3Poster.ID, + Mode: perm.AccessModeWrite, + } + require.NoError(t, db.Insert(t.Context(), collaboration)) + defer db.DeleteByBean(t.Context(), collaboration) + + require.NoError(t, pr3.LoadHeadRepo(t.Context())) + assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr3.HeadRepo, pr3Poster.ID)) + + setRepoAllowRebaseUpdate(t, pr3.BaseRepoID, false) + + pushAllowed, rebaseAllowed, err := IsUserAllowedToUpdate(t.Context(), pr3, pr3Poster) + assert.NoError(t, err) + assert.True(t, pushAllowed) + assert.False(t, rebaseAllowed) + }) +} diff --git a/tests/integration/pull_update_test.go b/tests/integration/pull_update_test.go index 46b3769b79..a873d50561 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",