diff --git a/docs/conventions/hook-telemetry/data/block-windows-drive-tmp.schema.json b/docs/conventions/hook-telemetry/data/block-windows-drive-tmp.schema.json index dfd88fd24..0bf5f710c 100644 --- a/docs/conventions/hook-telemetry/data/block-windows-drive-tmp.schema.json +++ b/docs/conventions/hook-telemetry/data/block-windows-drive-tmp.schema.json @@ -9,15 +9,15 @@ "properties": { "tool": { "type": "string", - "description": "The invoking tool — \"Bash\" or \"PowerShell\"; this guard matches both." + "description": "The invoking tool — \"Bash\" or \"PowerShell\" on the command lane, or \"Write\" / \"Edit\" / \"MultiEdit\" / \"NotebookEdit\" on the file-path lane; this guard matches all of them." }, "subject": { "type": "string", - "description": "Privacy-safe command subject: `Bash:` for a Bash call, with leading sudo / env-assignment prefixes stripped and the token basenamed; the bare tool name (`PowerShell`) for a PowerShell call, which is not tokenized. NEVER the full command or its arguments." + "description": "Privacy-safe command subject: `Bash:` for a Bash call, with leading sudo / env-assignment prefixes stripped and the token basenamed; the bare tool name (`PowerShell`, `Write`, `Edit`, `MultiEdit`, `NotebookEdit`) for every other tool, which is not tokenized. NEVER the full command, the written file path, or their arguments." }, "form": { "type": "string", - "description": "The matched form when blocked: \"redirect\" | \"write-utility\" | \"too-long\" (command exceeded the parse cap and was blocked fail-closed). Empty string when the command was allowed (status ok)." + "description": "The matched form when blocked: \"redirect\" | \"write-utility\" | \"too-long\" (command exceeded the parse cap and was blocked fail-closed) | \"file-path\" (a Write/Edit/MultiEdit/NotebookEdit target path was a drive-root temp path). Empty string when the call was allowed (status ok)." } } } diff --git a/docs/conventions/windows-path-emit/README.md b/docs/conventions/windows-path-emit/README.md index 3a36d5c96..22259a9a8 100644 --- a/docs/conventions/windows-path-emit/README.md +++ b/docs/conventions/windows-path-emit/README.md @@ -198,10 +198,11 @@ one costs a follow-up. Promote it when there is precision to point at. This is a different concern from [`plugins/guardrails/hooks/block-windows-drive-tmp.sh`](../../../plugins/guardrails/hooks/block-windows-drive-tmp.sh), -which blocks a *command* aimed at a drive-root temp path before it runs (#2594). That guard reads a -command string ahead of time; this detector reads the filesystem afterwards, and catches the class -where the offending path was never spelled in a command at all — it was computed inside a native -interpreter. +which blocks a *tool call* aimed at a drive-root temp path before it runs (#2594). That guard reads +the payload ahead of time — a Bash/PowerShell command string, and since guardrails 0.30.0 a +Write/Edit/MultiEdit/NotebookEdit target path as well; this detector reads the filesystem +afterwards, and catches the class where the offending path was never spelled in the payload at all — +it was computed inside a native interpreter. It is also outside the charter of [`scripts/check-shell-portability.sh`](../../../scripts/check-shell-portability.sh), whose token list diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index d0d6fe34c..27c08d070 100644 --- a/plugins/guardrails/.claude-plugin/plugin.json +++ b/plugins/guardrails/.claude-plugin/plugin.json @@ -52,7 +52,7 @@ "block_windows_drive_tmp_enabled": { "type": "boolean", "title": "block-windows-drive-tmp guard", - "description": "Block Bash/PowerShell writes whose target is a Windows drive-root temp path (/tmp, C:\\tmp, \\tmp, /c/tmp) that resolves to :\\tmp instead of %TEMP%", + "description": "Block writes whose target is a Windows drive-root temp path (/tmp, C:\\tmp, \\tmp, /c/tmp) that resolves to :\\tmp instead of %TEMP% — both Bash/PowerShell commands and Write/Edit/MultiEdit/NotebookEdit file paths. One switch covers both lanes", "default": true }, "block_exported_msys_pathconv_enabled": { @@ -147,5 +147,5 @@ "min": 1 } }, - "version": "0.29.24" + "version": "0.30.0" } diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index 9424a4a0a..cd1cd967c 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,6 +3,153 @@ All notable changes to the `guardrails` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.30.0] + +### Fixed + +- **`block-windows-drive-tmp` now sees a `Write`, not only a command.** The guard + exists to stop a Windows drive-root temp write (#2594), and it missed one: on + 2026-08-30 an empty `C:\tmp\tmp.rSFIkHm5DO` was created and nothing fired. The + hook read `.tool_input.command`; a `Write` payload carries `file_path` instead, + so `hook::jq_fields` produced an empty `COMMAND` and the `[[ -n "$COMMAND" ]]` + early exit returned before any matcher ran. The guard covered command-shaped + writes and not tool-shaped ones. It now also reads `.tool_input.file_path` and + `.tool_input.notebook_path`, and the early exit tests both doors. Verified + repro-first: the four seeded spellings (`C:\tmp\…`, `/tmp/…`, `/c/tmp/…`, + `C:/tmp/…`) exit 0 against the unmodified hook and 2 against this one. +- **A jq-less Linux or macOS host can edit files again.** Widening the matcher + to `Write`/`Edit`/`MultiEdit`/`NotebookEdit` had a consequence the widening + itself did not carry: the non-Windows host gate sat BELOW `hook::buffer_stdin` + and `hook::require_jq_blocking`, so on a host without `jq` on `PATH` every + file edit took the fail-closed `exit 2` — on a platform where `/tmp` is the + real POSIX temp and this guard can never find a violation. The host gate now + runs first, immediately after `hook::check_enabled` (which already exits + without draining stdin, so the shape is not new). Reading `OSTYPE` needs + nothing from the payload. **The Windows path is unchanged**: the case falls + through and `buffer_stdin` rc 2, jq absence, an unparsable payload, NUL + bytes and `MAX_COMMAND_LEN` all still fail closed in the same order. + Confirmed on a real jq-less `PATH`: the previous commit exits 2 on a Linux + `Write` and this one exits 0. That simulation is host-dependent (it needs + `jq` in a directory that does not also host `bash` and coreutils), which is + the portability constraint `require-jq-notice-isolation.test.sh` records, so + the committed regression test does not rely on it and asserts the ordering + from an xtrace instead: neither `buffer_stdin` nor `require_jq_blocking` is + reached. Both appear before the fix and neither after it. +- **Deliberate posture change, 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 #2146 posture is kept in full on Windows. It is dropped only + where the guard has no opinion at all — `/tmp` is the real POSIX temp there, + so every exit 2 it produced was a false positive by construction. +- **A `tmp` directory under a single-letter parent no longer blocks.** + `D:\a\tmp\x` matched: after slash-normalization the drive colon satisfied the + left boundary of the MSYS `//tmp` alternative, so `d:` + `/a/tmp` read + as a drive root. The identical MSYS spelling `/d/a/tmp/x` was allowed the + whole time, so one sink decided two ways. The defect predates the file-path + lane — the command lane blocked `mkdir -p D:\a\tmp\x` too — but the lane made + it reachable from every write, so it is fixed here rather than inherited. + The MSYS alternative is now two arms, because a `:` on the left is ambiguous + and the two readings decide oppositely. A DRIVE SPEC — exactly one + alphanumeric at a word boundary — no longer satisfies the boundary; every + other colon still does, including a PowerShell PARAMETER colon, so + `Set-Content -Path:/c/tmp/x` keeps blocking exactly as its space-bound twin + does. Excluding `:` outright, which a first attempt did, would have dropped + that whole class. The narrowing was then swept exhaustively against the + shipped matcher — every ASCII printable as the immediate left neighbour, + every two-character context ending in a colon, nine drive letters in eight + surrounding contexts, and the colon-bearing shapes a sweep alone does not + reach, 1,702 probes — and **all 266 changed verdicts are the drive-spec + reading**: 192 `:` contexts, 72 explicit `X:\a\tmp` probes, + the leading bare `:`, and `D:\a\tmp` itself. Every real drive-root spelling + still matches. Two accepted residuals, both unchanged from the shipped guard + rather than introduced: a PATH-style list (`PATH=/usr/bin:/c/tmp cmd`) + presents the multi-character token shape and still matches, and a remote spec + with a single-letter host (`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. Pinned repro-first on both lanes. + +### Added + +- **A `Write | Edit | MultiEdit | NotebookEdit` matcher registration for the same + guard.** The script change alone would have been inert: without the second + `hooks.json` registration the hook never receives those payloads. It is a + SEPARATE PreToolUse group, not a widening of the existing `Write|Edit| + NotebookEdit` matcher string, so `secret-pattern-detection` and + `hardcoded-path-check` do not silently acquire `MultiEdit`, and not a widening + of `Bash|PowerShell`, which would have attached seven command-lane guards to + every file write. Claude Code fires every group whose matcher matches, so a + `Write` now matches two guardrails groups — but this hook appears in exactly + one of them, so it still fires once per tool call. +- **A `file-path` telemetry form**, alongside `redirect` / `write-utility` / + `too-long`. The privacy floor is unchanged: `subject` is the bare tool name on + this lane (`Write`), and the target path never reaches the envelope. + +### Changed + +- **Both doors feed one matcher.** The file-path lane calls the shipped + `has_drive_root_tmp()` — there is no second matcher — so every spelling the + command lane blocks and every one it permits (`%TEMP%` expansions, `/var/tmp`, + `./tmp`, `foo/tmp`, `/tmpdir`, `C:/tmp2`, UNC `\\server\tmp`) decides + identically on a `Write`. The lane needs none of the command lane's inference: + on `Write`/`Edit` the path IS the write target, so there is no redirect to + parse, no producer-utility whitelist, and no segment splitting. +- **Case folding is pure shell.** `printf | tr` was a fork AND an exec (~280 ms + together on Windows Git Bash) to fold one character class; `${var,,}` does the + same work in-process. That removes two spawns from the existing per-Bash-call + cost as well, and keeps the new per-Write lane from adding them. +- **The command lane is skipped outright on a file-path payload.** + `has_redirect_to_drive_root_tmp` runs `mask_quoted_redirect_ops` in a command + substitution, and forking to scan an empty string would be per-Write budget + spent to reach a foregone answer. +- **The telemetry subject resolves inside `emit_tel`.** `hook::extract_bash_subject` + runs in a command substitution, and that fork was paid on every tool call even + with no telemetry sink wired — the default — and would now be paid on every + `Write` to obtain a constant, since the helper returns the bare tool name for + any tool but Bash. Same shape as the plugin's other lazily-resolved telemetry + fields. + +### Notes + +- **Fail-closed posture is unchanged and now covers the new field.** NUL-byte + handling, `hook::buffer_stdin` rc 2, jq absence and `MAX_COMMAND_LEN` all + behave exactly as before; a NUL in `file_path` fails closed by the same + already-shipped check, because only PATH fields were added to the + `hook::jq_fields` call. Content fields (`content` / `new_string` / + `new_source`) are deliberately NOT requested: `HOOK_JQ_FIELDS_NUL` is computed + across every requested field, so reading them would make this guard block on a + NUL anywhere in a file body — hardcoded-path-check's concern, not this one's. +- **No length ceiling on the file-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 three EREs with no tokenization, detection + does not degrade with length, and a blocking ceiling would only add a + false-positive class. The payload stays bounded by `hook::buffer_stdin`. +- **Measured budget share** for the widened surface is recorded in the README's + hook-budget accounting, alongside the ADR 0003 sweep for the new lane. +- **Two test-fidelity gaps closed alongside the lane.** The `hooks.json` + registration — the half of this change without which the script edit is inert + — is now asserted, by splitting the matcher on `|` and comparing the exact + alternative set rather than substring-searching it (a containment test for + `Edit` can never fail while `MultiEdit` passes, and a matcher with the pipes + removed routes nothing while satisfying every containment check). And the MSYS + `//tmp` cases build their payloads through local builders that set + `MSYS_NO_PATHCONV` explicitly. MSYS argv rewriting converts an argument only + when the argument is *entirely* a POSIX-absolute path, so `/c/tmp/x` becomes + `C:/tmp/x` while `mkdir -p /c/tmp/x` passes through untouched. Every command + fixture here is multi-token and was therefore already safe through the shared + `command_json`; setting it explicitly keeps a future lone-path fixture from + silently becoming a drive-letter payload. The shared helper's path-payload + builders (`write_json` and siblings) already set it, and the helper is + duplicated per plugin **by convention**, per + `docs/conventions/shell-test-helpers/README.md` — it is explicitly outside + `check-cross-plugin-source-drift.sh`'s scope (the copies live at different + paths per plugin and are not byte-identical, so `discover` never flags them + as a cluster) and has no entry in `scripts/cross-plugin-source-registry.txt`. + Nothing gates the duplication, so it is not edited from here as a matter of + convention rather than tooling. +- **The `file-path` telemetry form is now executed, not just documented.** The + existing telemetry case pipes a Bash payload; a file-path case asserts the + `Write` / `Write` / `file-path` envelope and that the target path never + reaches it. + ## [0.29.24] ### Fixed diff --git a/plugins/guardrails/README.md b/plugins/guardrails/README.md index 2d4e60727..7fa157f1b 100644 --- a/plugins/guardrails/README.md +++ b/plugins/guardrails/README.md @@ -29,7 +29,7 @@ Each guard is independently toggleable, so you run exactly the subset you want. | **block-no-verify** | PreToolUse · Bash \| PowerShell | **Blocks** (exit 2) | Git hook-bypass attempts on `git commit` / `git push`: `--no-verify` / `-n`, `core.hooksPath=` assignment, and hook-manager disable env vars, a configurable prefix set defaulting to `lefthook`, `husky`, `pre_commit`, `simple_git_hooks` (e.g. `LEFTHOOK=0`, `HUSKY=0`, `PRE_COMMIT_*=false`), tunable via `block_no_verify_hook_manager_prefixes`, including inside compound `cd … && …` commands. | | **block-dangerous-git** | PreToolUse · Bash \| PowerShell | **Blocks** (exit 2) | Irreversible git operations: `push --force`/`-f` plus the equivalent leading-`+` refspec and `--mirror` forms, and the unsafe `--force-with-lease` spellings, in the two kinds git itself treats differently. **No expected value** (bare `--force-with-lease` or `=`) leases against the remote-tracking ref, which git documents as "trivially defeated" by a background fetch, blocked unless `--force-if-includes` is present, which git documents as the mitigation for exactly this form. **A movable `=:`**, such as `origin/main`, `HEAD`, a tag, an *abbreviated* object id, or hex of the wrong width for this repository's hash format, all of which git resolves at push time, and gitrevisions resolves a short hex word as a ref before trying it as an object-id prefix, is blocked unconditionally, because git declares `--force-if-includes` a no-op alongside an explicit `:`. A lease passes only when `` is immutable: a **literal** object id of the pushed repository's own hash width (detection never evaluates substitutions, so resolve it with `git rev-parse` as a separate step and pass the result) (40 hex under SHA-1, 64 under SHA-256, read from `git rev-parse --show-object-format` with the command's own `-C`/`--git-dir`/`--work-tree`/`--namespace` replayed onto it; undeterminable fails closed) or the empty string asserting the ref must not exist. The other width is a ref name there, not an object id. git ignores a ref whose name is full-width hex for its own format, but resolves one of the other width like any name. git scopes a pin to its own ref, so a bare fallback alongside a pinned entry still governs every other ref being updated; where the same ref carries several lease entries, git consults the first, and so does this guard. A trailing `--no-force-with-lease` cancels every previous lease, and a push dry-run disarms the check. Also blocked: `reset --hard`, `clean` with a force flag (any dry-run flag disarms), worktree-wide `checkout`/`restore` pathspecs (`.`, `:/`, `:(top…)`; path-scoped forms and `restore --staged .` pass), and forced `checkout -f` / `switch --discard-changes`. Accepted unique-prefix abbreviations of the blocked long options match too. `branch -D` is deliberately not blocked (reflog-recoverable; sanctioned skill flows issue it). Per-repo/per-user allow-list via the `block_dangerous_git_allow` userConfig option (comma list, any subset of `push-force,push-lease-unsafe,reset-hard,clean-force,checkout-dot,restore-dot,checkout-force`). | | **block-hook-bypass** | PreToolUse · Bash \| PowerShell | **Blocks** (exit 2) | Bash file-write workarounds that circumvent the Write/Edit hook gates: `cat > file`, `echo … > file`, inline python code with file-write indicators (`python`/`python3`/`py`/`pypy`, with `-c` or reading the program from stdin as `python3 - <:\tmp` instead of `%TEMP%` and accumulate at the volume root. Redirects and write utilities (`mkdir`/`mktemp`/`tee`/`cp`/`Set-Content`/`Out-File`/…) are blocked with a redirect-to-`%TEMP%` message. Does not fire on non-Windows hosts; leaves `%TEMP%` / `$TEMP` / `$TMPDIR` / `$env:TEMP` / `/var/tmp` alone. | +| **block-windows-drive-tmp** | PreToolUse · Bash \| PowerShell **and** Write \| Edit \| MultiEdit \| NotebookEdit | **Blocks** (exit 2) | Windows-only: write targets that are a drive-root temp path: POSIX `/tmp`, MSYS `/c/tmp`, `C:\tmp`, or drive-root `\tmp`, which resolve to `:\tmp` instead of `%TEMP%` and accumulate at the volume root. On the **command lane** redirects and write utilities (`mkdir`/`mktemp`/`tee`/`cp`/`Set-Content`/`Out-File`/…) are blocked; on the **file-path lane** (since **0.30.0**) the tool's own `file_path` / `notebook_path` is matched directly, because on a Write the path *is* the write target. Both lanes call one matcher, so the same spellings block and the same ones pass. Does not fire on non-Windows hosts; leaves `%TEMP%` / `$TEMP` / `$TMPDIR` / `$env:TEMP` / `/var/tmp`, relative `./tmp`, `foo/tmp`, `/tmpdir`, `C:/tmp2` and UNC `\\server\tmp` alone. | | **block-exported-msys-pathconv** | PreToolUse · Bash \| PowerShell | **Blocks** (exit 2) | Windows-only: an **exported** `MSYS_NO_PATHCONV` / `MSYS2_ARG_CONV_EXCL` (also the `declare -x` / `typeset -x` spellings), which switches off MSYS argv rewriting for every *later* command in the same command string. A later path argument then reaches a Windows-native program unconverted and git resolves its leading `/` against the current drive, so `git worktree add /d/worktrees/x` creates `:\d\worktrees\x` (#2870). Deliberately keys on the environment, not on a path shape: the incident command's path argument was identical to one that had already worked. A prefix whose command word is a **shell** (`MSYS_NO_PATHCONV=1 bash -c '…'`, `env … sh -c '…'`) blocks too: the prefix scopes to one *process*, and when that process is an interpreter, one process is every command inside it. A prefix on a **non-shell** command word (`MSYS_NO_PATHCONV=1 git show …`) and a bare assignment are not matched. The first scopes to exactly that command, and the second has no effect at all because the MSYS runtime reads the environment. Does not fire on non-Windows hosts. | | **cli-flag-verify** | PostToolUse · Write \| Edit | **Advisory** (exit 0) | Hallucinated CLI flags: a `--flag` written as a command that does not exist in the binary's actual `--help` output. Surfaces via `additionalContext`, never blocks. | | **workflow-resilience-check** | PreToolUse · Workflow | **Advisory** (exit 0) | Un-throttled Workflow fan-out: a script calling `parallel()` / `pipeline()` with no wave-cap throttle (`inWaves` / `inWavesPipeline`) and no retry wrapper (`agentRetry`), which risks a burst 529 under wide Opus fan-out. Surfaces a resilience checklist via `additionalContext`, never blocks. **Opt-in. Default off since 0.20.0** (behavioral-class injector config-disabled per #2021; set `workflow_resilience_check_enabled=true` to enable). | @@ -224,6 +224,134 @@ out of scope until such a signal exists. `trailer_policy` of `none`, so demanding it would block the skill's own conformant output in repos whose convention forbids co-author trailers. +- **`block-windows-drive-tmp` guards two doors with one matcher, and only one of + them existed before 0.30.0.** A write reaches the drive root either as a + command string (`echo x > /tmp/f`) or as a tool's own target path (`Write` + with `file_path: C:\tmp\f`). The hook read `.tool_input.command` only, so the + second shape hit an empty-`COMMAND` early exit and passed unexamined — a real + `C:\tmp\tmp.rSFIkHm5DO` was created on 2026-08-30 with no guard firing. Both + shapes now feed the shipped `has_drive_root_tmp()`; there is no second matcher + to drift. The file-path lane carries **none** of the command lane's + string-matching floor, because it needs none: on `Write`/`Edit` the path is + the write target by construction, so there is no redirect to parse, no + producer-utility whitelist, and no quoted-prose ambiguity. Its residual is + narrower than the command lane's and of a different kind: a path assembled at + runtime and passed by a tool this guard does not match — an MCP file-write + tool, or a Bash form the command lane's own residuals already allow. +- **`block-windows-drive-tmp`'s file-path lane shipped blocking on a measured + sweep, per [ADR 0003](../../docs/adr/0003-verification-guards-earn-default-on-by-measured-precision.md).** + Corpus: 259 distinct `file_path` / `notebook_path` values that a real `Write`, + `Edit`, `MultiEdit` or `NotebookEdit` actually carried across 227 local Claude + Code session transcripts on a Windows host — absolute Windows and MSYS paths, + not the repo-relative ones a drive-root matcher could never match, which is + what makes a low finding count informative here. **1 finding in 259 (0.39% + firing), and it was a true positive**: `/tmp/tmp.rSFIkHm5DO/worktree-root`, the + very write that produced the `C:\tmp\tmp.rSFIkHm5DO` this lane exists to stop. + Six seeded spellings were detected end to end. + + **What that evidence does and does not support, stated plainly.** Precision is + 1/1, so the ratio is 100% and the sample is one — this is the ADR's + near-zero-findings branch, where the seeded-detection burden carries the + argument and the precision figure by itself does not. The corpus is one + Windows host and one operator, so it is evidence about this deployment and + weaker evidence about others; it contains no `MultiEdit` or `NotebookEdit` + entries at all, and those two tools are covered by the contract suite and by + the shared matcher, not by the sweep. **The ratio considered acceptable for + this surface is a false-positive rate near zero, and the justification is that + the cost of a wrong block here is unusually low** — the agent gets a stderr + line naming `%TEMP%` and reissues the write, which is a second of friction, + against a missed write that is silent by construction and was found only by + noticing litter on a volume root days later. The near-misses that would + falsify the ratio (`%TEMP%` paths, `/var/tmp`, `docs/tmp`, `./tmp`, `foo/tmp`, + `/tmpdir`, `C:/tmp2`, UNC `\\host\tmp`, and a `tmp` directory under a + single-letter parent) are pinned as MUST-stay-quiet cases in the contract + suite. Compare the ADR's shipped reference guard, promoted at 0.51% firing and + 57% precision. Corpus counts are as of 2026-08-31 on the measuring host and + grow as that host accumulates sessions; the figure that matters is the ratio. +- **`block-windows-drive-tmp` reads the payload's PATH fields, never its + content.** `.tool_input.content` / `.new_string` / `.new_source` are + deliberately not requested. `HOOK_JQ_FIELDS_NUL` is computed across every + requested field, so pulling written content in would make this guard fail + closed on a NUL anywhere in a file body — that surface belongs to + `hardcoded-path-check` and `secret-pattern-detection`. A prose mention of + `/tmp` inside a written file is therefore never a block on this lane. The + Bash lane is scoped differently but reaches the same place: it sees only the + command string, and a drive-root path there still has to sit in a + write-shaped position, so a heredoc body carrying `C:\tmp` inside a + `cat > file` does not block either. +- **`block-windows-drive-tmp` puts no length ceiling on a file path, and that is + a decision.** `MAX_COMMAND_LEN` (16384) fails the command lane closed because + that lane walks its string character by character twice before matching + anything, so past some length the guard genuinely cannot say what would run. + The file-path lane runs three EREs over one string with no tokenization: a + drive-root prefix matches at any length, so length creates no parse ambiguity + and a blocking ceiling would only refuse legitimate long paths. The payload as + a whole stays bounded by `hook::buffer_stdin`, whose stall path fails closed. + +### Hook budget accounting + +Per [`docs/conventions/hook-budget/README.md`](../../docs/conventions/hook-budget/README.md) +rule 1, widening an always-on hook's matcher states its measured share of the +fleet budget. `block-windows-drive-tmp` moved from `Bash|PowerShell` to that set +plus `Write|Edit|MultiEdit|NotebookEdit` in **0.30.0**, so the surface that +changed is the **per-`Write` tool call**, whose ceiling is ≤ 1 s typical / +≤ 2 s worst-case. + +**Method** (the convention's, unchanged): `EPOCHREALTIME` wall-clock around +direct hook invocation with a benign representative payload — a `Write` of a +short body to an ordinary repo path — sets launched concurrently (`&` + `wait`) +to approximate the harness's parallel dispatch. Windows 11 + Git Bash, +2026-08-30. + +**Host condition, stated because it changes how these numbers must be read.** +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**, +roughly 56× slower. Absolute milliseconds from this host are therefore not +comparable to the convention's figures. Every measurement below re-measures +`bash -c :` **interleaved with each trial** and reports the load-normalized +ratio (hook wall ÷ same-trial spawn baseline); the reference column converts +that ratio back at 80 ms. The spawn-equivalent figure is the stable one. + +| Measured (n=12 interleaved trials) | spawn-equivalents | @ 80 ms reference host | +| --- | --- | --- | +| `block-windows-drive-tmp` alone, one `Write` payload | 6.31 | ≈ 505 ms | +| guardrails per-`Write` PreToolUse set BEFORE (2 hooks, concurrent) | 12.59 | ≈ 1,007 ms | +| guardrails per-`Write` PreToolUse set AFTER (3 hooks, concurrent) | 12.34 | ≈ 987 ms | + +**The hook's own cost is the measurement that holds: ≈ 6.3 spawn-equivalents, +≈ 505 ms of reference-host work per `Write`.** The set rows are reported for +completeness and must not be read as a delta, because they do not resolve one — +`AFTER` measures *lower* than `BEFORE`, and adding a hook cannot make a set +faster. A separate **paired A/B** (n=15, BEFORE and AFTER launched back to back +inside each trial in alternating order so load drift biases both arms equally) +came out at a mean **1.26×**, but its per-trial ratios span **0.55×–1.82×** — +several trials put AFTER *faster* than BEFORE, which is physically impossible +and is the host's noise, not the hook's cost. **On this host the set-level delta +is below the noise floor and this accounting does not state one.** What can be +said: the harness dispatches matching hooks in parallel, so the set wall is the +max of its members rather than their sum, and a member costing ≈ 505 ms joining +a set already walling at ≈ 1 s cannot raise that wall by more than its own cost +and will usually raise it by less. A re-measurement on a quiet reference host is +the way to replace this bound with a number, and is the honest follow-up. + +**Share of the budget, and the overage.** The convention's ceiling is +≤ 1 s typical / ≤ 2 s worst-case **per tool call, counting `PreToolUse` and +`PostToolUse` together for one matcher** — so the surface this widening lands on +is larger than the table above measures: guardrails also runs three `PostToolUse` +verifiers on `Write|Edit`, and the fleet's binding accounting for the whole +per-`Write` set is **≈ 1.9 s** (two formatters plus three guardrails verifiers), +already over the typical ceiling before this change and deeper into overage than +the PreToolUse-only slice measured here suggests. Against that surface the +guard's own **≈ 505 ms is ≈ 25% of the ≤ 2 s worst-case ceiling as an upper +bound on its contribution**, and less than that in practice because it is +dispatched in parallel rather than added. Per the convention's rule 2 the budget +does not relax to absorb the overage: remediation is guardrails' own +spawn-reduction work (#1403), and this change pays part of its way — it removes +the `printf | tr` fork-and-exec pair from the shared normalizer and stops +resolving the telemetry subject in a subshell when no sink is wired, both costs +the pre-existing per-Bash-call lane was paying on every call. Operators who +cannot afford the addition have the per-hook kill switch below. + ## Per-hook kill switches Each guard is toggled by its own `userConfig` boolean, default **on**, except the two @@ -383,7 +511,7 @@ reads it from. | `block_no_verify_enabled` | boolean | `true` | `CLAUDE_PLUGIN_OPTION_BLOCK_NO_VERIFY_ENABLED` | Block git hook-bypass attempts (--no-verify, core.hooksPath=, hook-manager env-var disables for a configurable set — lefthook/husky/pre-commit/simple-git-hooks by default) | | `block_dangerous_git_enabled` | boolean | `true` | `CLAUDE_PLUGIN_OPTION_BLOCK_DANGEROUS_GIT_ENABLED` | Block irreversible git operations (push --force, push --force-with-lease leasing against a value git resolves at push time — either no expected value, or an expectation that is not an object id of the repository's own hash width — reset --hard, clean -f, worktree-wide checkout/restore discards) | | `block_hook_bypass_enabled` | boolean | `true` | `CLAUDE_PLUGIN_OPTION_BLOCK_HOOK_BYPASS_ENABLED` | Block Bash file-write workarounds that circumvent Write/Edit hook gates | -| `block_windows_drive_tmp_enabled` | boolean | `true` | `CLAUDE_PLUGIN_OPTION_BLOCK_WINDOWS_DRIVE_TMP_ENABLED` | Block Bash/PowerShell writes whose target is a Windows drive-root temp path (/tmp, C:\tmp, \tmp, /c/tmp) that resolves to :\tmp instead of %TEMP% | +| `block_windows_drive_tmp_enabled` | boolean | `true` | `CLAUDE_PLUGIN_OPTION_BLOCK_WINDOWS_DRIVE_TMP_ENABLED` | Block writes whose target is a Windows drive-root temp path (/tmp, C:\tmp, \tmp, /c/tmp) that resolves to :\tmp instead of %TEMP% — both Bash/PowerShell commands and Write/Edit/MultiEdit/NotebookEdit file paths. One switch covers both lanes | | `block_exported_msys_pathconv_enabled` | boolean | `true` | `CLAUDE_PLUGIN_OPTION_BLOCK_EXPORTED_MSYS_PATHCONV_ENABLED` | Block a leaking MSYS path-conversion suppressor on Windows: an EXPORTED MSYS_NO_PATHCONV / MSYS2_ARG_CONV_EXCL, or a prefix on a child shell (MSYS_NO_PATHCONV=1 bash -c ...). Either switches off conversion for later commands, letting an unconverted /d/... reach git as :\d\...; a prefix on a non-shell command word and a bare assignment are not matched | | `block_noncanonical_commit_enabled` | boolean | `true` | `CLAUDE_PLUGIN_OPTION_BLOCK_NONCANONICAL_COMMIT_ENABLED` | Block `git commit -m` when the message actually contains a newline (multi-line `-m` mangles across shells — pipe it via `-F -` instead; single-line `-m` passes); --amend, -C/-c, --fixup/--squash, -F , and an in-progress merge/rebase are exempt | | `block_convention_gate_enabled` | boolean | `true` | `CLAUDE_PLUGIN_OPTION_BLOCK_CONVENTION_GATE_ENABLED` | Block a commit subject or `gh pr create --title` that violates the team-tracked convention pattern in .claude/source-control.md (no tracked pattern = no enforcement; same exemptions as block-noncanonical-commit) | diff --git a/plugins/guardrails/hooks/block-windows-drive-tmp.sh b/plugins/guardrails/hooks/block-windows-drive-tmp.sh index c3b84ab71..fc1ad68ef 100755 --- a/plugins/guardrails/hooks/block-windows-drive-tmp.sh +++ b/plugins/guardrails/hooks/block-windows-drive-tmp.sh @@ -1,6 +1,16 @@ #!/usr/bin/env bash # PreToolUse hook: block writes whose target is a Windows drive-root temp path. -# Triggered on Bash and PowerShell tool calls. +# Triggered on Bash and PowerShell tool calls (a command string), and on +# Write / Edit / MultiEdit / NotebookEdit tool calls (a file path). +# +# TWO DOORS, ONE MATCHER. A write reaches the drive root through either shape, +# and until 0.30.0 only the command shape was inspected: the hook read +# `.tool_input.command`, a `Write` payload carries `file_path` instead, so the +# empty-COMMAND early exit returned before any matcher ran and an empty +# `C:\tmp\tmp.rSFIkHm5DO` was created with no guard noticing. Both doors now feed +# the SAME has_drive_root_tmp() matcher. The file-path lane needs none of the +# command lane's inference — no redirect parsing, no write-utility whitelist, +# no segment splitting — because on Write/Edit the path IS the write target. # # On Windows (Git Bash / MSYS / Cygwin), a hardcoded POSIX `/tmp` path resolves to # `:\tmp` (e.g. `C:\tmp`) rather than the platform temp directory @@ -37,9 +47,47 @@ source "$(dirname "${BASH_SOURCE[0]}")/hook-utils.sh" hook::check_enabled "BLOCK_WINDOWS_DRIVE_TMP" +# Non-Windows hosts: /tmp is the real POSIX temp, so this guard can never find a +# violation here. Skip entirely. Tests force OSTYPE=msys to exercise the Windows +# lane on Linux CI. +# +# THIS GATE RUNS FIRST, ahead of hook::buffer_stdin and hook::require_jq_blocking, +# and that ordering is load-bearing. This hook matches Write / Edit / MultiEdit / +# NotebookEdit as of 0.30.0. Evaluated any later, a Linux or macOS host without +# jq on PATH would take require_jq_blocking's fail-closed exit 2 on EVERY file +# edit — denying writes on a platform where the guard has no opinion at all. +# Reading OSTYPE needs nothing from the payload, so the gate is free to precede +# the read; hook::check_enabled above already exits without draining stdin, so +# that is an established shape in this hook, not a new one. +# +# Known and accepted: the shape is established, the EXPOSURE is not. check_enabled +# is a kill switch that fires only when an operator disables the guard, whereas +# this gate fires on every non-Windows tool call. A payload past roughly 64KB is +# therefore left undrained on Linux and macOS where it previously was not, so a +# writer that does not handle EPIPE would take SIGPIPE. Accepted rather than +# fixed, because the alternatives are worse: a naive builtin drain blocks until +# EOF and can hang a tool call (which is the whole reason hook::buffer_stdin is +# a bounded idle-timeout read), and draining via buffer_stdin first would put +# its rc-2 fail-closed exit back in front of the host gate — the very bug above. +# Tracked separately rather than widened into this change. +# +# The Windows path is UNCHANGED: the case falls through and every fail-closed +# posture below runs in exactly the same order as before — buffer_stdin rc 2, +# jq absence, unparsable payload, NUL bytes, MAX_COMMAND_LEN. +# +# block-exported-msys-pathconv.sh deliberately keeps the opposite ordering. It +# matches only Bash|PowerShell, where blocking on missing jq is the accepted +# #2146 posture; the blast radius that forces the hoist here does not exist +# there. Do not "fix the inconsistency" by aligning them. +case "${OSTYPE:-}" in +msys* | cygwin* | win32) ;; +*) exit 0 ;; +esac + # High-res start stamp for the telemetry envelope. EPOCHREALTIME is Bash 5.0+; # on older bash it is unset, so default to empty and skip telemetry (the block # still fires). Referencing it bare under `set -u` would abort before exit. +# Below the host gate so it stays adjacent to the work it actually times. start=${EPOCHREALTIME:-} # hook::buffer_stdin encapsulates the Win32-pipe-safe bounded fd0 read. rc 1 @@ -59,8 +107,16 @@ INPUT=$(hook::buffer_stdin) || { # absence — same posture as the other Bash/PowerShell blocking guards (#2146). hook::require_jq_blocking "guardrails-block-windows-drive-tmp" "block_windows_drive_tmp_enabled" +# Path fields only — never `.tool_input.content` / `.new_string` / `.new_source`. +# HOOK_JQ_FIELDS_NUL is computed across every REQUESTED field, so pulling the +# written CONTENT in here would make this guard block on a NUL anywhere in a +# file body: a false-positive class that is hardcoded-path-check's concern, not +# this guard's. `notebook_path` rides along in the same jq process (one spawn, +# not two) because NotebookEdit spells its target differently from Write/Edit. jq_rc=0 -hook::jq_fields "$INPUT" '.tool_input.command' '.tool_name' || jq_rc=$? +hook::jq_fields "$INPUT" \ + '.tool_input.command' '.tool_name' \ + '.tool_input.file_path' '.tool_input.notebook_path' || jq_rc=$? if ((jq_rc == 2)); then echo "BLOCKED: the hook payload could not be parsed." >&2 exit 2 @@ -69,30 +125,37 @@ fi # A NUL byte in EITHER field is fail-CLOSED (#2136 / #2122). if ((HOOK_JQ_FIELDS_NUL)); then - echo "BLOCKED: the payload carries a NUL byte, which a command cannot reliably carry." >&2 + echo "BLOCKED: the payload carries a NUL byte, which neither a command nor a file path can reliably carry." >&2 echo "What a guard can read is not dependably what would run, so this is refused rather than matched." >&2 echo "Fix: reissue the tool call without the embedded NUL." >&2 exit 2 fi COMMAND="${HOOK_JQ_FIELDS[0]}" -[[ -n "$COMMAND" ]] || exit 0 TOOL_NAME="${HOOK_JQ_FIELDS[1]:-Bash}" +# Write / Edit / MultiEdit spell the target `file_path`; NotebookEdit spells it +# `notebook_path`. Reading both and taking whichever is populated keeps the lane +# correct without depending on which spelling a given tool version emits. +FILE_PATH="${HOOK_JQ_FIELDS[2]:-}" +[[ -n "$FILE_PATH" ]] || FILE_PATH="${HOOK_JQ_FIELDS[3]:-}" -# Non-Windows hosts: /tmp is the real POSIX temp. Skip entirely. Tests force -# OSTYPE=msys to exercise the Windows lane on Linux CI. -case "${OSTYPE:-}" in -msys* | cygwin* | win32) ;; -*) exit 0 ;; -esac +# Neither door carried anything to inspect. Before 0.30.0 this exit tested +# COMMAND alone, which is exactly how a `Write` payload passed unexamined. +[[ -n "$COMMAND" || -n "$FILE_PATH" ]] || exit 0 MAX_COMMAND_LEN=16384 -SUBJECT=$(hook::extract_bash_subject "$TOOL_NAME" "$COMMAND") emit_tel() { [[ -n "$start" ]] || return 0 hook::telemetry_enabled || return 0 - local data + # Resolved HERE, not at top level: hook::extract_bash_subject runs in a + # command substitution, and that fork was being paid on every tool call even + # when no telemetry sink is wired — which is the default, and now on the + # per-Write surface too, where the helper returns the bare tool name and the + # fork buys a constant. Same shape as the plugin's other lazily-resolved + # telemetry fields. + local SUBJECT data + SUBJECT=$(hook::extract_bash_subject "$TOOL_NAME" "$COMMAND") data=$(jq -n --arg tool "$TOOL_NAME" --arg subject "$SUBJECT" --arg form "$2" \ '{tool:$tool,subject:$subject,form:$form}' 2>/dev/null) || data='{"tool":"Bash","subject":"","form":""}' hook::emit_telemetry "block-windows-drive-tmp" "PreToolUse" "$1" "$start" "$data" "${CLAUDE_PROJECT_DIR:-}" @@ -112,14 +175,35 @@ block() { } # Above this length the command is not parsed — fail closed (same ceiling as the -# other argv-faithful Bash guards). +# other argv-faithful Bash guards). The ceiling exists because the COMMAND lane +# below walks the string character by character twice (mask_quoted_redirect_ops, +# split_shell_segments) before it matches anything. NO EQUIVALENT CEILING GUARDS +# THE FILE-PATH LANE, and that is a decision rather than an omission: that lane +# runs three EREs against one string with no tokenization, so length buys no +# parse ambiguity there, and detection does not degrade with length — a +# drive-root prefix matches at any total length. A blocking ceiling would only +# add a false-positive class (a legitimate long path refused for its size). The +# payload as a whole is still bounded upstream by hook::buffer_stdin's idle +# timeout, which fails CLOSED on a truncated read. if ((${#COMMAND} > MAX_COMMAND_LEN)); then block "too-long" fi # Slash-normalize so C:\tmp, C:/tmp, and \tmp share one matcher. Lowercase for # case-insensitive Windows path compare without relying on bash [[ =~ ]] flags. -NORM=$(printf '%s' "${COMMAND//\\//}" | tr '[:upper:]' '[:lower:]') +# Pure shell, no `printf | tr`: that pipeline is a fork AND an exec (~280 ms +# together on Windows Git Bash) to fold one character class, and this guard now +# fires on the per-Write surface as well, where that pair would be pure added +# budget. Same bytes for ASCII paths and commands, which is all a drive-root +# matcher reads. Result lands in NORM_OUT rather than on stdout because a +# command substitution would fork the shell right back. +norm_lower() { + local s="${1//\\//}" + NORM_OUT="${s,,}" +} + +norm_lower "$COMMAND" +NORM="$NORM_OUT" # True when carries a drive-root tmp path reference: # /tmp[/...] — POSIX form (Git Bash maps this to :\tmp) @@ -133,8 +217,42 @@ has_drive_root_tmp() { if [[ "$s" =~ (^|[^[:alnum:]._/])\/tmp(\/|[^[:alnum:]_./-]|$) ]]; then return 0 fi - # MSYS //tmp - if [[ "$s" =~ (^|[^[:alnum:]._/])\/[a-z]\/tmp(\/|[^[:alnum:]_./-]|$) ]]; then + # MSYS //tmp, in two arms because a `:` on the left is ambiguous and + # the two readings decide oppositely. + # + # A DRIVE COLON must NOT satisfy the boundary: after slash-normalization + # `D:\a\tmp\x` reads as `d:` + `/a/tmp`, and that is an ordinary `tmp` + # directory two levels down, not a drive root. Blocking it was a FALSE + # POSITIVE that predates the file-path lane (the command lane blocked + # `mkdir -p D:\a\tmp\x` too), and it made the guard contradict its own + # premise, since the identical MSYS spelling `/d/a/tmp/x` was allowed — one + # sink deciding two ways. The lane makes it reachable from every Write/Edit, + # so it is fixed here rather than inherited. + # + # A PARAMETER COLON must still satisfy it. `-Path:` / `-FilePath:` / + # `-Destination:` is valid PowerShell binding, so `Set-Content -Path:/c/tmp/x` + # is a real drive-root write and one of its space-bound twins is a pinned + # MUST-fire case. Excluding `:` outright would have dropped that whole class. + # + # The discriminator is what sits before the colon. A DRIVE SPEC is exactly one + # alphanumeric at a word boundary — `D:`, ` D:`, `"D:`, `(D:` — so arm 2 + # excludes only that shape and takes every other colon, which is the narrowest + # change that fixes the false positive. Its three alternatives are: a + # non-alphanumeric immediately before the colon (`;:`, `":`, `):` — never a + # drive spec); two alphanumerics (a multi-character token such as `-Path:` or + # `host:`); and a single alphanumeric behind a flag dash (`-t:`). + # + # Two accepted residuals, both unchanged from before the fix rather than + # introduced by it. A PATH-style list (`PATH=/usr/bin:/c/tmp cmd`) presents + # the multi-character-token shape and still matches, so a command whose write + # target is elsewhere can be matched over a search-path entry — lexically + # indistinguishable from a bound parameter. And a remote spec with a + # single-letter host (`ssh u@h:/c/tmp/x`) now reads as a drive spec and is not + # matched; it names a path on another machine, which this guard never governed. + if [[ "$s" =~ (^|[^[:alnum:]._/:])\/[a-z]\/tmp(\/|[^[:alnum:]_./-]|$) ]]; then + return 0 + fi + if [[ "$s" =~ ([^[:alnum:]]|[[:alnum:]][[:alnum:]]|-[[:alnum:]]):\/[a-z]\/tmp(\/|[^[:alnum:]_./-]|$) ]]; then return 0 fi # Drive-letter X:/tmp @@ -317,12 +435,33 @@ has_write_utility_with_drive_root_tmp() { return 1 } -if has_redirect_to_drive_root_tmp "$NORM"; then - block "redirect" +# --- File-path lane: Write / Edit / MultiEdit / NotebookEdit ----------------- +# On these tools the payload's path IS the write target, so the whole +# write-shape inference the command lane needs — redirect parsing, the producer +# utility whitelist, per-segment splitting — is structurally absent here. The +# matcher is the shipped has_drive_root_tmp(), unchanged and unduplicated, so +# every spelling the command lane blocks (POSIX /tmp, MSYS /c/tmp, C:\tmp, +# drive-root \tmp) and every one it permits (%TEMP% expansions, /var/tmp, +# ./tmp, foo/tmp) decide identically on this lane. +if [[ -n "$FILE_PATH" ]]; then + norm_lower "$FILE_PATH" + if has_drive_root_tmp "$NORM_OUT"; then + block "file-path" + fi fi -if has_write_utility_with_drive_root_tmp "$NORM"; then - block "write-utility" +# --- Command lane: Bash / PowerShell ----------------------------------------- +# Skipped outright on a file-path payload: has_redirect_to_drive_root_tmp runs +# mask_quoted_redirect_ops in a command substitution, and forking the shell to +# scan an empty string would be per-Write budget spent to reach a foregone `no`. +if [[ -n "$COMMAND" ]]; then + if has_redirect_to_drive_root_tmp "$NORM"; then + block "redirect" + fi + + if has_write_utility_with_drive_root_tmp "$NORM"; then + block "write-utility" + fi fi emit_tel "ok" "" diff --git a/plugins/guardrails/hooks/block-windows-drive-tmp.test.sh b/plugins/guardrails/hooks/block-windows-drive-tmp.test.sh index 538b2efec..b9a40410c 100755 --- a/plugins/guardrails/hooks/block-windows-drive-tmp.test.sh +++ b/plugins/guardrails/hooks/block-windows-drive-tmp.test.sh @@ -51,9 +51,205 @@ run_posix_host() { assert_exit "$label" 0 "$rc" } +# File-path lane (0.30.0): Write / Edit / MultiEdit / NotebookEdit carry +# `file_path` (NotebookEdit `notebook_path`) instead of `command`, and before +# 0.30.0 the empty-COMMAND early exit returned before any matcher ran. +# is produced by the caller so one runner covers every tool. +run_win_payload() { + local label="$1" payload="$2" expected="$3" + shift 3 + local rc out + out=$(env OSTYPE=msys "$@" bash "$HOOK" <<<"$payload" 2>&1) + rc=$? + assert_exit "$label" "$expected" "$rc" + if ((expected == 2)); then + assert_contains "$label → message" "$out" "drive-root temp" + assert_contains "$label → fix" "$out" "%TEMP%" + fi +} + +run_posix_host_payload() { + local label="$1" payload="$2" + local rc + env OSTYPE=linux-gnu bash "$HOOK" <<<"$payload" >/dev/null 2>&1 + rc=$? + assert_exit "$label" 0 "$rc" +} + +notebook_path_json() { + MSYS_NO_PATHCONV=1 jq -n --arg fp "$1" \ + '{tool_name:"NotebookEdit",tool_input:{notebook_path:$fp,new_source:"x"}}' +} + +# Command payload builders that PRESERVE an MSYS `//tmp` spelling. +# MSYS argv rewriting converts an argument only when the argument is ENTIRELY a +# POSIX-absolute path: `/c/tmp/x` becomes `C:/tmp/x`, while `mkdir -p /c/tmp/x` +# passes through untouched. Every command fixture below is multi-token, so the +# shared command_json / pwsh_command_json are already safe for them and omit +# MSYS_NO_PATHCONV correctly (the shared PATH-payload builders — write_json and +# siblings — do set it, because a file_path IS a lone path). These local +# builders set it explicitly so a future lone-path command fixture cannot +# silently become a drive-letter payload and stop exercising the MSYS arm. +msys_command_json() { + MSYS_NO_PATHCONV=1 jq -n --arg cmd "$1" '{tool_name:"Bash",tool_input:{command:$cmd}}' +} +msys_pwsh_command_json() { + MSYS_NO_PATHCONV=1 jq -n --arg cmd "$1" '{tool_name:"PowerShell",tool_input:{command:$cmd}}' +} + # --- Host gate --------------------------------------------------------------- run_posix_host "Linux host: >/tmp/x allowed" 'echo x > /tmp/x' run_posix_host "Linux host: mkdir /tmp/x allowed" 'mkdir -p /tmp/x' +run_posix_host_payload "Linux host: Write /tmp/x allowed" "$(write_json '/tmp/x' 'body')" +run_posix_host_payload "Linux host: Edit /tmp/x allowed" "$(edit_json '/tmp/x' 'body')" + +# The host gate must be reached BEFORE hook::buffer_stdin and +# hook::require_jq_blocking. Widening the matcher to Write/Edit/MultiEdit/ +# NotebookEdit made the old ordering a hard break: on a Linux or macOS host with +# no jq on PATH, require_jq_blocking's fail-closed exit 2 fired on EVERY file +# edit, on a platform where this guard can never find a violation. +# +# Removing jq from PATH is simulable only where jq lives in a directory that +# does not ALSO host bash and coreutils — prune that one entry and the shell +# survives without jq. That holds on some hosts (it was demonstrated on a Cygwin +# host with a single jq location, where the parent commit exits 2 and this one +# exits 0 on a real jq-less PATH) and not on others: where jq sits in /usr/bin +# beside bash, pruning it takes the shell with it. That is the portability +# constraint require-jq-notice-isolation.test.sh and +# secret-pattern-detection.test.sh both record, so the assertion below does not +# depend on it. What IS portable, and what decides the bug either way, is +# whether the call is REACHED: `command -v jq` can only deny a write on a +# jq-less host if control flow gets to it. So assert the ordering from an +# xtrace of the real run. Both names appear in this trace before the fix and +# neither appears after it. +trace=$(env OSTYPE=linux-gnu bash -x "$HOOK" <<<"$(write_json '/srv/app/notes.txt' 'body')" 2>&1 >/dev/null) +assert_absent "Linux host: stdin is never buffered" "$trace" "buffer_stdin" +assert_absent "Linux host: the blocking jq requirement is never reached" "$trace" "require_jq_blocking" + +# --- File-path lane: drive-root targets (blocked) ---------------------------- +# The 2026-08-30 incident payload: an empty C:\tmp\tmp.rSFIkHm5DO that no guard saw. +run_win_payload "Write C:\\tmp\\tmp.rSFIkHm5DO (blocked)" \ + "$(write_json 'C:\tmp\tmp.rSFIkHm5DO' 'x')" 2 +run_win_payload "Write /tmp/x (blocked)" "$(write_json '/tmp/x' 'x')" 2 +run_win_payload "Write /c/tmp/x MSYS (blocked)" "$(write_json '/c/tmp/x' 'x')" 2 +run_win_payload "Write C:/tmp/x (blocked)" "$(write_json 'C:/tmp/x' 'x')" 2 +run_win_payload "Write \\tmp\\x drive-root (blocked)" "$(write_json '\tmp\x' 'x')" 2 +run_win_payload "Write D:\\tmp\\x other drive (blocked)" "$(write_json 'D:\tmp\x' 'x')" 2 +run_win_payload "Edit /tmp/x (blocked)" "$(edit_json '/tmp/x' 'x')" 2 +run_win_payload "Edit C:\\tmp\\x (blocked)" "$(edit_json 'C:\tmp\x' 'x')" 2 +run_win_payload "MultiEdit /tmp/x (blocked)" "$(other_tool_json 'MultiEdit' '/tmp/x')" 2 +run_win_payload "NotebookEdit file_path /tmp/n.ipynb (blocked)" \ + "$(notebook_json '/tmp/n.ipynb' 'x')" 2 +run_win_payload "NotebookEdit notebook_path C:\\tmp\\n.ipynb (blocked)" \ + "$(notebook_path_json 'C:\tmp\n.ipynb')" 2 + +# --- File-path lane: legitimate targets (allowed) ---------------------------- +# Three repo-wide CI gates constrain the literals below, and all three are +# satisfiable without weakening any fixture. The shell-portability scanner reads +# `\s`, `\b` and `\<` as GNU-only regex constructs wherever they appear, so no +# segment starts with those letters and no backslash precedes the placeholder; +# the machine-specific-paths gate rejects a concrete Windows user directory, so +# the user segment is the `` placeholder it prescribes. Hence the forward +# slashes here — a valid Windows spelling that the matcher slash-normalizes +# anyway, and the backslash form stays covered by the `D:\repo\docs\tmp` and +# `D:\a\tmp` cases below. None of it changes what is under test: the matcher +# decides on the presence of a drive-root `tmp` component and this path has none. +run_win_payload "Write under %TEMP% (allowed)" \ + "$(write_json 'C:/Users//AppData/Local/Temp/note.txt' 'x')" 0 +run_win_payload "Write under /var/tmp (allowed)" "$(write_json '/var/tmp/x' 'x')" 0 +run_win_payload "Write repo docs/tmp (allowed)" "$(write_json 'D:\repo\docs\tmp\x.md' 'x')" 0 +run_win_payload "Write relative ./tmp (allowed)" "$(write_json './tmp/x' 'x')" 0 +run_win_payload "Write path component foo/tmp (allowed)" "$(write_json 'foo/tmp/x' 'x')" 0 +run_win_payload "Write /tmpdir sibling (allowed)" "$(write_json '/tmpdir/x' 'x')" 0 +run_win_payload "Write C:/tmp2 sibling (allowed)" "$(write_json 'C:/tmp2/x' 'x')" 0 +run_win_payload "Write UNC //host/tmp (allowed)" "$(write_json '\\host\tmp\x' 'x')" 0 +run_win_payload "Edit ordinary repo file (allowed)" \ + "$(edit_json 'D:\repo\plugins\guardrails\README.md' 'x')" 0 +# Body content is never scanned — only the target path decides. +run_win_payload "Write body mentioning /tmp (allowed)" \ + "$(write_json 'D:\repo\notes.md' 'do not write to /tmp/x')" 0 +# A tool with no path field at all must stay a no-op. +run_win_payload "Read tool, no path (allowed)" \ + "$(jq -n '{tool_name:"Read",tool_input:{}}')" 0 + +# --- A `tmp` directory under a single-letter parent (allowed) ---------------- +# MUST-STAY-QUIET, added repro-first: before the left-boundary fix these all +# blocked, because after slash-normalization the drive colon satisfied the MSYS +# alternative's left boundary and `D:\a\tmp\x` read as `d:` + `/a/tmp`. The +# identical MSYS spelling `/d/a/tmp/x` was allowed the whole time, so one sink +# decided two ways. Both lanes are pinned: the file-path lane is where the +# defect became reachable on every write, the Bash lane is where it already was. +run_win_payload "Write D:\\a\\tmp\\x subdir tmp (allowed)" "$(write_json 'D:\a\tmp\x' 'x')" 0 +run_win_payload "Write C:\\q\\tmp\\out.log subdir tmp (allowed)" \ + "$(write_json 'C:\q\tmp\out.log' 'x')" 0 +run_win_payload "Write /d/a/tmp/x MSYS spelling (allowed)" "$(write_json '/d/a/tmp/x' 'x')" 0 +run_win "mkdir D:\\a\\tmp\\x subdir tmp (allowed)" 'mkdir -p D:\a\tmp\x' 0 +# MSYS spellings go through the local no-pathconv builders, or Git Bash rewrites +# them to the drive-letter form and the assertion stops testing this matcher. +run_win_payload "mkdir /d/a/tmp/x MSYS spelling (allowed)" \ + "$(msys_command_json 'mkdir -p /d/a/tmp/x')" 0 +# The genuine MSYS drive root must still block. +run_win_payload "mkdir /c/tmp/x drive root (still blocked)" \ + "$(msys_command_json 'mkdir -p /c/tmp/x')" 2 +run_win_payload "Write /c/tmp/x drive root (still blocked)" "$(write_json '/c/tmp/x' 'x')" 2 + +# A PowerShell parameter colon is NOT a drive colon. `-Path:/c/tmp/x` binds the +# same value as `-Path /c/tmp/x`, so both must block; the boundary fix above +# must not take this class out with the drive-colon false positive. +run_win_payload "PS: Set-Content -Path:/c/tmp/x colon-bound (blocked)" \ + "$(msys_pwsh_command_json 'Set-Content -Path:/c/tmp/x -Value hi')" 2 +run_win_payload "PS: New-Item -Path:/c/tmp/x colon-bound (blocked)" \ + "$(msys_pwsh_command_json 'New-Item -Path:/c/tmp/x -ItemType File')" 2 +run_win_payload "PS: Out-File -FilePath:/c/tmp/x colon-bound (blocked)" \ + "$(msys_pwsh_command_json "'hi' | Out-File -FilePath:/c/tmp/x")" 2 +run_win_payload "PS: Add-Content -Path:/c/tmp/x colon-bound (blocked)" \ + "$(msys_pwsh_command_json 'Add-Content -Path:/c/tmp/x -Value hi')" 2 +run_win_payload "PS: Copy-Item -Destination:/c/tmp/a colon-bound (blocked)" \ + "$(msys_pwsh_command_json 'Copy-Item .\a -Destination:/c/tmp/a')" 2 +run_win_payload "PS: Move-Item -Destination:/c/tmp/a colon-bound (blocked)" \ + "$(msys_pwsh_command_json 'Move-Item .\a -Destination:/c/tmp/a')" 2 +# ... and the drive-colon reading of the same character still must not fire. +run_win_payload "PS: Set-Content -Path:D:/a/tmp/x subdir tmp (allowed)" \ + "$(msys_pwsh_command_json 'Set-Content -Path:D:/a/tmp/x -Value hi')" 0 + +# --- Registration liveness --------------------------------------------------- +# The script half of this guard is inert without the matcher registration: a +# Write payload only reaches the hook because hooks.json routes it here. Reverting +# that registration alone would leave every assertion above green, so assert it. +# Matchers are split on `|` into EXACT alternatives, not substring-searched: a +# containment test for "Edit" is satisfied by "MultiEdit" and so can never fail +# on its own, and a matcher with the pipes removed ("WriteEditNotebookEdit") +# routes nothing while passing every containment check. Sorting also makes the +# assertions immune to a harmless reordering of the alternatives. +HOOKS_JSON="$HOOK_DIR/hooks.json" +reg=$(jq -r --arg h "block-windows-drive-tmp.sh" ' + [ .hooks.PreToolUse[] + | select([.hooks[].command] | any(contains($h))) + | .matcher | split("|")[] ] + | sort | join(" ")' "$HOOKS_JSON" 2>/dev/null) +assert_eq "hooks.json routes the guard to exactly the intended tools" \ + "Bash Edit MultiEdit NotebookEdit PowerShell Write" "$reg" +# The registration must also NAME A FILE THAT EXISTS — a command path typo +# registers cleanly and then fails to run on every tool call. +reg_cmd=$(jq -r --arg h "block-windows-drive-tmp.sh" ' + [ .hooks.PreToolUse[].hooks[].command | select(contains($h)) ] | first // ""' \ + "$HOOKS_JSON" 2>/dev/null) +reg_rel="${reg_cmd##*\"/}" +assert_eq "the registered command resolves to a file on disk" "yes" \ + "$([[ -n "$reg_rel" && -f "$HOOK_DIR/../$reg_rel" ]] && echo yes || echo no)" + +# --- File-path lane: fail-closed on a NUL-bearing path ----------------------- +# jq emits the escape textually, so the payload survives command substitution +# and the hook's own jq turns it back into a real NUL byte. +nul_out=$(env OSTYPE=msys bash "$HOOK" \ + <<<"$(jq -n '{tool_name:"Write",tool_input:{file_path:"C:/safe/\u0000/x"}}')" 2>&1) +assert_exit "Write with NUL in file_path fails closed" 2 "$?" +assert_contains "NUL block message" "$nul_out" "NUL byte" + +# --- File-path lane: kill switch --------------------------------------------- +run_win_payload "kill switch disables the file-path lane" \ + "$(write_json '/tmp/x' 'x')" 0 \ + CLAUDE_PLUGIN_OPTION_BLOCK_WINDOWS_DRIVE_TMP_ENABLED=false # --- Redirects to drive-root tmp (blocked) ----------------------------------- run_win "redirect >/tmp/x (blocked)" 'echo x > /tmp/x' 2 @@ -142,4 +338,30 @@ else fi assert_contains "blocked stderr still present with sink" "$out" "drive-root temp" +# --- Telemetry on the file-path lane ----------------------------------------- +# The `file-path` form and the Write-shaped `tool` / `subject` are documented in +# docs/conventions/hook-telemetry/data/block-windows-drive-tmp.schema.json, and +# the case above exercises only the Bash lane — so without this the new envelope +# is documented and never executed, and a fault in emit_tel's now-locally-scoped +# SUBJECT would leave every assertion green. `subject` must be the bare tool +# name: hook::extract_bash_subject does not tokenize a non-Bash tool, and the +# target path must never reach the envelope. +TEL_FP="$(mktemp "$TEST_TMPDIR/tmp.XXXXXXXXXX")" +SINK_FP=$(make_sink "cat > \"$TEL_FP\"") +out=$(env OSTYPE=msys HOOK_TELEMETRY_SINK="$SINK_FP" bash "$HOOK" \ + <<<"$(write_json 'C:\tmp\tmp.rSFIkHm5DO' 'x')" 2>&1) || true +wait_for_sink "$TEL_FP" || true +if [[ -s "$TEL_FP" ]]; then + tel_fp_body=$(cat "$TEL_FP") + assert_contains "file-path telemetry hook id" "$tel_fp_body" '"hook": "block-windows-drive-tmp"' + assert_contains "file-path telemetry blocked" "$tel_fp_body" '"status": "blocked"' + assert_contains "file-path telemetry form" "$tel_fp_body" '"form": "file-path"' + assert_contains "file-path telemetry tool" "$tel_fp_body" '"tool": "Write"' + assert_contains "file-path telemetry subject" "$tel_fp_body" '"subject": "Write"' + assert_absent "file-path telemetry carries no path" "$tel_fp_body" "rSFIkHm5DO" +else + ok "file-path telemetry sink empty (best-effort; block path already covered)" +fi +assert_contains "file-path blocked stderr present with sink" "$out" "drive-root temp" + report diff --git a/plugins/guardrails/hooks/hooks.json b/plugins/guardrails/hooks/hooks.json index 354cdd2b1..e16072f17 100644 --- a/plugins/guardrails/hooks/hooks.json +++ b/plugins/guardrails/hooks/hooks.json @@ -18,6 +18,17 @@ } ] }, + { + "matcher": "Write|Edit|MultiEdit|NotebookEdit", + "hooks": [ + { + "type": "command", + "command": "\"${CLAUDE_PLUGIN_ROOT}\"/hooks/block-windows-drive-tmp.sh", + "timeout": 60, + "statusMessage": "Checking for Windows drive-root /tmp writes..." + } + ] + }, { "matcher": "Bash|PowerShell", "hooks": [