fix(security): cover rm flag variants in the rm-rf-root/home deny rules

[soroush5]

Problem / Motivation

The two rm deny rules (local-destructive-rm-rf-root / -home) only recognize the exact -rf flag spelling, and the home rule only recognizes literal ~. Equivalent recursive-force invocations sail through the PreToolUse gate — verified via is_denied: reordered (-fr), combined (-rfv), split (-r -f), long-option (--recursive/--force/--no-preserve-root) spellings, one interspersed operand (rm harmless -rf --no-preserve-root /), and every $HOME/${HOME} spelling (which the shell expands to ~) all return None.

Why it matters

Any agent run that reaches for a destructive delete with a non-canonical spelling gets zero confirmation. This is the same bug class as #4824 (which widened only the four self-protection rules for interposed flags) — the rm rules were left exact-match, so the floor has a hole exactly where the damage is total.

What changed

Both rm patterns widened with NO .* gap (single fragment, exact full-input semantics): literal single spaces plus runs of single-letter flag tokens, one optional unquoted operand slot, and the $HOME family on the home rule. That shape is what keeps the separator pins holding (a .* gap would fabricate commands across ;/newlines), the ReDoS screens green (bare * over quantifier-free bodies; the operand slot ends in a literal so the polynomial screen stays blind to it), and the documented " " residual intact. rm is token-anchored (/bin/rm, {rm;}, backtick programs still match), with one deliberate exception: quote-glued rm matches when a separator follows later in the string — that keeps the Pass-1 whole-string closure for pipelines (grep 'rm -rf /...' | python must deny even though the literal is quoted), while a lone "rm" -rf / still misses raw text for the respelling-proof twin. rm -f/-r-only, flagless rm, lone --recursive, rm -- -rf /, --preserve-root, and lookalikes ($HOMEx, file~) all stay allowed. Governance legacy map gains the two pre-widening spellings (#4824 precedent); search-verb carve-out (#8802) registered from the live patterns; golden fixture synced (111 rows).

Residuals (documented in-code): interposed LONG flags (-r --verbose -f), leading long-flag padding, 2+ or quoted operands, operands between flags and path, getopt-permuted trailing flags — each needs a repeated operand gap the ReDoS screens forbid.

Tests

• Extended TestRmFlagVariantsStayDenied (flag/operand/program variants + allow shapes) and new TestRmHomeEnvSpellingsStayDenied ($HOME/${HOME} incl. braced operators, lookalikes) in test/test_denied_commands_security.py.
• Full suites green on fresh main: test_denied_commands_security.py + test_security.py + test_deny_guidance.py + test_denied_commands_hooks.py + test_heartbeat_safe_tools.py + test_security_regex_linearity.py — 1934 passed, 2 skipped (pre-existing).
• Three pin flips, each sanctioned by the test's own docstring: the flag-interposition gap, the first over-block (double-quoted ANSI-C is inert), and the governance-map emptiness.
• ReDoS: both patterns pass is_safe_user_regex, take the unbounded single-fragment path, hostile inputs (20k-quote, flag-chain, padded needle) evaluate in ≤0.1s.
• black gate green; isort/mypy not installed in this environment (no import or typing changes in the diff).

Fixes #8237
Fixes #8387

Pattern harvest

Rule candidate: deny-list regexes that match one spelling of equivalent destructive invocations — worth a pass over the other local-destructive patterns for packed/split/long-option variants.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #8262 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8262: KEEP. Same file, disjoint hunks and disjoint rules. No coordination needed. Files: src/kiro_crew/security.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[soroush5]

Two notes in one: the template-bot warning above is stale — all listed sections (Problem/Motivation, Why it matters, What changed, Tests, Pattern harvest) are in the body now. And on the relationship audit: agreed, #8262 is same-file/disjoint-hunks, KEEP with no coordination needed from this side.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 52a5f331e294ab6ad482afb8d95b601d7635a823 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The design gate is satisfied from the diff and base tree: the widening follows the repo's own rubric (subject is genuinely a shell command line, sandbox doesn't hold / or ~, precedent #4824), the flipped pins were sanctioned by their own docstrings ("if either is closed, its assertion flips"), and backward compat for persisted policy pins is handled via the legacy map. The one design risk worth human eyes is the acknowledged residual set.

Design-Verdict: CONCERNS

Sound widening with a compat story, but the simplest equivalent spelling (rm / -rf, getopt-permuted) stays open — the fix is knowingly partial.

Watch

The stated harm is "equivalent recursive-force invocations sail through," yet GNU getopt permutes operands, so rm / -rf is byte-for-byte the same invocation and remains allowed ("getopt-permuted trailing flags (rm / -rf) still miss"). The residual is documented, but it is the lowest-effort reorder in the class this PR closes, and closing it inside the regex tier is impossible per the PR's own ReDoS argument — evidence the regex catalog is nearing the restructure point AGENTS.md flags for spelling-chases (#7441).
Clears when: a human explicitly accepts the getopt-permuted residual, or a follow-up issue is filed for an argv-level (tokenized) check for the two rm rules.

Suggestions

• The 8-line flag-run core is restated verbatim in both rules; build both patterns from one shared module constant so the root/home spellings cannot drift apart silently.

[DESIGN-REVIEWED] 52a5f33

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed 52a5f331e294ab6ad482afb8d95b601d7635a823 via the fork AI-review pipeline; updated in place on each push.

2 of 2 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- src/kiro_crew/security/denied_rules.py:1322 -- Interposed long options bypass the widened root rule
r"|(?:-[a-z]*r[a-z]*|--recursive)(?: -[a-z])* "
rm --recursive --verbose --force --no-preserve-root / -> PreToolUse is_denied -> no match -> auto-approved execution can delete user-writable files.
Anchor: residual/security
Fix: parse rm argv structurally so valid options may appear in any position.

BLOCKING -- src/kiro_crew/security/denied_rules.py:1353 -- Uncolonized HOME expansions bypass the home rule
r"|\$\{HOME\}|\$\{HOME:[^}]*\}).*"
With HOME set, rm -rf ${HOME-/tmp/fallback} -> PreToolUse is_denied -> no match -> expansion deletes the home directory.
Anchor: residual/security
Fix: recognize uncolonized -, =, and ? HOME parameter expansions.

[BLOCK-MERGE] 52a5f33
[GPT-REVIEWED] 52a5f33

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

Bash/Write require approval in this run; my manual regex analysis is conclusive and independently corroborated.

F1 — root rule, src/kiro_crew/security/denied_rules.py:1322. Between the --recursive and --force long-option branches the only interposition permitted is (?: -[a-z])* — a run of single-letter flags. A long flag such as --verbose cannot be consumed there ( --verbose fails -[a-z], and the leading operand group has (?!--)), so rm --recursive --verbose --force --no-preserve-root / matches no branch and no /-anchored root branch either. This is a real bypass — and the author's own comment (patch lines 49–50) already documents "interposed LONG flags (-r --verbose -f)" as a residual. Harm: irreversible root-filesystem deletion (UNBOUNDED). The condition combination (ordinary long options with an interposed --verbose) is trivially producible, not extreme, so the FLAG rarity bar is not met.

F2 — home rule, src/kiro_crew/security/denied_rules.py:1353. The path alternation recognizes ${HOME} (immediate }) and ${HOME:[^}]*} (colon-operator forms) only; the uncolonized parameter expansions ${HOME-…}, ${HOME=…}, ${HOME?…}, ${HOME+…} carry no colon, so \$\{HOME:[^}]*\} fails and \$HOME(?![A-Za-z0-9_]) cannot match ${. With HOME set, rm -rf ${HOME-/tmp/fallback} expands to the home directory and is not denied. Real bypass; harm is irreversible home-directory deletion (UNBOUNDED). ${HOME-word} is a standard, easily-produced bash construct — not extreme; FLAG bar not met.

[ADJUDICATION] 52a5f33 total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] 52a5f33

