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/.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 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..e9f970d12e2 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,57 @@ 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 + annotationAdded := false // Track if we've already added the annotation + + for _, line := range lines { + result = append(result, 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) + + // 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 + } + } + } + + return strings.Join(result, "\n") +} diff --git a/pkg/workflow/jobs.go b/pkg/workflow/jobs.go index 650293b7343..7aa26c85b68 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 diff --git a/pkg/workflow/zizmor_annotation_test.go b/pkg/workflow/zizmor_annotation_test.go new file mode 100644 index 00000000000..d01ac63ab15 --- /dev/null +++ b/pkg/workflow/zizmor_annotation_test.go @@ -0,0 +1,163 @@ +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`, + }, + { + 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 { + 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") + } + } + }) + } +}