diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index 68dfe50ca..4b73d8323 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.27.2", + "version": "0.28.0", "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", diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index 9152b3123..bfb04b6d4 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,6 +3,90 @@ 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.28.0] + +### Fixed + +- **`block-hook-bypass` missed three inline-write forms that reach a real file, one of them a + residual the file itself recorded as accepted (#2217).** Measured against `4c90b454` (0.27.2), + hook invoked as a decision function on a `PreToolUse` Bash payload — `rc=2` blocked, `rc=0` + allowed: + + ``` + rc=0 :: python -c "open('f','w').write('x')" + rc=0 :: py -c "open('f','w').write('x')" + rc=0 :: python3.11 -c "open('f','w').write('x')" + rc=0 :: printf 'ab' > notes.md + rc=0 :: python3 - <<'PY' … open('f','w').write('x') … PY + rc=2 :: python3 -c "open('f','w').write('x')" # the one spelling that matched + rc=2 :: printf "a\nb\n" > notes.md # the same write, escaped newline + ``` + + Three separate causes, all in the direction of letting a write through: + + 1. **The interpreter detector was a spelling floor, not a rule.** Both lanes required the literal + `python3` — the Bash lane's `EXEC_LC` scan and the PowerShell lane's + `ps::might_write_via_python3` token test — so `python -c`, `py -c`, `py3 -c`, `python2 -c` and + `python3.11 -c` ran the identical inline write unseen. The guard's own scope message advertised + `python -c` as its example, naming the one spelling the regex did not match. The command word is + now the python family (`py`/`python`/`pypy` plus an optional version suffix and `.exe`), still + separator-anchored, so `notpython3`, `mypython3`, `mypy`, `spy`, `happy` and `pytest` stay + inert. `py -3 -c` is admitted because a `-` token cannot be a script path; no other gap + between interpreter and flag is, so `python3 build.py` and `python3 -m tool …` are still not + blocked. + + 2. **A physical newline inside a quoted span split a producer from its own redirect.** A newline + reached with a quote still OPEN is not a separator — bash is inside a quoted word, so the text + either side of the span is ONE word — but `strip_literals` re-emitted it, `normalize_segments` + split there, and `producer_redirect_bypass` requires producer and redirect in one segment. The + join is now empty rather than a newline. **Empty, not a space:** `ec""ho x > f` is + `echo x > f` to bash, and a space join leaves `ec ho`, which `_producer_head` does not match — + the fix ships with that case as an assertion. Joining empty cannot manufacture a token bash does + not also form, because an open quote is what makes the two sides one word. The kept-operand and + backslash-newline joins are unchanged; the multi-line `--body`/`-m` prose floor is unchanged + because a dropped span's content is dropped either way. + + 3. **REOPENED ACCEPTED RESIDUAL** — a stdin heredoc (`python3 - <" echo x > f`, and it is a false positive + being removed rather than a new exemption. Fusing the two sides of a span back into one segment + also puts whatever preceded the span at the segment start: there bash's command word is `foo` and + `echo` is one of its *arguments*, so the redirect's producer is another program and this guard is + producer-scoped by design. The newline previously split it into a bogus `echo x > f` segment. The + single-line spelling `foo "a" echo x > f` was already allowed, so this makes the multi-line form + agree with shipped behaviour; both are asserted, as is the mirror case (`echo "a" x > f`, + where the command word really is the producer) which moves the other way. + + It is one row and not a class, verified rather than reasoned: every command PREFIX the file already + models — env assignments, `env`, `if…then`, `!`, `exec -a NAME`, a leading redirect — was probed in + front of a multi-line span against both hooks, and all still block, because `_cmd_prefix` / + `_modifier_opt_arg` / `_leading_redir` peel on the fused segment. Each is pinned with its + single-line control. Every remaining floor keeps its `rc=0`: the name anchor, the `#1601`/`#2148` + over-block repros re-run for each new spelling, the multi-line prose/`--body` floor, the + `/dev/null` discard floor, and the stdin floor. + + **Accepted residual, restated at its narrowed width:** `python3 <` version selector, so `python3 -O - <`) 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 `=:`** — `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`, and `python3 -c` with file-write indicators. Executable-token detection ignores quoted prose/commit text that merely mentions the pattern. | +| **block-hook-bypass** | PreToolUse · Bash \| PowerShell | **Blocks** (exit 2) | Bash file-write workarounds that circumvent the Write/Edit hook gates — `cat > file`, `echo … > file`, and inline python code with file-write indicators (`python`/`python3`/`py`/`pypy`, with `-c` or reading the program from stdin as `python3 - <`, and any commit taken while a merge/rebase/cherry-pick/revert is in progress. Resolves `bash -lc` wrappers and git aliases (inline `-c` and persisted config alike). | @@ -93,12 +93,17 @@ out of scope until such a signal exists. `build.sh` performs. It is also producer-scoped by design, so a redirect whose producer is another program (`sort f > out`, `curl … > page.html`, `cat a b > c`) is allowed — only a content producer writing a real file - (`cat > f` consuming stdin, `echo`/`printf > f`, inline `python3 -c` writes, + (`cat > f` consuming stdin, `echo`/`printf > f`, inline python writes, the PowerShell write cmdlets, including `Tee-Object` and its `tee` alias on the - PowerShell tool) is blocked. On the **Bash** tool, **`tee` / `tee -a` and inline - writes via other interpreters (`node -e`, `perl -e`, `ruby -e`, `sed -i`, `dd - of=`, `awk >`, …) are accepted residuals** — outside the modeled surface, not - oversights. The block message carries a lane-specific scope note so a reader does + PowerShell tool) is blocked. The python lane matches the interpreter FAMILY + (`py`, `python`, `pypy`, with an optional version suffix — `py -c`, `python -c`, + `python3.11 -c` are the same write as `python3 -c`), and since **0.28.0** it also + covers a program read from stdin with an explicit `-` (`python3 - <`, …) are accepted residuals** — + outside the modeled surface, not oversights. The block message carries a lane-specific scope note so a reader does not credit the guard with coverage it never claimed. - **`block-hook-bypass` has one target-scoped exemption beyond `/dev/null`, and it is off unless an operator turns it on.** `block_hook_bypass_scratch_roots` diff --git a/plugins/guardrails/hooks/block-hook-bypass.sh b/plugins/guardrails/hooks/block-hook-bypass.sh index 81bdbe9e7..962f2ef2c 100755 --- a/plugins/guardrails/hooks/block-hook-bypass.sh +++ b/plugins/guardrails/hooks/block-hook-bypass.sh @@ -355,12 +355,24 @@ strip_literals() { esac fi done - # A newline INSIDE a kept operand is literal content, not a separator — a - # quoted target may span physical lines, and a backslash-newline inside one - # is removed by bash outright. Either way the operand must stay one token. + # A newline reached with a quote span still OPEN is not a separator: bash is + # inside a quoted word, so the text before the opening quote and the text + # after the closing quote are ONE word. Emitting the newline handed + # normalize_segments a boundary bash does not have, which split a producer + # from its own redirect (`printf 'ab' > notes.md` was allowed while + # the `\n`-escaped spelling blocked) and could split a command word in half. + # + # A kept operand additionally marks the join OPAQUE — its content is literal + # and the pathname must stop being recoverable. A DROPPED span joins with + # NOTHING, and nothing is the only correct join: a space would still splice + # `ec""ho x > f` into `ec ho`, which bash runs as `echo`. Joining + # empty cannot manufacture a token bash does not also form, because an open + # quote is what makes the two sides one word in the first place. + # A backslash-newline inside an operand is removed by bash outright and joins + # the same way. if [[ -n "$open_quote" && -n "$open_keep" ]]; then result+="${out}${_MARK_OPAQUE}" - elif ((op_cont)); then + elif [[ -n "$open_quote" ]] || ((op_cont)); then result+="$out" else result+="${out}"$'\n' @@ -943,7 +955,7 @@ if [[ "$TOOL_NAME" == "PowerShell" ]]; then # mangle-resistant CO-OCCURRENCE of # (a) a write INDICATOR in the raw command (_py_write — the tokens live in the # quoted `-c` payload, so the scan is raw, exactly as the Bash lane), AND - # (b) a python3 interpreter TOKEN plus a `-c` inline-code flag both present + # (b) a python interpreter TOKEN plus a `-c` inline-code flag both present # (ps::might_write_via_python3, quote-INTACT + backtick-recovered) — where a # COMPUTED `-c` (`python3 ('-'+'c') …`) is caught by fail-closing on a # non-tokenizable arg construct when no literal `-c` is present. @@ -953,11 +965,17 @@ if [[ "$TOOL_NAME" == "PowerShell" ]]; then # fail-closed choice the user approved for this lane): a command that only MENTIONS # `python3 … -c` + a write indicator in prose, a line/block comment, or a quoted # string now blocks; here-string mentions stay inert (blanked first, like the git - # lane). ACCEPTED RESIDUAL: a stdin heredoc (`python3 - <` token cannot be a script path; no other gap +# between the interpreter and its flag is allowed, so a script/module run +# (`python3 build.py`, `python3 -m tool …`) that merely touches an `open(`-like +# path is still NOT blocked. +_py_inline_c='(^|[[:space:];|&()/\]+)(pypy|python|py)[0-9]*(\.[0-9]+)*(\.exe)?([[:space:]]+-[0-9]+(\.[0-9]+)?)?[[:space:]]+-c' +# REOPENED ACCEPTED RESIDUAL (#2217 / AD-12). A stdin heredoc — `python3 - < blocked; the floors interleaved with them are +# what pin that NO assertion moved blocked -> allowed. + +# (1) THE INTERPRETER SPELLING FLOOR. The detector required the literal +# `python3`, so the same inline write spelled with any other name in the family +# ran unseen. The guard's own scope note advertised `python -c` as its example. +run "#2217: python -c open write (blocked)" \ + "python -c \"open('x','w').write('a')\"" 2 +run "#2217: py -c open write (blocked)" \ + "py -c \"open('x','w').write('a')\"" 2 +run "#2217: py3 -c open write (blocked)" \ + "py3 -c \"open('x','w').write('a')\"" 2 +run "#2217: python2 -c open write (blocked)" \ + "python2 -c \"open('x','w').write('a')\"" 2 +run "#2217: python3.11 -c open write (blocked)" \ + "python3.11 -c \"open('x','w').write('a')\"" 2 +run "#2217: pypy3 -c open write (blocked)" \ + "pypy3 -c \"open('x','w').write('a')\"" 2 +# The Windows launcher's version selector is admitted because a `-` +# token cannot be a script path. No other gap between interpreter and flag is. +run "#2217: py -3 -c open write (blocked)" \ + "py -3 -c \"open('x','w').write('a')\"" 2 +run "#2217: path-qualified python.exe -c open write (blocked)" \ + "/c/Python313/python.exe -c \"open('x','w').write('a')\"" 2 + +# THE NAME-ANCHOR FLOOR. Widening the name must not widen the boundary: every +# one of these merely CONTAINS a family spelling and stays inert. +run "#2217: mypy -c open write (allowed)" \ + "mypy -c \"open('x','w').write('a')\"" 0 +run "#2217: happy -c open write (allowed)" \ + "happy -c \"open('x','w').write('a')\"" 0 +run "#2217: spy -c open write (allowed)" \ + "spy -c \"open('x','w').write('a')\"" 0 +run "#2217: pytest -c open write (allowed)" \ + "pytest -c \"open('x','w').write('a')\"" 0 +run "#2217: mypython3 -c open write (allowed)" \ + "mypython3 -c \"open('x','w').write('a')\"" 0 +# THE OVER-BLOCK FLOOR (#1601 / #2148 are open against this arm in the OPPOSITE +# direction). Widening the name multiplies whatever false-positive rate the arm +# has, so the read-only and script-run shapes are pinned for the new spellings +# too — not just for `python3`. +run "#2217: python -c read-only open (allowed)" \ + "python -c \"print(open('f').read())\"" 0 +run "#2217: py -c print only (allowed)" \ + "py -c \"print(1+1)\"" 0 +run "#2217: python3.11 -c os.path.normpath (allowed)" \ + "python3.11 -c \"import os; print(os.path.normpath('a/b'))\"" 0 +run "#2217: python -m module run (allowed)" "python -m mytool --out f" 0 +run "#2217: python build.py script run (allowed)" "python build.py" 0 +run "#2217: python --version (allowed)" "python --version" 0 +run "#2217: py --list (allowed)" "py --list" 0 + +# (2) A PRODUCER SPLIT FROM ITS OWN REDIRECT BY A PHYSICAL NEWLINE. A newline +# reached with a quote span still OPEN is not a separator — bash is inside a +# quoted word — but strip_literals re-emitted it, so normalize_segments split +# the producer from the redirect it owned. The `\n`-escaped spelling of the same +# command always blocked; only the physical newline slipped. +PY_NL_SQ=$(printf 'printf \x27a\nb\n\x27 > notes.md') +run "#2217: printf, physical newline in a single-quoted arg (blocked)" "$PY_NL_SQ" 2 +PY_NL_DQ=$(printf 'echo "a\nb" > notes.md') +run "#2217: echo, physical newline in a double-quoted arg (blocked)" "$PY_NL_DQ" 2 +run "#2217: control — the same write with an escaped newline (blocked)" \ + "printf \"a\\nb\\n\" > notes.md" 2 +# THE DISCRIMINATOR between joining the two sides with NOTHING and joining them +# with a space. `ec""ho` is ONE bash word and bash runs it as `echo`; +# only an empty join reconstructs it. A space join leaves `ec ho` and this +# assertion fails — which is why the shipped fix does not use one. +PY_NL_SPLICE=$(printf 'ec"\n"ho x > f') +run "#2217: quote span splicing a command word (blocked)" "$PY_NL_SPLICE" 2 + +# THE MULTI-LINE PROSE FLOOR. The whole point of carrying an open quote across +# lines is that a `--body`/`-m` payload merely MENTIONING a write stays inert. +# Dropping the join newline must not change that — the span's content is dropped +# either way — so these stay allowed. +PY_NL_BODY=$(printf 'gh pr create --body "line one\necho x > f\nline three"') +run "#2217: multi-line PR body mentioning a write (allowed)" "$PY_NL_BODY" 0 +PY_NL_MSG=$(printf 'git commit -m "subject\n\ncat > notes.md is a bypass\n"') +run "#2217: multi-line commit message mentioning a write (allowed)" "$PY_NL_MSG" 0 +PY_NL_GREP=$(printf 'grep "foo\nbar" file | wc -l') +run "#2217: multi-line grep pattern piped, no redirect (allowed)" "$PY_NL_GREP" 0 +PY_NL_DEVNULL=$(printf 'echo "a\nb" > /dev/null') +run "#2217: multi-line span discarded to /dev/null (allowed)" "$PY_NL_DEVNULL" 0 +PY_NL_PIPE=$(printf 'printf \x27a\nb\x27 | wc -c') +run "#2217: multi-line span piped, no redirect (allowed)" "$PY_NL_PIPE" 0 +PY_NL_DANGLE=$(printf 'git status ; echo "dangling') +run "#2217: unterminated quote at end of command (allowed)" "$PY_NL_DANGLE" 0 +# Already blocked before the change; the segment split it relies on is a real +# separator on the closing line, so it must keep blocking. +PY_NL_THEN=$(printf 'grep "a\nb" f ; echo x > out.txt') +run "#2217: multi-line span then a real producer+redirect (blocked)" "$PY_NL_THEN" 2 + +# THE ONE ROW THAT MOVES THE OTHER WAY, pinned deliberately. Fusing the two +# sides of the span back into one segment can also put a NON-producer at the +# segment start, and that is the correct reading: in `foo "a" echo x > f` +# bash's command word is `foo` and `echo` is one of its ARGUMENTS, so the +# redirect's producer is another program and this guard is producer-scoped by +# design. Before the change the newline split it into a bogus `echo x > f` +# segment and it blocked — a false positive. The single-line spelling of the +# same command is the control: it has always been allowed on `main`, so this +# makes the multi-line form agree with the shipped single-line behavior rather +# than inventing a new exemption. Both are asserted so the agreement is pinned. +PY_NL_ARGECHO=$(printf 'foo "a\n" echo x > f') +run "#2217: producer is foo, echo is its argument (allowed)" "$PY_NL_ARGECHO" 0 +run "#2217: control — same command on one line (allowed)" "foo \"a\" echo x > f" 0 +# The mirror image, and the reason the row above is not a hole: when the command +# word really IS the producer, fusing the segment is what reveals the write. +PY_NL_REALECHO=$(printf 'echo "a\n" x > f') +run "#2217: producer is echo, span is its argument (blocked)" "$PY_NL_REALECHO" 2 +run "#2217: control — same command on one line (blocked)" "echo \"a\" x > f" 2 +# Same fusion on the cat lane, with its own single-line control. +PY_NL_CAT=$(printf 'cat "a\n" > f') +run "#2217: cat with a multi-line quoted arg then a redirect (blocked)" "$PY_NL_CAT" 2 +run "#2217: control — same command on one line (blocked)" "cat \"a\" > f" 2 +# A span that closes and re-opens across lines fuses at both joins. +PY_NL_TWO=$(printf 'echo "a\nb" "c\nd" > f') +run "#2217: two multi-line spans, one producer+redirect (blocked)" "$PY_NL_TWO" 2 +# THE BOUNDARY OF THE ROW ABOVE, and the reason it is one row and not a class. +# Fusion puts whatever preceded the span at the segment start, so the question is +# whether a LEGITIMATE command prefix in front of a multi-line span can hide a +# real producer from `_producer_head`'s `^` anchor. It cannot: `_cmd_prefix`, +# `_modifier_opt_arg` and `_leading_redir` peel exactly these, and the peel runs +# on the fused segment. Each is paired with the single-line control that already +# blocked on `main`, so a regression in the peel shows up as a pair splitting. +PY_NL_ENVASSIGN=$(printf 'FOO="a\nb" echo x > f') +run "#2217: env-assignment prefix before a multi-line span (blocked)" "$PY_NL_ENVASSIGN" 2 +run "#2217: control — same command on one line (blocked)" "FOO=\"ab\" echo x > f" 2 +PY_NL_ENV=$(printf 'env FOO="a\nb" echo x > f') +run "#2217: env modifier before a multi-line span (blocked)" "$PY_NL_ENV" 2 +run "#2217: control — same command on one line (blocked)" "env FOO=\"ab\" echo x > f" 2 +PY_NL_IF=$(printf 'if true ; then echo "a\nb" > f ; fi') +run "#2217: compound-command header before a multi-line span (blocked)" "$PY_NL_IF" 2 +PY_NL_NEG=$(printf '! echo "a\nb" > f') +run "#2217: pipeline negation before a multi-line span (blocked)" "$PY_NL_NEG" 2 +PY_NL_EXECA=$(printf 'exec -a n echo "a\nb" > f') +run "#2217: exec -a NAME before a multi-line span (blocked)" "$PY_NL_EXECA" 2 +PY_NL_LEADREDIR=$(printf '> f echo "a\nb"') +run "#2217: leading redirect before a multi-line span (blocked)" "$PY_NL_LEADREDIR" 2 +run "#2217: control — same command on one line (blocked)" "> f echo \"ab\"" 2 +PY_NL_PRINTF=$(printf 'FOO=1 printf "a\nb" > f') +run "#2217: env-assignment prefix, printf producer (blocked)" "$PY_NL_PRINTF" 2 +# The cat lane keeps its own shape: `cat FILE x > f` is a copy, not a stdin +# consume, and `_cat_redir` needs `cat` immediately before the redirect. Fusion +# leaves the argument words in place, so this stays allowed — before and after. +PY_NL_CATARG=$(printf 'cat "a\nb" x > f') +run "#2217: cat with args between it and the redirect (allowed)" "$PY_NL_CATARG" 0 + +# (3) THE REOPENED ACCEPTED RESIDUAL. `python3 - <