guardrails: a NUL byte in a payload value makes block-no-verify and block-dangerous-git fail open

[kyle-sexton]

Empirically confirmed against the installed, shipped guardrails 0.21.0, at the hook boundary — not a code-read inference, and not confined to an open PR.

One premise is explicitly unverified, and the finding is scoped accordingly: whether a NUL can actually reach .tool_input.command through the harness payload serialization is not something I can establish from here. Everything below is measured given such a payload. The defect stands either way — failing open on a parse failure is wrong regardless of how the parse came to fail — but the reachability question belongs to whoever fixes this, and should not be read as settled.

The defect

hook::jq_fields (hooks/hook-utils.sh) frames its fields with a NUL delimiter and validates the result by cardinality alone:

prog="[$prog] | .[] | (., \"\u0000\")"
...
while IFS= read -r -d "" v; do ... done < <(printf %s "$input" | jq -j "$prog" 2>/dev/null)
((${#values[@]} == $#)) || return 1

The delimiter is drawn from the same byte space as the decoded values it separates. \u0000 is valid JSON, so a payload value may legitimately contain a NUL. When it does, that one value splits into two, the cardinality check fails, and the function returns 1. Callers treat that as "cannot parse, carry on":

hooks/block-dangerous-git.sh:97  hook::jq_fields "$INPUT" ... || exit 0
hooks/block-no-verify.sh:82      hook::jq_fields "$INPUT" ... || exit 0

|| exit 0 is a PreToolUse allow.

Measured at the hook boundary

Not the library function in isolation — the shipped hooks themselves, fed on stdin:

block-no-verify.sh         clean   exit=2  BLOCKED: --no-verify / -n flags are not allowed with git pus...
block-no-verify.sh         NUL     exit=0  (no output)
block-dangerous-git.sh     clean   exit=2  BLOCKED: git reset --hard discards uncommitted work with no ...
block-dangerous-git.sh     NUL     exit=0  (no output)

Appending a JSON NUL escape to the command flips a blocked call into an allowed one, on both hooks, with no diagnostic of any kind. Reproduce with:

source <cache>/melodic-software/guardrails/0.21.0/hooks/hook-utils.sh
P="{\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"git push --no-verify\u0000x\"}}"
printf %s "$P" | bash <cache>/melodic-software/guardrails/0.21.0/hooks/block-no-verify.sh; echo rc=$?

Both hooks are on main today.

Direction

Failing closed is the smaller half and is not sufficient alone — a legitimate NUL-bearing payload would then block real work, converting a silent allow into a silent denial. The framing is the root cause: a delimiter must not be drawn from the value space it delimits. Length-prefixed framing, an explicit count emitted by jq and validated by the reader, or per-field @base64, all remove the collision without depending on payload contents.

Relationship to PR #2120

Review thread PRRT_kwDOTCGFQM6XtLpy on #2120 raises this against secret-pattern-detection.sh, which that PR is newly converting to hook::jq_fields. That finding is correct and should stand.

The scope is wider than the thread claims, in both directions. #2120 does not touch hooks/hook-utils.sh — its file list is nine hooks plus the manifest and changelog — so it does not address the root cause. And the two hooks above already consume jq_fields on main, so the bypass is live in shipped code independent of that PR. #2120 therefore extends the same failure mode to nine further hooks while leaving the collision in place. Fixing the framing in hook-utils.sh fixes every consumer at once, present and pending.

[kyle-sexton]

Three corrections to this issue, two of which weaken or complicate what I originally filed. All verified against main at 53a3ff61. Below, <NUL> denotes a JSON escape of the form backslash-u-0000 in the payload — jq materializes the byte on parse, which is the actual attack shape.

1. My reproduction payload is the weakest of the set — use a trailing NUL instead

I published git push --no-verify<NUL>x. It does fail open, but it is the one payload that will still be allowed after a correct fix, because it normalizes to --no-verifyx, which is not a --no-verify invocation. As a regression witness it is nearly useless: it looks identical before and after.

Every placement fails open today. Measured at the hook boundary on shipped 0.21.0:

== block-no-verify ==
clean control                      exit=2 blocked
trailing NUL                       exit=0 <- ALLOWED
NUL mid-keyword                    exit=0 <- ALLOWED
NUL then x (my original repro)     exit=0 <- ALLOWED
NUL early in command               exit=0 <- ALLOWED
== block-dangerous-git ==
clean control                      exit=2 blocked
trailing NUL                       exit=0 <- ALLOWED
NUL mid-keyword                    exit=0 <- ALLOWED

The canonical repro is git push --no-verify<NUL> — a genuine --no-verify invocation, allowed today, which must flip to exit 2. git reset --hard<NUL> and the mid-keyword git push --no-veri<NUL>fy are the other two that must flip. Any fix should be judged against those three, not against the payload I first published.

2. The root cause is narrower than "the count mismatches" — one exit path serves two conditions

Both callers spell the failure hook::jq_fields ... || exit 0, and the comment above each says the failure semantics exist for a missing jq, already surfaced once per session by hook::require_jq. That is a defensible response to a broken environment.

The NUL case is a different condition entirely — jq ran fine, the framing broke — riding the same exit path. So the defect is not merely that a count mismatches; it is that one || exit 0 conflates an environment fault with a payload-parsing fault, and those two have opposite correct responses. A fix that makes the count match while leaving both conditions sharing that exit has relocated the symptom rather than removed it.

3. Scope is much larger than this issue implies: hook-utils.sh is vendored 16 times

plugins/guardrails/hooks/hook-utils.sh is not the source of truth. lib/hook-utils.sh is, and scripts/sync-hook-utils.sh materializes it into every carrying plugin, with CI enforcing both --check (all copies byte-identical) and --check-bump <ref> (every carrying plugin version-bumped when the lib changes).

So a fix confined to the guardrails copy fails CI. The correct change is the lib plus all vendored copies plus a version bump and changelog entry per carrying plugin — precedent 9b90e351, ~50 files.

This does not widen the exploitable surface: the only two callers that spell || exit 0 on a payload parse remain block-dangerous-git.sh and block-no-verify.sh, and the other 15 carrying plugins are formatters that gate nothing. It widens the diff.

Residual that should be named, not silently fixed

After the framing is corrected, hook::jq_fields still returns 1 when jq is absent or errors, and callers still spell that || exit 0. No payload content can reach that path any more — only a genuine environment fault can, which hook::require_jq already surfaces. Collapsing it would require editing hook scripts across 15 formatter plugins where the current behaviour is correct. Worth naming as a known residual rather than leaving a reader to assume the conflation in item 2 was fully dissolved.

Items 2 and 3 came from two independent reviewers, not from me. Item 2 in particular reframes what "fixed" has to mean here.

[kyle-sexton]

RETRACTION — item 1 of my previous comment is wrong, and wrong in the dangerous direction. Correcting it immediately because a fix built on it would be worse than the bug this issue reports. <NUL> below denotes a JSON escape of the form backslash-u-0000.

What I got wrong

I wrote that git push --no-verify<NUL>x "normalizes to --no-verifyx, which is not a --no-verify invocation" and was therefore "nearly useless as a regression witness."

A NUL does not normalize or join neighbouring text. It TRUNCATES. Verified directly: passing a NUL-bearing argument through bash yields only the text before the NUL, and a real binary receiving one reports the truncated flag (git given a NUL inside --version answers unknown option: --vers).

argv entries are NUL-terminated C strings, so everything from the first NUL onward is discarded before the program is entered. This is execve semantics, not a platform quirk.

So git push --no-verify<NUL>x executes as git push --no-verify. It is not benign. It is the dangerous command exactly, and it is one of the most important witnesses in the set — the opposite of what I said.

Why this matters more than a corrected sentence

It changes what a correct fix looks like, and it rules out the most natural implementation.

The obvious disposal — strip NULs from the value, which also matches the CR handling directly above in the same function — produces this:

• the guard analyses --no-verifyx and allows,
• the shell executes --no-verify and bypasses,
• and the divergence is now by design, silent, and passes every test written against the stripped string.

That is strictly worse than the bug reported here. The current fail-open at least fails visibly across a whole payload class. A strip-based fix yields a guard that reasons confidently about a command that will never run.

Any fix that strips NULs from the command field must be rejected, however clean its test matrix looks.

Correct dispositions

1. Fail closed on any NUL in a payload value. Recommended. A NUL in a command string has no legitimate use, and this is correct under every candidate executor semantics.
2. Truncate at the first NUL, matching the executor, so the guard analyses exactly what will run.

Stripping is not a third option.

Open question that decides between them — not yet settled

Truncation is verified at the shell/execve boundary. What Claude Code own executor does between the hook and the shell is not established. If that layer strips rather than truncates, the correct disposition flips. A third possibility: Node child_process rejects arguments containing NUL outright, so the command may error rather than run.

The two dispositions give opposite correct answers on several witnesses — a leading NUL truncates the command to empty and should therefore be allowed under truncation, while a strip-based rule would demand it be blocked. So the disposition cannot be left implicit; it has to be decided, stated, and tested against.

Until that is settled empirically, fail closed is the only disposition correct under every branch.

Corrected witness table

All of these fail open today. What differs is what each must do afterward, and that depends on the disposition chosen above:

clean control              exit 2  -> must stay 2
trailing NUL               exit 0  -> must become 2
NUL right after keyword    exit 0  -> must become 2   (truncates to --no-verify)
NUL inside keyword         exit 0  -> truncates to --no-veri; verify whether still matched
NUL in a -m value          exit 0  -> must become 2   (--no-verify intact either way)
leading NUL                exit 0  -> allowed under truncation; blocked under strip
destructive-reset + NUL    exit 0  -> must become 2

A note on that last row: this comment could not originally be posted, because block-dangerous-git blocked the command carrying it — the literal reset invocation appeared in the body text. The guard behaved correctly on a payload with no NUL in it, which is a small live demonstration that the non-NUL path is sound and the defect really is confined to the framing.

Credit: an independent reviewer caught this against my published claim, before any code shipped on it. My original filing stands; my correction to it did not.

[kyle-sexton]

The open question from my previous comment is now settled empirically, and it resolves in favour of the recommendation rather than against it. It belongs in any fix as a demonstrated fact, not a deferred uncertainty.

Node child_process rejects NUL-bearing arguments outright

Reproduced independently on node v24.18.0:

node v24.18.0
args[] with NUL              THROW ERR_INVALID_ARG_VALUE: The argument args[1] must be a
                             string without null bytes.
args[] control, no NUL       OK  git version 2.54.0.windows.1
shell:true, NUL in command   THROW ERR_INVALID_ARG_VALUE: The argument file must be a
                             string without null bytes.
shell:true control, no NUL   OK  AAA

Both shapes throw — a NUL in an args[] entry, and a NUL in the command string under shell: true. The no-NUL controls pass, so the rejection is specific to the byte rather than an artefact of the surrounding harness.

The three boundaries, complete

• Node child_process — rejects; nothing executes.
• execve / argv — truncates at the first NUL.
• strip — implemented by no runtime in this path, and the disposition the most natural fix would have chosen.

So fail-closed is not merely the safe default; it is the only disposition correct at every boundary. No branch remains in which stripping or silently allowing is right.

Two things this does NOT license

1. Not an argument that the fail-open is harmless. If the runner is Node child_process, a NUL payload never reaches a shell. But the job of a guard is to return a verdict, and "something downstream will probably refuse it" is a property of a layer the guard does not own, cannot see, and is not versioned against. A guard that allows because it expects a later rejection is not a guard. The fix is still required.

2. Not an argument for looser matching. It tightens the case for rejecting outright: a NUL in the command field cannot correspond to any command this executor would ever run, so it is malformed input rather than an exotic-but-valid command. Fail closed, and say so in the code.

Precision caveat, stated rather than glossed

Demonstrated: (a) Node child_process rejects NUL-bearing arguments, and (b) execve truncates. NOT demonstrated: that Claude Code invokes child_process unmodified with these arguments — nobody has read that invocation path. The conclusion is robust to the gap, since fail-closed is correct whether the executor rejects, truncates, or strips, but a PR body should claim only the two results above.

A separate question is worth keeping distinct from this one: whether a NUL-bearing payload can reach a PreToolUse hook at all, given that tool-input validation rejects control characters before execution. If it cannot, this issue is a robustness defect rather than a live bypass. That is under investigation and does not change the fix.

[kyle-sexton]

Reachability settled, and it goes the opposite way to the hypothesis I raised. This is a live bypass, not a robustness defect. Do not re-scope it down.

The only control-character filter is per-field, and covers three fields

Extracted from the installed binary (2.1.226), every occurrence of that guard message:

command contains control characters that would be hidden in the approval dialog
script  contains control characters that would be hidden in the approval dialog
url     contains control characters that would be hidden in the approval dialog

Three fields. No equivalent guard exists for content, file_text, new_string, old_string, or prompt — searched for any differently-worded control-character or null-byte validation and the only other hits are Node internal path checks and documentation prose.

Cross-referenced against what the hooks actually read

11  tool_input.command      GUARDED
 2  tool_input.script       GUARDED
 5  tool_input.new_string   UNGUARDED
 5  tool_input.file_path    UNGUARDED
 5  tool_input.content      UNGUARDED
 2  tool_input.replace_all  UNGUARDED
 2  tool_input.new_source   UNGUARDED

Most hook-read fields have no control-character validation at all. So a NUL reaching a hook does not depend on the Bash-path ordering question I raised — it can arrive today through Write/Edit content or new_string, with no filter anywhere in that path.

The filter is not a security boundary, and its own message says so

The official hooks documentation says nothing about control characters, null bytes, input validation, or where validation sits relative to PreToolUse. So it is an implementation detail, not a documented guarantee.

More decisively, the message text states the purpose: "would be hidden in the approval dialog". That is an anti-spoofing measure for the human approval UI, not an input sanitizer. It exists so a command cannot conceal its true content from a human reviewer — which is precisely why it covers command, script, and url, and nothing else. Treating it as a sanitizer for hook input is a category error, and that conclusion holds however the ordering question resolves.

Consequence for PR #2120

That conversion should not land until this helper fails closed. #2120 converts secret-pattern-detection.sh to hook::jq_fields, and that hook reads content and new_string — fields with no upstream filter of any kind. Landing the conversion first would extend the fail-open into the least-protected field in the set. This makes #2120 the more urgent item, not the lesser one.

Still unresolved, marked rather than guessed

• Whether the character class on those three guarded fields includes NUL specifically. No [\x00-\x1F]-style literal was locatable in the minified bundle. Unresolved.
• Whether that validation runs before or after PreToolUse hooks. The docs are silent, and the only probe that would cleanly discriminate requires pairing a control character with a command a guard should block — a bypass-shaped payload, deliberately not run. Largely moot now: for the Write/Edit path there is no validation to be ordered against.

Net

Fail-closed remains correct, now on two independent grounds: it is right under all three NUL dispositions, and the only upstream filter that exists is an approval-UI anti-spoofing measure that does not cover the fields most hooks read.