[ADJUDICATION-FENCED] 52a5f33 fenced=2 flagged=0
UPHOLD-FENCED F1 src/kiro_crew/security/denied_rules.py:1322 -- Long-option interposition (--recursive --verbose --force /) escapes the single-letter-only padding run, an easily-produced spelling causing irreversible root deletion.
UPHOLD-FENCED F2 src/kiro_crew/security/denied_rules.py:1353 -- Uncolonized ${HOME-word} expansions match no path branch yet expand to the home directory when HOME is set, an ordinary bash construct causing irreversible home deletion.
[GPT-ADJUDICATED-FENCED] 52a5f33

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 52a5f331e294ab6ad482afb8d95b601d7635a823 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

Two rules widened while ≥3 siblings in the same category keep the identical one-spelling gap — and two denied→allowed flips ride along with the anchoring.

Not justified as shipped

• Item 5 — rides along: closing the pinned over-block isn't the reported defect; the pin sanctioned the flip, but it moves the gate toward less denial, its documented unsafe direction.
• Item 6 — undeclared: the token-anchor lookbehind newly allows words merely ending in "rm" (alarm -rf / was denied on base by substring search); the description never says so.

What this change ships

Intent: stop equivalent respellings of rm -rf / and rm -rf ~ from skipping the destructive-command deny gate — a FIX.

