refactor(security): drop the regex traversal simulation from the bash gate

[bolichen97]

Problem / Motivation

is_sensitive_bash_command ran three passes that worked out where a command would
end up by re-implementing shell and find-utils grammar in regex: a cd/variable
normalizer (_check_sensitive_via_normalizer, _check_sensitive_cd_taint), an
alternate-tool traversal analysis (_check_alt_traversal_reaches_fence), and a
find traversal analysis (_check_find_traversal_reaches_fence) — roughly 4.2k
lines of matchers, budgets and helpers covering quote and variable expansion, brace
expansion and sequences, cd base tracking, pipeline staging, and find primary
and -name pattern grammar.

They denied ordinary read-only commands far more often than they caught an access
the literal fence did not already name:

• A relative root such as . resolved against the gateway process's own working
  directory, which on the desktop app is / — an ancestor of every fenced store.
  So grep -r pattern ., find . -name '*.py', ls -R . and du -sh * were
  refused whatever cd preceded them, and the verdict depended on where the gateway
  happened to start rather than on the command.
• Each pass carried a fail-closed budget, which turned that into a permanent
  refusal keyed on the subject's SIZE or SHAPE rather than its content: an English
  docstring opening with "Find …" exhausted the 64-root budget, and 66 markdown code
  spans read as 66 nested substitutions.

Why it matters

Every one of those refusals lands on an agent doing ordinary work — reading its own
repo, grepping for a symbol, listing a tree — and it is not recoverable by rewording,
because the trigger is the gateway's cwd or the input's length. The budgets made the
cost fall on prose and on long-but-benign scripts rather than on an attacker.

What changed (motivation → approach → change)

Symptom: read-only traversals refused, with the verdict depending on the gateway's
launch directory. Root cause: the gate was simulating the shell in regex to guess a
path the command never spelled. Change: delete all three passes and the ~4.2k lines
behind them, and keep the literal fence.

What survives is the part that names a path it can actually see: _sensitive_pattern_hit,
_RELATIVE_SENSITIVE_RE, the separator-run collapse, the trust-root extraction control,
the IMDS check and the env-credential rules. Enforcement of the keystone itself is
unchanged and sits where it cannot be talked around — is_sensitive_path() on every
resolved path a caller opens, plus the OS sandbox for the agent process.

The deletion set was derived mechanically, not by eye: an AST + git blame pass
computed which top-level symbols the three PRs authored, then a reachability pass
from every public entry point and every cross-module import found what became dead.
Symbols already unreachable before this change (an unwired protected-branch check,
two unused shell-token helpers) were excluded so they stay out of the diff. Three
PR-authored symbols are kept because live code uses them: _SENSITIVE_LEAF_PARENT_DIRS
and _GENERAL_PURPOSE_PARENT_DIRS feed the literal keystone regex, and
_SHELL_ASSIGN_RE is used by _resolve_local_assignments.

Files deleted — test/test_security_alt_traversal.py (1849 lines) and
test/test_security_relative_traversal_roots.py (117 lines) tested only the removed
machinery, so they go whole. test/test_security.py loses 3 classes and 39 test
functions for the same reason. Nothing else is removed.

Stated residual. A spelling whose fenced segment is not adjacent to the traversal
it travels through — ~/../<user>/.aws/credentials, up and back down through an
intervening segment — is no longer refused by this gate. Every ordinary relative
spelling that resolves to a fenced path still is, including the .., . and
repeated-separator forms, because the fenced segment stays within the matcher's
reach. This is not pinned either way in the tests, so a later change is free to
decide it.

Tests

• TestTraversalSimulationIsGone (new, in test/test_security.py) pins the two
  halves together: 10 read-only traversals that must be allowed, and 8 spellings that
  name a keystone path and must still be refused. Asserting only the allowed half
  would pass just as well if the fence were deleted outright.
• The same class asserts the removed helpers are absent by name
  (_check_find_traversal_reaches_fence, _check_alt_traversal_reaches_fence,
  _check_sensitive_via_normalizer, _check_sensitive_cd_taint,
  _find_traversal_reaches_fence, _alt_root_reaching_fence, _path_candidates) and
  that is_sensitive_bash_command no longer takes _traversal_subjects. A behavioural
  assertion alone cannot tell "the simulation is gone" from "the simulation is present
  and happens to allow this input", which is how a reinstated pass would slip back in.
