Skip to content

Fix non-admins unable to automerge PRs from forks #36833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wxiaoguang merged 5 commits into from
Mar 6, 2026
Merged

Fix non-admins unable to automerge PRs from forks #36833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2783107
Test non-admins can automerge PRs from forks
Enzime Mar 5, 2026
75f4e11
Fix non-admins unable to automerge PRs from forks
Enzime Mar 5, 2026
3dcdd11
fix test
wxiaoguang Mar 5, 2026
13ed519
Merge branch 'main' into push-rmlwrooyqnro
lunny Mar 5, 2026
e4e3b9d
Merge branch 'main' into push-rmlwrooyqnro
wxiaoguang Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions services/automerge/automerge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
111 changes: 37 additions & 74 deletions tests/integration/pull_merge_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand All @@ -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")
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)")
Expand Down Expand Up @@ -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)")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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,
Expand All @@ -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",
Expand All @@ -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)

Expand All @@ -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",
Expand All @@ -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)

time.Sleep(2 * time.Second)
testSubmitReview(t, baseSession, "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK)

// 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})
})
}
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand All @@ -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")

Expand Down Expand Up @@ -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")

Expand Down