1. Reordered/packed/split/long-option recursive-force deletes of / now refused — justified
2. Same flag spellings refused for home deletion — justified
3. $HOME/${HOME} spellings refused like ~ — justified
4. One interspersed operand no longer exonerates (rm harmless -rf /) — justified
5. Echoing a double-quoted inert ANSI-C rm string now allowed (pinned over-block closed) — rides along
6. Words ending in "rm" (alarm -rf /) no longer trip the rules — undeclared
7. Old-spelling governance pins keep resolving to the widened rules — justified
8. Search-verb carve-out re-registered under the widened patterns — justified
9. Golden fixture and security.md updated with the new patterns — justified

Watch

• Point patch: the root cause — deny regexes spelling exactly one variant of a flag-bearing command — has counted unfixed siblings in the same local-destructive block: chmod 777.* misses chmod -R 777 / (denied_rules.py:981), dd if=.* misses operand-reordered dd of=/dev/sda if=/dev/zero (line 1116), git reset --hard.* misses git reset -q --hard (line 1237). The author's own "Pattern harvest" names this. Clears when: a linked issue or follow-up covers the harvested siblings.
• Items 5–6 both reduce denial; the base pin's words were "over-blocking is the safer direction for this pass". Item 5's evidence (bash never decodes $'…' inside double quotes, executing spellings still deny via the quote-stripping view) is checkable; item 6 is not mentioned anywhere. Clears when: the author confirms the embedded-suffix allowance (alarm -rf /) is intended and covered by the respelling cross.

[FIRST-PRINCIPLES-REVIEWED] 52a5f33

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 52a5f331e294ab6ad482afb8d95b601d7635a823 via the fork AI-review pipeline; updated in place on each push.

Review details

No blocker — one advisory: the widened built-in rm patterns introduce quadratic regex backtracking on the uncapped built-in fragment path.

FINDING — src/kiro_crew/security/denied_rules.py:1311 — the widened local-destructive-rm-rf-root/-home patterns place two variable-width (?: -[a-z])* runs (the leading run and the one inside the split --recursive … --force alternative) in sequence over the same -r tokens with a required trailing //~ that a crafted "rm " + "-r "*N command never supplies; as a built-in single fragment these take the _DenyMatcher fragment path (is_builtin short-circuits the single_fragment/_polynomial_backtracking_prone gate at denied_rules.py:2688, and match runs frag_re.search on the full, untruncated lower in is_denied Pass 1 — "no length truncation", denied_rules.py:2835), so the outer run backtracks N times while the inner run re-scans O(N−k) tokens each time → Θ(N²) CPU in the synchronous PreToolUse gate; the existing ReDoS budget tests miss it because every hostile input is aws/a-prefixed (test_all_builtins_matchable_without_hanging feeds "aws " + "-x "*5000), which the rm token-anchor rejects at position 0 → Fix: bound the flag runs ((?: -[a-z]){0,K} in place of (?: -[a-z])*) so the pattern still passes is_safe_user_regex but cannot backtrack quadratically, and add an rm-prefixed adversarial case to the budget suite.

