fix(891): a sourced path is code, so every hook resolves it from its own tree #903

Merged

timothy merged 17 commits from fix/891-hook-lib-self-location into main 2026-08-30 23:41:20 +02:00

Conversation 1 Commits 17 Files Changed 24

+983 -117

timothy commented 2026-08-30 22:31:30 +02:00

Owner

Copy Link

Copy Source

fixes #891
refs #858, #859, #776

The defect

Every hook under .claude/hooks/ opens by assigning a path and then . -SOURCING it:

ETV_HOOK_FIRE_LIB="${CLAUDE_PROJECT_DIR:-<self-location>}/scripts/hook-fire-log.sh" || true
[ -r "$ETV_HOOK_FIRE_LIB" ] && . "$ETV_HOOK_FIRE_LIB" || true

Sourcing is execution. Whatever that line names runs as code inside the hook, before stdin is read
and before the hook can decide anything — so a file of that name in an env-var-designated tree that
prints a decision and exits 0 is the decision. #858 fixed one hook (the merge gate, where
leaving a total bypass above the gate it was hardening would have made the rest decorative). This is
the sweep of the remaining twelve, plus the second resolution inside the sink itself.

Why it is reachable without an attacker. The obvious objection — $CLAUDE_PROJECT_DIR also
names the hook binary in .claude/settings.json, so a hostile value has already chosen which hook
runs — is correct for the Claude PreToolUse hooks, where the two roots agree by construction. It
does not hold for the prepush family, because husky is a different launcher: .husky/pre-push
invokes ./.claude/hooks/…, a path relative to the pushed tree and wholly independent of the
variable. A push from one worktree while the environment names another sources the other tree's
code into a gate that can block or allow the push. Ordinary in a repo that runs several worktrees at
once, and it fails by returning a confident wrong answer rather than visibly.

What changed

1. All twelve remaining hooks, together, in one change. Population derived from git ls-files
   (with -F — grep here is ugrep, and the same pattern without it matches nothing and exits 0,
   which is how this issue was first filed claiming 11 sites). Every tracked hook now carries the
   assignment byte-identically.

2. scripts/hook-fire-log.sh's own resolution reconciled, then hardened. etv_hook_fire_repo_root
   preferred $CLAUDE_PROJECT_DIR when deriving the hook POPULATION for report. That reads as
   harmless because a report decides nothing — but the report's two halves are that population and
   the log, so resolving them from two roots earns a NEVER-FIRED row for a hook that fired. It now
   self-locates and requires the root to own this file (-ef, not -e): self-location alone
   still resolves somewhere, and a root that happens to hold a .claude/hooks would otherwise be
   reported on confidently. A root that cannot be established refuses with exit 2, and the two
   refusals state their own distinct causes.

3. A behavioural guard with negative controls, in both launch constructs. Parametrized over the
   derived hook population: each hook is driven with $CLAUDE_PROJECT_DIR naming a decoy tree whose
   scripts/hook-fire-log.sh announces itself, and must not source it. Because that assertion is an
   ABSENCE — which an inert fixture also produces — each has a control where the marker must appear.
   The husky relative launch is tested separately from the absolute one: self-location under a
   relative ${BASH_SOURCE[0]} resolves through the CWD, so it is a different resolution, and it is
   the reachable one. Both run from repo-shaped scratch trees, so no test touches the live repo.

4. The static half pins BYTE-IDENTITY, and that is a withdrawal. A lexical rule over the line was
   defeated three rounds running — ${VAR:-<self>} satisfies a containment test; then backticks and
   $((…)); then $(printenv VAR), $1, $?, an indented or exported reassignment a
   column-anchored regex cannot see, and $'…' quoting. Each fix admitted the next shape, which is
   this repo's recorded signal to stop rather than patch again: a guard was withdrawn from this
   same file after four iterations of pattern-matching shell source. Every non-comment line
   mentioning the variable must now be one of the two canonical lines, byte for byte; the lexical
   arms survive only to say why a line is wrong.

