perf(hooks): fuse stdin jq completeness with field extract - #3838
Conversation
GNU Bash forks a subshell for every command substitution even when the body is only builtins. INPUT=$(hook::buffer_stdin) was that fork on every hook fire, and the dispatcher then spent a second jq on the same payload (jq -e . plus jq_fields). hook::buffer_stdin_to captures in-process; optional filters make the completeness check and the prime extract one jq. Spawn census (stable PATH shim, HOOK_TELEMETRY_SINK unset): git status --short 2 jq -> 1 jq; echo hello 2 -> 1; Write 5 -> 4. Isolation $(source) forks are unchanged. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
ShellCheck does not track printf -v through a nameref (wiki SC2154 Exceptions). Initialize dest with var="" so the assignment is visible, matching the official recommended form. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 6m 28s —— View job Code review complete
SummaryScoped to the actual logic changes: One finding posted inline on claude-code-plugins/lib/hook-utils.sh Lines 400 to 401 in f984504 hook::buffer_stdin_to (__hu_dest, claude-code-plugins/lib/hook-utils.sh Line 1529 in f984504 dest as its destination variable name — but a guard that ever does would have its own dest silently left unset while the function still returns 0, i.e. a fail-open with no diagnostic in the always-on dispatcher. Trivial rename fixes it (suggestion included on the comment).
I also traced through the "does the fused Verification claims in the PR description (jq spawn counts, wall-clock deltas, Branch: |
|
Claude finished @kyle-sexton's task in 2m 9s —— View job Security review
Skip gate: did not apply — PR is open, and the change touches a security-relevant surface (the guardrails dispatcher Scope reviewed: Findings: none. Specifically checked and found sound:
This is a pure spawn-count/latency change to hook plumbing; no guard's block/allow decision, fail-open/fail-closed boundary, or trust boundary moved. GitHub Actions files are not part of this diff, so no Actions-hardening findings apply. |
|
Last security-reviewed head: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9845041bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
_to helpers keep internals under a __hu_ prefix so printf -v writes the caller variable, not this frame. The dispatcher override uses __rg_dest for the same reason. Pins dest names that match former locals (input, read_timeout, fields_rc, dest). Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…d verdicts Review of #3871 found two field shapes where the one-process reader in pr-linkage-mcp-gate.sh turned a per-field DENY into an ALLOW. The CR probe, `contains("\r")`, errors on a non-string body, one erroring filter fails the whole hook::jq_fields batch, and the batch failure exited 0. A body of `5`, `true`, `{"a":1}` or `["x"]`, which the per-field reader rendered as text and blocked, was let through by the guard that exists to close the permissive direction. The probe now goes through `tostring` first, so it is total over every JSON type, and a batch that still fails (a tool_input or payload root that is not an object) falls back to the per-field reads it replaced instead of allowing outright. That fallback is unreachable for any object tool_input, so the fenced spawn counts do not move; on the paths that reach it the verdict is the per-field reader's own, blocks included. `$( )` chomped trailing newlines from every per-field read, so an `"owner": "acme-corp\n"` or a tool name with a trailing newline matched the exact-match guards and was gated; hook::jq_fields keeps the newline and both slipped past. TOOL, HOOK_CWD, T_OWNER, T_REPO and BODY are now chomped in-shell, byte-identical to their `$(jq -r)` form whenever they carry no CR. A CR inside tool_name, owner or repo is the one accepted stricter change: the per-field reader left it in place and the value never matched, hook::jq_fields strips it and the body is judged. The 154-payload differential (85 Bash, 69 MCP, rc + stdout + stderr) now shows the 7 deny-to-allow cases gone and no new mismatch: the Bash gate is verdict-identical on 85/85, the MCP gate on 67/69 with the two CR cases stricter. The MCP contract suite gains nine cases for these shapes (37/37); the Bash suite stays 146/146; the spawn budget stays 23/23 at 7/2, 11/3 and 11/4. The CHANGELOG entry no longer claims no verdict change; it names the stricter case. Version moves to 0.55.60: main is 0.55.58 and open PR #3838 already takes 0.55.59, and edits both gate files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
Merging main brought in #3838, which landed the fork-free hook::buffer_stdin_to in lib/hook-utils.sh. This guard now calls it, so the seventh process creation this PR documented as remaining is gone too. The contract suite pinned the benign share at exactly 1 and its own comment said the pin moves to 0 when that work lands. It has landed, so: - the strace pin is now 0, and its comment records why; - the changelog and README rows that named $(hook::buffer_stdin) as the remaining creation now say all seven are gone; - the measured figures are corrected against the merged tree: benign guard share 7 -> 0 creations, blocked 10 -> 4, stable across three repeats under the same instrument the table documents. The whole-dispatcher and wall-clock rows still describe the pre-#3838 base, which the README now states rather than restating numbers not measured on that host. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
One conflict: plugins/claude-ops/CHANGELOG.md. Both sides wrote a different entry under the same heading, 0.42.21 — main's is the #3838 hook::buffer_stdin_to sync, this branch's is the failure-audit fork reduction. A collision, not a stack. Main's 0.42.21 entry stays verbatim at the number main published it under. This branch's entry is renumbered to 0.42.22 and the manifest moves to match. hook-failure-audit.sh merged cleanly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
One conflict: plugins/guardrails/CHANGELOG.md. Both sides wrote a different entry under the same heading, 0.32.11 — main's is the #3838 dispatcher jq/stdin change, this branch's is the convention-gate redirect hoist. A collision, not a stack. Main's 0.32.11 entry stays verbatim at the number main published it under. This branch's entry is renumbered to 0.32.12 and the manifest moves to match. block-convention-violation.sh merged cleanly, and this branch touches no README row, so nothing else needed renumbering. Note for whoever merges second: PR #3869 also claims guardrails 0.32.12 against the same base. Both are correct against main's current 0.32.11, so the second of the two to merge needs a one-line renumber. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
One conflict: plugins/context-guard/CHANGELOG.md. Both sides wrote a different entry under the same heading, 0.7.46 — main's is the #3838 hook-utils sync, this branch's is the zone-crossing redirect hoist. A collision, not a stack. Main's 0.7.46 entry stays verbatim at the number main published it under. This branch's entry is renumbered to 0.7.47, with the manifest and the one README measurement reference (which cites the version the trace was taken under) moved to match. This resolves the merge conflict only. It does not address the acceptance- criteria gap this PR's own body declares: #3520's first criterion asks for no more than 2 external process creations and this branch reaches 4, with the remaining 4 -> 2 work blocked on #3740. That is an operator decision, not a lane one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
…ners (#3878) <!-- CURSOR_AGENT_PR_BODY_BEGIN --> No linked issue ## Summary Drop leftover process creations on the hottest Bash paths in this marketplace: the shared hook library every always-on hook sources, the always-on formatter Write/Edit paths (typos, ruff, biome, bash, powershell, go, actionlint, eol, markdown), the always-on desktop-notification Notification path, always-on guardrails verifiers, and CI scanners that used to spawn once per file, per plugin, or per allowlist entry. ## Fix GNU Bash runs command substitution in a subshell even for builtins (Command Substitution, [Bash Reference Manual](https://www.gnu.org/software/bash/manual/html_node/Command-Execution-Environment.html); [Greg's Wiki](https://mywiki.wooledge.org/CommandSubstitution)). Cygwin's `fork` is a non-copy-on-write Win32 `CreateProcess` ([Cygwin User's Guide, Process Creation](https://ftp.cygwin.com/cygwin-ug-net/highlights.html)): "fork will almost certainly always be inefficient under Win32." ### Shared hook library (`lib/hook-utils.sh`, synced to 17 carriers, patch bump) Same `_to` / in-process pattern as #3838, #3732, and #3678: - `hook::json_escape_to` deletes residual C0 bytes with parameter expansion instead of `printf | tr -d` - `hook::emit_channels` writes through `_to` instead of `$(hook::json_escape …)` - Fractional `read -t` landed in bash-4.0-alpha (CHANGES). `hook::read_supports_fractional_timeout` is `BASH_VERSINFO`; no TMPDIR probe file - `hook::notice_once` reads the marker with `read`, creates the directory only when missing, and prunes stale markers once per process - `hook::bash_parse_segments` walks `${cmd:i:1}` instead of `read -N1` from a process substitution, and `$'…'` bodies decode through `ansi_c_decode_to` (`printf -v`) - `hook::repo_root_to` / `hook::repo_relative_path_to` write in this shell so callers skip a leftover capture around git or builtins-only work Isolation `$(source …)` forks are unchanged (#3685). ### typos-format (always-on Write|Edit|NotebookEdit) - Basename via `${FILE##*/}` (plus a backslash trim), not `basename(1)` - `repo_root_to` / `repo_relative_path_to` instead of capture subshells - Directory existence check instead of `$(cd && pwd)` - `command -v typos` is no longer captured; the later exec looks the name up on PATH ### Remaining always-on formatters (ruff, biome, bash, powershell, go, actionlint, eol, markdown) Same leftover class as typos-format, now applied to every always-on formatter that still captured `_to` helpers or spawned `basename` / leftover `cd && pwd`: - `FILE_BASE` is `${FILE##*/}` (and a backslash trim) - `repo_root_to` / `repo_relative_path_to` write in-process - `$(cd && pwd)` canonicalize is an existence check on the path git already answered (ruff, biome, bash-format EditorConfig walk) - Nested `$(normalize_path "$(physical_path …)")` in powershell-format uses the `_to` forms - `command -v ruff|biome|goimports` is no longer captured - markdown-format keeps physical `pwd -P` containment and config discovery; leftover helper-capture and membership dirname on the root-resolution path are gone ### desktop-notification (always-on Notification) - Field extract fuses into `hook::buffer_stdin_to` so completeness and `.notification_type` / `.message` share one jq process - C0 stripping is parameter expansion, not `printf | tr` - `repo_root_to` writes in-process; OSC 9 / BEL use `printf -v`; `terminalSequence` uses `json_escape_jq_to` - `uname` stays so tests can PATH-stub Darwin; git for `repo_root` stays ### guardrails verifiers (always-on PostToolUse / PreToolUse) - `skill-reference-verify`, `stale-path-verify`, and `cli-flag-verify` call `repo_root_to` / `repo_relative_path_to` in-process - `hardcoded-path-check` and `secret-pattern-detection` use `normalize_path_to` instead of leftover `$(hook::normalize_path)` captures - Isolation `$(source …)` forks are unchanged (#3685) ### CI scanners - Orphaned-fixture scan: one `*.test.*` index, cached `evals.json` `files[]`, in-shell ERE escape. Unquoted `\\` matches one backslash (a quoted `'\\'` arm is two chars and leaves `\b` as a word boundary) - Purged-em-dash scan: one `git ls-files -z` with every `:(glob)` pathspec; in-process component-wise attribution so `*` cannot cross `/`. `--list` stdout is byte-identical to origin/main - Cross-plugin source drift: one `find plugins` plus one `sha256sum` of 2+ cluster paths. Discover stdout is byte-identical to origin/main - Discriminating-test-skips / silent-skips: one awk per corpus (`FNR` + `FILENAME`; mawk has no `ENDFILE`) - Hook-exec-form: one jq over every `hooks.json` and one over every `plugin.json` (`input_filename` attributes rows). Unreadable `hooks.json` still fails closed via per-file fallback; unreadable manifests are still skipped Hook-specific leftover-fork work already in flight (#3873, #3872, #3871, #3870, #3869, #3851, #3849, #3779, #3880, #3886) is out of scope here. ## Verification Independent census re-derived spawn counts from `84adf87b` vs `cdb93f61` without inheriting implementer figures. Kernel census `strace -f -e trace=clone,clone3,fork,vfork,execve`; counter over duration; 3 identical trials. **always-on formatters** (this revision vs `84adf87b`): | Hook | clones before | clones after | execve before | execve after | |---|---|---|---|---| | ruff-format no-config skip | 14 | 10 | 4 | 4 | | powershell-format no-settings skip | 23 | 16 | 7 | 7 | | bash-format no-EditorConfig (ShellCheck finding) | 17 | 13 | 6 (1 `basename`) | 5 (0 `basename`) | **guardrails** (this revision): | Hook | clones before | clones after | execve | |---|---|---|---| | skill-reference-verify Write, no skill refs | 18 | 17 | 8 unchanged | | secret-pattern-detection clean Write | 10 | 8 | 4 unchanged | Secret-pattern absolute counts are with `CLAUDE_PLUGIN_ROOT` set (Claude Code always sets it). Without that env the leftover `PLUGIN_ROOT=$(cd … && pwd)` fallback adds one clone on both sides (11→9); the drop of 2 is the same. **CI scanners** (successful execve, exclude ENOENT; earlier commits on this PR): | Gate | origin/main or prior HEAD | HEAD | |---|---|---| | purged-em-dashes `--list` | 478 | 9 | | cross-plugin-source-drift `--check` | 181 | 4 | | discriminating-test-skips | 316 (awk 312) | 5 (awk 1) | | silent-skips | 120 (awk 118) | 4 (awk 2) | | hook-exec-form `--check` | 196 execve, jq 96, tr 96, clones 292 | 7 execve, jq 2, tr 0, clones 9 | `--list` / discover stdout for the two listing gates is byte-identical to origin/main. **Local `scripts/affected-tests.sh --run`:** 153 shell suites passed or were skipped; 14 NOT RUN python/mjs ecosystems (exit 3, expected on this runner). No `FAIL`. Including: `lib/hook-utils.test.sh` PASS=323; bash-format PASS=54; eol-normalizer PASS=54; markdown-format PASS=174; powershell-format PASS=17; cli-flag-verify PASS=92; hardcoded-path-check PASS=118; secret-pattern-detection PASS=86; skill-reference-verify PASS=140; stale-path-verify PASS=108. ruff/biome/go/actionlint behavioral cases skipped here (binaries absent); skip-path and source pins still ran. `session-event-log.test.sh` PASS=53 isolated under the fan-out. **CI on `cdb93f61`:** lint, hook-utils, test-linux (0–3), test-windows, changes, ci-status, and managed-files-guard all succeeded. https://github.com/melodic-software/claude-code-plugins/actions/runs/34066676378 https://github.com/melodic-software/claude-code-plugins/actions/runs/34066676488 ## Related Refs #3838, #3732, #3678, #1979, #3488, #2891. Same leftover-fork class as open PRs #3849 / #3851 / #3869 / #3873 / #3872 / #3871 / #3870 / #3779 / #3880 / #3886 (those stay hook-specific). N/A for a dedicated issue. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-fdfdc962-be1b-4c9c-9833-3aec57852330?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-fdfdc962-be1b-4c9c-9833-3aec57852330&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…d verdicts Review of #3871 found two field shapes where the one-process reader in pr-linkage-mcp-gate.sh turned a per-field DENY into an ALLOW. The CR probe, `contains("\r")`, errors on a non-string body, one erroring filter fails the whole hook::jq_fields batch, and the batch failure exited 0. A body of `5`, `true`, `{"a":1}` or `["x"]`, which the per-field reader rendered as text and blocked, was let through by the guard that exists to close the permissive direction. The probe now goes through `tostring` first, so it is total over every JSON type, and a batch that still fails (a tool_input or payload root that is not an object) falls back to the per-field reads it replaced instead of allowing outright. That fallback is unreachable for any object tool_input, so the fenced spawn counts do not move; on the paths that reach it the verdict is the per-field reader's own, blocks included. `$( )` chomped trailing newlines from every per-field read, so an `"owner": "acme-corp\n"` or a tool name with a trailing newline matched the exact-match guards and was gated; hook::jq_fields keeps the newline and both slipped past. TOOL, HOOK_CWD, T_OWNER, T_REPO and BODY are now chomped in-shell, byte-identical to their `$(jq -r)` form whenever they carry no CR. A CR inside tool_name, owner or repo is the one accepted stricter change: the per-field reader left it in place and the value never matched, hook::jq_fields strips it and the body is judged. The 154-payload differential (85 Bash, 69 MCP, rc + stdout + stderr) now shows the 7 deny-to-allow cases gone and no new mismatch: the Bash gate is verdict-identical on 85/85, the MCP gate on 67/69 with the two CR cases stricter. The MCP contract suite gains nine cases for these shapes (37/37); the Bash suite stays 146/146; the spawn budget stays 23/23 at 7/2, 11/3 and 11/4. The CHANGELOG entry no longer claims no verdict change; it names the stricter case. Version moves to 0.55.60: main is 0.55.58 and open PR #3838 already takes 0.55.59, and edits both gate files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
) Closes #3528 ## Summary A guardrails hook that died between its first line and its own final `exit` (an unbound variable under `set -u`, a helper that no longer existed, a `source hook-utils.sh` that failed) ended with whatever status the last command had, usually 1, and wrote nothing of its own. Claude Code treats any status other than 0 and 2 as a non-blocking error: the tool call proceeds and the transcript records "exit 1, stderr: (none)". For a PreToolUse blocking guard that is enforcement silently skipped, which is what the issue recorded for `block-windows-drive-tmp` and `cli-flag-verify`. Every registered guardrails hook and the dispatcher now install a shared abort boundary that turns that outcome into a deliberate, visible one. Enforcement is unchanged: every hook declares fail-open, which is the status quo on abort; the notice is the only delta. ## Fix - New `plugins/guardrails/hooks/abort-boundary.sh` (a sibling library in `hooks/`, not `lib/hook-utils.sh`, which is contended by #3740 and #3838 and syncs to 17 copies). `guard::abort_boundary <name> <event> <open|closed> <chosen-status>...` installs an EXIT trap that passes the chosen statuses through untouched and turns any other into one stderr line naming the hook and the status plus a `systemMessage` / `additionalContext` document, then exits with the declared posture (`open`: exit 0, document on stdout; `closed`: exit 2, notice on stderr). The handler clears its own trap first, is builtins-only, and calls nothing from `hook-utils.sh`, so it still reports the case where that library failed to load. Stream choice was checked against the installed CLI (2.1.258: "Failed with non-blocking status code" on non-0/2, `hook_non_blocking_error`) and the repo's hook-observability doctrine: on exit 0 stderr reaches only the debug log and the stdout document is what the operator and agent see, so the notice goes on both. - All 14 registered hooks: `source abort-boundary.sh`, a per-hook posture comment beside the install line, and `source hook-utils.sh || exit 70` (a non-chosen status, so a failed library load is reported instead of limping through undefined `hook::` calls). Blocking guards choose `0 2`; advisory hooks choose `0`. `block-hook-bypass`'s bespoke handler (#3130 F5) is replaced by the shared one. - `run-guards.sh`: installs the same boundary around its own prologue and merge (an abort there skipped every guard of the event), primes `.hook_event_name` in its existing jq call so its notice names the event, and releases the trap before its deliberate aggregated `exit "$RC"` so a non-block status a guard returns still surfaces exactly as before. The event is looked up by NAME among the primed filters (review fix pass): the first cut read `RUN_GUARDS_VALUES[9]`, the field's position in `PRIME_FILTERS`, so a filter added ahead of it would have put the neighbouring value into every dispatcher notice. - One EXIT trap per shell (review fix pass): bash holds a single EXIT trap, so a guard that later runs its own `trap ... EXIT` replaces the boundary and is back to the bare rc 1 this PR removes (verified: with `trap ':' EXIT` after the install line a forced abort is rc 1, empty stdout, bash's own line only; without it, exit 0 plus the document). The suite now fails on any `trap` naming EXIT in a registered hook or in a library it sources (`hook-utils.sh`, the `--lib` paths). The library header states the supported way to chain exit-time work: a slot in the library that the handler calls before it decides, added with a suite case in the same change, never a second trap. No hook needs one today. - Version 0.32.16. Main is at 0.32.14; #3886 was renumbered to 0.32.15 after this branch opened (this body previously said it held 0.32.14, which was stale). Determined by reading `plugins/guardrails/.claude-plugin/plugin.json` at current `origin/main` and at the head of all 31 open PRs: #3886 is the only head above main, so 0.32.16 is the first free number. #3872 (0.32.13), #3849 and #3869 (0.32.12) and the rest sit at or below main and will need re-bumps of their own; whichever guardrails PR lands after this one takes 0.32.17. A CHANGELOG conflict at merge time is still expected. Exit-status contract mapped before the change (unchanged by it): guards exit 0 allow (including `hook::check_enabled` and `hook::require_jq`) and 2 deny (including `hook::require_jq_blocking`), nothing else on purpose (`cli-flag-verify`'s `exit 1`/`exit 2` strings are comment prose); `hook::buffer_stdin_to` returns 0 payload, 1 empty, 2 stalled/malformed, and the dispatcher exits 0 on rc 1 before any guard is sourced; the dispatcher exits 2 if any guard exited 2, else the highest non-zero guard status, else 0; guards run in `$( )` subshells, which do not inherit the parent's EXIT trap (verified empirically on bash 5.2, so the two boundaries never fire for one exit). ## Verification - `hooks/abort-boundary.test.sh`, 232 assertions, green (mode 100755, like its 17 siblings). It reads the registered set from `hooks.json` (15 scripts, never enumerated), asserts each sources the library and installs under its own name with a posture literal, asserts no registered hook and no sourced library (`hooks/hook-utils.sh`, `lib/powershell/ps-command.sh`) installs an EXIT trap of its own, asserts `PRIME_FILTERS` still carries `.hook_event_name`, forces a `set -u` abort in every registered guard on a plugin copy (exit 0, one stderr line naming the guard and `rc=1`, a JSON document whose `hookEventName` is an event the guard is registered for), forces the abort mid-hook through a failing shared helper on `block-windows-drive-tmp` (on a `D:/tmp` write the shipped guard still denies) and `cli-flag-verify`, checks the dispatched path keeps a sibling's deny (exit 2) beside an aborting guard and merges two notices into one document, checks the dispatcher's own boundary before and after priming plus its release, checks a prime filter inserted ahead of `.hook_event_name` on a copy still yields a notice naming `PreToolUse`, checks chosen statuses and the kill switch pass through silently, and checks the handler ends the process once when its own body fails. - Non-vacuity, one mutation at a time, suite re-run, reverted: handler ignores the chosen-status check (137 failures); `stale-path-verify` loses its install line (2); open posture exits 1 (27); dispatcher forgets its release (2); `block-hook-bypass` loses `|| exit 70` (1); closed posture exits 0 (1). Fix pass: the new suite against the previous positional `run-guards.sh` fails exactly the new shift assertion ("expected 'PreToolUse', got ''", 231/1); a copy of `block-no-verify` given `trap 'rm -f "$TMP_SCRATCH"' EXIT` after its install line fails the new trap assertion by file and line. One mutation is NOT caught: removing `trap - EXIT` from the handler (0 failures), because bash itself never re-runs an EXIT trap. The review confirmed this independently: that line survived 15 attack shapes, so on bash it is redundant defense in depth rather than load-bearing; the re-entry test proves the observable property (one notice, terminates), not that line. - Review confirmation of the enforcement claim: the reviewer independently ran 234 cases (22 payloads standalone and dispatched, plus 6 forced-abort modes: `hook-utils.sh` missing, `hook-utils.sh` unparseable, an injected `set -u` abort in every guard, a missing `hook::` helper, jq off PATH, stdin closed) and found no ALLOW/DENY flip anywhere. - A/B against a pristine `git archive origin/main` export of the plugin. (a) Suite level: all 17 existing guardrails suites produce identical `ok:`/`FAIL:` assertion lines on both trees (3,206 assertions on the pristine side), with two explained differences: `require-jq-posture` lists the new library in its census (now excluded, like `hook-utils.sh`), and `block-hook-bypass`'s "symlink: genuine temp write" case fails on EITHER tree when the suite runs from under `/tmp` and passes on either tree from outside it (location artefact, verified both ways). (b) Explicit corpus: 71 payloads (36 Bash, 5 PowerShell, 24 Write/Edit, 6 PostToolUse, 1 Workflow, plus non-JSON and empty stdin) run standalone through every guard and through the dispatcher as `hooks.json` registers it, on both trees: exit code, stdout, and stderr byte-identical across all 71 (branch census: 38 deny, 32 allow at the dispatcher, 1 Workflow-only). No DENY-to-ALLOW flip anywhere. - `bash scripts/affected-tests.sh --run --shard N/4`, N in 0..3, after the fix pass: 153 shell suites PASS (every guardrails suite among them, `abort-boundary.test.sh` at 232/0 under the runner), 1 FAIL: `plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh` (process-budget trace probe, 2 of 24 cases), reproduced identically on a clean `git archive origin/main` export, so pre-existing and unrelated. Shards 0, 2, 3 rc 3; shard 1 rc 1 is that one suite. 14 suites in other ecosystems reported NOT RUN by the shell runner. - Parity, all four modes green against current `origin/main`: `--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main` (159 headings preserved). - ShellCheck 0.11.0 clean on every changed shell file (`-x`, repo `.shellcheckrc`); `check-silent-skips.sh` clean; `check-shell-portability.sh origin/main` clean (22 files); `sync-hook-utils.sh --check`: all 17 copies match, `lib/hook-utils.sh` byte-identical to main; no em dash in any added line; no here-string introduced in production code. - Budget: `strace -f -e trace=clone,clone3,fork,vfork,execve` of the whole Bash dispatcher on a benign command, three repeats per side: creations 23 -> 23, execve 2 -> 2. Wall p50 moved about 2 to 5 ms on this host (recorded in the README budget section). The name lookup is a loop over ten array entries with builtins, no process. Not measured on Windows. Known consequence, stated plainly (verified against the 2.1.258 binary during review): exit 0 plus a valid JSON document IS read (`systemMessage` as a meta message, `additionalContext` accepted for `hookEventName: "PreToolUse"`), while stderr is never read on exit 0, so the stdout document was the correct channel. The trade-off is that `plugins/claude-ops/hooks/hook-failure-audit.sh:146` greps only `hook_non_blocking_error`, so these aborts leave NO hook failure record for that detector, and the bash error line that pristine surfaced through the exit-1 warning is now debug-log only (`claude --debug`). The notice in the transcript and the agent's context replaces the failure record; nothing else does. The issue's second verification item (that detector reporting zero records over a live session with a forced abort) was therefore not run and would hold by construction rather than by observation. The dispatcher's notice names the event only when the payload carries `hook_event_name` (Claude Code's payloads do; a payload without it gets `systemMessage` only). Adjacent and untouched: #3861 (guard fail-open when no interpreter resolves) is a different path; nothing here changes interpreter resolution. ## Related - Refs #3713 (same defect class on four hooks in other plugins; not reached by a guardrails-local library) - Refs #3507, #3130 F5 (the `block-hook-bypass` handler this generalizes) - Refs #3740, #3838 (open `lib/hook-utils.sh` PRs; deliberately not touched) - Refs #3886 (0.32.15, the number below this one), #3849, #3869, #3872 (open guardrails PRs at or below main; CHANGELOG conflict expected) - Refs #3861 (adjacent fail-open path, needs-human, not part of this change) - Refs #3508, #3512, #3517 (environmental trigger and latency work, out of scope) - docs/adr/0004-rightsize-instruction-surfaces-by-incumbent-first-arbitration.md (killed PreToolUse hook yields no decision) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…acement (#3871) Closes #3509 ## Summary `pr-body-linkage-gate.sh` timed out on **every** recorded run in the measurement window — 423 timeouts against a 15 s ceiling, the worst blocked count in the #3508 campaign. A gate that is killed before it renders a verdict protects nothing, so its cost is part of its contract. Its MCP-surface sibling `pr-linkage-mcp-gate.sh` and the validator both share is fixed here too. **The parent's stated cause does not hold, and this PR does not act on it.** #3508 attributes the cost to per-field `jq` forks needing a new shared helper in `lib/hook-utils.sh`. Shard #3520 (PR #3779) established the real mechanism, and merged PR #3788 fixed 34 hook scripts across 17 plugins while touching `lib/hook-utils.sh` **zero times**. This PR follows that precedent: **`lib/hook-utils.sh` and every `plugins/*/hooks/hook-utils.sh` copy are untouched** (unmerged PRs #3740 and #3838 own that file). The fix is entirely in-file. The mechanism, re-verified on this host before any edit was made: | Form | clone-family | `execve` | | --- | --- | --- | | `V=$(jq . f)` | 1 | 1 | | `V=$(jq . f 2>/dev/null)` | **2** | 1 | | `{ V=$(jq . f); } 2>/dev/null` | **1** | 1 | | `V=$(printf \| jq \| tr)` | 4 | 2 | | `V=$(cat -- f)` | 1 | 1 | | `V=$(<f)` | **0** | **0** | | `V=$(shellfunc)` | 1 | 0 | | out-variable call | **0** | 0 | | `read < <(printf …)` | 1 | 0 | Bash elides the extra fork and execs in the command substitution's own subshell only when the command carries no redirection of its own. A `2>/dev/null`, a `<<<`, or a pipeline inside the substitution defeats that. ## Fix Four shapes, all in-file: 1. **Per-field `jq` batched into one process.** Both gates read payload fields through `printf '%s' "$INPUT" | jq -r … 2>/dev/null | tr -d '\r'`, once per field — 4 clones and 2 execs each, **five times over** on the MCP surface, all asking about one buffered string. One `hook::jq_fields` call (the library's existing batched reader, called not changed) answers every field, and CR-strips exactly as the `tr` did. 2. **Redirection hoisted onto the enclosing group.** `ORIGIN=$(git … 2>/dev/null || true)` became `{ ORIGIN=$(git …) || ORIGIN=""; } 2>/dev/null`. The group holds exactly one command, so nothing beyond that git call is silenced. 3. **The shared validator's helpers write into a caller-named variable.** `strip_html_comments`, `mask_markdown_code`, `section_content` and `trim` were each read through `$(…)` over a `< <(printf …)` line reader: **12 forks and zero extra `execve` per judged body** — pure process-creation latency. They now use `printf -v` and an in-shell line split. `linkage::chomp_to` reproduces the trailing-newline strip that command substitution performed, which is the one thing a naive out-variable conversion gets wrong. 4. **`$(<file)` for the `--body-file` read, `printf -v '%q'` for wrapper re-quoting, `hook::json_str_object_to` for the telemetry envelope** — replacing a `cat`, a `printf` substitution, and a `jq -n` that bash can do itself. The `$(dirname …)` source line was already fixed in both gates by #3771. `pr-body-linkage-gate.sh` and `pr-linkage-validator.sh` now invoke **no external command of their own at all**; every remaining spawn on their path belongs to `lib/hook-utils.sh`. `pr-linkage-mcp-gate.sh` keeps three, each justified: the origin-remote scope guard, the `jq -e` defer-guard on a repo's own `settings.json` (behind a `[[ -f ]]` probe), and a carriage-return fallback described below. ### Where the MCP reader's behaviour could change, and what it does about each `hook::jq_fields` CR-strips every value it returns; the validator only strips a CR at end of line. A body with a **mid-line** CR would therefore be judged against different text — and stripping is the **permissive** direction: `## Sum<CR>mary` becomes a section that was previously missing, turning a BLOCK into a silent ALLOW. The batch therefore also reports whether the raw body holds a CR at all, and the MCP body is re-read losslessly with its own `jq` only in that case, which no real payload hits. Review of the first revision found two field shapes where that batched reader itself flipped a per-field DENY to ALLOW; both are fixed in the second commit (`fe3fd235`), and the differential below was re-run over them: - **The CR probe was not type-safe.** `contains("\r")` errors on a non-string body, one erroring filter fails the whole batch, and a failed batch exited 0 — so a body of `5`, `true`, `{"a":1}` or `["x"]`, which the per-field reader rendered as text and blocked, was allowed. The probe now goes through `tostring` first, so it is total over every JSON type. A batch that **still** fails (a `tool_input` or payload root that is not an object) falls back to the per-field reads it replaced instead of allowing outright, so a batch failure is now exactly as fail-closed as the per-field reader was: a determinable bad body still blocks. That fallback is unreachable for any object `tool_input`, so the fenced spawn counts do not move. - **Trailing newlines survived on the exact-match fields.** `$( )` chomped them from every per-field read, so `"owner": "acme-corp\n"` or a tool name with a trailing newline matched the guards and was gated; `hook::jq_fields` keeps the newline and both slipped past. `TOOL`, `HOOK_CWD`, `T_OWNER`, `T_REPO` and `BODY` are now chomped in-shell, byte-identical to their `$(jq -r …)` form whenever they carry no CR. **One accepted stricter change remains.** A CR *inside* `tool_name`, `owner` or `repo` used to stay in the value, so it never matched and the call was allowed; `hook::jq_fields` strips it, the value matches, and the body is judged. Neither GitHub nor the MCP server produces such a value; the stricter direction is kept and named in the CHANGELOG rather than asserted away. ## Verification **Behaviour, proven by differential rather than argued.** - **End-to-end, 154 payloads (85 Bash, 69 MCP), exit code, stdout and stderr compared byte-for-byte** between the merge-base gates and these, after `fe3fd235`. Bash surface: every body-flag spelling (`--body`, `-b`, `--body=`, `--body-file`, `-F`, attached forms), stdin and substitution heredocs, multiple-heredoc and unterminated cases, missing body files, `--repo`/`-R` and `cd` escapes, `env -S` / `env` / `sudo` wrappers, `gh.exe` and `./gh`, CRLF and mid-line-CR bodies, NUL bytes, NBSP/BOM/ZWSP/U+2028/U+2029, `--fill`/`--web`, absent and non-string `command`, trailing-newline and CR `cwd`. MCP surface: the tool x owner x body matrix, absent/empty/null body, non-string bodies (`5`, `true`, `{"a":1}`, `["x"]`, `false`), absent and non-object `tool_input`, NUL and CR in `owner`, trailing newline and CR in `owner`/`repo`/`tool_name`/`cwd`. **Bash gate: 85/85 verdict-identical. MCP gate: 67/69 verdict-identical; the remaining 2 are the CR-in-`owner` and CR-in-`tool_name` cases above, ALLOW to DENY.** Before `fe3fd235` the same run showed 7 DENY-to-ALLOW mismatches (the four non-string bodies, `owner\n`, `repo\n`, `tool_name\n`); all 7 are gone and no new one appeared. Bash's own "ignored null byte" warning line, whose text carries the script path, is excluded from the byte comparison. Denies: 69 at the merge base, 71 here. - **Validator differential, 425 bodies**, hand cases plus a seeded fuzz corpus over heading/fence/comment/CR/NBSP/backtick-run tokens. **0 mismatches.** - **Trailing-newline invariance** proved separately, because `hook::jq_fields` preserves trailing newlines where `$(printf | jq)` stripped them: 24 validator cases and 9 hook cases, 0 mismatches; the MCP gate's own fields are now chomped as well. - Contract suites: `pr-body-linkage-gate.test.sh` **146/146**, `pr-linkage-mcp-gate.test.sh` **37/37** (nine new cases: the four non-string bodies block, `owner\n` / `repo\n` / `tool_name\n` still gate, CR-in-owner gates as the accepted stricter case, a string `tool_input` takes the fallback and allows). **Cost, measured with `strace -f -e trace=clone,clone3,fork,vfork,execve`** — not an xtrace command count, which reads source positions rather than kernel spawns (#3520 measured xtrace at 2 against 8 real spawns on one script). Telemetry sink off; re-measured after `fe3fd235`, unchanged: | Path | clones before | clones after | `execve` before | `execve` after | | --- | --- | --- | --- | --- | | Bash gate, a `gh` call with no `pr` | 8 | **7** | 3 | **2** | | Bash gate, `gh pr create` with a body (ALLOW) | 28 | **11** | 6 | **3** | | Bash gate, `gh pr create` with a body (BLOCK) | 28 | **11** | 6 | **3** | | MCP gate, unrelated tool | 7 | **7** | 2 | **2** | | MCP gate, create (ALLOW) | 37 | **11** | 9 | **4** | | MCP gate, create (BLOCK) | 37 | **11** | 9 | **4** | Two components: the validator refactor alone removes **12 clones with `execve` unchanged** — that half is pure latency, no work removed. The rest is genuinely duplicated work removed: six `jq` processes re-parsing one buffered payload, plus two `tr` calls deleting a byte class bash rewrites in place. **No wall-clock figure is claimed.** This is a Linux host where a spawn costs ~3-5 ms; the campaign's host measures 0.3-0.9 s and is bimodal at 501 concurrent processes. A timing here would say nothing about there, so the process count is reported as the proxy, per #3508's own correction. **New gate: `hooks/pr-linkage-spawn-budget.test.sh`.** Ceilings are the measured counts with **no headroom**, per `hook-budget.md` rule 2. It refuses to report a pass it has not earned: - a self-check first proves the harness can distinguish `$(cmd 2>/dev/null)` from `{ …; } 2>/dev/null` at the kernel level, and **SKIPs** rather than passing if it cannot (no ptrace, no strace); - three mutants must each raise the count above the ceiling or **the suite fails itself**: a redirect moved back inside a substitution (11 -> 12 clones), one field split back out of the batch (11 -> 14 clones, 4 -> 5 execve), a validator helper re-forking (11 -> 13 clones); - it asserts both gates still exit 2 on a failing body, so a budget of zero spawns cannot pass as a no-op. Pointed at HEAD's hooks the new suite reports **13 failures**; against this branch, 23/23 pass. **Gates run** (re-run after `fe3fd235`). `scripts/affected-tests.sh --run`: 151 shell suites pass, 14 selected suites belong to ecosystems the runner does not execute (reported NOT RUN, not skipped), one pre-existing failure noted below; `scripts/check-changelog-parity.sh` in all four modes (`--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main`) all rc=0; `shellcheck -x` and `shfmt -d` clean on all five scripts; `markdownlint-cli2` and `editorconfig-checker` clean on the changed files; `check-shell-portability.sh`, `check-purged-em-dashes.sh`, `check-silent-skips.sh`, `check-discriminating-test-skips.sh`, `check-killswitch-hoist.sh`, `check-hook-exec-form.sh`, `check-fixture-git-isolation.sh` all rc=0 on the first revision. Manifest bumped 0.55.58 -> **0.55.60** with the matching CHANGELOG entry (see Related for why not 0.55.59), and the README carries the measured share per `hook-budget.md` rule 1. **Pre-existing failures, not from this branch** (both reproduce on a clean `HEAD` checkout, and this branch touches no file either reads): `plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh` (2 cases, its own xtrace-based budget probe) and `plugins/session-flow/scripts/tests/test_save_point.py::test_new_origin_falls_back_to_directory_name`. ### Acceptance criteria: two are not fully met, stated plainly | # | Criterion | Status | | --- | --- | --- | | 1 | No more than 2 external spawns on the common path (own shell + at most one `jq`) | **NOT MET — over by one.** The common path is now the hook's own shell, one `jq -e .`, and one batched `jq`. The batched `jq` is the criterion's allowance; the `jq -e .` is `hook::buffer_stdin`'s payload validation inside `lib/hook-utils.sh`, which this PR is fenced off from. Removing it is that library's change to make, in #3740/#3838. | | 2 | `grep`/`sed`/`cut`/`tr`/`basename`/`dirname` on the hot path replaced with builtins | **MET.** None appears in any of the three scripts; `cat` is gone too. | | 3 | Matcher or early guard exits before any spawn for non-matching invocations | **PARTIALLY MET.** The `if: Bash(*gh *)` filter (pre-existing) removes the hook process entirely for non-`gh` calls, and the two in-hook guards short-circuit before any repo I/O. They cannot run before `hook::buffer_stdin`, whose `jq` is the same library spawn as row 1 — the hook must read stdin before it can know what it is looking at. | | 4 | Existing behavioural tests still pass; the guard still blocks what it blocked before | **MET, with one named exception.** 183 contract cases plus the 154-payload differential: every merge-base DENY is reproduced, and two merge-base ALLOWs (a CR inside `owner` or `tool_name`) are now DENY, the stricter direction, recorded in the CHANGELOG. | | 5 | Under 2 s for a single run on a Windows host | **NOT VERIFIED.** No Windows host available; the process-count proxy above is offered instead, and no timing figure is invented. | ## Related - Parent: #3508 (Windows process-creation tax). Its stated cause — per-field `jq` needing a new shared `hook-utils.sh` helper — is not what this PR acts on; see Summary. - Precedent: #3520 / PR #3779 (established redirection placement as the real mechanism) and merged PR #3788 (34 scripts, 17 plugins, `lib/hook-utils.sh` untouched). - **Version and overlap with #3838.** `main` is at `source-control` 0.55.58. Open, ready PR #3838 (`cursor/shell-script-perf-phase1-bb5b`) bumps this plugin to **0.55.59** with its own `## [0.55.59]` heading, and edits **both gate files this PR touches** (`pr-body-linkage-gate.sh`, `pr-linkage-mcp-gate.sh`, introducing `hook::buffer_stdin_to`) plus the three worktree gates and the synced `hook-utils.sh`. This PR therefore takes **0.55.60**, verified against `main`, #3838, #3774 (0.55.58, no bump) and #3740 (tops at 0.55.54). Whichever of #3838 and this PR lands second needs a **rebase of the two gate files, not just a rebump**; that is the merge lane's call, flagged here so it is not a surprise. Sibling #3510 is in flight against this plugin and takes the next version after this one. - Fenced off: unmerged PRs #3740 and #3838 own `lib/hook-utils.sh`; #3510 owns `worktree-add-containment-gate.sh`, `worktree-add-claim-gate.sh` and `worktree-create-gate.sh`, which share this plugin's `CHANGELOG.md` and manifest. Neither set is touched here. - Prior art acknowledged: #1403 / PR #1385, whose revival bar (four contract-test regressions, two failing open) is what the deny-preserving differential above is aimed at. - Budget authority: `docs/conventions/hook-budget/README.md` (#1809), surfaced by `.claude/rules/hook-budget.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…3513) On a benign Bash call the guard created seven processes and executed none, so every exec census (PATH-shim spawn census, the dispatcher test's dirname/sed pin, an xtrace command count) read it as free. Six were in this file: 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), whose fork-free form is lib/hook-utils.sh work (#3740, #3838) and is not touched here. 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. Kernel census, strace -f -e trace=clone,clone3,fork,vfork,execve on the dispatched path, HOOK_TELEMETRY_SINK unset, guard share = count minus a no-op guard dispatched the same way: benign git status creations 7 -> 1, execve 0 -> 0; blocked echo > file creations 10 -> 5, execve 1 -> 1; whole Bash dispatcher 36 -> 30 creations, execve 3 -> 3. Verdicts unchanged across 244 paired runs against origin/main and the 602-case contract suite. The suite now pins the benign share at exactly 1 with the same instrument; each of the four changes reverted alone fails that pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
Merging main brought in #3838, which landed the fork-free hook::buffer_stdin_to in lib/hook-utils.sh. This guard now calls it, so the seventh process creation this PR documented as remaining is gone too. The contract suite pinned the benign share at exactly 1 and its own comment said the pin moves to 0 when that work lands. It has landed, so: - the strace pin is now 0, and its comment records why; - the changelog and README rows that named $(hook::buffer_stdin) as the remaining creation now say all seven are gone; - the measured figures are corrected against the merged tree: benign guard share 7 -> 0 creations, blocked 10 -> 4, stable across three repeats under the same instrument the table documents. The whole-dispatcher and wall-clock rows still describe the pre-#3838 base, which the README now states rather than restating numbers not measured on that host. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
…it (#3529) On every Bash and PowerShell call the guard created three processes of its own and executed none, so the PATH-shim census read it as free. One of the three was this file's: an eager SUBJECT=$(hook::extract_bash_subject) at file scope feeding a telemetry envelope that is off by default and that the verdict never reads. It is now derived inside emit_tel, behind the start-stamp and sink gates. Off the common path, three more sites paid a fork for a string: - the hash-width probe $(git ... rev-parse --show-object-format 2>&1) cost two creations for one exec, because bash execs a substitution's body in its own subshell only when that body carries no redirection. The 2>&1 has to stay inside (git's stderr is the diagnostic the block message quotes), so the body is now `exec git ...`; - a `!` alias reparse spent one $(printf '%q') per trailing argument, now printf -v; - its $(effective_dir ...) around a builtins-only function is now effective_dir_to, a nameref assignment. Kernel census (strace -f -e trace=clone,clone3,fork,vfork,execve, guard share = dispatched minus a no-op guard dispatched the same way): benign and blocked Bash calls 3 -> 2 creations, execve 0 -> 0; lease with a full-width oid 5 -> 3, execve 1 -> 1; `!` alias with three trailing args 8 -> 3; whole Bash dispatcher 35 -> 34, execve 3 -> 3. The two creations left are $(hook::buffer_stdin) and the shared parser's < <(printf ...), both lib/hook-utils.sh work (#3740, #3838), and the PowerShell lane's fourteen live in lib/powershell/ps-command.sh. Verdicts are unchanged: 190 paired runs against origin/main (87 Bash and 8 PowerShell commands, standalone and dispatched, three repository shapes, CR/BOM/zero-width/U+2028 variants) agree on exit code and full stderr; the contract suite passes at 492 and now pins each site by the same instrument, each pin checked by reverting its change alone. Guardrails 0.32.13 (0.32.11 and 0.32.12 are taken by #3849 and #3869). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
origin/main already landed hook::buffer_stdin_to and the parser's fork-free line split, so the two remaining creations this pin named are gone. Move the benign share from 2 to 0 and the !-alias reparse from +1 to +0. The hash-width probe is still one creation over benign. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…ach (#3873) Closes #3510 ## Summary The three `source-control` worktree gates spawned four processes to read each hook payload field. The parent campaign (#3508) blames per-field `jq` forks needing a shared helper; that diagnosis is wrong and this is the fourth PR to show it. Part of the cost is a **fork that never execs**, part is a `| tr -d '\r'` stage behind every read, and all of it is in-file: > `$(cmd)` = 1 process. `$(cmd 2>/dev/null)` = 2. `$(a | b | c)` = 4. `{ v=$(cmd); } 2>/dev/null` = 1. `$(printf '%s' "$X" | cmd)` = 3. Measured on this branch with `strace -f -e trace=clone,clone3,fork,vfork,execve`, confirming the mechanism before touching anything. `lib/hook-utils.sh` and its 17 synced plugin copies are **untouched**. Reported at **418 timeouts**, worst hook in the set after #3511. A scout counted roughly 7 in-substitution redirect/pipe sites; the actual count is 8: three in `worktree-add-containment-gate.sh`, four in `worktree-add-claim-gate.sh`, one in `worktree-create-gate.sh`. **`worktree-create-gate.sh` was partly clean already.** It carries the fixed `${BASH_SOURCE[0]%/*}` form (from #3788), and `hooks.json` registers it under `WorktreeCreate` at timeout 60, firing once per worktree creation. It is *not* on the `PreToolUse:Bash` path that produced the 418 timeouts, so none of those were its. Its two field reads were still costing 19 process creations, so they are fixed here, but the win is bookkeeping, not the timeout. **Review fix, second revision.** The first revision of this PR fed the whole payload to `jq` and `sed` by **here-string** (`{ V=$(jq …); } <<<"$INPUT"`), which is the exact form `lib/hook-utils.sh` forbids at `hook::json_complete` ("`printf | jq`, never `jq <<< "$1"`"). The reason is #1587 (`dfda6ec3`): bash fills a here-string's pipe itself, so a payload at or above the pipe capacity, **65536 bytes, traced hanging bash indefinitely on Git Bash**, blocks the shell before `jq` is ever exec'd. On these hooks that hang is the 15 s timeout, and on a containment gate that is a stall **and** a fail-open, the failure mode this campaign exists to remove. It does not reproduce on Linux bash 5.2 (the temp-file fallback engages at 65536 to 70000 bytes, verified in this session), which is how it slipped through. This revision adopts the library's own form and re-measures; the numbers below are the honest new ones, and several are worse than the first revision's. ## Fix In-file only, all mechanical: 1. **Both Bash gates' payload reads** (2 sites in containment, 3 in claim): `$(printf '%s' "$INPUT" | jq … 2>/dev/null | tr -d '\r')` becomes `$(printf '%s' "$INPUT" | jq … 2>/dev/null)` plus `V="${V//$'\r'/}"`. The feed is **kept as `printf '%s' "$INPUT" | jq`**, byte-for-byte the feed line inside `hook::jq_field`, the form the library prescribes for a hook payload; the jq program text is unchanged (`// empty`, no `gsub`), which is what keeps every differential below byte-identical to `main` on a non-string `.cwd`. Only the `tr` stage goes. `hook::jq_field` itself was measured too: it costs 4 creations to the inline form's 3 and adds a string-only `gsub` that changes the non-string-field behaviour, so it is not used. A here-string was measured at 1 creation and is not used either, for the reason above. 2. **`git_unlocated`**: `2>/dev/null` moves off the `git` command onto the enclosing subshell, so bash execs git in that subshell instead of forking for it. `unset` writes nothing to stderr, so the same stream is silenced and no other. 3. **`configured_root`**: `git config --get-all | tail -n 1 | tr -d '\r'` becomes one `git` plus `${r##*$'\n'}` and `${r//$'\r'/}`, on a single-command group (no payload involved). Multi-value last-wins verified A/B. 4. **Claim gate's stderr temp file**: `mktemp` + `2>"$err_file"` + `rm -f` where the file was **never read**. Replaced with `2>/dev/null` on a one-command group. This also removes the `|| continue` that silently skipped a claim whenever `TMPDIR` was unwritable, a fail-open now gone (`TMPDIR=/nonexistent` reproduced the lost claim on `main`). `claim_rc=$?` stays outside the group so it is still the helper's status. 5. **Create gate's `json_field`** becomes a `_to` form (the convention `hook-utils.sh` documents on its own `_to` helpers), dropping the two substitutions that wrapped each read; each rung is its own `printf '%s' "$payload" | jq` or `printf '%s' "$payload" | sed` pipeline. The jq program text is byte-for-byte `hook::jq_field`'s, `gsub` included, on purpose: `gsub` is string-only, so a numeric or object `.name` fails jq and falls through to the string-shaped `sed` rung, which yields nothing and refuses. 6. Nothing else. Every remaining `jq`, `git` and `sed` call is the same call with the same arguments. The two `<<<` left in the Bash gates are `IFS='/' read -r -a segs <<<"$rest"`, a builtin fed one path string, pre-existing on `main`; bash fills no pipe for a builtin, so that is not the hazard. ### Counts, per file, before → after Process creations (clone/clone3/fork/vfork) and `execve`, counted separately, re-measured after the review fix. No wall-clock figure: this Linux host is nothing like the campaign's contended Windows hosts, so a time here would be misleading. | Hook | Path | creations | `execve` | | --- | --- | --- | --- | | `worktree-add-containment-gate.sh` | a `worktree` command that is not an `add` (the hot path) | 8 → **7** | 3 → **2** | | | an `add` outside every repository (allow) | 22 → **18** | 7 → **5** | | | an `add` into a working tree (**block**) | 21 → **18** | 7 → **5** | | | an `add` into a `.git` directory (**block**) | 27 → **22** | 9 → **7** | | | `git -C <repo> worktree add sub/x` (**block**, names the root) | 26 → **20** | 10 → **6** | | | dynamic / post-`cd` / `echo git worktree add` (allow) | 13 → **11** | 5 → **3** | | `worktree-add-claim-gate.sh` | a `worktree` command that is not an `add` | 8 → **7** | 3 → **2** | | | a parsed `add` target | 28 → **22** | 11 → **6** | | `worktree-create-gate.sh` | payload with no `.name` (before the helper runs) | 19 → **13** | 6 → **4** | | | disabled by the kill switch | 0 → **0** | 0 → **0** | For the record, the first revision's here-string counts were 5 / 14 / 14 / 18 / 16 / 7 / 5 / 16 / 7 creations on those rows. The `printf | jq` form gives back exactly 2 creations per field read (3 where a here-string was 1), and no `execve`: the `execve` column is unchanged from the first revision. **`execve` dropping is not removed work.** Every drop is a named program replaced by a bash builtin or removed as dead code: | Program | Replacement | | --- | --- | | `tr -d '\r'` (×6 across the two Bash gates, 3 + 3; ×1 in the create gate; 7 removed) | `${v//$'\r'/}` | | `tail -n 1` (containment `configured_root`) | `${r##*$'\n'}` | | `head -n 1` (create gate fallback rung) | `${v%%$'\n'*}` | | `mktemp` + `rm` (claim gate) | removed; the temp file was written and deleted, never read | Four of the seven creations left on the hot path belong to `lib/hook-utils.sh` (the `hook::buffer_stdin` substitution and `hook::json_complete`'s `printf | jq -e .`), which is fenced off here. The gate's own share is the one `printf | jq` field read: 3 creations, 1 `execve`. ## Verification **Behaviour is the risk** on these gates: a swallowed non-zero status turns a block into a silent allow, which here means an unclaimed or out-of-tree worktree gets created. So every deny path and its near misses were A/B'd on **rc, full stdout and full stderr** against a pristine `git archive origin/main` copy of the plugin, re-run in full after the review fix: - **Containment, 52 probes**: 11 payloads × 4 root-resolution environments (nothing configured, plugin option, plugin data dir, `melodic.worktreeroot` git key) = 44, plus the kill switch, an empty stdin, a malformed payload, four non-string `.tool_input.command` values (number, object, `null`, `true`), and a non-string `.cwd`. Payloads: out-of-containment target, target inside a working tree, target inside a `.git` directory, `git -C`-composed relative target, valid external creation, `..`-escape out of the repo, dynamic `$HOME` target, post-`cd` target, `echo git worktree add`, an unrelated command, a CR-bearing path. **Byte-identical, all 52.** - **`configured_root` precedence** separately: git key beats plugin option beats data dir; multi-value `--get-all` last-wins; single value. Identical. - **Claim gate, real state**: a freshly-added unlocked worktree (claimed, correct `additionalContext`), a worktree already carrying **another session's** live claim (helper rc 4, not rewritten, reason string preserved verbatim), a target that was never created (nothing claimed), and a payload with no `session_id`. Identical, including the emitted JSON, modulo fixture path and timestamp. - **Create gate, 18 probes** including the disabled path, empty stdin, illegal branch name, missing root, non-repository cwd, unexpanded `${user_config}` placeholder, `name` 4242 / object / `null` with jq present, and a **jq-absent PATH** exercising the `sed` fallback rung with `name` 4242, `null` and a string. Identical apart from fixture paths and a fixture commit SHA. - **Existing suites**: `worktree-add-containment-gate.test.sh` (41 cases), `worktree-add-claim-gate.test.sh` (24), `worktree-create-gate.test.sh` (37). All pass. ### Permissive normalization The sibling shard's finding, that CR stripping can be the *permissive* direction, was checked here in both directions, with CR, BOM, zero-width space, U+2028, U+2029, NBSP, and trailing newline in the target path and in `.cwd`. **A/B identical on all 15 probes.** No batching was introduced, so neither of the two flip mechanisms is reachable: each field keeps its own `$(…)` and its own `// empty`, and the only string-only jq filter in the diff is the create gate's **pre-existing** `gsub("\r";"")`, kept byte-for-byte on purpose (see Fix 5). `name:4242`, `name:{"a":1}` and `name:null` are all still refused with the identical message, with jq present and absent. ### Pre-existing CR containment bypass (not fixed here; predates this work) Recorded precisely enough to act on without rediscovery, because a security finding should not live only in a PR body. **This PR does not fix it, and it is present identically on `origin/main`**; this lane does not file work items. - **Payload shape.** A Bash tool call whose command is `git worktree add ..<CR>/../outside/x` from a cwd inside a repository, where `<CR>` is the raw byte 0x0D immediately after a `..` segment. On the wire the harness JSON-escapes it, so the payload reads `"command":"git worktree add ..\r/../outside/x"`; that is the real `PreToolUse:Bash` payload path, nothing downstream re-parses the command, and the hook's tokenizer keeps the word whole. - **What the hook does.** `worktree-add-containment-gate.sh` strips every CR from the command before resolving, so it sees `../../outside/x`, resolves it to a path outside the repository, and returns rc 0 (allow). Plain `../outside/x` from the same cwd is blocked (rc 2, target `<repo>/outside/x`); `a<CR>b/../../outside/x` is also blocked, since a CR mid-segment leaves the nearest-existing-ancestor walk on the repository. The `..`-adjacent position is the one that flips. - **What git then does.** git does not strip the CR: `..<CR>` is an ordinary directory name, so `..<CR>/../outside/x` resolves to `<cwd>/outside/x`, and `git worktree add` creates it there, **inside the repository** (`<repo>/sub/outside/x`, rc 0, confirmed by `git worktree list`; a literal `..<CR>` directory is left in `<repo>/sub`). From the repository root, `..<CR>/../.git/wt` lands **inside `.git/`** (`<repo>/.git/wt`, created, rc 0). - **`worktree-add-claim-gate.sh`** (PostToolUse) returns rc 0 on the same command as well; it has no containment role, so nothing after the PreToolUse gate catches it. - **CR-only.** Quoting does not matter: unquoted, double-quoted and single-quoted CR words all allow in the hook and all create the directory in git. TAB is not in the class: an unquoted `..<TAB>/../outside/x` splits into two words for the shell and for the hook's tokenizer alike, and git then fails (rc 128, nothing created); a quoted `"..<TAB>/../outside/x"` stays one word, the hook does not strip TAB, resolves it lexically to `<repo>/sub/outside/x`, and **blocks**. Only CR is stripped before resolution, so only CR diverges from what git will do. - **Severity, the reviewer's read: low.** The gate is documented best-effort (the header declares fail-open on anything it cannot resolve statically) and already allows any `$VAR`-carrying target, so `git worktree add $PWD/x` reaches the same place with no CR at all. The CR shape grants nothing that `$PWD/x` does not. It is still a real fail-open of the class #3871 named, in shipped code, and the fix belongs with whoever owns the strip (strip CR only from the ends of the word, or resolve the un-stripped word and compare). ### New test `plugins/source-control/hooks/worktree-gates-spawn-budget.test.sh` (15 assertions) holds the counts as ceilings. It uses **strace, not xtrace and not a PATH shim**: both are blind to a fork that never execs. Upper bounds rather than equalities, so a later library change that removes more work does not fail it. **The ceilings cannot see a here-string regression**, because a here-string *lowers* the count; the suite's header says so, and three new cases grep each gate for `<<<` on `$INPUT` or `$payload` so that regression fails the suite anyway. It skips as a suite where `strace` is absent or cannot ptrace, rather than asserting on empty trace output. **Proven non-vacuous against seven mutants**, one per change, each reverted alone in a scratch copy and re-run after the review fix. A restored `tr` stage costs exactly +1 creation and +1 `execve` per field, which the exact ceilings catch: | Mutant | Suite | Cases it fails | | --- | --- | --- | | control (no mutation) | pass | none | | M1 containment `COMMAND` read, `tr` restored | fail | all 3 containment cases | | M2 containment `HOOK_CWD` read, `tr` restored | fail | 2 containment cases | | M3 containment `git_unlocated` | fail | 2 containment cases | | M4 containment `configured_root` | fail | the block case | | M5 claim's three field reads, `tr` restored | fail | both claim cases | | M6 claim `err_file` temp | fail | the parsed-target case | | M7 create `json_field_to` rungs (back to `hook::jq_field` + `sed \| head \| tr`) | fail | the create case | ### Gates | Gate | Result | | --- | --- | | `scripts/affected-tests.sh --run` | **exit 0**, all 4 selected suites pass; no unmapped file | | `check-changelog-parity.sh --check` | pass | | `check-changelog-parity.sh --check-order` | pass (91 changelogs, no duplicate heading) | | `check-changelog-parity.sh --check-bump origin/main` | pass | | `check-changelog-parity.sh --check-preserved origin/main` | pass (234 headings compared) | | `shellcheck -x` on all four scripts | clean | | `shfmt -d` (EditorConfig-driven) | clean | | `markdownlint-cli2` on README + CHANGELOG | 0 issues | | `check-killswitch-hoist.sh` | pass (31 hooks) | | `check-silent-skips.sh` | pass | | `check-discriminating-test-skips.sh` | pass | | `check-hook-exec-form.sh` | pass | | `check-fixture-git-isolation.sh` | pass (128 isolated) | ### Acceptance criteria, stated plainly | Criterion | Status | | --- | --- | | No more than 2 external spawns on the common path (own shell + at most one `jq`) | **Met for this hook's own share**: the gate execs exactly one `jq` of its own (3 creations, because the payload rides in on `printf \| jq` rather than a here-string, by library rule). **Not met literally**: a second `jq -e .` remains, and it belongs to `hook::json_complete` inside `lib/hook-utils.sh`, which is fenced off from this change (17 synced copies; unmerged #3740 and #3838 both target it). | | `grep`/`sed`/`cut`/`tr`/`basename`/`dirname` on the hot path replaced with builtins | **Met.** `tr`, `tail` and `head` are gone; `dirname` went in #3788. The one remaining `sed` is the create gate's jq-absent fallback rung, off the hot path, and is deliberately kept. | | Early exit before any spawn for non-matching invocations | **Already met on `main`** by the `if: Bash(*worktree*)` registration filter (#3621) plus each hook's own jq-free regex pre-filter. Unchanged here. | | Existing behavioural tests pass; the guard still blocks what it blocked | **Met.** 102 existing cases plus the A/B above. | | Under 2 s for a single run on a Windows host | **NOT VERIFIED.** No Windows host available. The budget doc's ceiling is stated as parallel wall time and this host's spawn cost is nothing like the campaign's, so a figure from here would mislead. Process creations are reported instead, which is the quantity that maps to the tax. | ### Reproduction ```bash strace -f -qq -e trace=clone,clone3,fork,vfork,execve -o t.txt \ bash plugins/source-control/hooks/worktree-add-containment-gate.sh <payload.json grep -cE '(clone3?|v?fork)\(' t.txt # creations grep -cE 'execve\(' t.txt # minus 1 for the traced program itself ``` ## Related - **#3508**: parent (Windows process-creation tax). Its stated cause, per-field `jq` forks needing a shared helper, is disproved again here: this PR touches `lib/hook-utils.sh` zero times. - **#1587** (`dfda6ec3`): the here-string deadlock trace this revision defers to; `lib/hook-utils.sh:1380` is the rule. - **#3779** (issue #3520): the precedent that first established the redirect-placement mechanism. - **#3788** (merged): fixed 34 scripts with zero `lib/hook-utils.sh` edits, and is where these three gates got their `${BASH_SOURCE[0]%/*}` form. - **#3871** (issue #3509): **sibling sharing this plugin's version chain**, and the source of the permissive-normalization class checked above. Its files (`pr-body-linkage-gate.sh`, `pr-linkage-mcp-gate.sh`, `pr-linkage-validator.sh`) are untouched here and its changelog entry is preserved verbatim. Its README hunk sits ~40 lines above this PR's, in the `pr-body-linkage-gate` section. - **#3838**: **overlaps this PR's files.** It edits all three worktree gates, replacing `INPUT=$(hook::buffer_stdin)` with the new `hook::buffer_stdin_to` form. The hunks do not overlap this PR's (its edits are on the `buffer_stdin` line; this PR's start below it), and a three-way merge was clean, but **the merge lane should expect a rebase rather than only a rebump**. The two changes are complementary: `buffer_stdin_to` removes one more creation from the same hot path, which is exactly why this PR's budget test asserts upper bounds rather than equalities. - **Version chain.** `main` is `0.55.58`; #3838 takes `0.55.59`; #3871 takes `0.55.59` and is being renumbered to `0.55.60`; **this PR takes `0.55.61`**, verified against `main` and every open `source-control` PR (#3774 and #3740 both claim versions at or below `main` and are stale, so they do not contend for 59 to 61). - **#3740**: unmerged `lib/hook-utils.sh` change; fenced off here, as is #3838's copy of it. - **#1403 / #1385**: prior art whose revival must first clear four contract-test regressions, two of which failed open. Not revived here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…l-commit (#3514) (#3886) Closes #3514 ## Summary `block-noncanonical-commit.sh` created processes that never exec'd, so every exec-counting census (PATH shim, `run-guards.test.sh`, an xtrace count) read it as free while a Windows host paid a full process creation for each. This PR removes the forks that belong to this file: the eager telemetry subject at file scope on every Bash and PowerShell call, the two command substitutions around builtins-only functions on the blocked multi-line commit path, one `$(printf '%q')` per trailing argument on a `!` alias reparse, and the `$(cd … && pwd)` plugin-root probe on the PowerShell lane. Verdicts are unchanged. The branch merges `main` at `1b681862` (#3878), which replaced the shared parser's `< <(printf …)` fork in `lib/hook-utils.sh` with a `${cmd:i:1}` walk. With that gone, this guard's own share on a benign Bash call is now **0 creations / 0 execve**, and every figure below is measured against that main. Guardrails is bumped to 0.32.15: `main` carries 0.32.14 (#3878), and the open PRs #3849, #3869 and #3872 hold 0.32.12 and 0.32.13. Scope is the one hook this issue names. `lib/hook-utils.sh` and its synced copies are untouched by this PR (the guardrails copy is byte-identical to `main`'s), as are the sibling guards (#3517, #3518, #3519, #3521). ## Fix In `plugins/guardrails/hooks/block-noncanonical-commit.sh`: - `SUBJECT=$(hook::extract_bash_subject …)` moves from file scope into `emit_tel`, behind the start-stamp and `hook::telemetry_enabled` gates. Only the envelope reads it, and the envelope is off by default. Same shape as `block-windows-drive-tmp.sh` and the sibling PRs #3869 and #3872. - `effective_dir` becomes `effective_dir_to <var> …` (nameref assignment) at its three call sites; `explicit_global` / `explicit_git_dir` become `explicit_global_to` / `explicit_git_dir_to`. Both were `$(…)` around builtins-only bodies on the blocked path. - The two `!` alias reparse loops quote trailing arguments with `printf -v` instead of `$(printf '%q' …)`. - The PowerShell lane sets `PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-$_HOOK_SELF/..}"`; `source` resolves that path to the same file the canonicalised spelling named, and nothing else reads `PLUGIN_ROOT`. One tradeoff, recorded in a comment: the kernel resolves `..` physically where `cd` resolved it logically, so a `hooks/` directory that is itself a symlink out of the plugin root needs `CLAUDE_PLUGIN_ROOT` set. Spaces and relative invocation are unaffected. Not changed, deliberately: `repo_git_probe`'s `$(git … 2>/dev/null; printf 'x')` still costs two creations for one exec. The compound body exists to keep a trailing newline in a repository path byte-exact (documented in the function), and neither hoisting the redirect nor `exec git` preserves that, so it stays. No matcher predicate was added ahead of the shared parser. A safe text pre-filter is constructible: the parser only removes or decodes characters, and decoding requires `$'`, so a predicate that admits any command containing `$'` or a `g…i…t` run with only quote, backslash and newline characters between the letters must hold for every word the parser can turn into `git` (the review verified zero fail-open across 32 DENY payloads, including `g''it`, `$'\x67it'`, `"git"`, `\git`, `g"i"t`, `g<LF>it`, `bash -c` / `-lc` / `sh -c` wrappers and an inline `!` alias). What it would buy is small: it skips only commands with no `git`-shaped substring at all, and the single fork it would have saved on those was the library parser's, which `main` has now removed. So criterion 3 of #3514 is moot rather than impossible, and this PR adds no predicate. ## Verification Everything in this section except the wall-clock paragraph was re-run **after** the merge of `main` at `1b681862`, on the merged tree at `5d3102b1`. The two arms are `git archive` exports: pristine = `origin/main` at `1b681862`, modified = `5d3102b1`. A content-hash comparison of the two exports (3767 files each) shows exactly five files differing, all under `plugins/guardrails/`: `.claude-plugin/plugin.json`, `CHANGELOG.md`, `README.md`, `hooks/block-noncanonical-commit.sh` and `hooks/block-noncanonical-commit.test.sh`. `lib/hook-utils.sh`, the guardrails `hooks/hook-utils.sh`, `hooks/run-guards.sh` and `lib/powershell/ps-command.sh` are byte-identical across the arms, and the pristine guard is byte-identical to the `1b681862` blob. **Kernel census** (`strace -f -e trace=clone,clone3,fork,vfork,execve`, dispatched through `run-guards.sh`, guard share = count minus a no-op guard dispatched the same way, this repository as cwd, `HOOK_TELEMETRY_SINK` unset, `CLAUDE_PROJECT_DIR` empty, three repeats each, identical every time; re-run post-merge on the two exports above, every figure reproduced). Against the pre-#3878 `main` (`c0fba152`) every creation count read one higher on both sides, the parser's. | Payload | creations before → after | execve before → after | |---|---|---| | Bash `git status --short` (benign) | 1 → 0 | 0 → 0 | | Bash `git commit -m 'feat: x'` (single-line, allowed) | 1 → 0 | 0 → 0 | | Bash multi-line `-m` (blocked, rc 2) | 5 → 2 | 1 → 1 | | Bash `git wibble` (persisted-alias probe) | 6 → 4 | 2 → 2 | | Bash inline `!` alias to multi-line `-m` (blocked) | 10 → 6 | 3 → 3 | | PowerShell `git status` | 14 → 12 | 3 → 3 | | Whole eight-guard Bash matcher, `git status --short` (absolute) | 22 → 21 | 2 → 2 | The execve column is unchanged everywhere, which is what makes this latency rather than removed work. A benign call now creates nothing in this guard; the remaining PowerShell creations are in `lib/powershell/ps-command.sh`. **Wall clock** (measured before the merge, against `c0fba152`, not re-run), Linux, `bash -c :` floor p50 3.0 ms / p95 7.2 ms, n=20 after 2 warmup, sides interleaved, guard alone under the dispatcher: benign p50 23.8 → 21.2 ms (p95 25.9 → 30.9 ms, noise at this scale); blocked p50 32.2 → 29.9 ms (p95 75.8 → 44.7 ms). The milliseconds are context; the process counts are the host-independent figure. **A/B differential** (re-run post-merge on the two exports above, in a fresh harness): 96 payloads (85 Bash, 9 PowerShell, 2 Write-tool) × 2 modes (standalone `bash <guard>` and dispatched `run-guards.sh --lib lib/powershell/ps-command.sh <guard>`) = 192 paired runs, plus 13 payloads × 2 modes = 26 paired runs under a PATH that carries no `git` at all (`command -v git` confirmed empty under that PATH; the blocked payloads in those runs still produce the full 452-byte stderr message, 429 bytes on the PowerShell lane, on both arms, so stderr is genuinely compared there rather than trivially empty) = **218 paired runs, 218 identical on exit code, stdout and stderr (byte compare with `cmp`)**. Verdicts: 50 deny / 46 allow per arm, identical on both arms in both modes; no payload the pristine arm denied is allowed by the modified arm. The corpus covers every `-m` spelling the guard matches (`-m`, `-am`, `-m<attached>`, `-qm`, `--message=`, `--message`, `--mess=`, `--m`, `$'…'`), the exempt forms (`--amend`, `--fixup`, `-F <path>`, `-F -` heredoc, repeated single-line `-m`, bare `git commit`, an in-progress merge with `MERGE_HEAD` via cwd and via `--git-dir=`), case variants (`GIT`, `Git`), `/usr/bin/git`, `git.exe`, `g''it`, `$'\x67it'`, `"git"`, `\git`, `g"i"t`, `git` split across a backslash-newline, `bash -c` / `bash -lc` / `sh -c` wrappers, `env`, `NAME=value`, `sudo` and `sudo FOO=1` prefixes, `cd sub && …`, `;`, `|`, `&&`, `||`, bare newline, CRLF, U+2028 in the message, a leading BOM, zero-width space inside `git` and inside the message, near-miss spellings that must not be caught (`gitt`, `mygit`, `git-commit`, a quoted string mention, a comment), `eval`, variable indirection, backtick, inline `-c alias.*` (git alias, `!` shell alias, `!` alias with trailing arguments), persisted aliases (multi-line commit, single-line, `!` shell, two-hop chain to `commit -F -`, undefined `git wibble`), `-C .`, `-C sub`, `--git-dir=`, repeated `--git-dir`, `--work-tree=`, `--trailer`, `--no-verify`, literal `\n`, `$(printf …)` message, `nohup … &`, tab separators, a `grep -m` false friend, a 70 KiB benign command, a 20 KiB multi-line message, an empty command, a control character in the message, an empty `cwd`, nine PowerShell forms (status, backtick-n, real newline, here-string `-m`, here-string piped to `-F -`, `&` call operator, `Invoke-Expression`, single-line, near-miss `gitt`), and two Write-tool payloads. Additionally 11 telemetry envelopes captured through a stub sink (`HOOK_TELEMETRY_SINK` set; the sink is fire-and-forget, so the capture waits for its write) agree on `status`, `tool`, `subject` and `form` between the two arms, across benign, blocked, inline `!` alias, persisted-alias probe, empty-`cwd` and PowerShell benign and blocked payloads; `subject` is the field the lazy `SUBJECT` change touches. No payload is skipped by any new predicate, because none was added. **Contract suite** on the merged tree: `block-noncanonical-commit.test.sh` PASS=227 FAIL=0 (216 on pristine main, plus 11 new assertions). The new section pins, by the same strace instrument: benign share exactly 0 creations / 0 execve; single-line `-m` equal to benign; blocked multi-line `-m` exactly +2 creations / 1 execve over benign; trailing `!` alias arguments add zero creations. It skips visibly where strace is absent. **Mutation checks** (re-run post-merge on `5d3102b1`; apply, run the suite, revert, worktree verified clean after each): restoring the eager file-scope `SUBJECT` fails the benign pin (expected 0, got 1); putting the explicit `--git-dir` back through a `$(…)` on the block path fails the blocked delta (expected 2, got 3); restoring a `$(printf '%q')` per trailing alias argument fails the trailing-args pin (expected 4, got 7, three trailing arguments). Each mutant reads PASS=226 FAIL=1 with only its named pin failing. The deltas do not depend on the library parser. All three revert clean. **Gates** on the merged tree (`5d3102b1`, re-run post-merge): `bash scripts/affected-tests.sh --run` selected 7 shell suites (`resolve-convention-pattern`, `block-convention-violation`, `block-noncanonical-commit`, `flag-commit-pr-skill-bypass`, `require-jq-posture`, `run-guards`, `commit-msg-convention`), "All 7 selected suites passed or were skipped", exit 0. `plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh` is not in that selection and fails two process-budget assertions on the clean `1b681862` export as well, so it is pre-existing and unrelated. `check-changelog-parity.sh` `--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main` all exit 0 against `main` at `1b681862`. `sync-plugin-options-docs.py --check` up to date. ShellCheck clean on the guard and its test. No em dash in any added line. **Unmet, stated plainly**: acceptance criterion 5, "measured on a Windows host, under 2 s", is not met by this PR. Everything above was measured on Linux. The substitute evidence is the host-independent process-creation and execve census (the Windows cost is a per-spawn multiplier on those counts) and Linux wall clock against a `bash -c :` floor. Criterion 1's "at most 2 spawns" is met on the common path on both counters (0 creations, 0 execve) once #3878 is in the base. ## Related - Refs #3508 (parent: Windows process-creation tax) - Refs #3878 (merged into this branch at `1b681862`; removed the shared parser fork that held the benign figure at 1) - Refs #3740, #3838 (`lib/hook-utils.sh` fork-free forms) - Refs #3869, #3872, #3849 (sibling guardrails spawn PRs; same `emit_tel` shape, and the version numbers they hold) - Refs #1403 / #1385 (prior spawn-reduction art) - Refs `docs/conventions/hook-budget/README.md` (budget accounting entry added to the guardrails README) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --------- Co-authored-by: Kyle Sexton <ksextonclaude@outlook.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…3513) On a benign Bash call the guard created seven processes and executed none, so every exec census (PATH-shim spawn census, the dispatcher test's dirname/sed pin, an xtrace command count) read it as free. Six were in this file: 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), whose fork-free form is lib/hook-utils.sh work (#3740, #3838) and is not touched here. 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. Kernel census, strace -f -e trace=clone,clone3,fork,vfork,execve on the dispatched path, HOOK_TELEMETRY_SINK unset, guard share = count minus a no-op guard dispatched the same way: benign git status creations 7 -> 1, execve 0 -> 0; blocked echo > file creations 10 -> 5, execve 1 -> 1; whole Bash dispatcher 36 -> 30 creations, execve 3 -> 3. Verdicts unchanged across 244 paired runs against origin/main and the 602-case contract suite. The suite now pins the benign share at exactly 1 with the same instrument; each of the four changes reverted alone fails that pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
Merging main brought in #3838, which landed the fork-free hook::buffer_stdin_to in lib/hook-utils.sh. This guard now calls it, so the seventh process creation this PR documented as remaining is gone too. The contract suite pinned the benign share at exactly 1 and its own comment said the pin moves to 0 when that work lands. It has landed, so: - the strace pin is now 0, and its comment records why; - the changelog and README rows that named $(hook::buffer_stdin) as the remaining creation now say all seven are gone; - the measured figures are corrected against the merged tree: benign guard share 7 -> 0 creations, blocked 10 -> 4, stable across three repeats under the same instrument the table documents. The whole-dispatcher and wall-clock rows still describe the pre-#3838 base, which the README now states rather than restating numbers not measured on that host. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
…3870) Closes #3515 > [!IMPORTANT] > **Which of #3515's acceptance criteria this PR meets, and which it does not.** > > | Criterion | Status | > |---|---| > | No more than 2 external process spawns on the common path (own shell + at most one `jq`) | **Met for the common path**, which is every interactive Stop: the hook's shell plus one `uname -s`. The one spawn is `uname`, not `jq`; it is the managed-settings platform primitive the trust design rests on (`$OSTYPE` is a variable a repo env block can set) and was left alone on purpose. **Not met for an enabled lane's stop**: that path launches 5 programs (4 `jq`, 1 `uname`), down from 18. One of those `jq` is in the fenced shared library's `hook::buffer_stdin` (its payload validation pass); the other three read three different inputs (payload, settings file, block decision). | > | `grep`/`sed`/`cut`/`tr`/`basename`/`dirname` replaced with builtins on the hot path | **Met.** None launches on either traced path; the suite pins their absence. `cksum` remains on the marker-consumption path only (it keys the ledger, and changing the key would orphan existing ledger entries). | > | An early guard exits before any spawn for non-matching invocations | **Partially met.** The payload-free pre-filter exits before stdin and before `jq`, but it spawns `uname` to know which fixed managed-settings path to test. Removing it would mean testing all three platform paths, one of which is cwd-relative on Linux, so it stays. | > | Existing behavioural tests pass; the guard blocks what it blocked before | **Met**, with one disclosed divergence in the stricter direction (below). | > | Measured on a Windows host, a single run completes in under 2 s | **Not measured.** This runner is Linux; wall-clock is not the right proxy here and no figure is invented. The hook-budget convention's per-turn 500 ms bar (the corrected criterion from the issue comments) also still needs the Windows measurement. | > > `Closes #3515` is kept because the `pr-issue-linkage` gate wants a native closing keyword. A reviewer may prefer to reopen #3515 on merge for the Windows measurement and the enabled-lane count, or split those into a follow-up. ## Summary `hooks/lane-stop-gate.sh` is the autonomy plugin's `Stop` hook. It fires on every turn of every session, gated or not, and #3515 recorded it at a 27.6 s average with 73 timeouts against its 15 s budget on the #3508 hosts. It carried the largest in-file fork signature of the campaign: about 12 redirections and 11 pipes written inside command substitutions, plus a `$( )` capture around every lib helper that is nothing but parameter expansion. Following #3779's diagnosis rather than #3508's original framing: the cost was redirection placement and per-helper subshells, not per-field `jq` needing a shared helper. `lib/hook-utils.sh` and all 17 `plugins/*/hooks/hook-utils.sh` copies are untouched (`git diff origin/main --name-only` shows only the six autonomy files). The one shared-library facility used, `hook::jq_fields`, already existed on `main`. ## Fix All in `plugins/autonomy/hooks/lane-stop-gate.sh` and `lane-stop-gate-lib.sh`: | Site | Before | After | |---|---|---| | Lib path helpers (`gate_data_dir`, `gate_trusted_data_dir`, `gate_arm_record_path`, `gate_arm_claim_path`, `gate_user_settings_file`) | `v=$(gate_x)`: one fork each, for a function that only expands parameters | `gate_x_to <var>` forms via `printf -v`; print forms delegate, so `lane-stop-gate-arm.sh` and the lib-level tests read exactly what they read before | | Pre-filter settings scan | `grep -q lane_stop_gate "$f" 2>/dev/null` per file | builtin NUL-chunk `read` + substring test (`gate_file_mentions`), with `2>/dev/null` written **before** the input redirection so an existing-but-unreadable settings file stays as silent as `grep` was (bash applies redirections left to right; see case 51) | | Managed-files list | `done < <(gate_managed_settings_files)` with `$(uname -s 2>/dev/null)` inside: 3 creations per call, called up to 4 times per stop | `gate_managed_settings_files_load` fills an array in-process; `{ platform=$(uname -s); } 2>/dev/null` is 1 creation; loaded once per stop and reused by option resolution | | Payload fields (`EVENT`, `SESSION_ID`, `CWD`, `STOP_ACTIVE`, `LAST`) | five `printf \| jq \| tr` pipelines, 3-4 creations each | one `hook::jq_fields` pass (3 creations: process substitution, printf writer, jq); values chomped of trailing newlines so each reads byte-for-byte as the old `$( )` capture did | | Settings options (3 keys x managed files + user file) | one `$(jq … <file 2>/dev/null)` per key per file, each behind two more captures (8 creations per key) | `gate_settings_options_to <file> <key>...`: one jq per file for every key, NUL-separated through a process substitution with the redirections on the enclosing group (1 creation per file); resolved once, answered from memory (`gate_option_to`) | | Arm record | `jq -ec .` validation + `printf \| jq` per field + `printf \| jq` per option: 8 creations plus 6 on lookup | one jq pass over the record file yielding armed_at, session_id, sentinel, marker, compact json (1 creation); `EPOCHSECONDS` for the TTL clock with `date` as the pre-5.0 fallback | | Sentinel escape and match | `$(printf \| sed …)` then `grep -qE … <<<"$LAST"` | shell loop over the same fifteen metacharacters; `[[ $LAST =~ (^\|\n)[[:space:]]*TOKEN[[:space:]]*(\n\|$) ]]`, which agrees with grep's per-line verdict on every message (argument in the code comment) | | Telemetry data object | `$(jq -nc --arg … 2>/dev/null)` on every evaluated stop | assembled in the shell from the closed vocabulary: identical bytes | | Marker ledger | `$(printf \| cksum \| tr -cd '0-9')`, `$(stat … \|\| stat … \|\| printf '')`, `$(dirname …)` | `{ key=$(cksum); } < <(printf '%s' "$path")` with a parameter-expansion digit filter (same key as before, verified), per-rung `{ …; } 2>/dev/null` stat groups, `${ledger%/*}` | | Post-nudge branch read | `$(git … 2>/dev/null \| tr -d '\000-\037')` | `{ BRANCH=$(git …); } 2>/dev/null` + `${BRANCH//[[:cntrl:]]/}` (a superset strip; git refuses every such byte in a ref name, and `lane::notify` strips C0 again) | | `gate_resolve_plugin_name` (unanchored installs) | `$(jq … <"$manifest" 2>/dev/null)` | group-hoisted redirections | Left alone on purpose: `uname -s` as the platform primitive; `hook::buffer_stdin` (shared library; its `$( )` capture, read-slice probe and `printf | jq -e` validation are 4 of the enabled path's remaining 10 creations); the `gate_arm_owned` subshell (one fork, and the cleanest scope for `umask`/`noclobber`); the final `jq -nc` block decision. **Disclosed divergence (one degenerate config):** a configured sentinel that itself holds a newline. `grep -E` read that newline as a pattern separator and authorized a stop on any line matching either half of the token. The shell match treats the token as one pattern, so only the whole token standing alone authorizes, which is what the block reason instructs the agent to emit. No shipped launcher writes such a token. Pinned by new case 50 and named in the 0.22.30 changelog entry. Group redirections were checked for over-suppression: every `{ …; } 2>/dev/null` here wraps exactly one command, and the two process-substitution loops wrap a `read` builtin (no stderr) plus the one jq whose stderr the old code already suppressed. ## Verification **Before/after, measured here** (`strace -f -e trace=clone,clone3,fork,vfork,execve`, staged `plugins/cache/<m>/<n>/<v>/hooks` install, hook launched by the harness so its own shell is not in the count): | Path | Creations before | After | Launches (`execve`) before | After | |---|---|---|---|---| | Default: no gate footprint anywhere (every interactive `Stop`) | 4 | **1** | 2 (`grep`, `uname`) | **1** (`uname`) | | Enabled by user settings, first stop, no signal (block) | 48 | **10** | 18 | **5** (`jq`, `jq`, `uname`, `jq`, `jq`) | | Enabled, sentinel on its own line (allow) | 44 | 9 | 16 | 4 | | Enabled, second stop after the nudge (allow, notify muted) | 54 | 10 | 21 | 5 | | Enabled, marker present (allow, consume) | 51 | 12 | 19 | 6 | | Env-only enable claim (once-per-session notice) | 31 | 22 | 11 | 8 (remainder is `hook::notice_once` in the shared library) | | Armed by the launcher, first stop (block) | 60 | 11 | 20 | 5 | Launches did not stay flat, and that is expected here: unlike #3779, this issue explicitly asks for the `grep`/`sed`/`tr`/`dirname` helpers to become builtins, so those launches are gone; every `jq` that reads a distinct input is still launched, and the payload/settings/arm-record `jq` calls that were batched read the same inputs once instead of several times. **Wall-clock is not measured and no figure is invented.** A spawn is about 1 ms on this Linux runner; #3508 measures 180 to 2,841 ms per spawn (median 1,108 ms at 501 concurrent processes) on the affected Windows hosts. Process creations by trace are the drift-immune proxy #3508's corrected criteria ask for. The plugin README now carries the table above under a "Hook cost" heading, per hook-budget Rule 1, and says the Windows wall-clock share still needs measuring. **Proof the verdicts did not change.** - All 89 pre-existing assertions in `lane-stop-gate.test.sh` pass unmodified; the suite is now 103 with the additions below (102 where `chmod 000` denies nothing and case 51 skips visibly). - A black-box differential harness (scratch, not committed) ran the `origin/main` hook and this branch's hook over 95 scenarios from identical fresh staged installs, comparing rc, stdout, and marker/ledger side effects: every settings shape the reader distinguishes (boolean/string/number/null values, `options` as string/array, `pluginConfigs` as array/string, null entry, other marketplace, malformed, two documents), sentinel edge cases (CRLF, tabs, blank lines, inline mention, substring, trailing text, 120 KB messages with early and no sentinel, invalid UTF-8 around the token, metacharacter sentinels, empty sentinel), marker absolute/relative/consumed/stale-ledger/undeletable, env-only claims, and the arm record's every parse and claim path (null/false/number/string/empty/malformed, missing or non-numeric or float `armed_at`, expired, legacy `session_id` match/mismatch/numeric, taken/empty/directory claim files, missing session id). **93 identical; the 2 differences are the two halves of the disclosed newline-sentinel case.** - The ledger key derivation and the sentinel escape were also compared directly against the old pipelines on sample inputs: identical. **New regression tests, verified non-vacuous.** - Case 49: `gate_settings_options_to` answers three keys from one pass with the single-key verdicts (including the trailing-newline chomp and the all-or-nothing `options`-not-an-object case). - Case 50: a newline-bearing sentinel authorizes only as a whole block (the disclosed divergence, pinned). - Case 51: an existing-but-unreadable settings file produces **no stderr** and does not change the verdict. Pins the redirection order in `gate_file_mentions` (commit `fd0e7578`): the first draft wrote `done <"$1" 2>/dev/null`, which attempts the open before stderr is silenced and prints `Permission denied` per turn where `grep -q … 2>/dev/null` was silent. Run as an unprivileged user the case passes on the fixed tree and fails with that exact line on the old ordering; where `chmod 000` denies nothing (root, or a filesystem without POSIX modes) it skips visibly rather than passing vacuously. - Trace budget: exactly 1 creation and 1 launch (`uname`) on the default path; ceilings of 10 creations and 5 launches on the enabled block path (a ceiling so a shared-library saving in #3740/#3838 lowers the count without failing here, while a regression in this plugin's files raises it and does); and no `dirname`, `tr`, `sed`, `grep`, `cksum` or `date` on either path. Skips where `strace` is unavailable; the CI Linux lane does not skip. Re-measured rather than assumed after the `fd0e7578` redirection swap: both pins hold, and the enabled path's four `jq` are still `jq -e .` (`hook::buffer_stdin`), the `jq -j` payload pass, the `jq -j` settings pass and the `jq -nc` block decision, in that order with `uname` third. - Mutation 1 (move the `uname` redirection back inside its substitution): `FAIL: default path creates 2 processes, budget is 1` and `FAIL: enabled block path creates 11 processes, ceiling is 10`. Mutation 2 (put the `sed` escape pipeline back): 13 creations, 6 launches, and the named-helper check all fail. Both on scratch copies; the committed tree passes. **Commands run in the foreground on the pushed head `fd0e7578`, with actual results** (the `lane-notify` / lane-launcher / `check-shell-portability` lines are from the `1ca98cbf` run; the second commit touches neither those files nor their inputs, and the full `affected-tests.sh --run` below was re-run on `fd0e7578`): ``` bash plugins/autonomy/hooks/lane-stop-gate.test.sh -> PASS=103 FAIL=0 (unprivileged) -> PASS=102 FAIL=0 as root, case 51 skips visibly bash plugins/autonomy/hooks/lane-notify.test.sh -> rc=0 bash plugins/claude-ops/skills/lanes/scripts/lane-launcher.test.sh -> rc=0 (calls lane-stop-gate-arm.sh) bash plugins/claude-ops/skills/lanes/scripts/restart-consumer.test.sh -> rc=0 bash scripts/affected-tests.sh --explain -> the R4 transitive walk from lane-stop-gate.sh fans out to the whole shell corpus (170 suites); plugin.json, CHANGELOG.md, README.md recorded as no-suite (non-shell lanes) bash scripts/affected-tests.sh --run (foreground) -> rc=1: one failing suite, plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh ("process budget: the trace probe actually counted something ... measured -1, the pid-stamped PS4 did not reach the traced shell"). Reproduced identically on a pristine `git archive origin/main` export: pre-existing and environmental, not from this change. Every other selected shell suite passed. shellcheck <3 changed .sh files> -> clean shfmt -d <3 changed .sh files> -> clean bash scripts/check-shell-portability.sh origin/main -> No unexcused GNU-only constructs in 3 shell file(s). node_modules/.bin/markdownlint-cli2 README.md CHANGELOG.md -> 0 issues typos <5 changed files> -> clean scripts/check-hook-exec-form.sh, check-killswitch-hoist.sh, check-silent-skips.sh, check-discriminating-test-skips.sh, check-hook-wiring-liveness.sh, check-purged-em-dashes.sh -> all rc=0 bash scripts/check-changelog-parity.sh --check -> rc=0 bash scripts/check-changelog-parity.sh --check-order -> rc=0 bash scripts/check-changelog-parity.sh --check-bump origin/main -> rc=0 bash scripts/check-changelog-parity.sh --check-preserved origin/main -> rc=0 (78 headings compared) ``` Manifest bumped 0.22.29 to 0.22.30 with a matching changelog entry. Not mine and pre-existing: `test_save_point.py::test_new_origin_falls_back_to_directory_name` (Python, in the NOT RUN ecosystem list) and the `cache-content-check.test.sh` trace-probe failure above. ## Related - Parent campaign: #3508 (Windows process-creation tax). Its corrected criteria ask for spawn counts before and after by trace, and the hook-budget bar; both are reported above. - Precedent for the in-file approach: #3779 (context-guard shard #3520), which located the cost in redirection placement rather than per-field `jq`, and merged PR #3788, which applied it across 34 hooks without touching `lib/hook-utils.sh`. - #3740 and #3838 own `lib/hook-utils.sh` and its 17 synced copies; this PR does not touch them. The enabled path's remaining shared-library share (4 of 10 creations) is theirs; the trace test's ceiling leaves it room. - Prior art #1403 / #1385 (`hook::jq_fields`, `strip_quoted_spans`, the 65-spawn census). `hook::jq_fields` is used here as it already exists on `main`; nothing from #1385 is revived. - Budget authority: `docs/conventions/hook-budget/README.md`, surfaced by `.claude/rules/hook-budget.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…it (#3529) On every Bash and PowerShell call the guard created three processes of its own and executed none, so the PATH-shim census read it as free. One of the three was this file's: an eager SUBJECT=$(hook::extract_bash_subject) at file scope feeding a telemetry envelope that is off by default and that the verdict never reads. It is now derived inside emit_tel, behind the start-stamp and sink gates. Off the common path, three more sites paid a fork for a string: - the hash-width probe $(git ... rev-parse --show-object-format 2>&1) cost two creations for one exec, because bash execs a substitution's body in its own subshell only when that body carries no redirection. The 2>&1 has to stay inside (git's stderr is the diagnostic the block message quotes), so the body is now `exec git ...`; - a `!` alias reparse spent one $(printf '%q') per trailing argument, now printf -v; - its $(effective_dir ...) around a builtins-only function is now effective_dir_to, a nameref assignment. Kernel census (strace -f -e trace=clone,clone3,fork,vfork,execve, guard share = dispatched minus a no-op guard dispatched the same way): benign and blocked Bash calls 3 -> 2 creations, execve 0 -> 0; lease with a full-width oid 5 -> 3, execve 1 -> 1; `!` alias with three trailing args 8 -> 3; whole Bash dispatcher 35 -> 34, execve 3 -> 3. The two creations left are $(hook::buffer_stdin) and the shared parser's < <(printf ...), both lib/hook-utils.sh work (#3740, #3838), and the PowerShell lane's fourteen live in lib/powershell/ps-command.sh. Verdicts are unchanged: 190 paired runs against origin/main (87 Bash and 8 PowerShell commands, standalone and dispatched, three repository shapes, CR/BOM/zero-width/U+2028 variants) agree on exit code and full stderr; the contract suite passes at 492 and now pins each site by the same instrument, each pin checked by reverting its change alone. Guardrails 0.32.13 (0.32.11 and 0.32.12 are taken by #3849 and #3869). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
origin/main already landed hook::buffer_stdin_to and the parser's fork-free line split, so the two remaining creations this pin named are gone. Move the benign share from 2 to 0 and the !-alias reparse from +1 to +0. The hash-width probe is still one creation over benign. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…3513) (#3869) Closes #3513 ## Summary `block-hook-bypass.sh` was the largest process creator of any guard in the plugin on a benign Bash call: 7 creations, 0 execs. Every exec-based census (the PATH-shim spawn census, `run-guards.test.sh`'s dirname/sed pin, an xtrace command-position count) reads a fork-without-exec as free, which is why the issue's numbers and the "per-field jq" cause never matched what the kernel does. Six of the seven were in this file; the seventh is `$(hook::buffer_stdin)`. Measured with `strace -f -e trace=clone,clone3,fork,vfork,execve` on the real dispatched path (`run-guards.sh block-hook-bypass.sh`), `HOOK_TELEMETRY_SINK` unset, guard share = count minus a no-op guard dispatched the same way, three identical repeats: | Path | creations before -> after | execve before -> after | |---|---|---| | Guard share, benign `git status --short` | 7 -> 1 | 0 -> 0 | | Guard share, blocked `echo hi > notes.md` | 10 -> 5 | 1 -> 1 | | Whole Bash dispatcher, benign | 36 -> 30 | 3 -> 3 | | Guard alone under the dispatcher, wall p50 / p95 (n=20, interleaved) | 27.4 / 29.2 ms -> 24.3 / 27.5 ms | | | Whole Bash dispatcher, wall p50 / p95 (n=20, interleaved) | 51.6 / 60.4 ms -> 48.2 / 49.6 ms | | The execve column does not move: this is latency, not removed work. **On the issue's "at most two spawns" line.** The one creation left in the guard's own share is `$(hook::buffer_stdin)`. Its fork-free form (`hook::buffer_stdin_to <var>`) belongs to `lib/hook-utils.sh`, which is fenced by unmerged #3740 and #3838, so it is not touched here. Whether #3513 counts as met depends on what one counts: the guard's in-file contribution is now 1 creation and 0 execs, and the dispatcher's own `$(source …)` isolation fork is the dispatcher's, not this file's. `Closes` here means the in-file work is done; the remaining fork is the hook-utils item. ## Fix Five in-file creations removed, one left in place: 1. `:159` eager `SUBJECT=$(hook::extract_bash_subject …)` at file scope. Only `emit_tel` read it, and `emit_tel` is gated on the start stamp and the opt-in sink. The subject is now derived inside `emit_tel` behind both gates. Verdicts never read it. 2. `:478` `EXECUTABLE=$(strip_literals "$COMMAND")`. Now `strip_literals_to EXECUTABLE "$COMMAND"`, assigning through a nameref; every trailing newline is stripped, as `$(…)` stripped them. 3. `:474`, `:1305`, `:1330`, `:1403` the four `done < <(printf '%s\n' …)` loops. One fork-free splitter, `split_lines_to`, does a sentinel-prefixed `IFS=$'\n'` word split under `set -f` (globbing restored to whatever the caller had). The sentinel prefix is what keeps blank lines and runs of newlines from collapsing, and a leading or trailing empty line from disappearing, so the array is exactly what `read` delivered, which `strip_literals` needs to keep its heredoc and open-quote state aligned with physical lines. `NORMALIZED_SEGMENTS` becomes an array filled once in `normalize_segments`; the three per-segment scans iterate it. `return` and `continue 2` inside those loops reach the same scopes as before because neither loop shape runs its body in a subshell. A here-string was not an option: at 65536-65663 bytes bash blocks forever writing it into the pipe (documented in `lib/path-detection/hardcoded-path-patterns.sh`). 4. `:112` `INPUT=$(hook::buffer_stdin)` is left as is. It is the fenced hook-utils item, and #3740 modifies this exact rc-handling block; the diff stays off it so the two do not fight at merge. Manifest 0.32.10 -> **0.32.12**, CHANGELOG entry, README "Hook budget accounting" entry with the method and table above. 0.32.11 is taken by #3849 (issue #3511), which bumps the same manifest off the same 0.32.10 base; this branch takes the next free number so the two do not collide on `plugins/guardrails/CHANGELOG.md`. Two documentation corrections carried in the same renumber commit: - The CHANGELOG entry and the new README row said "the 602-case contract suite passes". With the pins this change set adds the suite is 611 cases, which is what it reports. - The comment moved onto `emit_tel` carried two em dashes over from the file-scope comment it replaced. `.claude/rules/vendor-docs-are-not-style.md` bars them from this repo's instruction surfaces, and `check-purged-em-dashes.sh` scans markdown only, so nothing caught them. They are parentheses now. No other comment is touched, and no guard logic changed. ## Verification - **Deny paths still deny, compared against `origin/main`.** 244 paired runs (61 commands x Bash/PowerShell payloads x standalone/dispatched, plus a 70 KiB single-line command and a 3000-line command on each side) agree on exit code and first stderr line. The corpus covers every documented deny form (cat/echo/printf redirects, no-space forms, heredoc openers with a trailing redirect, `2>&1`/`>&2` before the file, prefix modifiers `command`/`exec --`/`FOO=bar`, group and `if` headers, leading redirects, discard-then-real-file `> /dev/null > real.txt`, python write indicators, staged `mv`/`cp`, quoted operands with embedded separators, `\;` and backslash-newline escapes, `ec""ho` / `ec"xy"ho` splices) and the allow forms (dup-only redirects, producer-scoped `bash x.sh > out && echo done`, quoted prose in commit and PR bodies, `command -v`, comments, blank-line-only and newline-padded commands, glob characters). - `split_lines_to` checked directly against `while IFS= read -r` over `printf '%s\n'` on: empty, single line, trailing newline, leading newline, runs of blank lines, whitespace-only lines, glob characters, the sentinel byte itself at line starts and ends, IFS characters, trailing backslashes, UTF-8, a 70 KiB line, 3001 lines. Identical arrays in every case; `set -f` preserved when the caller had it and restored off when it did not. - **New strace-based budget test** in `block-hook-bypass.test.sh`: pins the guard's benign share at exactly 1 creation and 0 execve on two benign payloads (single segment; multi-line, multi-segment), and that `echo > file` still exits 2 under the tracer. Skips visibly on a host without a working strace. It runs the real dispatched path, not the standalone guard, and subtracts a no-op guard so dispatcher forks do not leak into the pin. **Mutation-checked**: unmutated control 9/9; each of the four changes reverted alone (eager SUBJECT restored; splitter through `mapfile < <(…)`; strip through `$(…)`; segments through `mapfile < <(…)`) fails the pin on both payloads. - **The census was re-taken after the renumber commit**, since a documentation-only change must not move it, and it does not: whole Bash dispatcher on the benign payload reads 36 creations at `origin/main` against 30 on this branch, 3/3 identical repeats, execve flat at 3 on both; the blocked-path guard share is 5 creations and 1 execve; the contract suite's exact benign-share pin (1 creation, 0 execve) passes. - `scripts/affected-tests.sh --run`: 5 suites selected (`block-hook-bypass.test.sh` 611/611, `flag-commit-pr-skill-bypass.test.sh`, `require-jq-posture.test.sh`, `run-guards.test.sh`, `scripts/check-shell-portability.test.sh`), all passed, exit 0; the manifest, CHANGELOG and README map to the recorded no-suite allowlist. - `scripts/check-changelog-parity.sh` **all four modes** the script defines, on the committed tree: `--check` (pass), `--check-order` (pass, 91 changelogs read newest-first with no duplicate versions), `--check-bump origin/main` (pass), `--check-preserved origin/main` (pass, 155 headings preserved). `--check-order` is the mode that catches a doubled `## [<version>]` after a bad conflict resolution, which is the failure the renumber exists to avoid; an earlier revision of this PR ran only the other three. - `shellcheck -x` clean on the guard and the test; `shfmt -d` no diff; markdownlint-cli2 0 issues; editorconfig-checker clean; `check-purged-em-dashes.sh` clean; ai-slop detector 0 findings on the README and CHANGELOG. - Pre-existing and not touched: `test_save_point.py::test_new_origin_falls_back_to_directory_name`. ## Related - #3508 parent campaign. Its stated cause (per-field `jq` forks needing a shared helper) does not describe this guard: since #3788 the dispatcher primes the fields once, and this guard contributed 0 execs before this change. - #3779 precedent (#3520): an xtrace command-position count read 2 where the kernel made 8; the same instrument gap is why the shim-based budget test could not see any of these seven. - #3740 adjacent: modifies this script's `buffer_stdin` rc handling. This diff stays off that block. #3838 is the other fence on `lib/hook-utils.sh`. - #3849 (issue #3511) holds guardrails 0.32.11; this PR now takes 0.32.12, so the two no longer collide. - Credit note for the record: this file's `dirname` exec died in `a5f5e799` (#3781); #3788 never touched this file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…stitutions (#3849) Closes #3511 ## Summary `plugins/guardrails/hooks/block-convention-violation.sh` carried three `2>/dev/null` redirects **inside** command substitutions. GNU Bash execs the body of a command substitution in the substitution's own subshell, instead of forking a second time, only when that body carries no redirection of its own (Command Substitution, Bash Reference Manual; https://mywiki.wooledge.org/CommandSubstitution). So `v=$(cmd 2>/dev/null)` costs two process creations to run one program where `{ v=$(cmd); } 2>/dev/null` costs one. On the Windows Git Bash hosts #3508 measures, a fork is a full process creation at 0.3-0.9 s. **The parent's stated cause does not apply to this file.** #3508 attributes the cost to per-field `jq` forks needing a shared helper. This guard already batches all three payload fields through `hook::jq_fields`, already derives its own directory with `${BASH_SOURCE[0]%/*}` instead of `$(dirname …)`, and since #3788 is `source`d in-process by `run-guards.sh`, which buffers stdin and primes the jq fields once for the whole batch. What was left is redirection placement, the same cause shard #3520 established in PR #3779 and PR #3788 fixed across 34 scripts without touching `lib/hook-utils.sh`. This PR touches no shared library and no dispatcher: the change is in-file. ## Fix Three redirects hoisted onto single-command groups: | Site | Reached on | |---|---| | `bash "$RESOLVER" …` convention resolver (two calls) | first commit or `gh pr create` after a cache miss | | `git rev-parse --absolute-git-dir` sequencer probe | every stdin-form commit | | `git config --get alias.<sub>` alias probe | every non-builtin git subcommand, so this one is on the per-tool-call path | Each group holds **exactly one command**, so the group's status is still that command's. That matters here specifically because this is a *gate*: hoisting `2>/dev/null` over a multi-command group can swallow a non-zero status and turn a block into a silent allow. `|| conv_val=""` and `|| return 1` fire on exactly the failures they fired on before. ## Verification **Process counts, from the kernel** (`strace -f -e trace=clone,clone3,fork,vfork,execve`, `HOOK_TELEMETRY_SINK` unset). A PATH shim cannot see a fork that never execs, and `bash -x` prints one line whether a command costs one process or two, so neither existing counter in this repo can measure this. Guard invoked directly: | Scenario | creations before | after | execve before | after | |---|---|---|---|---| | `echo hello` | 11 | 11 | 3 | 3 | | `git status --short` | 11 | 11 | 3 | 3 | | `git wibble --x` (alias probe) | 14 | **13** | 4 | 4 | | stdin-form commit, cold cache | 34 | **31** | 16 | 16 | | stdin-form commit, warm cache | 19 | **18** | 5 | 5 | Guard's incremental cost inside `run-guards.sh`, the way it actually runs (full Bash matcher guard list, measured with and without this guard in the list, warm convention cache): | Scenario | guard's added creations before | after | guard's added execve | |---|---|---|---| | `git status --short` | 3 | 3 | 0 | | `git wibble --x` | 6 | **5** | 1 | | stdin-form commit | 11 | **10** | 2 | `execve` is unchanged everywhere, which is the evidence this removes latency rather than removing work. Wall-clock is not the claim: this Linux host's spawn floor is nothing like the contended Windows host in #3508, and inventing a millisecond figure from it would mislead. **Deny paths still deny.** All three touched sites were exercised against a fixture repo carrying a tracked `subject_pattern`: - violating commit subject blocked (exit 2), conforming allowed (exit 0) — proves the resolver site still returns a pattern - violating `gh pr create --title` blocked (exit 2) - `git qc` where `alias.qc = commit` blocked (exit 2) — proves the alias-probe site still resolves the alias, which fails **open** if it breaks - commit during an in-progress merge (`MERGE_HEAD` present) allowed (exit 0) — proves the sequencer-probe site still detects the sequencer, which is the exemption the probe exists for **New test, mutation-checked.** `block-convention-violation.test.sh` gains a per-site kernel-trace budget assertion: for each external command this file starts, the process that execs it must have a parent that itself execve'd. A parent that never execs and has exactly one child is the wasted fork. Each site must appear in the trace, so a scenario that stops reaching a site fails rather than passing by absence. Moving each redirect back inside its substitution, one at a time, failed exactly its own assertion and no other (3 runs, `PASS=80 FAIL=1` each). The block skips loudly, printing `skip: strace is unavailable…`, where `strace` is absent or not permitted, so it is a Linux-CI guard and not a Windows one. **Gates.** `scripts/affected-tests.sh --run` exit 0, all 5 selected suites pass (`block-convention-violation.test.sh` PASS=81 FAIL=0, `run-guards.test.sh` PASS=101, `require-jq-posture.test.sh`, `lib/resolve-convention-pattern.test.sh`, `commit-msg-convention.test.sh` PASS=15). `shellcheck -x` clean, `shfmt -d` clean, `check-shell-portability.sh --paths` clean, `check-killswitch-hoist.sh` clean, `check-changelog-parity.sh` clean in all four modes (`--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main`). Manifest 0.32.10 → 0.32.11 with the matching CHANGELOG entry. The prettier warning on `plugins/guardrails/CHANGELOG.md` is pre-existing at `origin/main` and untouched. ### Acceptance criteria not fully met — stated plainly | Criterion from #3511 | Status | |---|---| | Subprocess `grep`/`sed`/`cut`/`tr`/`basename`/`dirname` on the hot path replaced with builtins | **Already true before this PR.** None of them appear on any path of this file. | | Early guard clause exits before any spawn for non-matching invocations | **Already true before this PR** (#3820 deferred the convention load). `git status --short` and `echo hello` add **zero** execs from this guard. | | Existing behavioural tests pass; the guard blocks what it blocked before | **Met.** | | No more than 2 external process spawns on its common path (own shell plus at most one `jq`) | **Not met as literally written, and not achievable in-file.** Run standalone the guard costs 3 execs on the common path: 2 `jq` and the telemetry sink, all inside `lib/hook-utils.sh`. In the batch it actually runs in, it adds **0** execs to the common path and 3 pure forks, and those forks are the dispatcher's per-guard `$(source …)` isolation fork plus `hook-utils` internals. Both files are out of scope here: `hook-utils.sh` is synced across 17 plugin copies with unmerged work in flight (#3740, #3838), and the isolation fork is #3685. | | A single run completes in under 2 s on a Windows host | **Unverified.** No Windows host in this session. The comment thread on #3511 also supersedes this figure with the repo's own budget (`docs/conventions/hook-budget/README.md`: <= 1 s typical / <= 2 s worst case per tool call, parallel wall). | This PR does not on its own bring the guard under the hook budget on the host in #3508. It removes the cost this file owns and can fix without touching a fenced shared library. ## Related - Parent: #3508 (Windows process-creation tax), whose stated cause is corrected above - Precedent for the diagnosis: #3520, PR #3779 (found redirection placement is the real cost), PR #3788 (fixed 34 scripts across 17 plugins, `lib/hook-utils.sh` untouched) - Prior work on this file: #3820 (deferred the convention load off the benign path) - Out of scope and named above: #3685 (per-guard isolation fork), #3740 and #3838 (unmerged `hook-utils.sh` work) - Siblings sharing this plugin's manifest and CHANGELOG: #3521, #3529 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ Co-authored-by: Claude <noreply@anthropic.com>
… off substitutions (#3851) Closes #3512 ## Summary The Stop-event unsurfaced-failure detector (`plugins/claude-ops/hooks/hook-failure-audit.sh`) timed out 113 times in the #3508 window at a 21.9 s average, roughly 44x the 500 ms the hook-budget convention gives the whole always-on per-turn set. **The parent issue's stated cause is wrong for this hook.** #3508 blames per-field `jq` forks needing a new shared helper. Shard #3520 (PR #3779) established that the real cost is *redirection placement*, and merged PR #3788 fixed 34 hook scripts across 17 plugins while touching `lib/hook-utils.sh` zero times. This shard applies the same diagnosis. **No shared library is touched** — the fence around `lib/hook-utils.sh` and its 17 synced copies (unmerged #3740, #3838) holds. The mechanism: bash runs the command of a command substitution in the substitution's own subshell and skips the extra fork **only when that command carries no redirection of its own**. `$(wc -c <file 2>/dev/null)` is two processes for one byte count; `$(cat -- file 2>/dev/null)` is two for a file read; `{ V=$(cmd); } 2>/dev/null` is one; `$(<file)` is zero. Those forks are invisible to `bash -x`, which reads command positions — which is why the campaign kept mis-attributing the cost to the exec count. ## Fix Six in-file sites, all in `hook-failure-audit.sh`: | Site | Before | After | |---|---|---| | Transcript size | `SIZE=$(wc -c <"$TRANSCRIPT" 2>/dev/null)` | `wc` names the file, `read` drops the filename column and any padding, `2>/dev/null` rides a single-command group | | Pre-filter, under the cap | `read_window \| grep -F …`, where `read_window` was `cat -- file 2>/dev/null` | `{ RECORDS=$(grep -F … -- "$TRANSCRIPT"); } 2>/dev/null` — grep opens the transcript itself; the helper function was a second subshell on top of the substitution's own | | Marker read | `$(cat -- "$MARKER" 2>/dev/null)` | `$(<"$MARKER")` — no subshell, no exec | | Marker write | `jq … 2>/dev/null \| tr -d '\r' >>"$MARKER"` | `$(jq …)` then in-shell `${VAR//$'\r'/}` with a builtin `printf` append | | Marker directory | `mkdir -p "$MARKER_DIR" 2>/dev/null` every warned turn | `[[ -d … ]] ||` first; `mkdir -p` on an existing directory was a whole process to reach the same no-op | | Payload fields | two `hook::jq_field` calls | one `hook::jq_fields` call (the batching helper the library already ships, used by 10 other hooks) | The CHANGELOG entry originally said "Five sites" over that same list of six; it now says six. Deliberately **not** changed, both documented in the file: - **Over the tail cap**, `tail | sed '1d' | grep` and its `2>/dev/null` on `tail` are untouched. A pipeline element forks either way, so hoisting the redirect onto a group would newly silence `sed` and `grep` for no saving, and merging `sed` into `grep` would change the matching semantics. - **`printf '%s' "$RECORDS" | jq -cRs`** stays a pipeline rather than becoming a here-string, and the in-file comment now states the grounds as they actually stand. `hook::jq_field` in the shared library documents this hazard and refuses the here-string form for it: bash fills a here-string's pipe itself, so a payload at or above the pipe capacity can block before `jq` is exec'd. The trace behind that note (#1587: 65536 bytes hung indefinitely while 65000 returned at once) comes from this repo's **Windows Git Bash** hosts. It does **not** reproduce on Linux bash 5.2 — re-checked here at 65535, 65536, 65537, 200 kB and 2 MB, each returning immediately, including under an unwritable `TMPDIR`. The call keeps the library's conservative form anyway rather than bet the hazard is Linux-only: the forgone saving is one fork on the warning path only, since a turn with no failure record exits before that line. Earlier revisions of this PR and of the comment asserted the deadlock as universal fact, which is more than is known. ## Verification **Process counts**, measured with `strace -ff -qq -e trace=clone,clone3,fork,vfork,execve` (`-ff` so no syscall line is split across an `<unfinished>`/`<resumed>` pair), successful `execve` only, the harness's own top-level `bash <hook>` exec excluded: | Path | Creations before | after | execs before | after | |---|---|---|---|---| | No failure recorded, under the tail cap (the common case) | 18 | **9** | 6 | **4** | | No failure recorded, over the tail cap | 19 | **12** | 7 | **6** | | A turn that warns | 34 | **24** | 17 | **14** | The remaining 9 creations on the common path are 4 execs (`wc`, `grep`, and two `jq` passes inside the synced `hook-utils.sh`) plus subshell forks inside that same fenced library. The `execve` drop is one batched `jq` and two removed helper processes — removed *processes*, not removed work: every input is still read and every record still classified. **No wall-clock figure is claimed.** This Linux host says nothing about the Windows spawn tax the budget binds to, and on the #3508 host one process creation costs 180-2,841 ms (median 1,108 ms at 501 concurrent). The process count is the honest proxy and the README says so. **Behaviour is unchanged, proven, not asserted.** This hook's whole job is surfacing failures that otherwise pass unnoticed, so a carelessly hoisted `2>/dev/null` could silence exactly the diagnostic it exists to emit. Every hoisted group holds one command. The only stream now silenced that was not before is `grep`'s own stderr, and `cat`'s own `2>/dev/null` already discarded that same stream on the same path. Pre- and post-change stdout, stderr, exit codes **and marker-file contents** were compared byte for byte across ten scenarios — first warn, dedup, a new failing registration re-warning while the already-warned one stays muted, all three classification branches (launch / ambiguous / completed), the tail cap, a clean transcript, a missing transcript, the kill switch, and no data directory. Identical. **Budget test, mutation-checked.** `hook-failure-audit.test.sh` gains an strace-based assertion on both the creation and exec ceilings, skipping cleanly where strace is unavailable or not permitted. Non-vacuity was demonstrated, not assumed: moving either silenced redirect back inside its substitution — `SIZE=$(wc -c <"$TRANSCRIPT" 2>/dev/null)`, or `RECORDS=$(grep -F … -- "$TRANSCRIPT" 2>/dev/null)` — pushes creations from 9 to 10 with **no change to the exec count**, leaves all 93 behavioural assertions green, and fails the budget assertion. That is precisely the fork xtrace cannot see. **Gates, all foreground:** | Gate | Result | |---|---| | `scripts/affected-tests.sh --run` | exit 0, both selected suites pass (`hook-failure-audit.test.sh` 94/94, `audit-session-id.test.sh` 27/27) | | `shellcheck -x` on both changed shell files | clean | | `shfmt -d -i 2 -ci` | clean | | `scripts/check-changelog-parity.sh` `--check` / `--check-order` / `--check-bump origin/main` / `--check-preserved origin/main` | all four pass | | `markdownlint-cli2` on the changed markdown | clean | | `scripts/check-purged-em-dashes.sh` | pass | | `scripts/check-shell-portability.sh origin/main` | pass | | `scripts/check-silent-skips.sh`, `check-killswitch-hoist.sh`, `check-hook-exec-form.sh`, `check-hook-wiring-liveness.sh`, `check-cross-plugin-source-drift.sh`, `validate-plugins.sh` | pass | The wording-only follow-up commit re-ran the test suites, all four parity modes, `shellcheck -x`, `shfmt -d -i 2 -ci` and `markdownlint-cli2`: unchanged, including the 9-creation / 4-exec budget assertions, which is the expected result for a comment-and-prose change. `test_save_point.py::test_new_origin_falls_back_to_directory_name` is pre-existing and not in this diff's selection. ### Acceptance criteria not fully met Two of the issue's criteria are **not** satisfied, and no amount of in-file work would satisfy them: - **"A turn with no hook failures exits before any external process spawn."** Not met, and not reachable. The hook learns there is nothing to report *by reading the transcript*; there is no cheaper oracle. The floor without touching the fenced `hook-utils.sh` or abandoning the O(cap) tail bound is one `wc`, one `grep`, and the library's own payload parse — which is what this PR reaches. If the criterion is to be met literally it needs a different design (a marker written by the failing hook, or consolidation with #3515/#3516), not a further micro-optimization here. - **"The always-on per-turn set stays within <= 500 ms parallel wall."** Not measured. That figure is cross-plugin (this hook shares the budget with #3515 `autonomy` and #3516 `disk-hygiene`) and binds to Windows Git Bash, which this runner is not. The README records the figure as owed rather than implying it was taken. A third is met in a different form than specified: the issue asks for a **PATH shim** spawn census. This uses **strace** instead, which is strictly stronger for this defect — a PATH shim sees only programs that are `exec`'d, and the whole cost here is subshell forks that never exec anything. ## Related - Parent: #3508 (the campaign; its stated cause is corrected above for this hook) - Precedent: #3520 / PR #3779 (established redirection placement as the real cost), PR #3788 (34 scripts, 17 plugins, zero library edits) - Siblings on the same 500 ms per-turn budget: #3515 (`autonomy`), #3516 (`disk-hygiene`) - Fenced, deliberately untouched: #3740, #3838 (`lib/hook-utils.sh` and its 17 synced copies); the here-string note above cites `hook::jq_field` there but changes nothing in it - **Adjacency checked, no edit-surface overlap:** open PR #3769 adds correlation keys (`session_id`, `prompt_id`, `tool_use_id`, `agent_id`) to the hook-telemetry envelope spine inside `hook::emit_telemetry`. It does **not** touch `hook-failure-audit.sh`. This diff leaves that hook's `SESSION_ID` extraction and its `data.session_id` envelope key exactly as they are, so nothing here collides with the key migration. The two PRs both bump `plugins/claude-ops/.claude-plugin/plugin.json` and prepend to its CHANGELOG, so whichever lands second takes a trivial version/heading rebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
…it (#3529) On every Bash and PowerShell call the guard created three processes of its own and executed none, so the PATH-shim census read it as free. One of the three was this file's: an eager SUBJECT=$(hook::extract_bash_subject) at file scope feeding a telemetry envelope that is off by default and that the verdict never reads. It is now derived inside emit_tel, behind the start-stamp and sink gates. Off the common path, three more sites paid a fork for a string: - the hash-width probe $(git ... rev-parse --show-object-format 2>&1) cost two creations for one exec, because bash execs a substitution's body in its own subshell only when that body carries no redirection. The 2>&1 has to stay inside (git's stderr is the diagnostic the block message quotes), so the body is now `exec git ...`; - a `!` alias reparse spent one $(printf '%q') per trailing argument, now printf -v; - its $(effective_dir ...) around a builtins-only function is now effective_dir_to, a nameref assignment. Kernel census (strace -f -e trace=clone,clone3,fork,vfork,execve, guard share = dispatched minus a no-op guard dispatched the same way): benign and blocked Bash calls 3 -> 2 creations, execve 0 -> 0; lease with a full-width oid 5 -> 3, execve 1 -> 1; `!` alias with three trailing args 8 -> 3; whole Bash dispatcher 35 -> 34, execve 3 -> 3. The two creations left are $(hook::buffer_stdin) and the shared parser's < <(printf ...), both lib/hook-utils.sh work (#3740, #3838), and the PowerShell lane's fourteen live in lib/powershell/ps-command.sh. Verdicts are unchanged: 190 paired runs against origin/main (87 Bash and 8 PowerShell commands, standalone and dispatched, three repository shapes, CR/BOM/zero-width/U+2028 variants) agree on exit code and full stderr; the contract suite passes at 492 and now pins each site by the same instrument, each pin checked by reverting its change alone. Guardrails 0.32.13 (0.32.11 and 0.32.12 are taken by #3849 and #3869). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
origin/main already landed hook::buffer_stdin_to and the parser's fork-free line split, so the two remaining creations this pin named are gone. Move the benign share from 2 to 0 and the !-alias reparse from +1 to +0. The hash-width probe is still one creation over benign. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…it (#3529) (#3872) Closes #3529 ## Summary `block-dangerous-git.sh` is the guard with the highest external-command call-site count in the marketplace, but on the dispatched path it executes nothing: its cost is forks that never exec, which a PATH shim cannot see. On every Bash and PowerShell call the guard created three such processes of its own. This PR removes the one that was this file's to remove on the common path, and three more on the alias and lease paths, in-file only. `lib/hook-utils.sh`, its per-plugin copies, and `run-guards.sh` are untouched. ## Fix - **Common path (every Bash/PowerShell call):** the eager `SUBJECT=$(hook::extract_bash_subject ...)` at file scope fed a telemetry envelope that is off by default and that the verdict never reads. It is now derived inside `emit_tel`, behind the start-stamp and sink gates. - **Lease path (`--force-with-lease=<ref>:<hex>`):** `out="$(git ... rev-parse --show-object-format 2>&1)"` cost two creations for one exec, because bash execs a substitution's body in the substitution's own subshell only when the body carries no redirection of its own. The `2>&1` cannot move onto an outer group here (git's stderr is the diagnostic the block message quotes, and an outer `2>&1` would send it to the hook's stdout), so the body is now `exec git ...`: the subshell becomes git. A missing git still lands in the same `*` error branch, with bash's own "not found" text captured, and the wording of that captured text changes: see the stderr note under Verification. - **`!` alias reparse:** one `$(printf '%q')` per trailing argument is now `printf -v`; `$(effective_dir ...)` around a builtins-only function is now `effective_dir_to`, a nameref assignment (the default base is read before the nameref is written, so the caller's scratch variable then assigns `HOOK_EFFECTIVE_BASE`). - `$(dirname ...)` was already gone from this file before this PR (`${BASH_SOURCE[0]%/*}`); the scout's note was stale. The PowerShell lane's `$(cd "$_HOOK_SELF/.." && pwd)` is left alone: it runs only when `CLAUDE_PLUGIN_ROOT` is unset, which Claude Code never leaves unset. - Guardrails **0.32.13** (`main` is 0.32.10; #3849 takes 0.32.11 and #3869 takes 0.32.12, re-verified against `origin/main` and the open PR list immediately before opening this). README hook-budget accounting entry added per hook-budget Rule 1. ## Verification **Kernel census** (`strace -f -e trace=clone,clone3,fork,vfork,execve`; guard share = `run-guards.sh block-dangerous-git.sh` minus a no-op guard dispatched the same way; this repository as cwd, `HOOK_TELEMETRY_SINK` unset, `CLAUDE_PROJECT_DIR` empty; three identical repeats). strace rather than a PATH shim or xtrace because the subject is a fork that never execs, which neither of those can see. execve reported separately: it does not move anywhere, which is the evidence this is latency, not removed work. | Scenario | creations before → after | execve before → after | |---|---|---| | Guard share, benign `git status --short` / `echo hello` | 3 → 2 | 0 → 0 | | Guard share, blocked `git push --force origin main` / `git reset --hard` | 3 → 2 | 0 → 0 | | Guard share, lease `--force-with-lease=main:<40-hex>` | 5 → 3 | 1 → 1 | | Guard share, `!` alias no trailing args | 5 → 3 | 0 → 0 | | Guard share, `!` alias with three trailing args | 8 → 3 | 0 → 0 | | Guard share, `!` alias whose body carries a lease | 7 → 4 | 1 → 1 | | Guard share, PowerShell `git status` | 15 → 14 | 3 → 3 | | Standalone `bash block-dangerous-git.sh`, benign | 9 → 8 | 3 → 3 | | Whole Bash dispatcher line from hooks.json, benign | 35 → 34 | 3 → 3 | **Deny paths still deny.** A/B against a pristine `origin/main` export on exit code and stderr: 190 paired runs, 0 differences. 87 Bash commands (every form the guard matches: `--force`, `-f`, `+refspec`, `--mirror`, bare/`=ref`/`=ref:movable`/`=ref:<40-hex>`/`=ref:<64-hex>`/`--force-if-includes`/`--no-force-with-lease`/`--dry-run` lease spellings, `reset --hard`/`--h`, `clean -f/-fd/-fdx/--force`, `checkout .`/`:/`/`-f`/`--pathspec-from-file`/exclude-only, `restore .`, `switch --discard-changes`; their near-miss safe variants `reset --keep`, `clean -n`, `checkout -- file`, `restore --staged .`, `branch -D`, `filter-branch`, quoted text; `!` and inline aliases, `bash -c`/`sh -c`/`env`/`sudo` wrappers, multi-segment lines) and 8 PowerShell commands, standalone and dispatched, across a SHA-1 repo, a SHA-256 repo, a non-repo and a nonexistent `-C` dir (probe-failure path). Verdict tally on the new tree: 51 blocked / 36 allowed Bash, 5 / 3 PowerShell. **One stderr string is not identical, and it is not covered by those 190 runs.** Every run above had `git` on `PATH`. On a `PATH` carrying no `git` the lookup now fails inside the `exec` rather than around it, so the diagnostic the block message quotes changes wording: - before: `<hook>: line 341: git: command not found` - after: `<hook>: line 363: exec: git: not found` Measured directly on both trees, running the real hook against a `PATH` shadow that excludes `git` and nothing else: both exit **2**, both emit the same `BLOCKED: ... hash format could not be determined (...)` frame and the same remedy line, and the probe's own status is **127** on both, so the same `*` branch runs and the push is blocked either way. The claim this section previously made, "full stderr", overstated that: verdicts and stderr agree everywhere the 190 runs reached, and on the git-absent probe the only thing that moves is the wording of a quoted error message on a path that still denies. (Related, same mechanism: a `git` on `PATH` that is present but not executable is rc **126** on both trees, and the `exec` form appends a second `cannot execute: Permission denied` line.) The 0.32.13 CHANGELOG entry and the README hook-budget entry state the same qualification. **Permissive-normalization check.** Field reads are not touched by this PR. Probed anyway: a CR mid-token (`git push --for\rce origin main`) and after the token are both blocked, identically on standalone and dispatched paths and on both trees, so the CR strip already lives in the library's field read rather than in the dispatcher cache, and for this guard it acts in the restrictive direction. BOM-prefixed `git`, a zero-width space glued to `--force`, and U+2028 glued to `--force` are allowed on both trees (the bytes make the token something bash would hand to git verbatim, and git rejects it); no divergence introduced. **Contract suite.** `block-dangerous-git.test.sh` 479 → 492, all passing (re-run at the tip of this branch: `PASS=492 FAIL=0`, strace pins included). The 13 new assertions are strace-based pins: benign share exactly 2 creations / 0 execve; blocked share equals benign; `!` alias reparse is benign + 1 (the shared parser's re-entry); trailing alias arguments add 0; lease probe is benign + 1 creation and exactly 1 execve; and a per-process check that the `git rev-parse --show-object-format` exec's parent itself exec'd. Skips visibly where strace is absent. **Non-vacuity proven by mutation**, each change reverted alone: eager subject → benign pin fails (3); `exec` removed → lease pins fail (4, `extra-fork`); `$(printf)` restored → trailing-args pin fails (6); `$(effective_dir)` restored → alias pin fails (4). **Gates.** `scripts/affected-tests.sh --run`: every selected suite passes except `plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh` (2 of 24, PS4 trace probe), which fails identically on a clean `origin/main` export, so it is pre-existing and host-specific, not this change. `check-changelog-parity.sh` `--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main`: all four pass. `check-killswitch-hoist.sh` passes. `check-purged-em-dashes.sh` passes. shellcheck and shfmt clean on both shell files; markdownlint-cli2 clean on CHANGELOG and README. **Acceptance criteria in #3529, stated plainly.** (1) Spawn count before/after: reported above, by kernel trace, a superset of the PATH-shim count the issue asked for. (2) "No more than 2 external process spawns on the common path": this guard's own share is now 0 execs and 2 forks; the two forks are `$(hook::buffer_stdin)` and the shared parser's `< <(printf ...)`, both `lib/hook-utils.sh` (#3740, #3838) and outside this PR's fence, so that line is not closed from inside this file. (3) "A non-`git` command exits before any spawn": this guard spawns nothing of its own on any command; the dispatcher's two `jq` execs are batch-wide since #3788 and not attributable to this guard. (4) Behavioural tests pass and every form blocked before is still blocked (190/190 A/B on verdict). (5) Wall-clock alongside concurrent-process count: not reported. This Linux host's spawn floor is under a millisecond, so a timing here would not transfer to the Windows spawn tax the parent describes; the process count is the durable figure. ## Related - Refs #3508 (parent). Its "per-field jq" diagnosis does not apply to this guard: since #3788 the dispatcher buffers stdin and primes jq once per batch, so no individual guard contributes a jq spawn; the remaining cost here was forks that never exec. - Precedent for the mechanism and the strace pin: #3779 (context-guard), #3849 (guardrails 0.32.11), #3869 (guardrails 0.32.12), #3851, #3870, #3871. - Version-chain siblings: #3849, #3869 (entries kept verbatim; this PR adds only 0.32.13). - Out of fence, left for their owners: `$(hook::buffer_stdin)` fork-free form in #3740 / #3838; the PowerShell lane's 14 creations live in `lib/powershell/ps-command.sh`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
No linked issue
Summary
Phase 1 of a measurement-first pass over this repo's Bash scripts. Highest remaining PATH-visible cost on the always-on guardrails dispatcher (named leftover after 0.32.10) was two
jqprocesses per fire:jq -e .for stdin completeness plusjq_fieldsfor the primed payload. This PR makes them one, and stops paying a command-substitution subshell to capture stdin.Fix
Discovery ranked by the performance plugin's evidence tiers and this repo's own hook-budget accounting, not by file size:
jqjqexecsINPUT=$(hook::buffer_stdin)capture fork$(source guard)forksAuthoritative basis:
run-guards.shandhook-utils.sh.iffield: "The hook command only runs if the tool call matches the pattern" (https://code.claude.com/docs/en/hooks.md). Already applied; not reopened.performanceharness: spawn count outranks wall-clock; refuse two-pass claims from an unmeasurable host. This host was measurable (spread 1.87×, floor 0.4 ms).printf -vnameref assignments are not tracked; initialize dest withvar=""(https://www.shellcheck.net/wiki/SC2154, Exceptions).Concrete change:
hook::buffer_stdin_to dest [jq-filter...]writes the payload withprintf -v. Optional filters fuse completeness withhook::jq_fieldsso a caller that was about to parse anyway spends onejq. Locals are__hu_-prefixed so a dest namedinput/read_timeout/fields_rcstill receives the payload.run-guards.shuses the fused form withPRIME_FILTERS. Dest is initialized (RUN_GUARDS_INPUT="") so ShellCheck SC2154 sees the assignment. The override's dest local is__rg_dest.INPUT=$(hook::buffer_stdin)to_to.Isolation
$(source …)forks (#3685) and exec-form rows (#3686, blocked on a Windows live probe of upstream #90495) are later phases. CI scripts were timed once for ranking; without a PR diff they are not the cost.Verification
Host
spawn_probe: measurable, min 0.4 ms, median 0.7 ms, max 0.8 ms, spread 1.87×.PATH-shim spawn census (
plugins/performance/scripts/spawn-census.sh,HOOK_TELEMETRY_SINKunset, this repository as cwd, warm agreement n=3):git status --short2 jq)1 jq)echo hello2 jq)1 jq).md3 git,2 jq)3 git,1 jq)Wall p50/p95 n=20 after 2 warmup on this cheap-spawn Linux host (context, not the claim):
git status --short40.11/40.99 → 37.04/39.81 ms.bash lib/hook-utils.test.sh: PASS=308 FAIL=0 (includes dest-name collision pins).bash plugins/guardrails/hooks/run-guards.test.sh: PASS=103 FAIL=0 (pins the benign Bash dispatcher at onejq, and a guard dest nameddest).Related
Refs #3685, #3623, #1403, #3820. Follows
docs/conventions/hook-budget/README.md(budget never relaxes) andplugins/performance/reference/harness-integrity.md(spawn count over two-pass wall-clock).