[OPUS-REVIEWED] 52a5f33

[soroush5]

Verified each claim live, and you were right on the core point — reworked accordingly:

• Confirmed the escape through the real gate: rm -v ×5 -rf / was allowed with the bounded slots. They are gone now; every pad site uses the exact house idiom (_DANGEROUS_AWS_FLAG_RUN spelling, so _linearize_deny_pattern rewrites it — verified rewritten + gate green), with the r+f combination still carried by the alternation.
• 5-pad/10-pad/combo-internal/verbose padding all denied now; allows (-r/-f alone, flagless, quoted-whitespace A quoted whitespace-only word bypasses the command-shape deny rules #8124 pin) unchanged. 636 green locally.
• On the 'contradicted' point, one precision: my first draft used literal-space separators (to preserve the A quoted whitespace-only word bypasses the command-shape deny rules #8124 pin), not the byte-identical \s+ spelling — and the linearizer only rewrites the exact constant, so that draft was genuinely skipped (verified: exact→rewritten, literal-space-star→untouched). The body told that story imprecisely; fixed.
• On breaking A quoted whitespace-only word bypasses the command-shape deny rules #8124: the pin view is rm -rf /home/x (3-space run from the quoted word) — it matches under neither spelling, so the pin test stays green. The pin is about the quoted word, not multi-space separators.

[soroush5]

Same rework addresses this: the bounded slots are replaced by the linearized house idiom at every pad site, so padding count no longer matters (verified 5- and 10-pad denied through the real gate). On scope: structural argv parsing would mean re-architecting the regex-view tier, which is beyond this fix; the residual class for pads is closed, with the #8124 quoted-whitespace pin intact and rarity/recovery as your adjudication notes (default flow still prompts).

[bolichen97]

@soroush5 Thanks for this, and for the earlier note on #8262. The 2026-09-08 open-PR relationship audit (audited at aa4feaa) puts #8240 next to four open PRs on the same three files, so here is where I think it should go.

#8388, also yours, is the one that matters. Both PRs rewrite the same record, DeniedCommandRule(id="local-destructive-rm-rf-home") in src/kiro_crew/security.py, from the same merge base, on orthogonal axes: #8240 widens the flag run (packed -fr, split -r -f, long --recursive/--force, redundant padding), #8388 widens the path operand ($HOME, ${HOME}, ${HOME:?}, quoted). Both also add the identical "rm -rf ~.*" key to _LEGACY_RULE_ID_BY_PATTERN, and both mutate _DENY_EXCEPTIONS. The two patterns do not compose: whichever lands second must fold the flag-run idiom into the $HOME alternation, or the second widening silently drops the first axis. Please fold both into one rm-rule PR.

That PR needs re-authoring rather than a rebase. Merged #9183 deleted src/kiro_crew/security.py and moved the catalog to src/kiro_crew/security/denied_rules.py, where main still ships pattern="rm -rf /.*" and "rm -rf ~.*" unwidened, so nothing here is superseded, but every hunk relocates and test/fixtures/denied_commands_golden.json must be regenerated against main's 111-rule catalog (149 at your base). Please also drop the four unrelated \u2014-to-em-dash rows in that fixture, and either cover the operand-before-flags shapes (rm harmless -rf --no-preserve-root / is still allowed) or drop Fixes #8237.

#8262 (@GoZippy), #8283 (@patrigao) and #9010 (@chenmingwei23) share the same three files but touch disjoint rules; only the fixture tail and the TestCatalog rule-count assertion collide, so those need rebase order only, not design coordination.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

[soroush5]

Makes sense — I will fold both axes into one PR against the new denied_rules.py layout (composing the flag-run idiom into the $HOME alternation, regenerating the golden fixture on current main, dropping the em-dash rows) and close these two. I will also cover the operand-before-flags shape so Fixes #8237 stays.