The boundary this does NOT cross

A path the hook consumes as its subject — the tree it is asked to judge — is caller-supplied by
design. Two live cases, both deliberate: pretooluse-bom-guard.sh takes the committed tree from the
command's cd, then the payload cwd, then ${CLAUDE_PROJECT_DIR:-$PWD}; and decisions-guard.sh
cds to git rev-parse --show-toplevel and executes that tree's validator, whose exit code is the
hook's decision. Both are admissible for the same reason, and the test is not data-versus-code but
whose question the path answers. Written into the record so the next sweep does not "fix" them.

Verification

• Red witnessed for every guard, never assumed. Reverting a hook to the pre-#891 form reddens the
  static arm, the absolute-launch test, the husky relative-launch test and the mutation proof — each
  for its stated reason, with the decoy's marker in the output; the other twelve stay green.
  Disarming the byte-identity arm, the CLAUDE_PROJECT_DIR arm, the stray-expansion arm, the -ef
  ownership clause (both as : and as the weaker -e) and the empty-population arm each reddens
  exactly its own proof.
• Negative controls pass for every hook, which is what makes the absences evidence.
• Two arms of the checker were unsubsumed AND unpinned, and now carry proofs. Adversarial review
  found that the begin-call PRESENCE arm was defended only by a prose list:
  test_a_hook_that_LOSES_its_instrumentation_is_DETECTED looks like its proof and is not — it
  strips the whole preamble, which trips four arms on every hook, and asserts only that the fault
  list is NON-EMPTY, so deleting the arm leaves it green with three arms still answering. Sweeping
  the neighbours found a second, if not mode:. Both now have proofs asserting their OWN fault
  message (…_LOSES_its_begin_CALL_…, …_NAMING_NO_STDOUT_MODE_…); disarming either reddens
  exactly its own test on 13/13 hooks and nothing else. All six unsubsumed arms are now pinned, so
  the enumeration beside them is a map rather than the guard.
• Disarm with pass, not if False:. On the two begin-call arms the latter falls through to a
  .group(1) on None and reds with an AttributeError — a red for the wrong reason that reads
  like a proof, and which briefly convinced a review round that an unpinned arm was pinned.
• PYTHONPATH=. python3 -m pytest scripts/tests -q → 1565 passed, 3 skipped; ruff check /
  ruff format --check clean; decisions validator OK; catalog regenerated. Run on a FROZEN committed
  tree (git status --porcelain empty), because editing under a running suite invalidates it.

Docs updated in this PR

• docs/decisions/records/process/hook-resolves-inputs-from-repo-root.md — the rule gains the
  subject/authority boundary and both live cases; "scope actually shipped" records the sweep; the
  mechanics: frontmatter describes byte-identity and names the proofs. Catalog regenerated.
• docs/guard-inventory.md — the row's Blocks column gains the third defect, and the row's
  hand-maintained clause ENUMERATION is withdrawn. Every revision of that list was wrong in a new
  way, and so was the paragraph written to replace it — it misattributed a machine check and
  re-committed the scope error it had itself just recorded. Same shape as the two withdrawals already
  in that file, withdrawn on the same reasoning. What remains is three short paragraphs, each
  measured: read Proof ref and scripts/tests/mutation_manifest.py, which test_mutation_harness.py
  holds against each other and EXECUTES (verified — swapping the row's ref leaves
  test_guard_inventory.py green at 10 passed and reddens
  test_every_declared_mutation_names_the_row_s_OWN_proof_ref); read the arms in the comments INSIDE
  instrumentation_faults; and one fact no artifact records — Blocks is not machine-checked at all,
  because inventory_rows() parses that cell into a discard variable.
