guardrails: hook::jq_fields returning non-zero allows the command, collapsing jq-absent with payload-unparsable

[kyle-sexton]

Summary

Both blocking guardrails hooks call the payload reader as:

hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0            # block-no-verify.sh:83
hook::jq_fields "$INPUT" '.tool_input.command' '.cwd' '.tool_name' || exit 0     # block-dangerous-git.sh:109

exit 0 is allow. So every non-zero return from hook::jq_fields lets the command through a
hook whose entire job is to block. That path is deliberate and documented — it is not an oversight —
but it collapses two conditions that deserve opposite dispositions.

The two conditions it collapses

hook::jq_fields returns non-zero when:

1. Zero filters requested — a caller bug, unreachable via payload.
2. command -v jq fails — jq is absent. Environmental degradation. Allowing, plus the
   once-per-session notice hook::require_jq already surfaces, is a defensible availability
   tradeoff: the guard cannot function at all, and failing closed would break every Bash tool call on
   a host missing jq.
3. Record-count mismatch in either direction — jq could not parse the payload (malformed JSON,
   empty buffer), a filter aborted jq mid-stream (wrongly typed field), or the payload was two
   concatenated JSON documents (each yields flag plus values, doubling the record count).

Condition 2 is an availability problem. Condition 3 is an integrity problem: jq is present and
working, and the payload did not hold together. Those are not the same event, and allowing is only
obviously right for the first.

Why this is not urgent

Reachability is narrow. An attacker who controls only VALUE content — the command string — cannot
break JSON framing, because the harness serializes it. NUL was the one in-band value-space attack on
this path, and #2135 closes it. Malformed or concatenated payloads require the payload producer
itself to misbehave, and jq-absent is environmental rather than payload-controllable.

So this is a fail-open in principle with no demonstrated payload-driven route to it. Filing it so the
distinction is recorded rather than rediscovered.

What a fix would decide

Whether condition 3 should fail closed (exit 2) while condition 2 continues to allow with a
notice. That requires hook::jq_fields to distinguish the two in its return, which it currently does
not — today both are simply non-zero.

Anyone taking this should establish first whether a real payload producer ever emits a shape that
trips condition 3 in normal operation. If it does, failing closed there converts a silent allow into
a visible outage, and that tradeoff belongs to whoever owns the guards, not to the person who
notices the asymmetry.

Where the current intent is already pinned

• plugins/guardrails/hooks/block-no-verify.sh:76-82 — "rc 1 here means jq is absent, or jq could
  not parse the payload at all — it exits 0 exactly as the empty-COMMAND skip below did… That
  remaining allow-on-unparsable path is unchanged by guardrails: a NUL byte in a payload value makes block-no-verify and block-dangerous-git fail open #2122 and is NOT what the NUL check below
  covers"
• plugins/guardrails/hooks/block-dangerous-git.sh — the identical stance
• The guardrails 0.24.1 CHANGELOG third bullet, which enumerates the same conditions and says they
  are "documented rather than claimed away"

Relationship to other work

Distinct from #2136, which is about the twelve callers not consulting HOOK_JQ_FIELDS_NUL. This is
about what the guards do when the helper returns non-zero at all — a different branch of the same
function, reached by different conditions.

Not a regression, and not introduced by #2135: the path predates it and #2135 leaves it unchanged.

Conditions enumerated from the code at fix/hook-utils-nul-fail-open tip 043d60ce. Measurements on
jq 1.8.2 and bash 5.3.9 (Cygwin); no other version tested.

[kyle-sexton]

Not subsumed by #2178 — do not close this as a duplicate when that merges

PR #2178 (fix/2146-require-jq-fail-closed) is adjacent to this issue and easy to mistake for it. Checked the diff: #2178 changes hook::require_jq, and does not touch hook::jq_fields. A grep of its patch for jq_fields returns no added or removed lines.

The two cover different facts, which is this issue's whole point:

	function	fact it reports	#2178
availability	hook::require_jq	jq is not installed	fixed — now fails closed
integrity	hook::jq_fields	the payload cannot be parsed	untouched

And on current main, hook::jq_fields still folds both into a single non-zero return:

720: hook::jq_fields() {
...
725:   (($#)) || return 1                              # programmer error: no filters
726:   command -v jq >/dev/null 2>&1 || return 1       # jq absent

Three distinct conditions — no filters requested, jq missing, and (further down) a payload jq cannot read — all exit 1, and every caller sees one undifferentiated failure. That collapse is what this issue asks to separate, and #2178 does not address it.

Once #2178 lands, the two postures will also be visibly inconsistent within the same file: require_jq will fail closed on "I cannot read this input", while jq_fields still returns a value the callers treat as permissive. That inconsistency is an argument for resolving this issue, not evidence that it is already resolved.

Still open, unchanged: the disposition question in the title — whether jq-present-but-payload-unparsable should fail closed, and whether the caller contract should distinguish the three conditions rather than overloading one return code.