fix(release): reject unreadable workflows in action pin checks

[mikevillari]

Summary

The action-pin check can pass when one workflow is unreadable, provided the remaining workflows clear its count floor. Check readability before extracting each file and report the path that could not be read.

Closes #442.

Only tests/release/release-rehearsal.test.sh changes: a four-line guard and two regression cases. No workflow, action pin, count floor, or runtime code changes.

Proof at ce77358

The unreadable-file regression was written first. Running the complete release-rehearsal test before adding the guard returned:

exit=1
FAIL: pin check passed over an unreadable workflow

The fixture passes with both files readable. Then one .yaml file becomes mode 000 while the readable .yml file alone still clears the floor. The regression requires a nonzero exit and the exact FAIL: cannot read <path> line. It runs as a non-root user; root receives an explicit SKIP diagnostic.

The empty-directory regression uses a zero count floor. This matters: removing the discovery guard must not let the count guard answer on its behalf. It requires the exact FAIL: no workflow files matched under <path> diagnostic.

Independent mutations of the shipped test, each restored before the next final run:

M1 remove readability guard:          exit=1, unreadable-workflow regression fails
M2 remove discovery guard:            exit=1, empty-directory regression fails
M3 replace readability diagnostic:    exit=1
M4 replace discovery diagnostic:      exit=1
C1 restore final file:                exit=0, Release rehearsal contract passed.

The two diagnostic mutations follow the feedback on #399: a generic nonzero result must not satisfy the assertion for a specific refusal. The full script runs with its own shell options; the tested guard is not a reimplementation in a separate harness.

Validation

• Tests added and demonstrated failing before the fix.
• Complete release-rehearsal test passes on macOS arm64, non-root, GNU Bash 5.2.37.
• python3 scripts/check_evidence_claims.py passes.
• ShellCheck 0.11.0 (-x -S warning), Bash syntax, and git diff --check pass.
• Security impact considered; daemon trust boundary unchanged.
• Required CI checks pass on ce773581d763f74121b04fb4d9aa25ab671caeef: docs-and-hygiene, rust, frontend, security-audit, and postgres-contract. The Ubuntu release-rehearsal contract step passes. Scripts-lint, secret scan, dependency review, and JavaScript/TypeScript CodeQL also pass. Rust CodeQL analysis also passes. All checks have finished: eleven successful and one skipped (container-smoke); none failed or remain pending.

bash scripts/ci-local.sh --no-postgres completed with ten failing checks; no full local CI pass is claimed. Clippy, cargo doc, and frontend dependency installation failed with network-resolution errors. Other failing checks were setup tests, mount-edit, public-claims, release-rehearsal, smithery-manifest, story-metadata, and story-runner-verdicts. Required Postgres validation and unavailable tools were skipped. Those broad failures have not all been compared against an unmodified checkout.

The broad run's release-rehearsal failure was the existing unquoted $rehearsal invocation encountering the space in the checkout path. Invoking the complete test through a no-space symlink passes; the final validation above used that path. ShellCheck was installed locally afterward and run separately on the changed file. No Rust or Node package changed; the contribution guide's non-Rust exemption applies.

Notes for Reviewers

AI-assisted with Codex.

[vladimirrott]

Approving at ce773581d763f74121b04fb4d9aa25ab671caeef.

The empty-directory case is the part I want to point at, and the floor of zero is the reason. With min_uses at 0 the count arm cannot answer on the discovery arm's behalf, so the test proves the glob assertion and nothing else. #442 asked for that in one line near the end and it is the easiest line in the issue to skim past. Both of your assertions pin the whole diagnostic with grep -Fxq, so a refusal that happens for some other reason cannot satisfy them.

maintainer screen 443 returns DO NOT EXECUTE for this file, since the hygiene and shellcheck gates run it directly. Everything below ran in rootless podman with --network=none, as uid 1000, against an extracted copy of your head. Never on this host.

Your test passes as written.

