fix(security): cover HOME env spellings in the rm-rf-home deny rule

[soroush5]

Problem / Motivation

The rm -rf ~ deny rule only recognized the tilde spelling, so the equivalent $HOME / ${HOME} spellings (quoted or not — double quotes still expand) sailed through the deny gate (the sensitive tier never anchored bare home for ~ either, so only the deny gate changes here). Verified: rm -rf $HOME, rm -rf ${HOME}, and rm -rf "$HOME" all return "allow" while rm -rf ~ is denied.

Why it matters

The shell expands these spellings to the exact path the rule exists to protect, so a destructive home deletion runs with no policy trip. Same severity class as the ~ case already blocked.

What changed (motivation → approach → change)

Symptom → shell-equivalent spellings bypass the home rule. Cause → the operand tail anchors at ~.* only. Change → the tail now accepts ~, $HOME, and ${HOME} with an optional double quote (single-quoted forms stay allowed since no expansion happens there). Also added a legacy pin alias so persisted rm -rf ~.* pins keep resolving. Kept to the operand tail only so this rebases cleanly with #8240.

Tests

New TestRmHomeEnvSpellingsStayDenied (red without the fix, green with it), golden/catalog parity green, neighbor rm/catalog/ReDoS suites green (393 passed).

Manual verification

N/A — unit coverage sufficient (gate-level probes for all spellings plus lookalikes).

Fixes #8387

Pattern harvest

Rule candidate: home-directory operands need shell-equivalent-spelling coverage (~, $HOME, ${HOME}) — blocking one spelling while the shell expands the others leaves the same hole, as with flag respellings in #8240.

[dwu96]

👋 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.

