From d89c5ff1e52714813e4ce0f1e93fa4bab185ac8e Mon Sep 17 00:00:00 2001 From: Florian DAVID Date: Sat, 19 Sep 2026 14:16:08 +0200 Subject: [PATCH 1/2] fix: a tool_input value could pose as a JSON key, and requires: had no byte cap (#426, #427) pre-tool-hook.sh/pre-path-hook.sh/post-tool-hook.sh read their JSON payload positionally: every quoted field at an even split position was treated as a candidate key with no check that it sat at key position and no brace-depth tracking, so a tool_input STRING VALUE equal to tool_name/command/file_path/ pattern/skill/subagent_type could repoint the field it named -- last-wins, byte-identical to a genuine non-match -- bypassing every mode: block rule (#426). jit_hook_fields() (common.sh) replaces the positional read with a structural one: a string is a key only when followed by a colon, and a value is read only at the correct depth. Separately, JIT_MISSING_REQUIRES was the one exec-crossing list in common.sh with no byte cap, unlike every sibling that crosses the same pre-tool-hook.sh exec boundary -- an oversized committed requires: value could trip E2BIG and refuse (or, on an unwritable TMPDIR, silently pass) every tool call in a session (#427). jit_missing_requires() now refuses a non-bare-name value outright and caps the accumulated list with a reported truncation; rebuild-tsv.sh applies the same bare-name check at index time. Found by a Claude Security whole-repository scan; both confirmed by reading the awk/bash source rather than executed. Co-Authored-By: Max --- changelog.d/426.security.md | 1 + changelog.d/427.security.md | 1 + scripts/common.sh | 130 ++++++++++++++++++++++++++++++++++- scripts/post-tool-hook.sh | 22 +++--- scripts/pre-path-hook.sh | 20 +++--- scripts/pre-tool-hook.sh | 70 ++++++++++--------- scripts/rebuild-tsv.sh | 13 ++++ tests/test-pre-tool-hook.sh | 49 +++++++++++++ tests/test-requires-field.sh | 110 +++++++++++++++++++++++++++++ 9 files changed, 365 insertions(+), 51 deletions(-) create mode 100644 changelog.d/426.security.md create mode 100644 changelog.d/427.security.md diff --git a/changelog.d/426.security.md b/changelog.d/426.security.md new file mode 100644 index 0000000..27855ee --- /dev/null +++ b/changelog.d/426.security.md @@ -0,0 +1 @@ +- **A `tool_input` string value could pose as a JSON key and repoint `tool_name`/`command`, bypassing every `mode: block` rule** (#426). `pre-tool-hook.sh`, `pre-path-hook.sh` and `post-tool-hook.sh` each read the payload positionally: every quoted field at an even split position was treated as a candidate key, with no check that it actually sat at key position (followed by `:`) and no brace-depth tracking to tell a top-level field from one nested inside `tool_input`. A `tool_input` value equal to `tool_name`, `command`, `file_path`, `pattern`, `skill` or `subagent_type` could therefore repoint the field it named, last-wins, to whatever quoted string happened to follow it in the payload -- byte-identical to a genuine non-match, so a blocked call ran silently. `jit_hook_fields()` (`common.sh`) replaces the positional read with a structural one: a string is a key only when the next raw piece begins with `:`, and a value is read only at the correct depth -- the payload's own top level for `tool_name`, directly inside `tool_input` and nowhere deeper for everything else -- first-wins throughout. diff --git a/changelog.d/427.security.md b/changelog.d/427.security.md new file mode 100644 index 0000000..f90b483 --- /dev/null +++ b/changelog.d/427.security.md @@ -0,0 +1 @@ +- **`JIT_MISSING_REQUIRES` was the one exec-crossing list in `common.sh` with no byte cap, so an oversized `requires:` value in a committed `00-index.tsv` could trip `E2BIG` and refuse -- or, on an unwritable `TMPDIR`, silently pass -- every tool call in a session** (#427). Every sibling list that crosses the same `pre-tool-hook.sh` exec boundary (`JIT_SYMLINKS`, `JIT_NONFILES`, `JIT_CONFIG_REFUSED`, `JIT_LAYERS_REFUSED`, `JIT_ENTRY_AGES`) was already capped; this one was not. `jit_missing_requires()` now refuses any `requires:` value that is not a bare binary name (`^[A-Za-z0-9._+-]{1,255}$`) before it is ever added to the list, and bounds the accumulated list to a few KB with a sentinel that reports the truncation rather than silently dropping it -- the same discipline `rebuild-tsv.sh` now also applies at index time, refusing to index a row whose `requires:` value does not match. diff --git a/scripts/common.sh b/scripts/common.sh index bef2913..8ccf317 100644 --- a/scripts/common.sh +++ b/scripts/common.sh @@ -1461,6 +1461,23 @@ jit_frontmatter() { # shellcheck disable=SC2034 JIT_VALID_MODE_RE='^(remind|block|once)(,(remind|block|once))*$' +# A requires: value is free text out of a committed file, and jit_missing_requires() +# below hands the accumulated list across an exec boundary as a single awk -v argument +# (#427). A bare binary name is the only thing that column means (#203s own comment: a +# single name, never a list), so anything else is refused outright rather than carried +# forward at all -- the same discipline JIT_VALID_MODE_RE already applies to mode:, and +# for the same two reasons: an unbounded or hostile value should be looked at, not +# quietly indexed, and 255 bytes is generous for a real binary name while still bounding +# what one row can contribute to a list that many rows share. +# Shared between rebuild-tsv.sh, which refuses to index a tools row whose requires: +# value does not match this, and jit_missing_requires(), which refuses to carry a +# value that does not match this forward even out of an already-committed index -- +# the index-time refusal only protects a FUTURE rebuild by this repository own +# maintainer; a clone reads whatever is already committed. +# Consumed by rebuild-tsv.sh; shellcheck cannot see that from here. +# shellcheck disable=SC2034 +JIT_VALID_REQUIRES_RE='^[A-Za-z0-9._+-]{1,255}$' + # --- Invocation macros ------------------------------------------------------- # A rule that has to fire on an INVOCATION rather than on a word carries an anchor, and # the anchor is the part nobody can verify by reading. Four have been wrong: the \n @@ -2767,6 +2784,77 @@ function jit_json_fields(s, raw, fs, fe, n, i, k) { fe[k] = n return k } +# jit_hook_fields() walks the same logical fields jit_json_fields() produced, but +# structurally rather than positionally (#426). The old dispatch loops in +# pre-tool-hook.sh/pre-path-hook.sh/post-tool-hook.sh treated EVERY single-piece quoted +# field at an even logical index as a candidate key and read whatever quoted field +# followed two positions later as its value -- no check that the field actually sat at +# key position, no check of which object it was inside. A tool_input STRING VALUE equal +# to tool_name, command, file_path, pattern, skill or subagent_type could therefore +# repoint the field it named, last-wins, at whatever quoted string happened to follow -- +# including tool_use_id, defeating every mode: block/require/forbid rule. +# +# Two structural checks close that. A string is a key only when the raw piece right +# after its closing quote begins, after optional whitespace, with a colon -- +# jit_stop_hook_active() below already relies on exactly this check for +# stop_hook_active. And brace depth is tracked over the STRUCTURAL (odd-logical-index) +# fields, which are always ONE physical raw piece -- only quoted content can span an +# escaped quote, so only even indices ever do -- never over quoted content itself. TOP +# is populated from keys read at depth 1, the top level of the whole payload; TI is +# populated from keys read directly inside the top-level tool_input object and nowhere +# deeper. A tool_input value that merely spells a wanted key name is read at the wrong +# depth, is not followed by a colon, or both -- it is never assigned to TOP or TI. +# +# top_wanted/ti_wanted are caller-built membership arrays (name -> 1); only names +# present there are ever looked up. First occurrence wins for every field, the same +# shape jit_session_key() below already uses, and for the same reason given there: the +# runner-written value should never lose to a string an untrusted tool_input carries +# later in the payload. +function jit_hook_fields(raw, fs, fe, n, top_wanted, ti_wanted, TOP, TI, depth, ti_depth, pending_key, i, c, ch, txt, val, nxt, is_key) { + depth = 0 + ti_depth = -1 + pending_key = "" + for (i = 1; i <= n; i++) { + if (i % 2 == 1) { + txt = raw[fs[i]] + for (c = 1; c <= length(txt); c++) { + ch = substr(txt, c, 1) + if (ch == "{") { + depth++ + if (pending_key == "tool_input" && ti_depth == -1) ti_depth = depth + } else if (ch == "}") { + if (depth == ti_depth) ti_depth = -1 + depth-- + } + } + continue + } + # A field spanning several raw pieces -- an escaped quote inside it -- is never a + # bare key name this loop wants and can never BE the pending key either -- the same + # single-piece guard every dispatch loop in this file already used. + if (fs[i] != fe[i]) { pending_key = ""; continue } + val = raw[fs[i]] + is_key = 0 + if (i + 1 <= n) { + nxt = raw[fs[i+1]] + if (nxt ~ /^[[:space:]]*:/) is_key = 1 + } + if (!is_key) { pending_key = ""; continue } + pending_key = val + # The VALUE field i+2 may itself span several raw pieces -- a command carrying an + # escaped quote, or a Write payload own file body -- and jit_field() already + # reassembles a RANGE, so it is read over the full [fs[i+2], fe[i+2]] range rather + # than requiring it be single-piece too. Only the KEY (field i, checked above) has + # to be one bare piece; a spoofed key candidate that itself spans an escaped quote + # was already rejected by that same guard before reaching this point. + if (i + 2 > n) continue + if (depth == 1) { + if ((val in top_wanted) && !(val in TOP)) TOP[val] = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) + } else if (depth == ti_depth) { + if ((val in ti_wanted) && !(val in TI)) TI[val] = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) + } + } +} # --- Session identity, for the once-per-session markers --------------------- # Read here rather than in bash because the payload is already being parsed: a second awk # process per hook to fetch one field would cost more than every check in this file. @@ -3758,18 +3846,58 @@ jit_report_keyword() { # `--`, not a bare name, on the presence check: a requires: value is free text out of a # committed file, and a value starting with a hyphen must not be read as an OPTION to the # `command` builtin itself. +# #427: this list crosses an exec boundary in pre-tool-hook.sh -- handed to awk as a +# single -v missing_bins=... argument -- and it is the one such list in this file that +# was not byte-capped: JIT_SYMLINKS, JIT_NONFILES, JIT_CONFIG_REFUSED, +# JIT_LAYERS_REFUSED and JIT_ENTRY_AGES all cap themselves for exactly this reason. The +# source is a committed 00-index.tsv, so its size is chosen by whatever tree is cloned, +# not by this machine: a requires: value a few hundred KB long, or a few hundred rows +# each naming a distinct one, pushes the composed awk program past MAX_ARG_STRLEN / +# ARG_MAX, execve fails, and pre-tool-hook.sh refuses -- or on an unwritable TMPDIR, +# silently passes -- every call in the session, not just the row that named the +# oversized value. +JIT_MISSING_REQUIRES_MAX=4096 jit_missing_requires() { # $1 tools dimension base directory, $2 space-separated layer names (JIT_TOOL_LAYERS) - local base="$1" layers="$2" layer tsv bin seen=" " missing=" " + local base="$1" layers="$2" layer tsv bin seen=" " missing=" " cut=0 local LC_ALL=C for layer in $layers; do tsv="$base/$layer/00-index.tsv" [ -f "$tsv" ] || continue while IFS= read -r bin; do [ -z "$bin" ] && continue + # A bare binary name only -- JIT_VALID_REQUIRES_RE, the same discipline + # rebuild-tsv.sh applies at index time. The committed index may already carry a + # value that predates that check, or one from a tree this repository never + # indexed at all, so this is the check that actually protects a clone: refused + # here means never added to the seen list, never counted toward the cap below, + # and never handed to command -v as an argument. + case "$bin" in + *[!A-Za-z0-9._+-]*) continue ;; + esac + [ "${#bin}" -gt 255 ] && continue case "$seen" in *" $bin "*) continue ;; esac seen="$seen$bin " command -v -- "$bin" > /dev/null 2>&1 && continue + # The COUNT is not capped, only the list -- JIT_CONFIG_REFUSED's own reason: a + # truncated list that also under-reported would be this repository own defect + # class wearing a fix as a disguise. A binary dropped by the cap is simply never + # added to $missing, so the tools row that names it stops being treated as + # conditionally-bypassable (#203) and goes back to being enforced outright -- + # the fail-closed direction, not fail-open. + if [ "${#missing}" -gt "$JIT_MISSING_REQUIRES_MAX" ]; then + if [ "$cut" = 0 ]; then + cut=1 + # ONE token, no interior space: $missing is membership-tested downstream as + # " NAME " substrings (pre-tool-hook.sh), so a multi-word note would plant a + # plain word -- "cap", say -- as a false hit for any row that genuinely names + # a binary spelled the same. Brackets and colons are outside + # JIT_VALID_REQUIRES_RE, so no legitimate requires: value can ever equal this + # token outright either. + missing="${missing}[JIT-427:list-truncated-at-cap] " + fi + continue + fi missing="$missing$bin " done < <(LC_ALL=C awk -F "$(printf '\t')" '{ print (NF >= 7) ? $7 : "" }' "$tsv") done diff --git a/scripts/post-tool-hook.sh b/scripts/post-tool-hook.sh index ef28a1f..335a78f 100755 --- a/scripts/post-tool-hook.sh +++ b/scripts/post-tool-hook.sh @@ -76,14 +76,20 @@ PT_PARSED="$(LC_ALL=C awk "$JIT_AWK_JSON"' END { n = jit_json_fields(input, raw, fs, fe) tool = ""; fp = ""; cmd = "" - for (i = 2; i + 2 <= n; i += 2) { - if (fs[i] != fe[i]) continue - k = raw[fs[i]] - if (k == "tool_name") tool = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - else if (k == "file_path" && fp == "") fp = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - else if (k == "path" && fp == "") fp = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - else if (k == "command" && cmd == "") cmd = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - } + # #426: jit_hook_fields() (common.sh) reads structurally rather than positionally, so + # a tool_input STRING VALUE equal to one of these names can never repoint the field it + # names -- it is read at the wrong depth, is not followed by a colon, or both. + # tool_name is read at the payload own top level; file_path/path/command only + # directly inside tool_input, never deeper, never from the top level itself. + top_wanted["tool_name"] = 1 + ti_wanted["file_path"] = 1 + ti_wanted["path"] = 1 + ti_wanted["command"] = 1 + jit_hook_fields(raw, fs, fe, n, top_wanted, ti_wanted, TOP, TI) + tool = TOP["tool_name"] + fp = TI["file_path"] + if (fp == "") fp = TI["path"] + cmd = TI["command"] if (tool !~ /^[A-Za-z_]+$/ || length(tool) > 32) tool = "" # A Bash payload carries no file_path at all -- its free-text subject is `command`. # Folded into the same third printed line as file_path/path rather than adding a diff --git a/scripts/pre-path-hook.sh b/scripts/pre-path-hook.sh index 8146c2c..979d5f3 100755 --- a/scripts/pre-path-hook.sh +++ b/scripts/pre-path-hook.sh @@ -207,15 +207,17 @@ END { # one naming convention, not two. bytes_shown_file = jit_shown_file(state_dir, "bytes", raw, fs, fe, n) cmd = "" - for (i = 2; i + 2 <= n; i += 2) { - # A key this hook wants is quote-free, so a field spanning several raw pieces is not - # one; skipping it is what keeps a Write payload body from ever being reassembled. - if (fs[i] != fe[i]) continue - k = raw[fs[i]] - if (k == "file_path") file_path = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - else if (k == "path" && file_path == "") file_path = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - else if (k == "command") cmd = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - } + # #426: jit_hook_fields() (common.sh) reads structurally rather than positionally, so + # a tool_input STRING VALUE equal to file_path/path/command can never repoint the + # field it names -- it is read at the wrong depth, is not followed by a colon, or + # both. All three are read only directly inside tool_input, never from the top level. + ti_wanted["file_path"] = 1 + ti_wanted["path"] = 1 + ti_wanted["command"] = 1 + jit_hook_fields(raw, fs, fe, n, top_wanted, ti_wanted, TOP, TI) + file_path = TI["file_path"] + if (file_path == "") file_path = TI["path"] + cmd = TI["command"] # --- Collect paths to match against --- path_count = 0 diff --git a/scripts/pre-tool-hook.sh b/scripts/pre-tool-hook.sh index 38ce712..3dbe387 100755 --- a/scripts/pre-tool-hook.sh +++ b/scripts/pre-tool-hook.sh @@ -213,39 +213,43 @@ END { # still written on every delivery, so #389s own Stop-hook accounting keeps reading # exactly the file it always has. agent_shown_file = jit_agent_shown_file(state_dir, "vocab", raw, fs, fe, n) - for (i = 2; i + 2 <= n; i += 2) { - # Only a field that is ONE raw piece can be a key this hook wants — every key below is - # quote-free — and only the matching value is ever materialised or decoded. That is - # what keeps a Write payload, whose tool_input.content is the whole file body, from - # being reassembled and walked character by character on every single tool call. - if (fs[i] != fe[i]) continue - k = raw[fs[i]] - if (k == "tool_name") tool_name = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - else if (k == "command") command = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - else if (k == "skill") f_skill = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - else if (k == "file_path") f_file_path = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - else if (k == "pattern") f_pattern = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - # #182. An Agent dispatch carries description, prompt and subagent_type and none of - # the four above, so `cmd` came out empty and this hook printed {} and exited 59 - # lines before the layer loop. A `tool: Agent` rule -- including a `mode: block` one - # -- was written, validated, indexed, counted by every diagnostic, and inert. - # - # subagent_type ONLY, and the other two are a deliberate no. `prompt` and - # `description` are author-written prose, and two things go wrong with prose as a - # subject. It is matched by `forbid`/`require`/substring rules that were written - # about COMMANDS, so a prompt saying "do not run git push here" trips a deny-list - # rule about `git push`. And `cmd` is cut at the first ; & | or double quote (see - # the strip below), so a prose subject is compared as an arbitrary prefix of itself - # -- the #7 false-block shape, rebuilt. - # - # The cost is real too, though it is the weaker half of the argument. Measured on a - # two-rule tools index, 40 calls per point, interleaved, one-true-awk 20200816 on - # darwin 24.3.0, read out of the hook OWN timing in hooks.log rather than wall clock - # around the process: a 7-byte subject 91 ms median, a 4.4 KB one 97 ms, a 44 KB one - # 207 ms. A prompt is routinely in the second band and can reach the third. - # subagent_type is a bounded identifier and is always in the first. - else if (k == "subagent_type") f_subagent = jit_unescape(jit_field(raw, fs[i+2], fe[i+2])) - } + # #426: jit_hook_fields() (common.sh) reads structurally rather than positionally, so + # a tool_input STRING VALUE equal to one of these names can never repoint the field it + # names -- it is read at the wrong depth, is not followed by a colon, or both. + # tool_name is read at the payload own top level; everything else only directly + # inside tool_input, never deeper, never from the top level itself. + top_wanted["tool_name"] = 1 + ti_wanted["command"] = 1 + ti_wanted["skill"] = 1 + ti_wanted["file_path"] = 1 + ti_wanted["pattern"] = 1 + # #182. An Agent dispatch carries description, prompt and subagent_type and none of + # the four above, so `cmd` came out empty and this hook printed {} and exited 59 + # lines before the layer loop. A `tool: Agent` rule -- including a `mode: block` one + # -- was written, validated, indexed, counted by every diagnostic, and inert. + # + # subagent_type ONLY, and the other two are a deliberate no. `prompt` and + # `description` are author-written prose, and two things go wrong with prose as a + # subject. It is matched by `forbid`/`require`/substring rules that were written + # about COMMANDS, so a prompt saying "do not run git push here" trips a deny-list + # rule about `git push`. And `cmd` is cut at the first ; & | or double quote (see + # the strip below), so a prose subject is compared as an arbitrary prefix of itself + # -- the #7 false-block shape, rebuilt. + # + # The cost is real too, though it is the weaker half of the argument. Measured on a + # two-rule tools index, 40 calls per point, interleaved, one-true-awk 20200816 on + # darwin 24.3.0, read out of the hook OWN timing in hooks.log rather than wall clock + # around the process: a 7-byte subject 91 ms median, a 4.4 KB one 97 ms, a 44 KB one + # 207 ms. A prompt is routinely in the second band and can reach the third. + # subagent_type is a bounded identifier and is always in the first. + ti_wanted["subagent_type"] = 1 + jit_hook_fields(raw, fs, fe, n, top_wanted, ti_wanted, TOP, TI) + tool_name = TOP["tool_name"] + command = TI["command"] + f_skill = TI["skill"] + f_file_path = TI["file_path"] + f_pattern = TI["pattern"] + f_subagent = TI["subagent_type"] # Fallback chain for tool matching full_command = command diff --git a/scripts/rebuild-tsv.sh b/scripts/rebuild-tsv.sh index ad59034..8b1d571 100755 --- a/scripts/rebuild-tsv.sh +++ b/scripts/rebuild-tsv.sh @@ -492,6 +492,19 @@ build_tool_tsv() { continue fi + # #427: requires: is the one field this row can force ACROSS A DIFFERENT EXEC + # BOUNDARY, at fire time, in every OTHER tools row this tree indexes -- pre-tool-hook.sh + # collects the column from every row into one list and hands it to awk as a single + # -v argument. An oversized or malformed value here does not just misfire on ITS OWN + # row the way a bad mode: does; it can push that shared list past ARG_MAX and refuse + # every tool call in a session. Refused here rather than indexed, same shape as the + # mode: check above -- JIT_VALID_REQUIRES_RE (common.sh). + if [ -n "$requires" ] && ! printf '%s' "$requires" | LC_ALL=C grep -Eq "$JIT_VALID_REQUIRES_RE"; then + jit_unindexed "$label" "$filename" "requires: \"$(jit_report_keyword "$requires")\" is not a bare binary name -- entry skipped rather than indexed with an unverified requires:" + jit_rc 1 + continue + fi + if [ -z "$tool" ] || [ -z "$match" ]; then # Not `[ -z x ] || [ -z y ] && continue`: that is one AND-OR list evaluated left to # right, so the `&&` binds to the second test alone. It happened to behave here, and diff --git a/tests/test-pre-tool-hook.sh b/tests/test-pre-tool-hook.sh index 17ff3ab..9270246 100755 --- a/tests/test-pre-tool-hook.sh +++ b/tests/test-pre-tool-hook.sh @@ -1304,6 +1304,55 @@ else rm -rf "$D424" fi +# --- #426: a tool_input STRING VALUE cannot pose as a JSON key ------------- +# The old dispatch loop treated every single-piece quoted field at an even logical +# index as a candidate key, positionally -- no check that it sat at key position, no +# check of which object it was inside. A tool_input value equal to "tool_name" or +# "command" could repoint the field it named, last-wins, at whatever quoted string +# followed -- defeating a block/require/forbid rule byte-identically to a genuine +# non-match. Red on main before jit_hook_fields() (common.sh). +echo "" +echo "=== #426: tool_input value cannot repoint tool_name/command ===" +D426=$(mktemp -d) +T426="$D426/.claude/jit-context/tools/00-manual" +V426="$D426/.claude/jit-context/vocabulary" +mkdir -p "$T426" "$V426/00-manual" "$V426/10-auto" "$V426/20-grouped" "$V426/30-crosscutting" +IDX426NAME="00-index" +IDX426NAME="$IDX426NAME.tsv" +for l in 00-manual 10-auto 20-grouped 30-crosscutting; do : > "$V426/$l/$IDX426NAME"; done +printf 'Bash\tgit push\tblkpush.md\tblock\t\t\n' > "$T426/$IDX426NAME" +echo "blocked: git push" > "$T426/blkpush.md" +printf 'Agent\t~.*\tblkagent.md\tblock\t\t\n' >> "$T426/$IDX426NAME" +echo "blocked: agent" > "$T426/blkagent.md" + +run426() { + echo "$1" | CLAUDE_PROJECT_DIR="$D426" bash "$HOOK" 2> /dev/null +} + +# tool_input.description equal to "tool_name", followed by tool_use_id -- the shape +# Claude Code itself serializes after tool_input -- must not unname the Bash tool. +OUT426A=$(run426 '{"session_id":"s426a","tool_name":"Bash","tool_input":{"command":"git push origin main","description":"tool_name"},"tool_use_id":"toolu_01"}') +assert_blocked "#426 tool_input.description=tool_name cannot unname the Bash tool" "$OUT426A" + +# tool_input.description equal to "command" must not repoint the matched subject away +# from the real command. +OUT426B=$(run426 '{"session_id":"s426b","tool_name":"Bash","tool_input":{"command":"git push origin main","description":"command"},"tool_use_id":"toolu_01"}') +assert_blocked "#426 tool_input.description=command cannot repoint the matched subject" "$OUT426B" + +# Agent: a prompt equal to "tool_name" needs no trailing field at all -- the old bug +# read the NEXT quoted field, subagent_type's own KEY NAME (not its value), into +# tool_name, turning tool_name into the literal string "subagent_type" and defeating +# any tool: Agent rule. +OUT426C=$(run426 '{"session_id":"s426c","tool_name":"Agent","tool_input":{"prompt":"tool_name","subagent_type":"oss:developer"}}') +assert_blocked "#426 Agent tool_input.prompt=tool_name cannot unname the Agent tool" "$OUT426C" + +# Positive control: the same fixture still passes an ordinary, unspoofed call through -- +# #426 must not become a rule that blocks everything regardless of subject. +OUT426D=$(run426 '{"session_id":"s426d","tool_name":"Bash","tool_input":{"command":"echo hello"}}') +assert_not_contains "#426 control: an ordinary command is not blocked" "$OUT426D" '"decision":"block"' + +rm -rf "$D426" + # --- Cleanup --- rm -rf "$TEST_DIR" diff --git a/tests/test-requires-field.sh b/tests/test-requires-field.sh index 90983ec..4901815 100755 --- a/tests/test-requires-field.sh +++ b/tests/test-requires-field.sh @@ -220,6 +220,116 @@ assert_not_contains "no STALE report for the row this suite just rebuilt" "$DRYR rm -f "$TOOLS_DIR/rebuild-deploy.md" +# ============================================= +# SECTION 5 (#427): rebuild-tsv.sh refuses to index a requires: value that is not a +# bare binary name -- the value crosses an exec boundary at fire time (pre-tool-hook.sh +# hands the WHOLE accumulated list to awk as one -v argument), so a malformed or +# oversized value on ONE row is not this row's own problem alone. +# ============================================= +echo "" +echo "=== rebuild-tsv.sh refuses requires: with an embedded space (#427) ===" +printf '%s\n' \ + "---" \ + "title: Bad requires space" \ + "description: A requires: value that is not a bare binary name." \ + "tool: Bash" \ + "match: git bad-requires-space" \ + "mode: block" \ + "requires: not a binary name" \ + "---" \ + "" \ + "BAD-REQUIRES-SPACE-BODY-MARKER" > "$TOOLS_DIR/bad-requires-space.md" +# The refusal is expected here -- rebuild-tsv.sh exits non-zero when it skips a row -- +# so `|| true` is load-bearing under `set -e`, not decoration. +CLAUDE_PROJECT_DIR="$TEST_DIR" bash "$REBUILD" > /dev/null 2>&1 || true +ROW=$(grep -F "bad-requires-space.md" "$TOOLS_DIR/$TSV_NAME" || true) +assert_not_contains "the malformed row is not indexed at all" "$ROW" "bad-requires-space.md" +rm -f "$TOOLS_DIR/bad-requires-space.md" + +echo "" +echo "=== rebuild-tsv.sh refuses requires: longer than 255 bytes (#427) ===" +LONGREQ=$(head -c 300 /dev/zero | tr '\0' 'a') +printf '%s\n' \ + "---" \ + "title: Bad requires length" \ + "description: A requires: value past the bare-name length bound." \ + "tool: Bash" \ + "match: git bad-requires-length" \ + "mode: block" \ + "requires: $LONGREQ" \ + "---" \ + "" \ + "BAD-REQUIRES-LENGTH-BODY-MARKER" > "$TOOLS_DIR/bad-requires-length.md" +CLAUDE_PROJECT_DIR="$TEST_DIR" bash "$REBUILD" > /dev/null 2>&1 || true +ROW=$(grep -F "bad-requires-length.md" "$TOOLS_DIR/$TSV_NAME" || true) +assert_not_contains "the oversized row is not indexed at all" "$ROW" "bad-requires-length.md" +rm -f "$TOOLS_DIR/bad-requires-length.md" + +# ============================================= +# SECTION 6 (#427): jit_missing_requires() bounds its OWN output regardless of how +# large the committed index already is -- this is the check that protects a CLONE, not +# just a future rebuild by this repository own maintainer, and it has to hold even for +# an index nothing above ever validated. Built directly, bypassing rebuild-tsv.sh +# entirely, the same way a tree cloned before this fix would already be malformed on +# disk. +# ============================================= +echo "" +echo "=== jit_missing_requires() caps its own output regardless of input size (#427) ===" +CAP_DIR=$(mktemp -d) +CAP_TOOLS_BASE="$CAP_DIR/tools" +CAP_TOOLS="$CAP_TOOLS_BASE/00-manual" +mkdir -p "$CAP_TOOLS" +# 80 distinct, syntactically valid but never-resolving binary names, each padded to +# ~120 bytes -- about 9.6 KB total, well past a few-KB cap, built entirely of rows a +# bare-name check alone would accept, so this measures the CAP, not the validator. +{ + n=0 + while [ "$n" -lt 80 ]; do + pad=$(head -c 100 /dev/zero | tr '\0' 'x') + printf 'Bash\tirrelevant%d\tirrelevant%d.md\tremind\t\t\tnosuchbin%d%s\n' "$n" "$n" "$n" "$pad" + n=$((n + 1)) + done +} > "$CAP_TOOLS/$TSV_NAME" +CAP_OUT=$( + cd "$SCRIPT_DIR" || exit 1 + # shellcheck source=scripts/common.sh + source scripts/common.sh + jit_missing_requires "$CAP_TOOLS_BASE" "00-manual" +) +CAP_LEN=${#CAP_OUT} +if [ "$CAP_LEN" -le 4300 ]; then + PASS=$((PASS + 1)) + echo " PASS: the accumulated list stays bounded ($CAP_LEN bytes) no matter how many rows feed it" +else + FAIL=$((FAIL + 1)) + echo " FAIL: the accumulated list stays bounded no matter how many rows feed it" + echo " got $CAP_LEN bytes" +fi +assert_contains "and says plainly that it truncated" "$CAP_OUT" "JIT-427" +rm -rf "$CAP_DIR" + +echo "" +echo "=== a call unrelated to any requires: row still answers, even with a huge committed index (#427) ===" +BIG_DIR=$(mktemp -d) +BIG_TOOLS="$BIG_DIR/.claude/jit-context/tools/00-manual" +BIG_VOCAB="$BIG_DIR/.claude/jit-context/vocabulary" +mkdir -p "$BIG_TOOLS" "$BIG_VOCAB/00-manual" +touch "$BIG_VOCAB/00-manual/$TSV_NAME" +{ + n=0 + while [ "$n" -lt 80 ]; do + pad=$(head -c 100 /dev/zero | tr '\0' 'x') + printf 'Bash\tirrelevant%d\tirrelevant%d.md\tremind\t\t\tnosuchbin%d%s\n' "$n" "$n" "$n" "$pad" + n=$((n + 1)) + done + printf 'Bash\tgit ordinary-call\tordinary.md\tblock\t\t\t\n' +} > "$BIG_TOOLS/$TSV_NAME" +echo "ORDINARY-CALL-BODY-MARKER" > "$BIG_TOOLS/ordinary.md" +BIG_OUT=$(printf '%s' '{"tool_name":"Bash","tool_input":{"command":"git ordinary-call now"}}' | CLAUDE_PROJECT_DIR="$BIG_DIR" bash "$HOOK" 2> /dev/null) +assert_blocked "the hook still answers -- a real decision, not a crash envelope" "$BIG_OUT" +assert_contains "and the row unrelated to any requires: value still fires" "$BIG_OUT" "ORDINARY-CALL-BODY-MARKER" +rm -rf "$BIG_DIR" + echo "" echo "========================" TOTAL=$((PASS + FAIL)) From 260902c9531defdbb8380262e79bca11a49413a6 Mon Sep 17 00:00:00 2001 From: Florian DAVID Date: Sat, 19 Sep 2026 14:26:15 +0200 Subject: [PATCH 2/2] fix: a nested key merely spelled tool_input could hijack jit_hook_fields' real one Self-review finding on #426: jit_hook_fields() (common.sh) locked ti_depth onto the FIRST key anywhere in the payload spelled "tool_input", regardless of what depth that key itself sat at -- unlike the top-level TOP path, which already hardcodes depth == 1. A JSON payload carrying an earlier, unrelated top-level object with its own nested key coincidentally spelled "tool_input" would hijack every subsequent TI[...] read for the rest of the object it wraps, silently discarding the real top-level tool_input's own fields under first-wins. Fixed by tracking the depth a candidate key was itself read at (pending_key_depth) and only accepting it to set ti_depth when that key sat at depth 1 -- the genuine top level, mirroring TOP's own hardcoded check. Not known to be triggerable against Claude Code's actual PreToolUse payload shape today (every field preceding the real tool_input key is runner-written), but the code did not enforce that assumption, and the new-code comments and changelog fragment both claimed a guarantee this did not actually provide. Co-Authored-By: Max --- scripts/common.sh | 17 +++++++++++++---- tests/test-pre-tool-hook.sh | 8 ++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/scripts/common.sh b/scripts/common.sh index 8ccf317..96c4959 100644 --- a/scripts/common.sh +++ b/scripts/common.sh @@ -2810,10 +2810,11 @@ function jit_json_fields(s, raw, fs, fe, n, i, k) { # shape jit_session_key() below already uses, and for the same reason given there: the # runner-written value should never lose to a string an untrusted tool_input carries # later in the payload. -function jit_hook_fields(raw, fs, fe, n, top_wanted, ti_wanted, TOP, TI, depth, ti_depth, pending_key, i, c, ch, txt, val, nxt, is_key) { +function jit_hook_fields(raw, fs, fe, n, top_wanted, ti_wanted, TOP, TI, depth, ti_depth, pending_key, pending_key_depth, i, c, ch, txt, val, nxt, is_key) { depth = 0 ti_depth = -1 pending_key = "" + pending_key_depth = -1 for (i = 1; i <= n; i++) { if (i % 2 == 1) { txt = raw[fs[i]] @@ -2821,7 +2822,14 @@ function jit_hook_fields(raw, fs, fe, n, top_wanted, ti_wanted, TOP, TI, depth ch = substr(txt, c, 1) if (ch == "{") { depth++ - if (pending_key == "tool_input" && ti_depth == -1) ti_depth = depth + # #426 self-review finding: pending_key alone names WHICH key precedes this + # brace, not WHERE that key itself sat. Without pending_key_depth == 1 here, + # any earlier key spelled "tool_input" at ANY depth -- nested three objects + # deep, say -- would lock ti_depth onto ITS value object, and first-wins would + # then silently discard the real top-level tool_input for every name the + # impostor also claims. Only a "tool_input" key read while depth was still 1 + # (before this open brace bumps it) is the genuine top-level one. + if (pending_key == "tool_input" && pending_key_depth == 1 && ti_depth == -1) ti_depth = depth } else if (ch == "}") { if (depth == ti_depth) ti_depth = -1 depth-- @@ -2832,15 +2840,16 @@ function jit_hook_fields(raw, fs, fe, n, top_wanted, ti_wanted, TOP, TI, depth # A field spanning several raw pieces -- an escaped quote inside it -- is never a # bare key name this loop wants and can never BE the pending key either -- the same # single-piece guard every dispatch loop in this file already used. - if (fs[i] != fe[i]) { pending_key = ""; continue } + if (fs[i] != fe[i]) { pending_key = ""; pending_key_depth = -1; continue } val = raw[fs[i]] is_key = 0 if (i + 1 <= n) { nxt = raw[fs[i+1]] if (nxt ~ /^[[:space:]]*:/) is_key = 1 } - if (!is_key) { pending_key = ""; continue } + if (!is_key) { pending_key = ""; pending_key_depth = -1; continue } pending_key = val + pending_key_depth = depth # The VALUE field i+2 may itself span several raw pieces -- a command carrying an # escaped quote, or a Write payload own file body -- and jit_field() already # reassembles a RANGE, so it is read over the full [fs[i+2], fe[i+2]] range rather diff --git a/tests/test-pre-tool-hook.sh b/tests/test-pre-tool-hook.sh index 9270246..81d164a 100755 --- a/tests/test-pre-tool-hook.sh +++ b/tests/test-pre-tool-hook.sh @@ -1351,6 +1351,14 @@ assert_blocked "#426 Agent tool_input.prompt=tool_name cannot unname the Agent t OUT426D=$(run426 '{"session_id":"s426d","tool_name":"Bash","tool_input":{"command":"echo hello"}}') assert_not_contains "#426 control: an ordinary command is not blocked" "$OUT426D" '"decision":"block"' +# Self-review finding on #426: a NESTED key merely spelled "tool_input" must not +# hijack ti_depth away from the REAL top-level tool_input object. ti_depth may only +# lock onto a "tool_input" key that was itself read at depth 1 -- an earlier decoy +# nested inside some other top-level object must never claim it, or first-wins would +# silently prefer the decoy's own command/file_path/etc. fields over the real ones. +OUT426E=$(run426 '{"session_id":"s426e","tool_name":"Bash","weird":{"tool_input":{"command":"echo nothing-to-see"}},"tool_input":{"command":"git push origin main --force"}}') +assert_blocked "#426 a nested key spelled tool_input cannot hijack the real one (self-review)" "$OUT426E" + rm -rf "$D426" # --- Cleanup ---