$ cd /home/entropia/.local/state/sysknife-maint/checkout && rm -rf /tmp/wt443 && mkdir -p /tmp/wt443 && git archive p443 | tar -x -C /tmp/wt443 && echo "extracted head: $(git rev-parse p443)" && md5sum /tmp/wt443/tests/release/release-rehearsal.test.sh && chmod -R a+rX /tmp/wt443 && rm -rf /tmp/tmp443 && mkdir -p /tmp/tmp443 && chmod 777 /tmp/tmp443 && out="$(podman run --rm --network=none --user 1000:1000 -v /tmp/wt443:/repo:z -v /tmp/tmp443:/sktmp:z -w /repo -e TMPDIR=/sktmp -e HOME=/sktmp -e CARGO_NET_OFFLINE=true localhost/sk-reh3:1 bash -c 'id; 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"
extracted head: ce773581d763f74121b04fb4d9aa25ab671caeef
a509732f43b5a0eb1669d536bbe00534  /tmp/wt443/tests/release/release-rehearsal.test.sh
uid=1000(node) gid=1000(node) groups=1000(node)
Release rehearsal contract passed.
TEST rc=0
podman rc=0

Your four-line guard is load-bearing. Deleting it from the shipped file and re-running:

$ echo "--- guard lines after M1 ---"; grep -n 'cannot read' /tmp/wt443m1/tests/release/release-rehearsal.test.sh; echo "(only the assertion should remain)"; rm -rf /tmp/tmp443m1 && mkdir -p /tmp/tmp443m1 && chmod 777 /tmp/tmp443m1 && out="$(podman run --rm --network=none --user 1000:1000 -v /tmp/wt443m1:/repo:z -v /tmp/tmp443m1:/sktmp:z -w /repo -e TMPDIR=/sktmp -e HOME=/sktmp -e CARGO_NET_OFFLINE=true localhost/sk-reh3:1 bash -c '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"
--- guard lines after M1 ---
188:    grep -Fxq "FAIL: cannot read $pin_fixture/unreadable/blocked.yaml" <<<"$pin_output"
(only the assertion should remain)
FAIL: pin check passed over an unreadable workflow
TEST rc=1
podman rc=0