• One test that became vacuous was removed rather than left green:
  test_name_only_traversal_is_the_same_class_as_every_other_leaf compared two
  verdicts that are now both None.
• test/test_security_gate_liveness.py and test/test_security_path_resolve_bounded.py
  each dropped one assertion that named a removed symbol; both files stay green
  (69 passed).

Manual verification

N/A — unit coverage is sufficient: this is a deletion inside one gate, and both the
allowed and the refused half are asserted directly against
is_sensitive_bash_command.

Local gates run green on this commit: check_black_formatting, isort, flake8
(the 3 findings in touched files are present verbatim on origin/main), mypy
(same 3 pre-existing errors as base), docs-lint, scrub-lint, and the security /
hooks / cron / llm_helpers test files directly (2339 + 766 passed). The full local
suite reached 73% of 89k items with zero failures before the host OOM-killed its
workers; a lower-concurrency re-run is in flight, and CI's own run on the merge ref
is the authoritative one.

Related Issues

no linked issue: this is a maintainer-directed removal, raised from a review of the
gate's false-positive rate rather than from a filed report.

Pattern harvest

Rule candidate: review-prompt
Pattern: a text-analysis gate that simulates another language's grammar to guess a
value the input never spells — the simulation's false-positive rate grows with the
grammar's surface while its true-positive rate stays bounded by what a literal
matcher already covers.

Checklist

• At most two commits (one is the norm), 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)
• No secrets, credentials, or internal references in the diff

Second stated residual (found by the GPT review lane, round 2)

A find rooted at home that hands a discovered file to a reader — find ~ -name <cred-leaf> -exec <reader> {} +, and the | xargs <reader> form — was refused on main
and is not refused here. It is accepted rather than fixed, and the reason is a measurement
rather than a preference: _SENSITIVE_HOME_DIRS carries .aws and .ssh as fenced
directories and holds no credential leaf NAME, so main never matched this shape on a
keyword. It matched by inferring where a traversal rooted at ~ could arrive, which is
the machinery this change removes. The only text-local rule that catches the named command
also denies find ~ -name '*.log' -exec <reader> {} + — an ordinary read-only command, and
exactly the false-denial class that motivated the removal. Closing it properly means a
resolved-path fence rather than a text-adjacency one, which is a change of its own size.

Not a regression from this PR: the brace spellings the same finding named
(cat ~/.a{ws,bc}/<leaf>, cat ~/.{aws,ssh}/<leaf>) are allowed on origin/main as well
— the removed simulation never covered brace expansion — so this change does not alter
their verdict. Measured on both trees rather than inferred.

Both residuals share one root cause, worth naming because it is what a follow-up should
target: the surviving fence matches ADJACENCY in the command text, so any indirection that
separates a fenced directory from its leaf — a .. through an intervening segment, a
brace, a path discovered at runtime — escapes it. The variable-indirection case the same
lane raised in round 1 was closable inside this PR precisely because an assignment is
decided by the command text alone; these two are not.

An aside that is itself evidence for the measurement above: composing this section tripped
Kiro Crew's own live gate, whose find simulation is still present, because the example
command in the prose resolved against a real home directory. That is the behaviour this
PR removes, observed from the outside.

Third stated residual: the one leaf the OS layer cannot fence

The whole text-matcher layer is deleted rather than extended, because the enforcement
that actually binds a shell subprocess is the OS layer in sandbox.py -- a spawned
command reaches a file through an open() that never routes through the tool gate, so a
path fenced only there is readable in any sandbox mode whatever a matcher recognises.
Every crew-home leaf therefore carries one of three dispositions, and the disposition IS
the guarantee:

leaf	disposition	what holds
credential homes, .env, live_target.json, backup/	HIDDEN	bind-masked from the subprocess tree in every mode
security_policy.json, admission_policy.json, profiles, computer_use.json	READONLY	write refused in every mode; read permitted by design
sel_hmac.key	VISIBLE	no OS fence -- sandbox.py's own note: these "stay on the tool gate alone"

