timothy/ersatztv
1
0
Fork 0
Code Issues 57 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

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

New Issue
Closed
opened 2026-08-14 17:15:08 +02:00 by timothy · 1 comment
timothy commented 2026-08-14 17:15:08 +02:00
Owner
Copy Link
Copy Source

Found while landing #776, which added an assertion that every hook must reach a real decision. On the Linux CI runner that assertion failed for this hook, and the reason is a live defect rather than a test problem.

The defect

.claude/hooks/pretooluse-bom-guard.sh detects a BOM with:

if [ "$(head -c3 "$p" 2>/dev/null | xxd -p 2>/dev/null)" = "efbbbf" ]; then

xxd ships with vim and is absent on this repo's Linux CI runner — verified on the runner host directly (command -v xxd → absent; od present). Where xxd does not exist the substitution yields the empty string, never equals efbbbf, and the guard allows every BOM in silence. It has been fail-open on any machine without vim since it was written.

Impact is bounded in practice — the hook fires in Claude Code sessions, which run on a Mac where xxd exists — but the guard's correctness has been resting on an undeclared dependency the whole time, and nothing anywhere states it.

Why it survived

Two reasons worth recording, because they generalise:

  1. docs/guard-inventory.md graded it NONE — no proof it could go red. It was ranked first in that file's "gaps ranked by what a silent failure would let through", on the grounds that the defect it guards has recurred three times (#311, #402, #405). That ranking was right for a worse reason than intended: the guard was not merely unproven, it was already broken.
  2. Every review of it ran on macOS. The environment was a sampled population and the sample was unanimous — Family A from docs/defect-shapes-773.md, with the platform as the thing sampled. Twelve reviewer-passes over #776 shared the same blind spot; only running on Linux exposed it.

The fix

od -A n -t x1 -N 3 — POSIX, present everywhere, byte-identical output to xxd -p on both platforms (measured).

Ships with the proof the guard never had, in scripts/tests/test_bom_guard_detection.py:

  • the real hook driven end to end through the real payload shape;
  • driven again with xxd removed from PATH — the regression, and the case every non-vim host was in;
  • a negative control on a clean file (a guard that denied everything would pass the rest);
  • an anti-vacuity check that the fixture really stages a BOM;
  • a clause-level mutation — disarm the comparison in an isolated copy, detection must stop.

Verified RED against the pre-fix hook and green against the fixed one. Inventory row moves NONE → MUTATION.

Done-when

  • xxd dependency removed and the guard proven to detect a BOM without it
  • docs/guard-inventory.md row regraded, with counts updated
  • Adversarial review passed
Found while landing #776, which added an assertion that every hook must reach a real decision. On the Linux CI runner that assertion failed for this hook, and the reason is a live defect rather than a test problem. ## The defect `.claude/hooks/pretooluse-bom-guard.sh` detects a BOM with: ```sh if [ "$(head -c3 "$p" 2>/dev/null | xxd -p 2>/dev/null)" = "efbbbf" ]; then ``` **`xxd` ships with vim and is absent on this repo's Linux CI runner** — verified on the runner host directly (`command -v xxd` → absent; `od` present). Where `xxd` does not exist the substitution yields the empty string, never equals `efbbbf`, and the guard **allows every BOM in silence**. It has been fail-open on any machine without vim since it was written. Impact is bounded in practice — the hook fires in Claude Code sessions, which run on a Mac where `xxd` exists — but the guard's correctness has been resting on an undeclared dependency the whole time, and nothing anywhere states it. ## Why it survived Two reasons worth recording, because they generalise: 1. **`docs/guard-inventory.md` graded it `NONE`** — no proof it could go red. It was ranked *first* in that file's "gaps ranked by what a silent failure would let through", on the grounds that the defect it guards has recurred three times (#311, #402, #405). That ranking was right for a worse reason than intended: the guard was not merely unproven, it was already broken. 2. **Every review of it ran on macOS.** The environment was a sampled population and the sample was unanimous — Family A from `docs/defect-shapes-773.md`, with the platform as the thing sampled. Twelve reviewer-passes over #776 shared the same blind spot; only running on Linux exposed it. ## The fix `od -A n -t x1 -N 3` — POSIX, present everywhere, byte-identical output to `xxd -p` on both platforms (measured). Ships with the proof the guard never had, in `scripts/tests/test_bom_guard_detection.py`: - the real hook driven end to end through the real payload shape; - driven again with **`xxd` removed from PATH** — the regression, and the case every non-vim host was in; - a **negative control** on a clean file (a guard that denied everything would pass the rest); - an **anti-vacuity** check that the fixture really stages a BOM; - a **clause-level mutation** — disarm the comparison in an isolated copy, detection must stop. Verified **RED against the pre-fix hook** and green against the fixed one. Inventory row moves `NONE` → `MUTATION`. ## Done-when - [x] `xxd` dependency removed and the guard proven to detect a BOM without it - [x] `docs/guard-inventory.md` row regraded, with counts updated - [x] Adversarial review passed
timothy added the in-progresspriority: high labels 2026-08-14 17:15:28 +02:00
timothy commented 2026-08-14 18:51:27 +02:00
Author
Owner
Copy Link
Copy Source

Closing record

