Skip to content

Watch the six pins nothing was watching — zizmor is four minors behind and it audits these workflows (#373) - #374

Merged
VijitSingh97 merged 1 commit into
developfrom
feat/373-watch-remaining-pins
Aug 19, 2026
Merged

Watch the six pins nothing was watching — zizmor is four minors behind and it audits these workflows (#373)#374
VijitSingh97 merged 1 commit into
developfrom
feat/373-watch-remaining-pins

Conversation

@VijitSingh97

Copy link
Copy Markdown
Contributor

Closes #373.

The audit, as a natural experiment

Every pin the repo carries, checked against upstream today:

Watched — all current: XMRig (xmrig-bump.yml), shellcheck, shfmt, gitleaks, kcov
(tool-bump.yml), GitHub Actions (dependabot).

Unwatched — five of six stale:

tool pinned upstream gap
zizmor 1.25.2 v1.29.0 4 minors
pre-commit-hooks v5.0.0 v6.0.0 1 major
actionlint 1.7.7 v1.7.12 5 patches
diff-cover 10.3.0 v10.5.1 1 minor
jq 1.7.1 jq-1.8.2 1 minor
lychee 0.24.2 lychee-v0.24.2 current, by luck

Watched → current. Unwatched → stale. Nothing about these was harder to watch than the five that
already were; they were never added.

zizmor is the one that matters. It audits these workflows, it queries the GitHub Advisory
Database so a CVE against a pinned action fails the gate, and it blocks the merge. Four minors
behind means we have been auditing our own supply chain with a scanner missing four releases of
rules, with no signal anywhere that this was so. security.yml pins two tools — gitleaks was
watched and current, zizmor was neither.

What this does

Five join tool-bump.yml in its existing idiom — newer_release + open_pr, one PR per stale pin,
the dispatched checks on the branch as the verification. pre-commit-hooks goes to dependabot
instead, which supports that ecosystem natively; that beats hand-rolling a rev: rewriter and gives
the gitleaks rev: a second opinion alongside the lockstep sed already here.

XMRig stays with xmrig-bump.yml, unchanged. It is the only pin shipped to a rig, it is current, and
it is current because that watcher exists — which is the argument for this PR rather than against
it.

The trap, and why the diff is not just five copy-pasted blocks

newer_release does latest="${latest#v}" and then validates ^[0-9]+(\.[0-9]+)*$, returning 2
on a mismatch, which the callers turn into failed=1.

  • lychee tags lychee-v0.24.2
  • jq tags jq-1.8.2

Adding either as-is makes the shape check fail every week, on a repo whose pins are fine. So
newer_release takes a per-tool tag prefix, defaulted to v so every existing caller is
byte-identical — and the strict validation still runs after the strip. Normalising the spelling
must not mean trusting the value.

What was run

The comparison logic, driven against real upstream, using the exact cur= extraction expression
each new branch uses:

actionlint   cur=1.7.7     rc=0  -> 1.7.12
lychee       cur=0.24.2    rc=0  -> (current)
jq           cur=1.7.1     rc=0  -> 1.8.2
zizmor       cur=1.25.2    rc=0  -> 1.29.0
diff-cover   cur=10.3.0    rc=0  -> 10.5.1
--- regression: the three existing callers ---
shellcheck   cur=0.11.0    rc=0  -> (current)
shfmt        cur=3.13.1    rc=0  -> (current)
gitleaks     cur=8.30.1    rc=0  -> (current)

Every cur extracted correctly, both prefixes resolved, and the three existing callers are
unchanged.

Mutation — drop the prefix argument (i.e. today's behaviour):

lychee: SHAPE-FAIL on 'lychee-v0.24.2' -> return 2 -> failed=1 -> WORKFLOW GOES RED
jq:     SHAPE-FAIL on 'jq-1.8.2'       -> return 2 -> failed=1 -> WORKFLOW GOES RED

So the new argument is load-bearing, not decoration.

Every sed applied to the real file — all eight rewrites land and produce the expected string
(a sed that silently matches nothing would open a no-op bump PR):

actionlint-ver ✓  actionlint-sha ✓  diff-cover ✓  lychee-ver ✓
lychee-sha ✓      zizmor ✓          jq-url ✓      jq-sha ✓

The artifact URLs the bumps would fetch all resolve (so artifact_sha has something to
checksum): actionlint_1.7.12_linux_amd64.tar.gz 200, jq-linux-amd64 200,
lychee-x86_64-unknown-linux-gnu.tar.gz 200.

make lint-actions clean. yamllint --strict clean on both changed files.

What was NOT run

  • The workflow itself has not executed with these branches. They can only run once this is on
    develop. Dispatch it after merge and confirm the PRs it opens — I would rather that be a
    deliberate step than a discovery next Monday.
  • make test was not run: no shell that the suite covers changed. The diff is two YAML files.
  • Expect five bot PRs on the first run, since five pins are stale right now. That is the honest
    backlog, not a bug; each dedups on its own branch afterwards.

Not a defect in what exists

I went looking. tool-bump.yml's failure accounting is correct: a failed gh api leaves an empty
tag, the shape check rejects it, newer_release returns 2, and else [ $? -gt 1 ] && failed=1
records it — an unreachable upstream cannot read as "pin is current". xmrig-bump.yml validates the
tag before it reaches a ref and re-checks the tag→commit mapping against the build. Both are good
designs; this was only ever about coverage.

Sibling work: pithead #1128 /
PR #1143 closed the same gap there —
one shape, two outputs: a build-verified PR here where the tool gates its own bump, a report-only
tracking issue there where a component bump is a data migration.

Every pin in the repo, checked against upstream: the ones with a watcher were
current, and five of the six without one had gone stale.

  zizmor            1.25.2  -> 1.29.0   four minors
  pre-commit-hooks  v5.0.0  -> v6.0.0   a major
  actionlint        1.7.7   -> 1.7.12
  diff-cover        10.3.0  -> 10.5.1
  jq                1.7.1   -> 1.8.2
  lychee            0.24.2             current, by luck rather than process

zizmor is the one that matters: it audits these very workflows and it gates the
merge, so four minors behind means auditing our own supply chain with a scanner
missing four releases of rules. security.yml pins two tools; gitleaks was
watched and current, zizmor was neither.

Five join tool-bump.yml in its existing idiom. pre-commit-hooks goes to
Dependabot, which supports that ecosystem natively — better than hand-rolling a
`rev:` rewriter, and it gives the gitleaks `rev:` a second opinion alongside the
lockstep sed already here.

The work is the normalisation, not the fetch. lychee tags `lychee-v0.24.2` and
jq tags `jq-1.8.2`; newer_release strips only a leading `v` and then validates
the shape, so adding either as-is returns 2, sets failed=1, and turns the run
red every week on a repo whose pins are fine. Hence a per-tool prefix argument,
defaulted so every existing caller is byte-identical, with the strict validation
still running after the strip — normalising the spelling must not mean trusting
the value.
@VijitSingh97

Copy link
Copy Markdown
Contributor Author

Adversarial review of my own PR

Attack 1 — I asserted dependabot supports a pre-commit ecosystem. Does it?

That was the one claim in this diff I had taken on faith, and a wrong ecosystem name does not error —
it is silently ignored, which would be a watcher that looks added and watches nothing. Checked
against GitHub's supported-ecosystems documentation: pre-commit is a real, generally-available
identifier.

One caveat the docs make explicit and worth writing down: it supports version updates only, not
security updates
. So a CVE against a hook repo will not raise an alert-driven PR; the weekly
version bump is what we get. That is still strictly more than the nothing we had, but it is not
equivalent to the actions ecosystem beside it.

Attack 2 — five stale pins means open_pr runs five times in one run, which has probably never happened

The existing workflow has three tool blocks and its pins have been current, so the multi-branch path
may never have executed more than once in a single run. This PR takes it to nine blocks with five
stale today — so the first run exercises it for real.

Reproduced the exact branch dance offline with three stale pins (push and gh removed; the git
sequence is what is under test):

block A starts from: A_VERSION: "1.0.0"   (tree dirty? 0 files)
block B starts from: B_VERSION: "2.0.0"   (tree dirty? 0 files)
block C starts from: C_VERSION: "3.0.0"   (tree dirty? 0 files)

develop is UNTOUCHED:  A 1.0.0 / B 2.0.0 / C 3.0.0
bot/A-1.1.0:  A 1.1.0  B 2.0.0 | C 3.0.0
bot/B-2.1.0:  A 1.0.0  B 2.1.0 | C 3.0.0
bot/C-3.1.0:  A 1.0.0  B 2.0.0 | C 3.1.0

Each block starts from a clean develop, each bot branch carries only its own bump, and no branch
picks up a sibling's edit — including the case that worried me most, two blocks editing the same
file
(A and B both in ci.yml, which is real here: actionlint, diff-cover, shellcheck and shfmt
all live in ci.yml).

Attack 3 — one bad artifact URL kills the rest of the run

sha=$(artifact_sha …) under set -Eeuo pipefail aborts the whole step, so a 404 on one tool's
release asset means the tools after it are never checked that week. Pre-existing behaviour, not
introduced here, and aborting loudly is the right failure mode — but the blast radius grows from
three blocks to nine, so it is worth stating rather than discovering.

Mitigating it today: all three artifact URLs the currently-stale bumps would fetch return 200
(actionlint 1.7.12, jq 1.8.2, lychee 0.24.2). Not a fix, just evidence the first run will not hit it.

Attack 4 — the zizmor bump PR will probably be red, on purpose

A newer zizmor can raise findings a four-minors-older one did not, so bot/zizmor-1.29.0 may open
with its own Workflow audit check failing. That is the mechanism working — the whole reason the pin
being stale is a problem — but nobody should read it as the bot being broken. The PR body it writes
says so.

Attack 5 — do the extraction expressions actually pull the right string?

This is where a silent no-op bump would come from: a grep -oE that matches the wrong thing gives an
empty cur, the sed then matches nothing, and the bot opens a PR that changes no version. Ran each
one against the real file — every cur came out correct (1.7.7, 0.24.2, 1.7.1, 1.25.2,
10.3.0), and separately every sed expression was applied to the real file and confirmed to change
it and produce the expected string. Both runs are in the PR body.

The residual I cannot close here

This workflow cannot run with these blocks until it is on develop — same shape as the sibling
pithead PR. Dispatch it once after merge and read what it opens, rather than letting Monday be the
first time anyone sees it. Five PRs at once is the expected honest backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Six pins have no watcher, and five of them are stale — including zizmor (4 minors) and pre-commit-hooks (a major)

1 participant