fix(disk-hygiene): do not flag PowerShell 2>&1 as file redirect - #2627
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 1m 27s —— View job Security review — no issues found
This PR changes the regex Old: Analysis:
No injection, authorization, secrets-handling, or CI/workflow-permission issues in this changeset — it's a self-contained Python regex fix plus a test and changelog/version bump. No security concerns to report. |
|
Last security-reviewed head: |
Exclude stream-merge forms (`2>&1`, `*>&1`) from `_POWERSHELL_OUTPUT_REDIRECT` so ordinary stderr capture no longer prompts as a file-overwriting mutation (#2615). File redirects like `2>out.txt` still ask. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
45d24d5 to
1502bd8
Compare
|
Claude finished @cursor[bot]'s task in 26s —— View job Code review — skipped
Skip gate: this PR is already MERGED (not open), which matches skip-gate condition 1 ("PR is closed or not open") in the For reference, this PR was already reviewed for security by the |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…direction (#2671) ## Problem PR #2627 closed issue #2615 by excluding stream merges (`2>&1`, `1>&2`, `*>&1`) from `_POWERSHELL_OUTPUT_REDIRECT`. That fix is correct but partial: it only excludes `&`. In a **discard** the character after `>` is `$`, not `&`: ```python # on origin/main _POWERSHELL_OUTPUT_REDIRECT = re.compile(r"(?<![<>])>(?![=>&])") ``` So `2>$null` — PowerShell's `/dev/null`, and the standard way to silence a noisy read-only command — still matches, and the guard still prompts with *"disk-hygiene flagged shell output redirection (may overwrite a file)"*. The issue closed while the operator's actual failing commands kept prompting. ## Observed cost Two read-only commands from an attended session, both prompted: - `gh issue list --repo <owner/repo> --state open 2>$null` - `chezmoi managed | Select-String -Pattern claude 2>$null` Neither can write a file. Each prompt stalled the session. This is the approval-fatigue mechanism issue #2615 itself describes: a destructive-action guard that cries wolf on read-only work measurably degrades the signal of its genuine prompts. ## Fix ```python _POWERSHELL_OUTPUT_REDIRECT = re.compile( r"(?i)(?<![<>])>(?![=>&])(?![^\S\n]*\$null(?![\w-]))" ) ``` Three deliberate details: - **`(?i)`** — PowerShell variable names are case-insensitive, so `2>$NULL` is the same discard. - **`[^\S\n]*`** — only *horizontal* whitespace is skipped, so a trailing `>` at end of line cannot borrow a `$null` from the next line. - **`(?![\w-])`** — `$nullish` and `$null-backup` are ordinary variables, not the null device, and must still be treated as file targets. The exclusion is spelled the way guardrails' `ps::write_bypass` already spells the same `$null` exclusion, rather than inventing a second spelling for the same concept in the same fleet. ## Coverage Allowed (newly, 9 forms): `2>$null`, `*>$null`, `>$null`, `2> $null`, `2>$NULL`, `2>$null; <cmd>`, `2>$null | <cmd>`, and the two real-world commands above. Still flagged (7 forms): `2>out.txt`, `> out.txt`, `1>file`, `2>$nullish`, `2>$null-backup`, `2>$null > out.txt`, `2>&1 > out.txt` — the last two being the important ones: discarding one stream while redirecting another is still a file write, and the second `>` has no `$null` after it. ## Test evidence `test_powershell_null_discards_are_not_file_redirects` covers every form above. Full-suite comparison, same interpreter, same machine: | tree | tests | result | |---|---|---| | `origin/main` (pristine, extracted via `git archive`) | 282 | 3 failures, 4 skipped | | this branch | 283 | **the same 3 failures**, 4 skipped | The three pre-existing failures are `test_deny_emits_blocked_telemetry_when_sink_wired`, `test_preview_allows_root_children_os_managed_snapshot`, and `test_stash_must_exist_in_an_independent_checkout`. They are identical on both trees and unrelated to this change; this branch adds one passing test and introduces no new failure. ## Adjacent gap found, deliberately NOT fixed here `>>` (append) is matched by **neither** the old nor the new pattern — `(?![=>&])` rejects the first `>` of the pair, and the cmdlet word list catches `out-file`/`add-content` but not a bare `>>` redirect. So `<cmd> >> append.txt` writes a file without a prompt. That is pre-existing on `main` and orthogonal to this change, so it is reported rather than folded in. Worth its own issue. ## Related - #2627 — the stream-merge fix this completes; it closed #2615 for the `>&` form only. - #2674 — concurrent PR against the same plugin; it claims version `0.20.3`, this one `0.20.2`. - The `>>` append gap described above is unfiled and needs its own issue. Closes #2615 --------- Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
<!-- CURSOR_AGENT_PR_BODY_BEGIN --> Closes #2675 ## Summary `_POWERSHELL_OUTPUT_REDIRECT` matched neither character of a `>>` pair, so `<cmd> >> append.txt` wrote a file with no prompt while the same command with `>` prompted. ## Fix - Explicit append matcher (`>>` / `2>>` / `*>>`) without widening stream-merge / `$null`-discard behavior - `>> $null` stays silent, requiring a real token terminator after `$null` (same boundary as #2671) - Punctuation continuations like `>>$null/out.txt` stay flagged - Plugin version `0.20.5` (above #2671's `0.20.4`) ## Verification Focused GuardTests for append redirects and stream merges pass locally. ## Related Refs #2615 / #2627 / #2671 — stream-merge and `$null`-discard carve-outs that must stay narrow. <!-- CURSOR_AGENT_PR_BODY_END --> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Closes #2615
Summary
The disk-hygiene PowerShell belt treated
2>&1stream merges as file-overwriting redirects because_POWERSHELL_OUTPUT_REDIRECTmatched the>in2>&1.Fix
Exclude PowerShell stream-merge redirections (
N>&M) from the file-overwrite detector. Add regression coverage.Verification
See branch CI / local destructive_guard / hygiene tests on the PR checks.
Related
Refs #2589 — argument-surface docs (serial disk-hygiene lane).
Refs #2591 / #2618 — belt lifetime (later wave).