fix(security): follow a custom KIROCREW_HOME on the bash gate too

[leonlaiyc]

Problem / Motivation

The two gates that fence a write-protected file under the data home disagree
about a non-default KIROCREW_HOME:

• The tool gate — security.is_sensitive_write_path() — resolves through
  config_dir(), so it follows the override.
• The bash gate — security.is_sensitive_bash_command() — is a string
  matcher over the home SPELLINGS a command can carry (~, $HOME,
  /home/<user>, %USERPROFILE%) followed by a crew prefix (.kiro/crew,
  .kirocrew). A resolved override path carries neither.

So on such an install a write-protected file is fenced against the agent's file
tools and reachable by a bash redirect naming its real path. Protected on one
path only is not protected.

Reproduction from #4082, run against main and against this branch:

resolved:  <KIROCREW_HOME>/apps/ops-mission-control/data/rotation.yaml

                                             main     this branch
is_sensitive_write_path      (tool gate)     True     True
is_sensitive_bash_command    (shell gate)    None     Blocked
  ...POSIX spelling of same                  None     Blocked
  ...raw-text / embedded-script .env spelling    None     Blocked
  ...the ~/.kiro/crew/... spelling           Blocked  Blocked
  ...an ordinary file under the home         None     None

Why it matters

rotation.yaml is the worked example because it is an INPUT TO AN
AUTHORIZATION DECISION — an agent that names its own login there makes
rotation.authorize_action accept a forged shift — but the gap is generic to
every entry on the write-protected bash floor and to raw-text, embedded-script,
and Windows-native spellings of crew-prefixed _SENSITIVE_HOME_DIRS entries.
Ordinary path-like custom-home .env forms may already be caught by the base
is_sensitive_path() normalizer; this PR does not claim that all such forms were
previously unprotected.

A custom KIROCREW_HOME is a normal single-instance install, not an exotic
configuration — kirocrew pod and dev-backend.sh both use one routinely.

What changed (motivation → approach → change)

Motivation — make the shell gate fence the same files the tool gate already
fences, without widening what either one blocks on a default install.

Approach — reuse the tool gate's own resolution rather than inventing a
second one. _resolved_root_key is what _home_dir_targets already keys its
target set on, and _home_dir_targets_uncached already re-anchors every
crew-prefixed entry under the resolved home for exactly this reason. Deriving
the shell branch from those same two inputs is what keeps the gates from
drifting apart again.

Change — _build_sensitive_regex gains one more alternative, anchored on
the resolved crew home and carrying the same remainders the tool gate
re-anchors there.

The remainders are derived by stripping whichever crew prefix an entry carries,
so a leaf added to _SENSITIVE_HOME_DIRS or _WRITE_PROTECTED_BASH_LEAVES is
covered on both paths without a second edit.

_resolved_root_key only reads and resolves the env var: none of
config_dir()'s start-of-process maintenance (mkdir, legacy migration,
breadcrumb refresh, archive sweep) runs on this gate path. It answers None for
a default home, where the branch is omitted entirely and the compiled pattern is
byte-identical to before.

Both separators are accepted in the anchor: the resolved literal is
all-backslash on Windows while git-bash and msys tooling render the same root
with /.

The root segments are joined with the generalized separator the fenced-dir
branches above already use (canonical \. and \X\.. no-op chains), plus
repeated-separator tolerance. A root is as spellable as the remainder —
/x/./home, /x//home and /x/zz/../home all name the same directory, and
the tool gate fences every one because it resolves — so an exact-literal root
would fence none of them.

The anchor tolerates shell backslash escapes

The generated path fragments accept an optional backslash before every
character, not only before whitespace. An unquoted POSIX shell removes a
backslash escape before the path is resolved, whatever character follows it.
Measured, not assumed:

unquoted   cr\ewhome   -> crewhome      the escape collapses
double-q  "cr\ewhome"  -> cr\ewhome     the backslash survives
single-q  'cr\ewhome'  -> cr\ewhome     the backslash survives

So a command can spell a protected path with a backslash anywhere and still
reach that exact file. Tolerating it only before whitespace left every other
position open — an authorization bypass, since the tool gate resolves the path
and refuses it while the shell gate did not. Whitespace additionally folds
space and tab into one class: a path containing a space cannot appear bare in a
command at all, and C:\Users\First Last is an ordinary profile directory.

This over-matches, deliberately and in the fail-safe direction. The quoted
spellings above keep the backslash and therefore name a different path, but
this gate matches raw command text and cannot see quoting, so it fences them
too. Refusing a path that would not have resolved here is safe; missing one
that would is the bypass being closed. Segments never contain a separator (the
caller splits on [\\/] first), so no separator ambiguity is introduced.

Widening a security pattern was measured rather than argued — branch vs main,
same process shape, KIROCREW_HOME set:

	main	branch
compiled pattern length	15422	19135 (+24%)
repo's catastrophic-backtracking guard input	3.47s	3.72s (+7%)
200 consecutive backslashes (adversarial)	0.015s	0.015s
500 non-matching commands	1.23s	1.11s

\\?X is near-deterministic because \ and X are disjoint for every
character in these paths, so the optional group does not create the ambiguity
that drives backtracking.

The pattern cache had to be re-keyed

The compiled pattern is cached in _SENSITIVE_RE and now embeds a resolved
path. A stale pattern built for a PREVIOUS home fails open on the current
one — this same bug, reintroduced through the cache. _get_sensitive_re now
re-keys on KIROCREW_HOME.

Keyed on the RAW env var rather than the resolved root because this is read on
every gate call and resolving would put a Path.resolve() on that hot path. The
residual — a symlink UNDER the override repointed mid-process — can only leave
the resolved-literal branch naming the old location; it removes no existing
branch, so the home-spelling and bare-leaf strategies are unaffected and the
gate cannot fall below what it matched before.

