The action-pin check reads uses: with a line grep, so a flow-style mapping is never checked

[vladimirrott]

tests/release/release-rehearsal.test.sh states the strongest supply-chain invariant in this repo: every uses: in every workflow pins a full 40-hex SHA. It reads those lines with a line-oriented grep:

$ grep -n "grep -E '\^\[\[:space:\]\]\*(-\[\[:space:\]\]+)?uses:'" tests/release/release-rehearsal.test.sh

That pattern reads uses: at the start of a line, with an optional - in front. YAML also allows a flow mapping, and GitHub Actions accepts it:

      - { uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 }

The grep does not match - { uses:, so an action written that way is never handed to the SHA assertion.

#432 is open against this file and adds a floor: the extraction has to produce at least twenty lines or the run fails. That closes the case the extraction breaks wholesale. It does not close this one, because fifty-five real lines clear a floor of twenty however many flow-style entries sit beside them.

Measured

Against 31b9f2d6, which is #432's head with the floor in place. One flow-style, unpinned entry added to docs.yml next to the fifty-five ordinary ones:

$ grep -n 'attacker/exfil' .github/workflows/docs.yml
39:      - { uses: attacker/exfil@main }
$ echo "uses: lines the extraction still sees: $(cat .github/workflows/*.yml | grep -cE '^[[:space:]]*(-[[:space:]]+)?uses:')"
uses: lines the extraction still sees: 55
$ podman run --rm --network=none -v "$PWD:/repo:z" -v "$HOME/.cargo:/cargo:O" -v "$HOME/.rustup:/rustup:O" -w /repo -e HOME=/tmp -e CARGO_HOME=/cargo -e RUSTUP_HOME=/rustup -e CARGO_NET_OFFLINE=true -e PATH=/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin localhost/sk-reh2:1 bash -c 'bash tests/release/release-rehearsal.test.sh; echo "rc=$?"'
Release rehearsal contract passed.
rc=0

The same mutation against main's copy of the test gives the same result, so this predates #432 and #432 neither creates it nor widens it.

Why it matters

Every other pin in this tree is checked. This one line is the difference between "every action is pinned, and a gate says so" and "every action a grep happened to notice is pinned". A workflow diff still gets a human read before it merges, which is the control that actually stands between a fork and the token, so this is a gate with a hole rather than a live hole. A gate with a hole is worth closing on its own terms: the whole reason it exists is so the human read is not the only thing.

The fix

Parse the YAML instead of grepping it. docs-and-hygiene already sets up Python 3.11 and #377 is landing a PyYAML-based workflow reader for a neighbouring check, so the dependency and the pattern are both about to exist here:

for job in doc.get("jobs", {}).values():
    for step in job.get("steps", []) or []:
        if "uses" in step:
            ...

That reads block and flow mappings identically, because by the time PyYAML is done there is no difference between them. Keep #432's floor: a parser that returns an empty list for a file it could not read has the same failure mode the floor was written for.

Two things to hold on to while replacing the grep:

• The ./... exemption. A reusable workflow in this repository is referenced by path and resolves at the caller's commit, so it cannot carry a SHA. The exemption must stay anchored to ./ so a third-party owner/repo/.github/workflows/x.yml@ref is still required to pin.
• A negative fixture per spelling. test(release): fail if the action-pin check inspects nothing #432 added one for the flow mapping; the replacement should keep it and watch it turn green for the right reason, which means asserting the parser finds that uses: rather than asserting the run fails.

Blocked on #432 landing, so the floor and the fixture are in the file first.

Found while reviewing #432.

Tests first

Break what the guard protects. Add a fixture workflow whose only uses: is a
flow-style mapping naming an unpinned action, point the check at it, and require
a non-zero exit that names the action. Write it before touching the extractor and
confirm it passes today, which is the bug. Measured at b2c823e6. This runs the
shipped assert_action_pins unchanged, against a copy of the workflows
directory, so nothing under .github/ is touched:

$ T=$(mktemp -d)
$ cp .github/workflows/*.yml "$T"/
$ sed -n '/^assert_action_pins() {/,/^}/p' tests/release/release-rehearsal.test.sh > "$T"/fn.sh
$ bash -c "source $T/fn.sh; assert_action_pins $T 20; echo rc=\$?"
rc=0
$ printf '      - { uses: attacker/exfil@main }\n' >> "$T"/docs.yml
$ echo "uses: lines the extraction sees: $(cat $T/*.yml | grep -cE '^[[:space:]]*(-[[:space:]]+)?uses:')"
uses: lines the extraction sees: 55
$ bash -c "source $T/fn.sh; assert_action_pins $T 20; echo rc=\$?"
rc=0
$ rm -rf "$T"

Fifty-five real lines, one invisible one, and the same pass before and after.

Break the guard's own input. Assert the count, not only the exit code. Give
the parser a fixture with a known number of uses: entries and require it to
report exactly that many, so a reader that silently stops matching turns a test
red rather than passing over nothing. Keep #432's floor for the same reason: a
PyYAML reader that returns an empty list for a file it could not parse fails in
the shape the floor was written for, and #442 is the same shape a third time.

Keep a negative fixture per spelling, and assert on what the parser found
rather than only on the run failing. A test that watches the run go red cannot
tell you whether it went red for the right reason.

Difficulty

medium. The parsing is short. The care goes into the ./ exemption for
reusable workflows in this repository, and into ordering this behind #432 so the
floor and its fixture are in the file first. No Rust, no VM, no daemon.

Getting started

CONTRIBUTING.md
has the build and test commands. bash tests/release/release-rehearsal.test.sh
is the one that matters here, and docs-and-hygiene already sets up Python 3.11
if you go the PyYAML route. No CLA and no copyright waiver. The project is MIT.

[vladimirrott]

Confirmed at b2c823e6, now that #432 has landed. The floor is in place and this gap is untouched by it.

I ran the shipped assert_action_pins against a copy of the workflows directory, so nothing under .github/ moved:

$ T=$(mktemp -d)
$ cp .github/workflows/*.yml "$T"/
$ sed -n '/^assert_action_pins() {/,/^}/p' tests/release/release-rehearsal.test.sh > "$T"/fn.sh
$ bash -c "source $T/fn.sh; assert_action_pins $T 20; echo rc=\$?"
rc=0
$ printf '      - { uses: attacker/exfil@main }\n' >> "$T"/docs.yml
$ echo "uses: lines the extraction sees: $(cat $T/*.yml | grep -cE '^[[:space:]]*(-[[:space:]]+)?uses:')"
uses: lines the extraction sees: 55
$ bash -c "source $T/fn.sh; assert_action_pins $T 20; echo rc=\$?"
rc=0
$ rm -rf "$T"

Fifty-five lines before and after, because the extractor never sees the fifty-sixth. Same pass either way, which matches the measurement in the body taken against #432's branch head.

I have added Tests first, Difficulty and Getting started to the body. This is still blocked on #432, which is now merged, so it is startable.

[mikevillari]

Taking you up on #441. I will replace the line extraction with YAML parsing, preserve the count floor and local-workflow exemption, and prove both supported spellings and exact discovery counts. I will preserve #443's unreadable-file regression and keep this change separate from that PR while it awaits merge.

Afterward I will probe the #444 case to report whether it still needs a separate fix, without widening #441 to force that result. Please assign #441 to me; #444 can remain separate pending that result.