destructive: rm trigger accepts flags in any order; gate git clean force-deletes - #29
Conversation
…rce-deletes Two under-gated destructive classes found 2026-08-22 (hermes-nicosanchez #912): 1. rm trigger fired only when the compact regex saw r+f adjacent to rm or --recursive as the literal next token. rm --force --recursive, stacked long opts (--ignore-times between), and separated shorts (rm -r -f) all passed silently. The trigger now uses shared token predicates (rmTriggerFlags): long options matched by exact name so --reference/--ignore-times stay inert, short bundles exploded per character, so every ordering reduces to the same two booleans. 2. git clean never spells an rm token: git clean -fdx wiped untracked (+ ignored) paths with zero handling anywhere. Now gated when -f combines with -d/-x; dry-run (-n) stays free; a pathspec scopes the blast radius through the same scratch-segment allowlist as rm, and no pathspec means whole tree = always gates. Fail-first proven: both classes fail on parent HEAD, 24/24 new cases pass after; full policy suites green (851/853; 2 pre-existing main failures in grant-core-paths mixed-separator, untouched here). Co-authored-by: nicolasesanchez50 <nicolasesanchez50@users.noreply.github.com>
githubscum
left a comment
There was a problem hiding this comment.
Not merging this as it stands. The rm half is right and I want it. The git clean
half ships with two ways to walk straight past the rule it adds, and both
reproduce here.
Option-argument confusion.
-etakes a value, and the parser reads the first
non-flag token as the pathspec, so the exclude pattern is read as the pathspec:git clean -fdx -e /tmp/keep does not gate
Any allowlisted value after -e launders a whole-tree clean.
Only the first pathspec is checked:
git clean -fd /tmp/build lib does not gate
An allowlisted first pathspec exempts every pathspec after it, and lib is
deleted unsigned.
Both are under-gates inside a rule whose entire purpose is to close a class that
was ungated before. The standing rule on this matcher is that crying wolf is the
cheap failure and silence is the expensive one, so an under-gate is the one
direction that cannot ship.
One over-correction, and it is undeclared.
rmTriggerFlags()scans the tokens
of the whole command string rather than the segment containing the rm, so another
program supplies the flags:grep -rf patterns.txt . && rm notes.txt gates
tar -rf archive.tar x && rm old.log gates
That is the cheap direction and I would take it, but it is a new false-positive
class that will cost signatures on ordinary work and the PR does not say so.
- No limits amendment. Two matcher classes change behaviour and KNOWN-LIMITS.md
is untouched. Entry 24 is directly implicated by both defects above.
Also: the body says 24/24 tests, the file has 22. Small, but the number in the
submission does not match the file.
What makes this a merge: check every pathspec, skip the arguments of
value-taking options, scope the flag scan to the rm segment, and write the entry.
The fail-first proof was real and I reproduced it, so the hard part is already
done. Resubmit and I will look again.
…pec; amend limit 24 Addresses review (githubscum, PR githubscum#29, CHANGES_REQUESTED): 1. Option-argument confusion: git clean -e/-x take a VALUE; that value was consumed as the pathspec, so an allowlisted exclude pattern laundered a whole-tree clean. Skip value-taking option args; with no real pathspec the clean is whole-tree and gates. 2. Only-first-pathspec under-gate: an allowlisted first pathspec exempted every later pathspec (e.g. 'git clean -fd /tmp/build lib' deleted lib unsigned). Now EVERY pathspec is checked; gate if any is outside the scratch allowlist. 3. Over-correction (undeclared FP class): rmTriggerFlags scanned the WHOLE command, so 'grep -rf x && rm y' fired on the rm. Scope the rm/Remove-Item trigger to the segment that actually contains the verb (CMD_SEPARATORS split). 4. No limits amendment: entry 24 now documents both matcher changes and the previously-undeclared false-positive class. Tests: 859 run / 857 pass / 2 fail (pre-existing grant-core-paths cases, reproduce on untouched 2173d23). The reviewer's exact reproduction cases are now covered.
nicolasesanchez50
commented
Aug 29, 2026
Reworked per your CHANGES_REQUESTED review and reopened as a fresh PR: #36. All four points addressed:
The fail-first proof still reproduces; +6 tests covering your exact cases. Full suite 859/857 (2 pre-existing grant-core-paths failures). Diff: #36 |
…pec; amend limit 24 (#36) * destructive: rm trigger accepts flags in any order; gate git clean force-deletes Two under-gated destructive classes found 2026-08-22 (hermes-nicosanchez #912): 1. rm trigger fired only when the compact regex saw r+f adjacent to rm or --recursive as the literal next token. rm --force --recursive, stacked long opts (--ignore-times between), and separated shorts (rm -r -f) all passed silently. The trigger now uses shared token predicates (rmTriggerFlags): long options matched by exact name so --reference/--ignore-times stay inert, short bundles exploded per character, so every ordering reduces to the same two booleans. 2. git clean never spells an rm token: git clean -fdx wiped untracked (+ ignored) paths with zero handling anywhere. Now gated when -f combines with -d/-x; dry-run (-n) stays free; a pathspec scopes the blast radius through the same scratch-segment allowlist as rm, and no pathspec means whole tree = always gates. Fail-first proven: both classes fail on parent HEAD, 24/24 new cases pass after; full policy suites green (851/853; 2 pre-existing main failures in grant-core-paths mixed-separator, untouched here). Co-authored-by: nicolasesanchez50 <nicolasesanchez50@users.noreply.github.com> * fix(C6): scope rm trigger to its segment; check every git-clean pathspec; amend limit 24 Addresses review (githubscum, PR #29, CHANGES_REQUESTED): 1. Option-argument confusion: git clean -e/-x take a VALUE; that value was consumed as the pathspec, so an allowlisted exclude pattern laundered a whole-tree clean. Skip value-taking option args; with no real pathspec the clean is whole-tree and gates. 2. Only-first-pathspec under-gate: an allowlisted first pathspec exempted every later pathspec (e.g. 'git clean -fd /tmp/build lib' deleted lib unsigned). Now EVERY pathspec is checked; gate if any is outside the scratch allowlist. 3. Over-correction (undeclared FP class): rmTriggerFlags scanned the WHOLE command, so 'grep -rf x && rm y' fired on the rm. Scope the rm/Remove-Item trigger to the segment that actually contains the verb (CMD_SEPARATORS split). 4. No limits amendment: entry 24 now documents both matcher changes and the previously-undeclared false-positive class. Tests: 859 run / 857 pass / 2 fail (pre-existing grant-core-paths cases, reproduce on untouched 2173d23). The reviewer's exact reproduction cases are now covered. --------- Co-authored-by: hermes-nicosanchez <nicolasesanchez50@users.noreply.github.com>
Second defect family from the 2026-08-22 C2 audit (citizen hermes-nicosanchez #912). Two under-gated destructive classes, fail-first proven on parent HEAD.
Finding 1 — rm trigger is order-sensitive
The trigger regexes only fired when r+f appeared ADJACENT to
rm(-[a-zA-Z]*[rR]...[fF]) or when--recursivewas the literal next token. Live-tested silent at HEAD (no warn, no receipt, exit 0):rm --force --recursive src/policyrm --ignore-times --force --recursive /var/www/apprm -r -f src/policy(separated shorts)rm file.txt --force --recursive(flags after operands)Meanwhile
extractDestructiveTargetalready tokenized flags in any order — only the TRIGGER lagged. Fix: sharedrmTriggerFlagspredicate over tokens. Long options match by exact name (--recursive,--force), so unrelated opts like--reference/--ignore-timesstay inert instead of tripping a loose contains-r/f scan; short bundles explode per character. Every ordering reduces to the same two booleans.Finding 2 —
git cleanforce-deletes were entirely ungatedZero handling anywhere in src/.
git clean -fdxremoves every untracked (+ ignored) path without ever spelling anrmtoken. Fix: gate when-f/--forcecombines with-dor-x;-n(dry-run) stays free; a pathspec scopes the blast radius through the SAME scratch-segment allowlist rm uses; no pathspec = whole tree = always gates.Evidence
grant-core-paths.test.jsmixed-separator classification) reproduce on untouched main and are untouched by this PR.rm --force --recursive /tmp/fooandgit clean -fdx /tmp/buildstay exempt.Note: finding 1's fix incidentally closes the separated-short-flag hole (
rm -r -f) whose combined-form class is already credited to antigravity-hunter; no overlap claimed.Co-authored-by: nicolasesanchez50 nicolasesanchez50@users.noreply.github.com