diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index eed2789883..261e85c9bd 100644 --- a/plugins/guardrails/.claude-plugin/plugin.json +++ b/plugins/guardrails/.claude-plugin/plugin.json @@ -147,5 +147,5 @@ "min": 1 } }, - "version": "0.32.17" + "version": "0.32.18" } diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index 6a3884c152..61d960f3a2 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,6 +3,45 @@ 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.32.18] + +### Changed + +- **`block-hook-bypass` no longer forks for work that had no process in it.** + On a benign Bash call the guard created seven processes and executed none, + so every exec census (the PATH-shim spawn census, `run-guards.test.sh`'s + dirname/sed pin, an xtrace command count) read it as free. Six were the + guard's own: an eager `SUBJECT=$(hook::extract_bash_subject ...)` at file + scope, feeding a telemetry envelope that is off by default; a + `$(strip_literals ...)` around a builtin-only function; and four + `done < <(printf ...)` line loops (the literal strip and the three + per-segment scans). The seventh is `$(hook::buffer_stdin)`. After: the + subject is derived inside `emit_tel`, behind the start-stamp and sink gates; + the strip assigns through a nameref (`strip_literals_to`); and one fork-free + splitter (`split_lines_to`, a sentinel-prefixed IFS split under `set -f`, so + blank lines and runs of newlines arrive exactly as `read` delivered them) + feeds the strip and fills `NORMALIZED_SEGMENTS` once, as an array the three + scans iterate. `return` and `continue 2` inside those loops reach the same + scopes as before, since neither loop shape ran its body in a subshell. + Verdicts are unchanged: 244 paired runs against `origin/main` (61 commands, + Bash and PowerShell payloads, standalone and dispatched, plus 70 KiB + single-line and 3000-line commands) agree on exit code and first stderr + line, and the 611-case contract suite passes. The seventh creation, + `$(hook::buffer_stdin)`, is gone too: #3838 landed the fork-free + `hook::buffer_stdin_to` in `lib/hook-utils.sh` and this guard now calls it, + so the guard's own share on a benign Bash call is zero processes. + Kernel census with `strace -f -e trace=clone,clone3,fork,vfork,execve` on + the dispatched path (`run-guards.sh block-hook-bypass.sh`, this repository + as cwd, `HOOK_TELEMETRY_SINK` unset), guard share = count minus a no-op + guard dispatched the same way, three identical repeats: benign + `git status --short` creations **7 -> 0**, execve **0 -> 0**; blocked + `echo hi > notes.md` creations **10 -> 4**, execve **1 -> 1**. Whole Bash + dispatcher on the benign payload: creations **36 -> 30**, execve **3 -> 3** + (bash plus the two primed `jq`). The unchanged execve column is the + evidence this is latency, not removed work. The contract suite now pins + the guard's benign share at exactly 0 by the same instrument, and skips + visibly where strace is absent. + ## [0.32.17] ### Changed diff --git a/plugins/guardrails/README.md b/plugins/guardrails/README.md index 329c4660f1..af4f527c09 100644 --- a/plugins/guardrails/README.md +++ b/plugins/guardrails/README.md @@ -398,6 +398,50 @@ out of scope until such a signal exists. ### Hook budget accounting +**0.32.18, forks with no exec in `block-hook-bypass`.** 2026-09-06, Linux CI +host. Every earlier row in this section counts execs through a PATH shim, and +a shim cannot see a fork: `$(builtin-only function)`, `< <(printf ...)` and a +pipeline each create a process that never execs. On a benign Bash call this +guard created seven such processes and executed none. Six were in the guard +itself (an eager telemetry subject at file scope, a `$(strip_literals)`, four +process-substitution line loops); the seventh was `$(hook::buffer_stdin)`, +whose fork-free form is `lib/hook-utils.sh` work. That form landed in #3838 +and this guard now calls `hook::buffer_stdin_to`, so all seven are gone and +the guard's own share on a benign Bash call is zero processes. +No verdict changed: 244 paired runs against `origin/main` +(61 commands, Bash and PowerShell payloads, standalone and dispatched, plus +70 KiB single-line and 3000-line commands) agree on exit code and first +stderr line, and the 611-case contract suite passes. + +*Method.* Kernel census, `strace -f -e trace=clone,clone3,fork,vfork,execve`, +on the dispatched path (`run-guards.sh block-hook-bypass.sh`), this repository +as cwd, `HOOK_TELEMETRY_SINK` unset, `CLAUDE_PROJECT_DIR` empty. The guard's +share is the count minus a no-op guard dispatched the same way, which removes +the dispatcher's own stdin, jq and isolation forks. Creations are clone-family +returns; execve is counted separately so an exec cannot pass for a removed +fork. Three repeats, identical each time. Wall clock is p50/p95 of 20 samples +after 2 warmup, sides interleaved, on a host whose `bash -c :` floor is about +1 ms; the milliseconds are context, the durable figure is the process count. + +| Counter | before | after | +|---|---|---| +| Guard share, benign `git status --short`: creations / execve | 7 / 0 | 0 / 0 | +| Guard share, blocked `echo hi > notes.md`: creations / execve | 10 / 1 | 4 / 1 | +| Whole Bash dispatcher, benign: creations / execve | 36 / 3 | 30 / 3 | +| Guard alone under the dispatcher, wall p50 / p95 (n=20) | 27.4 / 29.2 ms | 24.3 / 27.5 ms | +| Whole Bash dispatcher, wall p50 / p95 (n=20) | 51.6 / 60.4 ms | 48.2 / 49.6 ms | + +The execve column does not move, which is what makes this latency rather than +removed work. The contract suite pins the guard's benign share at exactly 0 by +the same instrument, so the figure moves with the code rather than with this +table. + +The two guard-share rows are current after merging #3838. The +whole-dispatcher and wall-clock rows were measured against this branch's +base before #3838 landed, so they still carry the dispatcher's own pre-fusion +cost; the reduction that change made to the dispatcher is recorded in the +0.32.11 row below, not here. + **0.32.17, forks with no exec in `block-noncanonical-commit`.** 2026-09-06, Linux CI host. A PATH shim counts execs, and a fork that never execs is invisible to it. On every Bash and PowerShell call this guard created two diff --git a/plugins/guardrails/hooks/block-hook-bypass.sh b/plugins/guardrails/hooks/block-hook-bypass.sh index 3c21be7fe1..72e15c2376 100755 --- a/plugins/guardrails/hooks/block-hook-bypass.sh +++ b/plugins/guardrails/hooks/block-hook-bypass.sh @@ -141,22 +141,27 @@ TOOL_NAME="${HOOK_JQ_FIELDS[1]:-Bash}" # costs a cache lookup rather than a jq process. HOOK_CWD="${HOOK_JQ_FIELDS[2]:-}" -# Privacy-safe telemetry subject: `Bash:` with leading `sudo` / -# env-assignment prefixes stripped and the token basenamed. Never the full -# command. The shared helper is used rather than a local copy so the aborts that -# keep an assignment VALUE out of the subject — a quoted value spanning the -# whitespace the tokenizer splits on, and a bare/trailing `NAME=value` no -# following command consumed — hold here too (#3372). -SUBJECT=$(hook::extract_bash_subject "$TOOL_NAME" "$COMMAND") - # Emit one telemetry envelope: $1 status, $2 form ("" when not blocked). Gated # on the high-res start stamp and the opt-in sink, so the unwired default path # spawns no telemetry-only subprocess. +# +# The privacy-safe subject (`Bash:` with leading `sudo` / +# env-assignment prefixes stripped and the token basenamed, never the full +# command) is derived HERE, behind both gates, not at file scope. The shared +# helper answers through a command substitution, and that is a fork on every +# fire; only the envelope reads the subject, and the envelope is off by default, +# so deriving it eagerly spent a process on every Bash call for a value nothing +# consumed (#3513). The verdict never reads it. The shared helper is still used +# rather than a local copy so the aborts that keep an assignment VALUE out of +# the subject (a quoted value spanning the whitespace the tokenizer splits on, +# and a bare/trailing `NAME=value` no following command consumed) hold here +# too (#3372). emit_tel() { [[ -n "$start" ]] || return 0 hook::telemetry_enabled || return 0 - local data - hook::json_str_object_to data tool "$TOOL_NAME" subject "$SUBJECT" form "$2" + local data subject + subject=$(hook::extract_bash_subject "$TOOL_NAME" "$COMMAND") + hook::json_str_object_to data tool "$TOOL_NAME" subject "$subject" form "$2" hook::emit_telemetry "block-hook-bypass" "PreToolUse" "$1" "$start" "$data" "${CLAUDE_PROJECT_DIR:-}" } @@ -232,6 +237,37 @@ _keep_char() { esac } +# Split $2 on newlines into the array named by $1, one element per line: the +# lines `while IFS= read -r line; do …; done < <(printf '%s\n' "$2")` delivers, +# in the same order, with the same trailing empty element when $2 ends in a +# newline, and one empty element for an empty $2. Without the process +# substitution: bash forks for `<(…)` on every call (Process Substitution, Bash +# Reference Manual), and this guard split four times per Bash call (#3513). A +# here-string is not the alternative: at 65536-65663 bytes bash blocks forever +# writing it into the pipe (see lib/path-detection/hardcoded-path-patterns.sh). +# +# Word-splitting on IFS= alone would MERGE runs of newlines and drop a +# leading or trailing empty line (newline is IFS whitespace), and strip_literals +# needs every physical line, blank ones included, to keep its heredoc and +# open-quote state aligned with the command's own lines. So each line is first +# prefixed with one sentinel byte, which makes every field non-empty, and the +# prefix is removed again after the split. The prefix is added and removed +# exactly once per line, so the sentinel's own value never matters: a line that +# already begins with it keeps that byte. Globbing is off across the split +# (a `*` in a command must stay a `*`) and restored to what the caller had. +split_lines_to() { + local -n _sl_out="$1" + local _sl_s=$'\x1f' _sl_text _sl_noglob=0 + _sl_text="${_sl_s}${2//$'\n'/$'\n'"$_sl_s"}" + [[ $- == *f* ]] && _sl_noglob=1 + set -f + local IFS=$'\n' + # shellcheck disable=SC2206 # the split IS the point; IFS is newline-only and globbing is off + _sl_out=($_sl_text) + ((_sl_noglob)) || set +f + _sl_out=("${_sl_out[@]#"$_sl_s"}") +} + # Strip single- and double-quoted literal spans so the executable-token scan # sees only shell syntax, not payload text. Heredoc bodies are dropped wholesale # (their content is data, not a command). The quote strip carries an OPEN quote @@ -240,8 +276,17 @@ _keep_char() { # of leaking its tokens from the second line on. An unquoted `#` comment is dropped # to end-of-line without carrying quote state, so an unmatched quote inside a # comment cannot leak a span onto the next line (see the `#` case below). -strip_literals() { - local cmd="$1" line result="" in_heredoc=0 delim="" trimmed +# +# strip_literals_to : the stripped text lands in the variable +# named by $1 rather than on stdout. A `$(strip_literals …)` at the call site +# was one fork per Bash call for a builtin-only function (#3513); the assignment +# through a nameref is none. The value is what the substitution produced: +# every trailing newline removed, as `$(…)` removes them. +strip_literals_to() { + local -n _sl_result="$1" + local cmd="$2" line result="" in_heredoc=0 delim="" trimmed + local -a _bbh_lines=() + split_lines_to _bbh_lines "$cmd" # `open_quote` carries a single- or double-quote span across lines: "" outside # any quote, "'" or '"' inside one that opened on an earlier line. `open_keep` # carries, alongside it, whether that span is a REDIRECT OPERAND (a quoted @@ -260,7 +305,7 @@ strip_literals() { # instead of being swallowed into a bogus `EOF>file` delimiter. local heredoc_start_re='(^|[^<])<<-?[[:space:]]*([^[:space:]<>]+)' - while IFS= read -r line || [[ -n "$line" ]]; do + for line in "${_bbh_lines[@]}"; do if ((in_heredoc)); then # Trim + literal compare, NOT `=~ "$delim"`: inside a bash regex the # delimiter would be treated as a pattern, so a metachar delim (EOF+, @@ -462,11 +507,13 @@ strip_literals() { else result+="${out}"$'\n' fi - done < <(printf '%s\n' "$cmd") # not <<<: a >=64KiB here-string deadlocks (see hardcoded-path-patterns.sh) - printf '%s' "${result%$'\n'}" + done + while [[ "$result" == *$'\n' ]]; do result="${result%$'\n'}"; done + _sl_result="$result" } -EXECUTABLE=$(strip_literals "$COMMAND") +EXECUTABLE="" +strip_literals_to EXECUTABLE "$COMMAND" EXEC_LC="${EXECUTABLE,,}" COMMAND_LC="${COMMAND,,}" @@ -653,6 +700,7 @@ py_write_indicator() { # Segmentation is shared with the `cat >` scan (cat_redirect_bypass) through # normalize_segments, so both lanes agree on escaped separators and on the # fd-duplication sentinel instead of drifting apart behind two splitters. +NORMALIZED_SEGMENTS=() normalize_segments() { local exec_lc="$1" seps=$';\n|&()' soh=$'\x01' esc=$'\x02' normalized s i # Protect backslash-escaped separators (`echo x \; > file`, an escaped-newline @@ -690,7 +738,12 @@ normalize_segments() { # _redir_scan's target class excludes the sentinel as well as `&`. normalized="${normalized//"$soh"/\&}" normalized="${normalized//"$esc"/ }" - NORMALIZED_SEGMENTS="$normalized" + # One segment per element, split once here. The three per-segment scans below + # each used to re-split the same text through a `< <(printf …)` loop, which is + # a fork apiece on every Bash call (#3513); iterating an array is none, and + # `return` / `continue 2` inside the loop bodies reach the same scopes as + # before because neither loop shape runs its body in a subshell. + split_lines_to NORMALIZED_SEGMENTS "$normalized" } # The EFFECTIVE stdout destination of a segment, into LAST_STDOUT_TARGET (empty @@ -1259,7 +1312,7 @@ parse_mv_cp_operands() { # destination that is not scratch-exempt. staged_write_move_bypass() { local seg src dest seen="" prior rest - while IFS= read -r seg || [[ -n "$seg" ]]; do + for seg in "${NORMALIZED_SEGMENTS[@]}"; do [[ -n "${seg//[[:space:]]/}" ]] || continue if parse_mv_cp_operands "$seg"; then dest="$MOVE_DEST" @@ -1293,7 +1346,7 @@ staged_write_move_bypass() { # Discard is never a staging file worth tracking. [[ "$TARGET_TEXT" == "/dev/null" ]] && continue seen+="${TARGET_TEXT}"$'\n' - done < <(printf '%s\n' "$NORMALIZED_SEGMENTS") # not <<<: a >=64KiB here-string deadlocks (see hardcoded-path-patterns.sh) + done return 1 } @@ -1303,7 +1356,7 @@ staged_write_move_bypass() { # cat > real.txt` still blocks on its second segment. cat_redirect_bypass() { local seg - while IFS= read -r seg || [[ -n "$seg" ]]; do + for seg in "${NORMALIZED_SEGMENTS[@]}"; do [[ "$seg" =~ $_cat_redir ]] || continue set_last_stdout_target "$seg" # No FILE operand means no file write: `cat 1>&2` duplicates stdout onto @@ -1318,13 +1371,13 @@ cat_redirect_bypass() { # the EFFECTIVE target, so `cat > /allowed/tmp/f > real.txt` still blocks. scratch_target_exempt "$TARGET_TEXT" && continue return 0 - done < <(printf '%s\n' "$NORMALIZED_SEGMENTS") # not <<<: a >=64KiB here-string deadlocks (see hardcoded-path-patterns.sh) + done return 1 } producer_redirect_bypass() { local seg - while IFS= read -r seg || [[ -n "$seg" ]]; do + for seg in "${NORMALIZED_SEGMENTS[@]}"; do seg="${seg#"${seg%%[![:space:]]*}"}" # Peel leading command-prefix tokens (see _cmd_prefix) and leading redirections # (see _leading_redir) into `head` so a producer hidden behind an env assignment @@ -1391,7 +1444,7 @@ producer_redirect_bypass() { # target, so `echo x > /allowed/tmp/f > real.txt` still blocks. scratch_target_exempt "$TARGET_TEXT" && continue return 0 - done < <(printf '%s\n' "$NORMALIZED_SEGMENTS") # not <<<: a >=64KiB here-string deadlocks (see hardcoded-path-patterns.sh) + done return 1 } diff --git a/plugins/guardrails/hooks/block-hook-bypass.test.sh b/plugins/guardrails/hooks/block-hook-bypass.test.sh index f87bfb3ceb..f215ac31a6 100755 --- a/plugins/guardrails/hooks/block-hook-bypass.test.sh +++ b/plugins/guardrails/hooks/block-hook-bypass.test.sh @@ -2131,4 +2131,57 @@ run_pwsh "PS: double-quoted dash-prefixed path is a literal (blocked — #2906)" # above; the empty-string #2965 pin (`& $py $script ""`) must also keep # passing — an empty span is still deleted, not counted as a literal. +# --- Process creations on the dispatched hot path (#3513) -------------------- +# run-guards.test.sh pins the dispatcher's PATH-visible execs through shims, and +# a shim cannot see a fork: `$(builtin-only function)`, `< <(printf …)` and a +# pipeline each create a process that never execs. On a benign Bash call this +# guard spent seven of those, with zero execs, so every exec census read it as +# free: an eager telemetry subject at file scope, a `$(strip_literals)`, four +# process-substitution line loops, and its `$(hook::buffer_stdin)`. The kernel +# is the instrument that counts them. strace follows the dispatcher's subshells +# (-f) and reports each clone/clone3/fork/vfork return, and execve separately, +# so an exec cannot pass for a removed fork or the reverse. The guard's own +# share is the difference against a no-op guard dispatched through the same +# run-guards.sh on the same payload, which subtracts the dispatcher's own +# stdin, jq and isolation forks. HOOK_TELEMETRY_SINK is cleared: the envelope +# is opt-in and off by default, and it is the one path that still derives the +# subject through a substitution. A host without a working strace (Windows Git +# Bash, macOS) skips visibly; the Linux CI lane is where the pin holds. +# +# The pin is EXACT on purpose. It was 1 while the guard still read stdin +# through `$(hook::buffer_stdin)`; #3838 landed the fork-free +# `hook::buffer_stdin_to` in lib/hook-utils.sh, so that last creation is gone +# and the pin is now 0 — the guard creates no process of its own on a benign +# Bash call. A count that rises is a fork put back on every Bash call. Under -f strace +# may split a call into `` and `<... resumed>` halves, so both +# spellings of a completed call are counted. +strace_census() { # → CENSUS_RC CENSUS_CREATIONS CENSUS_EXECVE + local log="$TEST_TMPDIR/strace.log" + CENSUS_RC=0 + env -u HOOK_TELEMETRY_SINK CLAUDE_PROJECT_DIR= \ + strace -f -e trace=clone,clone3,fork,vfork,execve -o "$log" \ + bash "$HOOK_DIR/run-guards.sh" "$2" <<<"$1" >/dev/null 2>&1 || CENSUS_RC=$? + CENSUS_CREATIONS=$(grep -cE '((clone|clone3|fork|vfork)\(|<\.\.\. (clone|clone3|fork|vfork) resumed>).* = [0-9]+$' "$log") + CENSUS_EXECVE=$(grep -cE '(execve\(|<\.\.\. execve resumed>).* = 0$' "$log") +} +if command -v strace >/dev/null 2>&1 && strace -o /dev/null -e trace=execve true 2>/dev/null; then + printf '#!/usr/bin/env bash\nexit 0\n' >"$TEST_TMPDIR/noop-guard.sh" + for benign in 'git status --short' \ + $'git status --short && git diff --stat | head\nsort f > out; cat README.md'; do + strace_census "$(command_json "$benign")" "$TEST_TMPDIR/noop-guard.sh" + noop_rc=$CENSUS_RC noop_cre=$CENSUS_CREATIONS noop_exe=$CENSUS_EXECVE + strace_census "$(command_json "$benign")" block-hook-bypass.sh + assert_exit "strace: dispatcher with only the no-op guard exits 0 (${benign%%$'\n'*})" 0 "$noop_rc" + assert_exit "strace: dispatched benign command exits 0 (${benign%%$'\n'*})" 0 "$CENSUS_RC" + assert_eq "strace: guard's own process creations on a benign Bash call (${benign%%$'\n'*})" \ + 0 "$((CENSUS_CREATIONS - noop_cre))" + assert_eq "strace: guard's own execve count on a benign Bash call (${benign%%$'\n'*})" \ + 0 "$((CENSUS_EXECVE - noop_exe))" + done + strace_census "$(command_json 'echo hi > notes.md')" block-hook-bypass.sh + assert_exit "strace: dispatched echo > file still blocks under the tracer" 2 "$CENSUS_RC" +else + echo "ok: process-creation pin skipped (no working strace on this host)" +fi + report