diff --git a/lib/hook-utils.sh b/lib/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/lib/hook-utils.sh +++ b/lib/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/lib/hook-utils.test.sh b/lib/hook-utils.test.sh index 1f6caeb798..79865d1d8e 100755 --- a/lib/hook-utils.test.sh +++ b/lib/hook-utils.test.sh @@ -2174,6 +2174,127 @@ else fail "jq_fields non-string handling: got '${HOOK_JQ_FIELDS[0]-}' / '${HOOK_JQ_FIELDS[1]-}'" fi +# --- jq_fields: a NUL in a value must not split the frame (#2122) ------------- +# The separator was drawn from the same byte space as the values it separates, +# so a JSON NUL escape inside a value split that value in two, failed the +# cardinality check, and returned 1 — which every guardrail caller spells +# `|| exit 0`, i.e. ALLOW. jq now STRIPS every NUL out of each value, so the +# separator cannot occur inside one and the record count no longer depends on +# what a parseable payload holds. Stripping is not a claim about how a NUL would +# execute — it SPLICES the bytes either side into a token the payload never +# carried contiguously — which is exactly why the NUL itself is reported in +# HOOK_JQ_FIELDS_NUL: only the caller can own the verdict, and the two guards +# refuse on the flag rather than match the spliced value. +# +# A NUL cannot live in a shell variable, so the payload is built inside jq: +# `[0] | implode` is the one-character NUL string (jq re-emits it as a NUL +# escape on the wire), and `[92] | implode` is a backslash — which keeps both +# a raw NUL and an escape out of this file's own source. +jf_bs=$(jq -rn '[92] | implode') +jf_nul=$(jq -n '{ + tool_name: "Bash", + tool_input: { command: ("git push --no-veri" + ([0] | implode) + "fy") }, + session_id: ("s" + ([0] | implode) + "1"), + only_nul: ([0] | implode), + runs: ("p" + ([0] | implode) + ([0] | implode) + "q"), + literal: (([92] | implode) + "u0000") +}') + +if hook::jq_fields "$jf_nul" \ + '.tool_input.command' '.tool_name' '.session_id' '.only_nul' '.runs' '.literal' '.absent'; then + if [[ "${#HOOK_JQ_FIELDS[@]}" -eq 7 && + "${HOOK_JQ_FIELDS[0]}" == "git push --no-verify" && + "${HOOK_JQ_FIELDS[1]}" == "Bash" && + "${HOOK_JQ_FIELDS[2]}" == "s1" && + -z "${HOOK_JQ_FIELDS[3]}" && + "${HOOK_JQ_FIELDS[4]}" == "pq" && + "${HOOK_JQ_FIELDS[5]}" == "${jf_bs}u0000" && + -z "${HOOK_JQ_FIELDS[6]}" ]]; then + ok "jq_fields: a NUL-bearing value keeps every slot and is stripped, not split" + else + fail "jq_fields NUL framing: got (${#HOOK_JQ_FIELDS[@]}) '${HOOK_JQ_FIELDS[0]-}' / '${HOOK_JQ_FIELDS[1]-}' / '${HOOK_JQ_FIELDS[2]-}' / '${HOOK_JQ_FIELDS[3]-}' / '${HOOK_JQ_FIELDS[4]-}' / '${HOOK_JQ_FIELDS[5]-}' / '${HOOK_JQ_FIELDS[6]-}'" + fi +else + fail "jq_fields returned $? on a payload whose value carried a NUL" +fi + +# The point of the fix is what the CALLER sees. A NUL used to make the helper +# return non-zero, which every guardrail turns into an allow; now it returns +# success and reports the NUL, so the caller can fail CLOSED on its own terms. +if hook::jq_fields "$jf_nul" '.tool_input.command'; then + if [[ "$HOOK_JQ_FIELDS_NUL" == 1 ]]; then + ok "jq_fields: a NUL is reported to the caller instead of returning its fail-open code" + else + fail "jq_fields HOOK_JQ_FIELDS_NUL: expected 1 on a NUL-bearing payload, got '$HOOK_JQ_FIELDS_NUL'" + fi +else + fail "jq_fields returned $? on a NUL-bearing payload — callers spell that ALLOW" +fi + +# A NUL only the LAST filter carries must still raise the flag — the report is +# over every requested value, not just the first. +if hook::jq_fields "$jf_nul" '.tool_name' '.only_nul' && [[ "$HOOK_JQ_FIELDS_NUL" == 1 ]]; then + ok "jq_fields: a NUL in any requested field raises the flag" +else + fail "jq_fields flag on a trailing NUL field: got '$HOOK_JQ_FIELDS_NUL'" +fi + +# A leading NUL KEEPS its text under stripping — it is the splice, not an empty +# value, that a command guard has to survive. `--no-verifyx` arrives as the +# single token `--no-verifyx`, which no matcher recognizes, so a caller reading +# only the value would allow it. The flag is the only thing that separates it +# from a clean payload, which is why the two guards refuse on the flag instead of +# matching the value. +jf_lead=$(jq -n '{tool_input: {command: (([0] | implode) + "git push --force")}}') +if hook::jq_fields "$jf_lead" '.tool_input.command' && + [[ "${HOOK_JQ_FIELDS[0]}" == "git push --force" && "$HOOK_JQ_FIELDS_NUL" == 1 ]]; then + ok "jq_fields: a leading NUL keeps the value's text and still raises the flag" +else + fail "jq_fields leading NUL: got '${HOOK_JQ_FIELDS[0]-}' flag '$HOOK_JQ_FIELDS_NUL'" +fi + +jf_splice=$(jq -n '{tool_input: {command: ("git commit --no-verify" + ([0] | implode) + "x")}}') +if hook::jq_fields "$jf_splice" '.tool_input.command' && + [[ "${HOOK_JQ_FIELDS[0]}" == "git commit --no-verifyx" && "$HOOK_JQ_FIELDS_NUL" == 1 ]]; then + ok "jq_fields: stripping splices a token the payload never carried, and flags it" +else + fail "jq_fields splice: got '${HOOK_JQ_FIELDS[0]-}' flag '$HOOK_JQ_FIELDS_NUL'" +fi + +# A value that is NOTHING BUT NUL bytes strips to empty, and is then +# indistinguishable from an absent field. That case — not a leading NUL — is why +# both guards consult the flag AHEAD of their empty-command skip. +jf_allnul=$(jq -n '{tool_input: {command: (([0] | implode) + ([0] | implode))}}') +if hook::jq_fields "$jf_allnul" '.tool_input.command' && + [[ -z "${HOOK_JQ_FIELDS[0]}" && "$HOOK_JQ_FIELDS_NUL" == 1 ]]; then + ok "jq_fields: an all-NUL value strips to empty and still raises the flag" +else + fail "jq_fields all-NUL: got '${HOOK_JQ_FIELDS[0]-}' flag '$HOOK_JQ_FIELDS_NUL'" +fi + +# Set on EVERY call, not only when a NUL is present: a caller that never resets +# it must not inherit a stale 1 from an earlier invocation. Both assertions run +# the NUL payload FIRST on purpose — a single call cannot observe a stale value +# however it is written. +hook::jq_fields "$jf_nul" '.tool_input.command' +if hook::jq_fields "$jf_input" '.tool_name' && [[ "$HOOK_JQ_FIELDS_NUL" == 0 ]]; then + ok "jq_fields: a clean payload clears the flag rather than leaving it stale" +else + fail "jq_fields stale flag: expected 0 after a clean payload, got '$HOOK_JQ_FIELDS_NUL'" +fi + +# The early returns — no filters, and a host without jq — fire before any NUL +# could be observed, so the flag has to be cleared in the same unconditional +# block that resets the array rather than on detection. In a guard a stale 1 +# would block a clean payload on the strength of an earlier one. +hook::jq_fields "$jf_nul" '.tool_input.command' +hook::jq_fields "$jf_nul" || true +if [[ "$HOOK_JQ_FIELDS_NUL" == 0 ]]; then + ok "jq_fields: an early return clears the flag instead of leaking the previous call's" +else + fail "jq_fields early-return flag: expected 0, got '$HOOK_JQ_FIELDS_NUL'" +fi + echo echo "PASS=$PASS FAIL=$FAIL" [[ $FAIL -eq 0 ]] diff --git a/plugins/actionlint/.claude-plugin/plugin.json b/plugins/actionlint/.claude-plugin/plugin.json index 12701e70b0..6aabb755c2 100644 --- a/plugins/actionlint/.claude-plugin/plugin.json +++ b/plugins/actionlint/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "actionlint", - "version": "0.8.2", + "version": "0.8.3", "description": "Lint GitHub Actions workflow files on edit via actionlint, surfacing findings as advisory context.", "author": { "name": "Melodic Software", diff --git a/plugins/actionlint/CHANGELOG.md b/plugins/actionlint/CHANGELOG.md index 133809ebbf..eb8196ad48 100644 --- a/plugins/actionlint/CHANGELOG.md +++ b/plugins/actionlint/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `actionlint` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.8.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.8.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.8.2] ### Fixed diff --git a/plugins/actionlint/hooks/hook-utils.sh b/plugins/actionlint/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/actionlint/hooks/hook-utils.sh +++ b/plugins/actionlint/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/autonomy/.claude-plugin/plugin.json b/plugins/autonomy/.claude-plugin/plugin.json index 76cef3c4b1..bce46582ec 100644 --- a/plugins/autonomy/.claude-plugin/plugin.json +++ b/plugins/autonomy/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "autonomy", - "version": "0.14.2", + "version": "0.14.3", "description": "Governed autonomous agent operation: role-topology, binding-seam, wiring-vs-advisor, telemetry, return-accounting, trigger-dispatch, per-work-class guardrail-matrix, standing-routine-catalog, and design-only runner-charter contracts for climbing the AI-adoption ladder, plus a guided-setup skill that discovers an adopting org's state, writes its schema-versioned binding, wires standards-pinned OTLP emission with a zero-cost file-artifact default, wires human-attested return capture at the task boundary, wires signal adapters with one governed dispatch entrypoint, binds the five-class guardrail matrix to an org's isolation substrates with an in-boundary live-validation probe before recording each fail-closed binding, and stands up standing-routine-catalog classes as scheduled temporal signal adapters behind the one governed queue with free scheduling defaults wired as reviewable changes and each routine's work-class mapping homed on the security surface.", "author": { "name": "Melodic Software", diff --git a/plugins/autonomy/CHANGELOG.md b/plugins/autonomy/CHANGELOG.md index 7b99bb46a0..8becb4e812 100644 --- a/plugins/autonomy/CHANGELOG.md +++ b/plugins/autonomy/CHANGELOG.md @@ -6,6 +6,22 @@ All notable changes to the `autonomy` plugin are documented here. Format follows Versions 0.1.0–0.7.0 predate this file (introduced with 0.7.1); their history lives in the merged work-package PRs (#333, #343, #356, #372, #377, #600, #676). +## [0.14.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.14.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.14.2] ### Fixed diff --git a/plugins/autonomy/hooks/hook-utils.sh b/plugins/autonomy/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/autonomy/hooks/hook-utils.sh +++ b/plugins/autonomy/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/bash-format/.claude-plugin/plugin.json b/plugins/bash-format/.claude-plugin/plugin.json index e1638ec4f9..efc813aa44 100644 --- a/plugins/bash-format/.claude-plugin/plugin.json +++ b/plugins/bash-format/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "bash-format", - "version": "0.7.2", + "version": "0.7.3", "description": "Auto-format and lint shell scripts on edit via shfmt + ShellCheck, using the consuming repo's own .editorconfig and .shellcheckrc.", "author": { "name": "Melodic Software", diff --git a/plugins/bash-format/CHANGELOG.md b/plugins/bash-format/CHANGELOG.md index 970ae13dad..f15ed048c0 100644 --- a/plugins/bash-format/CHANGELOG.md +++ b/plugins/bash-format/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `bash-format` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.7.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.7.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.7.2] ### Fixed diff --git a/plugins/bash-format/hooks/hook-utils.sh b/plugins/bash-format/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/bash-format/hooks/hook-utils.sh +++ b/plugins/bash-format/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/biome-format/.claude-plugin/plugin.json b/plugins/biome-format/.claude-plugin/plugin.json index ed0a0fb4b2..2afc5fa7b6 100644 --- a/plugins/biome-format/.claude-plugin/plugin.json +++ b/plugins/biome-format/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "biome-format", - "version": "0.6.2", + "version": "0.6.3", "description": "Auto-format and lint JS/TS/JSX/JSON on edit via Biome, only when a biome.json governs the repo — using the consuming repo's own Biome config.", "author": { "name": "Melodic Software", diff --git a/plugins/biome-format/CHANGELOG.md b/plugins/biome-format/CHANGELOG.md index f44086f903..c166cc4a70 100644 --- a/plugins/biome-format/CHANGELOG.md +++ b/plugins/biome-format/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `biome-format` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.6.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.6.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.6.2] ### Fixed diff --git a/plugins/biome-format/hooks/hook-utils.sh b/plugins/biome-format/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/biome-format/hooks/hook-utils.sh +++ b/plugins/biome-format/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/claude-ops/.claude-plugin/plugin.json b/plugins/claude-ops/.claude-plugin/plugin.json index 32eedc98a7..acdda73c84 100644 --- a/plugins/claude-ops/.claude-plugin/plugin.json +++ b/plugins/claude-ops/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "claude-ops", - "version": "0.28.2", + "version": "0.28.3", "description": "Claude Code operations toolkit. Seven skills: observability (read locally captured telemetry — OTEL store, collector, hook-event JSONL, ccusage — with trend reports and store pruning), known-issues (search known Claude product GitHub bugs, check service health, maintain a persistent tracked-issue registry), changelog (ingest Claude Code changelog entries and integrate them into the current repo), plugins (bring a machine's plugin fleet current on demand — marketplace refresh, effective-scope updates including in-repo project/local installs, new-plugin install per policy, scope-divergence detection and explicit convergence), morning-brief (read-only gh-based operator morning view — queue-label counts, merge-ready PRs, parked decisions with their RECOMMENDED lines, and loop-lane telemetry freshness), lanes (start/restart/stop/status loop lanes as named background Claude Code sessions seeded from canonical prompt files, with per-lane model/effort, a repo-pull + marketplace-refresh launch step, and a consume-restarts action — an OS-schedulable reader that relaunches stopped lanes whose telemetry carries a restart_request), and a re-runnable setup action that settles where the known-issues registry lives. Plus a family of seven advisory *-audit telemetry-emitter hooks (API errors, config changes, instruction loads, permission denials, pre-compaction, skill usage, tool failures) that emit the shared hook-telemetry envelope, and a reference sink that maps envelopes into the hook-events.jsonl the observability skill reads.", "author": { "name": "Melodic Software", diff --git a/plugins/claude-ops/CHANGELOG.md b/plugins/claude-ops/CHANGELOG.md index a27245973d..b2cea01994 100644 --- a/plugins/claude-ops/CHANGELOG.md +++ b/plugins/claude-ops/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `claude-ops` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.28.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.28.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.28.2] ### Fixed diff --git a/plugins/claude-ops/hooks/hook-utils.sh b/plugins/claude-ops/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/claude-ops/hooks/hook-utils.sh +++ b/plugins/claude-ops/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/context-guard/.claude-plugin/plugin.json b/plugins/context-guard/.claude-plugin/plugin.json index ddc4f0dd7e..dfdd3fe926 100644 --- a/plugins/context-guard/.claude-plugin/plugin.json +++ b/plugins/context-guard/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "context-guard", - "version": "0.6.2", + "version": "0.6.3", "description": "Per-session context-window observability plus the first shipped consumer: a statusline wrapper tees each session's context_window fields to a per-session snapshot file, a zone resolver classifies usage into smart/acceptable/dumb bands (percentage bands plus window-class token bands, conservative-min combination, zones.json SSOT with shipped defaults), a reader contract fixes how consuming sessions interpret the snapshots, and zone-crossing hooks report once per transition into a worse zone across two channels — the continuation menu to the operator, who owns that choice, and to the model only the zone determination plus the counter-steer that a zone word is not a decay signal (advisory by default; an optional blocking mode gates new mutating work on a fresh dumb-zone snapshot with handoff-writing exempt), with a PostCompact hook persisting an evidence-degraded marker.", "author": { "name": "Melodic Software", diff --git a/plugins/context-guard/CHANGELOG.md b/plugins/context-guard/CHANGELOG.md index a37e1809bd..62295ad857 100644 --- a/plugins/context-guard/CHANGELOG.md +++ b/plugins/context-guard/CHANGELOG.md @@ -5,6 +5,22 @@ All notable changes to the `context-guard` plugin. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.6.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.6.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.6.2] ### Fixed diff --git a/plugins/context-guard/hooks/hook-utils.sh b/plugins/context-guard/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100755 --- a/plugins/context-guard/hooks/hook-utils.sh +++ b/plugins/context-guard/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/desktop-notification/.claude-plugin/plugin.json b/plugins/desktop-notification/.claude-plugin/plugin.json index ab71e24ff0..e1c7ce9443 100644 --- a/plugins/desktop-notification/.claude-plugin/plugin.json +++ b/plugins/desktop-notification/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "desktop-notification", - "version": "0.6.2", + "version": "0.6.3", "description": "Alert you when Claude Code needs input — an audible terminal bell, an OSC 9 terminal notification, and an OS-native toast (macOS/Linux) on permission and idle prompts.", "author": { "name": "Melodic Software", diff --git a/plugins/desktop-notification/CHANGELOG.md b/plugins/desktop-notification/CHANGELOG.md index 8e889dd152..4fffff23ac 100644 --- a/plugins/desktop-notification/CHANGELOG.md +++ b/plugins/desktop-notification/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `desktop-notification` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.6.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.6.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.6.2] ### Fixed diff --git a/plugins/desktop-notification/hooks/hook-utils.sh b/plugins/desktop-notification/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/desktop-notification/hooks/hook-utils.sh +++ b/plugins/desktop-notification/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/eol-normalizer/.claude-plugin/plugin.json b/plugins/eol-normalizer/.claude-plugin/plugin.json index 50e8b2837a..c6d5e890fc 100644 --- a/plugins/eol-normalizer/.claude-plugin/plugin.json +++ b/plugins/eol-normalizer/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "eol-normalizer", - "version": "0.6.2", + "version": "0.6.3", "description": "Normalize a written file's working-tree line endings to its .gitattributes eol value on edit — symmetric CRLF/LF driven by git check-attr, advisory and never blocking.", "author": { "name": "Melodic Software", diff --git a/plugins/eol-normalizer/CHANGELOG.md b/plugins/eol-normalizer/CHANGELOG.md index 5212e710e0..1087db6e37 100644 --- a/plugins/eol-normalizer/CHANGELOG.md +++ b/plugins/eol-normalizer/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `eol-normalizer` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.6.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.6.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.6.2] ### Fixed diff --git a/plugins/eol-normalizer/hooks/hook-utils.sh b/plugins/eol-normalizer/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/eol-normalizer/hooks/hook-utils.sh +++ b/plugins/eol-normalizer/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/go-format/.claude-plugin/plugin.json b/plugins/go-format/.claude-plugin/plugin.json index 049cc81531..0aa7e35bc1 100644 --- a/plugins/go-format/.claude-plugin/plugin.json +++ b/plugins/go-format/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "go-format", - "version": "0.3.2", + "version": "0.3.3", "description": "Auto-fix Go formatting and import management on edit via goimports — runs unconditionally (no consumer-config gate), skipping generated files.", "author": { "name": "Melodic Software", diff --git a/plugins/go-format/CHANGELOG.md b/plugins/go-format/CHANGELOG.md index a2f09d4ed8..3fa4a25854 100644 --- a/plugins/go-format/CHANGELOG.md +++ b/plugins/go-format/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `go-format` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.3.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.3.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.3.2] ### Fixed diff --git a/plugins/go-format/hooks/hook-utils.sh b/plugins/go-format/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/go-format/hooks/hook-utils.sh +++ b/plugins/go-format/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index 1abff997fa..0a8507b19d 100644 --- a/plugins/guardrails/.claude-plugin/plugin.json +++ b/plugins/guardrails/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "guardrails", - "version": "0.24.0", + "version": "0.24.1", "description": "Twelve safety guards that block secret/credential writes, hardcoded machine-specific paths, git hook-bypass attempts, irreversible git operations (force-push, reset --hard, worktree-wide checkout/restore discards), Bash file-write workarounds that circumvent Write/Edit hooks, multi-line `git commit -m` messages (an actual-newline `-m` mangles across shells; single-line `-m` passes), commit subjects and gh pr create titles that violate the repo's tracked team convention (when one is declared in .claude/source-control.md), (advisory) hallucinated CLI flags, (advisory) /plugin:skill references that do not resolve, (advisory) markdown citing a repo path the repo's own history shows was removed, (advisory, opt-in) un-throttled Workflow fan-out that risks burst 529s, and (advisory, opt-in) direct gh pr create calls bypassing this marketplace's own pull-request skill — each independently toggleable.", "author": { "name": "Melodic Software", diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index 63211c4d20..aa25be9f2a 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,6 +3,44 @@ All notable changes to the `guardrails` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.24.1] + +### Fixed + +- **`block-no-verify` and `block-dangerous-git` still allowed a NUL-bearing command after + 0.23.1 (#2122).** 0.23.1 stopped the helper's cardinality failure by stripping every NUL out of + each value, which closed the fail-open for the content guards. It did not close the command + guards: stripping SPLICES the bytes on either side of the NUL into one token the payload never + carried contiguously, and the guards then match against that token. Measured at the hook boundary + on the shipped hooks, `origin/main` at `fd075c27` versus this change, identical fixtures whose NUL + is a real byte decoded from a JSON `\u0000` escape: + + | payload | main | this change | + | --- | --- | --- | + | `git commit --no-verifyx` | **0 allowed** | **2 blocked** | + | `git push --forcex` | **0 allowed** | **2 blocked** | + | a lone NUL, and a trailing NUL | **0 allowed** | **2 blocked** | + | `git commit --no-verify` | 2 blocked | 2 blocked | + | clean `--no-verify` / clean `--force` / harmless | 2 / 2 / 0 | 2 / 2 / 0 | + + The last two rows are stated rather than counted: the splice happens to reassemble a real + `--no-verify` in the fourth row, so `main` already blocks it and it evidences nothing, and no + clean command changed verdict in either direction. + +- **Both guards now fail CLOSED on a NUL byte in any field they read.** The new + `HOOK_JQ_FIELDS_NUL` global reports the byte, and both guards block on it — ahead of their + empty-command skip, so a command consisting only of NUL bytes, which strips to nothing, cannot + pass as "no command". They refuse rather than match because the text a guard can read is not + dependably the text that would run: bash **discards** a NUL while parsing a command it reads, + Node's `child_process` **refuses** a NUL-bearing string outright, and which of them (if either) a + hook payload reaches has not been traced. Refusing is correct under all of them and needs no such + trace. Synced from `lib/hook-utils.sh`. + +- **A non-zero return from `hook::jq_fields` still means the guards allow, and that is unchanged.** + jq being absent, a malformed payload, a wrongly typed field, two concatenated JSON documents, or + an empty buffer all still reach it, and every caller spells it `|| exit 0`. That path is out of + scope here and is documented rather than claimed away. + ## [0.24.0] ### Fixed diff --git a/plugins/guardrails/README.md b/plugins/guardrails/README.md index 6bc5696faa..1e7f7996dd 100644 --- a/plugins/guardrails/README.md +++ b/plugins/guardrails/README.md @@ -70,6 +70,16 @@ out of scope until such a signal exists. **These are friction guards against accidental/casual bypass, not a sandbox.** (A command longer than 16 KB is not parsed and is blocked fail-closed.) +- **A NUL byte in the payload blocks, whatever the command says.** + `block-no-verify` and `block-dangerous-git` refuse any payload whose read + fields carry a NUL, before they look at the command at all — including one + that leaves no command text behind. The reason is that the text a guard can + read is not dependably the text that would run: two behaviours were measured + and they disagree — bash **discards** a NUL while parsing a command it reads, + and Node's `child_process` **refuses** a NUL-bearing string outright — and + which of them, if either, a hook payload reaches has not been traced. Refusing + is the one verdict correct under all of them, and needs no such trace. A NUL is + treated as malformed input rather than as an exotic-but-valid command. - **`block-hook-bypass` string-matching floor.** Detection strips quoted literal spans before matching the executable token, so quoted prose or a commit message merely mentioning `cat >` / `python3 -c open(...)` is not flagged. The diff --git a/plugins/guardrails/hooks/block-dangerous-git.sh b/plugins/guardrails/hooks/block-dangerous-git.sh index f76859dd26..ed24f5ff70 100755 --- a/plugins/guardrails/hooks/block-dangerous-git.sh +++ b/plugins/guardrails/hooks/block-dangerous-git.sh @@ -103,7 +103,32 @@ hook::require_jq "PreToolUse" "guardrails-block-dangerous-git" "$INPUT" # cleared a 40-hex lease that is a movable REF NAME where the push actually runs. # block-noncanonical-commit has read this field since it shipped; this guard did # not, and the same chain is adopted here rather than a second mechanism. +# +# That remaining allow-on-unparsable path is unchanged by #2122 and is NOT what +# the NUL check below covers. hook::jq_fields "$INPUT" '.tool_input.command' '.cwd' '.tool_name' || exit 0 + +# A NUL byte in ANY field read above is fail-CLOSED, and is decided BEFORE +# the empty-COMMAND skip below: the helper strips every NUL out of a value, so a +# command consisting only of NUL bytes arrives EMPTY and would otherwise be waved +# through by that skip as "no command" (#2122). Verified, not assumed — a lone +# NUL yields an empty value with the flag set, while a leading NUL followed by +# text keeps the text. +# +# Blocking rather than matching, because the value a guard can read is not +# reliably the thing that would run. Two behaviours were measured and they +# disagree — bash DISCARDS a NUL while parsing a command it reads, and Node's +# child_process REFUSES a NUL-bearing string outright — and which of them, if +# either, a hook payload reaches has not been traced. Blocking is the one verdict +# correct under all of them, so it needs no such trace. A NUL here is malformed +# input, not an exotic-but-valid command. +if ((HOOK_JQ_FIELDS_NUL)); then + echo "BLOCKED: the payload carries a NUL byte, which a command cannot reliably carry." >&2 + echo "What a guard can read is not dependably what would run, so this is refused rather than matched." >&2 + echo "Fix: reissue the tool call without the embedded NUL." >&2 + exit 2 +fi + COMMAND="${HOOK_JQ_FIELDS[0]}" [[ -n "$COMMAND" ]] || exit 0 HOOK_CWD="${HOOK_JQ_FIELDS[1]}" diff --git a/plugins/guardrails/hooks/block-dangerous-git.test.sh b/plugins/guardrails/hooks/block-dangerous-git.test.sh index 4ae15b0660..cd5f03f8fa 100755 --- a/plugins/guardrails/hooks/block-dangerous-git.test.sh +++ b/plugins/guardrails/hooks/block-dangerous-git.test.sh @@ -827,4 +827,60 @@ run_pwsh "PS: call-op, single-quoted literal git (blocked by name)" \ run_pwsh "PS: call-op, quoted literal path whose basename is git (blocked by name)" \ '& "C:\Git\cmd\git.exe" reset --hard' 2 +# --- A NUL in the payload must not void the guard (#2122) -------------------- +# hook::jq_fields separates its fields with a NUL. A JSON NUL escape inside the +# command used to split that value in two, fail the helper's cardinality check +# and return non-zero — which this hook spells `|| exit 0`, a PreToolUse ALLOW +# with no diagnostic. Asserted at the HOOK boundary, not in the helper, because +# the boundary is where the bypass was observable. +# +# The rule is one line with no exceptions: a NUL in any field the hook reads +# BLOCKS, whatever the surrounding text says. That includes a command whose text +# is entirely NUL bytes, which strips to nothing and would otherwise be waved +# through by the empty-command skip, and a NUL in a command with nothing +# dangerous in it. The guard refuses rather than matching because the text it can +# read is not dependably the text that would run — stripping SPLICES the bytes +# either side of the NUL into a token the payload never carried contiguously — +# and which executor behaviour applies has not been traced. +# +# A NUL cannot live in a shell variable, so the payload is assembled inside jq: +# `[0] | implode` is the one-character NUL string, which jq re-emits as a NUL +# escape on the wire — the form the harness would deliver. +run_nul() { + local label="$1" head="$2" tail="$3" expected="$4" rc + (cd "$REPO_SHA1" && bash "$HOOK" <<<"$(jq -n --arg h "$head" --arg t "$tail" \ + '{tool_name:"Bash",tool_input:{command:($h + ([0] | implode) + $t)}}')" >/dev/null 2>&1) + rc=$? + assert_exit "$label" "$expected" "$rc" +} +run_nul "NUL after --hard (blocked)" "git reset --hard" "" 2 +run_nul "NUL splitting the flag itself (blocked)" "git reset --ha" "rd" 2 +run_nul "NUL then junk (blocked)" "git reset --hard" "x" 2 +run_nul "leading NUL, text preserved (blocked)" "" "git reset --hard" 2 +run_nul "all-NUL command strips to empty (blocked)" "" "" 2 +run_nul "NUL in an otherwise harmless command (blocked)" "git status" "; echo bye" 2 + +# The block has to say what is wrong and what to do about it, not just refuse. +# +# Asserted HERE as well as in block-no-verify.test.sh, and the duplication is the +# point: both guards emit the same three lines by design, so a message edited in +# one and not the other is exactly the drift neither file would otherwise catch. +# Exit-code-only coverage cannot see it — the verdict is identical either way. +nul_stderr() { + bash "$HOOK" <<<"$(jq -n --arg h "$1" --arg t "$2" \ + '{tool_name:"Bash",tool_input:{command:($h + ([0] | implode) + $t)}}')" 2>&1 >/dev/null +} +assert_contains "NUL msg: names the byte" "$(nul_stderr 'git reset --hard' 'x')" "NUL byte" +assert_contains "NUL msg: gives the fix" "$(nul_stderr 'git reset --hard' 'x')" \ + "reissue the tool call without the embedded NUL" + +# The all-NUL command reaches the flag BEFORE the empty-COMMAND skip — its block +# must carry the NUL reason, and an empty command with no NUL must still take +# that skip. The pair is what pins the ordering; either row alone is equally +# consistent with a guard that refuses every empty command or blocks for some +# other reason. +assert_contains "NUL msg: all-NUL command refused by the flag, not skipped" \ + "$(nul_stderr '' '')" "NUL byte" +run "empty command, no NUL (allowed)" "" 0 + report diff --git a/plugins/guardrails/hooks/block-no-verify.sh b/plugins/guardrails/hooks/block-no-verify.sh index cacd9cdbaa..c6844bc42a 100755 --- a/plugins/guardrails/hooks/block-no-verify.sh +++ b/plugins/guardrails/hooks/block-no-verify.sh @@ -75,11 +75,34 @@ hook::require_jq "PreToolUse" "guardrails-block-no-verify" "$INPUT" # Both payload fields in ONE jq process (hook::jq_fields), not two. A jq spawn is # ~140 ms of fork() emulation on Windows Git Bash and this guard runs on every -# Bash/PowerShell call. Failure semantics are unchanged: a missing jq or an -# unparsable payload yields rc 1 here, which exits 0 exactly as the empty-COMMAND -# skip below did — hook::require_jq above has already made the degraded state -# visible once per session. +# Bash/PowerShell call. 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, and +# hook::require_jq above has already made a missing jq visible once per session. +# That remaining allow-on-unparsable path is unchanged by #2122 and is NOT what +# the NUL check below covers. hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 + +# A NUL byte in EITHER field read above is fail-CLOSED, and is decided BEFORE +# the empty-COMMAND skip below: the helper strips every NUL out of a value, so a +# command consisting only of NUL bytes arrives EMPTY and would otherwise be waved +# through by that skip as "no command" (#2122). Verified, not assumed — a lone +# NUL yields an empty value with the flag set, while a leading NUL followed by +# text keeps the text. +# +# Blocking rather than matching, because the value a guard can read is not +# reliably the thing that would run. Two behaviours were measured and they +# disagree — bash DISCARDS a NUL while parsing a command it reads, and Node's +# child_process REFUSES a NUL-bearing string outright — and which of them, if +# either, a hook payload reaches has not been traced. Blocking is the one verdict +# correct under all of them, so it needs no such trace. A NUL here is malformed +# input, not an exotic-but-valid command. +if ((HOOK_JQ_FIELDS_NUL)); then + echo "BLOCKED: the payload carries a NUL byte, which a command cannot reliably carry." >&2 + echo "What a guard can read is not dependably what would run, so this is refused rather than matched." >&2 + echo "Fix: reissue the tool call without the embedded NUL." >&2 + exit 2 +fi + COMMAND="${HOOK_JQ_FIELDS[0]}" [[ -n "$COMMAND" ]] || exit 0 TOOL_NAME="${HOOK_JQ_FIELDS[1]:-Bash}" diff --git a/plugins/guardrails/hooks/block-no-verify.test.sh b/plugins/guardrails/hooks/block-no-verify.test.sh index af864f0e1e..8d0d388725 100755 --- a/plugins/guardrails/hooks/block-no-verify.test.sh +++ b/plugins/guardrails/hooks/block-no-verify.test.sh @@ -312,4 +312,55 @@ assert_contains "PS msg: iex of a literal gets the same actionable advice" \ "$(pwsh_stderr "iex 'git commit --no-verify'")" \ "Drop the iex/'&'/'.'" +# --- A NUL in the payload must not void the guard (#2122) -------------------- +# hook::jq_fields separates its fields with a NUL. A JSON NUL escape inside the +# command used to split that value in two, fail the helper's cardinality check +# and return non-zero — which this hook spells `|| exit 0`, a PreToolUse ALLOW +# with no diagnostic. Asserted at the HOOK boundary, not in the helper, because +# the boundary is where the bypass was observable. +# +# The rule is one line with no exceptions: a NUL in any field the hook reads +# BLOCKS, whatever the surrounding text says. That includes a command whose text +# is entirely NUL bytes, which strips to nothing and would otherwise be waved +# through by the empty-command skip, and a NUL in a command with nothing +# dangerous in it. The guard refuses rather than matching because the text it can +# read is not dependably the text that would run — stripping SPLICES the bytes +# either side of the NUL into a token the payload never carried contiguously — +# and which executor behaviour applies has not been traced. +# +# A NUL cannot live in a shell variable, so the payload is assembled inside jq: +# `[0] | implode` is the one-character NUL string, which jq re-emits as a NUL +# escape on the wire — the form the harness would deliver. +run_nul() { + local label="$1" head="$2" tail="$3" expected="$4" rc + bash "$HOOK" <<<"$(jq -n --arg h "$head" --arg t "$tail" \ + '{tool_name:"Bash",tool_input:{command:($h + ([0] | implode) + $t)}}')" >/dev/null 2>&1 + rc=$? + assert_exit "$label" "$expected" "$rc" +} +run_nul "NUL after --no-verify (blocked)" "git push --no-verify" "" 2 +run_nul "NUL splitting the flag itself (blocked)" "git push --no-veri" "fy" 2 +run_nul "NUL then junk (blocked)" "git push --no-verify" "x" 2 +run_nul "leading NUL, text preserved (blocked)" "" "git push --no-verify" 2 +run_nul "all-NUL command strips to empty (blocked)" "" "" 2 +run_nul "NUL in an otherwise harmless command (blocked)" "echo hi" "; echo bye" 2 + +# The block has to say what is wrong and what to do about it, not just refuse. +nul_stderr() { + bash "$HOOK" <<<"$(jq -n --arg h "$1" --arg t "$2" \ + '{tool_name:"Bash",tool_input:{command:($h + ([0] | implode) + $t)}}')" 2>&1 >/dev/null +} +assert_contains "NUL msg: names the byte" "$(nul_stderr 'git push --no-verify' 'x')" "NUL byte" +assert_contains "NUL msg: gives the fix" "$(nul_stderr 'git push --no-verify' 'x')" \ + "reissue the tool call without the embedded NUL" + +# The all-NUL command reaches the flag BEFORE the empty-COMMAND skip — its block +# must carry the NUL reason, and an empty command with no NUL must still take +# that skip. The pair is what pins the ordering; either row alone is equally +# consistent with a guard that refuses every empty command or blocks for some +# other reason. +assert_contains "NUL msg: all-NUL command refused by the flag, not skipped" \ + "$(nul_stderr '' '')" "NUL byte" +run "empty command, no NUL (allowed)" "" 0 + report diff --git a/plugins/guardrails/hooks/hook-utils.sh b/plugins/guardrails/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/guardrails/hooks/hook-utils.sh +++ b/plugins/guardrails/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/markdown-format/.claude-plugin/plugin.json b/plugins/markdown-format/.claude-plugin/plugin.json index aa582bd8ea..2fb66390f3 100644 --- a/plugins/markdown-format/.claude-plugin/plugin.json +++ b/plugins/markdown-format/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "markdown-format", - "version": "0.11.3", + "version": "0.11.4", "description": "Auto-format and lint Markdown on edit via markdownlint-cli2 — only in repos that carry their own markdownlint config.", "author": { "name": "Melodic Software", diff --git a/plugins/markdown-format/CHANGELOG.md b/plugins/markdown-format/CHANGELOG.md index d7e4919ec0..9d7cb80977 100644 --- a/plugins/markdown-format/CHANGELOG.md +++ b/plugins/markdown-format/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `markdown-format` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.11.4] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.11.2 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.11.3] ### Fixed diff --git a/plugins/markdown-format/hooks/hook-utils.sh b/plugins/markdown-format/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/markdown-format/hooks/hook-utils.sh +++ b/plugins/markdown-format/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/powershell-format/.claude-plugin/plugin.json b/plugins/powershell-format/.claude-plugin/plugin.json index ebcf0e4efb..81c687c2df 100644 --- a/plugins/powershell-format/.claude-plugin/plugin.json +++ b/plugins/powershell-format/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "powershell-format", - "version": "0.7.2", + "version": "0.7.3", "description": "Auto-format and lint PowerShell on edit via PSScriptAnalyzer, only when a PSScriptAnalyzerSettings.psd1 governs the repo — using the consuming repo's own analyzer settings.", "author": { "name": "Melodic Software", diff --git a/plugins/powershell-format/CHANGELOG.md b/plugins/powershell-format/CHANGELOG.md index 66c61d8b91..a1d6455cab 100644 --- a/plugins/powershell-format/CHANGELOG.md +++ b/plugins/powershell-format/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `powershell-format` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.7.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.7.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.7.2] ### Fixed diff --git a/plugins/powershell-format/hooks/hook-utils.sh b/plugins/powershell-format/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/powershell-format/hooks/hook-utils.sh +++ b/plugins/powershell-format/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/rate-limit-guard/.claude-plugin/plugin.json b/plugins/rate-limit-guard/.claude-plugin/plugin.json index f998dc7376..8ad84ae51e 100644 --- a/plugins/rate-limit-guard/.claude-plugin/plugin.json +++ b/plugins/rate-limit-guard/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "rate-limit-guard", - "version": "0.5.2", + "version": "0.5.3", "description": "Shared rate-limit guard for loop lanes: a statusline wrapper tees the subscription rate-limit windows to a fixed machine-scope file, a StopFailure hook records rate-limit stops reactively, and a reader contract fixes how consuming sessions pause and resume.", "author": { "name": "Melodic Software", diff --git a/plugins/rate-limit-guard/CHANGELOG.md b/plugins/rate-limit-guard/CHANGELOG.md index ce1a9189b7..3c487da74c 100644 --- a/plugins/rate-limit-guard/CHANGELOG.md +++ b/plugins/rate-limit-guard/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `rate-limit-guard` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.5.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.5.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.5.2] ### Fixed diff --git a/plugins/rate-limit-guard/hooks/hook-utils.sh b/plugins/rate-limit-guard/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100755 --- a/plugins/rate-limit-guard/hooks/hook-utils.sh +++ b/plugins/rate-limit-guard/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/ruff-format/.claude-plugin/plugin.json b/plugins/ruff-format/.claude-plugin/plugin.json index 737b08894c..3fbfca7bce 100644 --- a/plugins/ruff-format/.claude-plugin/plugin.json +++ b/plugins/ruff-format/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "ruff-format", - "version": "0.6.2", + "version": "0.6.3", "description": "Auto-format and lint Python on edit via Ruff, only when a Ruff config governs the repo — using the consuming repo's own Ruff config.", "author": { "name": "Melodic Software", diff --git a/plugins/ruff-format/CHANGELOG.md b/plugins/ruff-format/CHANGELOG.md index 884d6c9a3f..65458e4272 100644 --- a/plugins/ruff-format/CHANGELOG.md +++ b/plugins/ruff-format/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `ruff-format` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.6.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.6.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.6.2] ### Fixed diff --git a/plugins/ruff-format/hooks/hook-utils.sh b/plugins/ruff-format/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/ruff-format/hooks/hook-utils.sh +++ b/plugins/ruff-format/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index dd2c9fd3af..b418c8c148 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.51.4", + "version": "0.51.5", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /babysit-loop (the loop-lane merge lane: a standing or drain loop that invokes babysit-prs per cycle, configured through repo-scoped babysit_loop_* keys on the layered source-control.md seam, with merge authority human-only until the target repo's tracked config adopts the lane, a gate-proven C2-mechanical baseline once adopted, and standing merge-rung raises binding from the team-tracked layer only — with one named exception, where an invocation line explicitly typing both the autopilot tier keyword and the dedicated raise argument --merge c3-this-run widens that single invocation's merge authority up to C3 behind a fresh independent frontier-tier resolver, while C4-structural and C5-untrusted-provenance stay unconditionally human-merge), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply — interview the repo and write the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index ff3b4bf0bc..55fa81301d 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.51.5] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.51.2 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.51.4] ### Fixed diff --git a/plugins/source-control/hooks/hook-utils.sh b/plugins/source-control/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/source-control/hooks/hook-utils.sh +++ b/plugins/source-control/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For diff --git a/plugins/typos-format/.claude-plugin/plugin.json b/plugins/typos-format/.claude-plugin/plugin.json index c5c4536651..b9b0e6e490 100644 --- a/plugins/typos-format/.claude-plugin/plugin.json +++ b/plugins/typos-format/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "typos-format", - "version": "0.6.2", + "version": "0.6.3", "description": "Spell-check on edit via typos-cli, unconditionally — report-only by default, honoring the consuming repo's own typos configuration when one is present.", "author": { "name": "Melodic Software", diff --git a/plugins/typos-format/CHANGELOG.md b/plugins/typos-format/CHANGELOG.md index 33905a2f20..e8d188602a 100644 --- a/plugins/typos-format/CHANGELOG.md +++ b/plugins/typos-format/CHANGELOG.md @@ -3,6 +3,22 @@ All notable changes to the `typos-format` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.6.3] + +### Fixed + +- **Shared `hook-utils.sh`: `hook::jq_fields` now REPORTS a NUL byte in a payload value + (#2122).** 0.6.1 stopped a NUL from failing the helper's cardinality check, by stripping every + NUL out of each value. That keeps the helper working, but stripping also silently rewrites the + value — `--no-verifyx` arrives as `--no-verifyx` — so a caller that owns a block/allow + verdict cannot tell a clean payload from one that carried a NUL, and matches against a token the + payload never held contiguously. The fact is now reported in a new `HOOK_JQ_FIELDS_NUL` global, + set on EVERY call including every failure path, so such a caller can fail closed on its own terms. + It is computed from the values as the payload carried them, BEFORE the strip; strip first and the + flag would read "0" on every payload. Values themselves are unchanged — still stripped, so a + scanning caller still sees everything after the NUL. This plugin's own hooks do not consult the + new global, so their behaviour is unchanged. Synced from `lib/hook-utils.sh`. + ## [0.6.2] ### Fixed diff --git a/plugins/typos-format/hooks/hook-utils.sh b/plugins/typos-format/hooks/hook-utils.sh index 805f68da60..dcb0e820fc 100644 --- a/plugins/typos-format/hooks/hook-utils.sh +++ b/plugins/typos-format/hooks/hook-utils.sh @@ -622,49 +622,107 @@ hook::jq_field() { # the whole contract) would DROP the field here and silently shift every later # index onto the wrong filter. Emptiness stays the caller's decision. # -# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent or fails, or when -# fewer values came back than filters were asked for, so a partial read can -# never be mistaken for a complete one. Values are CR-stripped, as in +# Returns 1 — with HOOK_JQ_FIELDS empty — when jq is absent, when jq cannot +# parse the payload, or when the record count does not match what was asked for +# in EITHER direction (an over-count rejects too: two concatenated well-formed +# JSON documents parse fine and yield twice the records). Every guardrail caller +# spells that `|| exit 0`, a PreToolUse ALLOW, so what can reach it matters. NUL +# CONTENT no longer can (#2122). A payload jq itself rejects — malformed JSON, a +# wrongly typed field, an empty buffer — still does, exactly as it did before +# this change; that path is untouched here, and process substitution means jq's +# own exit status is not observed either. Values are CR-stripped, as in # hook::jq_field. # +# HOOK_JQ_FIELDS_NUL is set on EVERY call — "1" when any REQUESTED field carried +# a NUL byte, "0" otherwise, and "0" on every failure path — so a caller can +# never inherit a stale "1" from an earlier invocation by forgetting to reset it. +# A caller that owns a block/allow verdict MUST consult it and fail CLOSED. +# # Fields are NUL-separated on the wire, and every value has its own NUL bytes -# REMOVED jq-side first, so the delimiter provably cannot occur inside a -# value: the values carry arbitrary text (a Bash command spans newlines -# routinely), and once NUL is out of the value alphabet no payload can put it -# back. Without that strip, a JSON input encoding a literal \u0000 INSIDE a -# string value — which a Write/Edit/NotebookEdit content field legitimately -# may — made jq emit the raw byte, split that value in two, and fail the -# cardinality check below, turning a caller's `|| exit 0` into a silent skip -# of the entire guard. +# REMOVED jq-side first, so the delimiter provably cannot occur inside a value +# and the record count no longer depends on what a PARSEABLE payload holds. jq +# used to emit the raw byte, the value split in two, the cardinality check below +# failed, and the callers' `|| exit 0` turned a PreToolUse BLOCK into a silent +# ALLOW (#2122). +# +# The FLAG is computed from the values as the payload carried them, BEFORE the +# strip — the ordering is load-bearing and is the one thing a textual merge of +# this function will get wrong. Strip first and `index(0)` looks at a value that +# no longer holds a NUL, so the flag reads "0" on every payload and the guards +# that consult it never fire: a fail-open with no diagnostic, which is the exact +# defect #2122 filed. Anyone editing the jq program must keep the flag ahead of +# the strip. +# +# Removing the NUL is NOT a claim about how a NUL executes, and must not be read +# as one. Two behaviours were measured and they disagree: bash DISCARDS a NUL +# while parsing a command it reads (stdin or a script file), so `echo hard` +# prints `hard`; Node's child_process REFUSES a NUL-bearing string outright, on +# argv, on `shell: true`, and on exec alike. Which of those — if either — a hook +# payload would ever reach is UNTRACED. No value semantics chosen here can claim +# fidelity to the executor, in either direction, and none is claimed. +# +# That is exactly why the verdict is fail-CLOSED on the flag rather than a match +# against the value: blocking is correct under deletion, under truncation, and +# under refusal alike, so it does not depend on anyone having traced the path — +# and it cannot be invalidated by tracing it later. The flag is the load-bearing +# part; the value semantics are not. +# +# Deletion over truncation is chosen on grounds that appeal to no shell at all, +# and it is the disposition #2120 already shipped: a truncating helper hides +# everything after the first NUL from the ten scanning callers #2120 converted, +# none of which consults the flag, so truncation would make a credential past a +# NUL invisible to secret-pattern-detection and hardcoded-path-check. Deletion +# keeps those callers seeing the whole value. The trade runs the other way for a +# COMMAND caller that forgets the flag — `--no-verifyx` joins to +# `--no-verifyx` and matches nothing — which is precisely why the two guards +# refuse on the flag before reading a value at all, rather than relying on the +# disposition to keep them safe. +# +# split/join (1-arity, a plain string split — NOT gsub, which would put a NUL +# inside an Oniguruma pattern) for the strip, and explode/index for the flag, so +# that no regex pattern and no string literal in the jq PROGRAM text carries a +# NUL byte: a construct whose behaviour varied across jq builds would fail EVERY +# payload, which is strictly worse than the payload-dependent bug being fixed. +# +# The library cannot impose the verdict itself — the plugins sourcing it include +# formatters, for which exiting 2 would be wrong — so policy stays with the +# caller and the library only reports the fact. # -# Dropping the NUL rather than encoding around it is not the lesser option, -# it is the only representable one: a bash variable cannot hold a NUL byte, -# so no framing scheme (length prefix, base64, …) could deliver one into -# HOOK_JQ_FIELDS. It is also exactly what the per-field command substitution -# this helper replaced did — $( ) discards NUL bytes and keeps the rest of -# the value, so content AFTER a NUL is still returned and still scanned. -# Read through a process substitution rather than $( ) because the delimiter -# itself must survive the read; command substitution would strip it too. +# Read through a process substitution rather than $( ): command substitution +# strips NUL bytes, which would eat the separators themselves. # # hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || exit 0 +# if ((HOOK_JQ_FIELDS_NUL)); then echo "BLOCKED: …" >&2; exit 2; fi # COMMAND="${HOOK_JQ_FIELDS[0]}" TOOL_NAME="${HOOK_JQ_FIELDS[1]}" -# shellcheck disable=SC2034 # result global is consumed by the sourcing hook, not this file +# shellcheck disable=SC2034 # result globals are consumed by the sourcing hook, not this file hook::jq_fields() { local input="$1" shift HOOK_JQ_FIELDS=() + HOOK_JQ_FIELDS_NUL=0 (($#)) || return 1 command -v jq >/dev/null 2>&1 || return 1 local prog="" filter for filter in "$@"; do [[ -n "$prog" ]] && prog+="," - # split/join (1-arity, a plain string split — NOT gsub, which would put a - # NUL inside an Oniguruma pattern) removes every NUL from the value. - prog+="((${filter}) // \"\" | tostring | split(\"\\u0000\") | join(\"\"))" + # Raw here: the NUL must still be in the value when the flag is computed + # below. The strip happens after, on the whole array. + prog+="((${filter}) // \"\" | tostring)" done - # `-j` concatenates outputs verbatim, so emitting the NUL as its own output - # after each value yields exactly value NUL value NUL … with nothing added. - prog="[$prog] | .[] | (., \"\\u0000\")" + # The NUL flag rides in front of the values as one more record, so reporting + # it costs no second jq process — the whole point of this helper. It is + # computed FIRST, from the values exactly as the payload carried them; only + # then is each value stripped. `-j` concatenates outputs verbatim, so emitting + # the separator as its own output yields exactly value NUL value NUL … with + # nothing added. `A | (X, Y)` feeds the SAME array to both branches, so the + # flag and the values come from one input with no jq variable in the program + # text. Moving the split/join up into the loop above — which is what a textual + # merge of this function produces — would silently zero the flag on every + # payload. + prog="[$prog]" + prog+=' | ((if (map(explode | index(0) != null) | any) then "1" else "0" end),' + prog+=' (.[] | split("\u0000") | join("")))' + prog+=" | (., \"\\u0000\")" local -a values=() local v clean # `read -d ''` (not `mapfile -d ''`, which is Bash 4.4+; this lib supports @@ -684,8 +742,12 @@ hook::jq_fields() { clean="${v//$'\r'/}" values+=("$clean") done < <(printf '%s' "$input" | jq -j "$prog" 2>/dev/null) - ((${#values[@]} == $#)) || return 1 - HOOK_JQ_FIELDS=("${values[@]}") + # One record for the flag plus one per filter. Short of that, jq produced no + # usable output — it is absent, it failed, or it rejected the payload. What can + # no longer shorten it is NUL CONTENT: the values carry no separator byte. + ((${#values[@]} == $# + 1)) || return 1 + HOOK_JQ_FIELDS_NUL="${values[0]}" + HOOK_JQ_FIELDS=("${values[@]:1}") } # Reduce a tool + optional Bash command to a privacy-safe subject label. For