timothyandtimothy
cf5f42edf9
#859 was filed as a wrong STATED CAUSE. It was masking a live false-open in the merge gate. Gitea reports a GLOB branch-protection rule with an EMPTY `branch_name` — the canonical name lives only in `rule_name`. Measured 2026-08-30 on a scratch repo against 1.27.1. jq's `//` fires on null and false but NOT on `""`, so `(.branch_name // .rule_name // "")` resolved every glob rule to the empty string — a name with no metacharacters — and the glob test, the entire basis of the classifier's undecidable-first ordering, never saw it. Measured on the predecessor: glob `m*` (not requiring review-verdict/h10) beside plain `main` (requiring it) resolved to `exact` on `main` and AUTO-GRANTED a scheduled merge, while Gitea — ordering by Priority then plain-name-ness — may be applying `m*`. That is #622's hole, reached through the ordering written to close it. Mirror case: a glob alone resolved to `none` and DENIED about a rule that provably governs the base. A name is now a non-empty string. Each field resolves to a NAME, a SKIP (absent/null/ empty — fall through), or POISON (present, wrong type — poisons whichever field carries it). A rule with no usable name is a distinct `unreadable` verdict with its own operator cause, instead of feeding `none`, whose whole authority is "the full rule list was read and none matches". The short-circuit is STRUCTURAL: jq binds `as` eagerly, so the flat form still evaluated `offs`/`nonascii` on the bad name and died before reaching the arm meant to prevent that. Also #859: `branch_protections` is fetched ONCE per run, not twice. The round trip is the smaller half — it is mutable config, so two reads can disagree and the two arms then decide about different repo states with neither able to notice. #858: `verdict_script` resolves from `$repo_root`, not `$CLAUDE_PROJECT_DIR`. And the finding that mattered more — `ETV_HOOK_FIRE_LIB` is `. `-SOURCED, so it is CODE running before stdin is read and before `decide` exists. A first draft exempted it as "telemetry, not a predicate"; cold review refuted that by execution: a decoy hook-fire-log.sh in an env-var-named tree printing an allow and exiting 0 GRANTS THE MERGE, bypassing every check. Classify a path by how it is CONSUMED, never by what it is called. This hook's copy is self-located; the other twelve are #891 (high/security), which records the reachable case — husky launches the prepush hooks by RELATIVE path, so the two roots diverge there. check-required-contexts.sh gains an array-type gate (a JSON object previously printed `nomatch`, a positive claim about server config from a body it cannot consume). Verification: 1377 passed / 2 skipped; 11 declared mutants, 11 detected, disjoint reddened sets; classifier executed across jq 1.8.2 and 1.6 with identical results; both env-var tests ship a negative control, because the passing outcome is also what an inert decoy produces. Four cold review rounds plus a bounded prose check. Every round found defects the previous round's fixes introduced — a type conflation that re-opened the auto-grant, a comment asserting the opposite of the line its own commit changed, and a corrected sentence whose identical twin survived in the same diff. Docs: new record `process.hook-resolves-inputs-from-repo-root`; both inline sites cite it rather than arguing it twice. docs/remote-state-inventory.md's row for the second read updated. Follow-ups filed: #891 (the other 12 hooks), #895 ("all N tests green" claims). fixes #858 fixes #859 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
5.8 KiB
key, title, status, since, supersedes, superseded-by, rule, signals, mechanics
| key | title | status | since | supersedes | superseded-by | rule | signals | mechanics |
|---|---|---|---|---|---|---|---|---|
| process.hook-resolves-inputs-from-repo-root | 2026-08-30 — a hook resolves every path it sources or executes from `$repo_root`, never from an environment variable (#858) | active | 2026-08-30 | none | none | A hook resolves every path whose CONTENT it sources, executes, or consults to decide, from `$repo_root` — derived from the hook's own `${BASH_SOURCE[0]}` — and never from `$CLAUDE_PROJECT_DIR` or any other environment variable. There is NO telemetry exemption, and the attempt to write one is instructive: the first draft of this record exempted `ETV_HOOK_FIRE_LIB` on the grounds that "a wrong log destination is not a wrong verdict", and cold review refuted it by execution — that path is `. `-SOURCED, so whatever it names runs as CODE inside the hook before stdin is read and before `decide` exists; a file there that prints an `allow` decision and exits 0 grants the merge having bypassed every check in the file. A path's PURPOSE does not bound its authority; how the hook consumes it does. The failure that matters is not an attacker — the same variable already names the hook binary in `.claude/settings.json`, so a hostile value has chosen which hook runs and the gate is moot before any inner path is read — it is LAUNCHER DIVERGENCE, and it is reachable: husky invokes the prepush hooks as `./.claude/hooks/…`, a relative path wholly independent of `$CLAUDE_PROJECT_DIR`, so the two roots genuinely disagree there. A wrong path holding a plausible file returns a confident answer about another tree; a missing path only asks, so the silent direction is the dangerous one. Two halves of one comparison must never come from two roots. | env var as a security input · `$CLAUDE_PROJECT_DIR` vs `$repo_root` · `${BASH_SOURCE[0]}` self-location · sourced file is code not configuration · which checkout's code decided this · launcher divergence · husky relative path vs harness env · a wrong path returns a confident answer, a missing one asks · both halves of a comparison from one root · paths: `.claude/hooks/pretooluse-merge-consent.sh`, `scripts/hook-fire-log.sh`, `scripts/check-review-verdict.sh`, `scripts/check-required-contexts.sh` · issues: #858, #891, #787, #629 | No linter can spot "this path should have come from `$repo_root`". Enforcement is review plus a behavioural test per site: point `$CLAUDE_PROJECT_DIR` at a decoy tree whose copy of the resolved file returns the OPPOSITE decision, and assert the outcome is unchanged. That test needs a NEGATIVE CONTROL or it proves nothing — the passing outcome is also what an inert decoy produces — so run the same decoy again with the hook COPIED INTO it, making the decoy genuinely `$repo_root`, and assert the decision DOES flip. Two worked pairs live in `test_merge_consent_required_check.py`: `test_CLAUDE_PROJECT_DIR_cannot_choose_which_verdict_classifier_runs` and `test_the_SOURCED_fire_log_library_cannot_be_chosen_by_CLAUDE_PROJECT_DIR`, each beside its `..._IS_honoured/sourced_when_it_genuinely_is_the_repo_root` control. |
.claude/hooks/pretooluse-merge-consent.sh resolved three decision-bearing paths from two different
roots, in one file. #787 bound the guard-scope freshness checker and its snapshot to $repo_root and
wrote the reasoning beside them; the H10 verdict classifier, added at #629, kept
${CLAUDE_PROJECT_DIR:-.}; and the sourced fire-log library kept ${CLAUDE_PROJECT_DIR:-<self>}.
Two resolutions of the same question inside one file is the state most likely to be "tidied" toward the weaker one. That is why this is a record rather than a second inline comment: a reader reconciling the inconsistency has even odds of reconciling it the wrong way, and the comment that would have stopped them is attached to the other site.
The exemption this record tried to grant, and why it does not exist. The first draft drew the
boundary at "does its CONTENT change the decision" and placed ETV_HOOK_FIRE_LIB outside it as
telemetry. Cold review refuted that by running it: a decoy hook-fire-log.sh in an env-var-named tree
printing an allow decision and exiting 0 took the merge decision, 500 lines before the gate. The
line is . -SOURCED — sourcing is execution, and a file's stated purpose grants no protection at all.
The lesson generalises past this variable: classify a path by how it is CONSUMED, never by what it
is called.
Name the real threat, or the rule gets argued away. "An env var is a security input" invites the
correct objection that $CLAUDE_PROJECT_DIR already selects the hook itself, so an attacker who
controls it has won already — and an argument refutable on its face gets the rule reverted. The
defensible claim is narrower and holds: the harness launcher and husky are DIFFERENT launchers.
Claude PreToolUse hooks are started as $CLAUDE_PROJECT_DIR/.claude/hooks/…, so there the two roots
agree by construction and binding to $repo_root is consistency rather than repair. Husky starts the
prepush hooks as ./.claude/hooks/… from the pushed tree, wholly independent of the variable —
measured 2026-08-30, a push from one worktree with the variable naming another sources the other
tree's code into a gate that can block or allow the push. That is ordinary, non-adversarial, and
produces a confident wrong answer rather than a visible failure.
Scope actually shipped, so nobody reads more into it. #858 fixed the verdict classifier and the
fire-log resolution in pretooluse-merge-consent.sh only. The other twelve tracked hooks still
carry the env-var-first fire-log line; sweeping them needs its own derived population and its own
review, and is #891. One copy was fixed because leaving a total gate bypass above the gate that PR
was hardening would have made the rest of it decorative — not because one hook is special.