mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-01 20:59:45 +09:00
f44e64be81
Fixes several gaps in the approval of fork pull request runs: 1. Approving a run that was cancelled while awaiting approval revived its cancelled jobs. Such a run is no longer treated as awaiting approval by the merge box, run page, approve actions and API, and rerunning it approves it. 2. Approval no longer revives jobs cancelled while the run was pending, no longer lets two jobs sharing a concurrency group cancel each other, and re-emits the run so jobs needing a cancelled job get resolved. 3. An unapproved run applies its workflow-level concurrency only once approved. 4. For workflows from the pull request, both the event actor and the pull request author must be trusted to skip approval. Workflows from the default branch, like `issue_comment`, still only check the actor. --------- Co-authored-by: silverwind <me@silverwind.io>
145 lines
6.0 KiB
Go
145 lines
6.0 KiB
Go
// Copyright 2026 The Gitea Authors. All rights reserved.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package actions
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"testing"
|
|
|
|
actions_model "gitea.dev/models/actions"
|
|
issues_model "gitea.dev/models/issues"
|
|
repo_model "gitea.dev/models/repo"
|
|
"gitea.dev/models/unittest"
|
|
user_model "gitea.dev/models/user"
|
|
actions_module "gitea.dev/modules/actions"
|
|
"gitea.dev/modules/actions/jobparser"
|
|
webhook_module "gitea.dev/modules/webhook"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
func TestIfNeedApproval(t *testing.T) {
|
|
alwaysWrite := func(_ context.Context, _ *repo_model.Repository, _ *user_model.User) (bool, error) {
|
|
return true, nil
|
|
}
|
|
neverWrite := func(_ context.Context, _ *repo_model.Repository, _ *user_model.User) (bool, error) {
|
|
return false, nil
|
|
}
|
|
hasMerged := func(_ context.Context, _, _ int64) (bool, error) { return true, nil }
|
|
noMerged := func(_ context.Context, _, _ int64) (bool, error) { return false, nil }
|
|
errPerm := errors.New("perm error")
|
|
errMerge := errors.New("merge error")
|
|
|
|
forkRun := &actions_model.ActionRun{IsForkPullRequest: true, TriggerEvent: actions_module.GithubEventPullRequest}
|
|
nonForkRun := &actions_model.ActionRun{IsForkPullRequest: false, TriggerEvent: actions_module.GithubEventPullRequest}
|
|
prTargetRun := &actions_model.ActionRun{IsForkPullRequest: true, TriggerEvent: actions_module.GithubEventPullRequestTarget}
|
|
|
|
repo := &repo_model.Repository{ID: 1}
|
|
normalUser := &user_model.User{ID: 10}
|
|
restrictedUser := &user_model.User{ID: 11, IsRestricted: true}
|
|
|
|
t.Run("not a fork PR never needs approval", func(t *testing.T) {
|
|
need, err := ifNeedApprovalWith(t.Context(), nonForkRun, repo, normalUser, alwaysWrite, hasMerged)
|
|
require.NoError(t, err)
|
|
assert.False(t, need)
|
|
})
|
|
|
|
t.Run("pull_request_target never needs approval even when fork", func(t *testing.T) {
|
|
need, err := ifNeedApprovalWith(t.Context(), prTargetRun, repo, normalUser, alwaysWrite, hasMerged)
|
|
require.NoError(t, err)
|
|
assert.False(t, need)
|
|
})
|
|
|
|
t.Run("restricted user always needs approval", func(t *testing.T) {
|
|
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, restrictedUser, alwaysWrite, hasMerged)
|
|
require.NoError(t, err)
|
|
assert.True(t, need)
|
|
})
|
|
|
|
t.Run("fork PR with write permission does not need approval", func(t *testing.T) {
|
|
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, alwaysWrite, noMerged)
|
|
require.NoError(t, err)
|
|
assert.False(t, need)
|
|
})
|
|
|
|
t.Run("fork PR with merged PR but no write permission does not need approval", func(t *testing.T) {
|
|
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, neverWrite, hasMerged)
|
|
require.NoError(t, err)
|
|
assert.False(t, need)
|
|
})
|
|
|
|
t.Run("fork PR with no write and no merged PR needs approval", func(t *testing.T) {
|
|
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, neverWrite, noMerged)
|
|
require.NoError(t, err)
|
|
assert.True(t, need)
|
|
})
|
|
|
|
t.Run("canWriteActions error is propagated", func(t *testing.T) {
|
|
failWrite := func(_ context.Context, _ *repo_model.Repository, _ *user_model.User) (bool, error) {
|
|
return false, errPerm
|
|
}
|
|
_, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, failWrite, noMerged)
|
|
require.ErrorIs(t, err, errPerm)
|
|
})
|
|
|
|
t.Run("hasMergedPR error is propagated", func(t *testing.T) {
|
|
failMerge := func(_ context.Context, _, _ int64) (bool, error) { return false, errMerge }
|
|
_, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, neverWrite, failMerge)
|
|
require.ErrorIs(t, err, errMerge)
|
|
})
|
|
|
|
t.Run("restricted user skips permission check entirely", func(t *testing.T) {
|
|
// The perm and merge functions must not be called for a restricted user.
|
|
called := false
|
|
trackWrite := func(_ context.Context, _ *repo_model.Repository, _ *user_model.User) (bool, error) {
|
|
called = true
|
|
return true, nil
|
|
}
|
|
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, restrictedUser, trackWrite, noMerged)
|
|
require.NoError(t, err)
|
|
assert.True(t, need)
|
|
assert.False(t, called, "permission check must not run for restricted user")
|
|
})
|
|
}
|
|
|
|
func TestGetApprovalUsersAddsForkPullRequestAuthorUnlessDefaultBranchWorkflow(t *testing.T) {
|
|
require.NoError(t, unittest.PrepareTestDatabase())
|
|
|
|
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1})
|
|
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
|
|
|
approvalUsers, err := getApprovalUsers(t.Context(), ¬ifyInput{Doer: doer, PullRequest: pr, Event: webhook_module.HookEventPullRequest}, true)
|
|
require.NoError(t, err)
|
|
require.Len(t, approvalUsers, 2)
|
|
assert.Equal(t, []int64{doer.ID, pr.Issue.PosterID}, []int64{approvalUsers[0].ID, approvalUsers[1].ID})
|
|
|
|
approvalUsers, err = getApprovalUsers(t.Context(), ¬ifyInput{Doer: doer, PullRequest: pr, Event: webhook_module.HookEventIssueComment}, true)
|
|
require.NoError(t, err)
|
|
assert.Equal(t, []*user_model.User{doer}, approvalUsers)
|
|
}
|
|
|
|
func TestFilteredWorkflowCommitStatusForForkPullRequest(t *testing.T) {
|
|
forkPR := &issues_model.PullRequest{
|
|
Flow: issues_model.PullRequestFlowGithub,
|
|
BaseRepoID: 1,
|
|
HeadRepoID: 2,
|
|
}
|
|
input := newPullRequestReviewNotifyInput(&repo_model.Repository{ID: 1}, &user_model.User{ID: 2}, actions_module.GithubEventPullRequest, "refs/pull/1/head", forkPR)
|
|
|
|
assert.True(t, isForkPullRequestInput(input))
|
|
assert.Equal(t, "refs/pull/1/head", input.Ref.String())
|
|
assert.False(t, shouldCreateSkippedCommitStatusForFilteredWorkflow(input, &actions_module.DetectedWorkflow{
|
|
TriggerEvent: &jobparser.Event{Name: actions_module.GithubEventPullRequest},
|
|
}))
|
|
assert.True(t, shouldCreateSkippedCommitStatusForFilteredWorkflow(input, &actions_module.DetectedWorkflow{
|
|
TriggerEvent: &jobparser.Event{Name: actions_module.GithubEventPullRequestTarget},
|
|
}))
|
|
|
|
assert.True(t, shouldCreateSkippedCommitStatusForFilteredWorkflow(newNotifyInput(&repo_model.Repository{ID: 1}, &user_model.User{ID: 2}, actions_module.GithubEventPullRequest), &actions_module.DetectedWorkflow{
|
|
TriggerEvent: &jobparser.Event{Name: actions_module.GithubEventPullRequest},
|
|
}))
|
|
}
|