Outcome: Merged as #798 (squash). pretooluse-bom-guard.sh detects a BOM with od -A n -t x1 -N 3 instead of xxd -p, and ships scripts/tests/test_bom_guard_detection.py — the proof it never had. Inventory row NONE → MUTATION. The twin recipes in docs/decisions/records/process/bom-format-detection-recipe.md and docs/contributing.md are corrected in the same PR.

Root cause: the guard depended on xxd, which ships with vim and is absent on plain Linux hosts including this repo's CI runner. Where it is absent the command substitution yields the empty string, never equals efbbbf, and every BOM is allowed silently. Fail-open since the guard was written.

Decisions/conventions changed: process.bom-format-detection-recipe — the prescribed detector changes from xxd -p to od -A n -t x1 -N 3. Its original warning is kept and clarified rather than dropped: the form that once reported all-clean over 19 dirty files was od -An -c | grep '357 273 277' (octal character output, variable spacing), which is a different invocation of the same tool, not this one.

Reusable knowledge:

  1. A guard graded NONE is not merely untested — it is a guard whose current behaviour nobody has established. This one was ranked first in the inventory's "gaps ranked by what a silent failure would let through". That ranking was right for a worse reason than intended: it was already broken.
  2. The environment is a sampled population. Every review of this guard ran on macOS, where xxd exists, including twelve reviewer-passes over #776. Unanimity across a sample that shares one blind spot is not evidence. It surfaced only when an assertion demanding the hook reach a real decision ran on Linux.
  3. A mutation proof can be green over a dead guard. test_DISARMING_..._stops_detection passed against the pre-fix hook on Linux — "the mutant is silent" is trivially true when the original is silent too. A mutation proof needs an in-test positive control that the unmutated subject still detects.
  4. Fix the twin. The hook was fixed while the manual recipe it mirrors stayed broken — on the path a human runs precisely when the hook is bypassed with --no-verify.

Verification: 589 tests pass; CI green on every context. od/xxd equivalence measured over 17 inputs (empty, sub-3-byte, exactly the BOM, BOM+NUL, binary, UTF-16 BOM, unreadable, 20 MB, awkward filenames) on BSD od, GNU od on the runner host, and inside the actual CI runner image — identical in every cell, with xxd confirmed absent there. New test verified RED against the pre-fix hook and green against the fixed one.

Deferred: none. The residual is #785's wider population of unproven guards; this closes one of them.

Docs updated: docs/guard-inventory.md, docs/decisions/records/process/bom-format-detection-recipe.md, docs/contributing.md.

## Closing record **Outcome:** Merged as #798 (squash). `pretooluse-bom-guard.sh` detects a BOM with `od -A n -t x1 -N 3` instead of `xxd -p`, and ships `scripts/tests/test_bom_guard_detection.py` — the proof it never had. Inventory row `NONE` → `MUTATION`. The twin recipes in `docs/decisions/records/process/bom-format-detection-recipe.md` and `docs/contributing.md` are corrected in the same PR. **Root cause:** the guard depended on `xxd`, which ships with vim and is absent on plain Linux hosts including this repo's CI runner. Where it is absent the command substitution yields the empty string, never equals `efbbbf`, and every BOM is allowed silently. Fail-open since the guard was written. **Decisions/conventions changed:** `process.bom-format-detection-recipe` — the prescribed detector changes from `xxd -p` to `od -A n -t x1 -N 3`. Its original warning is kept and clarified rather than dropped: the form that once reported all-clean over 19 dirty files was `od -An -c | grep '357 273 277'` (octal character output, variable spacing), which is a different invocation of the same tool, not this one. **Reusable knowledge:** 1. **A guard graded `NONE` is not merely untested — it is a guard whose current behaviour nobody has established.** This one was ranked *first* in the inventory's "gaps ranked by what a silent failure would let through". That ranking was right for a worse reason than intended: it was already broken. 2. **The environment is a sampled population.** Every review of this guard ran on macOS, where `xxd` exists, including twelve reviewer-passes over #776. Unanimity across a sample that shares one blind spot is not evidence. It surfaced only when an assertion demanding the hook reach a real decision ran on Linux. 3. **A mutation proof can be green over a dead guard.** `test_DISARMING_..._stops_detection` passed against the pre-fix hook on Linux — "the mutant is silent" is trivially true when the original is silent too. A mutation proof needs an in-test positive control that the *unmutated* subject still detects. 4. **Fix the twin.** The hook was fixed while the manual recipe it mirrors stayed broken — on the path a human runs precisely when the hook is bypassed with `--no-verify`. **Verification:** 589 tests pass; CI green on every context. `od`/`xxd` equivalence measured over 17 inputs (empty, sub-3-byte, exactly the BOM, BOM+NUL, binary, UTF-16 BOM, unreadable, 20 MB, awkward filenames) on BSD `od`, GNU `od` on the runner host, and inside the actual CI runner image — identical in every cell, with `xxd` confirmed absent there. New test verified RED against the pre-fix hook and green against the fixed one. **Deferred:** none. The residual is #785's wider population of unproven guards; this closes one of them. **Docs updated:** `docs/guard-inventory.md`, `docs/decisions/records/process/bom-format-detection-recipe.md`, `docs/contributing.md`.
timothy removed the in-progress label 2026-08-14 18:51:38 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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 priority: high
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#797