fix(C6): scope rm trigger to its segment; check every git-clean pathspec; amend limit 24 - #36
Merged
githubscum merged 2 commits intoAug 29, 2026
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>
…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.
githubscum added a commit
that referenced
this pull request
Sep 1, 2026
…ng saw it dc1910b (PR #36) appended an amendment to entry 24 and, in the same hunk, deleted the "## 25." heading line. Entry 25's body (gh as the authenticated vendor CLI the rules could not see) has been orphaned inside entry 24 since 2026-08-29. main today carries 60 entries numbered 1..61: a citation of KNOWN-LIMITS 25 resolves to nothing, and a reader of entry 24 gets a section that changes subject mid-way. This is the 2026-08-22 incident the pin exists to prevent, one level worse: then a number meant something else, now it means nothing. It survived code review and 891 green tests, because no test had ever read the shipped log as a structure. - Restore the "## 25." heading. A faithful revert of the deleted line; the body is not moved. After: 61 entries, contiguous 1..61, no duplicates. - Add test/known-limits-numbering.test.js: read-only over the committed log, asserting contiguity from 1, uniqueness, ascending order, and a parse floor. It writes nothing and needs no pin, unlike the two existing cases that advertise the real log and write the state they then assert. - Amend entry 29 with the finding. Fail-first: RED on main at a2ac5e2 (60 entries, highest 61, missing 25 - the assertion names it). GREEN here. Full suite 895/895. NOT done, unchanged from this branch's first commit: the stale pin is not stamped. This run verified the log's structure, not the truth of 61 entries against dc1910b. Stamping on a numbering check would be a smaller lie and still a lie. The stamp is owed by whoever verifies. Co-Authored-By: Claude Opus 5 (1M context) <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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Rework of PR #29 — addresses review (githubscum, CHANGES_REQUESTED)
The rm half was wanted; the git-clean half shipped two under-gates. Fixed all four points, and the fail-first proof still holds.
1. Option-argument confusion (
-evalue read as pathspec)git clean -e <pattern>(and-x) take a VALUE. The old parser read the first non-flag token as the pathspec, so an allowlisted exclude pattern was consumed as the pathspec and laundered a whole-tree clean. Fixed: value-taking option args are skipped; with no real pathspec the clean is whole-tree and gates. Nowgit clean -fdx -e /tmp/keepgates (correct — it is a whole-tree clean).2. Only the first pathspec was checked
git clean -fd /tmp/build libdeletedlibunsigned because only the first pathspec was tested. Fixed: every pathspec is checked; the clean gates if ANY is outside the scratch allowlist.git clean -fdx /tmp/build /tmp/keep(all allowlisted) still stays exempt.3. Over-correction (undeclared false-positive class)
rmTriggerFlagsscanned the WHOLE command, sogrep -rf x && rm yandtar -rf archive.tar x && rm old.logfired on the rm. That is the cheap direction (a new FP class that costs signatures) and the PR did not say so. Fixed: the rm/Remove-Item trigger now scans only the segment that actually contains the verb (split on&&/||/;/|/newline).grep -rf x && rm notes.txtno longer gates;cd /srv && rm -rf /srv/appstill does.4. No limits amendment
Entry 24 now documents both matcher changes (rm flag-order/long-form trigger; git-clean
-f+-d/-xgating with the pathspec semantics) and the previously-undeclared false-positive class. Both defects above are named explicitly in the record, not just the code.Tests
node --test test/policy-destructive-flag-order-git-clean.test.js→ 28/28 pass (was 22; +6 covering the reviewer's exact cases:-evalue, multi-pathspec, and thegrep -rf && rm/tar -rf && rmnegatives).grant-core-pathsmixed-separator cases, reproduced identically on untouched2173d23— untouched by this PR.Residuals (declared)
rm -rfin ANY segment (not just the led one) now gates. That is intended — a later-segmentrm -rfincat a && rm -rf bshould gate regardless of the leading read verb (consistent with limit 21's segment rule).-e/-xvalue skip assumes standard git-clean option syntax; a pathologicalgit clean -fdx -- -e(pathspec beginning with-e) is not separately handled, same as the existing pathspec-quoting gap. Not reachable from normal usage.~ hermes-nicosanchez (citizen #912)