guardrails: twelve hooks read payload values through hook::jq_fields and none consults the NUL signal

[kyle-sexton]

Follow-up, deliberately scoped out of the helper PR so its blast radius is chosen rather than inherited.

State on main today

hook::jq_fields strips NUL bytes from every value (landed with #2120). Twelve hooks call it. Zero consult any NUL signal, and eight own a block verdict:

HOOK                             jq_fields  NUL-check  exit 2
block-convention-violation       2          0          3
block-dangerous-git              2          0          7
block-hook-bypass                2          0          2
block-noncanonical-commit        2          0          5
block-no-verify                  2          0          3
cli-flag-verify                  2          0          1
flag-commit-pr-skill-bypass      2          0          0
hardcoded-path-check             2          0          2
secret-pattern-detection         2          0          2
skill-reference-verify           3          0          0
stale-path-verify                3          0          0
workflow-resilience-check        2          0          0

Ten of the twelve were newly converted by #2120; six of those own exit 2.

Why a signal is needed at all

Whatever the helper does with a NUL — strip it, truncate at it, or refuse — the caller is deciding a verdict on a value that is not the bytes the payload carried. Stripping joins text across the byte; truncating discards everything after it. Either way a hook that reports "scanned, clean" has scanned something other than what it was given, and today it cannot tell.

Two hooks read content fields with no upstream protection of any kind: secret-pattern-detection.sh and hardcoded-path-check.sh both take .tool_input.content, .new_string, .new_source, and both own exit 2. The control-character validation that exists in the harness covers command, script, and url only — and its own message names its purpose as hiding text from the human approval dialog, so it is an anti-spoofing measure rather than an input sanitizer.

What this issue asks for

Once the helper exposes a NUL signal, add the check to the callers — roughly three lines each — so a hook refuses rather than reporting a clean scan of a modified value.

It is not in the helper PR on purpose. Six verdict-owning hooks going fail-closed simultaneously is a real change in behaviour on main, and it deserves its own verification pass rather than riding along with a library change. Doing it in one sweep also means a single mistake affects every guard at once.

Suggested sequencing

1. The helper exposes the signal (in progress).
2. This issue: add checks to the two content-scanning verdict-owners first — secret-pattern-detection.sh and hardcoded-path-check.sh — since those are the ones where a modified value can hide a credential or a machine path.
3. Then the remaining verdict-owners.
4. The four non-verdict hooks last, or never, if refusing is the wrong response for them — that is a per-hook judgement, not a sweep.

Measured, not assumed

The census above was run three times independently and agreed on twelve, zero, and eight. An earlier count of "four and two" was published and was wrong — it enumerated the hooks named in a review thread rather than the conversion set. Re-run it before acting rather than trusting this table; the file list moves.

exit 2 reachability through the content fields specifically was confirmed on both scanning hooks — a credential in .tool_input.content and in .new_string each produce exit 2, with clean controls at 0. Note when reproducing that hardcoded-path-check.sh returns early when CLAUDE_PROJECT_DIR is unset, so a probe without it exits 0 on every payload and reads exactly like "not reachable".

[kyle-sexton]

Scope note — measured, from the #2135 implementation work

hook::jq_fields now reports a payload NUL in HOOK_JQ_FIELDS_NUL, but reporting only helps a caller
that reads it: of the twelve hooks now calling the helper, the ten converted in #2120 do not, and six
of those own an exit 2 — secret-pattern-detection, hardcoded-path-check,
block-convention-violation, block-hook-bypass, block-noncanonical-commit, and cli-flag-verify
(counts measured at 9fb8383d: jq_fields call sites 2–3 each, flag checks 0 each, exit 2 paths
2/2/3/2/5/1).

Until each consults the flag, a NUL-bearing payload still gets a verdict rendered on cleaned text —
for the two content scanners that means a pattern spanning the NUL is joined rather than seen as
written, and for the command guards it means a token the payload never carried contiguously.

#2135 makes the correct behaviour available; it does not make the conversion safe, and merge order
does not change that.

Related, and deliberately not folded into #2135

Both guards call hook::jq_fields … || exit 0. hook::jq_fields still returns 1 for a payload jq
cannot parse, so that path is a fail-open on helper failure — the same class this work exists to
close, one layer up. The NUL case no longer reaches it (the helper strips, flags, and returns 0), so
it does not undermine #2135. Whether the exit 0 is a deliberate "no command could be extracted, so
there is nothing to judge" or an unexamined default is still being established; if it proves to be a
real fail-open it gets its own issue rather than being folded in here.

Not established, and not claimed in either direction

How a command travels from a hook payload to execution is untraced. Bash discards a NUL while
parsing a command it reads; Node's child_process refuses a NUL-bearing string on argv, under both
shell: true and exec. Nobody has established which — if either — applies here, so no
executor-fidelity argument is available for or against any disposition. The design is deliberately
built so that it does not matter: fail-closed is correct under deletion, truncation, and refusal
alike, which is why it is right.

Measurements above were taken on jq 1.8.2 and bash 5.3.9 (Cygwin). No other version was tested.