fix: the BOM guard was fail-open wherever xxd is not installed #798

Merged

timothy merged 2 commits from fix/bom-guard-xxd into main 2026-08-14 18:51:00 +02:00

Conversation 1 Commits 2 Files Changed 6

+227 -13

timothy commented 2026-08-14 17:15:29 +02:00

Owner

Copy Link

Copy Source

pretooluse-bom-guard.sh detected a BOM with head -c3 | xxd -p. xxd ships with vim and is
absent on this repo's Linux CI runner — verified on the runner host — so the substitution yielded
empty, never equalled efbbbf, and the guard allowed every BOM in silence. Fail-open on any machine
without vim since it was written.

fixes #797

od -A n -t x1 -N 3 is POSIX and byte-identical on macOS and Linux.

Why it survived

docs/guard-inventory.md graded it NONE — no proof it could go red — and ranked it first in
"gaps ranked by what a silent failure would let through". That ranking was correct for a worse
reason than intended: the guard was not merely unproven, it was already broken. And every review of
it ran on macOS, where xxd exists — the environment was a sampled population, and twelve
reviewer-passes over #776 shared the blind spot. It surfaced only because #776 added an assertion
that this hook must reach a real decision, and that ran on Linux.

The proof it never had

scripts/tests/test_bom_guard_detection.py:

end-to-end	the real hook, real payload shape, staged BOM → deny
the regression	same, with xxd removed from PATH
negative control	a clean file must be allowed silently
anti-vacuity	the fixture really stages a BOM
mutation	disarm the comparison in an isolated copy → detection must stop

Verified RED against the pre-fix hook, green against this one. Inventory row NONE → MUTATION;
counts updated (13 proof files, 5 mutation, 20 none).

Why this is its own PR

Split out of #776 deliberately. #776's non-vacuity assertion cannot pass on Linux without this fix,
so it had to land ahead rather than behind — and a guard behaviour change deserves a review that
isn't buried under thirteen commits about hook observability. #776 will rebase onto it.

Verification

pytest scripts/tests — 589 passed. No .cs and no web/ files changed.

`pretooluse-bom-guard.sh` detected a BOM with `head -c3 | xxd -p`. **`xxd` ships with vim and is absent on this repo's Linux CI runner** — verified on the runner host — so the substitution yielded empty, never equalled `efbbbf`, and the guard allowed every BOM in silence. Fail-open on any machine without vim since it was written. fixes #797 `od -A n -t x1 -N 3` is POSIX and byte-identical on macOS and Linux. ## Why it survived `docs/guard-inventory.md` graded it `NONE` — no proof it could go red — and ranked it **first** in "gaps ranked by what a silent failure would let through". That ranking was correct for a worse reason than intended: the guard was not merely unproven, it was already broken. And every review of it ran on macOS, where `xxd` exists — the environment was a sampled population, and twelve reviewer-passes over #776 shared the blind spot. It surfaced only because #776 added an assertion that this hook must reach a real decision, and that ran on Linux. ## The proof it never had `scripts/tests/test_bom_guard_detection.py`: | | | |---|---| | end-to-end | the real hook, real payload shape, staged BOM → `deny` | | **the regression** | same, with `xxd` removed from PATH | | negative control | a clean file must be allowed silently | | anti-vacuity | the fixture really stages a BOM | | **mutation** | disarm the comparison in an isolated copy → detection must stop | Verified **RED against the pre-fix hook**, green against this one. Inventory row `NONE` → `MUTATION`; counts updated (13 proof files, 5 mutation, 20 none). ## Why this is its own PR Split out of #776 deliberately. #776's non-vacuity assertion cannot pass on Linux without this fix, so it had to land ahead rather than behind — and a guard behaviour change deserves a review that isn't buried under thirteen commits about hook observability. #776 will rebase onto it. ## Verification `pytest scripts/tests` — 589 passed. No `.cs` and no `web/` files changed.

timothy added 1 commit 2026-08-14 17:15:30 +02:00

fix: the BOM guard was fail-open wherever xxd is not installed ... 4873870727

`pretooluse-bom-guard.sh` detected a BOM with `head -c3 | xxd -p`. `xxd` ships
with vim and is ABSENT on this repo's Linux CI runner — verified on the runner
host — so the command substitution yielded empty, never equalled `efbbbf`, and
the guard allowed every BOM in silence. It has been fail-open on any machine
without vim since it was written.

`od -A n -t x1 -N 3` is POSIX and byte-identical on macOS and Linux.

