ci: add actionlint + zizmor GitHub Actions linters#1545
Conversation
Add two lint jobs to edr-ci.yml: - actionlint via rhysd's official digest-pinned Docker image (bundles shellcheck + pyflakes), gating workflow correctness. - zizmor via taiki-e/install-action (the installer already used for cargo-hack), gating GitHub Actions security. Fails on findings; GH_TOKEN enables the online audits. Also: - .github/actionlint.yaml declares the self-hosted runner labels (edr-benchmark-runner, hardhat-linux-amd64-self-hosted) so actionlint stops flagging them as unknown. - Suppress two verified-safe zizmor github-env findings in the setup-rust and setup-llvm-cov composite actions (static crt-static flag; rustc host-tuple output — neither is external input). zizmor still reports 41 findings (26 excessive-permissions across workflows + 15 in edr-npm-release.yml); those are handled separately.
Resolves zizmor's excessive-permissions findings by declaring an explicit permissions: block on every workflow that relied on the default token scopes. Scoped to what each actually needs: - contents: read for the read-only workflows (edr-ci, check-generated-files, edr-benchmark, hardhat-tests, hardhat-regression-tests, and the two scheduled test-recent-*-block replays). Benchmark push and Slack notify use PAT/webhook secrets, not GITHUB_TOKEN. - pull-requests: read for check-changeset-added (only reads PR files/labels). - mdbook: keep the workflow read-only and move pages: write + id-token: write down to the deploy job that needs them. Also drops the now-redundant per-job permissions blocks on the actionlint and zizmor jobs (they inherit the new top-level contents: read). Remaining zizmor findings are confined to edr-npm-release.yml and handled separately.
Fix the template-injection findings that expand needs.* outputs into run: blocks by routing them through env: (no behaviour change): - needs.prepare.outputs.filename -> $FILENAME in both Decompress steps - needs.check_commit.outputs.tag -> $TAG in the Publish step Fix two cache-poisoning findings by disabling caching on the publish path (a publication workflow must not restore attacker-poisonable caches): - validation job's setup-node: package-manager-cache: false - cooldown-check job: drop the actions/cache step (also closes a small gap where a poisoned cache could hide a too-fresh dependency at release) Suppress the remaining findings with justifications: - template-injection on strategy.job-total (a controlled integer) - cache-poisoning on the build job's cargo cache: releases build cold (guarded by tag == ''), so the published artifact never comes from cache; zizmor just can't statically evaluate the needs.*-based guard - superfluous-actions x2: addnab/docker-run-action is a deliberate cross-arch (arm64 / arm64-musl) test runner - adhoc-packages: pinned npm@11.6.2 required for Trusted Publishing zizmor now reports no findings across .github.
Trim the comments added with the linters to the non-obvious bit only: drop "exits non-zero -> blocks" narration and the Renovate meta note from the actionlint/zizmor jobs, collapse the permissions rationales to their surprising part (PAT / CODECOV_TOKEN), and remove the mdbook comment that pointed at another job instead of explaining itself locally.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1545 +/- ##
==========================================
- Coverage 79.67% 79.66% -0.02%
==========================================
Files 446 446
Lines 76647 76647
Branches 76647 76647
==========================================
- Hits 61072 61064 -8
- Misses 13457 13466 +9
+ Partials 2118 2117 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Prettier reformats `run: | # zizmor: ignore[...]` to `run:\n | # ...`; adopt that form so both prettier --check and the zizmor suppression are satisfied (the ignore comment must stay on the run node, which the block scalar indicator line still is).
actionlint's Docker image runs shellcheck over run: blocks, surfacing
pre-existing findings. Fix the real ones and quiet the rest:
- SC2145 (error): echo "...${ALL_FILES[@]}" -> ${ALL_FILES[*]} in edr-npm-release
- SC2046 (warning): quote "$(pwd)" in the two docker-run test steps
- SC2166 (warning): `[ p -o q ]` -> `[ p ] || [ q ]` in the release-commit check
- SC2011 (warning): disable on the fixed-name .node artifact count
- SC1090 (warning): disable on `source <(cargo llvm-cov show-env)` (dynamic,
not a static file) in edr-ci and hardhat-tests
- SC2086 (info): drop info-level noise via SHELLCHECK_OPTS=--severity=warning
on the actionlint job
Verified locally with shellcheck on PATH (the earlier "green" run had none
installed, so the run:-block analysis was silently skipped).
Based on successful usage for Slang, I've added
actionlintandzizmoras they respectively catch syntax and security issues in GitHub actions.Claude summary
Adds two static-analysis gates for the repo's own GitHub Actions, which had none before:
runs-onlabels, and shellcheck overrun:blocks).Both run as blocking jobs in
edr-ci.ymland fail the check on any finding.How they run
uses: docker://rhysd/actionlint@<digest>taiki-e/install-actionwithtool: zizmor@1.26.1cargo-hack.GH_TOKENenables the online audits.Both are separate jobs with
permissions: contents: readand noneeds:, so they run in parallel with the rest of CI..github/actionlint.yamldeclares the two self-hosted runner labels (edr-benchmark-runner,hardhat-linux-amd64-self-hosted) soruns-on:isn't flagged as unknown.Findings resolved
The first run surfaced 43 zizmor findings (plus 4 actionlint). All are resolved so both gates land green:
excessive-permissions(26) — added explicit least-privilegepermissions:blocks across 9 workflows. Scoped per workflow:contents: readfor the read-only ones (benchmark push and Slack notify use PAT/webhook secrets, notGITHUB_TOKEN);pull-requests: readforcheck-changeset-added; and mdbook'spages/id-token: writemoved down to the deploy job.template-injection(7 of 9) — routedneeds.*outputs (filename, publishtag) throughenv:inedr-npm-release.yml; no behaviour change.cache-poisoning(2 of 3) — disabled caching on the publish path:package-manager-cache: falseon the validation job's setup-node, and removed the cooldown-check cache step (also closes a small gap where a poisoned cache could hide a too-fresh dependency at release).github-env(2) — suppressed as verified-safe (a static+crt-staticflag andrustc --print host-tupleoutput — neither is external input).Suppressions kept (with justifications)
cache-poisoningon the build job's cargo cache — releases already build cold (guarded byif: tag == ''), so the published artifact never comes from cache; zizmor just can't statically evaluate theneeds.*-based guard. Fixing it "for real" would drop PR build caching for zero security gain.template-injectiononstrategy.job-total— a controlled integer, not external input.superfluous-actions(×2) —addnab/docker-run-actionis a deliberate cross-arch (arm64 / arm64-musl) test runner.adhoc-packages— pinnednpm@11.6.2, required for Trusted Publishing.Verification
Ran both tools locally against
.github— actionlint and zizmor both report no findings.Note: CI-only change, so apply the "no changeset needed" label (otherwise
check-changeset-addedfails).