fix(classify): Remove-Item Users overmatch - deep per-user paths are not a system root#70
Conversation
…not a system root The Windows recursive-force-delete black rule matched `Users\b`, which made EVERY path under any user profile a "drive/system root" — a deep project dir (...\Desktop\proj\_worktree) black-blocked the same as C:\. That contradicts the rule's own comment (drive-root / system targets only) and the documented `Remove-Item node_modules stays clean` stance. The Users branch now blacks only C:\Users itself or a WHOLE profile (C:\Users\<name> - one segment, then end-of-target, separator-smuggle still caught). Windows/Program branches unchanged. Companion RED rule: any Remove-Item -Recurse -Force of a drive-anchored absolute path is now red (approve) rather than invisible - relative deletes like node_modules stay allow, preserving the no-FP guarantee. Tests: +6 black cases (Users root, whole profile quoted/unquoted, separator smuggle, System32) and a new red-not-black test for deep per-user and absolute non-system paths. 200/200 pass.
sprayberry-reviewer
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Verdict: no blocking findings — approving. Two non-blocking observations inline.
What I checked
src/classify.mjs— new BLACK regex for the Users branch, new RED_SHELL rule for all drive-anchored Remove-Item,Windows→Windows\btightening.test/attacks.test.mjs— 6 new block assertions + 2 new approve assertions (deep per-user and absolute non-system paths).- Ran all regex permutations locally:
C:\Users(BLACK),C:\Users\bob(BLACK),"C:\Users\bob"(BLACK),C:\Users\bob; echo done(BLACK),C:\Windows\System32\config(BLACK),node_modules(allow),.\\node_modules(allow),C:\code\repo\node_modules(RED/approve), deep pathC:\Users\bob\Desktop\proj\_worktree(RED/approve). All matched stated expectations.
Non-blocking observations
1. ..-traversal through Users goes BLACK → RED (no test covers it)
The old regex matched Users\b, which caught C:\Users\bob\..\Windows\System32 because the \b fires between s and \. The new Users branch:
src/classify.mjs:167
Users(?:\\[^\\"';|&]+)?\\?["']?\s*(?:$|[;|&])
The [^\\"';|&]+ character class excludes \, so it captures exactly one non-backslash segment (bob), then requires end-of-command ($ / ; / | / &). For C:\Users\bob\..\Windows\System32 the next char after \bob\ is ., not an end-of-command character, so the Users branch fails. The Windows\b branch also fails (text immediately after C:\ is Users, not Windows). The command falls through to the new RED_SHELL rule, which catches it (approve, not block).
Remove-Item -Recurse -Force C:\Users\bob\..\Windows\System32 is a realistic payload — PowerShell resolves .. at the filesystem level. It was previously AUTO-BLOCKED; it is now APPROVE (human review required). The RED rule is a sound safety net here, but there is no test for this specific case, and the PR description says "separator-smuggle still caught" — it is, but only at RED level, not BLACK.
Suggested addition (non-blocking, for documentation parity):
// In 'more destructive blocked' or a new test:
// Traversal through a profile to a system dir is approve, not block
approve({ tool: 'powershell', input: { command: 'Remove-Item -Recurse -Force C:\\Users\\bob\\..\\Windows\\System32' } });This won't change behaviour (it already approves), but it makes the trade-off explicit and regression-proof.
2. Quiet Windows → Windows\b tightening
src/classify.mjs:167 (old) …Windows|Program|…
src/classify.mjs:167 (new) …Windows\b|Program|…
Adding \b narrows the Windows branch: C:\WindowsApps\… (MS Store apps) was previously BLACK; it is now only RED. C:\Windows.old is fine — . is a non-word character so \b still fires. The change is arguably beneficial (fewer FPs against MS Store dirs) and the paths still hit RED, but it was not called out in the PR description or tested. Worth a brief comment or a // WindowsApps intentionally excluded — not a system root in the rule.
What's good
The Users-branch regex is carefully constructed: the single-segment constraint ([^\\"';|&]+ stopping at backslash), optional trailing \, optional quote, and (?:$|[;|&]) end-anchor together correctly implement the "whole profile only" contract. The semicolon-smuggle test (C:\Users\bob; echo done → BLACK) verifies the critical exploit path. The RED_SHELL companion uses [^;\n] delimiters (not [^|]) so it doesn't need to cross pipeline stages to catch absolute-path removals. CI 200/200, coverage of all explicitly claimed cases is solid.
fix(classify): Remove-Item Users overmatch — deep per-user paths are not a system root
The Windows recursive-force-delete black rule matched
Users\b, whichmade EVERY path under any user profile a "drive/system root" — a deep
project dir (...\Desktop\proj_worktree) black-blocked the same as C:.
That contradicts the rule's own comment (drive-root / system targets
only) and the documented
Remove-Item node_modules stays cleanstance.The Users branch now blacks only C:\Users itself or a WHOLE profile
(C:\Users<name> - one segment, then end-of-target, separator-smuggle
still caught). Windows/Program branches unchanged.
Companion RED rule: any Remove-Item -Recurse -Force of a drive-anchored
absolute path is now red (approve) rather than invisible - relative
deletes like node_modules stay allow, preserving the no-FP guarantee.
Tests: +6 black cases (Users root, whole profile quoted/unquoted,
separator smuggle, System32) and a new red-not-black test for deep
per-user and absolute non-system paths. 200/200 pass.