• scripts/tests/test_hook_fire_log.py — the instrumentation_faults comment enumerating the arms
  byte-identity does NOT subsume was missing the begin call's PRESENCE (if not m:), a live detector
  pinned by test_a_hook_that_LOSES_its_instrumentation_is_DETECTED. That omission is the list's own
  failure mode — an arm left off it reads as already covered — so it is now listed and the trap named.
• scripts/tests/mutation_manifest.py — the why prose no longer pins an arm count.
• Stale twins swept by subject across code, inventory, manifest and two decision records.

fixes #891 refs #858, #859, #776 ## The defect Every hook under `.claude/hooks/` opens by assigning a path and then `. `-SOURCING it: ```sh ETV_HOOK_FIRE_LIB="${CLAUDE_PROJECT_DIR:-<self-location>}/scripts/hook-fire-log.sh" || true [ -r "$ETV_HOOK_FIRE_LIB" ] && . "$ETV_HOOK_FIRE_LIB" || true ``` Sourcing is execution. Whatever that line names runs as code inside the hook, before stdin is read and before the hook can decide anything — so a file of that name in an env-var-designated tree that prints a decision and exits 0 **is** the decision. #858 fixed one hook (the merge gate, where leaving a total bypass above the gate it was hardening would have made the rest decorative). This is the sweep of the remaining twelve, plus the second resolution inside the sink itself. **Why it is reachable without an attacker.** The obvious objection — `$CLAUDE_PROJECT_DIR` also names the hook binary in `.claude/settings.json`, so a hostile value has already chosen which hook runs — is correct for the Claude PreToolUse hooks, where the two roots agree by construction. It does not hold for the prepush family, because **husky is a different launcher**: `.husky/pre-push` invokes `./.claude/hooks/…`, a path relative to the pushed tree and wholly independent of the variable. A push from one worktree while the environment names another sources the *other* tree's code into a gate that can block or allow the push. Ordinary in a repo that runs several worktrees at once, and it fails by returning a confident wrong answer rather than visibly. ## What changed 1. **All twelve remaining hooks, together, in one change.** Population derived from `git ls-files` (with `-F` — `grep` here is ugrep, and the same pattern without it matches nothing and exits 0, which is how this issue was first filed claiming 11 sites). Every tracked hook now carries the assignment byte-identically. 2. **`scripts/hook-fire-log.sh`'s own resolution reconciled, then hardened.** `etv_hook_fire_repo_root` preferred `$CLAUDE_PROJECT_DIR` when deriving the hook POPULATION for `report`. That reads as harmless because a report decides nothing — but the report's two halves *are* that population and the log, so resolving them from two roots earns a NEVER-FIRED row for a hook that fired. It now self-locates **and requires the root to own this file** (`-ef`, not `-e`): self-location alone still resolves *somewhere*, and a root that happens to hold a `.claude/hooks` would otherwise be reported on confidently. A root that cannot be established refuses with exit 2, and the two refusals state their own distinct causes. 3. **A behavioural guard with negative controls, in both launch constructs.** Parametrized over the derived hook population: each hook is driven with `$CLAUDE_PROJECT_DIR` naming a decoy tree whose `scripts/hook-fire-log.sh` announces itself, and must not source it. Because that assertion is an ABSENCE — which an inert fixture also produces — each has a control where the marker must appear. The **husky relative launch** is tested separately from the absolute one: self-location under a relative `${BASH_SOURCE[0]}` resolves through the CWD, so it is a different resolution, and it is the reachable one. Both run from repo-shaped scratch trees, so no test touches the live repo. 4. **The static half pins BYTE-IDENTITY, and that is a withdrawal.** A lexical rule over the line was defeated three rounds running — `${VAR:-<self>}` satisfies a containment test; then backticks and `$((…))`; then `$(printenv VAR)`, `$1`, `$?`, an indented or `export`ed reassignment a column-anchored regex cannot see, and `$'…'` quoting. Each fix admitted the next shape, which is this repo's recorded signal to stop rather than patch again: a guard was **withdrawn from this same file** after four iterations of pattern-matching shell source. Every non-comment line mentioning the variable must now be one of the two canonical lines, byte for byte; the lexical arms survive only to say *why* a line is wrong. ## The boundary this does NOT cross A path the hook consumes as its **subject** — the tree it is asked to judge — is caller-supplied by design. Two live cases, both deliberate: `pretooluse-bom-guard.sh` takes the committed tree from the command's `cd`, then the payload `cwd`, then `${CLAUDE_PROJECT_DIR:-$PWD}`; and `decisions-guard.sh` `cd`s to `git rev-parse --show-toplevel` and executes *that* tree's validator, whose exit code is the hook's decision. Both are admissible for the same reason, and the test is not data-versus-code but **whose question the path answers**. Written into the record so the next sweep does not "fix" them. ## Verification - **Red witnessed for every guard, never assumed.** Reverting a hook to the pre-#891 form reddens the static arm, the absolute-launch test, the husky relative-launch test and the mutation proof — each for its stated reason, with the decoy's marker in the output; the other twelve stay green. Disarming the byte-identity arm, the `CLAUDE_PROJECT_DIR` arm, the stray-expansion arm, the `-ef` ownership clause (both as `:` and as the weaker `-e`) and the empty-population arm each reddens **exactly its own proof**. - Negative controls pass for every hook, which is what makes the absences evidence. - **Two arms of the checker were unsubsumed AND unpinned, and now carry proofs.** Adversarial review found that the begin-call PRESENCE arm was defended only by a prose list: `test_a_hook_that_LOSES_its_instrumentation_is_DETECTED` looks like its proof and is not — it strips the whole preamble, which trips four arms on every hook, and asserts only that the fault list is NON-EMPTY, so deleting the arm leaves it green with three arms still answering. Sweeping the neighbours found a second, `if not mode:`. Both now have proofs asserting their OWN fault message (`…_LOSES_its_begin_CALL_…`, `…_NAMING_NO_STDOUT_MODE_…`); disarming either reddens exactly its own test on 13/13 hooks and nothing else. All six unsubsumed arms are now pinned, so the enumeration beside them is a map rather than the guard. - **Disarm with `pass`, not `if False:`.** On the two begin-call arms the latter falls through to a `.group(1)` on `None` and reds with an `AttributeError` — a red for the wrong reason that reads like a proof, and which briefly convinced a review round that an unpinned arm was pinned. - `PYTHONPATH=. python3 -m pytest scripts/tests -q` → **1565 passed, 3 skipped**; `ruff check` / `ruff format --check` clean; decisions validator OK; catalog regenerated. Run on a FROZEN committed tree (`git status --porcelain` empty), because editing under a running suite invalidates it. ## Docs updated in this PR - `docs/decisions/records/process/hook-resolves-inputs-from-repo-root.md` — the rule gains the subject/authority boundary and both live cases; "scope actually shipped" records the sweep; the `mechanics:` frontmatter describes byte-identity and names the proofs. Catalog regenerated. - `docs/guard-inventory.md` — the row's `Blocks` column gains the third defect, and the row's hand-maintained clause ENUMERATION is **withdrawn**. Every revision of that list was wrong in a new way, and so was the paragraph written to replace it — it misattributed a machine check and re-committed the scope error it had itself just recorded. Same shape as the two withdrawals already in that file, withdrawn on the same reasoning. What remains is three short paragraphs, each measured: read `Proof ref` and `scripts/tests/mutation_manifest.py`, which `test_mutation_harness.py` holds against each other and EXECUTES (verified — swapping the row's ref leaves `test_guard_inventory.py` green at 10 passed and reddens `test_every_declared_mutation_names_the_row_s_OWN_proof_ref`); read the arms in the comments INSIDE `instrumentation_faults`; and one fact no artifact records — `Blocks` is not machine-checked at all, because `inventory_rows()` parses that cell into a discard variable. - `scripts/tests/test_hook_fire_log.py` — the `instrumentation_faults` comment enumerating the arms byte-identity does NOT subsume was missing the begin call's PRESENCE (`if not m:`), a live detector pinned by `test_a_hook_that_LOSES_its_instrumentation_is_DETECTED`. That omission is the list's own failure mode — an arm left off it reads as already covered — so it is now listed and the trap named. - `scripts/tests/mutation_manifest.py` — the `why` prose no longer pins an arm count. - Stale twins swept by subject across code, inventory, manifest and two decision records.

