Pin all GitHub Actions to commits and add the zizmor linter - #55
Merged
Conversation
Follow-up to #51. Denis raised two things on that PR: whether `taiki-e/install-action` is trustworthy, and whether we should pin actions by SHA and run zizmor like other rubyatscale repos do. This does the pinning, adds the linter, and clears every finding it reports. ## zizmor `.github/workflows/zizmor.yml` is copied verbatim from shared-config, which is also what visualize_packs uses. It is a copied file rather than a `uses:` of a reusable workflow because shared-config's zizmor.yml has no `workflow_call` trigger, unlike its codeql.yml. It defaults to `advanced-security: true`, which runs zizmor in SARIF mode. zizmor exits 0 in that mode even when it has findings, so this reports into the Security tab and cannot turn the build red on its own. ## Pinning Every action in the repo is now pinned to a commit, with the version in a trailing comment: - `audit.yml`: `install-action` to v2.86.3 and `checkout` to v7.0.1. `checkout` was on v5, two majors behind the v7.0.1 that shared-config standardizes on. - `ci.yml`: three `checkout@v2` uses, also now v7.0.1. - `release.yml`: all 17 uses, via `[dist.github-action-commits]` in dist-workspace.toml. That file is generated by dist, so editing it directly does not survive the next `dist generate`; the config is the supported knob for this and landed in dist 0.29.0, and we pin 0.30.3. Each action stays on the major version dist 0.30.3 already emitted, so this is a pure pin with no behavior change. `dist generate --check` passes, and the regenerated diff touches nothing but the pinned refs. ## Hardening beyond pinning - `persist-credentials: false` on every checkout, so the job's token is not left behind in `.git/config` (zizmor's artipacked audit). - `permissions: {}` at the top of audit.yml, leaving the job to opt into `contents: read`. - `fallback: none` on install-action. Its default falls back to cargo-binstall, which it passes a token to, and can reach `cargo install` -- the exact path whose compile failure #51 fixed. This keeps it from quietly coming back if a download fails. On install-action itself: it verifies SHA256 checksums by default and attestations where upstream publishes them, and applies a dependency cooldown, none of which the `actions-rs/audit-check` it replaced did. ## What is ignored, and why `.github/zizmor.yml` ignores three audits, scoped to release.yml only: excessive-permissions, template-injection, unpinned-images. All ten are structural to dist's template with no config knob, and dist would overwrite inline `# zizmor: ignore` comments. They are file-scoped rather than line-scoped because regeneration shifts line numbers. Reasoning for each is in the file. Net: 50 findings before, 26 after the pins and hardening, 0 with the documented ignores. `cargo test` passes.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
dduugg
enabled auto-merge (squash)
August 19, 2026 22:21
dkisselev
approved these changes
Aug 19, 2026
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.
Follow-up to #51. Review there raised two questions: is
taiki-e/install-actiontrustworthy, and should we pin actions by SHA and run zizmor like other rubyatscale repos. This does the pinning, adds the linter, and clears everything it reports.On
taiki-e/install-actionWorth stating plainly: it had no precedent in either org —
taiki-eappears nowhere inGustoorrubyatscale. So the ask for due diligence was fair. What I found:cargo-hack,cargo-llvm-cov,pin-project); Apache-2.0, 532 stars, released v2.86.3 the day before this PR.casey/just,orhun/git-cliff,rolldown/rolldown.actions-rs/audit-check, which it replaced, did none of these.I also found one gap #51 left open: the default
fallback: cargo-binstall. On a failed download it falls back to cargo-binstall (which gets a token passed to it) and can reachcargo install— the exact compile path whose failure #51 existed to fix. This setsfallback: noneso that cannot quietly come back.zizmor
.github/workflows/zizmor.ymlis copied verbatim from shared-config, byte-identical to the one visualize_packs uses. It's a copied file rather than auses:of a reusable workflow because shared-config'szizmor.ymlhas noworkflow_calltrigger — unlike itscodeql.yml, which we do call.This cannot turn the build red on its own. The action defaults to
advanced-security: true, which runs zizmor in SARIF mode, and zizmor exits 0 in that mode even with findings (plain mode exits 14). Verified against all workflows with findings present: exit 0. Results land in the Security tab.Pinning
Every action in the repo is now pinned to a commit, version in a trailing comment.
audit.ymlinstall-action→ v2.86.3;checkoutv5 → v7.0.1ci.ymlcheckout@v2→ v7.0.1release.ymlcheckoutwas on v5, two majors behind the v7.0.1 shared-config standardizes on. I confirmed the pinned SHAs expose the inputs used (tool/checksum/fallback;3d3c42e5is "prep v7.0.1 release").release.yml is generated
release.ymlis autogenerated by dist (cargo-dist), so editing it directly does not survive the nextdist generate. The supported knob is[dist.github-action-commits]indist-workspace.toml— it landed in dist 0.29.0 and we pin 0.30.3, so no version bump was needed.Each action stays on the major version dist 0.30.3 already emitted, making this a pure pin with no behavior change. I installed dist 0.30.3 and verified:
dist generate --checkpassed before any change, so dist reproduces the checked-in file exactly.dist generate --checkpasses now, so the file and config are in sync.Hardening beyond pinning
persist-credentials: falseon every checkout, so the job token isn't left in.git/config(zizmor'sartipacked).permissions: {}at the top ofaudit.yml, leaving the job to opt intocontents: read.What's ignored, and why
.github/zizmor.ymlignores three audits, scoped torelease.ymlonly:excessive-permissions,template-injection,unpinned-images. All ten findings are structural to dist's template with no config knob, and dist would overwrite inline# zizmor: ignorecomments. They're file-scoped rather than line-scoped because regeneration shifts line numbers. Per-audit reasoning is in the file.Briefly:
contents: writeis at workflow level because dist's announce job creates the Release; thetemplate-injectionhits are dist interpolatinggithub.ref_name/needs.plan.outputs.tag*/ its ownmatrix.*plan output, where the tag-derived values require push access and expand to''onpull_request; andunpinned-imagesismatrix.container.image, which is always null since this repo configures no custom container runners.One caveat found while validating: zizmor does not error on an unknown rule name in its config — a typo silently does nothing. So I verified the ignores take effect by diffing against
--no-configrather than trusting the file.Result
--no-config)cargo testpasses (0 failures), and all five touched YAML files parse.