fix(guardrails): PowerShell guard differential + budget/flag-keyed refusals (#4682) - #4715
Merged
Merged
Conversation
) scripts/check-guardrails-ps-differential.sh <base-ref> extracts the base ref's plugins/guardrails with git archive and runs a corpus of PowerShell commands, as stdin payloads, through both trees: each of the five blocking consumers directly, the run-guards.sh Bash row with each tree's own hooks.json argv, and block-dangerous-git under all 31 non-empty ps-unparsable-* token subsets for commands the base refuses token-free. It exits 1 on any cell where the base exits 2 and the working tree does not, and prints the cell table. Every cell pins CLAUDE_PLUGIN_ROOT to its own tree and clears inherited plugin options, so an operator's environment cannot hand both arms one library or decide a cell. The corpus is 582 commands harvested from the guard suites (--harvest) plus 31 hand-written ones. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
… budget (#4682) block-dangerous-git exited 0 once _ps_sink_attempts passed 4, with the remainder of the command never read. Under all five ps-unparsable-* tokens a command with five sink triggers and git reset --hard passed, and so did a granted shape whose blanking makes no progress, such as $a=& 'git reset --hard' under ps-unparsable-dynamic-invocation alone. The loop now re-classifies after the fifth blank and refuses at the top of the next round (form powershell-unparsable-budget-exhausted), and no allow token clears it. Token-free behavior is unchanged. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…ag (#4682) block-convention-violation and block-noncanonical-commit refused a commented here-string opener only when PS_SINK_TRIGGER was herestring-comment-char, which the classifier records only when no other trigger fired. A commit whose opener carried a # behind a { or an iex reported that other trigger and both guards deferred. They now test PS_HERESTRING_OPENER_COMMENT_CHAR, a strict superset that only moves exits from 0 to 2. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Contributor
|
PR body contract — issue linkage This PR body does not yet satisfy the issue-linkage contract:
Edit the body and this comment updates itself on the next run. |
Excuse the jq regex class and the fixture sed -i, and mark the differential test executable. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Re-bump guardrails to 0.38.2 after 0.38.1 landed. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Re-bump guardrails to 0.38.3 after 0.38.2 landed. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
# Conflicts: # plugins/guardrails/CHANGELOG.md Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
kyle-sexton
added a commit
that referenced
this pull request
Sep 30, 2026
…5340) No related issue: audit follow-up for the guardrails plugin. Every issue it touches is already closed and several are being reopened by hand, so no closing keyword is used. ## Summary An audit of the 499 PRs an unattended agent opened between 2026-09-27 and 2026-09-29 found guardrails defects in six groups: guard gaps that let a destructive command through, tests that no longer test what they claim, a non-atomic cache, an undeclared Node requirement, README and CHANGELOG statements that were wrong, and an unratified performance claim in PLAN.md. This PR fixes what does not need the owner, and leaves the owner-reserved questions alone (see Related). Guardrails goes 0.41.9 to 0.42.0. ## Fix - `block-windows-drive-tmp`: a drive-root `\tmp` is refused on a usertemp host; `curl`/`wget` short-flag clusters, `--output-dir` and an option after a bare `-O` are judged; the usertemp mount fallback needs `usertemp` on the `/tmp` mount's own line. The suite feeds commands on stdin, so the 9 `/usr/bin` writer cases that were skipped now run. - `block-root-delete-target` (PowerShell): a delete after LF, CRLF or a bare CR, or inside a scriptblock, grouping or `$( )`, is judged; `$env:NAME\subpath` no longer refused as a bare variable (`$env:TEMP`, `$env:TEMP\`, `$env:TEMP\*` still are). - PowerShell classifier: one untrusted-reduction flag instead of two, CR scan skipped when the command has no CR, sink-budget tests rebuilt so they exercise budget exhaustion. - `stale-path-verify` and `skill-reference-verify`: a partly written cache is a miss and is rebuilt; no extra process on the hook path. - `wsl` regression rows added to the `block-hook-bypass`, `block-noncanonical-commit` and `block-convention-violation` suites. - Node on PATH is declared in the README, checked by `/guardrails:setup check`, and listed in `prerequisites.json`; setup's apply text follows the reconfiguration convention. - README: `wsl` since-version, six re-parsing guards, PowerShell no-token over-blocks, plugin-scoped GitHub MCP matcher, contributor to-do and tracker asides removed. PLAN.md: the "no further process can be removed" floor is scoped to the PostToolUse cold path and the goal is marked a draft. - CHANGELOG: five released entries corrected in place (0.41.7, 0.40.0, 0.38.11, 0.38.1, 0.37.3), no heading removed. They are named in the new 0.42.0 entry. - From other groups' requests (extending the 0.42.0 entry, no second bump): - README: the exec-form dispatcher rows are dated to 0.41.3 (they said 0.41.0), and each fire is stated as two processes, node and then the bash it spawns, with the candidate order left to the header of `hooks/exec-bash.mjs` (hook-launcher). - `/guardrails:setup check` probes the bash `hooks/exec-bash.mjs` resolves, not the Bash tool's own bash (hook-launcher). - PLAN.md says its census rows and floor line were measured under shell form and not re-measured under exec form, and that no hook or skill reads it (hook-launcher). - `exec-bash.test.sh` accepts the first bash on `PATH` (a Homebrew bash) as well as `/bin/bash` and `/usr/bin/bash`, and the plugin's resolver test gains the PATH-first cases from `lib/exec-bash.resolver.test.mjs` (hook-launcher). ## Verification - `git merge origin/main`: two conflicts, the plugin version and the CHANGELOG head, resolved by keeping 0.42.0 above the 0.41.9 entry that #5309 added on main. - `scripts/check-changelog-parity.sh --check --check-order`: pass. `--check-preserved origin/main`: all 225 headings preserved. - `scripts/validate-plugins.sh`: all manifests and the catalog validate. - `scripts/affected-tests.sh --run --jobs 8`: every guardrails suite passes (block-windows-drive-tmp, block-root-delete-target, block-dangerous-git, block-hook-bypass, block-noncanonical-commit, run-guards, stale-path-verify, skill-reference-verify, setup and the rest). Three suites fail on this host for missing tools, unrelated to the change: `scripts/check-html-assets.test.sh` and `scripts/check-script-contract.test.sh` (htmlhint not installed), `scripts/hook-census.test.sh` (strace not installed). 25 Node and Python suites are selected but belong to other lanes. - `scripts/check-guardrails-ps-differential.sh origin/main`: 613 commands, 0 cells where main refuses and this branch does not. The scratch under-block differentials for block-windows-drive-tmp (6648 cells, 0 lost) and block-root-delete-target (15823 payloads; the 31 cells that differ are `$env:NAME\subpath` spellings Bash also allows, plus one backtick-before-CR-CR-LF read as PowerShell reads it) are recorded in the task notes. - Baseline differential runs at the two earlier commits and at main: 0 cells lost in all three. Substitution-cap cost on WSL2 Linux: 2000 unquoted `$(:)` took a median 6038 ms; the same 2000 inside single quotes or a heredoc body took 140 ms, level with a 137 ms control with no substitution text. No Windows measurement was taken. - No Windows host and no strace here: Windows behavior is proven by the test-windows lane after the ready flip, and the spawn ceilings by CI's ratchet step. - Cross-group pass, on head `c3111b8d6` after merging origin/main `0589e13f4` (clean): - `exec-bash.test.sh` and `exec-bash.resolver.test.sh` pass. With a bash symlink first on `PATH` outside `/bin` and `/usr/bin`, the old pin fails (`FAIL: bash was not a real path`) and the new one passes. - `check-changelog-parity.sh --check`, `--check-order`, `--check-preserved origin/main` (226 headings) and `--check-bump origin/main` pass. - `validate-plugins.sh`, `sync-exec-bash.sh --check` (21 copies) and `check-skill.sh` on the setup skill (PASS, 0 errors) pass. So do `coverage-manifest.test.sh` (17 checks), `check-skill-portability.sh` and `check-shell-portability.sh`. markdownlint, typos and shellcheck are clean on the changed files. - `scripts/affected-tests.sh --run --jobs 8` against origin/main: 287 selected suites pass. The same three as before fail on this host for missing tools: `check-html-assets.test.sh` and `check-script-contract.test.sh` (htmlhint), and `hook-census.test.sh` (strace). - PR #4715's disclosed "`git reset --hard` + nested here-string" bypass: 12 PowerShell nested here-string payloads carrying `git reset --hard` exit 2 from `block-dangerous-git.sh` on this branch and on main, so no issue was filed. One Bash probe exits 0 on both: a `-c` operand built from a command substitution, which is the README's declared `$(…)` residual. - Ready pass, on head `600ec5b3c` after merging origin/main `a82943beb` (clean; main's 7 new commits touch no guardrails path): - Security review of the PR diff: no findings. Every guard change adds refusals or matches the Bash lane. Probes still refuse `$env:TEMP\..`, `${env:TEMP}\.`, a stray `)` or `}` before a delete, a delete after a keyword block, unclosed levels, and a delete after a JSON `\r` or `\u000d`. A raw control byte in the payload is invalid JSON, so the CR re-read keeps the command as first read. - `check-changelog-parity.sh` (`--check`, `--check-order`, `--check-preserved origin/main`, `--check-bump origin/main`), `validate-plugins.sh`, `sync-exec-bash.sh --check`, shellcheck, markdownlint and typos on the changed files: pass. `exec-bash.test.sh` and the node resolver test pass. - `scripts/affected-tests.sh --run --jobs 8 origin/main`: the same three suites fail for missing tools (htmlhint, strace). `audit-coverage.test.sh` and `cant-fail-scan.test.sh` also failed in the 8-job run; both pass run alone on this head and on a snapshot of `a82943beb`, and this PR does not touch their paths. ## Related Audit: `.work/audit/REPORT.md` (local, not committed). Refs #3683 #3686 #3951 #4118 #4236 #4242 #4247 #4251 #4261 #4390 #4516 #4527 #4528 #4651 #4678 #4679 #4681 #4682 #4683 #4685. Left for the owner, not implemented here: - #4390: reopened with a decision packet on the cache design, plus an addendum on PLAN.md's Done-when (ratify the ceilings, k × S, or re-measure on Windows first) and `async` keep-or-reverse. - #4684: reopened with a decision packet on the substitution cap against the hook-precision rule. - #5341: a new decision issue for block-root-delete-target scope (launcher grammar, second PowerShell parser). - #4118, #4681 and #4683: reopened with ratify-or-reverse packets, because #4766, #4755 and #5036 took decisions reserved for the owner and were merged by `app/cursor`. #3683 gets the same packet for #4845's host-skip call. - #4679: a packet on whether hook-observability condition 3 admits a once-per-(session, agent) latch, or the admission is recorded as an owner-approved exception. - #3951: a packet on whether to build the heredoc-aware fix now in its own PR, since the owner's 2026-09-28 comment closing #4811 said more guard logic is not justified now. Operator-only steps stay open on #3683, #4236, #4261, #4527, #4678, #4679 and #4684 (Windows-host measurements and runs, and one interactive render check). Found and declared, not fixed: `env -f x rm -rf /` exits 0; PowerShell `$r = Remove-Item ...`, comma-list and here-string forms remain gaps in block-root-delete-target. An unmeasured Windows `RUN_GUARDS_PROFILE` cost stays on #4236. Cross-group requests: - claude-ops: `audit_skill_visibility.test.sh` skips its whole `--installed` contract on any Git Bash host (apply `host_path` to the `--installed` argument or narrow the probe; correct the CHANGELOG guess). - claude-config: `unhobble/SKILL.md` (~L382) and `unhobble/evals/evals.json` restate the block-hook-bypass exit-code claim with no verification record. - planning: `planning/skills/setup/SKILL.md` (~L158) has an unwrapped line in the apply bullet. - conventions: `docs/conventions/hook-observability/README.md` condition 3 needs to say whether a once-per-(session, agent) latch counts as a state transition. - scripts: add the shapes from the PowerShell differential findings to `scripts/guardrails-ps-differential-corpus.jsonl`. - hook-launcher: #5309 (0.41.9) landed the PATH lookup and the visible notice for an unresolvable bash in `exec-bash.mjs`. Still open: a missing node needs a launcher design that does not depend on node (#3708). The README wording and the PLAN.md census note are now in this PR. - scripts: the 12 nested here-string `git reset --hard` payloads above are candidates for the same corpus. Cross-group requests received, checked against origin/main `0589e13f4`: - hook-launcher: - The README exec-form version and two-process shape, the PLAN.md annotation, the relaxed launcher test pin, the PATH-first resolver cases and the setup bash probe: applied (see Fix). - Node declaration: already done here (README Requirements, setup check item 3, `prerequisites.json`). - Five owner decisions: #4390 already had its packet. #3683 was already reopened and now has a ratify-or-reverse packet. #4118, #4681 and #4683 are now reopened with packets. #3686 belongs to hook-launcher. - ci: - #4527 already carries the per-suite questions. - The `\tmp\x` downgrade follow-up is not filed, because bca5f57 in this PR restores the block assertion: the helper no longer downgrades a backslash-led `\tmp`, and the quoted spellings expect 2. - #3683: #5316's windows-2025 run shows `cloud-bootstrap-plugins` PASS=79 with no probe skip, so its probe needs no rework. The results, and #4845's causes marked as hypotheses, are on #3683. - tracker: - #3951 not implemented: owner decision (packet above). - #4235 not delivered by this PR; it needs its own dispatch. The owner's comment closing #4817 asks for "a design scoped to #4235", which belongs in its own PR: its `ps-command.sh` changes overlap this PR's classifier edits and need the token-subset differential. The tracker posts the scope comments. - #4678: reopened with Windows operator steps (not run here). - #4679's render confirmation: already an open item with the exact steps. - The #4715 bypass: probed, not reproducible (see Verification). Its payloads go to the scripts group rather than this PR, because the corpus lives in `scripts/`. - conventions: - The setup apply bullet already carries main's scope-matching caveat 2. - #4679: the condition-3 packet is posted. The wording goes to conventions after the owner answers. - source-control: commented on #4247 with #5317's widened matcher. - performance: PLAN.md is already relabelled and #4390 already carries both labels. The Done-when and `async` questions are posted as an addendum. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4682
Summary
Adds
check-guardrails-ps-differential.sh(613-command corpus) proving branch never allows what base refuses. Lands scoped fixes: sink-attempt budget exhaustion refuses (no allow token); flag-keyed here-string comment refusals for convention/noncanonical commit guards.guardrails→ 0.37.5 (serialize after #4684 / 0.37.4).Notes
Unfiled bypass remains on both main and branch (
git reset --hard+ nested here-string shape) — out of this issue's scope; see implementer note.Test plan