timothy added 17 commits 2026-08-30 22:31:31 +02:00

wip: 891 for review 843a3f3af7

wip: 891 review round 1 fixes 8fbae8412b

wip: 891 review round 2 fixes — withdraw the parser for byte-identity 3259c67f8e

wip: 891 review round 3 fixes f50858e062

wip: 891 review round 4 fixes e442f98bb8

wip: 891 review round 5 fixes 43f5d3eaec

wip: 891 record the widened-clause shape 19b84e6f46

wip: 891 review round 6 fixes 8082e83c37

wip: 891 review round 7 fixes 4c1595e03e

wip: 891 review round 8 fixes d65b5c0a7e

wip: 891 review round 9 fixes d84289aaea

wip: 891 review round 10 fixes ecf5a62d15

wip: 891 review round 10 fixes (2/2) 50602f418b

wip: 891 review round 11 fixes — withdraw the prose block 37c306505f

wip: 891 review round 12 fixes 453951889b

wip: 891 review round 13 — pin the two unpinned arms instead of describing them ... 61dabe1b7c

Seven rounds of prose defects around one guard, each fix spawning the next. The
mechanism was an arm defended by a hand-maintained list rather than by a test, so
the list had to be believed and kept being wrong.

Both unpinned arms now carry a proof asserting their OWN fault message:
test_a_hook_that_LOSES_its_begin_CALL_is_DETECTED and
test_a_begin_call_NAMING_NO_STDOUT_MODE_is_DETECTED. Disarming either with `pass`
reddens exactly its own test on 13/13 hooks and nothing else.

