Suppress zizmor false positive for generated runtime paths - #60138
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>
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
No ADR enforcement needed: PR does not have the 'implementation' label and has <=100 new lines of code in business logic directories.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
🟢 Approval recommended
The focused compiler change matches established suppression syntax and includes appropriate regression coverage.
Pull request overview
Adds a targeted zizmor suppression for a trusted GitHub Actions runtime path while preserving existing secure environment-variable handling.
Changes:
- Annotates the generated runtime-path script with the justified
github-envsuppression. - Updates regression coverage to require the annotation.
File summaries
| File | Description |
|---|---|
pkg/workflow/compiler_yaml_step_generation.go |
Emits the inline zizmor suppression. |
pkg/workflow/agentic_output_test.go |
Verifies the suppression appears in generated output. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed with the Impeccable harden/audit lens (bug-fix category: zizmor false-positive suppression).
The change adds an inline # zizmor: ignore[github-env] - runner.tool_cache is set by GitHub Actions, not user input. comment on the generated Set runtime paths step, consistent with the identical pattern already used for GITHUB_SERVER_URL suppressions across many generated .lock.yml workflows in this repo. Test coverage in agentic_output_test.go was updated to assert the suppression string is present.
No correctness, security, or maintainability issues found on the changed lines.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 18.2 AIC · ⌖ 13.6 AIC · ⊞ 8.4K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs — the annotation itself is correct and consistent with the existing ghes_host_step.go pattern, but the fix appears incomplete: it doesn't yet reach the generated artifacts that zizmor actually scans.
📋 Key Themes & Highlights
Key Themes
- Fix not propagated to lock files: The compiler change in
compiler_yaml_step_generation.gois correct, but none of the 299.lock.ymlfiles under.github/workflows/were regenerated (make recompilewasn't run). Zizmor scans the committed lock files, not the compiler source, so as committed this PR does not actually suppress the false positive in CI. - Test update is sound: The updated unit test in
agentic_output_test.gocorrectly asserts the new suppression string is present on therun:line itself (matching the precedent set byghes_host_step_test.go's comment about span association).
Positive Highlights
- ✅ Follows the established suppression pattern (
# zizmor: ignore[github-env] - ... is set by GitHub Actions, not user input.) already used inghes_host_step.go. - ✅ Comment placement on the
run: |line itself (not a preceding line) — this matches the documented zizmor span requirement. - ✅ Minimal, targeted diff with an accompanying test update.
@copilot please address the review comment above (regenerate and commit lock files via make recompile).
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 45.1 AIC · ⌖ 15 AIC · ⊞ 10.4K
Comment /matt to run again
| " env:\n", | ||
| " GH_AW_RUNNER_TOOL_CACHE: ${{ runner.tool_cache }}\n", | ||
| " run: |\n", | ||
| " run: | # zizmor: ignore[github-env] - runner.tool_cache is set by GitHub Actions, not user input.\n", |
There was a problem hiding this comment.
[/diagnosing-bugs] This fixes the symptom in generated code, but make recompile was not run — none of the 299 committed .lock.yml files (e.g. ab-testing-advisor.lock.yml line 482) contain the new suppression comment. CI/zizmor still scans the stale lock files, so the fix has no actual effect until regenerated.
💡 Evidence
$ grep -c "run: | # zizmor: ignore\[github-env\] - runner.tool_cache" .github/workflows/*.lock.yml | grep -v ':0' | wc -l
0
Run make recompile and commit the resulting lock-file diffs so the suppression is actually deployed.
@copilot please address this.
There was a problem hiding this comment.
Addressed in commit 2605005: regenerated workflow lock files and verified the suppression is present in generated .lock.yml output. I also ran the local validation gate afterward; latest follow-up fix is in f22d711.
|
@copilot Please address the latest blocking signals on this PR:
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Done. I addressed the blocking thread and refreshed generated lock files ( |
|
🎉 This pull request is included in a new release. Release: |
Zizmor flags the generated
Set runtime pathsstep because it writesRUNNER_TOOL_CACHEto$GITHUB_ENV, despite the value coming from the trusted${{ runner.tool_cache }}context.github-envsuppression with the trusted-source rationale:Run: https://github.com/github/gh-aw/actions/runs/34552262956
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.