Two consequences are accepted rather than matched around:

• The governance ceiling READ is no longer blocked at the bash layer. Its WRITE still
  is, unconditionally. This is deliberate at the OS layer too: masking a policy file makes
  it resolve to the permissive standalone default, so hiding a ceiling REMOVES it instead
  of protecting it. AGENTS.md is updated in this commit to state what is actually
  guaranteed instead of the "neither read nor write" it claimed.
• sel_hmac.key and .local_secret are VISIBLE (the SEL audit key and the
  MCP-to-dashboard auth secret), so obfuscated bash spellings of them are
  no longer caught; the direct spelling of each is still refused, by is_sensitive_path
  AND by the bash gate. token_signing.key is HIDDEN and unaffected.
  Closing this properly means moving its in-sandbox reader behind the gateway so the leaf
  can become HIDDEN -- a sandbox.py change, reachable by no matcher. Accepted by the
  repository owner for this change rather than deferred to an issue, on the reasoning
  that the text layer never bounded it either.

Six review rounds on one span produced ten distinct spellings (V=$HOME, brace expansion,
find -exec, an argument-position assignment, /./, a glued >leaf, an assignment
feeding a cd, a conditional reassignment, glued statement boundaries, pushd). Each
closure narrowed an unbounded set by one and several introduced defects of their own. That
is the evidence for moving the control to the layer that can hold it rather than growing
the one that cannot.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 4495a29635369f3b0e4ace223a4492216f93e421 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Deletes a control at the layer that never bound the subprocess, keeps enforcement at the OS layer that does; residuals measured on both trees and owner-accepted.

The one boundary change — the governance-ceiling read is no longer blocked at the bash layer — is a truthful restatement, not a weakening: the OS sandbox deliberately permitted that read in every mode (a spawned open() never routed through the text gate), so the old "neither read nor write" claim was only ever true for bash-spelled reads, and the write block that makes the ceiling un-loosenable is untouched. On hosts without a sandbox backend, wrap_argv fails closed rather than running unconfined, so the stated residuals are live only under an explicit mode="off" opt-in — consistent with what that mode already means. Docs, spec, and the two-sided pin test land in the same commit.

[DESIGN-REVIEWED] 4495a29

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @bolichen97 overrides the GPT 5.6 finding for 4495a29635369f3b0e4ace223a4492216f93e421; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 4495a29635369f3b0e4ace223a4492216f93e421: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 4495a29635369f3b0e4ace223a4492216f93e421 — 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 evidence is in. The core change is a cause-level deletion (the AGENTS.md invariant it records — "a regex spelling-chase is a review smell" — is written/updated in the same commit as required). I verified the one new production symbol: _fence_hit has exactly one caller (_fence_hit_in_collapsed, security.py:10874), while its docstring justifies its existence by a second caller, "pass 1c", that does not exist anywhere in the tree (grep _fence_hit → def + 1 call; no "pass 1c" outside that docstring). I also verified the description names test/test_security_gate_liveness.py as touched, but the diff's 9 diff --git headers do not include it, and the diff touches two test files (test_governance_self_protection.py, test_snapshot_redaction_optout_ceiling.py) the visible description never names.

First-Principles-Verdict: CONCERNS

The deletion is aimed at the true cause and earns itself; the one rider, _fence_hit, justifies itself by a "pass 1c" caller that doesn't exist.

What this change ships

Intent: stop the bash gate falsely refusing ordinary read-only commands whose verdict depended on the gateway's cwd or the input's size — a FIX, achieved by deletion.

