fix(guardrails): make the drive-root /tmp guard see a Write, not only a command

[kyle-sexton]

Summary

block-windows-drive-tmp exists to stop a write landing at a Windows drive root (<drive>:\tmp) instead of %TEMP% (#2594). On 2026-08-30 it missed one: an empty C:\tmp\tmp.rSFIkHm5DO was created and nothing fired.

Root cause. The hook read .tool_input.command. A Write payload carries file_path and no command, so hook::jq_fields produced an empty COMMAND and the [[ -n "$COMMAND" ]] || exit 0 early exit returned before any matcher ran. The guard covered command-shaped writes and not tool-shaped ones. Confirmed from the local transcript, not inferred: a Write with file_path: /tmp/tmp.rSFIkHm5DO/worktree-root at 18:15:54, then a record naming realParentDir: C:\tmp\tmp.rSFIkHm5DO at 18:15:56.884 — matching that directory's birth time to the millisecond.

Opened as a draft: the brief says do not merge, and .claude/source-control.md sets babysit_loop_merge: c3-autonomous, so prose is not enforcement and draft state is.

Fix

Host gate hoisted ahead of stdin and the jq requirement (P1 from review). Widening the matcher to Write/Edit/MultiEdit/NotebookEdit had a consequence the widening itself did not carry: the non-Windows OSTYPE gate sat below hook::buffer_stdin and hook::require_jq_blocking, so on a Linux or macOS host with no jq on PATH, the fail-closed exit 2 fired on every file edit — on a platform where /tmp is the real POSIX temp and this guard can never find a violation. Before the widening the hook matched only Bash|PowerShell, where blocking on missing jq is the accepted #2146 posture, so the ordering was not reachable this way.

The gate now runs immediately after hook::check_enabled. Reading OSTYPE needs nothing from the payload. The Windows path is unchanged — the case falls through and buffer_stdin rc 2, jq absence, an unparseable payload, NUL bytes and MAX_COMMAND_LEN all still fail closed in the same order, verified verdict-by-verdict against the parent commit.

One posture change is deliberate and stated rather than silent: on a non-Windows host this guard no longer fails closed on jq's absence for the Bash lane either. That posture is kept in full on Windows and dropped only where every exit 2 it produced was a false positive by construction.

block-exported-msys-pathconv.sh keeps the opposite ordering on purpose — it matches only Bash|PowerShell, so the blast radius forcing the hoist here does not exist there. The comment says so, to stop a future author aligning them back.

Two edits, because either alone is inert.

1. plugins/guardrails/hooks/block-windows-drive-tmp.sh also reads .tool_input.file_path and .tool_input.notebook_path, and the early exit tests both doors.
2. plugins/guardrails/hooks/hooks.json gains a separate PreToolUse group matching Write|Edit|MultiEdit|NotebookEdit. Without it the hook never receives those payloads and edit 1 is dead code.

Why a separate group rather than a widened matcher string. Widening the existing Write|Edit|NotebookEdit group would hand MultiEdit to secret-pattern-detection and hardcoded-path-check as a side effect; widening Bash|PowerShell would attach seven command-lane guards to every file write and blow the per-Write budget several times over. Claude Code fires every matching group, so a Write now matches two guardrails groups — but this hook sits in exactly one, so it still fires once per tool call.

One matcher, not two. The file-path lane calls the shipped has_drive_root_tmp(). It carries none of the command lane's write-shape inference — no redirect parsing, no producer-utility whitelist, no segment splitting — because on Write/Edit the path is the write target.

Fail-closed posture preserved, and extended to the new field. NUL handling, hook::buffer_stdin rc 2, jq absence and MAX_COMMAND_LEN behave exactly as before, verified by execution against origin/main. Only PATH fields were added to the hook::jq_fields call: HOOK_JQ_FIELDS_NUL is computed across every requested field, so pulling content / new_string / new_source in would have made this guard block on a NUL anywhere in a file body — a new false-positive class belonging to hardcoded-path-check, not here.

No length ceiling on the path lane, by decision. MAX_COMMAND_LEN exists because the command lane walks its string character by character twice before matching; the path lane runs EREs with no tokenization, detection does not degrade with length, and a blocking ceiling would only refuse legitimate long paths. The payload stays bounded by hook::buffer_stdin, whose stall path fails closed.

A pre-existing false positive, fixed because this change made it reachable — and then corrected again after it over-reached. D:\a\tmp\x blocked: after slash-normalization the drive colon satisfied the left boundary of the MSYS /<drive>/tmp alternative, so d: + /a/tmp read as a drive root, while the identical MSYS spelling /d/a/tmp/x was allowed — one sink deciding two ways. The first attempt excluded : from that boundary outright, which regressed six colon-bound PowerShell write spellings (Set-Content -Path:/c/tmp/x, New-Item -Path:, Out-File -FilePath:, Add-Content -Path:, Copy-Item -Destination:, Move-Item -Destination:) — one of whose space-bound twins the suite pins as MUST-fire. The matcher is now two arms, discriminating on what sits before the colon: a drive spec is exactly one alphanumeric at a word boundary and no longer satisfies it; every other colon, including a parameter colon, still does.

Budget housekeeping in passing. Case folding moves from printf | tr to ${var,,}, and the telemetry subject resolves inside emit_tel — removing a fork-and-exec pair and a fork that the pre-existing per-Bash-call lane was already paying on every call with no sink wired.

Verification

The matcher narrowing was swept exhaustively, not argued. Both versions' has_drive_root_tmp() were lifted from their own files by sed (no retyped copy) and compared over 1,702 probes: every ASCII printable as the immediate left neighbour of /c/tmp/x, every two-character context ending in a colon, nine drive letters in eight surrounding contexts, deeper subdirectory shapes, and the colon-bearing forms a sweep alone does not reach.

All 266 changed verdicts are the drive-spec reading — 192 <non-alnum><alnum>: contexts, 72 explicit X:\a\tmp probes, D:\a\tmp itself, and a leading bare :. Every real drive-root spelling still matches on both versions: /tmp/x, /c/tmp/x, C:/tmp/x, C:\tmp\x, \tmp\x, D:\tmp\x, the redirect and mkdir forms, and all six colon-bound PowerShell writers. Two accepted residuals, both unchanged from the shipped guard rather than introduced: PATH=/usr/bin:/c/tmp cmd presents the multi-character-token shape and still matches, and ssh u@h:/c/tmp/x now reads as a drive spec and does not — it names a path on another machine, which this guard never governed.

Hook budget — measured, per .claude/rules/hook-budget.md and docs/conventions/hook-budget/README.md, recorded in the guardrails README. Method is the convention's: EPOCHREALTIME around direct invocation with a benign representative payload, sets launched concurrently. The measuring host was under heavy concurrent agent load — its bash -c : spawn baseline measured 4,498 ms against the convention's reference-host ≈ 80 ms, ~56x slower — so every figure re-measures bash -c : interleaved with each trial and is reported as a load-normalized spawn-equivalent, converted back at 80 ms.

Measured (n=12 interleaved)	spawn-equivalents	@ 80 ms reference
the guard alone, one Write payload	6.31	≈ 505 ms
guardrails per-Write PreToolUse set BEFORE (2 hooks)	12.59	≈ 1,007 ms
guardrails per-Write PreToolUse set AFTER (3 hooks)	12.34	≈ 987 ms

The hook's own ≈ 505 ms is the figure that holds; the set-level delta is below this host's noise floor and the README states no delta. AFTER measuring lower than BEFORE is impossible, and a paired A/B (n=15, arms alternated within each trial) spans 0.55x–1.82x with several trials putting AFTER faster. Against the convention's ≤ 2 s worst-case per-tool-call ceiling — which counts PostToolUse too, so the ≈ 1.9 s per-Write overage recorded in the convention is deeper than the PreToolUse slice above shows — ≈ 505 ms is ≈ 25% of the ceiling as an upper bound on this widening's contribution, less in practice because matching hooks dispatch in parallel. Per rule 2 the budget does not relax: remediation stays guardrails' spawn-reduction work (#1403), which this change pays part of.

ADR 0003 decision: ships BLOCKING and default-on, and here is why that is not a dodge. ADR 0003 is a pre-ship gate on whether a verification guard earns default-on; it hands an in-tree guard to docs/conventions/hook-precision/ thereafter. ADR 0002's advisory-first promotion governs the GitHub Actions AI review lanes, not plugin hooks. This guard is already blocking and default-on; this is a matcher widening, and the new lane's oracle is strictly less inferential than the shipped lane's — on a Write the path is the write target, so the redirect parsing and utility whitelist that create the command lane's ambiguity are structurally absent, and precision on the new lane is bounded below by the shipped lane's. Shipping advisory-on-Write beside blocking-on-Bash for one hazard class would also be a posture no operator can reason about. The widening nonetheless opted into 0003's evidentiary discipline:

• Corpus. 259 distinct file_path / notebook_path values that a real Write/Edit/MultiEdit/NotebookEdit actually carried, across 227 local Claude Code session transcripts on a Windows host — absolute Windows and MSYS paths, not repo-relative ones a drive-root matcher could never match. Re-run against the final matcher, not an earlier draft of it.
• Result. 1 finding in 259 (0.39% firing), and it was a true positive: /tmp/tmp.rSFIkHm5DO/worktree-root, the incident write itself. 6/6 seeded spellings detected end to end.
• Stated honestly. Precision is 1/1, so this is the near-zero-findings case where seeded detection carries the argument and the ratio alone does not. The corpus is one host and one operator, and contains no MultiEdit or NotebookEdit entries — those two are covered by the contract suite and the shared matcher, not by the sweep. Per rule 3 the README names the acceptable ratio for this surface (a false-positive rate near zero) and justifies it from the asymmetry: a wrong block costs one stderr line and a reissued write, a missed one is silent by construction and was found only as litter on a volume root. Compare the ADR's shipped reference guard at 0.51% firing, 57% precision.

Tests. plugins/guardrails/hooks/block-windows-drive-tmp.test.sh green, PASS=181 FAIL=0 (was 142 before this branch). New coverage: the incident Write payload and every drive-root spelling on Write/Edit/MultiEdit/NotebookEdit (both file_path and notebook_path); legitimate %TEMP%, /var/tmp, docs/tmp, ./tmp, foo/tmp, /tmpdir, C:/tmp2, UNC and single-letter-parent tmp targets in both spellings; the six colon-bound PowerShell writers as MUST-fire beside -Path:D:/a/tmp/x as MUST-stay-quiet; Write content mentioning /tmp; the Linux-host exit for Write and Edit; NUL in file_path; the kill switch on the new lane; the file-path telemetry envelope; and the hooks.json registration itself. Every pre-existing Bash and PowerShell case is unchanged — the test diff is insertions only.

Sibling suite block-exported-msys-pathconv.test.sh green (PASS=127 FAIL=0); scripts/check-drive-root-litter.test.sh exit 0. scripts/affected-tests.sh --explain exits 0 with three suites selected and no changed file mapping to zero suites. check-changelog-parity.sh --check, --check-bump origin/main and --check-preserved origin/main all exit 0. check-shell-portability.sh exits 0 — two fixture literals this branch introduced carried \s/\b and failed it; resolved by renaming the fixture segments rather than suppressing the scanner. shellcheck -x and markdownlint-cli2 clean.

Repro-first, per the hook-precision convention. The new MUST-fire cases exit 0 against the unmodified hook and 2 against this one. The MUST-stay-quiet cases for the single-letter-parent false positive exit 2 against the first commit and 0 now, while /c/tmp/x and C:\tmp\x stay at 2 throughout. The six colon-bound PowerShell cases exit 2 / 0 / 2 across origin/main, the over-reaching first fix, and HEAD.

Round 3 (on the host-gate fix), nine criteria, reasoning withheld. PASS on all of them, with zero Windows-lane divergence across 60+ payloads compared verdict-by-verdict against the parent commit — both lanes, both hosts, including the colon-bound PowerShell spellings — and every fail-closed posture checked independently (NUL, malformed JSON, MAX_COMMAND_LEN, empty stdin, jq absence). Suite 183/0. The new regression assertions were confirmed red against the parent and green against the fix, so the test guards the change rather than merely passing.

It also falsified a claim in my own commit prose: I wrote that removing jq from PATH is not portably simulable and told a future author not to re-attempt it. The verifier did it — pruning the single jq PATH entry on this host leaves bash and coreutils intact — and got direct evidence for the motivation: on a real jq-less PATH, the parent exits 2 on a Linux Write and this commit exits 0. The simulation is host-dependent (it needs jq outside the directory holding bash), which is the actual constraint the repo records, so the committed test still uses the portable xtrace assertion. The prose is corrected in both the hook comment and the CHANGELOG.

Two fresh-context verification rounds before that, reasoning withheld, diff and criteria only. Round 1 (on the first commit) independently confirmed the root cause from the transcript, executed every payload against both hook versions, and found the portability failure, the single-letter-parent false positive, the unasserted registration, and README figures stated beyond their evidence. Round 2 (on the fix commit) found that the false-positive fix had regressed the six colon-bound PowerShell spellings, and that a new README sentence promised a cat > heredoc block that has never existed on either version. Both are fixed: the two-arm matcher and the corrected sentence.

Round 2 also raised a suspected gap in the shared guardrails-test-helpers.sh — that command_json omits MSYS_NO_PATHCONV and so rendered every MSYS assertion vacuous on Windows. That finding was wrong and is retracted here rather than left implied by the diff. MSYS argv rewriting converts an argument only when the argument is entirely a POSIX-absolute path: measured on a Windows host, --arg cmd '/c/tmp/x' becomes C:/tmp/x, while --arg cmd 'mkdir -p /c/tmp/x' and --arg cmd 'Set-Content -Path:/c/tmp/x -Value hi' pass through untouched. Every command fixture in every suite is multi-token, so nothing routed through command_json was ever mangled; only a lone bare path is exposed, and those go through the shared write_json family, which already sets MSYS_NO_PATHCONV deliberately and says so in its own comment. There is no shared-helper defect and no cross-plugin blast radius. The local command builders in this suite are kept and set the variable explicitly, so that a future lone-path fixture cannot silently become a drive-letter payload; their comment now states that reason instead of the retracted one.

Related

• Closes guardrails: block-windows-drive-tmp misses a Write, it only inspects tool_input.command #3501
• Filed from this work, not fixed here: block-windows-drive-tmp: non-Windows host gate leaves large payloads undrained on stdin #3504 — the non-Windows host gate now exits without draining stdin, so a payload past roughly 64KB is left undrained on Linux and macOS (measured: writer rc 141 at 256KB and 1MB, rc 0 on the Windows lane). Accepted rather than patched here: the harness already tolerates an undrained exit via hook::check_enabled's kill-switch path, a naive builtin drain can hang a tool call, and draining through buffer_stdin first would put its rc-2 fail-closed exit back ahead of the host gate — the exact bug this PR fixes. Whether hook-utils.sh should grow a shared bounded-drain helper for early-exit paths is a cross-hook decision, not a single-guard one.
• Filed from this work, not fixed here: guardrails: block-windows-drive-tmp misses a write utility invoked by absolute path #3502 — a write utility invoked by absolute path (/usr/bin/mkdir -p /tmp/x) is not blocked, because segment_writes_drive_root_tmp anchors its verb alternation on (^|[[:space:]]). Identical on origin/main and on this branch; a command-lane coverage gap, orthogonal to which payload shapes the guard inspects.
• guardrails: nothing catches a POSIX /tmp path resolving to a Windows drive-root C:\tmp #2594 — the original guard this one missed a door on
• docs/adr/0003-verification-guards-earn-default-on-by-measured-precision.md
• docs/conventions/hook-budget/README.md, .claude/rules/hook-budget.md
• docs/conventions/hook-precision/README.md — repro-first stay-quiet discipline

Three CI gate interactions worth recording, because a future author will hit the same triangle. All three were found by CI, not locally.

1. plugin-options-docs-gate — the guardrails README options table is generated from .claude-plugin/plugin.json userConfig. Hand-editing the README row passes markdownlint and fails the gate. The fix is to edit the userConfig description; scripts/sync-plugin-options-docs.py --check is the local pre-flight.
2. hygiene — the ci-workflows machine-specific-paths composite action rejects a realistic Windows user path in a test fixture (C:\Users\dev\AppData\Local\Temp\note.txt). The prescribed remedy is the <user> placeholder.
3. …which then trips shell-portability-lint, because \< is a GNU word boundary. The only spelling satisfying all three gates is forward slashes plus the placeholder: C:/Users/<user>/AppData/Local/Temp/note.txt. The fixture carries a comment recording all three constraints so it is not "simplified" back into a failure.

Not in this PR, deliberately. scripts/check-drive-root-litter.sh and scripts/check-shell-portability.sh are owned by other workers in the same program. One line in docs/conventions/windows-path-emit/README.md is corrected here because it now misdescribed this guard as reading only a command string; if that conflicts with the litter-detector worker's branch, take theirs and reapply the clause.

Measurement scripts are not committed (they would need a suite mapping for a throwaway); they are reproducible from the method stated in the README, and the sweep harness lifts both matchers from their own files rather than copying them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Egdf11hXdBch1HTFmjB8FV

[claude]

Claude finished @kyle-sexton's task in 5m 5s —— View job

---

Security Review

• Gather context (PR body, changed files)
• Invoke /review:security-review (plugins/review/skills/security-review/SKILL.md) and follow its criteria
• Review the security-relevant surface: plugins/guardrails/hooks/block-windows-drive-tmp.sh and plugins/guardrails/hooks/hooks.json
• Post findings

Skip gate: does not apply — this PR modifies a blocking security guard hook (block-windows-drive-tmp.sh) and its hooks.json registration, so it has security-relevant surface.

Scope reviewed (per skill instructions, changed files only):

• plugins/guardrails/hooks/block-windows-drive-tmp.sh — adds a file-path detection lane and reworks the MSYS-drive-colon boundary regex
• plugins/guardrails/hooks/hooks.json — new Write|Edit|MultiEdit|NotebookEdit matcher group
• Docs/schema/test/CHANGELOG files — reviewed for consistency, no independent security surface

What I checked:

• Fail-open/fail-closed posture — NUL handling (HOOK_JQ_FIELDS_NUL), jq-absence (hook::require_jq_blocking), and hook::buffer_stdin timeout all still fail closed with the two new fields (file_path, notebook_path) added to the same hook::jq_fields call. No new fail-open path introduced.
• Injection surfaces — no eval, no unsanitized interpolation into a re-executed command; jq -n --arg is used correctly for telemetry JSON construction; the intentional unquoted tokens=( $subject ) word-split in segment_destination_operand (pre-existing, unchanged by this diff) only builds an array, it does not execute anything.
• ReDoS — the new two-arm MSYS-colon regex (block-windows-drive-tmp.sh lines ~221–226) uses bounded, non-nested alternation; the char-by-char scanners (mask_quoted_redirect_ops, split_shell_segments) are linear. No catastrophic-backtracking shape.
• Detection-bypass (false negative) analysis on the new drive-spec discriminator — traced the boundary logic by hand against the PR's own claimed matrix (D:\a\tmp\x allowed, D:\tmp\x and the six colon-bound PowerShell writer forms still blocked, MSYS /c/tmp/x still blocked). The arithmetic holds: a real drive-root spelling is never routed through the newly-excluded "single alnum at a word boundary before :" branch, since a bare drive letter's \tmp root match comes from the separate, unmodified X:/tmp branch. I did not find a case where this diff weakens detection of an actual drive-root write relative to origin/main.
• hooks.json double-registration — block-windows-drive-tmp.sh now appears in two matcher groups (Write|Edit|MultiEdit|NotebookEdit and Bash|PowerShell), but the groups are disjoint on tool name, so a given tool call still triggers the script exactly once. No double-execution or ordering hazard.
• New attack surface from widening to Write/Edit — the file-path lane reads only file_path/notebook_path, never file content (content/new_string/new_source), so it doesn't introduce the NUL-in-file-body false-positive/DoS class the PR explicitly called out and avoided.
• Verified no secrets/tokens/credentials appear in the diff (docs, tests, CHANGELOG).

Result: no security issues found. This is a well-scoped hardening fix for a pre-existing detection gap (a Write payload bypassing a blocking guard because the guard only read .tool_input.command). The change closes that gap without introducing a new injection surface, ReDoS, or fail-open path. The one known limitation mentioned in the PR body (/usr/bin/mkdir -p /tmp/x invoked by absolute path) is pre-existing on origin/main and already tracked separately as #3502 — out of scope for this diff per the high-signal bar (exclude pre-existing issues).

[github-actions]

Last security-reviewed head: b11d006ddb81def4da8a994305bcedb7aea0995d. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

Claude finished @kyle-sexton's task in 40s —— View job

---

Security Review

• Gather context (PR body, changed files, prior review comment)
• Determine skip-gate applicability (new commit since last reviewed head)
• Review security-relevant surface for commits since 04ffd34
• Post findings

Skip gate applies. The prior review comment on this PR recorded 04ffd348b4d5967c4bc64206dd9636c0ac0ba33d as the last security-reviewed head. The only commit since then is a188792 ("satisfy the options-docs and machine-paths gates on the widened guard"), and its full diff is:

• plugins/guardrails/.claude-plugin/plugin.json — updates the block_windows_drive_tmp_enabled setting's description string to mention the new Write/Edit/MultiEdit/NotebookEdit lane. Documentation text only, no behavioral or logic change.
• plugins/guardrails/hooks/block-windows-drive-tmp.test.sh — swaps one test fixture's literal path (C:\Users\dev\AppData\Local\Temp\note.txt → C:/Users/<user>/AppData/Local/Temp/note.txt) to satisfy this repo's check-shell-portability.sh and machine-specific-paths CI gates. Same test case, same expected exit code (0, allowed), no assertion logic changed.

Neither change touches the guard's matching logic, hooks.json registration, telemetry, or any other security-relevant surface — this is condition 2 of the skip gate (no security-relevant surface in the new commits) combined with condition 3 (the prior review, which did cover block-windows-drive-tmp.sh and hooks.json in full, still applies to those files since they are unchanged since 04ffd34).

No new security review needed; the prior review's "no security issues found" verdict stands.

[claude]

Claude finished @kyle-sexton's task in 53s —— View job

---

Security Review

• Gather context (PR body, changed files, prior review comments)
• Invoke /review:security-review and follow its instructions exactly
• Determine skip-gate applicability for HEAD SHA 158ca9e
• Post findings

Skip gate applies. The prior review comment on this PR recorded a1887921d8bea537c4f4fee8cbdb8078fc9b540f as the last security-reviewed head. The only commit since then is 158ca9e ("docs(guardrails): retract the incorrect command_json pathconv finding"), and its full diff is:

• plugins/guardrails/CHANGELOG.md — rewrites a Notes paragraph to retract an incorrect claim from a prior verification round (that command_json omitted MSYS_NO_PATHCONV and made MSYS assertions vacuous). Documentation prose only.
• plugins/guardrails/hooks/block-windows-drive-tmp.test.sh — every changed line is a # comment above msys_command_json/msys_pwsh_command_json, correcting the rationale for why those local builders set MSYS_NO_PATHCONV explicitly. The builders' actual bodies (the MSYS_NO_PATHCONV=1 jq -n ... lines) are unchanged, and no assertions, fixtures, or expected exit codes changed. Confirmed by diff: 18 insertions / 13 deletions, entirely inside comment blocks.

Neither file touches the guard's matching logic (block-windows-drive-tmp.sh), the hooks.json registration, telemetry, or any other security-relevant surface — this is skip-gate condition 2 (no security-relevant surface in the new commit) combined with condition 3 (the prior review of block-windows-drive-tmp.sh and hooks.json, unchanged since a188792, still applies).

No new security review needed; the prior review's "no security issues found" verdict stands.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-31T11:30:14.028151Z	158ca9e	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 6m 28s —— View job

---

Code Review

• Gather context (PR body, changed files)
• Invoke /review:code-review and follow its instructions
• Review changed files for correctness/maintainability issues
• Post inline findings and summary

Skip gate: does not apply. This PR is open, not draft (the trigger was ready_for_review), the diff is a genuine hook-logic change with real review surface, and no prior code-review-lane review exists on this head (the three earlier comments on this PR are all from the security-review lane).

Scope reviewed (changed files only, correctness/maintainability — security is out of scope for this lane and already covered by the security-review lane's three passes):

• plugins/guardrails/hooks/block-windows-drive-tmp.sh — new file-path lane, two-arm MSYS-colon discriminator, budget micro-optimizations
• plugins/guardrails/hooks/hooks.json — new Write|Edit|MultiEdit|NotebookEdit matcher group
• plugins/guardrails/hooks/block-windows-drive-tmp.test.sh — new coverage
• plugins/guardrails/CHANGELOG.md, README.md, .claude-plugin/plugin.json, telemetry schema, docs/conventions/windows-path-emit/README.md

What I checked:

• Traced the two-arm drive-spec vs. parameter-colon regex (block-windows-drive-tmp.sh lines 221–226) by hand against the PR's own claimed matrix — drive specs (D:, D:, "D:, (D:), parameter colons (-Path:, -Destination:), the D:\a\tmp\x false-positive fix, the six colon-bound PowerShell MUST-fire cases, and the two accepted residuals (PATH=...:/c/tmp, ssh u@h:/c/tmp/x). Did not find a case where the new arms diverge from the PR's stated intent.
• Verified NORM/NORM_OUT global reuse between the file-path lane and command lane doesn't cross-contaminate (file-path lane's block() exits before the command lane runs when it matches; when it doesn't match, $NORM was already captured from $COMMAND before the file-path lane overwrites $NORM_OUT).
• Verified hooks.json's two matcher groups are disjoint on tool name, so a Write fires this hook exactly once.
• Verified the options-docs table and plugin.json userConfig description now match verbatim (the plugin-options-docs-gate fix from a188792).
• Confirmed via git grep and scripts/cross-plugin-source-registry.txt that the one factual claim below doesn't hold elsewhere in the diff.

Finding (posted inline):

• plugins/guardrails/CHANGELOG.md#L117-L119 — the Notes entry says guardrails-test-helpers.sh is "duplicated across plugins under a source-drift gate." That's not accurate: docs/conventions/shell-test-helpers/README.md explicitly places this file outside check-cross-plugin-source-drift.sh's scope, and it has no entry in scripts/cross-plugin-source-registry.txt. The duplication is real and intentional, but no automated gate enforces it — only the doc's convention does. Worth a reword so a future contributor doesn't assume a check will catch drift here.

Not re-litigated: the matcher-narrowing regex logic, MSYS pathconv behavior, and hook-budget measurements are already exhaustively self-verified in the PR body (1,702-probe sweep, two independent fresh-context rounds, security-review passes) — my own hand-trace agreed with those claims everywhere I checked, so I'm not restating that work as new findings.
· Branch

[claude]

Claude finished @kyle-sexton's task in 36s —— View job

---

Security Review

• Gather context (PR body, changed files, prior review comments)
• Determine skip-gate applicability for HEAD SHA 158ca9e
• Post findings

Skip gate applies. The most recent prior review on this PR already covered HEAD SHA 158ca9eb36265f97a1e73b6c3e8ac7fce48b3278 (see the comment above: "Last security-reviewed head: 158ca9eb36265f97a1e73b6c3e8ac7fce48b3278"), and the current HEAD SHA for this invocation is the identical commit 158ca9eb36265f97a1e73b6c3e8ac7fce48b3278. There are no new commits since that review to evaluate — this is skip-gate condition 3 (this head already has a successful security review that still applies).

That review covered the full security-relevant surface of this PR:

• plugins/guardrails/hooks/block-windows-drive-tmp.sh — new file-path detection lane and the two-arm MSYS-drive-colon boundary regex
• plugins/guardrails/hooks/hooks.json — new Write|Edit|MultiEdit|NotebookEdit matcher group

with the verdict: no security issues found (fail-closed posture preserved across the new fields, no injection surface, no ReDoS, no double-execution hazard, no new attack surface from the file-content-blind path lane).

No new security review needed; the prior review's "no security issues found" verdict stands.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 158ca9eb36

ℹ️ 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".

[github-actions]

Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @kyle-sexton's task in 1m 45s —— View job

---

Code Review

• Gather context (PR body, changed files, prior reviews)
• Invoke /review:code-review and follow its instructions
• Review changed files (correctness/maintainability)
• Post inline findings and summary

Skip gate: does not apply. The PR is open (ready for review), and this lane's last review on this PR covered head 158ca9e. HEAD is now 205fd68, one new commit ahead, so that prior review does not cover it.

Scope reviewed: the single new commit, 205fd68 ("run the host gate before stdin and the jq requirement"), touching:

• plugins/guardrails/hooks/block-windows-drive-tmp.sh
• plugins/guardrails/hooks/block-windows-drive-tmp.test.sh
• plugins/guardrails/CHANGELOG.md

This commit fixes the two outstanding findings on this PR: Codex's P1 finding (posted as a review comment on hooks.json:22) that a jq-less Linux/macOS host took require_jq_blocking's fail-closed exit 2 on every Write/Edit because the OSTYPE gate ran after stdin buffering and the jq requirement, and this lane's own prior finding that the CHANGELOG mischaracterized guardrails-test-helpers.sh as covered by an automated drift gate.

What I checked:

• Traced control flow in block-windows-drive-tmp.sh#L48-L74 by hand: the OSTYPE case now runs immediately after hook::check_enabled, before hook::buffer_stdin (line 89) and hook::require_jq_blocking (line 97). Confirmed hook::check_enabled itself (hook-utils.sh:44-46) only calls hook::is_enabled and exits — it never touches stdin, so nothing upstream of the new gate can drain or depend on the payload. A non-Windows host now hits exit 0 before either the stdin read or the jq check, fixing the reported bug.
• Confirmed the Windows path is genuinely unchanged: the case falls through unmodified when OSTYPE matches msys*|cygwin*|win32, so buffer_stdin, require_jq_blocking, NUL handling, and MAX_COMMAND_LEN all still run in their original relative order for the platform this guard actually protects.
• The new regression test (block-windows-drive-tmp.test.sh#L106-L124) asserts the fix behaviorally rather than by mocking a missing jq: it xtraces a real OSTYPE=linux-gnu run and asserts buffer_stdin/require_jq_blocking never appear in the trace. assert_absent and $HOOK are both pre-existing shared helpers/vars in this file, so the test is wired correctly. This is a reasonable approach given the PR's own stated constraint that an isolated no-jq bin dir isn't portable across Git Bash and Linux CI.
• The CHANGELOG correction (CHANGELOG.md#L131-L138) now matches docs/conventions/shell-test-helpers/README.md and the absence of guardrails-test-helpers.sh from scripts/cross-plugin-source-registry.txt — this resolves the finding from my prior pass on this PR.
• No other files changed in this commit; hooks.json and the rest of the surface reviewed in earlier passes are untouched.

Not independently run: I don't have shell-execution approval in this environment to run block-windows-drive-tmp.test.sh myself, so I can't independently confirm the PR's claimed PASS=183 FAIL=0. My review above is a hand-trace of the control flow and test logic, not a test-execution confirmation.

Result: no new findings. This commit correctly fixes both outstanding issues without introducing new ones.
· Branch

[claude]

Claude finished @kyle-sexton's task in 1m 4s —— View job

---

Security Review

• Gather context (PR body, changed files, prior review comments)
• Determine skip-gate applicability for HEAD SHA 205fd68
• Review security-relevant surface for commits since last reviewed head
• Post findings

Skip gate does not fully apply, but the change is narrow. The prior review comment recorded 158ca9eb36265f97a1e73b6c3e8ac7fce48b3278 as the last security-reviewed head. One commit has landed since: 205fd68 ("run the host gate before stdin and the jq requirement"), which fixes the P1 finding Codex raised on this PR: a jq-less non-Windows host was taking hook::require_jq_blocking's fail-closed exit 2 on every Write/Edit/MultiEdit/NotebookEdit call, because the OSTYPE host gate sat below hook::buffer_stdin and the jq requirement instead of above them.

What changed (block-windows-drive-tmp.sh#L48-L74):

• The case "${OSTYPE:-}" in msys*|cygwin*|win32) ;; *) exit 0 ;; esac gate moved to immediately after hook::check_enabled, ahead of hook::buffer_stdin and hook::require_jq_blocking.
• Verified hook::check_enabled (hook-utils.sh:44-46) only calls hook::is_enabled and exits — it does not touch stdin, so the comment's claim that this ordering "is not a new shape" holds.
• The Windows lane is untouched: buffer_stdin rc 2, jq absence, NUL handling, and MAX_COMMAND_LEN still run in the same order they did before, just after the new gate instead of before it.
• New regression coverage (block-windows-drive-tmp.test.sh) asserts via an xtrace of a real OSTYPE=linux-gnu run that neither buffer_stdin nor require_jq_blocking is reached — a reasonable substitute given jq-removal isn't portably simulable (matches the constraint require-jq-notice-isolation.test.sh already documents).

Security assessment of this commit:

• OSTYPE is a shell-set environment variable reflecting the actual host, not data derived from the tool-call payload — moving the branch earlier introduces no new attacker-controlled decision point and no spoofing/bypass surface.
• No change to the Windows detection logic, the two-arm MSYS-colon discriminator, or hooks.json registration — those were already reviewed and are unchanged since 158ca9e.
• This is a strict fail-open-repair: it removes an unintended fail-closed (blocking legitimate non-Windows file writes) without weakening the guard's fail-closed posture on Windows, where the guard's actual job is.
• Rest of the diff (CHANGELOG.md) is documentation/rationale text only, including the retraction/reword of the guardrails-test-helpers.sh drift-gate claim from the code-review lane's inline comment — no security surface.

No security issues found in the commits since the last review. This fix closes the gap the Codex bot identified cleanly, and does not introduce a new one.
· Branch

[claude]

Claude finished @kyle-sexton's task in 1m 9s —— View job

---

Security Review

• Gather context (PR body, changed files, prior review comments)
• Determine skip-gate applicability for HEAD SHA 1bddd98
• Review security-relevant surface for commits since last reviewed head
• Post findings

Skip gate applies. The prior review comment on this PR recorded 205fd68cce9951a51c2d736b456448b107bd67d9 as the last security-reviewed head. The only commit since then is 1bddd98 ("correct the jq-simulation claim and record the drain trade-off").

Its full diff, verified line-by-line:

• plugins/guardrails/hooks/block-windows-drive-tmp.sh#L60-L74 — every added line is a # comment (confirmed with git diff 205fd68..HEAD -- block-windows-drive-tmp.sh | grep '^+' | grep -v '^+#' → empty output). It documents, as an accepted trade-off, that the OSTYPE host gate added in 205fd68 now runs ahead of hook::buffer_stdin on non-Windows hosts, so stdin payloads over ~64KB go undrained there and a non-EPIPE-tolerant writer could see SIGPIPE. This is disclosure of an already-existing behavioral consequence of the prior commit, not a new code path.
• plugins/guardrails/hooks/block-windows-drive-tmp.test.sh#L109-L124 — same check, comment-only (empty diff for non-# added lines). Corrects the portability claim about simulating a jq-less PATH (it's host-dependent, not universally unsimulable) without changing the actual assertions (assert_absent ... buffer_stdin / require_jq_blocking are unchanged).
• plugins/guardrails/CHANGELOG.md — prose only: elaborates the jq-simulation correction and adds a "deliberate posture change" note making the same SIGPIPE/undrained-stdin trade-off explicit in the changelog.

No line of executable logic changed in this commit — this is condition 2 of the skip gate (no security-relevant surface in the new commit) combined with condition 3 (the prior review of the guard's actual control flow, unchanged since 205fd68, still applies).

On the newly-disclosed trade-off itself (not new, but since it's freshly documented, worth confirming it doesn't change the verdict): an unbuffered large stdin payload on a non-Windows host causing SIGPIPE in a non-EPIPE-tolerant caller is an availability/robustness concern for the calling tool, not an exploitable path for the guard itself — there's no attacker-controlled bypass of the Windows-side drive-root detection, and the guard's own fail-closed posture on Windows (where it actually has an opinion) is explicitly unchanged. Consistent with the prior review's assessment of 205fd68.

No new security review needed; the prior review's "no security issues found" verdict stands.
· Branch

[claude]

Claude finished @kyle-sexton's task in 1m 58s —— View job

---

Code Review

• Gather context (PR body, changed files, prior reviews)
• Invoke /review:code-review and follow its instructions
• Review changed files (correctness/maintainability)
• Post summary

Skip gate: condition 3 applies to the delta under review. The PR is open (not draft), and this lane's last review covered head 205fd68. HEAD is now 1bddd98, one commit ahead, so that prior review doesn't formally cover it — but the new commit (1bddd98, "correct the jq-simulation claim and record the drain trade-off") is comment- and CHANGELOG-only by its own description, and I confirmed that against the actual diff: every changed line in block-windows-drive-tmp.sh and block-windows-drive-tmp.test.sh is inside a # comment block, and CHANGELOG.md is prose. No executable line changed, so there is no code-behavior review surface in this commit.

What I checked anyway, since comment accuracy is itself a maintainability concern:

• Hand-traced the two claims the new comment/CHANGELOG prose makes and checked them against the actual control flow (I don't have shell-execution approval in this lane, so this is static verification, not a test run):
  • "the previous commit exits 2 on a Linux Write and this one exits 0" — confirmed from 205fd68's diff: before that commit, the OSTYPE case sat below the [[ -n "$COMMAND" || -n "$FILE_PATH" ]] || exit 0 line, which is itself below hook::require_jq_blocking. A jq-less non-Windows host would hit the fail-closed jq check first. 205fd68 moved the OSTYPE gate to run immediately after hook::check_enabled, ahead of buffer_stdin/require_jq_blocking (current code, lines 48–90). The claim holds.
  • Undrained-stdin exposure on non-Windows hosts — confirmed the code path: case "${OSTYPE:-}" in ... *) exit 0 ;; esac (line 88) runs before hook::buffer_stdin (line 89), so a non-Windows host now exits without reading fd0 at all. The new comment's framing (established shape via check_enabled, but a wider exposure since this gate now fires on every non-Windows tool call rather than only when the guard is disabled) matches the code.
• Confirmed the CHANGELOG's guardrails-test-helpers.sh bullet (lines 140–147) now states the corrected governance claim — "Nothing gates the duplication, so it is not edited from here as a matter of convention rather than tooling" — resolving the finding from my prior pass on this PR (inline comment on the earlier CHANGELOG wording).
• No other files changed in this commit.

Result: no findings. This commit is a documentation correction (retracting an overstated "not portably simulable" claim and recording a known/accepted stdin-drain trade-off as its own filed issue, #3504) with no logic change. Everything it asserts about the code matches the code as I read it.

Not re-litigated: the matcher logic, hooks.json registration, and prior CHANGELOG issue were reviewed in earlier passes on this PR and are unchanged since then.

Branch

[claude]

Claude finished @kyle-sexton's task in 1m 25s —— View job

---

Code Review

• Gather context (PR body, changed files, prior reviews)
• Invoke /review:code-review and follow its instructions
• Review changed files (correctness/maintainability)
• Post inline findings and summary

Skip gate: does not apply outright (HEAD moved since the last code-review-lane pass on 1bddd98), but the delta is trivial. PR is open/ready, not draft.

Scope reviewed: everything since the last code-review-lane head 1bddd98, i.e. commits 55225d3 and the merge abbb371:

• 55225d3 — touches plugins/guardrails/hooks/block-windows-drive-tmp.sh and plugins/guardrails/CHANGELOG.md. Confirmed by diff both changed lines are the identical one-word comment/prose fix, unparseable → unparsable (the repo's typos gate rejects the other spelling). No executable line touched.
• abbb371 — merge of origin/main into this branch, bringing in an unrelated machine-health feature (#3498, drive-root-litter check) and its own edits to docs/conventions/windows-path-emit/README.md.

What I checked on the merge specifically, since a merge into a long-running branch is where content can silently get clobbered:

• docs/conventions/windows-path-emit/README.md is touched by both origin/main (which added the whole file fresh via #3498) and this PR's own earlier commit (7c21cdc, which changed the "different concern from block-windows-drive-tmp.sh" paragraph to describe the new Write/Edit/MultiEdit/NotebookEdit lane). Post-merge, the paragraph at lines 199–205 correctly retains this PR's wording ("blocks a tool call... since guardrails 0.30.0 a Write/Edit/MultiEdit/NotebookEdit target path as well"), not origin/main's stale "blocks a command" text. The version number cited (0.30.0) matches plugins/guardrails/.claude-plugin/plugin.json's current version, which this PR itself bumps from 0.29.24 — consistent.
• Grepped for leftover conflict markers (<<<<<<</=======/>>>>>>>) across docs/conventions and plugins/guardrails — none found (the ==== hits are pre-existing test-file section banners, not conflict artifacts).
• Confirmed plugins/guardrails/hooks/block-windows-drive-tmp.sh, hooks.json, and block-windows-drive-tmp.test.sh are otherwise byte-identical since 1bddd98 (only the one-word comment diff above) — the merge did not touch these at all, since origin/main's incoming commits don't overlap them.
• The unrelated origin/main content pulled in (machine-health drive-root-litter check, source-control worktree doc tweak) is out of scope for this PR's own diff and isn't reviewed here — it's already-merged, already-reviewed main history.

Result: no findings. This delta is a one-word comment/CHANGELOG spelling correction plus a clean merge from main that correctly preserved this PR's own edits to the one file both sides touched.

Not re-litigated: the matcher logic, hooks.json registration, host-gate ordering, and the guardrails-test-helpers.sh CHANGELOG wording were reviewed and resolved in earlier passes on this PR (see prior review comments above) and are unchanged in this delta.
· Branch

[claude]

Claude finished @kyle-sexton's task in 51s —— View job

---

Security Review

• Gather context (PR body, changed files, prior review comments)
• Determine skip-gate applicability for HEAD SHA abbb371
• Review security-relevant surface for commits since last reviewed head
• Post findings

Skip gate applies. The prior review comment on this PR recorded 1bddd989c3f8415dff3c9fad213f231edc9f2ac7 as the last security-reviewed head. Two commits landed since then:

1. 55225d3 ("spell unparsable per the repo typos gate") — touches plugins/guardrails/CHANGELOG.md and plugins/guardrails/hooks/block-windows-drive-tmp.sh. The entire diff is a one-word spelling fix inside a # comment (unparseable → unparsable, to satisfy the repo's typos gate). No executable line changed.
2. abbb371 (merge of origin/main into this branch) — pulls in 949c54b (machine-health drive-root-litter feature) and other unrelated main content. Confirmed by diff that this merge touches none of this PR's guardrails hook logic:
   • plugins/guardrails/hooks/block-windows-drive-tmp.sh and plugins/guardrails/hooks/hooks.json — both byte-identical to 1bddd98 (git diff 1bddd98 abbb371 -- plugins/guardrails/hooks/hooks.json is empty; the .sh file's only change across the full range is the comment fix in commit 1 above).
   • The merge's only overlap with this PR's own changed files is docs/conventions/windows-path-emit/README.md, which gained a second detection-net bullet for scripts/check-drive-root-litter.sh's new temp-sink-name class (from main's unrelated machine-health PR feat(machine-health): add drive-root-litter check surfacing stray volume-root entries #3498) — documentation about a different script, not a change to block-windows-drive-tmp.sh's behavior.
   • The rest of the merge's diff (machine-health/**, source-control/**, scripts/check-drive-root-litter*.sh) belongs to main's already-landed, unrelated PR and is out of scope for this review (not part of this PR's own diff against origin/main).

Neither commit touches the guard's matching logic, fail-closed posture, telemetry, or the hooks.json registration — this is skip-gate condition 2 (no security-relevant surface in the new commits) combined with condition 3 (the prior review of block-windows-drive-tmp.sh and hooks.json, unchanged in substance since 1bddd98, still applies).

No new security review needed; the prior review's "no security issues found" verdict stands.
· Branch

[claude]

Claude finished @kyle-sexton's task in 38s —— View job

---

Security Review

• Gather context (PR body, changed files, prior review comments)
• Invoke /review:security-review and follow its instructions
• Determine skip-gate applicability for HEAD SHA b11d006
• Review security-relevant surface for commits since last reviewed head
• Post findings

Skip gate applies. The prior review comment on this PR recorded abbb371853545e11f0520d34f7eab22946c800e0 as the last security-reviewed head. The only commit since then is b11d006 ("drop the user path from the host-gate regression fixture").

Its full diff, verified directly:

- trace=$(env OSTYPE=linux-gnu bash -x "$HOOK" <<<"$(write_json '/home/u/x.txt' 'body')" 2>&1 >/dev/null)
+ trace=$(env OSTYPE=linux-gnu bash -x "$HOOK" <<<"$(write_json '/srv/app/notes.txt' 'body')" 2>&1 >/dev/null)

One line, in plugins/guardrails/hooks/block-windows-drive-tmp.test.sh#L125. It swaps one test fixture's literal file_path value (/home/u/x.txt → /srv/app/notes.txt) to satisfy this repo's machine-specific-paths hygiene gate, which flags /home/u/... as a realistic Linux user path. The assertions themselves (assert_absent ... buffer_stdin / require_jq_blocking, confirming the non-Windows host gate short-circuits before stdin buffering and the jq requirement) are unchanged — only the input literal used to exercise them changed, and both values are equally non-drive-root-tmp paths on a Linux host, so the test's behavior and coverage are identical.

No change to block-windows-drive-tmp.sh's matching logic, the hooks.json registration, telemetry, or any other security-relevant surface — this is skip-gate condition 2 (no security-relevant surface in the new commit) combined with condition 3 (the prior review of the guard's control flow, unchanged since abbb371, still applies).

No new security review needed; the prior review's "no security issues found" verdict stands.

Branch ·

[claude]

Claude finished @kyle-sexton's task in 49s —— View job

---

Code Review

• Gather context (PR body, changed files, prior reviews)
• Invoke /review:code-review and follow its instructions
• Review changed files (correctness/maintainability)
• Post summary

Skip gate: condition 3 applies to the delta under review. PR is open (ready for review, not draft). This lane's last review covered head abbb371 (commits 55225d3 + the merge). HEAD is now b11d006, one commit ahead.

Scope reviewed: the single new commit, b11d006 ("drop the user path from the host-gate regression fixture"), the only change since the last code-review-lane pass:

• plugins/guardrails/hooks/block-windows-drive-tmp.test.sh#L125 — swaps the regression fixture's Linux-host Write path literal from /home/u/x.txt to /srv/app/notes.txt, to satisfy the machine-specific-paths hygiene gate (which flags /home/<user>/... as a machine-specific path). One line, one insertion/one deletion.

What I checked:

• The assertion this fixture feeds (assert_absent ... buffer_stdin / require_jq_blocking, added in 205fd68) only depends on the payload being routed through the Linux-host branch of the hoisted OSTYPE gate — it doesn't depend on the path's shape at all, since the whole point is that buffer_stdin/require_jq_blocking are never reached before the gate exits. /srv/app/notes.txt satisfies this exactly as well as the path it replaced.
• Confirmed /srv/app/notes.txt doesn't collide with the guard's own drive-root detection (no drive letter, no tmp segment), so it can't accidentally flip the test's expected outcome.
• Grepped the file for the old literal — no other reference to /home/u/x.txt was left behind after the swap.
• No other files changed in this commit; the rest of the diff (hooks.json, the matcher logic, CHANGELOG.md, etc.) is unchanged since the prior pass and was already reviewed.

Result: no findings. This is a correct, minimal test-fixture fix for a CI hygiene-gate false positive, consistent with the PR body's own account of this exact issue ("Three CI gate interactions worth recording").

Not re-litigated: the matcher logic, hooks.json registration, host-gate ordering, and the guardrails-test-helpers.sh CHANGELOG wording were reviewed and resolved in earlier passes on this PR and are unchanged in this delta.

Branch ·