From a016d678db92f408974f3ae20005e2c37724fdd8 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 1 Jul 2026 13:10:11 -0700 Subject: [PATCH] fix(workflows): branch protection status checks fail when workflow uses on: paths filter (#38237) (#38302) --- modules/actions/scoped_workflows.go | 25 +-- modules/actions/workflows.go | 156 +++++++++++------- modules/actions/workflows_test.go | 46 ++++-- services/actions/commit_status.go | 67 +++++++- services/actions/notifier_helper.go | 68 +++++++- .../actions_scoped_workflow_test.go | 53 ++++++ tests/integration/actions_trigger_test.go | 21 ++- 7 files changed, 339 insertions(+), 97 deletions(-) diff --git a/modules/actions/scoped_workflows.go b/modules/actions/scoped_workflows.go index e528d0d1be6..0789015ac18 100644 --- a/modules/actions/scoped_workflows.go +++ b/modules/actions/scoped_workflows.go @@ -55,29 +55,34 @@ func ParseScopedWorkflows(sourceCommit *git.Commit) ([]*ParsedScopedWorkflow, er return parsed, nil } -// MatchScopedWorkflows evaluates already-parsed scoped workflows against one consuming event, returning those whose `on:` matches. +// MatchScopedWorkflows evaluates already-parsed scoped workflows against one consuming event. +// It returns the workflows whose `on:` matches, and those that matched the event but were excluded by a branch/paths filter (filtered). func MatchScopedWorkflows( parsed []*ParsedScopedWorkflow, consumerGitRepo *git.Repository, consumerCommit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader, -) []*DetectedWorkflow { - workflows := make([]*DetectedWorkflow, 0, len(parsed)) +) (matched, filtered []*DetectedWorkflow) { for _, p := range parsed { for _, evt := range p.Events { if evt.IsSchedule() { // schedule is a non-target for scoped workflows continue } - if detectMatched(consumerGitRepo, consumerCommit, triggedEvent, payload, evt) { - workflows = append(workflows, &DetectedWorkflow{ - EntryName: p.EntryName, - TriggerEvent: evt, - Content: p.Content, - }) + dwf := &DetectedWorkflow{ + EntryName: p.EntryName, + TriggerEvent: evt, + Content: p.Content, + } + switch detectWorkflowMatch(consumerGitRepo, consumerCommit, triggedEvent, payload, evt) { + case detectMatched: + matched = append(matched, dwf) + case detectFilteredOut: + filtered = append(filtered, dwf) + case detectNotApplicable: } } } - return workflows + return matched, filtered } diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index 3ea9c4b6cd0..d5886309f73 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -30,6 +30,14 @@ type DetectedWorkflow struct { Content []byte } +type detectResult int + +const ( + detectMatched detectResult = iota // event matched; run normally + detectNotApplicable // event/type doesn't apply; create nothing + detectFilteredOut // matched but excluded by a branch/paths filter; emits a skipped commit status +) + func init() { model.OnDecodeNodeError = func(node yaml.Node, out any, err error) { // Log the error instead of panic or fatal. @@ -172,18 +180,16 @@ func DetectWorkflows( triggedEvent webhook_module.HookEventType, payload api.Payloader, detectSchedule bool, -) ([]*DetectedWorkflow, []*DetectedWorkflow, error) { +) (workflows, schedules, filtered []*DetectedWorkflow, err error) { _, entries, err := ListWorkflows(commit) if err != nil { - return nil, nil, err + return nil, nil, nil, err } - workflows := make([]*DetectedWorkflow, 0, len(entries)) - schedules := make([]*DetectedWorkflow, 0, len(entries)) for _, entry := range entries { content, err := GetContentFromEntry(entry) if err != nil { - return nil, nil, err + return nil, nil, nil, err } // one workflow may have multiple events @@ -203,18 +209,24 @@ func DetectWorkflows( } schedules = append(schedules, dwf) } - } else if detectMatched(gitRepo, commit, triggedEvent, payload, evt) { + } else { dwf := &DetectedWorkflow{ EntryName: entry.Name(), TriggerEvent: evt, Content: content, } - workflows = append(workflows, dwf) + switch detectWorkflowMatch(gitRepo, commit, triggedEvent, payload, evt) { + case detectMatched: + workflows = append(workflows, dwf) + case detectFilteredOut: + filtered = append(filtered, dwf) + case detectNotApplicable: + } } } } - return workflows, schedules, nil + return workflows, schedules, filtered, nil } func DetectScheduledWorkflows(gitRepo *git.Repository, commit *git.Commit) ([]*DetectedWorkflow, error) { @@ -252,9 +264,9 @@ func DetectScheduledWorkflows(gitRepo *git.Repository, commit *git.Commit) ([]*D return wfs, nil } -func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader, evt *jobparser.Event) bool { +func detectWorkflowMatch(gitRepo *git.Repository, commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader, evt *jobparser.Event) detectResult { if !canGithubEventMatch(evt.Name, triggedEvent) { - return false + return detectNotApplicable } switch triggedEvent { @@ -268,7 +280,7 @@ func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent web log.Warn("Ignore unsupported %s event arguments %v", triggedEvent, evt.Acts()) } // no special filter parameters for these events, just return true if name matched - return true + return detectMatched case // push webhook_module.HookEventPush: @@ -279,14 +291,20 @@ func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent web webhook_module.HookEventIssueAssign, webhook_module.HookEventIssueLabel, webhook_module.HookEventIssueMilestone: - return matchIssuesEvent(payload.(*api.IssuePayload), evt) + if matchIssuesEvent(payload.(*api.IssuePayload), evt) { + return detectMatched + } + return detectNotApplicable case // issue_comment webhook_module.HookEventIssueComment, // `pull_request_comment` is same as `issue_comment` // See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_comment-use-issue_comment webhook_module.HookEventPullRequestComment: - return matchIssueCommentEvent(payload.(*api.IssueCommentPayload), evt) + if matchIssueCommentEvent(payload.(*api.IssueCommentPayload), evt) { + return detectMatched + } + return detectNotApplicable case // pull_request webhook_module.HookEventPullRequest, @@ -300,34 +318,49 @@ func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent web case // pull_request_review webhook_module.HookEventPullRequestReviewApproved, webhook_module.HookEventPullRequestReviewRejected: - return matchPullRequestReviewEvent(payload.(*api.PullRequestPayload), evt) + if matchPullRequestReviewEvent(payload.(*api.PullRequestPayload), evt) { + return detectMatched + } + return detectNotApplicable case // pull_request_review_comment webhook_module.HookEventPullRequestReviewComment: - return matchPullRequestReviewCommentEvent(payload.(*api.PullRequestPayload), evt) + if matchPullRequestReviewCommentEvent(payload.(*api.PullRequestPayload), evt) { + return detectMatched + } + return detectNotApplicable case // release webhook_module.HookEventRelease: - return matchReleaseEvent(payload.(*api.ReleasePayload), evt) + if matchReleaseEvent(payload.(*api.ReleasePayload), evt) { + return detectMatched + } + return detectNotApplicable case // registry_package webhook_module.HookEventPackage: - return matchPackageEvent(payload.(*api.PackagePayload), evt) + if matchPackageEvent(payload.(*api.PackagePayload), evt) { + return detectMatched + } + return detectNotApplicable case // workflow_run webhook_module.HookEventWorkflowRun: - return matchWorkflowRunEvent(payload.(*api.WorkflowRunPayload), evt) + if matchWorkflowRunEvent(payload.(*api.WorkflowRunPayload), evt) { + return detectMatched + } + return detectNotApplicable default: log.Warn("unsupported event %q", triggedEvent) - return false + return detectNotApplicable } } -func matchPushEvent(commit *git.Commit, pushPayload *api.PushPayload, evt *jobparser.Event) bool { +func matchPushEvent(commit *git.Commit, pushPayload *api.PushPayload, evt *jobparser.Event) detectResult { // with no special filter parameters if len(evt.Acts()) == 0 { - return true + return detectMatched } matchTimes := 0 @@ -393,14 +426,14 @@ func matchPushEvent(commit *git.Commit, pushPayload *api.PushPayload, evt *jobpa filesChanged, err := commit.GetFilesChangedSinceCommit(pushPayload.Before) if err != nil { log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", commit.ID.String(), err) - } else { - patterns, err := workflowpattern.CompilePatterns(vals...) - if err != nil { - break - } - if !workflowpattern.Skip(patterns, filesChanged) { - matchTimes++ - } + return detectNotApplicable + } + patterns, err := workflowpattern.CompilePatterns(vals...) + if err != nil { + break + } + if !workflowpattern.Skip(patterns, filesChanged) { + matchTimes++ } case "paths-ignore": if refName.IsTag() { @@ -410,14 +443,14 @@ func matchPushEvent(commit *git.Commit, pushPayload *api.PushPayload, evt *jobpa filesChanged, err := commit.GetFilesChangedSinceCommit(pushPayload.Before) if err != nil { log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", commit.ID.String(), err) - } else { - patterns, err := workflowpattern.CompilePatterns(vals...) - if err != nil { - break - } - if !workflowpattern.Filter(patterns, filesChanged) { - matchTimes++ - } + return detectNotApplicable + } + patterns, err := workflowpattern.CompilePatterns(vals...) + if err != nil { + break + } + if !workflowpattern.Filter(patterns, filesChanged) { + matchTimes++ } default: log.Warn("push event unsupported condition %q", cond) @@ -427,7 +460,10 @@ func matchPushEvent(commit *git.Commit, pushPayload *api.PushPayload, evt *jobpa if hasBranchFilter && hasTagFilter { matchTimes++ } - return matchTimes == len(evt.Acts()) + if matchTimes == len(evt.Acts()) { + return detectMatched + } + return detectFilteredOut } func matchIssuesEvent(issuePayload *api.IssuePayload, evt *jobparser.Event) bool { @@ -478,7 +514,7 @@ func matchIssuesEvent(issuePayload *api.IssuePayload, evt *jobparser.Event) bool return matchTimes == len(evt.Acts()) } -func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool { +func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) detectResult { acts := evt.Acts() activityTypeMatched := false matchTimes := 0 @@ -525,7 +561,7 @@ func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayloa headCommit, err = gitRepo.GetCommit(prPayload.PullRequest.Head.Sha) if err != nil { log.Error("GetCommit [ref: %s]: %v", prPayload.PullRequest.Head.Sha, err) - return false + return detectNotApplicable } } @@ -557,33 +593,39 @@ func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayloa filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.MergeBase) if err != nil { log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", headCommit.ID.String(), err) - } else { - patterns, err := workflowpattern.CompilePatterns(vals...) - if err != nil { - break - } - if !workflowpattern.Skip(patterns, filesChanged) { - matchTimes++ - } + return detectNotApplicable + } + patterns, err := workflowpattern.CompilePatterns(vals...) + if err != nil { + break + } + if !workflowpattern.Skip(patterns, filesChanged) { + matchTimes++ } case "paths-ignore": filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.MergeBase) if err != nil { log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", headCommit.ID.String(), err) - } else { - patterns, err := workflowpattern.CompilePatterns(vals...) - if err != nil { - break - } - if !workflowpattern.Filter(patterns, filesChanged) { - matchTimes++ - } + return detectNotApplicable + } + patterns, err := workflowpattern.CompilePatterns(vals...) + if err != nil { + break + } + if !workflowpattern.Filter(patterns, filesChanged) { + matchTimes++ } default: log.Warn("pull request event unsupported condition %q", cond) } } - return activityTypeMatched && matchTimes == len(evt.Acts()) + if !activityTypeMatched { + return detectNotApplicable + } + if matchTimes != len(evt.Acts()) { + return detectFilteredOut + } + return detectMatched } func matchIssueCommentEvent(issueCommentPayload *api.IssueCommentPayload, evt *jobparser.Event) bool { diff --git a/modules/actions/workflows_test.go b/modules/actions/workflows_test.go index aec7144783c..4577e52a11d 100644 --- a/modules/actions/workflows_test.go +++ b/modules/actions/workflows_test.go @@ -101,49 +101,49 @@ func TestDetectMatched(t *testing.T) { triggedEvent webhook_module.HookEventType payload api.Payloader yamlOn string - expected bool + expected detectResult }{ { desc: "HookEventCreate(create) matches GithubEventCreate(create)", triggedEvent: webhook_module.HookEventCreate, payload: nil, yamlOn: "on: create", - expected: true, + expected: detectMatched, }, { desc: "HookEventIssues(issues) `opened` action matches GithubEventIssues(issues)", triggedEvent: webhook_module.HookEventIssues, payload: &api.IssuePayload{Action: api.HookIssueOpened}, yamlOn: "on: issues", - expected: true, + expected: detectMatched, }, { desc: "HookEventIssues(issues) `milestoned` action matches GithubEventIssues(issues)", triggedEvent: webhook_module.HookEventIssues, payload: &api.IssuePayload{Action: api.HookIssueMilestoned}, yamlOn: "on: issues", - expected: true, + expected: detectMatched, }, { desc: "HookEventPullRequestSync(pull_request_sync) matches GithubEventPullRequest(pull_request)", triggedEvent: webhook_module.HookEventPullRequestSync, payload: &api.PullRequestPayload{Action: api.HookIssueSynchronized}, yamlOn: "on: pull_request", - expected: true, + expected: detectMatched, }, { desc: "HookEventPullRequest(pull_request) `label_updated` action doesn't match GithubEventPullRequest(pull_request) with no activity type", triggedEvent: webhook_module.HookEventPullRequest, payload: &api.PullRequestPayload{Action: api.HookIssueLabelUpdated}, yamlOn: "on: pull_request", - expected: false, + expected: detectNotApplicable, }, { 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, + expected: detectNotApplicable, }, { desc: "HookEventPullRequest(pull_request) `closed` action doesn't match GithubEventPullRequest(pull_request) with branches", @@ -155,56 +155,56 @@ func TestDetectMatched(t *testing.T) { }, }, yamlOn: "on:\n pull_request:\n branches: [main]", - expected: false, + expected: detectNotApplicable, }, { desc: "HookEventPullRequest(pull_request) `label_updated` action matches GithubEventPullRequest(pull_request) with `label` activity type", triggedEvent: webhook_module.HookEventPullRequest, payload: &api.PullRequestPayload{Action: api.HookIssueLabelUpdated}, yamlOn: "on:\n pull_request:\n types: [labeled]", - expected: true, + expected: detectMatched, }, { desc: "HookEventPullRequestReviewComment(pull_request_review_comment) matches GithubEventPullRequestReviewComment(pull_request_review_comment)", triggedEvent: webhook_module.HookEventPullRequestReviewComment, payload: &api.PullRequestPayload{Action: api.HookIssueReviewed}, yamlOn: "on:\n pull_request_review_comment:\n types: [created]", - expected: true, + expected: detectMatched, }, { desc: "HookEventPullRequestReviewRejected(pull_request_review_rejected) doesn't match GithubEventPullRequestReview(pull_request_review) with `dismissed` activity type (we don't support `dismissed` at present)", triggedEvent: webhook_module.HookEventPullRequestReviewRejected, payload: &api.PullRequestPayload{Action: api.HookIssueReviewed}, yamlOn: "on:\n pull_request_review:\n types: [dismissed]", - expected: false, + expected: detectNotApplicable, }, { desc: "HookEventRelease(release) `published` action matches GithubEventRelease(release) with `published` activity type", triggedEvent: webhook_module.HookEventRelease, payload: &api.ReleasePayload{Action: api.HookReleasePublished}, yamlOn: "on:\n release:\n types: [published]", - expected: true, + expected: detectMatched, }, { desc: "HookEventPackage(package) `created` action doesn't match GithubEventRegistryPackage(registry_package) with `updated` activity type", triggedEvent: webhook_module.HookEventPackage, payload: &api.PackagePayload{Action: api.HookPackageCreated}, yamlOn: "on:\n registry_package:\n types: [updated]", - expected: false, + expected: detectNotApplicable, }, { desc: "HookEventWiki(wiki) matches GithubEventGollum(gollum)", triggedEvent: webhook_module.HookEventWiki, payload: nil, yamlOn: "on: gollum", - expected: true, + expected: detectMatched, }, { desc: "HookEventSchedule(schedule) matches GithubEventSchedule(schedule)", triggedEvent: webhook_module.HookEventSchedule, payload: nil, yamlOn: "on: schedule", - expected: true, + expected: detectMatched, }, { desc: "push to tag matches workflow with paths condition (should skip paths check)", @@ -222,7 +222,19 @@ func TestDetectMatched(t *testing.T) { }, commit: nil, yamlOn: "on:\n push:\n paths:\n - src/**", - expected: true, + expected: detectMatched, + }, + { + desc: "push branch filter excludes -> filtered out", + triggedEvent: webhook_module.HookEventPush, + payload: &api.PushPayload{ + Ref: "refs/heads/feature/x", + Before: "0000000", + Commits: []*api.PayloadCommit{{ID: "abc", Added: []string{"a.go"}, Message: "x"}}, + }, + commit: nil, + yamlOn: "on:\n push:\n branches: [main]", + expected: detectFilteredOut, }, } @@ -231,7 +243,7 @@ func TestDetectMatched(t *testing.T) { evts, err := GetEventsFromContent(fullWorkflowContent(tc.yamlOn)) assert.NoError(t, err) assert.Len(t, evts, 1) - assert.Equal(t, tc.expected, detectMatched(nil, tc.commit, tc.triggedEvent, tc.payload, evts[0])) + assert.Equal(t, tc.expected, detectWorkflowMatch(nil, tc.commit, tc.triggedEvent, tc.payload, evts[0])) }) } } diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 1fa3fd027e5..2d2f0f051f5 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -14,8 +14,10 @@ import ( repo_model "gitea.dev/models/repo" user_model "gitea.dev/models/user" actions_module "gitea.dev/modules/actions" + "gitea.dev/modules/actions/jobparser" "gitea.dev/modules/commitstatus" "gitea.dev/modules/log" + api "gitea.dev/modules/structs" "gitea.dev/modules/util" webhook_module "gitea.dev/modules/webhook" commitstatus_service "gitea.dev/services/repository/commitstatus" @@ -147,21 +149,78 @@ func createCommitStatus(ctx context.Context, repo *repo_model.Repository, event, // scopedPrefix is computed once per run by the caller. The settings page derives the same string to preview expected checks. ctxName = actions_module.ScopedWorkflowStatusContextName(scopedPrefix, displayName, job.Name, event) } + targetURL := fmt.Sprintf("%s/jobs/%d", run.Link(), job.ID) + return createWorkflowCommitStatus(ctx, repo, commitID, ctxName, run.WorkflowID, toCommitStatus(job.Status), targetURL, toCommitStatusDescription(job)) +} +// CreateSkippedCommitStatusForFilteredWorkflow posts a skipped commit status for each job of a +// workflow that matched the triggering event but was excluded by a branch/paths filter. +// This lets a required status check tied to that context be satisfied without the workflow running. +// No ActionRun is created, so the status has no target URL (there is no run/job to link to). +// A non-empty scopedPrefix prefixes each context with its source repo, matching scoped runs. +func CreateSkippedCommitStatusForFilteredWorkflow(ctx context.Context, repo *repo_model.Repository, event webhook_module.HookEventType, triggerEvent, workflowID string, content []byte, payload api.Payloader, scopedPrefix string) error { + // Derive the status event name and target commit from the payload. + // TODO: this mirrors getCommitStatusEventNameAndCommitID, which derives the same from a persisted run. Should merge the logic if possible. + var statusEvent, commitID string + switch event { + case webhook_module.HookEventPush: + if p, ok := payload.(*api.PushPayload); ok && p.HeadCommit != nil { + statusEvent, commitID = "push", p.HeadCommit.ID + } + case webhook_module.HookEventPullRequest, + webhook_module.HookEventPullRequestSync, + webhook_module.HookEventPullRequestAssign, + webhook_module.HookEventPullRequestLabel, + webhook_module.HookEventPullRequestReviewRequest, + webhook_module.HookEventPullRequestMilestone: + if p, ok := payload.(*api.PullRequestPayload); ok && p.PullRequest != nil && p.PullRequest.Head != nil { + statusEvent, commitID = "pull_request", p.PullRequest.Head.Sha + if triggerEvent == actions_module.GithubEventPullRequestTarget { + statusEvent = "pull_request_target" + } + } + } + if statusEvent == "" || commitID == "" { + return nil // unsupported event or missing commit id, nothing to post + } + + workflows, err := jobparser.Parse(content) + if err != nil { + return fmt.Errorf("jobparser.Parse: %w", err) + } + + displayName := actions_module.WorkflowDisplayName(workflowID, content) + for _, sw := range workflows { + _, job := sw.Job() + if job == nil { + continue + } + jobName := util.EllipsisDisplayString(job.Name, 255) // run creation truncates job names the same way + ctxName := actions_module.WorkflowStatusContextName(displayName, jobName, statusEvent) + if scopedPrefix != "" { + ctxName = actions_module.ScopedWorkflowStatusContextName(scopedPrefix, displayName, jobName, statusEvent) + } + // "Skipped" mirrors toCommitStatusDescription for StatusSkipped. + if err := createWorkflowCommitStatus(ctx, repo, commitID, ctxName, workflowID, commitstatus.CommitStatusSkipped, "", "Skipped"); err != nil { + return err + } + } + return nil +} + +// createWorkflowCommitStatus posts the commit status for one workflow-job context. +func createWorkflowCommitStatus(ctx context.Context, repo *repo_model.Repository, commitID, ctxName, workflowID string, state commitstatus.CommitStatusState, targetURL, description string) error { // Mix the workflow file path into the hash so two workflow files that // share the same `name:` and job name produce distinct commit statuses // even though they render identically — matching GitHub's behavior // (issue #35699). - ctxHash := git_model.HashCommitStatusContext(ctxName + "\x00" + run.WorkflowID) + ctxHash := git_model.HashCommitStatusContext(ctxName + "\x00" + workflowID) // Pre-fix rows were hashed from Context alone. If a pre-existing row with // the legacy hash is still the "latest" for this SHA, reuse that hash so // the new row supersedes it; otherwise the old pending status would stay // stuck forever (it lives in its own dedupe group). Only relevant for // in-flight workflows at upgrade time. legacyHash := git_model.HashCommitStatusContext(ctxName) - state := toCommitStatus(job.Status) - targetURL := fmt.Sprintf("%s/jobs/%d", run.Link(), job.ID) - description := toCommitStatusDescription(job) statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commitID, db.ListOptionsAll) if err != nil { diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 7a8b5bd67ec..20f8a19467a 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -183,8 +183,9 @@ func notify(ctx context.Context, input *notifyInput) error { } var detectedWorkflows []*actions_module.DetectedWorkflow + var filteredWorkflows []*actions_module.DetectedWorkflow actionsConfig := input.Repo.MustGetUnit(ctx, unit_model.TypeActions).ActionsConfig() - workflows, schedules, err := actions_module.DetectWorkflows(gitRepo, commit, + workflows, schedules, filtered, err := actions_module.DetectWorkflows(gitRepo, commit, input.Event, input.Payload, shouldDetectSchedules, @@ -212,6 +213,17 @@ func notify(ctx context.Context, input *notifyInput) error { } } + for _, wf := range filtered { + if actionsConfig.IsWorkflowDisabled(wf.EntryName) { + log.Trace("repo %s has disable workflows %s", input.Repo.RelativePath(), wf.EntryName) + continue + } + + if wf.TriggerEvent.Name != actions_module.GithubEventPullRequestTarget { + filteredWorkflows = append(filteredWorkflows, wf) + } + } + if input.PullRequest != nil { // detect pull_request_target workflows baseRef := git.BranchPrefix + input.PullRequest.BaseBranch @@ -219,7 +231,7 @@ func notify(ctx context.Context, input *notifyInput) error { if err != nil { return fmt.Errorf("gitRepo.GetCommit: %w", err) } - baseWorkflows, _, err := actions_module.DetectWorkflows(gitRepo, baseCommit, input.Event, input.Payload, false) + baseWorkflows, _, baseFiltered, err := actions_module.DetectWorkflows(gitRepo, baseCommit, input.Event, input.Payload, false) if err != nil { return fmt.Errorf("DetectWorkflows: %w", err) } @@ -227,11 +239,24 @@ func notify(ctx context.Context, input *notifyInput) error { log.Trace("repo %s with commit %s couldn't find pull_request_target workflows", input.Repo.RelativePath(), baseCommit.ID) } else { for _, wf := range baseWorkflows { + if actionsConfig.IsWorkflowDisabled(wf.EntryName) { + log.Trace("repo %s has disable workflows %s", input.Repo.RelativePath(), wf.EntryName) + continue + } if wf.TriggerEvent.Name == actions_module.GithubEventPullRequestTarget { detectedWorkflows = append(detectedWorkflows, wf) } } } + for _, wf := range baseFiltered { + if actionsConfig.IsWorkflowDisabled(wf.EntryName) { + log.Trace("repo %s has disable workflows %s", input.Repo.RelativePath(), wf.EntryName) + continue + } + if wf.TriggerEvent.Name == actions_module.GithubEventPullRequestTarget { + filteredWorkflows = append(filteredWorkflows, wf) + } + } } if shouldDetectSchedules { @@ -244,6 +269,8 @@ func notify(ctx context.Context, input *notifyInput) error { return err } + handleFilteredWorkflows(ctx, input, filteredWorkflows) + return detectAndHandleScopedWorkflows(ctx, input, ref, gitRepo, commit) } @@ -369,6 +396,16 @@ func buildApproveAndInsertRun( return nil } +// handleFilteredWorkflows posts a skipped commit status for each workflow that matched the event but was excluded by a branch/paths filter. +func handleFilteredWorkflows(ctx context.Context, input *notifyInput, filteredWorkflows []*actions_module.DetectedWorkflow) { + for _, dwf := range filteredWorkflows { + if err := CreateSkippedCommitStatusForFilteredWorkflow(ctx, input.Repo, input.Event, dwf.TriggerEvent.Name, dwf.EntryName, dwf.Content, input.Payload, ""); err != nil { + log.Error("repo %s: skipped commit status for workflow %s: %v", input.Repo.RelativePath(), dwf.EntryName, err) + continue + } + } +} + func newNotifyInputFromIssue(issue *issues_model.Issue, event webhook_module.HookEventType) *notifyInput { return newNotifyInput(issue.Repo, issue.Poster, event) } @@ -640,7 +677,7 @@ func detectAndHandleScopedWorkflows( continue } - sourceCommitSHA, detected, err := detectScopedWorkflowsForSource(ctx, input, consumerGitRepo, consumerCommit, sourceRepo) + sourceCommitSHA, detected, filtered, err := detectScopedWorkflowsForSource(ctx, input, consumerGitRepo, consumerCommit, sourceRepo) if err != nil { log.Error("scoped workflows: source %d for consumer %s: %v", sourceRepoID, input.Repo.RelativePath(), err) continue @@ -658,23 +695,40 @@ func detectAndHandleScopedWorkflows( continue } } + + // A filtered-out scoped workflow posts a skipped commit status. + if len(filtered) > 0 { + scopedPrefix := actions_model.ScopedStatusContextPrefix(ctx, sourceRepo.ID) + for _, dwf := range filtered { + if actions_model.ScopedWorkflowOptedOut(actionsConfig, sources, sourceRepo.ID, dwf.EntryName) { + continue + } + if err := CreateSkippedCommitStatusForFilteredWorkflow(ctx, input.Repo, input.Event, dwf.TriggerEvent.Name, dwf.EntryName, dwf.Content, input.Payload, scopedPrefix); err != nil { + log.Error("scoped workflows: skipped commit status for source %s workflow %s: %v", sourceRepo.RelativePath(), dwf.EntryName, err) + continue + } + } + } } return nil } -// detectScopedWorkflowsForSource detects the scoped workflows from the source repo at its default branch +// detectScopedWorkflowsForSource detects the scoped workflows from the source repo at its default branch. +// detected are the workflows to run; filtered matched the event but were excluded by a branch/paths +// filter and post a skipped commit status. func detectScopedWorkflowsForSource( ctx context.Context, input *notifyInput, consumerGitRepo *git.Repository, consumerCommit *git.Commit, sourceRepo *repo_model.Repository, -) (sourceCommitSHA string, detected []*actions_module.DetectedWorkflow, err error) { +) (sourceCommitSHA string, detected, filtered []*actions_module.DetectedWorkflow, err error) { // scoped workflow content is always taken from the source repo's default branch; the parse is cached per (source, default-branch SHA) and reused across consuming repos/events sourceCommitSHA, parsed, err := LoadParsedScopedWorkflows(ctx, sourceRepo) if err != nil { - return "", nil, err + return "", nil, nil, err } - return sourceCommitSHA, actions_module.MatchScopedWorkflows(parsed, consumerGitRepo, consumerCommit, input.Event, input.Payload), nil + detected, filtered = actions_module.MatchScopedWorkflows(parsed, consumerGitRepo, consumerCommit, input.Event, input.Payload) + return sourceCommitSHA, detected, filtered, nil } diff --git a/tests/integration/actions_scoped_workflow_test.go b/tests/integration/actions_scoped_workflow_test.go index 8feb0b051a6..23777a116f5 100644 --- a/tests/integration/actions_scoped_workflow_test.go +++ b/tests/integration/actions_scoped_workflow_test.go @@ -344,6 +344,59 @@ jobs: }) }) + t.Run("Filtered required scoped check passes as skipped and allows merge", func(t *testing.T) { + // A required scoped workflow excluded by a paths filter posts a skipped (success) commit status, + // so the required check is satisfied and the PR can merge. + + const scopedFilteredPRWorkflow = `name: Scoped Filtered PR +on: + pull_request: + paths: + - src/** +jobs: + scoped-filtered-job: + runs-on: ubuntu-latest + steps: + - run: echo scoped-filtered +` + source := createTestRepo(t, "sw-filtered-source", false) + createRepoWorkflowFile(t, user2, user2Token, source, ".gitea/scoped_workflows/pr.yaml", scopedFilteredPRWorkflow) + registerUserScopedSource(t, source, "pr.yaml") // required + + consumer := createTestRepo(t, "sw-filtered-consumer", false) + // Protect the default branch (its own status check stays off, so only the required scoped check gates the merge). + user2Session.MakeRequest(t, NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings/branches/edit", consumer.OwnerName, consumer.Name), map[string]string{ + "rule_name": consumer.DefaultBranch, + "enable_push": "true", + "block_admin_merge_override": "true", // otherwise the repo owner bypasses the status check + }), http.StatusSeeOther) + + // Open a PR that changes a file NOT matching the workflow's `paths: [src/**]`, so it is filtered out. + prFile := &api.CreateFileOptions{ + FileOptions: api.FileOptions{ + BranchName: consumer.DefaultBranch, NewBranchName: "filtered-pr", Message: "pr change", + Author: api.Identity{Name: user2.Name, Email: user2.Email}, + Committer: api.Identity{Name: user2.Name, Email: user2.Email}, + Dates: api.CommitDateOptions{Author: time.Now(), Committer: time.Now()}, + }, + ContentBase64: base64.StdEncoding.EncodeToString([]byte("pr change")), + } + createWorkflowFile(t, user2Token, consumer.OwnerName, consumer.Name, "docs.txt", prFile) + apiCtx := NewAPITestContext(t, user2.Name, consumer.Name, auth_model.AccessTokenScopeWriteRepository) + pr, err := doAPICreatePullRequest(apiCtx, consumer.OwnerName, consumer.Name, consumer.DefaultBranch, "filtered-pr")(t) + require.NoError(t, err) + + // Filtered: no scoped run is created, but a skipped commit status is posted on the PR head. + assert.Equal(t, 0, unittest.GetCount(t, &actions_model.ActionRun{RepoID: consumer.ID, IsScopedRun: true}), "filtered scoped workflow creates no run") + assertSkippedCommitStatusExists(t, consumer.ID, pr.Head.Sha, "pull_request") + + // The skipped (success) status satisfies the required scoped check (prefixed with the source repo), so the merge is allowed. + assert.NoError(t, queue.GetManager().FlushAll(t.Context(), 5*time.Second)) + mergeReq := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", consumer.OwnerName, consumer.Name, pr.Index), + &forms.MergePullRequestForm{Do: string(repo_model.MergeStyleMerge), MergeMessageField: "merge"}).AddTokenAuth(user2Token) + user2Session.MakeRequest(t, mergeReq, http.StatusOK) + }) + t.Run("Settings page required patterns", func(t *testing.T) { source := createTestRepo(t, "sw-settings-source", false) createRepoWorkflowFile(t, user2, user2Token, source, ".gitea/scoped_workflows/push.yaml", scopedPushWorkflow) diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index d222aac3626..adcc403ba22 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -215,8 +215,9 @@ jobs: err = pull_service.NewPullRequest(t.Context(), prOpts) assert.NoError(t, err) - // the new pull request cannot trigger actions, so there is still only 1 record + // the new pull request is filtered by paths, so no run is created; a skipped commit status is posted instead assert.Equal(t, 1, unittest.GetCount(t, &actions_model.ActionRun{RepoID: baseRepo.ID})) + assertSkippedCommitStatusExists(t, baseRepo.ID, addFileToForkedResp.Commit.SHA, "pull_request_target") }) } @@ -338,6 +339,9 @@ jobs: }) assert.NoError(t, err) assert.NotEmpty(t, addFileToBranchResp) + // the push to test-skip-ci is filtered by branches, so no run is created; a skipped commit status is posted instead + assert.Equal(t, 1, unittest.GetCount(t, &actions_model.ActionRun{RepoID: repo.ID})) + assertSkippedCommitStatusExists(t, repo.ID, addFileToBranchResp.Commit.SHA, "push") resp := testPullCreate(t, session, "user2", "skip-ci", true, "master", "test-skip-ci", "[skip ci] test-skip-ci") @@ -345,7 +349,7 @@ jobs: url := test.RedirectURL(resp) assert.Regexp(t, "^/user2/skip-ci/pulls/[0-9]*$", url) - // the pr title contains a configured skip-ci string, so there is still only 1 record + // the pr title contains a configured skip-ci string, so no run and no skipped status are created assert.Equal(t, 1, unittest.GetCount(t, &actions_model.ActionRun{RepoID: repo.ID})) }) } @@ -1879,3 +1883,16 @@ jobs: runner.fetchNoTask(t) }) } + +// assertSkippedCommitStatusExists asserts that a filtered-out workflow posted a skipped commit status on sha +func assertSkippedCommitStatusExists(t *testing.T, repoID int64, sha, eventSuffix string) { + t.Helper() + statuses, err := git_model.GetLatestCommitStatus(t.Context(), repoID, sha, db.ListOptionsAll) + require.NoError(t, err) + for _, s := range statuses { + if s.State == commitstatus.CommitStatusSkipped && strings.Contains(s.Context, "("+eventSuffix+")") { + return + } + } + assert.Failf(t, "missing skipped commit status", "no skipped commit status with event %q on %s (found %d statuses)", eventSuffix, sha, len(statuses)) +}