Tests

New: test/test_security.py::TestBashGateFollowsACustomKirocrewHome (11
tests). Each one pins parity rather than the branch: the assertion states
what the tool gate already answers and requires the shell gate to answer the
same, so the class keeps its meaning if the implementation is rewritten.

Test	Behavior locked in
test_the_two_gates_agree_on_a_write_protected_leaf	The parity statement itself, with the tool-gate answer asserted first as a precondition so the class cannot silently stop testing anything
test_every_write_form_is_refused_not_just_a_redirect	Verb independence across redirect / cp / tee / python -c open / sed -i / mv — a narrow verb allowlist is bypassable
test_a_crew_secret_leaf_under_the_override_is_blocked_too	Raw-text / embedded-script .env spellings follow the override too; ordinary path-like forms may already be covered by base normalization
test_either_separator_spells_the_same_root	The resolved literal is all-backslash on Windows; the / spelling a shell actually carries names the same root
test_an_ordinary_file_under_the_override_stays_writable	The branch fences named leaves, not the whole data home — the false-positive floor
test_the_home_spellings_are_unchanged	~ / $HOME / /home/<user> / /Users/<user> still match, asserted with an override set (the configuration whose pattern is rebuilt)
test_changing_the_override_re_keys_the_compiled_pattern	Home A then home B in one process; a pattern still built for A fails open on B
test_a_home_with_a_space_is_fenced_in_every_shell_spelling	All four shell spellings of a home containing a space (bare / double-quoted / single-quoted / backslash-escaped) — a literal anchor fences three and leaves the fourth open
test_a_canonical_no_op_spelling_of_the_root_still_anchors	Five spellings of the same root (plain, /./, //, /x/../, backslash) — plus two negative controls: a sibling root sharing a prefix is not fenced, and an ordinary file under the real root stays writable
test_both_spellings_of_a_relocated_root_are_fenced	A symlinked/junctioned override names one directory under two paths — the lexical one the operator configured and the resolved real one — and both must be fenced; divergence is injected at the resolver seam, so the case needs no elevation and does not skip on Windows
test_dropping_the_override_returns_to_the_default_home	Unsetting returns the tmp dir to being an ordinary directory, and the default-home spellings stay gated

| test_an_escaped_ordinary_character_does_not_bypass_the_fence | A shell backslash before an ordinary character — pinned at four positions (mid-root, first character of a root segment, mid-remainder, mid-intermediate-dir), each via both a redirect and an embedded python -c, with the tool gate asserted first as the reference answer; plus two controls so the widening cannot fence more than the root |

Fail-before / pass-after. Run against pristine origin/main the class is
9 failed / 2 passed — and the 2 that pass are exactly the controls that must
not change (test_an_ordinary_file_under_the_override_stays_writable,
test_the_home_spellings_are_unchanged). Against this branch, 12 passed.

The escaped-character regression was added in response to a blocking review on
cf6fc0d9 and verified the same way, with only security.py reverted: 1
failed / 11 passed before, 12 passed after. The other 11 pass on both
sides, so that change is additive rather than a rewrite of this branch.

Updated: ops_mission_control/tests/test_security.py::test_the_shell_path_is_closed_too.
This is the test that surfaced the bug. It recorded the asymmetry in its
docstring rather than pinning it, because handing it the resolved path asserted
nothing while the suite read the operator's real home; pinning KIROCREW_HOME
per test in #4065 made that visible. It now asserts the resolved form alongside
the home spellings — 4 failed on main, 43 passed here.

Regression suites, run with -p no:randomly:

• test_security.py, test_governance_self_protection.py,
  test_connections_tool_aliases.py, test_browser_cli_launch.py,
  test_mcp_cron_security.py, ops_mission_control/tests/test_security.py
  — 1033 passed, 18 skipped, plus one failure that is not from this diff:
  TestHomeDirTargetsCache::test_second_call_does_not_rebuild (assert 8 == 1) reproduces identically on pristine main under the same load and
  passes when its class runs alone — a wall-clock-vs-TTL flake in that test
• test_hooks.py, test_config_loader.py, test_computer_use_api.py,
  test_computer_use_enable_state.py, test_spawn_audit.py
  — 575 passed, 7 skipped, plus one Windows-local symlink failure
  (TestSafeReadFile::test_allows_benign_symlink, WinError 1314, needs
  elevation) that reproduces identically on pristine main on this host.

flake8 clean on both changed source files. mypy src/kiro_crew/security.py
reports 3 errors — resource.getrlimit / RLIM_INFINITY / setrlimit at lines
8695-8703, POSIX-only attributes that are pre-existing on Windows and untouched
by this diff.

Manual verification

The reproduction table under Problem / Motivation was run as a standalone
probe against both trees (a pristine origin/main worktree vs this branch),
outside pytest, so the result does not depend on any fixture. It exercises the
issue's own scenario end to end: set KIROCREW_HOME, resolve
schedule_file.schedule_path(), and hand the resolved path to both gates.

Cross-tree baselines were taken by copying the changed test files into a clean
origin/main worktree and running pytest there, rather than by pointing
PYTHONPATH at it — the repo's conftest inserts the invoking worktree's src
ahead of PYTHONPATH, so the latter silently tests the branch and reports green.

Scope

security.py is the keystone gate and backend-security-controls is a
blocking: true AUTOSDE rule, so this is deliberately the only production
change in the diff: one added alternative, one cache key. The read gate's
_SENSITIVE_HOME_DIRS question raised at the end of #4082 is answered by the
same branch — the crew-prefixed entries of that list are part of the remainder
set, and test_a_crew_secret_leaf_under_the_override_is_blocked_too pins it.

Related Issues

Fixes #4082

Context: #4065, which pinned KIROCREW_HOME per test and made the asymmetry
visible.

Checklist

• Single commit with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — docs/architecture/security-deep-dive.md, Layer 2
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

The repository template currently contains an OSPO placeholder, so no CLA wording is reproduced.

Pattern harvest

Rule candidate: lint/AST check
Pattern: a regex path-fence branch defining its own bespoke right-boundary (terminator) class instead of reusing the module's shared path_end — each private terminator that omits the shell-operator class reopens the operator-glued bypass this PR's R4 round fixed. Candidate check: flag any new string-literal terminator alternation in security.py fence branches that is not composed from path_end/win_sep.

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

• ## 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]

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

• ## 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:

• ## 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]

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

Reviewed 2e0eafa430bb343cfc3e9b3fc7e0c444bce18478 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.py:9935 -- Raw cache key leaves a repointed custom home unfenced
home_key = os.environ.get("KIROCREW_HOME")
Repointed override symlink -> cached regex retains old resolved root -> embedded script writes the new resolved .env.
Anchor: backend-security-controls
Fix: Include the current resolved crew root in the cache key.

BLOCKING -- src/kiro_crew/security.py:9200 -- Empty shell quotes bypass the custom-home fence
out.append(r"\\?" + (r"[ \t]" if ch in " \t" else re.escape(ch)))
Custom home /tmp/crewhome -> python -c "open('/tmp/cr""ewhome/.env','w')" -> shell joins the quoted fragments and writes the protected file.
Anchor: backend-security-controls
Fix: Match empty-quote concatenations between generated path characters.

[BLOCK-MERGE] 2e0eafa
[GPT-REVIEWED] 2e0eafa

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

Both adjudicable findings are absent (0). Two FENCED findings remain; ruling annotate-only.

F1 — Cache keyed on the raw KIROCREW_HOME string (_get_sensitive_re, home_key = os.environ.get("KIROCREW_HOME")). _resolved_env_root resolves symlinks (security.py:10239 .resolve()), so a symlink repointed mid-process — without the env string changing — leaves the cached crew_home anchor stale. But the lexical anchor is built with os.path.abspath(os.path.expanduser(...)) (no resolve) and the $KIROCREW_HOME variable anchor are both unaffected, so the natural spellings a command carries stay fenced; only a literal name of the new real path is open, and the cache comment confirms the gate "cannot fall below what it matched before" (pre-PR base fenced none). Reaching harm requires a live repoint of the operator's own data-home symlink plus the attacker naming the new resolved literal instead of the still-fenced forms — extreme and self-inflicted, bounded residual. Record complete → FLAG.

F2 — _shell_escape_tolerant (security.py:9200) tolerates a backslash before each character but not empty-quote concatenation, so python -c "open('/tmp/cr""ewhome/.env','w')" survives the regex, and the path sits inside a non-path Python-source token so the normalizer's is_sensitive_path routing (is_sensitive_bash_command Pass 2) never sees it. Empty-quote concatenation is an ordinary, easily-typed obfuscation (the codebase already handles ca""t ~/.aws/... in the normalizer), not an extreme or contradictory condition, and a governance-file overwrite has no recovery. Cannot complete the rarity record → UPHOLD-FENCED.

[ADJUDICATION] 2e0eafa430bb343cfc3e9b3fc7e0c444bce18478 total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] 2e0eafa430bb343cfc3e9b3fc7e0c444bce18478
[ADJUDICATION-FENCED] 2e0eafa430bb343cfc3e9b3fc7e0c444bce18478 fenced=2 flagged=1
FLAG F1 src/kiro_crew/security.py:9935 -- Stale only after a live mid-process repoint of the data-home symlink without an env-string change; the $KIROCREW_HOME and lexical spellings stay fenced and the residual never falls below the pre-PR baseline.
UPHOLD-FENCED F2 src/kiro_crew/security.py:9200 -- Empty-quote concatenation inside a python -c payload is an ordinary evasion, not an extreme condition, and a governance-file overwrite has no recovery path.
[GPT-ADJUDICATED-FENCED] 2e0eafa430bb343cfc3e9b3fc7e0c444bce18478