[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.

[dwu96]

👋 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.

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

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

The design checks out: the exception carve-out (_exception_eligible refuses any view containing $/{) means the new _DENY_EXCEPTIONS entry can never exonerate a $HOME spelling — it only preserves the existing ~ inert-search carve-out under the new pattern key, so the change fails closed. The legacy pin alias handles persisted pins. The spelling-enumeration approach matches the existing rule architecture (same shape as the #8240 flag-respelling work) and is proportionate versus building a shell-expansion normalizer.

Design-Verdict: PASS

Real bypass on the agent-untrusted boundary, closed at the right layer, with the exception path provably failing closed and persisted pins migrated.

Suggestions

• Unbraced \$HOME has no trailing variable-name boundary, so rm -rf $HOMEBREW_CACHE (a different variable) is falsely denied; the tests caught the braced lookalike (${HOMEx}) but not this unbraced analog — add (?![A-Za-z0-9_]) after \$HOME and a matching negative test.

[DESIGN-REVIEWED] e7fb431

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

Widened home-deletion deny rule still misses ${HOME%/}-family parameter expansions (advisory, non-blocking).

FINDING — src/kiro_crew/security.py:1282 — the new braced branch \$\{HOME(?:\}|:[^}]*\}) accepts only ${HOME} and :-operator expansions, so rm -rf ${HOME%/} (and ${HOME#…}, ${HOME/x/y}, ${HOME^}, ${HOME,}), which the shell resolves to the home directory unchanged, do not match local-destructive-rm-rf-home and pass is_denied — the deny view is not expanded, so nothing else catches them and the home directory is deletable. This gap is inherited (the whole $HOME family was uncovered before this PR, which only partially closes it), so it is advisory, not a regression. → Fix: extend the braced branch to accept any parameter-expansion operator, e.g. \$\{HOME(?:\}|[-:#%^,/@!*+][^}]*\}).

[OPUS-REVIEWED] e7fb431

[github-actions]

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

Premise-level review of e7fb431f20f65db1dfc781eb6b110fcb64a45d96 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.

All checks are done. The mechanisms are verified: the legacy-pin alias follows the documented upgrade-monotonicity mechanism in base (security.py:1775-1785), the sensitive tier's home_alts covers a different tier and can't be composed into catalog pattern strings, only one deny rule carries a home operand (absolute-path spellings are already caught by rm -rf /.*), and the golden fixture is consumed only via json.loads field equality. Two findings survive: the four unrelated \u2014→— re-encodings in the golden fixture (undeclared, parse-equivalent churn) and the stale "rm -rf ~.*" key left in _DENY_EXCEPTIONS after no rule carries that pattern anymore.

First-Principles-Verdict: CONCERNS

The fix is aimed at the real cause; four unrelated em-dash re-encodings ride along in the golden fixture, and a now-dead exception key stays behind.

What this change ships

Intent: stop shell-equivalent $HOME spellings from bypassing the home-deletion deny rule — a FIX.

1. rm -rf $HOME / ${HOME} (quoted or bare) is now denied — justified
2. ${HOME:?} / ${HOME:-…}-family expansions also denied — rides along; declared only in a code comment, same cause
3. Old persisted rm -rf ~.* governance pins keep resolving — justified (documented upgrade-monotonicity mechanism, security.py:1775)
4. Grep carve-out follows the widened pattern; old "rm -rf ~.*" exception key kept — stale, zero consumers
5. Four unrelated golden descriptions re-encoded \u2014 → literal em dash — undeclared, rides along
6. New deny/lookalike/legacy-pin tests — justified

Watch

• The description names only "~, $HOME, and ${HOME}"; the shipped pattern and tests also cover the ${HOME:…} expansion family. Same root cause, but the diff is wider than the declaration.
• Description honestly scopes out the sensitive tier; the audit-only SUSPICIOUS_BASH_PATTERNS list (security.py:18219, "rm -rf ~") is the one remaining sibling spelling gap, non-blocking tier.

Subtractions

• Delete the stale "rm -rf ~.*": list(_INERT_SEARCH_GLOBS) entry in _DENY_EXCEPTIONS (security.py:2380): exceptions are looked up by the evaluated rule's pattern string, and after this change zero rules carry that pattern (grepped pattern="rm -rf ~ — 1 hit, rewritten by this PR). The PR added the new key beside it instead of replacing it.
• Drop the four \u2014 → — hunks in test/fixtures/denied_commands_golden.json: the only consumers parse it with json.loads (test_denied_commands_security.py:181,352), so the re-encoding changes nothing and churns shipped fixture history.

[FIRST-PRINCIPLES-REVIEWED] e7fb431

[github-actions]

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

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

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

BLOCKING -- src/kiro_crew/security.py:1284 -- Non-colon HOME expansions bypass the deny rule
pattern="rm -rf \"?(?:~|\\$HOME|\\$\\{HOME(?:\\}|:[^}]*\\})).*",
${HOME?}, ${HOME-}, or ${HOME=} -> deny gate misses -> shell resolves HOME -> recursive home deletion proceeds.
Anchor: residual/security
Fix: Include non-colon ?, -, and = HOME parameter expansions.
[BLOCK-MERGE] e7fb431
[GPT-REVIEWED] e7fb431

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

The adjudicable block is empty (0 findings). One fenced finding, F1.

F1 — The new pattern's brace branch is \$\{HOME(?:\}|:[^}]*\}), which matches only ${HOME} and colon-forms ${HOME:...} (patch line 12 / security.py:1284). Bash's non-colon parameter expansions ${HOME?}, ${HOME-}, ${HOME=} are valid, resolve to $HOME whenever HOME is set (always, in a normal shell), and are not matched — so rm -rf ${HOME?} passes the deny gate and the shell deletes the home tree. These are ordinary, non-contrived shell spellings, not extreme or writer-impossible inputs. Recovery path for recursive home deletion: none (irreversible loss). The harm term is unbounded (irreversible user-data loss); the conditions are commonplace, so the rarity argument required for FLAG cannot be completed. UPHOLD-FENCED.

[ADJUDICATION] e7fb431f20f65db1dfc781eb6b110fcb64a45d96 total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] e7fb431f20f65db1dfc781eb6b110fcb64a45d96
[ADJUDICATION-FENCED] e7fb431f20f65db1dfc781eb6b110fcb64a45d96 fenced=1 flagged=0
UPHOLD-FENCED F1 src/kiro_crew/security.py:1284 -- Non-colon HOME expansions (${HOME?}/${HOME-}/${HOME=}) are ordinary valid shell that resolves to home and bypasses the deny rule, causing irreversible home deletion; conditions are commonplace, not extreme.
[GPT-ADJUDICATED-FENCED] e7fb431f20f65db1dfc781eb6b110fcb64a45d96

[soroush5]

On the FP CONCERNS: verified the sibling claim — home_var is the literal $HOME, so ${HOME} indeed evades home_alts/win_home_alts while the other anchors treat both as one set. Keeping this PR to the deny gate (different tier, deliberately different semantics — a drive-by there risks behavior drift), and filed the tier gap as a tracked follow-up. Also clarified the body sentence about which tier changes.

[soroush5]

The template warnings above are stale — the body has carried all required sections (plus Pattern harvest) since the update. No workflow action needed from my side beyond what is already there.

[soroush5]

Confirmed live (rm -rf ${HOME:?} sailed through) and fixed: the braced alternative now matches the :-family expansions (${HOME}, ${HOME:?}, ${HOME:-x}, ${HOME:=x}, ${HOME:+x}), while a different variable name (${HOMEx}) stays allowed — pinned by regression tests either way. Pushed, 636 green locally including the golden sync.

[soroush5]

Superseded by #8240 — I folded the HOME axis into that PR (rebased on the new denied_rules.py package layout, single-fragment patterns, same $HOME matrix plus lookalikes). Closing this one so there's a single review surface; issues #8237 and #8387 stay open until #8240 lands.