Generate zizmor annotations for workflow_run triggers#4129
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds automatic generation of zizmor ignore annotations for workflow_run triggers to replace brittle manual line-number suppressions. The compiler already secures workflow_run triggers with role and fork validation checks, so these annotations inform zizmor to skip the "dangerous-triggers" warnings.
Key changes:
- Automatic inline comment injection at workflow trigger level (
workflow_run:lines) - Job-level annotation support via new
HasWorkflowRunSafetyChecksfield for jobs with workflow_run safety conditions - Removal of manual suppressions from
.github/zizmor.ymlfor ci-doctor and dev-hawk workflows
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/zizmor_annotation_test.go | Unit tests for annotation generation at trigger and job levels |
| pkg/workflow/jobs.go | Added HasWorkflowRunSafetyChecks field and annotation rendering in renderJob() |
| pkg/workflow/frontmatter_extraction.go | Implemented addZizmorIgnoreForWorkflowRun() for trigger-level annotations |
| pkg/workflow/compiler_jobs.go | Sets HasWorkflowRunSafetyChecks flag when building activation jobs |
| pkg/workflow/compiler.go | Integrated annotation generation in parseOnSection() |
| .github/zizmor.yml | Removed manual line-number suppressions for ci-doctor and dev-hawk |
| .github/workflows/dev-hawk.lock.yml | Generated workflow with inline annotations |
| .github/workflows/ci-doctor.lock.yml | Generated workflow with inline annotations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 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) |
There was a problem hiding this comment.
The function adds an annotation after every line that contains workflow_run: when trimmed, but doesn't verify this is actually a YAML key at the appropriate level. This could incorrectly annotate lines like # Comment about workflow_run: or string values containing workflow_run:. Consider checking that the line doesn't start with # (comment) and matches the expected YAML key pattern.
| // 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) | |
| // 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. | |
| // We'll check that after trimming leading whitespace, the line starts with 'workflow_run:' and | |
| // is either exactly 'workflow_run:' or 'workflow_run:' followed by whitespace or a comment. | |
| 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) | |
| } |
There was a problem hiding this comment.
Updated the function to validate that workflow_run: is a proper YAML key by:
- Skipping lines that start with
#(comments) - Verifying nothing follows
workflow_run:except whitespace or a comment - Only annotating lines matching the pattern
workflow_run:orworkflow_run: # comment
Added comprehensive test coverage for these edge cases. Commit: cd5b9cf
| 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:" { |
There was a problem hiding this comment.
The function will add multiple annotations if workflow_run: appears more than once in the YAML string. While this is unlikely in well-formed YAML (since YAML keys should be unique), it could cause duplicate annotations in malformed input. Consider adding a flag to ensure the annotation is only added once, or document this behavior.
There was a problem hiding this comment.
Added an annotationAdded flag to ensure the annotation is only added once, even if workflow_run: appears multiple times in malformed YAML. Added test case to verify this behavior. Commit: cd5b9cf
|
@copilot lint |
|
Agentic Changeset Generator triggered by this pull request. |
- 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>
Linting completed successfully. All validation checks pass (formatting, Go vet, staticcheck, JavaScript linting). |
Add zizmor annotations for workflow_run validation ✅
Problem
The compiler adds specific validation (role checking, fork prevention) to secure workflow_run triggers. However, zizmor flags these as "dangerous-triggers" which required manual configuration in
.github/zizmor.ymlto suppress the warnings.Solution - COMPLETE ✅
on:section for workflow_run triggers.github/zizmor.ymlImplementation Complete
Added automatic inline zizmor annotations that suppress dangerous-triggers warnings for workflow_run triggers.
Key Changes:
HasWorkflowRunSafetyChecksfield to track jobs with workflow_run safety validationrenderJobto add zizmor ignore comment before jobifconditions when flag is setaddZizmorIgnoreForWorkflowRunfunction to add annotations afterworkflow_run:lineextractTopLevelYAMLSectionandparseOnSectioncode paths.github/zizmor.ymlGenerated Annotation Format:
Tests:
TestAddZizmorIgnoreForWorkflowRun- Comprehensive tests including edge cases for comments, inline comments, duplicates, and valuesTestJobHasWorkflowRunSafetyChecks- Verifies job field is set and rendered correctlyValidation:
.github/zizmor.ymlImpact:
The compiler now automatically generates inline zizmor annotations when compiling workflows with workflow_run triggers. The annotation function includes robust validation to:
#)workflow_run:keys without valuesThis eliminates the need for brittle line-number-based suppressions that break when workflows are modified.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.