Browse Source

Fix the bug when getting files changed for `pull_request_target` event (#26320)

Follow #25229

Copy from
https://github.com/go-gitea/gitea/pull/26290#issuecomment-1663135186

The bug is that we cannot get changed files for the
`pull_request_target` event. This event runs in the context of the base
branch, so we won't get any changes if we call
`GetFilesChangedSinceCommit` with `PullRequest.Base.Ref`.
tags/v1.21.0-rc0
Zettat123 10 months ago
parent
commit
9a8af92577
No account linked to committer's email address

+ 21
- 9
modules/actions/workflows.go View File

return events, nil return events, nil
} }


func DetectWorkflows(commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader) ([]*DetectedWorkflow, error) {
func DetectWorkflows(gitRepo *git.Repository, commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader) ([]*DetectedWorkflow, error) {
entries, err := ListWorkflows(commit) entries, err := ListWorkflows(commit)
if err != nil { if err != nil {
return nil, err return nil, err
} }
for _, evt := range events { for _, evt := range events {
log.Trace("detect workflow %q for event %#v matching %q", entry.Name(), evt, triggedEvent) log.Trace("detect workflow %q for event %#v matching %q", entry.Name(), evt, triggedEvent)
if detectMatched(commit, triggedEvent, payload, evt) {
if detectMatched(gitRepo, commit, triggedEvent, payload, evt) {
dwf := &DetectedWorkflow{ dwf := &DetectedWorkflow{
EntryName: entry.Name(), EntryName: entry.Name(),
TriggerEvent: evt.Name, TriggerEvent: evt.Name,
return workflows, nil return workflows, nil
} }


func detectMatched(commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader, evt *jobparser.Event) bool {
func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader, evt *jobparser.Event) bool {
if !canGithubEventMatch(evt.Name, triggedEvent) { if !canGithubEventMatch(evt.Name, triggedEvent) {
return false return false
} }
webhook_module.HookEventPullRequestSync, webhook_module.HookEventPullRequestSync,
webhook_module.HookEventPullRequestAssign, webhook_module.HookEventPullRequestAssign,
webhook_module.HookEventPullRequestLabel: webhook_module.HookEventPullRequestLabel:
return matchPullRequestEvent(commit, payload.(*api.PullRequestPayload), evt)
return matchPullRequestEvent(gitRepo, commit, payload.(*api.PullRequestPayload), evt)


case // pull_request_review case // pull_request_review
webhook_module.HookEventPullRequestReviewApproved, webhook_module.HookEventPullRequestReviewApproved,
return matchTimes == len(evt.Acts()) return matchTimes == len(evt.Acts())
} }


func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
acts := evt.Acts() acts := evt.Acts()
activityTypeMatched := false activityTypeMatched := false
matchTimes := 0 matchTimes := 0
} }
} }


var (
headCommit = commit
err error
)
if evt.Name == GithubEventPullRequestTarget && (len(acts["paths"]) > 0 || len(acts["paths-ignore"]) > 0) {
headCommit, err = gitRepo.GetCommit(prPayload.PullRequest.Head.Sha)
if err != nil {
log.Error("GetCommit [ref: %s]: %v", prPayload.PullRequest.Head.Sha, err)
return false
}
}

// all acts conditions should be satisfied // all acts conditions should be satisfied
for cond, vals := range acts { for cond, vals := range acts {
switch cond { switch cond {
matchTimes++ matchTimes++
} }
case "paths": case "paths":
filesChanged, err := commit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref)
filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref)
if err != nil { if err != nil {
log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", commit.ID.String(), err)
log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", headCommit.ID.String(), err)
} else { } else {
patterns, err := workflowpattern.CompilePatterns(vals...) patterns, err := workflowpattern.CompilePatterns(vals...)
if err != nil { if err != nil {
} }
} }
case "paths-ignore": case "paths-ignore":
filesChanged, err := commit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref)
filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref)
if err != nil { if err != nil {
log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", commit.ID.String(), err)
log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", headCommit.ID.String(), err)
} else { } else {
patterns, err := workflowpattern.CompilePatterns(vals...) patterns, err := workflowpattern.CompilePatterns(vals...)
if err != nil { if err != nil {

+ 1
- 1
modules/actions/workflows_test.go View File

evts, err := GetEventsFromContent([]byte(tc.yamlOn)) evts, err := GetEventsFromContent([]byte(tc.yamlOn))
assert.NoError(t, err) assert.NoError(t, err)
assert.Len(t, evts, 1) assert.Len(t, evts, 1)
assert.Equal(t, tc.expected, detectMatched(tc.commit, tc.triggedEvent, tc.payload, evts[0]))
assert.Equal(t, tc.expected, detectMatched(nil, tc.commit, tc.triggedEvent, tc.payload, evts[0]))
}) })
} }
} }

