From 231cf5072d28f00d2ee7645bb4a8c8850a58f070 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 4 Aug 2023 12:20:23 +0800 Subject: [PATCH 1/5] use head commit when event is pull_request_target --- modules/actions/workflows.go | 26 +++++++++++++++++++------- modules/actions/workflows_test.go | 2 +- services/actions/notifier_helper.go | 4 ++-- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index 2c7cec5591de..71aaae65d55f 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -95,7 +95,7 @@ func GetEventsFromContent(content []byte) ([]*jobparser.Event, error) { 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) if err != nil { return nil, err @@ -114,7 +114,7 @@ func DetectWorkflows(commit *git.Commit, triggedEvent webhook_module.HookEventTy } for _, evt := range events { 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{ EntryName: entry.Name(), TriggerEvent: evt.Name, @@ -128,7 +128,7 @@ func DetectWorkflows(commit *git.Commit, triggedEvent webhook_module.HookEventTy 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) { return false } @@ -168,7 +168,7 @@ func detectMatched(commit *git.Commit, triggedEvent webhook_module.HookEventType webhook_module.HookEventPullRequestSync, webhook_module.HookEventPullRequestAssign, webhook_module.HookEventPullRequestLabel: - return matchPullRequestEvent(commit, payload.(*api.PullRequestPayload), evt) + return matchPullRequestEvent(gitRepo, commit, payload.(*api.PullRequestPayload), evt) case // pull_request_review webhook_module.HookEventPullRequestReviewApproved, @@ -331,7 +331,7 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j 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() activityTypeMatched := false matchTimes := 0 @@ -370,6 +370,18 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload } } + var ( + headCommit *git.Commit = commit + err error + ) + if evt.Name == GithubEventPullRequestTarget && (len(acts["paths"]) > 0 || len(acts["paths-ignore"]) > 0) { + headCommit, err = gitRepo.GetCommit(git.BranchPrefix + prPayload.PullRequest.Head.Ref) + if err != nil { + log.Error("GetCommit [ref: %s]: %v", git.BranchPrefix+prPayload.PullRequest.Head.Ref, err) + return false + } + } + // all acts conditions should be satisfied for cond, vals := range acts { switch cond { @@ -392,7 +404,7 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload matchTimes++ } case "paths": - filesChanged, err := commit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref) + filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref) if err != nil { log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", commit.ID.String(), err) } else { @@ -405,7 +417,7 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload } } case "paths-ignore": - filesChanged, err := commit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref) + filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref) if err != nil { log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", commit.ID.String(), err) } else { diff --git a/modules/actions/workflows_test.go b/modules/actions/workflows_test.go index ef553c4a5726..2d57f19488e4 100644 --- a/modules/actions/workflows_test.go +++ b/modules/actions/workflows_test.go @@ -125,7 +125,7 @@ func TestDetectMatched(t *testing.T) { evts, err := GetEventsFromContent([]byte(tc.yamlOn)) assert.NoError(t, err) 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])) }) } } diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 764d24a7dbc8..0cc2d17f4ef4 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -143,7 +143,7 @@ func notify(ctx context.Context, input *notifyInput) error { } 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 { return fmt.Errorf("DetectWorkflows: %w", err) } @@ -164,7 +164,7 @@ func notify(ctx context.Context, input *notifyInput) error { if err != nil { 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 { return fmt.Errorf("DetectWorkflows: %w", err) } From 978f51a33a93646fe9e6b80ae1acc1e0ac15a405 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 4 Aug 2023 13:35:04 +0800 Subject: [PATCH 2/5] lint --- modules/actions/workflows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index 71aaae65d55f..469e033a6995 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -371,7 +371,7 @@ func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayloa } var ( - headCommit *git.Commit = commit + headCommit = commit err error ) if evt.Name == GithubEventPullRequestTarget && (len(acts["paths"]) > 0 || len(acts["paths-ignore"]) > 0) { From 7e481ffbe8a9e6eae1f0d63d7329dbacd65c0cd8 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 4 Aug 2023 16:59:50 +0800 Subject: [PATCH 3/5] use sha --- modules/actions/workflows.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index 469e033a6995..f0d391cf279a 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -375,9 +375,9 @@ func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayloa err error ) if evt.Name == GithubEventPullRequestTarget && (len(acts["paths"]) > 0 || len(acts["paths-ignore"]) > 0) { - headCommit, err = gitRepo.GetCommit(git.BranchPrefix + prPayload.PullRequest.Head.Ref) + headCommit, err = gitRepo.GetCommit(prPayload.PullRequest.Head.Sha) if err != nil { - log.Error("GetCommit [ref: %s]: %v", git.BranchPrefix+prPayload.PullRequest.Head.Ref, err) + log.Error("GetCommit [ref: %s]: %v", prPayload.PullRequest.Head.Sha, err) return false } } From 66f6c5edd496d3c398b12a6e3f82f658b2858904 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 4 Aug 2023 17:27:22 +0800 Subject: [PATCH 4/5] update test --- tests/integration/actions_trigger_test.go | 54 ++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index 241d6172f7a8..1c5d2fed61d1 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -67,7 +67,7 @@ func TestPullRequestTargetEvent(t *testing.T) { { Operation: "create", 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", @@ -138,8 +138,60 @@ func TestPullRequestTargetEvent(t *testing.T) { assert.NoError(t, err) // 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}) assert.Equal(t, addFileToForkedResp.Commit.SHA, actionRun.CommitSHA) 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})) }) } From a38dbb8f078b7ce55ceac951009926d5bea278e6 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 4 Aug 2023 17:51:10 +0800 Subject: [PATCH 5/5] replace 'commit' with 'headCommit' in error log --- modules/actions/workflows.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index f0d391cf279a..de340a74ec9c 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -406,7 +406,7 @@ func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayloa case "paths": filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref) 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 { patterns, err := workflowpattern.CompilePatterns(vals...) if err != nil { @@ -419,7 +419,7 @@ func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayloa case "paths-ignore": filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref) 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 { patterns, err := workflowpattern.CompilePatterns(vals...) if err != nil {