ci: add zizmor workflow linting and harden GitHub Actions#2055
Open
Pouyanpi wants to merge 5 commits into
Open
ci: add zizmor workflow linting and harden GitHub Actions#2055Pouyanpi wants to merge 5 commits into
Pouyanpi wants to merge 5 commits into
Conversation
Adopt zizmor as a pre-commit hook (.github/zizmor.yml) gating on high severity. The unpinned-uses policy is relaxed to ref-pin to match the repo's version-tag pinning convention (kept current via Dependabot). Resolve the high-severity findings zizmor surfaced in existing workflows: - Move attacker-controllable template expansions into env vars in release.yml and publish-pypi-approval.yml. - Scope triage-label.yml permissions to the job level. - Add documented zizmor ignores for the intentional pull_request_target / workflow_run / cache-poisoning patterns in triage-label.yml, pr-merge-guidance.yml, publish-pypi-approval.yml, and test-and-build-wheel.yml. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Align the pre-commit zizmor pin with the current release. v1.25.2's cache-poisoning audit flags actions/setup-node, which defaults package-manager-cache to true and restores an npm cache when package.json declares npm. docs-build runs on pull_request and publishes previews and staging docs, so set package-manager-cache: false to keep cache restoration out of the publishing path and keep the --min-severity high gate green.
…ction Apply zizmor's lower-severity fixes (below the high gate enforced in pre-commit). Set persist-credentials: false on checkouts that do not push with the persisted token, so the GITHUB_TOKEN is not written to .git/config where it could leak via uploaded artifacts (artipacked). Move step-output and context expansions out of run-block bodies into env variables referenced as shell vars, so untrusted values are passed as data rather than interpolated into the script (template-injection). create-tag keeps its persisted credentials because it pushes the release tag with git push origin; it uploads no artifacts, so the token is not exposed.
The previous comment claimed the cached .venv is not an input to the published wheel, which is inaccurate: build.sh builds the wheel inside that .venv, so a poisoned cache could alter the artifact. State the actual reason the suppression is safe instead: this workflow has no pull_request trigger (push to main/develop/tags and schedule only) and GitHub scopes caches by ref, so fork PRs cannot write a cache these trusted runs restore. Comment only; no behavior change.
Drop --min-severity high from the zizmor pre-commit hook so the gate matches
the tool default (and pydantic-ai's setup), then resolve everything it
surfaces:
- excessive-permissions: add least-privilege permissions: blocks to the
workflows that previously relied on the default token. contents: read where
a checkout runs (directly or via the reusable _test.yml), {} for pure
status/install-from-PyPI/publish jobs, and contents: write + pull-requests:
read for docs-remove-stale-reviews whose reusable workflow pushes to
gh-pages.
- secrets-inherit: pr-tests passes only CODECOV_TOKEN to _test.yml instead of
inheriting all secrets; _test.yml declares it as an optional workflow_call
secret (backward compatible with callers that pass nothing).
- artipacked (create-tag) and use-trusted-publishing (publish-wheel) are
suppressed inline with rationale: the tag push needs the checkout's
persisted credentials, and the API-token publish is intentional pending a
separate OIDC trusted-publishing migration.
github-actions
Bot
added
the
status: needs triage
Pouyanpi
changed the title
Contributor
|
Staged Fern docs preview: https://nvidia-preview-pr-2055.docs.buildwithfern.com/nemo/guardrails |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Pouyanpi
added
status: triaged
Contributor
Greptile SummaryThis PR integrates zizmor as a pre-commit and CI lint gate, then resolves every finding it surfaces across the entire workflow set — covering template injection, excessive permissions, credential persistence, cache poisoning, and secrets scoping.
|
| Filename | Overview |
|---|---|
| .github/workflows/_test.yml | Added optional CODECOV_TOKEN secret declaration and persist-credentials: false to checkout — correct and complete |
| .github/workflows/create-tag.yml | Template injection fixed (tag name moved to env vars), artipacked suppression correctly documented; job-level permissions: contents: write present |
| .github/workflows/release.yml | Template injection fixed (version/INPUT_VERSION moved to env vars), persist-credentials: false added; workflow lacks a top-level permissions block unlike most other hardened workflows |
| .github/workflows/publish-pypi-approval.yml | Template injection fixed, persist-credentials: false added, dangerous-triggers suppressed with rationale; workflow lacks top-level permissions block unlike most other hardened workflows |
| .github/workflows/triage-label.yml | Permissions correctly moved from workflow-level to per-job scope; actions/github-script@v7 tag pin not upgraded to SHA unlike the same action in the analogous pr-merge-guidance.yml |
| .github/workflows/pr-tests.yml | secrets: inherit replaced with explicit CODECOV_TOKEN pass-through; top-level contents: read added |
| .github/workflows/pr-merge-guidance.yml | dangerous-triggers suppression added with correct rationale; job uses SHA-pinned github-script and never checks out PR head code |
| .github/workflows/docs-build.yaml | package-manager-cache: false added to three setup-node steps, closing a cache-poisoning vector on fork-PR-triggered builds |
| .github/workflows/test-and-build-wheel.yml | cache-poisoning suppression correctly documented (no pull_request trigger), persist-credentials: false added, contents: read scoped |
| .github/zizmor.yml | New zizmor config allowing ref-pin (tag) for all actions, matching Dependabot-maintained tag-pin convention |
| .pre-commit-config.yaml | zizmor pre-commit hook added at pinned v1.25.2 — gating CI via make pre-commit in lint.yml is correct |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Dev as Developer / PR Author
participant GH as GitHub Actions
participant PreCommit as pre-commit (zizmor)
participant LintCI as lint.yml (make pre-commit)
participant Workflows as Hardened Workflows
Dev->>PreCommit: git commit / pre-commit run
PreCommit->>PreCommit: zizmor v1.25.2 scans .github/workflows/
alt findings present
PreCommit-->>Dev: Block commit, report violations
else clean
PreCommit-->>Dev: Pass
end
Dev->>GH: Push / open PR
GH->>LintCI: Trigger lint.yml
LintCI->>PreCommit: make pre-commit (includes zizmor)
PreCommit-->>LintCI: No findings (default severity)
LintCI-->>GH: Pass
GH->>Workflows: Trigger workflow (pull_request / push / workflow_run)
note over Workflows: permissions scoped per job<br/>persist-credentials: false<br/>${{ }} in run: moved to env: vars<br/>secrets: inherit replaced with explicit token
Workflows-->>GH: Executes with least-privilege token
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Dev as Developer / PR Author
participant GH as GitHub Actions
participant PreCommit as pre-commit (zizmor)
participant LintCI as lint.yml (make pre-commit)
participant Workflows as Hardened Workflows
Dev->>PreCommit: git commit / pre-commit run
PreCommit->>PreCommit: zizmor v1.25.2 scans .github/workflows/
alt findings present
PreCommit-->>Dev: Block commit, report violations
else clean
PreCommit-->>Dev: Pass
end
Dev->>GH: Push / open PR
GH->>LintCI: Trigger lint.yml
LintCI->>PreCommit: make pre-commit (includes zizmor)
PreCommit-->>LintCI: No findings (default severity)
LintCI-->>GH: Pass
GH->>Workflows: Trigger workflow (pull_request / push / workflow_run)
note over Workflows: permissions scoped per job<br/>persist-credentials: false<br/>${{ }} in run: moved to env: vars<br/>secrets: inherit replaced with explicit token
Workflows-->>GH: Executes with least-privilege token
Comments Outside Diff (1)
-
.github/workflows/release.yml, line 20-26 (link)No top-level
permissions:block onrelease.ymlMost workflows in this PR received a top-level
permissions:block (e.g.contents: readorpermissions: {}) in addition to any job-level scope.release.ymlonly has a job-level declaration (contents: write, pull-requests: write) and no workflow-level restriction. Without the top-level block, any hypothetical future job added to this file without explicit permissions would inherit the repository default token — which could be broad. The same pattern is absent frompublish-pypi-approval.yml. Addingpermissions: {}at the workflow level (letting each job override with only what it needs) would be consistent with the rest of the hardening applied here.Prompt To Fix With AI
This is a comment left during a code review. Path: .github/workflows/release.yml Line: 20-26 Comment: **No top-level `permissions:` block on `release.yml`** Most workflows in this PR received a top-level `permissions:` block (e.g. `contents: read` or `permissions: {}`) in addition to any job-level scope. `release.yml` only has a job-level declaration (`contents: write, pull-requests: write`) and no workflow-level restriction. Without the top-level block, any hypothetical future job added to this file without explicit permissions would inherit the repository default token — which could be broad. The same pattern is absent from `publish-pypi-approval.yml`. Adding `permissions: {}` at the workflow level (letting each job override with only what it needs) would be consistent with the rest of the hardening applied here. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
.github/workflows/triage-label.yml:32
**Tag-pinned `github-script` in `pull_request_target` context**
`actions/github-script@v7` is a mutable tag — its resolved SHA can change if the upstream tag is moved. The analogous `pr-merge-guidance.yml` (also `pull_request_target` with write permissions) was updated to use `actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3` — a SHA pin. For consistency with that hardening, this workflow should pin the action to a specific commit SHA as well, so a tag-move in the upstream repo cannot silently change the code executing with the PR write token.
### Issue 2 of 2
.github/workflows/release.yml:20-26
**No top-level `permissions:` block on `release.yml`**
Most workflows in this PR received a top-level `permissions:` block (e.g. `contents: read` or `permissions: {}`) in addition to any job-level scope. `release.yml` only has a job-level declaration (`contents: write, pull-requests: write`) and no workflow-level restriction. Without the top-level block, any hypothetical future job added to this file without explicit permissions would inherit the repository default token — which could be broad. The same pattern is absent from `publish-pypi-approval.yml`. Adding `permissions: {}` at the workflow level (letting each job override with only what it needs) would be consistent with the rest of the hardening applied here.
Reviews (1): Last reviewed commit: "ci: run zizmor at default severity and r..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughApplies repository-wide GitHub Actions security hardening: introduces zizmor as a pre-commit linter with tag-based pinning policy, adds explicit ChangesGitHub Actions Security Hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
1 participant
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.
Description
Adds zizmor as a pre-commit hook (also enforced in CI via
lint.yml'smake pre-commit) to lint GitHub Actions workflows, runs it at zizmor'sdefault severity, and resolves every finding it surfaces. Our workflows use privileged
triggers (
pull_request_target,workflow_run), publish to PyPI, and push tags, so gatingthem catches credential-persistence, template-injection, excessive-permission, and
cache-poisoning mistakes before they ship.
Tooling
zizmorpre-commit hook (pinnedv1.25.2), at default severity..github/zizmor.yml:unpinned-uses: ref-pinso tag pins (@v6, kept current byDependabot) satisfy the linter instead of requiring SHA pins.
Least-privilege permissions (
excessive-permissions)permissions:blocks added to workflows that relied on the default token:contents: readwhere a checkout runs (incl. callers of the reusable_test.yml),permissions: {}for pure status / install-from-PyPI / token-publish jobs, andcontents: write+pull-requests: readfordocs-remove-stale-reviews(its reusableworkflow pushes to
gh-pages).Injection / credential hardening
${{ … }}values moved out ofrun:bodies intoenv:/ shell vars(
template-injection).persist-credentials: falseon checkouts that don't push (artipacked).package-manager-cache: falseonsetup-nodeindocs-build(closes a reachablecache-poisoning vector —
build-docsruns on fork PRs).pr-testspasses onlyCODECOV_TOKENto_test.ymlinstead ofsecrets: inherit(
secrets-inherit);_test.ymldeclares it as an optionalworkflow_callsecret.Documented suppressions (inline
# zizmor: ignore[...], each with rationale)dangerous-triggers×3 (triage-label,pr-merge-guidance,publish-pypi-approval) —none check out or execute PR-head code; event data read as data;
workflow_runupstreamhas no PR trigger.
cache-poisoning(test-and-build-wheel) — nopull_requesttrigger; trusted-ref-onlyruns, so fork PRs can't write the cache these runs restore.
artipacked(create-tag) — needs the checkout's persisted credentials togit pushthe release tag; uploads no artifacts.
use-trusted-publishing(publish-wheel) — intentional API-token publish; OIDC migrationtracked separately.
Areas needing careful review: the
permissions:scopes (esp.docs-remove-stale-reviews,which must keep
contents: writefor itsgh-pagespush) and thesecrets-inheritchangeto the
_test.ymlinterface.Verification
develop.uvx zizmor .github/workflows/(default severity) →No findings to report across the full workflow set, including workflows
developrecently added.
pre-commit run --all-filespasses (incl. the newzizmorhook).secrets-inheritchange can only be confirmedon the first CI run — verify the Codecov upload still succeeds on the coverage job
(Python 3.11). Logic is backward-compatible; residual risk is low.
AI Assistance
Checklist
Summary by CodeRabbit
Release Notes
Security
Infrastructure