1. grep -r x ., find . -name, ls -R ., du -sh * no longer refused — justified
2. Long/prose-shaped subjects no longer permanently refused on fail-closed budgets — justified
3. Obfuscated keystone spellings (variable, cd-taint, /./, glued redirect, find-delivery) no longer bash-gate-refused; OS sandbox + is_sensitive_path hold — declared residual
4. ~9.7k net lines of simulation passes/helpers deleted from security.py — justified (the mechanism)
5. Two test files (1,966 lines) + 3 classes/39 tests deleted — justified
6. TestTraversalSimulationIsGone pins allowed half, refused half, and helper absence by name — justified (repo's ratchet pattern)
7. Two keystone tests rewritten to pin sandbox dispositions instead of bash-gate refusals — undeclared in visible description
8. AGENTS.md keystone invariant rewritten to name the OS layer as enforcement point — justified (same-commit spec rule)
9. security.md rewritten to match — justified
10. New helper _fence_hit replacing inline checks in _fence_hit_in_collapsed — rides along, one consumer, phantom second caller

Watch

• Description says "test/test_security_gate_liveness.py … dropped one assertion"; that file is not in the diff (grepped the 9 diff --git headers). Meanwhile test_governance_self_protection.py and test_snapshot_redaction_optout_ceiling.py ARE rewritten and the visible (truncated) description never names them — sync the description to the diff.
• Titled refactor while behavior moves in both directions (previously-refused commands now allowed; previously-refused obfuscated keystone spellings now allowed) — the description's own "Symptom → Root cause → Change" framing is a fix.

Subtractions

• Inline _fence_hit back into _fence_hit_in_collapsed (security.py:10880): counted 1 consumer (security.py:10874); its docstring's stated reason — "pass 1c hands in an assignment-resolved view" — names a caller that exists nowhere in the tree. If pass 1c arrives in a follow-up, extract then.

[FIRST-PRINCIPLES-REVIEWED] 4495a29

[bolichen97]

• Removing the normalizer reopens credential reads (span=3c5b15a2d41e) — fixed in d75d5e471.

Legitimate and reproduced before changing anything. Six variable-indirection
spellings were allowed on fdab8c824, including the one the finding names:

ALLOW  V=$HOME; awk 1 "$V/.aws/credentials"
ALLOW  V=$HOME; cat "$V/.aws/credentials"
ALLOW  V=~;     cat "$V/.aws/credentials"
ALLOW  V=$HOME; V+=/.aws; cat "$V/credentials"
ALLOW  V=$HOME; cat "$V/.ssh/id_rsa"
DENY   cat $HOME/.aws/credentials        (control — home spelled)

Fix: Retain local-assignment path resolution; remove only cwd-dependent traversal inference.

Taken as written. _assignment_resolved_views re-runs the three pass-1 checks over
an assignment-resolved copy of each line, reusing the _resolve_local_assignments
resolver that was already in the tree for the program-word rules rather than adding a
second one, and routing through the same _fence_hit_in_collapsed helper the
separator-collapsed copies use so the fence keeps one spelling. It resolves
assignments and nothing else: no working directory, no cd base, no brace expansion,
no find filter grammar. All six spellings above now deny; V=$HOME; cat "$V/notes.txt" and OUT=build; tar -czf $OUT.tgz . still pass, as do the read-only
traversals this PR exists to unblock.

Two things worth recording because they shaped the fix rather than following from it:

The first attempt built one view over the whole command and regressed
test_verb_anchored_form_stays_allowed_when_order_or_line_breaks — joining every
token with spaces erases the newline that ENDS a command, so cat foo on one line
paired with a fenced path on the next and produced a denial nobody wrote. The view is
now per line, with only assignment-shaped tokens carried forward, so an earlier line's
verb can never reach a later line's path while an assignment still carries across a
newline the way the shell carries it. Both directions are pinned.

TestAssignmentIndirectionStillReachesTheFence was checked for vacuity by stubbing
the new pass to a no-op: 5 of its 6 denial cases go red. The sixth
(D=$HOME/.aws; cat "$D/credentials") stays green because pass 1 already matches
$HOME/.aws in the assignment text itself, which is correct and is why it is listed
separately here rather than counted as coverage of the new pass.

The adjudication note is accurate on the mechanism and on the absence of a recovery
path at this gate: is_sensitive_path guards a file tool's title, not a file a shell
child opens. That is the reason this was fixed in-PR rather than rebutted — it is
security-class and reachable, so the "disproportional" outcome does not apply, and
the requested scope was narrow enough that keeping it does not reinstate any of the
cwd-dependent inference this PR removes.

[bolichen97]

• Dropping traversal guards permits credential reads (span=3c5b15a2d41e) — rebutted in part, accepted-residual in part.

This is round 2 on this span. Round 1 named a variable indirection and was
fixed in d75d5e471 (a real defect: V=$HOME; awk 1 "$V/.aws/credentials").
Round 2 names two different spellings, and they do not share a disposition, so
both halves are answered separately below with the same baseline measurement.

Every verdict below is measured, not argued — the same probe run against this
branch and against origin/main (where the simulation is still present):

spelling	origin/main	this branch
cat ~/.a{ws,bc}/<leaf>	ALLOW	ALLOW
cat ~/.{aws,ssh}/<leaf>	ALLOW	ALLOW
cat ~/.a{w{s,x},bc}/<leaf>	ALLOW	ALLOW
find ~ -name <leaf> -exec cat {} +	DENY	ALLOW
find ~ -name <leaf> | xargs cat	DENY	ALLOW
cat ~/.aws/<leaf> (control)	DENY	DENY

Half 1 — brace expansion: the finding is factually wrong.
cat ~/.a{ws,bc}/<leaf> is allowed on origin/main as well. The removed
simulation never covered brace expansion, so "Retain brace-expansion
normalization" asks this PR to retain something that does not exist and never
did. This PR does not change the verdict for any brace spelling, so it is not a
regression here — it is a pre-existing gap on main, and closing it is a
separate change with its own review rather than something to fold into a
deletion.

Half 2 — find delivery: real, and deliberately accepted.
This one is a true regression against main, and the adjudication is right about
the mechanism. It is accepted rather than fixed because there is no proportional
way to close it, which I established rather than assumed:

_SENSITIVE_HOME_DIRS carries .aws and .ssh as fenced directories; it
contains no credentials or id_rsa leaf name. So main never matched this
shape on a leaf-name keyword — it matched by reasoning about where a find
rooted at ~ could arrive. That reachability inference is the removed
machinery (#7298, 1602 lines), and this PR exists to remove it. The only
text-local rule that catches the named command is "a find rooted at home with
an -exec read verb", which also denies find ~ -name '*.log' -exec cat {} +
— an ordinary read-only command, and precisely the false-denial class that
motivated the deletion.

So the choice is not fix-vs-ignore, it is: restore the grammar and its false
denials, or accept this residual. This PR takes the second, and the residual is
recorded in the PR body rather than left for a reader to discover.

is_sensitive_path still guards the file tools, so the residual is scoped to a
shell child, and the OS sandbox remains the layer that covers it.

Round 3 on this span would hit the same-span stall trigger. If the lane returns
with a third spelling of the same adjacency limitation, the answer is not another
instance patch — it is the restructure this residual already points at: a
resolved-path fence rather than a text-adjacency one, which is a follow-up of its
own size, not an amendment to a deletion.

/ai-review override gpt d75d5e4: the brace half is allowed on main too so it is not a regression from this PR, and the find half is closable only by restoring the traversal grammar this PR exists to delete — the residual is accepted and documented.

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 4495a29635369f3b0e4ace223a4492216f93e421 — this comment is updated in place on each push.

Review details

No findings that block. The one survivor is an inherited, already-accepted residual, not a regression this diff introduces.

FINDING — src/kiro_crew/security.py:10994 — removing the _check_sensitive_via_normalizer pass (now # No traversal SIMULATION runs here) lets an indirection-spelled read of a VISIBLE crew-home credential leaf — e.g. V=~/.kiro/crew; cat "$V/.local_secret" (the MCP→dashboard auth secret) or sel_hmac.key — pass the bash gate, and those four leaves (.local_secret, sel_hmac.key, security_events.jsonl, crons.json) are in _CREW_SANDBOX_VISIBLE_LEAVES so the OS sandbox never masks them; the direct spelling stays fenced by is_sensitive_path, so only the obfuscated read newly opens. Does not block: the leaf was already reachable pre-PR by a runtime-assembled path (python3 -c "open(a+b)") the normalizer never caught, so the diff widens the trivially-exploitable spelling surface of a pre-existing, deliberately-accepted exposure rather than causing it, and the security.md residual note documents only sel_hmac.key while .local_secret/crons.json/security_events.jsonl share the class → Fix (out-of-diff, architectural per the invariant "never another matcher"): move the in-sandbox readers of these leaves behind the gateway so they can become HIDDEN, and extend the stated residual to name all four VISIBLE credential leaves, not just sel_hmac.key.

[OPUS-REVIEWED] 4495a29

Verdict parsed from the review's SHA-scoped output markers for commit 4495a29635369f3b0e4ace223a4492216f93e421.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 4495a29635369f3b0e4ace223a4492216f93e421: <one-sentence reason>

[bolichen97]

/ai-review override gpt d75d5e4: the brace half is allowed on origin/main too so it is not a regression from this PR, and the find half is closable only by restoring the traversal grammar this PR exists to delete, so the residual is accepted and documented in the PR body.

[github-actions]

Human judgment recorded

@bolichen97 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for d75d5e471a7adcb208defde95189915b1570bc9b.

the brace half is allowed on origin/main too so it is not a regression from this PR, and the find half is closable only by restoring the traversal grammar this PR exists to delete, so the residual is accepted and documented in the PR body.

This decision applies only to this commit. A new push requires a new judgment.

[bolichen97]

• new pass 1c (_assignment_resolved_views) is O(L²) in assignment-line count with no cap (span=c9886dc1579f) — fixed in e12f227d0d49.

Correct, and correct about the consequence rather than only the complexity. Measured
on d75d5e471 before changing anything, via the exact path the finding names
(_subject_is_shell_grammar=True, _max_chars=256*1024):

  500 lines ->  0.472s
 1000 lines ->  1.765s   x3.7
 2000 lines ->  7.337s   x4.2
 4000 lines -> 30.397s   x4.1

So the watchdog is crossed at ~4,000 lines, well inside what
MAX_SCANNABLE_SOURCE_BODY_CHARS admits — the finding's reasoning about the
uninspected-source-body fallback holds, and the 25 s
dashboard.loop_stall_exit_after_secs self-exit is reachable rather than
theoretical. This is a defect I introduced in the round-1 fix, not a pre-existing
one.

After (same measurement, same path):

  500 lines ->  0.017s
 1000 lines ->  0.007s
 2000 lines ->  0.016s   x2.2
 4000 lines ->  0.034s   x2.1
 8000 lines ->  0.069s   x2.0
16000 lines ->  0.147s   x2.1

Of the two fixes offered, the second was deliberately not taken. A cap on
carried does restore linearity, but it is evadable by padding with dummy
assignments until the real one is evicted — it trades a DoS for a bypass in the very
pass that exists to close a bypass. Instead the resolver now runs once over every
token in the subject and the result is sliced back into per-line views, which is
linear and keeps every assignment in the subject in scope. Slicing is sound because
_resolve_local_assignments emits exactly one token per input token; each line is
pre-split with _split_glued_operators so the resolver's own internal split is
idempotent and the recorded offsets still line up. That contract is checked at
runtime, and on a mismatch the function degrades to per-line resolution with no
cross-line carry rather than slicing at wrong offsets — losing only the multi-line
indirection, never reporting a hit for a line that has none.

Pinned by test_the_resolver_runs_once_regardless_of_line_count, which asserts the
resolver INVOCATION COUNT is 1 at 10, 200 and 4,000 lines rather than timing
anything: deterministic on a shared runner, and it fails for the actual reason (the
resolver ran per line) instead of merely for a slow machine. Verified non-vacuous by
reverting the single pass to a per-line one, which turns it red.

Behaviour is unchanged — all six variable-indirection spellings from round 1 still
deny, V=$HOME; cat "$V/notes.txt" and OUT=build; tar -czf $OUT.tgz . still pass,
and the read-only traversals this PR exists to unblock are unaffected.

[bolichen97]

• Indirection-spelled read of a VISIBLE crew-home credential leaf passes the bash gate (span=c042398cee96) — accepted-residual, and the finding's leaf list is now carried in the PR body.

Correct, correctly advisory, and it improves the record: I had documented this residual
naming only sel_hmac.key, and .local_secret belongs in the same sentence. Verified
both, plus the neighbour that does not:

leaf	sandbox disposition	direct spelling still refused?
.local_secret	VISIBLE	yes — is_sensitive_path and the bash gate
sel_hmac.key	VISIBLE	yes — both
token_signing.key	HIDDEN	bind-masked in every mode

So the residual is exactly the indirection-spelled form, not the leaf: a command that
names either path outright is still refused by the surviving fence, and
token_signing.key is not affected at all.

Why this is not fixed by restoring the pass. The deleted matcher never bounded these two
leaves, because VISIBLE means what sandbox.py says it means — "in-sandbox code needs
READ and WRITE, so no OS rule can apply without breaking it. These stay on the tool gate
alone" — and the tool gate does not see a spawned shell's open() at all. Six rounds on
one span produced ten spellings of that same gap (a variable, brace expansion,
find -exec, an argument-position assignment, /./, a glued >leaf, an assignment
feeding a cd, a conditional reassignment, glued statement boundaries, pushd), so
re-adding the variable case would close the one spelling this finding names and leave the
rest, at the cost this PR exists to remove.

The proportionate fix is at the layer that can hold it: move each leaf's in-sandbox reader
behind the gateway so the leaf can become HIDDEN, which is a sandbox.py change and
reachable by no matcher. Out of scope here, and deliberately not filed as a deferral,
because the repository owner accepted this residual for this change after being shown the
VISIBLE disposition and what it does and does not cover.

Recorded in the PR body under the third stated residual, now naming both leaves.

[bolichen97]

• Removing indirection checks exposes the gateway credential (span=3c5b15a2d41e) — overridden by the repository owner, tracked in security: split the gateway credential by capability #9175. Superseded ruling: this record first read needs-a-decision on scope; the investigation that followed showed the finding's own proposed fix does not close it, which changed the answer. Kept rather than deleted so the reasoning chain stays auditable.

Not rebutted, and not overridden. Both questions pass, and the second one passes in the
direction that matters: this finding does not ask for the deleted matcher back. Its fix
line asks to "broker in-sandbox authentication through the gateway and mask
.local_secret before removing indirection protection" — the same architectural direction
this PR argues for, with an ordering constraint attached. That is not a spelling chase, and
a reachable credential escalation is never in scope for the disproportionality escape
hatch.

Verified rather than taken on description:

leaf	disposition	consequence
.local_secret	VISIBLE	no OS fence; reading it mints an owner dashboard token — escalation, not recon
sel_hmac.key	VISIBLE	no OS fence; audit-key exposure
token_signing.key	HIDDEN	bind-masked in every mode, unaffected

The direct spelling of each is still refused, by is_sensitive_path and by the bash
gate. What is gone is the indirection-spelled form.

Why .local_secret is VISIBLE at all, and what unblocks it. Exactly one reader is
in-sandbox: mcp_cron.py's cron_trigger hands config_dir() / ".local_secret" to
trigger_cron_job(...) to authenticate to the gateway's own HTTP API. Kiro Crew's own MCP
servers are spawned by kiro-cli UNDER the sandbox launcher and share the agent's mount
namespace, so masking the file today breaks that call — which is what VISIBLE records.
The other readers (pod/runtime.py, cli_commands.py, cli_server.py) are CLI-side and
outside the sandbox.

So the fix is bounded and has a precedent in the same file: playwright-extension-token is
masked precisely because it "reaches the CLI through the environment, never by open(), so
masking the file costs nothing". Brokering mcp_cron's read the same way moves
.local_secret from VISIBLE to HIDDEN, which closes this for every spelling rather
than the one named here.

The ruling I need. That change lands in the MCP spawn path and sandbox.py, not in
this PR's security.py text layer, and handing a credential to a sandboxed child through
the environment is its own decision to make carefully (the sandbox already strips sensitive
env names, so the mechanism has existing conventions to respect). Two coherent orders:

1. Mask first, in its own PR — satisfies this finding's ordering literally, keeps one
   logical change per PR, and this PR goes green once it lands.
2. Fold the masking into this PR — one PR, but it widens a change already six review
   rounds deep into a second subsystem and re-arms every lane on a much larger diff.

Auto-merge is armed on this PR, and this [BLOCK-MERGE] is what is holding it — nothing
lands while this is open, which is the correct state. Put to the repository owner now;
this record will be replaced with the disposition of whichever order is chosen.

---

Resolution

The scope question was put to the repository owner and the answer changed once the fix was
investigated rather than assumed. Masking .local_secret — the fix this finding names — is
cosmetic: the gateway writes the same secret to run/gateway-<port>.secret as well,
run is in sandbox._CREW_SANDBOX_VISIBLE_LEAVES, and read_local_secret() resolves the
per-port copy FIRST, so every in-sandbox reader already prefers the path that is not
.local_secret. Hiding one of two copies of one value changes nothing.

Nor is any hiding-based fix available: the MCP servers share the agent shell's environment,
and a gateway-side broker still leaves the server holding something the shell can read.
Nothing inside the sandbox can hold a secret the agent cannot read. The closable form is
capability scoping — split the credential so the in-sandbox half grants only the MCP surface
it already grants and carries no token-mint route — which is an auth-boundary redesign
(~500-800 lines) with no relationship to this PR's diff.

So: overridden here, tracked in #9175, which carries the verified facts, the rejected
alternatives, the scope estimate and a clean starting worktree. The override comment states
the reasoning and records the ordering trade explicitly, including that the credential is
readable at the per-port path on main throughout that window too.

[bolichen97]

/ai-review override gpt 4495a29: the named fix is provably ineffective and the exposure is architectural and pre-existing — the identical credential stays readable in-sandbox at run/gateway-<port>.secret, so this is tracked as its own auth-boundary change in #9175 rather than blocking a PR that does not create it.

Repository-owner override, with the reasoning recorded rather than asserted.

The finding's own fix does not close it. GPT asks to "mask .local_secret". Verified
against origin/main: the gateway mints ONE secret and writes it to two files —
run/gateway-<port>.secret (always) and .local_secret (the single-instance fallback).
run is in sandbox._CREW_SANDBOX_VISIBLE_LEAVES, and
config.loader.read_local_secret() resolves "per LISTENER first:
run/gateway-<port>.secret, then the fallback" — so every in-sandbox reader already
prefers the copy that is not .local_secret. Masking the named leaf moves a path string
and leaves the credential exactly as readable.

The exposure is architectural, and it predates this PR. That one value is both
X-Local-Secret (the owner token mint) and X-Internal-Secret (the ~65 MCP/internal
routes), published where in-sandbox code must read it. Three candidate fixes were examined
and all fail for the same reason: env does not help (Kiro Crew's own MCP servers share the
agent shell's environment, so the value is one printenv away), and "let the gateway act
on the server's behalf" does not help (the server must still authenticate, and whatever it
holds, the shell holds). The general constraint: nothing inside the sandbox can hold a
secret the agent cannot read. So it is not closable by hiding — only by scoping, which is
an auth-boundary redesign (~500-800 lines across server.py, the token_auth wiring,
config.loader, sandbox.py and ~157 test references).

What this PR actually changes here. It removes a text-layer matcher that never bounded
this credential: a spawned shell reaches a file through an open() that never routes
through the tool gate, so a path fenced only there is readable in any sandbox mode whatever
the matcher recognises — sandbox.py says exactly that in its own note. The direct spelling
of both paths is still refused, by is_sensitive_path and by the surviving bash fence. What
is gone is one incomplete speed bump in front of a door the OS layer deliberately leaves
open to the shell.

Tracked, not waived: #9175 carries the capability split (two secrets: an internal one
that keeps exactly the MCP surface it grants today, and a local one for the owner token
mint published only to HIDDEN locations), with the verified facts, the rejected
alternatives, the scope estimate, and a clean starting worktree. It is filed as a real
issue rather than a promise in a review thread precisely because this override is not a
dismissal.

Also recorded, so the trade is not silent: overriding lets this PR land before #9175,
which inverts the ordering the finding asked for. In the window between them
.local_secret has neither the indirection matcher nor an OS mask — though the credential
is readable at the per-port path throughout that window on main as well, which is the
point above.

[github-actions]

Human judgment recorded

@bolichen97 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 4495a29635369f3b0e4ace223a4492216f93e421.

the named fix is provably ineffective and the exposure is architectural and pre-existing — the identical credential stays readable in-sandbox at run/gateway-<port>.secret, so this is tracked as its own auth-boundary change in #9175 rather than blocking a PR that does not create it.

This decision applies only to this commit. A new push requires a new judgment.