Why nobody noticed: `docs/guard-inventory.md` graded this guard NONE — no proof
it could go red — and every review of it ran on macOS, where xxd exists. The
environment was a sampled population and the sample was unanimous. It surfaced
only because an unrelated change (#776) added an assertion that this hook must
reach a real decision, and that assertion ran on Linux.

So it ships with the proof it never had: the real hook driven end to end, driven
again with xxd removed from PATH (the regression), a negative control on a clean
file, an anti-vacuity check that the fixture really stages a BOM, and a
clause-level mutation — disarm the comparison in an isolated copy and detection
must stop. Verified RED against the pre-fix hook and green against this one.

The inventory row moves NONE -> MUTATION, and the "gaps ranked by what a silent
failure would let through" list records that its first entry was ranked correctly
for a worse reason than intended.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

timothy referenced this issue from a commit 2026-08-14 17:30:07 +02:00

fix(797): the twin recipe was still broken, and the mutation proof was green over a dead guard

timothy added 1 commit 2026-08-14 17:30:08 +02:00

fix(797): the twin recipe was still broken, and the mutation proof was green over a dead guard ... 6a6ce36b2c

Two independent reviews of #798. No Blocker; the functional fix is confirmed
sound — one reviewer ran a 17-input matrix on BSD od, GNU od, and inside the
actual CI runner image, identical in every cell, with `xxd` confirmed absent
there. Everything below is what they found around it.

THE FIX STOPPED AT THE HOOK. `docs/decisions/records/process/bom-format-detection-recipe.md`
(active) and `docs/contributing.md:120` both still prescribed
`head -c3 | xxd -p | grep -q '^efbbbf'` as the standing manual pre-push check —
same absent xxd, same silent all-clean, on the path a human runs PRECISELY when
the hook is being bypassed with --no-verify. Worse, that record's "Verify your
detector" paragraph rejected an `od` form by name, so the active record argued
against the hook this PR ships.

Both corrected. The record keeps its original warning, which is still right and
was never about `od` as a tool: the form that reported all-clean over 19 dirty
files was `od -An -c | grep '357 273 277'` — octal character output, whose
spacing and escaping vary. The replacement is `od -A n -t x1 -N 3`, hexadecimal
bytes, measured across the matrix above. The record now says its own prescribed
detector had been an all-clean detector for three weeks, which is the thing it
was written to prevent.

THE MUTATION PROOF WAS GREEN OVER A DEAD GUARD. Run against the pre-fix hook on
Linux, `test_DISARMING_the_BOM_comparison_stops_detection` PASSED while the guard
detected nothing at all — "the mutant is silent" is trivially true when the
original is silent too. The inventory row cites that function by name. It now
asserts the unmutated guard DOES detect, first.

THE STAND-IN PATH STARVED THE HOOK. `_path_without_xxd` hand-listed six tools the
hook never calls and omitted `tail`, which it does call for cd-detection — so a
payload whose cwd differs from the repo failed open for a reason unrelated to
xxd. The list is derived from the hook's own source now, with a required-set
assertion so a starved PATH fails loudly instead of passing quietly.

Also: the inventory narrative restated the NONE count and drifted the moment this
guard was regraded, invisibly to the summary parser. The duplicate count is
removed rather than corrected.

Refs: #797

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

timothy commented 2026-08-14 18:50:43 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ 6a6ce36

Two independent cold reviews (cross-family Codex + Opus). No Blocker from either. od/xxd equivalence measured over 17 inputs on BSD od, GNU od and inside the actual CI runner image - identical in every cell, xxd confirmed absent there. Findings fixed: the twin xxd recipe in the active decision record and contributing.md; the mutation proof was green over a dead guard and now asserts the unmutated guard detects first; the stand-in PATH starved the hook of tail and is now derived from the hook source. 589 tests pass; CI green.

Review-verdict: MERGEABLE @ 6a6ce36 Two independent cold reviews (cross-family Codex + Opus). No Blocker from either. od/xxd equivalence measured over 17 inputs on BSD od, GNU od and inside the actual CI runner image - identical in every cell, xxd confirmed absent there. Findings fixed: the twin xxd recipe in the active decision record and contributing.md; the mutation proof was green over a dead guard and now asserts the unmutated guard detects first; the stand-in PATH starved the hook of tail and is now derived from the hook source. 589 tests pass; CI green.

timothy merged commit 84165ab755 into main 2026-08-14 18:51:00 +02:00

timothy deleted branch fix/bom-guard-xxd 2026-08-14 18:51:01 +02:00

timothy referenced this pull request 2026-08-14 18:51:27 +02:00

pretooluse-bom-guard.sh is fail-open wherever xxd is not installed — including the CI runner #797

timothy referenced this issue from a commit 2026-08-14 18:56:55 +02:00

docs(776): restore two inventory sections the rebase resolution dropped

timothy referenced this pull request 2026-08-16 09:53:33 +02:00

Make hooks report that they fired — PreToolUse/PostToolUse execution is currently unobservable #776

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#798