The discovery arm bites too. Deleting the ((${#workflows[@]})) block:

$ echo "--- discovery arm after M2 ---"; grep -n 'no workflow files matched' /tmp/wt443m2/tests/release/release-rehearsal.test.sh; rm -rf /tmp/tmp443m2 && mkdir -p /tmp/tmp443m2 && chmod 777 /tmp/tmp443m2 && out="$(podman run --rm --network=none --user 1000:1000 -v /tmp/wt443m2:/repo:z -v /tmp/tmp443m2:/sktmp:z -w /repo -e TMPDIR=/sktmp -e HOME=/sktmp -e CARGO_NET_OFFLINE=true localhost/sk-reh3:1 bash -c '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"
--- discovery arm after M2 ---
172:grep -Fxq "FAIL: no workflow files matched under $pin_fixture/empty" <<<"$pin_output"
FAIL: pin check passed over an empty workflow directory
TEST rc=1
podman rc=0

A different refusal does not satisfy your assertion. I replaced the diagnostic with a generic one and kept the return 1, which is the mutation the #399 thread argued about:

$ sed -n '116,122p' /tmp/wt443m3/tests/release/release-rehearsal.test.sh; rm -rf /tmp/tmp443m3 && mkdir -p /tmp/tmp443m3 && chmod 777 /tmp/tmp443m3 && out="$(podman run --rm --network=none --user 1000:1000 -v /tmp/wt443m3:/repo:z -v /tmp/tmp443m3:/sktmp:z -w /repo -e TMPDIR=/sktmp -e HOME=/sktmp -e CARGO_NET_OFFLINE=true localhost/sk-reh3:1 bash -c '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"
    uses_count=0
    for workflow in "${workflows[@]}"; do
        [ -r "$workflow" ] || {
            printf 'FAIL: workflow unusable\n' >&2
            return 1
        }
        while IFS= read -r uses_line; do
TEST rc=1
podman rc=0

set -e takes the script down on the failing grep -Fxq with nothing on stdout, which is correct and silent. If you ever want that arm to say why, a || { printf ...; exit 1; } on the two new greps would do it, and the file's older assertions have the same quiet shape, so this is a preference rather than a gap.

And on the real call site. assert_action_pins "${repo_root}/.github/workflows" 20 is what runs in docs-and-hygiene. I reproduced #442 there, once on b2c823e6 and once on your head, with docs.yml at mode 000 and nothing else changed:

$ for pair in "main:/tmp/wtmain443" "pr443:/tmp/wt443u"; do name=${pair%%:*}; dir=${pair##*:}; t=/tmp/tmp_$name; rm -rf $t; mkdir -p $t; chmod 777 $t; echo "=== $name with .github/workflows/docs.yml at mode 000 ==="; 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-reh3: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 with .github/workflows/docs.yml at mode 000 ===
grep: /repo/.github/workflows/docs.yml: Permission denied
Release rehearsal contract passed.
TEST rc=0
podman rc=0

=== pr443 with .github/workflows/docs.yml at mode 000 ===
FAIL: cannot read /repo/.github/workflows/docs.yml
TEST rc=1
podman rc=0

Your root skip is the right call and it costs no coverage on the runner. The CI log for this head carries the pass line and no skip line, so the unreadable case ran there rather than skipping:

$ gh run view 35169473011 --repo lacs-project/sysknife --log --job 105037797197 2>/dev/null | grep -i 'release rehearsal\|SKIP: unreadable\|cannot read' | head -10
docs-and-hygiene	UNKNOWN STEP	2026-09-17T01:12:26.8892080Z Release rehearsal contract passed.

The container's own root run does take the skip, prints it to stderr and leaves the other assertions in place, and both runs clean up after themselves: ls -1A on the mounted TMPDIR came back empty each time.

ShellCheck 0.10.0 at --severity=warning and bash -n are clean on my side too, which matches your board.

One optional note, and I filed it rather than asking you to grow this PR

-r is true for a directory. grep on a directory exits 2 with Is a directory, the process substitution swallows that exit status the same way it swallowed Permission denied, and the count stays put:

PROBE: pin check PASSED over a directory named blocked.yaml
Release rehearsal contract passed.
TEST rc=0

That is the same shape you closed, one step further out. It is #444 now, with the probe command, that output and a suggested [ -f ] arm. Nothing for you to do here. This PR does what #442 asked and I would rather land it than widen it.

What is holding the merge is mine, not yours

My merge gate runs a pull request's own test in a container before it will issue a receipt, and that container is python:3.12, which scripts/release_rehearsal.sh cannot run in:

$ maintainer-merge verify 443 ce773581 'tests/release/release-rehearsal.test.sh' '/^        \[ -r "\$workflow" \] || {$/,+3d' shell
  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

$ podman run --rm --network=none docker.io/library/python:3.12 bash -c 'for t in bash python3 git cargo node npm sha256sum file; do printf "%-10s %s\n" "$t" "$(command -v $t || echo MISSING)"; done'
bash       /usr/bin/bash
python3    /usr/local/bin/python3
git        /usr/bin/git
cargo      MISSING
node       MISSING
npm        MISSING
sha256sum  /usr/bin/sha256sum
file       /usr/bin/file

Three of the five tools that script requires are absent, so no pull request touching this file can earn a receipt until I fix the image. Your eleven checks pass and container-smoke skips as it does on every PR. This is my queue, not your turn.

Next

#444 is four lines in the arm you just wrote, and it is yours if you want it. I am holding it. If you would rather have something with more in it, #441 is the other half of the same function: the extractor reads uses: with a line grep, so a flow-style mapping still goes unchecked, and the fixture for it is already sitting in this file as missed.yml.

Your PR body says macOS arm64, which is the awkward platform for this project: SysKnife drives systemd, apt and rpm-ostree, so there is nothing for it to do on a Mac. If you keep an Ubuntu VM or a box somewhere, the read-only surface is worth ten minutes. sysknife "what is using my disk" plans and shows you the actions before anything runs, and the daemon stays out of it until you approve. Skip it without a second thought if Linux is not where you spend your time; it changes nothing about this PR.