[OPIK-6366] [INFRA] feat: add zizmor security scan for GitHub Actions workflows#7546
Open
JetoPistola wants to merge 4 commits into
Open
[OPIK-6366] [INFRA] feat: add zizmor security scan for GitHub Actions workflows#7546JetoPistola wants to merge 4 commits into
JetoPistola wants to merge 4 commits into
Conversation
… workflows
Add zizmor (Trail of Bits) as a security scanner for GitHub Actions
workflows, complementing actionlint's syntax/shell linting. Runs as a
pre-commit hook in the unified Code Quality workflow at high severity,
regular persona, offline — matching how actionlint and hadolint are wired.
Rule tuning lives in .github/zizmor.yml (auto-discovered); unpinned-uses
is disabled there pending a separate SHA-pinning migration.
Fixes all 63 high-severity findings on main:
- 44 template-injection: hoist ${{ }} out of run: bodies into env:
- 18 excessive-permissions: scope write grants down to the jobs that
use them; workflow defaults drop to contents: read (packages: write
passed down to reusable-workflow-call jobs to preserve GHCR pushes)
- 1 dangerous-triggers: documented inline ignore on labeler.yml's
pull_request_target (safe — never executes PR-controlled code)
Document local zizmor usage in CONTRIBUTING.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
⏱️ pre-commit per-hook timing
⏭️ 39 skipped (no matching files changed)
|
Contributor
The zizmor hook showed a blank description in the Code Quality per-hook timing comment because it had no entry in precommit-hook-descriptions.tsv. - Add the missing zizmor description. - Add a coverage test in test_precommit_wrappers.sh that reads every hook name from .pre-commit-config.yaml and asserts each resolves to a non-empty description via precommit-hook-desc.py — so a hook added without a TSV entry now fails CI instead of silently rendering blank. - Widen the wrapper-tests hook's files: trigger to include the TSV, the desc resolver, and .pre-commit-config.yaml so the check reruns when a hook is added or renamed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
git config user.name was "Github Actions" in three sync/commit workflows; fixed to "GitHub Actions". Flagged by Baz on PR #7546. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JetoPistola
marked this pull request as ready for review
July 21, 2026 07:08
Contributor
Python SDK E2E Tests Results (Python 3.13)286 tests 283 ✅ 4m 6s ⏱️ Results for commit 9edea8f. ♻️ This comment has been updated with latest results. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
Adds zizmor (Trail of Bits) as a security scanner for GitHub Actions workflows, complementing the syntax/shell linting actionlint already provides. actionlint asks "will this YAML run?"; zizmor asks "if it runs, can it be exploited?" (template injection, dangerous triggers, excessive
GITHUB_TOKENpermissions).CI wiring — zizmor runs as a pre-commit hook in the unified
🐙 Code Qualityworkflow, exactly like actionlint and hadolint, driven by.pre-commit-config.yamlas SSOT. This diverges from the ticket's original standalone-zizmor.yml/SARIF-upload proposal: the repo standardized on pre-commit-as-SSOT for workflow linters after the ticket was filed, so a standalone workflow would fight that architecture. (SARIF → Security-tab upload is therefore intentionally not included; can be revisited as a scheduled scan later.)zizmorcore/zizmor-pre-commit@v1.27.0(→zizmor==1.27.0), run--offline --min-severity high --persona regular(no GitHub API calls → no rate limits; blocks only high-confidence, high-severity findings)..github/zizmor.yml.unpinned-uses(SHA-pin every action) is disabled there pending a follow-up migration — same split discipline as actionlint→SC2086 cleanup.Findings fixed — all 63 high-severity findings on
main(zizmor --min-severity highnow reports 0):${{ }}out ofrun:bodies intoenv:, reference quoted$VARwritegrants to the jobs that use them; workflow defaults →contents: read# zizmor: ignoreon labeler.yml'spull_request_target(safe — never executes PR code)Permission fixes preserve the caller→reusable-workflow
packages: writechain (a called workflow's permissions can only be reduced from the caller's, never elevated), so GHCR image pushes keep working.Timing-table hardening — the new hook initially showed a blank description in the Code Quality per-hook timing comment (no entry in
precommit-hook-descriptions.tsv). Added the missing description and a coverage test that reads every hook name from.pre-commit-config.yamland asserts each resolves to a non-empty description, so a future hook added without a TSV entry fails CI instead of silently rendering blank.Review follow-ups — corrected a pre-existing
Github→GitHubbranding typo in the commit-author name of three sync workflows (Baz). A separate pre-existingresumeUrl-logging leak Baz flagged in the release-n-deploy wrapper is tracked as OPIK-7414 rather than folded in here (out of this ticket's scope).Change checklist
.github/zizmor.ymlconfigGithub→GitHubcommit-author typo fixed in three workflowsCONTRIBUTING.mddocuments local zizmor usage, severity floor, suppression policyIssues
OPIK-6366. Follow-ups: OPIK-7413 (deferred 257
unpinned-usesSHA-pinning migration) and OPIK-7414 (pre-existingresumeUrllog leak in the release-n-deploy wrapper) — both filed and linked.Testing
zizmor --offline --min-severity high --persona regular .github/workflows/→ No findings.pre-commit run actionlint+pre-commit run zizmoron all changed workflows → Passed (shellcheck included; newly-visible SC2086 in docker-vulnerability-scan.yaml fixed by quoting).scripts/test_precommit_wrappers.sh→ All passed, including the new hook-description coverage test. Verified it fails (exit 1, naming the offending hook) when a description is removed, and passes when restored.Documentation
CONTRIBUTING.md→ "GitHub Actions workflows" section gains a zizmor subsection (local usage, severity floor, explicit-suppression policy, template-injection fix pattern).