Fix activity type match in `matchPullRequestEvent` (#25746) (#25797)

Backport #25746

Fix #25736
Caused by #24048

Right now we only check the activity type for `pull_request` event when
`types` is specified or there are no `types` and filter. If a workflow
only specifies filters but no `types` like this:
```
on:
  pull_request:
    branches: [main]
```
the workflow will be triggered even if the activity type is not one of
`[opened, reopened, sync]`. We need to check the activity type in this
case.

Co-authored-by: Giteabot <teabot@gitea.io>

diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go
index e22ad9f851000459061ab9b5ef680cc09df5906c..c7ca9e04d157cec78734193bbdd7632ac351b430 100644 (file)
--- a/modules/actions/workflows.go
+++ b/modules/actions/workflows.go
@@ -299,44 +299,47 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j
 }
 
 func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
-       // with no special filter parameters
-       if len(evt.Acts()) == 0 {
+       acts := evt.Acts()
+       activityTypeMatched := false
+       matchTimes := 0
+
+       if vals, ok := acts["types"]; !ok {
                // defaultly, only pull request `opened`, `reopened` and `synchronized` will trigger workflow
                // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
-               return prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened
+               activityTypeMatched = prPayload.Action == api.HookIssueSynchronized || prPayload.Action == api.HookIssueOpened || prPayload.Action == api.HookIssueReOpened
+       } else {
+               // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
+               // Actions with the same name:
+               // opened, edited, closed, reopened, assigned, unassigned
+               // Actions need to be converted:
+               // synchronized -> synchronize
+               // label_updated -> labeled
+               // label_cleared -> unlabeled
+               // Unsupported activity types:
+               // converted_to_draft, ready_for_review, locked, unlocked, review_requested, review_request_removed, auto_merge_enabled, auto_merge_disabled
+
+               action := prPayload.Action
+               switch action {
+               case api.HookIssueSynchronized:
+                       action = "synchronize"
+               case api.HookIssueLabelUpdated:
+                       action = "labeled"
+               case api.HookIssueLabelCleared:
+                       action = "unlabeled"
+               }
+               log.Trace("matching pull_request %s with %v", action, vals)
+               for _, val := range vals {
+                       if glob.MustCompile(val, '/').Match(string(action)) {
+                               activityTypeMatched = true
+                               matchTimes++
+                               break
+                       }
+               }
        }
 
-       matchTimes := 0
        // all acts conditions should be satisfied
-       for cond, vals := range evt.Acts() {
+       for cond, vals := range acts {
                switch cond {
-               case "types":
-                       // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
-                       // Actions with the same name:
-                       // opened, edited, closed, reopened, assigned, unassigned
-                       // Actions need to be converted:
-                       // synchronized -> synchronize
-                       // label_updated -> labeled
-                       // label_cleared -> unlabeled
-                       // Unsupported activity types:
-                       // converted_to_draft, ready_for_review, locked, unlocked, review_requested, review_request_removed, auto_merge_enabled, auto_merge_disabled
-
-                       action := prPayload.Action
-                       switch action {
-                       case api.HookIssueSynchronized:
-                               action = "synchronize"
-                       case api.HookIssueLabelUpdated:
-                               action = "labeled"
-                       case api.HookIssueLabelCleared:
-                               action = "unlabeled"
-                       }
-                       log.Trace("matching pull_request %s with %v", action, vals)
-                       for _, val := range vals {
-                               if glob.MustCompile(val, '/').Match(string(action)) {
-                                       matchTimes++
-                                       break
-                               }
-                       }
                case "branches":
                        refName := git.RefName(prPayload.PullRequest.Base.Ref)
                        patterns, err := workflowpattern.CompilePatterns(vals...)
@@ -385,7 +388,7 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload
                        log.Warn("pull request event unsupported condition %q", cond)
                }
        }
-       return matchTimes == len(evt.Acts())
+       return activityTypeMatched && matchTimes == len(evt.Acts())
 }
 
 func matchIssueCommentEvent(commit *git.Commit, issueCommentPayload *api.IssueCommentPayload, evt *jobparser.Event) bool {

diff --git a/modules/actions/workflows_test.go b/modules/actions/workflows_test.go
index 219229c51f8bd6c275c76ea6d29ee19ba13d4d36..19f72dc7f34a92cb76db92a53c936d98ed74f940 100644 (file)
--- a/modules/actions/workflows_test.go
+++ b/modules/actions/workflows_test.go
@@ -60,6 +60,25 @@ func TestDetectMatched(t *testing.T) {
                        yamlOn:       "on: pull_request",
                        expected:     false,
                },
+               {
+                       desc:         "HookEventPullRequest(pull_request) `closed` action doesn't match GithubEventPullRequest(pull_request) with no activity type",
+                       triggedEvent: webhook_module.HookEventPullRequest,
+                       payload:      &api.PullRequestPayload{Action: api.HookIssueClosed},
+                       yamlOn:       "on: pull_request",
+                       expected:     false,
+               },
+               {
+                       desc:         "HookEventPullRequest(pull_request) `closed` action doesn't match GithubEventPullRequest(pull_request) with branches",
+                       triggedEvent: webhook_module.HookEventPullRequest,
+                       payload: &api.PullRequestPayload{
+                               Action: api.HookIssueClosed,
+                               PullRequest: &api.PullRequest{
+                                       Base: &api.PRBranchInfo{},
+                               },
+                       },
+                       yamlOn:   "on:\n  pull_request:\n    branches: [main]",
+                       expected: false,
+               },
                {
                        desc:         "HookEventPullRequest(pull_request) `label_updated` action matches githubEventPullRequest(pull_request) with `label` activity type",
                        triggedEvent: webhook_module.HookEventPullRequest,