ci: address zizmor findings across workflows#11608
Conversation
Resolves all 30 zizmor alerts reported on main after #11607: - template-injection (19): move `${{ ... }}` interpolations in `run:` blocks to `env:` so untrusted-ish values (workflow_dispatch inputs, github.ref_name, github.actor) can't break out of shell quoting. - artipacked (8): add `persist-credentials: false` to `actions/checkout` in audit, benchmark, ci, codeql-analysis, docker, release, test workflows. `update-lockfile.yml` keeps the persisted token (later step pushes to a branch) with a `zizmor: ignore[artipacked]` comment and justification. - dependabot-cooldown (1): add a 7-day cooldown so brand-new (potentially malicious) Actions releases don't get auto-PR'd day-of-release. - ref-version-mismatch (1): `bluwy/release-for-reddit-action` SHA pointed at the `v2` tag, not a non-existent `v2.0.0`. Fix the comment. - superfluous-actions (1): mark `softprops/action-gh-release` with a `zizmor: ignore` and justification — the release pipeline is sensitive and the action is battle-tested; we're not swapping it for `gh release` here. Verified locally with `zizmor --persona regular .github` (online audits on): No findings to report. Good job! (2 ignored, 32 suppressed) --- Written by an agent (Claude Code, claude-opus-4-7).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (14)
📝 WalkthroughWalkthroughGitHub Actions workflows across the pnpm repository are hardened with credential persistence disabled in checkout steps, environment variable refactoring for safer parameter passing in benchmark and test execution, and configuration updates to dependabot and release automation tooling. ChangesGitHub Actions Security and Configuration Hardening
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the repository’s GitHub Actions and Dependabot configuration to remediate zizmor security findings introduced/flagged after adding zizmor scanning, primarily by hardening workflow shell execution and reducing credential persistence.
Changes:
- Mitigates potential workflow template injection by moving
${{ ... }}interpolations out ofrun:blocks intoenv:and referencing shell variables instead. - Reduces “credential persistence” findings by setting
persist-credentials: falseonactions/checkoutin workflows that don’t need to push. - Adds a 7-day cooldown to Dependabot updates and fixes an action pin comment to match the referenced version.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/update-lockfile.yml | Keeps persisted credentials intentionally for a later git push, with inline zizmor ignore + justification. |
| .github/workflows/update-latest.yml | Moves workflow inputs into env for safer shell usage; fixes pinned action comment version. |
| .github/workflows/test.yml | Disables persisted checkout credentials and moves GitHub context usage into env for shell steps. |
| .github/workflows/release.yml | Disables persisted checkout credentials; keeps softprops/action-gh-release with inline ignore + rationale. |
| .github/workflows/docker.yml | Disables persisted checkout credentials (no push needed). |
| .github/workflows/codeql-analysis.yml | Disables persisted checkout credentials for CodeQL workflow. |
| .github/workflows/ci.yml | Disables persisted checkout credentials for CI workflow. |
| .github/workflows/benchmark.yml | Disables persisted checkout credentials; moves GitHub context usage into env for shell steps. |
| .github/workflows/audit.yml | Disables persisted checkout credentials for audit workflow. |
| .github/dependabot.yml | Adds Dependabot cooldown to reduce update churn per zizmor recommendation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Without an explicit shell, the step ran under PowerShell on windows-latest, where `$TEST_SCRIPT` is not a variable (PowerShell exposes env vars as `$env:TEST_SCRIPT`). `pn run ""` then exited 0 and just listed available scripts — the Windows test legs have been silently no-op'ing since the env-var move in #11608.
…1659) Without an explicit shell, the step ran under PowerShell on windows-latest, where `$TEST_SCRIPT` is not a variable (PowerShell exposes env vars as `$env:TEST_SCRIPT`). `pn run ""` then exited 0 and just listed available scripts — the Windows test legs have been silently no-op'ing since the env-var move in #11608. The sibling `Verify Node version` and `Determine test scope` steps already pin `shell: bash`; this brings `Run tests` in line. --- Written by an agent (Claude Code, claude-opus-4-7).
…short names (#11661) `os.tmpdir()` on GitHub's Windows runners returns the 8.3 short-name form of the user-profile directory (e.g. `C:\Users\RUNNER~1\AppData\Local\Temp`) because `runneradmin` is longer than 8 characters. The `~` then trips the `quoteShellArg` allowlist regex and every test that calls `sendLineScript` or `generateSendStdinScript` throws "Unsupported character in shell argument". The tilde is safe to allow: - cmd.exe performs no tilde expansion at all. - POSIX shells only expand `~` when it is unquoted at the start of a word; inside the double-quoted `"${arg}"` wrapper produced here it is literal. The matching CodeQL shell-injection sanitization argument is unchanged — the allowlist is still anchored and still rejects every metacharacter. The bug was masked until #11659 because the Windows test legs had been silently no-op'ing since #11608. --- Written by an agent (Claude Code, claude-opus-4-7).
…pm#11659) Without an explicit shell, the step ran under PowerShell on windows-latest, where `$TEST_SCRIPT` is not a variable (PowerShell exposes env vars as `$env:TEST_SCRIPT`). `pn run ""` then exited 0 and just listed available scripts — the Windows test legs have been silently no-op'ing since the env-var move in pnpm#11608. The sibling `Verify Node version` and `Determine test scope` steps already pin `shell: bash`; this brings `Run tests` in line. --- Written by an agent (Claude Code, claude-opus-4-7).
…short names (pnpm#11661) `os.tmpdir()` on GitHub's Windows runners returns the 8.3 short-name form of the user-profile directory (e.g. `C:\Users\RUNNER~1\AppData\Local\Temp`) because `runneradmin` is longer than 8 characters. The `~` then trips the `quoteShellArg` allowlist regex and every test that calls `sendLineScript` or `generateSendStdinScript` throws "Unsupported character in shell argument". The tilde is safe to allow: - cmd.exe performs no tilde expansion at all. - POSIX shells only expand `~` when it is unquoted at the start of a word; inside the double-quoted `"${arg}"` wrapper produced here it is literal. The matching CodeQL shell-injection sanitization argument is unchanged — the allowlist is still anchored and still rejects every metacharacter. The bug was masked until pnpm#11659 because the Windows test legs had been silently no-op'ing since pnpm#11608. --- Written by an agent (Claude Code, claude-opus-4-7).
Summary
Resolves all 30 zizmor alerts reported on
mainafter #11607 wired up the scanner.template-injection(error)${{ … }}toenv:and reference shell varsartipacked(note)persist-credentials: falsetoactions/checkout(1 inline-ignored)dependabot-cooldown(warning)dependabot.ymlref-version-mismatch(warning)v2, notv2.0.0superfluous-actions(note)Notable decisions
update-lockfile.ymlkeeps the persisted checkout token because the latergit push -f origin chore/update-lockfilestep depends on it. The line has# zizmor: ignore[artipacked]plus a comment explaining why.release.ymlkeepssoftprops/action-gh-release. zizmor's suggestion (usegh release create) is valid in principle, but the release pipeline is sensitive and the action is battle-tested. Inline-ignored with justification.test.yml:76(runningpn run ${{ steps.test-scope.outputs.script }}) was technically a false positive — the upstream step hardcodes the output to one of two literals — but moved toenv:anyway for consistency and to keep the rule on by default.Verification
Ran zizmor locally against the modified workflows (with online audits enabled):
The 2 "ignored" are the inline-justified
release.ymlandupdate-lockfile.ymlcases above. After merge, the zizmor workflow itself will re-run onmainand the Code scanning dashboard should drop to 0 open alerts.Test plan
Written by an agent (Claude Code, claude-opus-4-7).
Summary by CodeRabbit