Files
ersatztv/scripts/tests
timothy 5e7623b8d5 fix(648,649): security-review round 2 — close the version-parse hole and the untested caller contract
Two real defects, and three docs claims that were simply wrong.

jq-preflight.sh parsed the version by stripping around the first `-` and `.`, which
assumed the format is exactly `jq-X.Y`. A build printing `jq version 1.6` left major
empty; the sanity check concatenated major+minor into "6", which is non-empty and
all-digits, so it PASSED. The floor comparison then ran `[ "" -lt 1 ]`, which errors —
and `set -e` exempts a failing command in an `if` condition, so the conditional read
false and the script exited 0 having asserted nothing, after printing a plausible
"parsed" line. The silently-untested-axis failure this script exists to eliminate,
reproduced inside the script itself. Now parsed by explicit regex, failing closed with a
diagnosis when there is no <digits>.<digits> match. Also: `--expect` with no value exited
1 with empty output on both streams.

The hook's exit-status check was pinned by nothing: mutating `if files=$(...)` into
`files=$(...) || true; files_complete=yes` left the ENTIRE suite green. It survived only
by redundancy — the script writes stdout once, right before exit 0, so failures also
happen to yield empty stdout and `[ -n "$files" ]` catches it. Safe by accident, which is
the exact criticism this branch levels at the old code. Four tests now pin it, with a
stub that FAILS while emitting a docs-only list (the one case redundancy cannot absorb)
plus a positive control proving the harness can see the difference. Verified: the
mutation now turns exactly those tests red.

Docs corrections. The record claimed the --expect pin was safe because script-tests is
"advisory, not a required check" — false. The merge-consent hook reads the COMBINED
status (ci.advisory-red-blocks-the-merge-gate, #598), so firing the tripwire blocks every
non-docs-only merge until someone re-pins. Kept anyway, for a stated reason, but no
longer described as free. The record also asserted in the present tense that
review-verdict.yml checks out the base ref; it has no checkout step at all, so that is
now a future-tense requirement on the follow-up. And the documented .status allow-list
named GitHub's `removed`, which the code rejects.

The drift-guard regex anchored on `?limit=`, so a re-inlined copy written
`files?page=1&limit=50` would have walked past it.

Decisions-Edit: yes
2026-07-26 22:21:07 +02:00
..