🏷️ Fenced finding(s) machine-flagged as likely edge case

The security fence keeps these findings blocking regardless of adjudication; the only clearance path is a human override recorded by a repository writer, who must independently verify a rationale before recording it — it is machine-authored, and a wrong override on a security-class finding ships exactly the class the fence exists to stop. (This lane's comment deliberately carries no override command.)

• F1 src/kiro_crew/security.py:9935 — Stale only after a live mid-process repoint of the data-home symlink without an env-string change; the $KIROCREW_HOME and lexical spellings stay fenced and the residual never falls below the pre-PR baseline.

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

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

Design-Verdict: PASS

Real gate-parity hole, closed at the right seam — the shell branch derives from the same resolver and leaf lists the tool gate uses, so the gates can't silently drift.

Suggestions

• The lexical root is re-derived inline (os.path.abspath(os.path.expanduser(crew_override))) while the PR's own thesis is "reuse the tool gate's resolution"; hoist it into _ResolvedRoots (e.g. crew_home_lexical) so _candidate_forms and this branch read one derivation — otherwise a future normalization change in the resolver reopens the lexical-spelling half of exactly the drift this PR forbids.
• The "Pattern harvest" lint (flag a fence branch with a bespoke terminator not composed from path_end/win_path_end) is worth landing as a follow-up: four review rounds each caught one spelling, which is evidence the invariant needs a machine check, not more reviewer rounds.

[DESIGN-REVIEWED] 2e0eafa

[leonlaiyc]

The finding is valid and reproducible. With KIROCREW_HOME set to a directory containing a space, the branch as reviewed fenced three shell spellings and left the fourth open:

                                 0e95bdb5c
tool gate  is_sensitive_write_path     True
bash: bare (unescaped space)           Blocked
bash: double-quoted                    Blocked
bash: single-quoted                    Blocked
bash: BACKSLASH-escaped spaces         None     <-- the gap

A path with a space cannot appear bare in a command — the shell requires it quoted or backslash-escaped. The quoted forms keep the raw space, so the literal anchor matched them; .../my\ home/... carries a backslash the literal does not have. And this is the ordinary case rather than a corner, since C:\Users\First Last is a perfectly normal profile directory.

Fixed by making the anchor accept shell-escaped whitespace rather than by reverting the hunk: each segment now goes through _shell_whitespace_tolerant, which allows an optional backslash before each whitespace character. Tabs fold in with spaces — over-matching in the fail-safe direction for a gate that blocks on naming alone. On 5f693238c all four spellings block.

test_a_home_with_a_space_is_fenced_in_every_shell_spelling pins all four spellings against a tmp_path / "my home" override, with the tool-gate answer asserted first as a precondition so the case cannot silently stop testing anything. Against pristine origin/main the class is now 7 failed / 2 passed (the 2 remain the controls that must not change: an ordinary file under the override stays writable, and the ~/$HOME//home/<user> spellings still match); on this branch, 9 passed. Security suites re-run at 1033 passed / 18 skipped; flake8 clean.

[github-actions]

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

Premise-level review of 2e0eafa430bb343cfc3e9b3fc7e0c444bce18478 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 verification is done. The base confirms: _resolved_root_key exists as claimed, the new branch is not a duplicate (the normalizer pass covers only the read floor via is_sensitive_path, so write-protected leaves under an override had no shell-side control), but two of the new tolerances are point patches — the attached-option boundary exists on only 1 of 12 branches while pass 2 explicitly skips --prefixed tokens, and the resolved-override anchoring covers crew_home while _ResolvedRoots carries four more override roots the tool gate anchors (the $KIRO_HOME/agents residual is explicitly recorded at security.py:9529).

First-Principles-Verdict: CONCERNS

Two of the new tolerances fix gate-wide root causes at one branch only: attached-option paths and resolved-override anchoring each leave counted siblings open.

What this change ships

Intent: make the shell gate refuse the same files the tool gate refuses under a custom KIROCREW_HOME (#4082) — a FIX.

1. Shell writes naming a protected leaf's resolved custom-home path are refused — justified
2. $KIROCREW_HOME/%…%/$env: spellings of those paths refused — justified, mirrors branch (8)
3. Backslash-escaped spellings refused (over-matches quoted forms, fail-safe) — justified
4. Lexical and resolved spellings of a symlinked override both refused — justified, matches _candidate_forms
5. Keystone .tmp/.lock artifacts fenced under the override — justified, mirrors (3b)
6. Attached-option paths (curl -o<path>) refused on the new branch only — symptom-level, 11 unfixed siblings
7. Pattern cache re-keyed per KIROCREW_HOME, bounded at 8 — justified, stale key fails open
8. Dropping the override un-fences the old path — justified
9. Security-deep-dive paragraph updated — mandated same-commit
10. Ops-app test asserts the resolved path; linearity count 11→12 — justified

Watch

• The attached-option boundary (?:--?[A-Za-z][\w-]*)? guards 1 of 12 branches. Counted: 11 sibling anchors (grepped rf"|(?:^|[\s'\"=:,;]) in _build_sensitive_regex), and pass 2 skips --prefixed candidates (security.py:13604) while _path_candidates (security.py:11921) peels only =-values and redirects — so curl -o$HOME/.aws/credentials stays open on a default install while the identical spelling under an override is now refused. The general fix (shared boundary, or a _path_candidates peel) is reachable.
• Override-root generality: _ResolvedRoots carries kiro_home, codex_home, claude_config_dir, claude_home, all anchored by the tool gate; the new branch anchors crew_home alone. The resolved $KIRO_HOME/agents spelling — write-tier, uncovered by pass 2 — remains the "accepted residual" recorded at security.py:9529; this PR's own mechanism (resolved root + remainders) extends to it. 1 counted sibling of the same asymmetry.

[FIRST-PRINCIPLES-REVIEWED] 2e0eafa

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

Both symbols and the resolver exist; no NameError is introduced by the new branch, and the branch is empty (resolved_crew_branch = "") on a default home, so existing matching is unaffected.

Assessing the two candidates:

Candidate 1 (concurrent RuntimeError on eviction): Fails (a). There is no concrete concurrent caller — the candidate itself concedes it "could not confirm any caller actually invokes the gate from multiple threads concurrently," and the gate runs synchronously. Single-threaded, dict.pop(next(iter(d))) materializes one key then pops after the iterator is discarded; no size-change-during-iteration occurs. The (a)/(b) chain is "if two threads were to…", which the bar forbids. Dropped.

Candidate 2 (security.md not updated): Doc-freshness against an AGENTS.md convention, not an AUTOSDE rule and not a code behavior. It has no observable runtime wrong outcome — (c) cannot be re-derived from what the code does when executed. The candidate rates it low and calls it "a judgment call." It does not meet the (a)/(b)/(c) bar. Dropped.

No grounded Step-2 finding surfaced: the cache is strictly finer-grained than the prior single-slot form (keyed on raw KIROCREW_HOME, empty branch on default home), and the over-matching in _shell_escape_tolerant / root tolerance is documented and fail-safe (more fencing, never less).

No findings.

[OPUS-REVIEWED] 2e0eafa

[leonlaiyc]

Valid, and reproduced before fixing. With KIROCREW_HOME set to a custom root, 5f693238c fenced only the exact-literal spelling:

                            5f693238c   c9e3f4639
plain                       Blocked     Blocked
/parent/./home/...          None        Blocked
/parent//home/...           None        Blocked
/parent/zz/../home/...      None        Blocked
backslash spelling of same  None        Blocked

The root is as spellable as the remainder, and the tool gate fences every one of those because it resolves the path — so an exact-literal root was a one-character bypass of the whole branch.

Fixed by joining the root segments with the same generalized separator the fenced-dir branches above already use, plus repeated-separator tolerance:

crew_gsep = rf"{win_gsep}{win_sep}*"
crew_anchor = crew_gsep.join(
    _shell_whitespace_tolerant(part) for part in re.split(r"[\\/]", crew_home)
)

win_gsep is the existing canonical-no-op chain (\. and \X\.. excursions); the trailing {win_sep}* adds //. The same separator now joins the anchor to the remainder, so both halves of the branch are tolerant in the same way. Over-matching a path that ends elsewhere is the safe direction here — the trade win_gsep already documents for the branches it was written for.

test_a_canonical_no_op_spelling_of_the_root_still_anchors pins all five spellings, and carries two negative controls that keep the tolerance honest: a sibling root sharing a prefix (.../other home/...) is not fenced, and an ordinary file under the real root stays writable. Against pristine origin/main the class is now 8 failed / 2 passed — the 2 remain the controls that must not change; on this branch, 10 passed.

Security suites re-run at 1033 passed / 18 skipped, with one failure that is not from this diff: TestHomeDirTargetsCache::test_second_call_does_not_rebuild (assert 8 == 1) reproduces identically on pristine origin/main under the same load, and passes when its class runs alone. It is a wall-clock-vs-TTL flake in that test, unrelated to this branch. flake8 and mypy --platform linux clean.

[leonlaiyc]

Valid, and reproduced before fixing. A symlinked (or junctioned) KIROCREW_HOME names one directory under two paths, and _resolved_root_key() returns only the second:

	c9e3f4639	4bc50ac20
resolved spelling	Blocked	Blocked
lexical spelling	None	Blocked
ditto via python -c open()	None	Blocked

The lexical path is the one an operator actually types, because it is the one they configured — so anchoring on the resolved path alone left the whole branch open to the spelling most likely to be used. The tool gate does not have this gap: _candidate_forms expands a target into its resolved and its lexical forms.

Fixed exactly as the finding asks — the anchor now matches both roots:

crew_roots = [crew_home]
crew_override = os.environ.get("KIROCREW_HOME")
if crew_override:
    crew_lexical = os.path.abspath(os.path.expanduser(crew_override))
    if crew_lexical and crew_lexical not in crew_roots:
        crew_roots.append(crew_lexical)
crew_anchor = "|".join(
    crew_gsep.join(
        _shell_whitespace_tolerant(part) for part in re.split(r"[\\/]", root)
    )
    for root in crew_roots
)

abspath(expanduser(...)) is the same normalization _resolved_root_key itself falls back to on OSError, so one set of rules spells both roots. Both alternatives go through the identical separator tolerance, shell-whitespace tolerance and remainder derivation — the remainder list is not duplicated, only the anchor gained a second alternative — and when resolution is a no-op the two collapse to one and the pattern is byte-for-byte what it was. _SENSITIVE_RE_HOME_KEY is already keyed on the raw override, which is what the lexical form is derived from, so no cache change was needed.

No real symlink privilege was needed for the red-before. os.symlink requires elevation or Developer Mode on Windows, which would make the case skip on exactly the platform where a junction is the ordinary way to relocate a data home. The causal condition is lexical spelling != resolved spelling, not the OS call that produces it, so test_both_spellings_of_a_relocated_root_are_fenced injects the divergence at the resolver seam instead: KIROCREW_HOME is set to <tmp>/alias-home while _resolved_root_key is patched — to its real contract, (home, crew_home | None), with the genuine home preserved — to report <tmp>/real-home. The pattern is then built exactly as production builds it. No os.symlink, no sleeps, no elevation, no platform branch.

Two negative controls keep the widened anchor honest: an ordinary file under the lexical home stays writable, and a sibling root that merely shares its prefix (.../alias-home-two/...) is not fenced.

Fail-before/pass-after: on c9e3f4639 the resolved half of the new test passes and the lexical half fails (lexical spelling of the same protected leaf is not fenced); against pristine origin/main the class is now 9 failed / 2 passed — the 2 remain the controls that must not change — and on this branch, 11 passed.

Security suites re-run at 1185 passed / 18 skipped, with one failure that is not from this diff: TestHomeDirTargetsCache::test_second_call_does_not_rebuild (assert 8 == 1), the same wall-clock-vs-TTL flake reported on the previous push — it passes when its class runs alone, and this diff touches neither _home_dir_targets nor its TTL. flake8, isort and mypy --platform linux src/kiro_crew/security.py clean.

(Edited: the quoted snippet above had lost one backslash in the character class - the code on the branch is and always was [\/], matching the existing spelling at security.py:4928. Correcting the quote so it matches what is actually pushed at 4bc50ac; no code changed.)

[NicholasRBowers]

Same-span stall on span=3c5b15a2d41e — escalating to a human decision

Span: src/kiro_crew/security.py (gpt / blocking) — the custom-KIROCREW_HOME fence in resolved_crew_branch.

Span ledger

Round	Finding	Outcome
R1–R2	Earlier custom-home fence gaps	Patched per-instance
R3 (Aug 26)	Env-var-spelling bypass — $KIROCREW_HOME/... spellings reached the protected tree while the gate matched only the resolved literal path	Fixed on head a2a3872cb: added crew_var_anchor (POSIX $VAR/${VAR}, cmd %VAR%/!VAR!, pwsh $env:VAR spellings) to resolved_crew_branch, mirroring the $KIRO_HOME precedent; 2 parity tests added
R4 (this round)	Shell operators bypass the custom-home fence — the crew_var_anchor terminator `(?:{win_sep}	\s

Four-plus blocking rounds on one span is the same-span stall trigger: per protocol I am stopping instance patches on this span and asking for a ruling instead of pushing a 5th fix.

Options

1. Unify terminators (recommended). Delete the bespoke terminator in crew_var_anchor and reuse the shared path_end class for every anchor branch (resolved literal, $KIRO_HOME, and all KIROCREW_HOME var-spellings). This removes the divergence axis entirely — there is no per-operator whack-a-mole left, because there is only one terminator definition. ~5-line change; red-before-green verifiable with the same parity harness used in R3.
2. Spot-patch. Add the missing operator class to the bespoke terminator. Smallest diff, but it is the 5th instance patch on this span and leaves the two-terminator divergence in place for a future round.
3. Rule the residual out of scope. Treat operator-glued spellings as beyond this PR (which targets security: the bash gate does not follow a custom KIROCREW_HOME while the tool gate does #4082's basic custom-home parity), document the residual, and dispose the finding with that rationale.

Recommendation: option 1 — it is the restructure the stall rule asks for, small, and it makes the R4 class of finding structurally impossible rather than individually patched.

Awaiting decision; not patching further on this span until then.

[NicholasRBowers]

span=3c5b15a2d41e
disposition=fixed

R4 fix (nrb-ruled restructure, head b4ffab3c0): the crew branch now shares branch (3)'s path_end terminator.

The finding was real: resolved_crew_path closed with a bespoke terminator (win_sep/whitespace/EOL/quote) that omitted the shell-operator class, so an operator glued straight onto the protected path — echo x > $KIROCREW_HOME/<leaf>;id, cat $KIROCREW_HOME/<leaf>|wc -l — slipped the fence while the tool gate refused the same file.

Per the escalation ruling (option 1 in the span ledger), this is the restructure, not a 5th spot-patch: the bespoke terminator is deleted and the branch reuses the shared path_end class (plus the Windows separator), so there is one terminator definition for every anchor branch — an operator added to path_end covers the custom-home branch with no second edit, and there is no remaining divergence axis for this family of findings.

Proven red-before-green: test_an_operator_glued_onto_the_path_still_terminates_it (6 operator spellings: ; | & backtick, paren, glued redirect) fails on the previous head and passes on this one; the R3 parity tests and both negative controls ($KIROCREW_HOME_BACKUP, ordinary files under the override) still pass. Full security suite 1012 passed; black (26.3.1)/isort/flake8/mypy clean; rebased onto current main.

[NicholasRBowers]

span=c042398cee96
disposition=fixed

Fixed at head b4ffab3c0 by the same terminator unification that resolves the GPT blocking finding.

The advisory was correct: resolved_crew_path ended with the narrow terminator (?:{win_sep}|\s|$|['\"]), omitting the shell-operator class that branch (3)'s path_end carries, so an operator-glued spelling on a custom-KIROCREW_HOME install escaped the fence. The bespoke terminator is now deleted and the branch reuses the shared path_end class directly — exactly the divergence this advisory pointed at. Regression-tested with six operator spellings (red on the previous head, green on this one); the anti-over-match negative controls still pass.

[bolichen97]

Audit note — part of this has already landed; the rest has not

This PR is not a duplicate and is not finished by anything on main. The audit checked it part by part against main, and some of what it does is already there. Flagging it so a reviewer does not have to rediscover the overlap, and so the PR is not mistaken for fully-covered work.

Already landed

Two of the six references actually landed and both cover ZERO of this PR's behaviour: e536451 (#6993, the landed-commit index for main:270, ancestor of origin/main) and 2df4e2e (the landed-commit index for main:443, ancestor of origin/main). the initial scan called e536451 PARTIAL coverage; that is too generous — it obsoletes one implementation detail (crew_gsep's {win_sep}*) and blocks none of the PR's spellings. the initial scan's three other relations are non-landing by construction: #5336 and #5411 are still OPEN PRs (the issue/PR reference check:269, :273, merged_at '-'), so neither can cover anything, and #4082 — which the initial scan flagged UNVERIFIED — is an ISSUE (the issue/PR reference check:166), open, titled with the exact bug this PR fixes. #4065 is a genuinely merged PR (2026-08-17) but is cited as context (per-test KIROCREW_HOME pinning) and fixes nothing here.

Which parts main already has

Nothing the PR does is on main. What IS on main, verified: (1) subject-side separator-run collapsing — src/kiro_crew/security.py:8165 _SEPARATOR_RUN_RE / :8171 _separator_collapsed_variants / :8270 (from the landed e536451, #6993) — which makes the {win_sep}* half of the PR's crew_gsep redundant and, per main's own measurement, undesirable; (2) the shared terminator win_path_end at security.py:7143 (from the landed 2df4e2e), which supplies the exact restructure the PR's stalled escalation asked a maintainer to authorize; (3) pre-existing (already in the PR's merge base, conceded in the PR body) Pass-2 coverage of ordinary path-like spellings of keystone leaves under a custom home, via _is_path_like/_win_anchor_roots -> is_sensitive_path(). Items 1 and 2 are rebase mechanics, not behaviour this PR would duplicate.

What is still genuinely yours

The PR's entire capability, all 9 parts A-G plus the raw-text half of H. Empirically confirmed by importing origin/main's own src (git archive origin/main src/kiro_crew, no checkout) with KIROCREW_HOME set: the tool gate answers is_sensitive_write_path(/apps/ops-mission-control/data/rotation.yaml) = True while is_sensitive_bash_command returns None for the redirect, cp, python -c open and curl -o forms, for every $KIROCREW_HOME/%KIROCREW_HOME%/!..!/$env: spelling, for the backslash-escaped root and for the doubled-separator remainder — while the ~/.kiro/crew control is BLOCKED. Corroborated from main's own side: the merged d402acd (#7356, 2026-09-02) states 'the bash gate covers none of them' for the write-protected entries and adds test/test_security_posture.py:1227 test_the_bash_gate_really_does_not_cover_write_protected_paths as a tripwire, and issue #4082 is still OPEN.

Suggested action: REBASE — the remainder is real work; rebase onto the landed part rather than closing.

---

_{From a repository-wide duplicate/overlap audit of every pull request open against main (2026-09-02, 330 PRs, one reviewer per PR). Each PR was read as its full merge-base diff plus its description and every comment and review, then compared against each candidate PR's own diff and against origin/main at 1a765b88ceb7. This PR is not being closed — the note is informational. If the reading is wrong, please correct the reasoning rather than just the conclusion.}

[NicholasRBowers]

Escalation: CI perf guard is systemically flaky on CI runners (not a code regression)

test/test_security.py::TestNativeHomeEntryThenFencedRead::test_traversal_prefix_does_not_backtrack_catastrophically has now failed 4 times across 2 heads (b4ffab3 and 4aedd38) with timings of 1.07–1.24s against its 1.0s budget, and passed once on a lucky rerun. Locally the same call completes in <0.2s (5x headroom), verified repeatedly. CI runners execute it ~5x slower, putting the 1.0s budget right at the noise floor there.

All other lanes on head 4aedd38 are green. Options for the maintainer:

1. Bump the budget (e.g. 1.0s → 3s — still catches catastrophic backtracking, which is orders of magnitude slower)
2. Mark the test flaky / rerun-on-fail in CI
3. Accept as-is and rerun manually

Halting the automated drive here per the agreed flake-escalation threshold; not patching the test without a ruling.

[NicholasRBowers]

Escalation resolved per maintainer ruling — no budget change.

Root cause of the perf flake was in this PR, not the runners' speed alone: the compiled-pattern cache had become single-slot keyed on KIROCREW_HOME, so every env toggle (constant in the test suite) re-ran the full ~30–70ms pattern build inside whatever the next caller timed. Fixed with a per-key bounded cache; added a regression test that counts builder invocations across an env round-trip (failed red at 4 builds, now 2). Default-home hot path is byte-identical to main. Rebased onto current main (adopting its anchor optimizations and the widened _resolved_root_key contract). Head 0e90653.

[NicholasRBowers]

Escalation: timing tests now failing for reasons that implicate the CI runner, not this PR's code.

Context from scratch: this PR (fork, author @leonlaiyc, fixes #4082) makes the bash gate fence a custom KIROCREW_HOME like the default home. After the maintainer's ruling ("optimize, don't raise the 1.0s budget"), the pattern-cache churn this PR had introduced was fixed (per-key cache, head 0e90653) and a regression test locks it in.

On this head's CI round, Linux shard 3 failed two independent timing tests:

• test_traversal_prefix_does_not_backtrack_catastrophically: 1.29s vs 1.0s budget (5th failure across 3 heads).
• test_long_nonshell_line_does_not_blow_up (main's own complexity guard, not touched by this PR): 7.19s vs its 6s ceiling, where main's docstring expects ~1.5s on a dev box.

Local evidence that the code is not the cause: best-of-3 match time on the guard's 20KB line is identical between origin/main and this head (1.498s vs 1.496s), and the suite-induced pattern rebuilds are gone (build-count regression test passes). A runner ~5x slower than baseline fails both tests — including main's own guard at main's own cost.

Also on this round, test_autonudge_stop_auth (5 failures, Windows AND Linux shard 1) — monitor-loop code this PR never touches; passes locally at this head; the previous head was green there. Arrived with the rebase; main's last ~20 CI runs are all cancelled by push churn, so no main-side verdict exists.

One finding WAS ours and is already fixed locally awaiting push: main's new anchor-count pin (11) vs our added fence branch (12) — pin bumped, full suite (1363) green.

Maintainer's call requested — options in the Slack summary.

[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 #5336 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 #5336: MERGE_DISCUSSION. Different bypass class in the same anchor construction; complementary, conflicting only textually. Files: src/kiro_crew/security.py.

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

[NicholasRBowers]

Maintainer decision requested: the 1.0s perf test is now demonstrably measuring cold compile + runner load, not this PR's matching cost.

Context from scratch: this fork PR (author @leonlaiyc, fixes #4082) extends the bash gate to fence a custom KIROCREW_HOME. After the earlier ruling ("optimize, don't raise the budget"), the PR's real defect — a single-slot pattern cache that rebuilt the regex on every env toggle — was fixed with a per-key cache (regression-tested), and the branch now uses field access into _ResolvedRoots.

On head abaa6ae's round, everything is green except test_traversal_prefix_does_not_backtrack_catastrophically (1.456s vs 1.0s; 6th failure across 5 heads). The decisive controls:

• Main's own 6s complexity guard (test_long_nonshell_line_does_not_blow_up) passed on the same shard — no quadratic behaviour.
• Local best-of-3 match cost is identical between origin/main and this head (1.498s vs 1.496s on the guard's 20KB input; the perf test's near-miss is ~30ms warm).
• The test times one single call with no warm-up, so its 1.0s window includes the one-time pattern compile (~70ms local, amplified 5–20x on loaded CI runners). Its docstring guards against catastrophic backtracking — compilation is not backtracking.
• Post-cache-fix record: fail 1.29s → pass → fail 1.456s across three rounds of the same code, which is the signature of runner variance, not a code property.

Options (maintainer's call):

1. Warm the gate before the timer (one untimed is_sensitive_bash_command() call before started = time.perf_counter()): the test then measures pure matching against its 1.0s budget — no budget change, and arguably what it always meant to measure.
2. Raise the budget for this one test (e.g. 2s), keeping cold compile in the window.
3. Accept it as a flake and rerun-on-red (leaves every future PR paying this tax).

Recommendation: option 1. The 6s guard already covers the pathological-input case with a load-tolerant ceiling.

[NicholasRBowers]

self-added: yes
mechanism: nested-payload variable anchor in the crew branch (${KIROCREW_HOME... skipping lazily to the first separator)

• parameter-expansion modifier bypasses the custom-home fence span=3c5b15a2d41e — fixed in 4e13f67.

Fixed. The braced anchor's [^}]* had to reach the closing brace before any remainder was read, so a payload nested inside the expansion (${KIROCREW_HOME:+$KIROCREW_HOME/<leaf>}) never met the remainder chain. A second alternative now anchors on the outer braced name and skips lazily to the first separator, where the ordinary gsep->remainder->closer chain takes over; the branch closer gained } so the path ends at the expansion's own brace. This rules on the whole nested-payload family for the crew variable, either inner spelling, not one modifier. Regression: test_a_payload_nested_in_a_parameter_expansion_is_fenced (includes the ${KIROCREW_HOME_BACKUP} name-boundary negative control).

[NicholasRBowers]

self-added: yes
mechanism: crew-relative keystone artifact remainders (mirror of branch (3b) inside branch (9))

• custom-home branch omits keystone publish artifacts span=3c5b15a2d41e — fixed in 4e13f67.

Fixed. The crew remainder set was derived from _SENSITIVE_HOME_DIRS + _WRITE_PROTECTED_BASH_LEAVES only, so the atomic-write temp and lock sibling of a keystone leaf (branch (3b)'s shapes) were fenced under the default home but not under an override — the embedded-script spelling (python -c "open('$KIROCREW_HOME/security_policy.json.tmp','w')") is regex-only and had no backstop. The crew branch now derives crew-prefix-relative artifact parents from _KEYSTONE_ARTIFACT_PARENTS and closes with (3b)'s own suffix lookahead, so a leaf added to the parent list is covered here without a second edit. This ruling covers the artifact-shape family under the override, all three spellings (variable, resolved literal, embedded script). Regression: test_keystone_publish_artifacts_are_fenced_under_the_override (with the .tmpx different-file negative control).

[NicholasRBowers]

self-added: yes
mechanism: crew branch closer widened to the shared win_path_end (plus / and })

• crew branch terminator lacks literal $ (POSIX path_end on a Windows-capable branch) — fixed in 4e13f67, exactly as prescribed.

Fixed with the review's own prescription. The crew branch matches PowerShell/cmd variable anchors and resolved literals but closed with the POSIX path_end, whose $ is the end-anchor, not a literal — so Set-Content $env:KIROCREW_HOME/security_policy.json$null x failed the terminator and no other branch anchors a custom-home path. The closer is now the shared win_path_end (which carries the literal $ and the Windows separator) plus the POSIX / continuation and }; sharing the class means an operator added to win_path_end covers this branch without a second edit. Regression: test_an_expansion_tail_does_not_end_the_path_early (all three spellings from the finding).

[NicholasRBowers]

self-added: yes
mechanism: none this round (no code change; ruling on the resolution call site)

• Custom-home resolution can wedge the gateway loop span=3c5b15a2d41e — rebutted (not a defect of this diff).

Not a defect introduced here. The flagged line (crew_home = _resolved_root_key().crew_home) executes only inside _build_sensitive_regex(), i.e. on a pattern-cache MISS — at most _SENSITIVE_RE_MAX_KEYS (8) times per process. The base already resolves the SAME roots synchronously on EVERY gate call: _home_dir_targets() computes roots = _resolved_root_key() BEFORE its cache lookup (security.py:10581), reached per-call from _path_in_home_dirs / is_sensitive_path / _is_keystone_publish_artifact — and the tool gate (is_sensitive_write_path) resolves through the data home on the loop by definition. A stalled network mount wedges those pre-existing per-call paths first; this diff's cache-miss-only call adds no new exposure.
The prescribed fix (resolve off-loop, pass the result in) is the exact fail-OPEN TOCTOU the base fixed by design: key and build MUST come from one resolution ("Resolve the roots ONCE and use the same tuple for both the key and the build", pinned by test_roots_are_resolved_once_for_key_and_build). Splitting resolution from compilation here would file one root's pattern under another root's key.
Class ruling: findings requiring THIS PR to move the base's own synchronous root resolution off the event loop are covered — that is a base architecture change (all gate paths, not this branch) and out of this PR's scope.
Maintainer override, if preferred over the rebuttal: /ai-review override gpt 3833d1b: the flagged blocking resolve is the base's own per-call cache-key path (_home_dir_targets), not introduced by this diff; moving resolution off-loop is a base-wide change out of scope here.

[bolichen97]

Closing as superseded on main by merged #9183 (with #9089).

Those two PRs deleted the text-layer path fence in src/kiro_crew/security.py outright: the file, _build_sensitive_regex, _get_sensitive_re and _SENSITIVE_RE no longer exist on main (replaced by the src/kiro_crew/security/ package), and test/test_security_posture.py::test_the_bash_gate_really_does_not_cover_write_protected_paths now pins that the bash gate does NOT match paths in command text. Every symbol this PR edits is gone, so there is nothing to rebase onto, and re-landing this diff would reverse a merged architectural decision.

The underlying report (issue #4082, custom KIROCREW_HOME not followed) should be re-triaged against the new design (is_sensitive_path + sandbox bind-mask) rather than against this branch. Thanks for the careful work here -- the KIROCREW_HOME observation was correct even though the layer it patched has since been removed.