Skip to content

ci: lint the workflows and fix what it finds - #552

Merged
LKSNDRTMLKV merged 4 commits into
mainfrom
ci/workflow-lint
Oct 30, 2026
Merged

LKSNDRTMLKV merged 4 commits into
mainfrom
ci/workflow-lint

Conversation

@LKSNDRTMLKV

@LKSNDRTMLKV LKSNDRTMLKV commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Adds static analysis of the workflows, and fixes what it found. dpp-core got the same in its #455; the engine's five workflows had never been through either tool.

The new workflow

workflow-lint.yml runs zizmor (security) and actionlint (correctness) on every pull request, on main, weekly (Monday 05:00 UTC) and on demand. It is not path-filtered, so either job can be made a required check without blocking pull requests that leave .github/ alone; that is a ruleset decision and this PR does not make it. The default regular persona gates; pedantic does not.

Both tools are downloaded from their release and checked against a SHA-256 recorded in the workflow: zizmor 1.30.1 and actionlint 1.7.12, the versions dpp-core pins, with the digests GitHub publishes for those assets. Dependabot cannot see either pin, so moving one is a deliberate edit on its own PR. zizmor gets the workflow's own read-only token for its online audits (whether a pinned commit belongs to the repository it names, published advisories, whether a version comment matches its pin).

What the tools reported, and what changed

A local run over the five workflows on main (zizmor offline, regular persona) gave 27 findings. The first CI run of the new jobs, which is the first time the online audits and shellcheck saw these files, added eight more. All are fixed.

Finding Where Change
artipacked: the checkout keeps the job token 24 of 25 actions/checkout steps (22 in ci.yml, one each in advisories.yml and release.yml; only fuzz.yml set it) persist-credentials: false on all 24. No job needs the credentials: the one git fetch in CI (scripts/migrations-check.sh) reads a public repository and is already || true.
dependabot-cooldown the cargo, docker and github-actions entries cooldown: default-days: 7, as in dpp-core
dtolnay/rust-toolchain pinned to a commit upstream no longer has (found by hand; the audit for it is online-only) all 9 uses pinned 4cda84d5…, a commit on upstream's stable branch from 2026-07-16; GitHub's compare against master reports it diverged, 1 ahead and 14 behind pinned to the master commit 683b588c…, master's head today. Every use already passed toolchain:, so the same code runs. No branch-name comment, since it would go stale.
release.yml granted packages: write to the whole workflow release.yml granted on the publish job, the only job there. Its effective permissions are unchanged.
ref-version-mismatch (online), 7 sites every Swatinem/rust-cache pin, c8037a86… # v2 The pin was not a release: it is one commit ahead of v2.9.3, reached through Dependabot's grouped bump #512, and v2 is a moving tag that points at v2.9.3. Now d1527c8f… # v2.9.3, as in dpp-core. This moves rust-cache back one unreleased upstream commit, "include a virtual workspace's root Cargo.toml in the cache key". The engine's root manifest is virtual, so until the next release an edit to that file alone may not change a cache key; Cargo.lock is still in the key.
shellcheck SC2012 (via actionlint) fuzz.yml, ls "$corpus" | wc -l find "$corpus" -mindepth 1 -maxdepth 1 | wc -l: the same count of the same plain files.

One change that no tool asked for: eight pinned actions carried a major-only comment (# v7, # v4, # v6), which is true only while upstream leaves that tag alone. Each is now the exact release tag of the commit it pins (checkout v7.0.1, upload-artifact v7.0.2, setup-node v7.1.0, setup-buildx-action v4.4.1, login-action v4.7.0, metadata-action v6.2.0, build-push-action v7.4.0), each looked up with git ls-remote against the upstream repository. It belongs here because of the cooldown above: a release moves its major tag at once while Dependabot now waits a week to propose it, and over that week the ref-version-mismatch audit would fail every run. dpp-core comments exact versions for the same reason.

Left alone

pedantic mode also reports 9 superfluous-actions (actions a shell command could replace) and 2 unpinned-images (container images pinned by tag, not by digest). Neither is gated, in the engine or in core.

Checked

  • just check exits 0 on the branch tip: 1636 of 1636 tests, including ci-guard-check and fuzz-targets-check, the two guard scripts that read the workflow files.
  • zizmor (offline, regular) and actionlint report nothing on the workflows as changed here; before, zizmor exited 13. The online findings above were not visible offline, which is why the first commit failed these two jobs.
  • Each tool fails on input made to fail: a workflow with an unpinned action, a run: that interpolates github.event.issue.title, a step output that does not exist and a misspelt context.
  • Every version comment in the workflows was checked against upstream: each tag points at the commit it sits beside.
  • The tarballs were checked against the recorded SHA-256 before they were unpacked or run.

Not run locally: zizmor's online audits (no token goes to a downloaded binary) and shellcheck, so the second CI run is the confirmation.

bench.yml

bench.yml reached main after this branch was cut and came in with the merge of main, still on the old rust-toolchain commit and rust-cache at c8037a86… # v2, so the online audit failed on it. The last commit pins it as the other workflows are pinned here: rust-toolchain 683b588c…, rust-cache d1527c8f… # v2.9.3, and exact release comments on checkout (v7.0.1) and upload-artifact (v7.0.2).

@LKSNDRTMLKV LKSNDRTMLKV added the security Security-relevant issue label Oct 30, 2026
@LKSNDRTMLKV LKSNDRTMLKV added this to the v0.15.0 milestone Oct 30, 2026
@LKSNDRTMLKV
LKSNDRTMLKV merged commit cd8cd05 into main Oct 30, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security Security-relevant issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant