From 57133b8ba5d5bf62b5faacabca4b53123e865d73 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 13:38:54 +0000 Subject: [PATCH 1/5] Initial plan From cc8cf3c238af8766c196a520d14c8cd456f06a07 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 14:01:30 +0000 Subject: [PATCH 2/5] Add automatic zizmor annotations for workflow_run triggers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/ci-doctor.lock.yml | 2 ++ .github/workflows/dev-hawk.lock.yml | 2 ++ pkg/workflow/compiler.go | 2 ++ pkg/workflow/compiler_jobs.go | 17 ++++++------ pkg/workflow/frontmatter_extraction.go | 36 ++++++++++++++++++++++++++ pkg/workflow/jobs.go | 34 ++++++++++++++---------- 6 files changed, 71 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci-doctor.lock.yml b/.github/workflows/ci-doctor.lock.yml index b398cfa1b31..fa455053098 100644 --- a/.github/workflows/ci-doctor.lock.yml +++ b/.github/workflows/ci-doctor.lock.yml @@ -48,6 +48,7 @@ name: "CI Failure Doctor" "on": workflow_run: + # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation branches: - main types: @@ -70,6 +71,7 @@ run-name: "CI Failure Doctor" jobs: activation: needs: pre_activation + # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation if: > ((needs.pre_activation.outputs.activated == 'true') && (github.event.workflow_run.conclusion == 'failure')) && ((github.event_name != 'workflow_run') || ((github.event.workflow_run.repository.id == github.repository_id) && diff --git a/.github/workflows/dev-hawk.lock.yml b/.github/workflows/dev-hawk.lock.yml index 1d23e49d5da..5d684ab6faf 100644 --- a/.github/workflows/dev-hawk.lock.yml +++ b/.github/workflows/dev-hawk.lock.yml @@ -38,6 +38,7 @@ name: "Dev Hawk" "on": workflow_run: + # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation branches: - copilot/* types: @@ -58,6 +59,7 @@ run-name: "Dev Hawk" jobs: activation: needs: pre_activation + # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation if: > ((needs.pre_activation.outputs.activated == 'true') && (github.event.workflow_run.event == 'workflow_dispatch')) && ((github.event_name != 'workflow_run') || ((github.event.workflow_run.repository.id == github.repository_id) && diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index ba37aa93051..9ececa9eb80 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -1320,6 +1320,8 @@ func (c *Compiler) parseOnSection(frontmatter map[string]any, workflowData *Work yamlStr = parser.QuoteCronExpressions(yamlStr) // Apply comment processing to filter fields (draft, forks, names) yamlStr = c.commentOutProcessedFieldsInOnSection(yamlStr) + // Add zizmor ignore comment if workflow_run trigger is present + yamlStr = c.addZizmorIgnoreForWorkflowRun(yamlStr) // Keep "on" quoted as it's a YAML boolean keyword workflowData.On = yamlStr } else { diff --git a/pkg/workflow/compiler_jobs.go b/pkg/workflow/compiler_jobs.go index cb699930df0..153ed8e4cc6 100644 --- a/pkg/workflow/compiler_jobs.go +++ b/pkg/workflow/compiler_jobs.go @@ -665,14 +665,15 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate } job := &Job{ - Name: constants.ActivationJobName, - If: activationCondition, - RunsOn: c.formatSafeOutputsRunsOn(data.SafeOutputs), - Permissions: permissions, - Environment: environment, - Steps: steps, - Outputs: outputs, - Needs: activationNeeds, // Depend on pre-activation job if it exists + Name: constants.ActivationJobName, + If: activationCondition, + HasWorkflowRunSafetyChecks: workflowRunRepoSafety != "", // Mark job as having workflow_run safety checks + RunsOn: c.formatSafeOutputsRunsOn(data.SafeOutputs), + Permissions: permissions, + Environment: environment, + Steps: steps, + Outputs: outputs, + Needs: activationNeeds, // Depend on pre-activation job if it exists } return job, nil diff --git a/pkg/workflow/frontmatter_extraction.go b/pkg/workflow/frontmatter_extraction.go index ee4bb756a6a..7f7cbd48703 100644 --- a/pkg/workflow/frontmatter_extraction.go +++ b/pkg/workflow/frontmatter_extraction.go @@ -113,6 +113,8 @@ func (c *Compiler) extractTopLevelYAMLSection(frontmatter map[string]any, key st // Special handling for "on" section - comment out draft and fork fields from pull_request if key == "on" { yamlStr = c.commentOutProcessedFieldsInOnSection(yamlStr) + // Add zizmor ignore comment if workflow_run trigger is present + yamlStr = c.addZizmorIgnoreForWorkflowRun(yamlStr) } return yamlStr @@ -653,3 +655,37 @@ func (c *Compiler) extractFirewallConfig(firewall any) *FirewallConfig { return nil } + +// addZizmorIgnoreForWorkflowRun adds a zizmor ignore comment for workflow_run triggers +// The comment is added after the workflow_run: line to suppress dangerous-triggers warnings +// since the compiler adds proper role and fork validation to secure these triggers +func (c *Compiler) addZizmorIgnoreForWorkflowRun(yamlStr string) string { + // Check if the YAML contains workflow_run trigger + if !strings.Contains(yamlStr, "workflow_run:") { + return yamlStr + } + + lines := strings.Split(yamlStr, "\n") + var result []string + + for _, line := range lines { + result = append(result, line) + + // Check if this is the workflow_run: line + trimmedLine := strings.TrimSpace(line) + if trimmedLine == "workflow_run:" { + // Get the indentation of the workflow_run line + indentation := "" + if len(line) > len(trimmedLine) { + indentation = line[:len(line)-len(trimmedLine)] + } + + // Add zizmor ignore comment with proper indentation + // The comment explains that the trigger is secured with role and fork validation + comment := indentation + " # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation" + result = append(result, comment) + } + } + + return strings.Join(result, "\n") +} diff --git a/pkg/workflow/jobs.go b/pkg/workflow/jobs.go index 650293b7343..52319a34f54 100644 --- a/pkg/workflow/jobs.go +++ b/pkg/workflow/jobs.go @@ -13,20 +13,21 @@ var jobLog = logger.New("workflow:jobs") // Job represents a GitHub Actions job with all its properties type Job struct { - Name string - DisplayName string // Optional display name for the job (name property in YAML) - RunsOn string - If string - Permissions string - TimeoutMinutes int - Concurrency string // Job-level concurrency configuration - Environment string // Job environment configuration - Container string // Job container configuration - Services string // Job services configuration - Env map[string]string // Job-level environment variables - Steps []string - Needs []string // Job dependencies (needs clause) - Outputs map[string]string + Name string + DisplayName string // Optional display name for the job (name property in YAML) + RunsOn string + If string + HasWorkflowRunSafetyChecks bool // If true, the job's if condition includes workflow_run safety checks + Permissions string + TimeoutMinutes int + Concurrency string // Job-level concurrency configuration + Environment string // Job environment configuration + Container string // Job container configuration + Services string // Job services configuration + Env map[string]string // Job-level environment variables + Steps []string + Needs []string // Job dependencies (needs clause) + Outputs map[string]string // Reusable workflow call properties Uses string // Path to reusable workflow (e.g., ./.github/workflows/reusable.yml) @@ -189,6 +190,11 @@ func (jm *JobManager) renderJob(job *Job) string { // Add if condition if present if job.If != "" { + // Add zizmor ignore comment if this job has workflow_run safety checks + if job.HasWorkflowRunSafetyChecks { + yaml.WriteString(" # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation\n") + } + // Check if expression is multiline or longer than MaxExpressionLineLength characters if strings.Contains(job.If, "\n") || len(job.If) > constants.MaxExpressionLineLength { // Use YAML folded style for multiline expressions or long expressions From d9601623dc0f9ad01761db48cef5d55321b8cf2d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 14:03:45 +0000 Subject: [PATCH 3/5] Add tests for zizmor workflow_run annotations Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/zizmor_annotation_test.go | 109 +++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 pkg/workflow/zizmor_annotation_test.go diff --git a/pkg/workflow/zizmor_annotation_test.go b/pkg/workflow/zizmor_annotation_test.go new file mode 100644 index 00000000000..f8bc79cb733 --- /dev/null +++ b/pkg/workflow/zizmor_annotation_test.go @@ -0,0 +1,109 @@ +package workflow + +import ( + "strings" + "testing" +) + +func TestAddZizmorIgnoreForWorkflowRun(t *testing.T) { + c := NewCompiler(false, "", "test") + + tests := []struct { + name string + input string + expected string + }{ + { + name: "workflow_run trigger gets annotation", + input: `"on": + workflow_run: + branches: + - main`, + expected: `"on": + workflow_run: + # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation + branches: + - main`, + }, + { + name: "no workflow_run trigger", + input: `"on": + push: + branches: + - main`, + expected: `"on": + push: + branches: + - main`, + }, + { + name: "workflow_run with different indentation", + input: `"on": + workflow_run: + branches: + - main`, + expected: `"on": + workflow_run: + # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation + branches: + - main`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := c.addZizmorIgnoreForWorkflowRun(tt.input) + if result != tt.expected { + t.Errorf("Expected:\n%s\n\nGot:\n%s", tt.expected, result) + } + }) + } +} + +func TestJobHasWorkflowRunSafetyChecks(t *testing.T) { + tests := []struct { + name string + job *Job + expectField bool + }{ + { + name: "job with workflow_run safety checks", + job: &Job{ + Name: "activation", + HasWorkflowRunSafetyChecks: true, + If: "github.event.workflow_run.repository.id == github.repository_id", + RunsOn: "runs-on: ubuntu-latest", + }, + expectField: true, + }, + { + name: "job without workflow_run safety checks", + job: &Job{ + Name: "build", + HasWorkflowRunSafetyChecks: false, + If: "github.event_name == 'push'", + RunsOn: "runs-on: ubuntu-latest", + }, + expectField: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.job.HasWorkflowRunSafetyChecks != tt.expectField { + t.Errorf("Expected HasWorkflowRunSafetyChecks=%v, got %v", tt.expectField, tt.job.HasWorkflowRunSafetyChecks) + } + + // Test that the field is present in rendered YAML when true + jm := NewJobManager() + jm.AddJob(tt.job) + yaml := jm.RenderToYAML() + + if tt.expectField { + if !strings.Contains(yaml, "# zizmor: ignore[dangerous-triggers]") { + t.Error("Expected zizmor annotation in rendered YAML") + } + } + }) + } +} From eb90ae4f90eb681946ec90a2c1e58c06276e3808 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 14:05:08 +0000 Subject: [PATCH 4/5] Remove manual zizmor suppressions for ci-doctor and dev-hawk Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/zizmor.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/zizmor.yml b/.github/zizmor.yml index 1c4415ef97d..05a49732f6b 100644 --- a/.github/zizmor.yml +++ b/.github/zizmor.yml @@ -1,12 +1,11 @@ # Zizmor security scanner configuration # This file configures which security findings to ignore for specific workflows +# +# Note: workflow_run triggers in ci-doctor and dev-hawk now use inline annotations +# and no longer need manual suppressions here. rules: dangerous-triggers: ignore: - # CI Doctor - monitors other workflow failures - - ci-doctor.lock.yml:43 - # Dev Hawk - monitors copilot branch workflows - - dev-hawk.lock.yml:35 # Smoke Detector - monitors smoke test failures - smoke-detector.lock.yml:53 From cd5b9cf7b2f4402e3b6670243fbacc6cb2271cb8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 18:00:25 +0000 Subject: [PATCH 5/5] Improve workflow_run annotation validation and add edge case tests - Add validation to skip comment lines and lines with values after workflow_run: - Add flag to prevent duplicate annotations in malformed YAML - Add comprehensive tests for edge cases: comments, inline comments, duplicates, values Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/frontmatter_extraction.go | 42 ++++++++++++++------ pkg/workflow/jobs.go | 2 +- pkg/workflow/zizmor_annotation_test.go | 54 ++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 12 deletions(-) diff --git a/pkg/workflow/frontmatter_extraction.go b/pkg/workflow/frontmatter_extraction.go index 7f7cbd48703..e9f970d12e2 100644 --- a/pkg/workflow/frontmatter_extraction.go +++ b/pkg/workflow/frontmatter_extraction.go @@ -667,23 +667,43 @@ func (c *Compiler) addZizmorIgnoreForWorkflowRun(yamlStr string) string { lines := strings.Split(yamlStr, "\n") var result []string + annotationAdded := false // Track if we've already added the annotation for _, line := range lines { result = append(result, line) - // Check if this is the workflow_run: line + // Skip if we've already added the annotation (prevents duplicates) + if annotationAdded { + continue + } + + // Check if this is a non-comment workflow_run: key at the correct YAML level trimmedLine := strings.TrimSpace(line) - if trimmedLine == "workflow_run:" { - // Get the indentation of the workflow_run line - indentation := "" - if len(line) > len(trimmedLine) { - indentation = line[:len(line)-len(trimmedLine)] - } - // Add zizmor ignore comment with proper indentation - // The comment explains that the trigger is secured with role and fork validation - comment := indentation + " # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation" - result = append(result, comment) + // Skip if the line is a comment + if strings.HasPrefix(trimmedLine, "#") { + continue + } + + // Match lines that are only 'workflow_run:' (possibly with trailing whitespace or a comment) + // e.g., 'workflow_run:', 'workflow_run: # comment', ' workflow_run:' + // But not 'someworkflow_run:', 'workflow_run: value', etc. + if idx := strings.Index(trimmedLine, "workflow_run:"); idx == 0 { + after := strings.TrimSpace(trimmedLine[len("workflow_run:"):]) + // Only allow if nothing or only a comment follows + if after == "" || strings.HasPrefix(after, "#") { + // Get the indentation of the workflow_run line + indentation := "" + if len(line) > len(trimmedLine) { + indentation = line[:len(line)-len(trimmedLine)] + } + + // Add zizmor ignore comment with proper indentation + // The comment explains that the trigger is secured with role and fork validation + comment := indentation + " # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation" + result = append(result, comment) + annotationAdded = true + } } } diff --git a/pkg/workflow/jobs.go b/pkg/workflow/jobs.go index 52319a34f54..7aa26c85b68 100644 --- a/pkg/workflow/jobs.go +++ b/pkg/workflow/jobs.go @@ -17,7 +17,7 @@ type Job struct { DisplayName string // Optional display name for the job (name property in YAML) RunsOn string If string - HasWorkflowRunSafetyChecks bool // If true, the job's if condition includes workflow_run safety checks + HasWorkflowRunSafetyChecks bool // If true, the job's if condition includes workflow_run safety checks Permissions string TimeoutMinutes int Concurrency string // Job-level concurrency configuration diff --git a/pkg/workflow/zizmor_annotation_test.go b/pkg/workflow/zizmor_annotation_test.go index f8bc79cb733..d01ac63ab15 100644 --- a/pkg/workflow/zizmor_annotation_test.go +++ b/pkg/workflow/zizmor_annotation_test.go @@ -48,6 +48,60 @@ func TestAddZizmorIgnoreForWorkflowRun(t *testing.T) { branches: - main`, }, + { + name: "workflow_run in comment should not get annotation", + input: `"on": + push: + # This is not a workflow_run: trigger + branches: + - main`, + expected: `"on": + push: + # This is not a workflow_run: trigger + branches: + - main`, + }, + { + name: "workflow_run with inline comment gets annotation", + input: `"on": + workflow_run: # This is a workflow_run trigger + branches: + - main`, + expected: `"on": + workflow_run: # This is a workflow_run trigger + # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation + branches: + - main`, + }, + { + name: "multiple workflow_run keys only annotates first", + input: `"on": + workflow_run: + branches: + - main + workflow_run: + branches: + - develop`, + expected: `"on": + workflow_run: + # zizmor: ignore[dangerous-triggers] - workflow_run trigger is secured with role and fork validation + branches: + - main + workflow_run: + branches: + - develop`, + }, + { + name: "workflow_run with value should not get annotation", + input: `"on": + push: + branches: + - workflow_run: something`, + expected: `"on": + push: + branches: + - workflow_run: something`, + }, } for _, tt := range tests {