perf(disk-hygiene): skip the Stop monitor when no guard launched - #4190
Conversation
The engine-gate rows are `if`-gated on the engine's file name, so most
sessions never launch destructive_guard.py at all, yet the Stop monitor
started a whole Python every turn to discover that from the transcript.
Stop rows accept neither `matcher` nor `if`, so the gate has to live in
the bash launcher, before the interpreter is resolved.
run-python-hook.sh takes three optional leading flags, consumed there and
never forwarded to Python:
--marker-root <dir> root the marker tree lives under
--launch-marker <subdir> write <root>/<subdir>/<session>.launched
before exec'ing the target
--skip-unless-marker <subdir> exit 0 without exec'ing when it is absent
The root is passed explicitly rather than read from CLAUDE_PLUGIN_DATA in
the environment, which a hook subprocess can inherit naming a different
plugin's data directory; a writer and a reader that disagree about the
root skip silently, which is the missed detection this monitor exists to
prevent. The session id is recovered by a regex anchored to the payload's
opening key, so nothing nested inside the payload can reach it, with an
unanchored fallback so a reordered payload degrades to running Python
rather than to keying on nothing; a payload it cannot key on records
nothing and skips nothing. The payload is buffered with `read` and
replayed on a here-string, so Python reads the same bytes plus the newline
`<<<` appends; both consumers ignore trailing whitespace.
Process creations per Stop on Windows, job-object census at n=5 against a
1-creation harness floor: 5 before, 3 with no marker, 5 with one present.
Wall clock 271 ms to 120 ms on the skipped path. The engine-gate row pays
nothing for the marker after the first launch in a data root, which spends
one mkdir (7 creations once, then 5). With a marker present the monitor's
systemMessage and its guard-decisions record are byte-identical.
The marker is written before the interpreter resolves, so a guard that
launches and dies still leaves it. Markers are never removed and nothing
sweeps the data root; a session whose guard rows never fired is skipped by
design, and a skipped turn emits no telemetry envelope where it used to
emit an `ok` one.
run-python-hook.test.sh gains eighteen assertions over the four halves of
that contract, and stubs `py` alongside python3/python in the no-python
case, which a host carrying the Windows py launcher resolved through.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.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 2s —— View job Review:
|
|
Claude finished @kyle-sexton's task in 5m 59s —— View job Security review complete
Scope: all 5 changed files ( Checked in particular: the new session-id extraction/sanitization ( One finding survived adversarial verification (posted inline at run-python-hook.sh#L208-L231): SUGGESTION — The new launch marker's write path ( No CRITICAL or IMPORTANT findings (vulnerabilities, authz/tenancy gaps, injection, or secret handling). GitHub Actions hardening is out of scope for this lane (zizmor's lane) and this PR touches no workflow files regardless. Version bump, changelog, and hooks.json flag wiring carry no security-relevant surface. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
|
Last security-reviewed head: |
CI's typos lane read the hyphenated form as a misspelling of miss. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Stop gate skipped the guard-launch monitor whenever no marker file was found, so a launch whose mkdir failed for both candidate roots silenced the monitor for the rest of the session: a silent failure in the one detector that exists to report silent failures. The gate now skips only when at least one candidate marker directory exists and none of them holds a marker for the session, and otherwise runs the monitor exactly as the launcher did before these flags. Both tests are bash builtins, so the skip path still reaches its exit without spawning anything. The residual is stated rather than implied, in the launcher header and in the changelog: a candidate directory that exists while the marker file itself could not be written still degrades to silence for that session, because that file is the only channel between the launch row and the Stop row. The fail-open also makes the skip inert in a plugin data root where no guard has ever launched, until the first launch spends its mkdir. The suite stages an unwritable marker root structurally rather than through permission bits MSYS cannot set against Windows ACLs: mkdir -p refuses a parent that is a regular file, and refuses a target that already exists as one. New cases cover a launch that can write no marker at all, the Stop that follows it, and a candidate directory holding no marker for the session. Process creations per Stop on Windows, job-object census at n=5: 5 with no candidate directory, 3 with an empty one, 5 with a marker present, against a 1-creation harness floor. The engine-gate launch path is unchanged at 5 after the first launch in a data root spends its mkdir. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…anged (#4193) No related issue: handoff-inbox item 20260913-034032 under program item 20260915-153000 (spawn budget per tool call); no GitHub issue was filed. Stacked on #4185 (base branch `guardrails-run-guards-in-process-no-subs`): the envelope parse rides on that PR's `hook::jq_fields` builtin parser in the synced lib. GitHub retargets it to main when #4185 merges. ## Summary context-guard's `zone-crossing-inject.sh` fires on every `PostToolBatch` and `UserPromptSubmit`. Each fire spawned a jq for the envelope and then the zone resolver (its own bash plus a jq) even when nothing the resolver reads had changed since the last fire, which is the common case: the statusline snapshot is rewritten only when the statusline renders. ## Fix - An unchanged-input skip. A `$STATE_DIR/$SESSION.seen` mark records the inputs behind the last completed resolve in two ways: its mtime, stamped with a redirection and compared with `-nt`, and one flags line (`z=<0|1> c=<0|1>`) recording whether `zones.json` and the compaction marker existed when that resolve ran, read back with builtin `read`. The fire exits before starting a process only when the per-session snapshot, `zones.json` and the compaction marker are all no newer than the mark AND both existence flags still match the current `-e` results; a mark with no readable flags line never takes the skip. The mark moves only after the resolve persisted its markers, so a resolver failure, an `unknown` reading and a failed marker write are each retried next fire. A missing snapshot is never skippable. Skipping can only choose silence: no arrangement of timestamps or existence changes can manufacture an injection the full path would not have made. - The envelope parse is size-branched. Under 64 KiB it goes through `hook::jq_fields`' builtin parser (zero spawns); above it keeps the single here-string jq (2 creations), because the helper's oversize fallback reads through a process substitution and measured 4. Plain routing through the helper would have made the large-payload path 9 to 11 creations; the branch is what keeps every cell at or below before. The `65536` literal mirrors the helper's private proof ceiling and is documented at the site. - `STATE_DIR` resolution moves ahead of the resolver, so a session with no state root exits one process earlier. context-guard is bumped 0.7.64 to 0.7.65 with the numbers in the CHANGELOG, and the README gains a "Skipping the resolve when nothing moved" subsection carrying the table. ## Verification Job-object census (n=5, identical across reps; subject floor 3 = `bash -c`, `env`, bash): | Fire | Payload | Creations before | Creations after | |---|---|---|---| | first (resolves) | small | 11 | 9 | | repeat, nothing moved | small | 9 | 3 | | snapshot rewritten | small | 9 | 7 | | first | 150 KB | 11 | 11 | | repeat, nothing moved | 150 KB | 9 | 5 | | snapshot rewritten | 150 KB | 9 | 9 | No cell is worse than before. Small repeat fire wall p50 1448 ms to 237 ms (re-run 365 ms; the host is bimodal). On Windows a resolve costs 4 creations rather than 2 because `bash "$RESOLVER"` hits the `bin\bash.exe` wrapper, which re-spawns `usr\bin\bash`. Crossing messages are byte-identical, asserted in the suite against a control session driven through the same zone sequence with no skipped fire. Suite: `zone-crossing-inject.test.sh` 78/1 to 97/2, where both failures are `strace: no usable trace` on this host (Git Bash's cygwin strace rejects `-e trace=`; the second is the new strace block, not a regression); `zone-gate` 26/0, `post-compact-mark` 18/0. shellcheck clean; `check-changelog-parity.sh --check-bump origin/main` and `--check` green. Residuals: the strace pins (steady fire 0 creations / 1 execve; resolving fire 2 / 3) are set by reasoning and verified only on CI's Linux lane. The one miss window is the resolve itself: a snapshot written between the resolver's read and the stamp is marked seen and its crossing is reported one fire late, never lost, since the statusline rewrites the snapshot on its next render; on a filesystem or bash build that compares mtimes at whole-second granularity the window is up to one second. The README still carries the older "0.7.49 brought it to 3" paragraph; the new subsection supersedes it. ## Related - #4185 (base of this stack). Program item 20260915-153000; siblings #4188, #4189, #4190, #4191, #4192. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
#4192) No related issue: handoff-inbox item 20260913-034033 under program item 20260915-153000 (spawn budget per tool call); no GitHub issue was filed. Stacked on #4185 (base branch `guardrails-run-guards-in-process-no-subs`): this hook rides on that PR's `hook::buffer_stdin_to` field read in the synced lib. GitHub retargets it to main when #4185 merges. ## Summary claude-ops' `hook-failure-audit.sh` `Stop` hook set the per-turn wall: it re-scanned the whole session transcript on every stop (a `wc`, a `grep`, a jq over every candidate) to discover that no new `hook_non_blocking_error` record had appeared, which is the common case. ## Fix - A per-session cursor, `${CLAUDE_PLUGIN_DATA}/hook-failure-audit/<session>.cursor`, holds the count of complete lines already audited and the `transcript_path` it was taken against, beside the existing warning marker and under the same 7-day prune. - A warm `Stop` reads with `mapfile -s` from one line before the cursor (that line is an anchor: the same read that fetches new lines proves the file still has that many; an empty result means shrinkage), prefilters candidates in bash with the same fixed string the grep used, and runs jq only for a candidate line. `mapfile` runs without `-t` so joined candidates are byte-identical to `$(grep …)`; a final line with no newline is scanned but not counted, so it is re-read next turn. - The cursor resets to 0 (a full rescan) on: no data home, malformed cursor, different `transcript_path`, fewer lines than the cursor, pruned cursor, or bash without `mapfile` (the old grep path). It advances only at the four disposal points (no candidate, empty summary, nothing new, after the system message); a jq failure leaves it put. Rescanning cannot re-warn, because the marker still decides that. - The cold scan keeps its tail cap; one `wc -lc` now answers both the cap decision and the cursor's starting line count. - Payload fields ride on `hook::buffer_stdin_to INPUT '.transcript_path' '.session_id'`, which fuses the library's validation probe into the builtin field read; `hook::require_jq` moves after it. claude-ops is bumped 0.56.14 to 0.56.15 with the numbers in the CHANGELOG. Sibling #4189 also claims 0.56.14 on its branch; whichever merges second needs its version and CHANGELOG re-based (parity is checked against origin/main). ## Verification Job-object census (n=5, Windows Git Bash; floor 3 = `bash -c`, `env`, `bash`): | Arm | Creations before | Creations after | |---|---|---| | second Stop, 20 benign lines appended (the common turn) | 10 | 3 (= floor) | | first Stop, data dir exists | 10 | 5 | | first Stop, fresh data dir | 10 | 7 | | second Stop, a failure record appended | 30 | 21 | Wall clock after 0.23 to 0.36 s where before ran 0.36 to 1.38 s across two runs; the host drifts about 4x within an hour, so the creation count is the record. Byte identity, old versus new hook on fresh data dirs, identical stdout: a single record; three mixed classes; multiple registrations plus both false-positive shapes; an over-cap transcript; a last line without a trailing newline; and a two-turn incremental sequence where turn 2 appends a failure past the cursor. The suite asserts the same in-tree: incremental turn-2 output equals a full rescan against identical marker state. Suites: `hook-failure-audit.test.sh` 84/4 to 94/4 (10 new assertions; the 4 failures are pre-existing on this host, a Windows CRLF artefact in `jq … @tsv` on `HAS_COMPLETED`). Discrimination checked: the jq PATH-shim test fails against the old hook, and the path-reset test fails against a copy with the path check removed. Degraded paths: a no-`mapfile` copy warns then dedups, an unwritable marker home still warns, an empty transcript is silent. shellcheck, shfmt, markdownlint and `check-changelog-parity.sh --check-bump origin/main` clean. Unproven: the strace budget block runs only on CI's Linux lane; the warm ceiling (0 creations, 0 execs) follows from the Windows census at floor, the cold ceilings (6 creations, 2 execs) are conservative estimates and are what to adjust if CI reports otherwise. The bash 3.2 fallback was exercised by forcing `HAVE_MAPFILE=0`, not on a real bash 3.2. An over-cap cold scan sets the cursor from the whole file's line count while reading only the last 2 MB; pre-window lines were never read before either, and the cursor makes that permanent for the session. ## Related - #4185 (base of this stack), #4189 (item 3, the other claude-ops bump). Program item 20260915-153000; siblings #4188, #4190, #4191 and the context-guard hook as its own PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
No related issue: handoff-inbox item 20260913-034034 under program item 20260915-153000 (spawn budget per tool call); no GitHub issue was filed.
Summary
disk-hygiene's
Stoprow started a whole Python interpreter on every interactive stop to read the transcript and discover that this session never launcheddestructive_guard.py, which is true of most sessions because the engine-gate rows areif-gated on the engine's file name.Stoprows accept neithermatchernorif, so the gate has to live in the bash launcher, before the interpreter is resolved.Fix
hooks/run-python-hook.shtakes three optional leading flags, consumed there and never forwarded to Python:--marker-root <dir>: the root the marker tree lives under, spelled identically on the writer and the reader rows ("${CLAUDE_PLUGIN_DATA}", the same literal the rows already pass as--authorized-data-root). An explicit root rather than the environment variable, because a writer and a reader that disagree about the root skip silently, which is the missed detection this monitor exists to prevent.--launch-marker <subdir>(the engine-gate rows): write<root>/<subdir>/<session>.launchedbefore exec'ing the target, so a guard that launches and dies still leaves the marker that keeps the monitor running.--skip-unless-marker <subdir>(theStoprow): exit 0 without exec'ing anything when that file is absent.Candidates mirror the monitor's own
_marker_path_candidates(the data root, then a${TMPDIR:-/tmp}fallback beside the.warnedmarker). The session id is recovered by a bash regex anchored to the payload's opening key, with an unanchored fallback so a reordered payload degrades to running Python rather than to keying on nothing; a payload it cannot key on records nothing and skips nothing. The payload is buffered with thereadbuiltin and replayed to Python on a here-string.disk-hygiene is bumped 0.23.11 to 0.23.12 with the numbers and the marker semantics in the CHANGELOG.
Verification
Job-object census (n=5, Git Bash
-c "<row string>",CLAUDE_PLUGIN_DATAat a temp dir, harness floorbash -c ':'= 1):mkdir)The
mkdiris paid once per data root, never per session or per launch.Byte identity: the fixture target writes
sys.stdin.buffer.read(); stdin after equals the payload plus the one trailing newline<<<appends, which both consumers (json.load(sys.stdin)in the guard,sys.stdin.read()+json.loadsin the monitor) ignore. End to end on a transcript carrying a realhook_non_blocking_error: stdout 534 bytes on both roots,cmpidentical; stderr identical;guard-decisions/decisions.jsonlidentical with timestamp and session normalized. Skipped case: rc 0, 0 bytes out, 0 files written.Suites:
run-python-hook.test.sh50 pass / 0 fail (32 pre-existing + 18 new; the pre-existing no-python case now also stubspy, because this host's Windows py launcher resolved a real 3.13 and aborted the suite before this change);test_guard_launch_monitor.py28/28 both sides. shellcheck andcheck-shell-portability.sh --pathsclean;check-changelog-parity.sh --check-bump origin/main, check-hook-exec-form, check-hook-userconfig-argv, check-hook-wiring-liveness, validate-plugin-contracts.mjs and validate-plugins.sh green. Pre-existing and reproduced on the unmodified tree:test_hook_telemetry2 sink-timeout failures,test_hygiene10 git-fixture errors.Residuals, for review:
lib/guard_decision_log.pyrotates only its own log). Stated in the wrapper and the CHANGELOG; a retention sweep is a separate decision.okone.session_idas the opening key but documents no ordering guarantee; anchored-first plus fallback covers both, not proven against a live wire payload.Related
🤖 Generated with Claude Code