diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index 74ad42c733..91fc5e824d 100644 --- a/plugins/guardrails/.claude-plugin/plugin.json +++ b/plugins/guardrails/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "guardrails", - "version": "0.25.0", + "version": "0.25.1", "description": "Twelve safety guards that block secret/credential writes, hardcoded machine-specific paths, git hook-bypass attempts, irreversible git operations (force-push, reset --hard, worktree-wide checkout/restore discards), Bash file-write workarounds that circumvent Write/Edit hooks, multi-line `git commit -m` messages (an actual-newline `-m` mangles across shells; single-line `-m` passes), commit subjects and gh pr create titles that violate the repo's tracked team convention (when one is declared in .claude/source-control.md), (advisory) hallucinated CLI flags, (advisory) /plugin:skill references that do not resolve, (advisory) markdown citing a repo path the repo's own history shows was removed, (advisory, opt-in) un-throttled Workflow fan-out that risks burst 529s, and (advisory, opt-in) direct gh pr create calls bypassing this marketplace's own pull-request skill — each independently toggleable.", "author": { "name": "Melodic Software", @@ -125,7 +125,7 @@ "block_hook_bypass_scratch_roots": { "type": "string", "title": "block-hook-bypass scratch roots", - "description": "Comma-separated ABSOLUTE directories block-hook-bypass exempts as scratch/temp write targets (e.g. /tmp/scratch,/d/jobtmp/session); empty (the default) exempts nothing and leaves the guard's shipped behaviour unchanged. Matching is on the effective stdout target after lexical normalization, at a path-component boundary — a sibling merely sharing the name prefix, a `..` escape out of a root, a quoted or escaped operand, and a discard-then-real-file redirect all still block. Symlinks are not followed", + "description": "Comma-separated ABSOLUTE directories block-hook-bypass exempts as scratch/temp write targets (e.g. /tmp/scratch,/d/jobtmp/session); empty (the default) exempts nothing and leaves the guard's shipped behaviour unchanged. Matching is on the effective stdout target after lexical normalization, at a path-component boundary — a sibling merely sharing the name prefix, a `..` escape out of a root, and a discard-then-real-file redirect all still block. Any quote or backslash after the first `>` CHARACTER in the command (operator or not) cancels the exemption for the whole command — so a quote in a later segment, or a `>` inside quoted content, also cancels it. Symlinks are not followed", "default": "" }, "stdin_read_timeout": { diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index 4e5253ae8c..1a18f38e00 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,10 +3,59 @@ 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.25.1] + +### Changed + +- **0.25.0 described the scratch-root exemption's fail-close inaccurately on every surface, twice + over. Corrected, and pinned (#2236; root cause #2226).** No behaviour changes — four documents + become accurate and four regression tests now pin the boundaries they describe. + + 0.25.0 said the exemption fails closed on "a quoted or escaped **operand**", "after the first + redirect **operator**". Both halves are wrong, in the same direction — they imply precision the + check does not have: + + 1. **It is not operand-scoped.** It reads the whole raw command tail, not the segment being + evaluated and not the target word, so a quote in an unrelated *later* segment cancels the + exemption for an earlier, unambiguous write. + 2. **It is not keyed on the redirect operator.** `${COMMAND#*>}` splits at the first literal `>` + **character**, without deciding whether that `>` is an operator at all. A `>` inside quoted + *content* therefore starts the scanned tail early, and the closing quote of that same content + lands inside it. + + Consequence of (2), and the case 0.25.0's own text got backwards: it claimed quotes *before* the + redirect are the ordinary case and keep the exemption. That holds only while the quoted content + contains no `>`. + + | command, root `/tmp/scratch` | verdict | + | --- | --- | + | `echo x > /tmp/scratch/f` | allowed | + | `echo "hello world" > /tmp/scratch/f` | allowed | + | `echo x > /tmp/scratch/f && grep foo "notes.txt"` — quote in a later segment | **blocked** | + | `echo x > /tmp/scratch/f && grep foo notes.txt` | allowed | + | `echo "a > b" > /tmp/scratch/f` — `>` inside quoted content | **blocked** | + | `echo 'x > y' > /tmp/scratch/f` | **blocked** | + + The breadth stays. It is one-directional — the check can only ever *refuse* an exemption, never + grant one — so the failure mode is lost convenience, never a bypass. Keying it on the real redirect + operator, or narrowing it to the operand, both need the same thing: knowing which `>` and which + quotes are syntax rather than content. That is exactly the association `strip_literals` destroys + before this code runs, which is **#2226**, not a separate fix, and it is deliberately not attempted + here. Recorded on #2226 as further evidence. + + The hook comment, the README, the manifest's option description and this entry now state the + mechanism as it is: **any quote or backslash after the first `>` character, operator or not, + anywhere in the command.** + + 0.25.0's entry below is left as it shipped, per Keep a Changelog; this entry is its erratum. + ## [0.25.0] ### Added +> **Erratum:** the fail-close scope described in this entry is inaccurate in two ways. See 0.25.1 +> above for the corrected mechanism. The behaviour described elsewhere in this entry is unchanged. + - **`block-hook-bypass` gains an opt-in scratch-root exemption, and with it its first target-scoped axis (#2210).** A read-only investigation that writes a throwaway probe file under a session or job temp root was blocked exactly like a repo-file write — reproduced twice, diff --git a/plugins/guardrails/README.md b/plugins/guardrails/README.md index b8d9cf7cc7..4f93a605de 100644 --- a/plugins/guardrails/README.md +++ b/plugins/guardrails/README.md @@ -111,11 +111,21 @@ out of scope until such a signal exists. fails closed rather than being documented: the quote strip drops a kept target's quotes and the segment split then reads a `;`, `|`, `&` or space *inside* the operand as syntax, so `> "/tmp/scratch/a;/../../etc/passwd"` — - one pathname to bash — would otherwise be judged on `/tmp/scratch/a`. Any - quote or backslash after the first redirect operator therefore cancels the - exemption; quotes before it (`echo "hi there" > /tmp/scratch/f`) do not. The - same truncation reaches the `/dev/null` exemption and predates this option — - filed as #2226 and pinned by a control test. Two residuals remain, both + one pathname to bash — would otherwise be judged on `/tmp/scratch/a`. The rule + is therefore blunt, and blunt in two directions worth stating exactly: **any + quote or backslash after the first `>` character in the command — operator or + not — cancels the exemption.** It is not scoped to the segment being evaluated, + so a quote in an unrelated later segment cancels it too + (`echo x > /tmp/scratch/f && grep foo "notes.txt"` blocks; the same compound + without quotes does not). And it is not keyed on the redirect *operator*, so a + `>` inside quoted content starts the scan early and that content's own closing + quote falls inside it — `echo "hi there" > /tmp/scratch/f` is exempt, but + `echo "a > b" > /tmp/scratch/f` is **not**. Both are the safe direction: the + check can only ever refuse an exemption, never grant one. Making it precise + needs the same thing in both cases — knowing which `>` and which quotes are + syntax rather than content — which is exactly what the quote strip destroys + before this code runs. The same root cause reaches the `/dev/null` exemption and + predates this option — filed as #2226 and pinned by a control test. Two residuals remain, both deliberate and both pinned: normalization is lexical, so symlinks out of a root are not followed, and the compare is case-insensitive because the segment scan runs over the lowercased command. Naming a root is accepting that root's @@ -303,7 +313,7 @@ reads it from. | `block_dangerous_git_allow` | string | *(none)* | `CLAUDE_PLUGIN_OPTION_BLOCK_DANGEROUS_GIT_ALLOW` | Comma-separated forms block-dangerous-git permits: push-force, push-lease-unsafe, reset-hard, clean-force, checkout-dot, restore-dot, checkout-force; empty blocks all | | `block_noncanonical_commit_allow` | string | *(none)* | `CLAUDE_PLUGIN_OPTION_BLOCK_NONCANONICAL_COMMIT_ALLOW` | Comma-separated form tokens to allow (currently: message-flag, which permits `-m` even when the message contains a newline) | | `block_no_verify_hook_manager_prefixes` | string | *(none)* | `CLAUDE_PLUGIN_OPTION_BLOCK_NO_VERIFY_HOOK_MANAGER_PREFIXES` | Comma-separated hook-manager env-var name prefixes block-no-verify treats as a bypass when set to 0/false (e.g. lefthook,husky); empty uses the built-in default set (lefthook, husky, pre_commit, simple_git_hooks) | -| `block_hook_bypass_scratch_roots` | string | *(none)* | `CLAUDE_PLUGIN_OPTION_BLOCK_HOOK_BYPASS_SCRATCH_ROOTS` | Comma-separated ABSOLUTE directories block-hook-bypass exempts as scratch/temp write targets (e.g. /tmp/scratch,/d/jobtmp/session); empty (the default) exempts nothing and leaves the guard's shipped behaviour unchanged. Matching is on the effective stdout target after lexical normalization, at a path-component boundary — a sibling merely sharing the name prefix, a `..` escape out of a root, a quoted or escaped operand, and a discard-then-real-file redirect all still block. Symlinks are not followed | +| `block_hook_bypass_scratch_roots` | string | *(none)* | `CLAUDE_PLUGIN_OPTION_BLOCK_HOOK_BYPASS_SCRATCH_ROOTS` | Comma-separated ABSOLUTE directories block-hook-bypass exempts as scratch/temp write targets (e.g. /tmp/scratch,/d/jobtmp/session); empty (the default) exempts nothing and leaves the guard's shipped behaviour unchanged. Matching is on the effective stdout target after lexical normalization, at a path-component boundary — a sibling merely sharing the name prefix, a `..` escape out of a root, and a discard-then-real-file redirect all still block. Any quote or backslash after the first `>` CHARACTER in the command (operator or not) cancels the exemption for the whole command — so a quote in a later segment, or a `>` inside quoted content, also cancels it. Symlinks are not followed | | `stdin_read_timeout` | number
*min 1* | `2` | `CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT` | Idle bound on reading the hook payload from stdin — how long a silent pipe is tolerated before a blocking guard fails closed | ### How to set these diff --git a/plugins/guardrails/hooks/block-hook-bypass.sh b/plugins/guardrails/hooks/block-hook-bypass.sh index e61f519bcd..189dce1d4c 100755 --- a/plugins/guardrails/hooks/block-hook-bypass.sh +++ b/plugins/guardrails/hooks/block-hook-bypass.sh @@ -604,11 +604,34 @@ scratch_target_exempt() { # reaches the compare as the safe-looking prefix `/tmp/scratch/a`. Exempting # that is precisely the one-token bypass the `/dev/null` precedent warns about. # The only surviving evidence is the RAW command, so refuse the exemption when - # any quote or backslash appears after the first redirect operator. Deliberately - # conservative — a compound whose LATER segment quotes something also loses the - # exemption, and an operator who wants it writes the target unquoted. The same - # truncation reaches the `/dev/null` exemption and predates this axis; it is - # filed as #2226 and pinned by a control test. + # any quote or backslash appears after the first `>` in it. + # + # STATE THE TRUE SCOPE — it is broader than "the operand" in TWO ways, and both + # are easy to describe more precisely than they behave: + # + # 1. NOT segment-scoped. This reads the WHOLE raw tail, not the segment being + # evaluated and not the target word, so a quote anywhere later in a compound + # command costs an EARLIER, unambiguous write its exemption: + # `echo x > /tmp/scratch/f && grep foo "notes.txt"` blocks, even though + # segment 1's target is a plain path. + # 2. NOT keyed on the redirect OPERATOR. `${COMMAND#*>}` splits at the first + # literal `>` CHARACTER and never decides whether it is an operator, so a + # `>` inside quoted CONTENT starts the tail early and that content's own + # closing quote lands inside it: `echo "a > b" > /tmp/scratch/f` blocks. + # Quotes before the redirect are otherwise fine — `echo "hi there" > …` is + # exempt — but only while the quoted content holds no `>`. + # + # Both are deliberate and both are the safe direction: this test can only ever + # REFUSE an exemption, never grant one, so the failure mode is lost convenience, + # never a bypass. Fixing either one needs the same thing — knowing which `>` and + # which quotes are SYNTAX rather than CONTENT — and that is exactly the + # association strip_literals destroys before this code runs. Same root cause as + # #2226; not attempted here. Four regression tests pin these boundaries so the + # breadth cannot silently widen or narrow. An operator who wants the exemption + # keeps quotes, backslashes and `>` out of the command after the target. + # + # The same truncation reaches the `/dev/null` exemption and predates this axis; + # it is filed as #2226 and pinned by a control test. [[ "${COMMAND#*>}" == *[\"\'\\]* ]] && return 1 _norm_path "$target" || return 1 [[ -n "$_NORM_PATH" ]] || return 1 diff --git a/plugins/guardrails/hooks/block-hook-bypass.test.sh b/plugins/guardrails/hooks/block-hook-bypass.test.sh index 4852319c9f..6777dc91df 100755 --- a/plugins/guardrails/hooks/block-hook-bypass.test.sh +++ b/plugins/guardrails/hooks/block-hook-bypass.test.sh @@ -865,10 +865,31 @@ run "scratch: escaped operand is not exempted (blocked)" \ "echo x > /tmp/scratch/a\\ ../../etc/pw" 2 "$SCRATCH_ENV=/tmp/scratch" run "scratch: even a benign quoted target is not exempted (blocked)" \ "echo x > \"/tmp/scratch/f\"" 2 "$SCRATCH_ENV=/tmp/scratch" -# Quotes BEFORE the redirect operator are the ordinary case and must not cost -# the exemption — only the operand side is suspect. +# Quotes BEFORE the redirect are the ordinary case and must not cost the +# exemption — as long as the quoted content holds no `>`; see the pair below. run "scratch: quoted content, unquoted target (allowed)" \ "echo \"hello world\" > /tmp/scratch/f" 0 "$SCRATCH_ENV=/tmp/scratch" +# THE BREADTH, pinned rather than left to drift. It is blunt in TWO directions +# and both sides of each boundary are asserted, so a later change cannot quietly +# widen or narrow either. Both are one-directional — the check can only refuse an +# exemption, never grant one — and making either precise needs to know which `>` +# and which quotes are syntax rather than content, which is exactly what +# strip_literals destroys before this runs (#2226). +# +# (1) NOT segment-scoped: a quote in an unrelated LATER segment cancels the +# exemption for an earlier, unambiguous write. +run "scratch: quote in an unrelated later segment cancels it (blocked)" \ + "echo x > /tmp/scratch/f && grep foo \"notes.txt\"" 2 "$SCRATCH_ENV=/tmp/scratch" +run "scratch: the same compound with no quotes keeps it (allowed)" \ + "echo x > /tmp/scratch/f && grep foo notes.txt" 0 "$SCRATCH_ENV=/tmp/scratch" +# (2) NOT keyed on the redirect OPERATOR: `${COMMAND#*>}` splits at the first `>` +# CHARACTER, so a `>` inside quoted CONTENT starts the tail early and that +# content's own closing quote lands inside it. This is the case the 0.25.0 +# text got backwards by promising that quotes before the redirect are safe. +run "scratch: > inside double-quoted content cancels it (blocked)" \ + "echo \"a > b\" > /tmp/scratch/f" 2 "$SCRATCH_ENV=/tmp/scratch" +run "scratch: > inside single-quoted content cancels it (blocked)" \ + "echo 'x > y' > /tmp/scratch/f" 2 "$SCRATCH_ENV=/tmp/scratch" # Control: the SAME truncation reaches the /dev/null exemption and predates this # axis, which is why it is filed as #2226 rather than fixed here. Pinned so a # future strip_literals fix flips it visibly.