fix(guardrails): PowerShell here-string / bare-CR shapes hiding git (#4683) - #5036
Merged
Merged
Conversation
…ent-span, and bare CR
Generalize the no-token herestring-comment-char flag to a reduction-untrusted
family so a quote/backslash/backtick on a confirmed opener prefix, a <# earlier
in the command, or a CR that is not part of CRLF cannot drop live git or write
lines as here-string body. classify returns 2 when the flag is up even if
another sink trigger already fired, so a { plus commented opener reaches the
FLAG-keyed readers. No new allow tokens.
Closes #4683
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…ing git as data payload_has_bare_cr now glob-matches the two-character JSON \r escape (a single-quoted \\r pattern was two backslashes and missed the encoding hook::jq_fields leaves on INPUT). Git-freedom after a trusted opener still runs on PS_BLANKED, so a verbatim body that merely names git stays data. Closes #4683 Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
A column-zero '@ / "@ with no confirmed opener hid git behind a quote the Bash tokenizer never closed. Trailing CR is chomped per here-string line so a CRLF canonical commit still classifies as the LF form. Guardrails 0.40.2. Closes #4683 Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
The trailing-space here-string opener is unconfirmed, so `"@` at column zero is herestring-orphan-closer. Pin the budget-exhausted message on the quoted-call no-progress shape, and expect the git-free payload to refuse as that closer. 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. |
…reset Shellcheck SC2034 flags the cross-file reset of PS_HERESTRING_OPENER_COMMENT_CHAR. The portability gate flags fixture strings that name grep or sort. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
This was referenced Sep 29, 2026
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 #4683.
Supersedes closed draft #4978 (stale tip on
cursor/4683-ps-herestring-bare-cr-37e9).Summary
Refuse PowerShell here-string opener-untrusted, comment-span, and bare-CR shapes that hide git from the dangerous-git / no-verify / convention / noncanonical matchers. Match JSON
\rfor bare CR; keep verbatim here-string git as data; refuse N2 orphan closers; retarget #4682 budget pins.Claim
These PowerShell shapes must refuse when they would otherwise hide git mutation from the guardrails matchers.
Basis
#4683 reproduction shapes; Windows PowerShell here-string / CR semantics; in-repo matcher differential vs
origin/main(no under-block). Real matcher coverage, not a docs park.As of
2026-09-28
Recheck
New PS hide shapes appear in the differential corpus, or upstream PowerShell quoting changes the opener grammar.
Test plan
block-dangerous-gittable payloads for fix(guardrails): refuse PowerShell here-string and bare-CR shapes hiding git #4683 ok after budget-pin retargetorigin/mainexit 0