Fix positional JSON key confusion in the hook dispatch loops, and cap JIT_MISSING_REQUIRES

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.

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.

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,86 @@ 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, 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]]
+      for (c = 1; c <= length(txt); c++) {
+        ch = substr(txt, c, 1)
+        if (ch == "{") {
+          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--
+        }
+      }
+      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 = ""; 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 = ""; 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
+    # 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 +3855,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

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

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

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