+ 2
- 2
services/actions/notifier_helper.go View File

} }


var detectedWorkflows []*actions_module.DetectedWorkflow var detectedWorkflows []*actions_module.DetectedWorkflow
workflows, err := actions_module.DetectWorkflows(commit, input.Event, input.Payload)
workflows, err := actions_module.DetectWorkflows(gitRepo, commit, input.Event, input.Payload)
if err != nil { if err != nil {
return fmt.Errorf("DetectWorkflows: %w", err) return fmt.Errorf("DetectWorkflows: %w", err)
} }
if err != nil { if err != nil {
return fmt.Errorf("gitRepo.GetCommit: %w", err) return fmt.Errorf("gitRepo.GetCommit: %w", err)
} }
baseWorkflows, err := actions_module.DetectWorkflows(baseCommit, input.Event, input.Payload)
baseWorkflows, err := actions_module.DetectWorkflows(gitRepo, baseCommit, input.Event, input.Payload)
if err != nil { if err != nil {
return fmt.Errorf("DetectWorkflows: %w", err) return fmt.Errorf("DetectWorkflows: %w", err)
} }

+ 53
- 1
tests/integration/actions_trigger_test.go View File

{ {
Operation: "create", Operation: "create",
TreePath: ".gitea/workflows/pr.yml", TreePath: ".gitea/workflows/pr.yml",
ContentReader: strings.NewReader("name: test\non: pull_request_target\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: echo helloworld\n"),
ContentReader: strings.NewReader("name: test\non:\n pull_request_target:\n paths:\n - 'file_*.txt'\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: echo helloworld\n"),
}, },
}, },
Message: "add workflow", Message: "add workflow",
assert.NoError(t, err) assert.NoError(t, err)


// load and compare ActionRun // load and compare ActionRun
assert.Equal(t, 1, unittest.GetCount(t, &actions_model.ActionRun{RepoID: baseRepo.ID}))
actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID}) actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID})
assert.Equal(t, addFileToForkedResp.Commit.SHA, actionRun.CommitSHA) assert.Equal(t, addFileToForkedResp.Commit.SHA, actionRun.CommitSHA)
assert.Equal(t, actions_module.GithubEventPullRequestTarget, actionRun.TriggerEvent) assert.Equal(t, actions_module.GithubEventPullRequestTarget, actionRun.TriggerEvent)

// add another file whose name cannot match the specified path
addFileToForkedResp, err = files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, user3, &files_service.ChangeRepoFilesOptions{
Files: []*files_service.ChangeRepoFile{
{
Operation: "create",
TreePath: "foo.txt",
ContentReader: strings.NewReader("foo"),
},
},
Message: "add foo.txt",
OldBranch: "main",
NewBranch: "fork-branch-2",
Author: &files_service.IdentityOptions{
Name: user3.Name,
Email: user3.Email,
},
Committer: &files_service.IdentityOptions{
Name: user3.Name,
Email: user3.Email,
},
Dates: &files_service.CommitDateOptions{
Author: time.Now(),
Committer: time.Now(),
},
})
assert.NoError(t, err)
assert.NotEmpty(t, addFileToForkedResp)

// create Pull
pullIssue = &issues_model.Issue{
RepoID: baseRepo.ID,
Title: "A mismatched path cannot trigger pull-request-target-event",
PosterID: user3.ID,
Poster: user3,
IsPull: true,
}
pullRequest = &issues_model.PullRequest{
HeadRepoID: forkedRepo.ID,
BaseRepoID: baseRepo.ID,
HeadBranch: "fork-branch-2",
BaseBranch: "main",
HeadRepo: forkedRepo,
BaseRepo: baseRepo,
Type: issues_model.PullRequestGitea,
}
err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
assert.NoError(t, err)

// the new pull request cannot trigger actions, so there is still only 1 record
assert.Equal(t, 1, unittest.GetCount(t, &actions_model.ActionRun{RepoID: baseRepo.ID}))
}) })
} }

Loading…
Cancel
Save