fix(release): parse workflow action references as YAML

[mikevillari]

Summary

Parse workflow YAML when checking action pins so block and flow mappings receive the same validation. Inspect step actions and job-level reusable workflows, preserve the reference-count floor and anchored ./ exemption, and propagate read and parse failures.

The checker also rejects a directory named like a workflow. A regression now requires that specific read-error diagnostic even when a readable companion already clears the count floor.

Three files change:

• tests/release/release-rehearsal.test.sh: YAML checker and regression fixtures, including the requested directory-shaped input case.
• .github/workflows/ci.yml: install PyYAML==6.0.2 explicitly alongside pinned yamllint.
• docs/developer-guide.md: document the parser dependency and its explicit CI installation.

Related Issue

Closes #441 and closes #444. Depends on #443; merge #443 first. This branch retains that prerequisite. It can be rebased after #443 lands.

Proof

The flow-style regression failed before the parser replacement: the old checker accepted attacker/exfil@main in a real-workflow copy that otherwise cleared the floor. The YAML checker rejects it and names the action.

For #444, valid.yml already satisfies the floor of 1. A directory named blocked.yaml sits beside it. The assertion requires both failure and the exact cannot read diagnostic, including the offending path.

A targeted mutation inserts if workflow.is_dir(): continue into the shipped checker:

Previous tests at 968aa9ca, directory-skipping mutation  exit=0
Updated tests, unmodified checker                      exit=0
Updated tests, directory-skipping mutation              exit=1
FAIL: pin check accepted .../unpinned; expected FAIL: cannot read .../blocked.yaml: [Errno 21] Is a directory: '.../blocked.yaml'

The earlier mutation checks still reject removed SHA validation, a removed count floor, skipped job/step references, swallowed YAML errors, a removed local exemption, and an incorrect count.

Validation

• Complete release-rehearsal contract passes locally as a non-root user with GNU Bash 5.2.37 and PyYAML 6.0.2.
• Directory regression and existing mutation proofs pass; no checker behavior was added for The action-pin check counts a directory named x.yml as a workflow it read #444.
• Evidence-claim check, ShellCheck, Bash syntax, YAML lint, and diff whitespace checks pass.
• Fresh Ubuntu CI on 15a545c8c2812fa3e4f0e6c34dc9192ad25c31e8: all eleven checks pass, including Rust tests and both CodeQL analyses; container-smoke is skipped. The hygiene log confirms PyYAML 6.0.2, 55 checked references, and a passing release-rehearsal contract.

AI-assisted with Codex.

[vladimirrott]

Reviewed at 968aa9ca641737bae542954f53f61ad84e5e21ec. Approved.

The -S fixture is the part I did not expect. Shadowing python3 with a function inside a subshell, so assert_action_pins calls the real interpreter with site-packages omitted, gives the missing-parser branch a live test without touching the environment any other block runs in. Most people would have left that except ImportError arm asserted by inspection. You also wrote the flow-style regression before the replacement and let it fail for the right reason, which is the order that proves a regression is a regression.

maintainer screen 446 returns DO NOT EXECUTE, so everything below ran in rootless podman, --network=none, uid 1000, against an extracted copy of your head. The image is localhost/sk-reh4:1, built from public bases and holding no repository content.

The hole this closes, on the real workflows

I put the same flow-style unpinned action into .github/workflows/extra.yaml in both trees and ran the shipped contract:

$ cd /home/entropia/.local/state/sysknife-maint/checkout && rm -rf /tmp/wtmain446 && mkdir -p /tmp/wtmain446 && git archive origin/main | tar -x -C /tmp/wtmain446 && echo "main tree: $(git rev-parse origin/main)" && rm -rf /tmp/wt446x && cp -a /tmp/wt446 /tmp/wt446x && for d in /tmp/wtmain446 /tmp/wt446x; do printf 'name: extra\njobs: {x: {steps: [{uses: attacker/exfil@main}]}}\n' > "$d/.github/workflows/extra.yaml"; chmod -R a+rX "$d"; done && echo "--- injected file (identical in both trees) ---" && cat /tmp/wt446x/.github/workflows/extra.yaml && md5sum /tmp/wtmain446/.github/workflows/extra.yaml /tmp/wt446x/.github/workflows/extra.yaml
main tree: b2c823e68cd5829de77976cb3be2e9b163a6fe05
--- injected file (identical in both trees) ---
name: extra
jobs: {x: {steps: [{uses: attacker/exfil@main}]}}
15cd205cf7302543d69281e524eee296  /tmp/wtmain446/.github/workflows/extra.yaml
15cd205cf7302543d69281e524eee296  /tmp/wt446x/.github/workflows/extra.yaml
$ for pair in "main-b2c823e6:/tmp/wtmain446" "pr446-968aa9ca:/tmp/wt446x"; do name=${pair%%:*}; dir=${pair##*:}; t=/tmp/tmpx_$name; rm -rf $t; mkdir -p $t; chmod 777 $t; echo "=== $name with a flow-style unpinned action in .github/workflows/extra.yaml ==="; out="$(podman run --rm --network=none --user 1000:1000 -v $dir:/repo:z -v $t:/sktmp:z -w /repo -e TMPDIR=/sktmp -e HOME=/sktmp -e CARGO_NET_OFFLINE=true localhost/sk-reh4:1 bash -c 'bash tests/release/release-rehearsal.test.sh; echo "TEST rc=$?"' 2>&1)"; prc=$?; printf '%s\n' "$out" | grep -v 'level=warning'; echo "podman rc=$prc"; echo; done
=== main-b2c823e6 with a flow-style unpinned action in .github/workflows/extra.yaml ===
Release rehearsal contract passed.
TEST rc=0
podman rc=0

=== pr446-968aa9ca with a flow-style unpinned action in .github/workflows/extra.yaml ===
FAIL: extra.yaml action is not pinned to a 40-hex SHA: attacker/exfil@main
TEST rc=1
podman rc=0

On main a third-party action pinned to a mutable branch sits in .github/workflows and docs-and-hygiene prints a pass over it. That is the observable consequence, and it is the reason this is worth landing rather than a tidy-up.

Mutations

Clean run first, so the failures below mean something:

$ rm -rf /tmp/tmp446 && mkdir -p /tmp/tmp446 && chmod 777 /tmp/tmp446 && out="$(podman run --rm --network=none --user 1000:1000 -v /tmp/wt446:/repo:z -v /tmp/tmp446:/sktmp:z -w /repo -e TMPDIR=/sktmp -e HOME=/sktmp -e CARGO_NET_OFFLINE=true localhost/sk-reh4:1 bash -c 'id; python3 -c "import yaml;print(\"PyYAML\",yaml.__version__)"; bash tests/release/release-rehearsal.test.sh; echo "TEST rc=$?"' 2>&1)"; rc=$?; printf '%s\n' "$out" | grep -v 'level=warning'; echo "podman rc=$rc"
uid=1000(node) gid=1000(node) groups=1000(node)
PyYAML 6.0
Checked 55 uses: entries.
Checked 2 uses: entries.
Release rehearsal contract passed.
TEST rc=0
podman rc=0

Four mutations of the shipped checker, applied to four separate copies. I dropped the 40-hex requirement, removed the count floor, turned the YAML parse error into continue, and deleted the job-level reference arm:

$ for m in m1 m2 m3 m4; do echo "=== M:$m ==="; t=/tmp/tmp446$m; rm -rf $t; mkdir -p $t; chmod 777 $t; chmod -R a+rX /tmp/wt446$m; out="$(podman run --rm --network=none --user 1000:1000 -v /tmp/wt446$m:/repo:z -v $t:/sktmp:z -w /repo -e TMPDIR=/sktmp -e HOME=/sktmp -e CARGO_NET_OFFLINE=true localhost/sk-reh4:1 bash -c 'bash tests/release/release-rehearsal.test.sh; echo "TEST rc=$?"' 2>&1)"; rc=$?; printf '%s\n' "$out" | grep -v 'level=warning' | tail -6; echo "podman rc=$rc"; echo; done
=== M:m1 ===
Checked 55 uses: entries.
FAIL: pin check accepted /sktmp/tmp.fY1VMkPsvh/unpinned; expected FAIL: action.yml action is not pinned to a 40-hex SHA: attacker/exfil@main
TEST rc=1
podman rc=0

=== M:m2 ===
Checked 55 uses: entries.
FAIL: pin check accepted /sktmp/tmp.ORh1duEzoG/workflows; expected FAIL: extracted 4 uses: entries under /sktmp/tmp.ORh1duEzoG/workflows; need at least 5
TEST rc=1
podman rc=0

=== M:m3 ===
Checked 55 uses: entries.
FAIL: pin check accepted /sktmp/tmp.C7oH7sAJNy/unpinned; expected FAIL: cannot parse /sktmp/tmp.C7oH7sAJNy/unpinned/action.yml
TEST rc=1
podman rc=0

=== M:m4 ===
Checked 54 uses: entries.
FAIL: extracted 2 uses: entries under /sktmp/tmp.mrkhJDKeFV/workflows; need at least 4
TEST rc=1
podman rc=0

Each one names the specific diagnostic it lost, rather than reporting a bare non-zero exit. M:m4 also answers a question your PR body raised in passing: the drop from 55 to 54 says this tree has exactly one job-level reusable-workflow reference, so that arm is load-bearing on one line today and would fail closed the moment somebody added a second.

ShellCheck 0.10.0 at CI's --severity=warning is clean, bash -n is clean, and the mounted TMPDIR is empty after the clean run.

#444 is fixed here, and I would rather it did not land untested

You said this PR does not claim to close #444, and you were right to say so, because there is no regression fixture for it. The behaviour does change. Same fixture as the issue, one readable block-style workflow clearing a floor of 1, with a directory named blocked.yaml beside it:

$ set -e
cd /home/entropia/.local/state/sysknife-maint/checkout
for pair in "main-b2c823e6:/tmp/wtmain446" "pr446-968aa9ca:/tmp/wt446"; do
  name=${pair%%:*}; dir=${pair##*:}; src=/tmp/dp2_$name; rm -rf $src; cp -a $dir $src
  rm -f $src/.github/workflows/extra.yaml 2>/dev/null || true
  python3 - "$src" <<'PY'
import sys
p=sys.argv[1]+'/tests/release/release-rehearsal.test.sh'
s=open(p).read()
anchor="""if grep -Fq -- '--no-verify' "$release_workflow"; then"""
probe='''# PROBE (maintainer, not part of the PR): one readable block-style workflow
# clears the floor of 1 on its own; a *directory* named blocked.yaml sits
# beside it and passes the [ -r ] test.
mkdir -p "$pin_fixture/dir444"
printf 'jobs:\\n  build:\\n    steps:\\n      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1\\n' > "$pin_fixture/dir444/readable.yml"
mkdir "$pin_fixture/dir444/blocked.yaml"
if probe_out="$(assert_action_pins "$pin_fixture/dir444" 1 2>&1)"; then
    printf 'PROBE: pin check PASSED over a directory named blocked.yaml (output: %s)\\n' "$probe_out" >&2
else
    printf 'PROBE: pin check refused: %s\\n' "$probe_out" >&2
fi

'''
assert s.count(anchor)==1
open(p,'w').write(s.replace(anchor, probe+anchor))
PY
  t=/tmp/tmpdp2_$name; rm -rf $t; mkdir -p $t; chmod 777 $t; chmod -R a+rX $src
  echo "=== $name: readable.yml clears the floor, blocked.yaml is a directory ==="
  out="$(podman run --rm --network=none --user 1000:1000 -v $src:/repo:z -v $t:/sktmp:z -w /repo -e TMPDIR=/sktmp -e HOME=/sktmp -e CARGO_NET_OFFLINE=true localhost/sk-reh4:1 bash -c 'bash tests/release/release-rehearsal.test.sh; echo "TEST rc=$?"' 2>&1)"; printf '%s\n' "$out" | grep -v 'level=warning' | grep -E 'PROBE|TEST rc'; echo
done
=== main-b2c823e6: readable.yml clears the floor, blocked.yaml is a directory ===
PROBE: pin check PASSED over a directory named blocked.yaml (output: grep: /sktmp/tmp.jQBR7ykGJr/dir444/blocked.yaml: Is a directory)
TEST rc=0

=== pr446-968aa9ca: readable.yml clears the floor, blocked.yaml is a directory ===
PROBE: pin check refused: FAIL: cannot read /sktmp/tmp.TiYx4t4ge1/dir444/blocked.yaml: [Errno 21] Is a directory: '/sktmp/tmp.TiYx4t4ge1/dir444/blocked.yaml'
TEST rc=0

[ -r ] is true for a directory, so the bash arm waves it through; read_text() raises IsADirectoryError, your except OSError catches it, and the refusal names the path. Fixed behaviour with no test holding it in place is the state that regresses next, so: would you add three lines to the unpinned block, a mkdir and an assert_pin_failure on the cannot read diagnostic, and put Closes #444 in the body? Your call. If you would rather ship this as it stands I will leave #444 open with this reproduction attached, and it will not hold the merge up.

Optional

CI reaches PyYAML through pip install yamllint==1.38.0, which happens to bring it. Your developer-guide.md line says so and is accurate. Naming PyYAML on that pip install line in ci.yml would make the coupling visible to whoever next edits the lint tooling. The failure mode today is loud rather than silent, so this is housekeeping rather than a hole.

Merge order, and what is holding it

#443 goes in first. This branch carries ce773581 under your own commit, so merging this one alone would also land #443's change without #443 ever being merged. Cleanest order is #443, then this rebases to a single commit.

Both are waiting on me, not on you. My merge gate runs the named test in a container before and after a mutation and refuses to issue a receipt unless it sees the test pass clean and fail mutated. That container has no cargo, and release_rehearsal.sh requires it:

$ out="$(maintainer-merge verify 446 968aa9ca641737bae542954f53f61ad84e5e21ec 'tests/release/release-rehearsal.test.sh' '/if re.fullmatch(r"\[^@\\s\]+@\[0-9a-f\]{40}", reference):/,+1d' shell 2>&1)"; rc=$?; printf '%s\n' "$out" | tail -22; echo "verify rc=$rc"
  nothing in this diff is in PROD_GLOBS, so no path here can invalidate
  the receipt at merge. The shell suite runs 1 of 2 changed path(s); the
  rest are not covered by the receipt and it proves nothing about them:
    docs/developer-guide.md
  CI still gates them.
  suite: shell (docker.io/library/python:3.12 under podman)
  running 'tests/release/release-rehearsal.test.sh' unmutated
maintainer-merge: the unmutated test failed on my suite environment, not the pull request.

    ERROR: required tool is missing: cargo

  That is a missing binary or an unexecutable path inside the verify
  container, so it says nothing about #446. Fix the suite in
  profiles/*/verify.d/ (suite_image, suite_needs, the mount flags) and run
  this again. Do NOT report the contributor as failing their own test.
verify rc=1

That is the third pull request in three days stopped by that image, all of them touching this file. The mutation proof above is the same work the gate wants to observe, and it passes; I am fixing the image rather than routing around the gate. Nothing for you to do.

Next

#369 is yours if you want it: four action pins name a version they are not, and nothing checks. You have spent two pull requests inside that machinery and you now know more about the pin checker than anyone. Say the word and I will hold it.

And if you have an Ubuntu host you look after, the read-only side of SysKnife takes nothing away from it: sysknife --dry-run "what is listening on this box" plans and prints and executes nothing. Somebody who reads workflow YAML at this level would spot the first wrong thing it says, and I would like to know what that is.

[mikevillari]

Added at 15a545c8c2812fa3e4f0e6c34dc9192ad25c31e8.

The #444 fixture sits in the existing unpinned block: valid.yml clears the floor of 1, while blocked.yaml is a directory. assert_pin_failure requires the exact read-error diagnostic and path. The checker itself is unchanged.

I tested a mutation that skips directories before reading them:

968aa9ca tests + directory-skipping mutation  exit=0
Updated tests + unmodified checker           exit=0
Updated tests + directory-skipping mutation  exit=1

The last run fails because the pin check accepted the fixture instead of reporting cannot read .../blocked.yaml. The existing SHA, count-floor, job/step reference, YAML-error, local-reference, and reported-count mutations still fail as intended.

CI now installs PyYAML==6.0.2 explicitly, and the developer guide reflects that. Ubuntu's fresh hygiene run confirms the install, Checked 55 uses: entries., and Release rehearsal contract passed.

All eleven checks on this head pass, including Rust and both CodeQL analyses; container-smoke is skipped.

The PR body now closes #444 as well as #441. Merge order remains #443 first, then this branch rebased onto it.