mirror of
https://github.com/go-gitea/gitea.git
synced 2026-03-07 09:51:06 +01:00
Fix non-admins unable to automerge PRs from forks (#36833)
Make `handlePullRequestAutoMerge` correctly check the permissions of the merging user against pr.BaseRepo. --------- Co-authored-by: Michael Hoang <enzime@users.noreply.github.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
Michael Hoang
GitHub
parent
9c2c9c5a00
commit
c710ce34fb
@ -245,9 +245,9 @@ func handlePullRequestAutoMerge(pullID int64, sha string) {
|
||||
return
|
||||
}
|
||||
|
||||
perm, err := access_model.GetUserRepoPermission(ctx, pr.HeadRepo, doer)
|
||||
perm, err := access_model.GetUserRepoPermission(ctx, pr.BaseRepo, doer)
|
||||
if err != nil {
|
||||
log.Error("GetUserRepoPermission %-v: %v", pr.HeadRepo, err)
|
||||
log.Error("GetUserRepoPermission %-v: %v", pr.BaseRepo, err)
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@ -98,7 +98,7 @@ func TestPullMerge(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
hookTasksLenBefore := len(hookTasks)
|
||||
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
|
||||
|
||||
@ -135,7 +135,7 @@ func TestPullRebase(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
hookTasksLenBefore := len(hookTasks)
|
||||
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
|
||||
|
||||
@ -172,7 +172,7 @@ func TestPullRebaseMerge(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
hookTasksLenBefore := len(hookTasks)
|
||||
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
|
||||
|
||||
@ -209,7 +209,7 @@ func TestPullSquash(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
hookTasksLenBefore := len(hookTasks)
|
||||
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited!)\n")
|
||||
@ -235,7 +235,7 @@ func TestPullSquashWithHeadCommitID(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
hookTasksLenBefore := len(hookTasks)
|
||||
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited!)\n")
|
||||
@ -275,7 +275,7 @@ func TestPullSquashWithHeadCommitID(t *testing.T) {
|
||||
|
||||
func TestPullCleanUpAfterMerge(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited - TestPullCleanUpAfterMerge)\n")
|
||||
|
||||
@ -326,7 +326,7 @@ func TestPullCleanUpAfterMerge(t *testing.T) {
|
||||
|
||||
func TestCantMergeWorkInProgress(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
|
||||
|
||||
@ -345,7 +345,7 @@ func TestCantMergeWorkInProgress(t *testing.T) {
|
||||
|
||||
func TestCantMergeConflict(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "conflict", "README.md", "Hello, World (Edited Once)\n")
|
||||
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "base", "README.md", "Hello, World (Edited Twice)\n")
|
||||
@ -387,7 +387,7 @@ func TestCantMergeConflict(t *testing.T) {
|
||||
|
||||
func TestCantMergeUnrelated(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "base", "README.md", "Hello, World (Edited Twice)\n")
|
||||
|
||||
@ -479,7 +479,7 @@ func TestCantMergeUnrelated(t *testing.T) {
|
||||
|
||||
func TestFastForwardOnlyMerge(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "update", "README.md", "Hello, World 2\n")
|
||||
|
||||
@ -514,7 +514,7 @@ func TestFastForwardOnlyMerge(t *testing.T) {
|
||||
|
||||
func TestCantFastForwardOnlyMergeDiverging(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "diverging", "README.md", "Hello, World diverged\n")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World 2\n")
|
||||
@ -631,7 +631,7 @@ func TestConflictChecking(t *testing.T) {
|
||||
|
||||
func TestPullRetargetChildOnBranchDelete(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testEditFileToNewBranch(t, session, "user2", "repo1", "master", "base-pr", "README.md", "Hello, World\n(Edited - TestPullRetargetOnCleanup - base PR)\n")
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFileToNewBranch(t, session, "user1", "repo1", "base-pr", "child-pr", "README.md", "Hello, World\n(Edited - TestPullRetargetOnCleanup - base PR)\n(Edited - TestPullRetargetOnCleanup - child PR)")
|
||||
@ -668,7 +668,7 @@ func TestPullRetargetChildOnBranchDelete(t *testing.T) {
|
||||
|
||||
func TestPullDontRetargetChildOnWrongRepo(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "base-pr", "README.md", "Hello, World\n(Edited - TestPullDontRetargetChildOnWrongRepo - base PR)\n")
|
||||
testEditFileToNewBranch(t, session, "user1", "repo1", "base-pr", "child-pr", "README.md", "Hello, World\n(Edited - TestPullDontRetargetChildOnWrongRepo - base PR)\n(Edited - TestPullDontRetargetChildOnWrongRepo - child PR)")
|
||||
@ -733,7 +733,7 @@ func TestPullRequestMergedWithNoPermissionDeleteBranch(t *testing.T) {
|
||||
func TestPullMergeIndexerNotifier(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
// create a pull request
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
|
||||
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
|
||||
createPullResp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "Indexer notifier test pull")
|
||||
@ -793,27 +793,13 @@ func TestPullMergeIndexerNotifier(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func testResetRepo(t *testing.T, repo *repo_model.Repository, branch, commitID string) {
|
||||
assert.NoError(t, gitrepo.UpdateRef(t.Context(), repo, git.BranchPrefix+branch, commitID))
|
||||
|
||||
gitRepo, err := gitrepo.OpenRepository(t.Context(), repo)
|
||||
assert.NoError(t, err)
|
||||
defer gitRepo.Close()
|
||||
id, err := gitRepo.GetBranchCommitID(branch)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, commitID, id)
|
||||
}
|
||||
|
||||
func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
// create a pull request
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
|
||||
forkedName := "repo1-1"
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", forkedName, "")
|
||||
defer func() {
|
||||
testDeleteRepository(t, session, "user1", forkedName)
|
||||
}()
|
||||
testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n")
|
||||
testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull")
|
||||
|
||||
@ -867,16 +853,10 @@ func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
sha, err := baseGitRepo.GetRefCommitID(pr.GetGitHeadRefName())
|
||||
assert.NoError(t, err)
|
||||
masterCommitID, err := baseGitRepo.GetBranchCommitID("master")
|
||||
assert.NoError(t, err)
|
||||
|
||||
branches, _, err := baseGitRepo.GetBranchNames(0, 100)
|
||||
assert.NoError(t, err)
|
||||
assert.ElementsMatch(t, []string{"sub-home-md-img-check", "home-md-img-check", "pr-to-update", "branch2", "DefaultBranch", "develop", "feature/1", "master"}, branches)
|
||||
baseGitRepo.Close()
|
||||
defer func() {
|
||||
testResetRepo(t, baseRepo, "master", masterCommitID)
|
||||
}()
|
||||
|
||||
err = commitstatus_service.CreateCommitStatus(t.Context(), baseRepo, user1, sha, &git_model.CommitStatus{
|
||||
State: commitstatus.CommitStatusSuccess,
|
||||
@ -897,18 +877,17 @@ func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) {
|
||||
func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
|
||||
// create a pull request
|
||||
session := loginUser(t, "user1")
|
||||
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
|
||||
forkedName := "repo1-2"
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", forkedName, "")
|
||||
defer func() {
|
||||
testDeleteRepository(t, session, "user1", forkedName)
|
||||
}()
|
||||
testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n")
|
||||
testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull")
|
||||
baseUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||
baseSession := loginUser(t, "user2")
|
||||
forkUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
|
||||
forkSession := loginUser(t, "user5")
|
||||
forkedName := "repo1-fork"
|
||||
testRepoFork(t, forkSession, "user2", "repo1", forkUser.Name, forkedName, "")
|
||||
testEditFile(t, forkSession, forkUser.Name, forkedName, "master", "README.md", "Hello, World (Edited)\n")
|
||||
testPullCreate(t, forkSession, forkUser.Name, forkedName, false, "master", "master", "Indexer notifier test pull")
|
||||
|
||||
baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"})
|
||||
forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: forkedName})
|
||||
forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: forkUser.Name, Name: forkedName})
|
||||
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{
|
||||
BaseRepoID: baseRepo.ID,
|
||||
BaseBranch: "master",
|
||||
@ -924,15 +903,15 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) {
|
||||
"status_check_contexts": "gitea/actions",
|
||||
"required_approvals": "1",
|
||||
})
|
||||
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
baseSession.MakeRequest(t, req, http.StatusSeeOther)
|
||||
|
||||
// first time insert automerge record, return true
|
||||
scheduled, err := automerge.ScheduleAutoMerge(t.Context(), user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
|
||||
scheduled, err := automerge.ScheduleAutoMerge(t.Context(), baseUser, pr, repo_model.MergeStyleMerge, "auto merge test", false)
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, scheduled)
|
||||
|
||||
// second time insert automerge record, return false because it does exist
|
||||
scheduled, err = automerge.ScheduleAutoMerge(t.Context(), user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
|
||||
scheduled, err = automerge.ScheduleAutoMerge(t.Context(), baseUser, pr, repo_model.MergeStyleMerge, "auto merge test", false)
|
||||
assert.Error(t, err)
|
||||
assert.False(t, scheduled)
|
||||
|
||||
@ -946,14 +925,9 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
sha, err := baseGitRepo.GetRefCommitID(pr.GetGitHeadRefName())
|
||||
assert.NoError(t, err)
|
||||
masterCommitID, err := baseGitRepo.GetBranchCommitID("master")
|
||||
assert.NoError(t, err)
|
||||
baseGitRepo.Close()
|
||||
defer func() {
|
||||
testResetRepo(t, baseRepo, "master", masterCommitID)
|
||||
}()
|
||||
|
||||
err = commitstatus_service.CreateCommitStatus(t.Context(), baseRepo, user1, sha, &git_model.CommitStatus{
|
||||
err = commitstatus_service.CreateCommitStatus(t.Context(), baseRepo, baseUser, sha, &git_model.CommitStatus{
|
||||
State: commitstatus.CommitStatusSuccess,
|
||||
TargetURL: "https://gitea.com",
|
||||
Context: "gitea/actions",
|
||||
@ -968,16 +942,13 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) {
|
||||
assert.Empty(t, pr.MergedCommitID)
|
||||
|
||||
// approve the PR from non-author
|
||||
approveSession := loginUser(t, "user2")
|
||||
testSubmitReview(t, approveSession, "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK)
|
||||
testSubmitReview(t, baseSession, "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK)
|
||||
|
||||
time.Sleep(2 * time.Second)
|
||||
|
||||
// reload pr again
|
||||
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
|
||||
assert.True(t, pr.HasMerged)
|
||||
assert.Eventually(t, func() bool {
|
||||
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
|
||||
return pr.HasMerged
|
||||
}, 2*time.Second, 100*time.Millisecond)
|
||||
assert.NotEmpty(t, pr.MergedCommitID)
|
||||
|
||||
unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{PullID: pr.ID})
|
||||
})
|
||||
}
|
||||
@ -1035,7 +1006,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing.
|
||||
HeadBranch: "user2/test/head2",
|
||||
})
|
||||
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
// Change master branch to protected
|
||||
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{
|
||||
"rule_name": "master",
|
||||
@ -1067,13 +1038,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing.
|
||||
assert.NoError(t, err)
|
||||
sha, err := baseGitRepo.GetRefCommitID(pr.GetGitHeadRefName())
|
||||
assert.NoError(t, err)
|
||||
masterCommitID, err := baseGitRepo.GetBranchCommitID("master")
|
||||
assert.NoError(t, err)
|
||||
baseGitRepo.Close()
|
||||
defer func() {
|
||||
testResetRepo(t, baseRepo, "master", masterCommitID)
|
||||
}()
|
||||
|
||||
err = commitstatus_service.CreateCommitStatus(t.Context(), baseRepo, user1, sha, &git_model.CommitStatus{
|
||||
State: commitstatus.CommitStatusSuccess,
|
||||
TargetURL: "https://gitea.com",
|
||||
@ -1087,7 +1052,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing.
|
||||
assert.Empty(t, pr.MergedCommitID)
|
||||
|
||||
// approve the PR from non-author
|
||||
approveSession := loginUser(t, "user1")
|
||||
approveSession := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testSubmitReview(t, approveSession, "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK)
|
||||
|
||||
// reload pr again
|
||||
@ -1102,11 +1067,9 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing.
|
||||
func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, u *url.URL) {
|
||||
// create a pull request
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
forkedName := "repo1-1"
|
||||
testRepoFork(t, session, "user2", "repo1", "user1", forkedName, "")
|
||||
defer testDeleteRepository(t, session, "user1", forkedName)
|
||||
|
||||
testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n")
|
||||
testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull")
|
||||
|
||||
@ -1144,7 +1107,7 @@ func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) {
|
||||
|
||||
func TestPullSquashMergeEmpty(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, u *url.URL) {
|
||||
session := loginUser(t, "user1")
|
||||
session := loginUser(t, "user1") // FIXME: don't use admin user for testing
|
||||
testEditFileToNewBranch(t, session, "user2", "repo1", "master", "pr-squash-empty", "README.md", "Hello, World (Edited)\n")
|
||||
resp := testPullCreate(t, session, "user2", "repo1", false, "master", "pr-squash-empty", "This is a pull title")
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user