Decisions-Edit: yes

wip: 891 round 14 — scope the disarm advice, drop a rottable duplicate b1b478f293

timothy referenced this pull request 2026-08-30 22:58:57 +02:00

`test_docs_only_detector_clone_depth.py` flakes ~8% in the CI container (measured, pre-existing on main) #904

timothy commented 2026-08-30 23:40:51 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ b1b478f

15 review rounds. Rounds 1-5 functional; 6-13 prose only, each fix spawning the next until the mechanism (an arm defended by a prose list rather than a test) was removed by pinning both unpinned arms. Rounds 14 and 15 clean; 15 found nothing and independently ran the full suite at this sha (1565 passed, 3 skipped, 0 failed) and verified the rebase is exactly the union of both parents. The two script-suite reds were ersatztv#904, measured pre-existing: 30 runs of that file at this sha failed 2, at main 3.

Review-verdict: MERGEABLE @ b1b478f 15 review rounds. Rounds 1-5 functional; 6-13 prose only, each fix spawning the next until the mechanism (an arm defended by a prose list rather than a test) was removed by pinning both unpinned arms. Rounds 14 and 15 clean; 15 found nothing and independently ran the full suite at this sha (1565 passed, 3 skipped, 0 failed) and verified the rebase is exactly the union of both parents. The two script-suite reds were ersatztv#904, measured pre-existing: 30 runs of that file at this sha failed 2, at main 3.

timothy merged commit 8fd9eae0bf into main 2026-08-30 23:41:20 +02:00

timothy deleted branch fix/891-hook-lib-self-location 2026-08-30 23:41:23 +02:00

timothy referenced this pull request 2026-08-30 23:42:11 +02:00

`ETV_HOOK_FIRE_LIB` is SOURCED from an env-var path in 12 hooks — a decoy tree's code runs inside the gate #891

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#903