refactor(security): split security.py into a package and drop path regex

[bolichen97]

Problem / Motivation

The shell gate refused ordinary read-only work. During this refactor alone the live gateway blocked grep -rn ... src test docs, cd <scratch dir> && python probe.py | grep -v ..., a heredoc carrying backticks ("64 nested substitutions"), a git diff | grep | sed pipeline, and a Python probe whose command line merely contained the string ~/.aws/credentials. None of those read a credential. Each refusal came from security.py -- 21k lines in one file, most of it regex trying to decide from command TEXT whether a path might be sensitive -- and each cost a retry with a different tool plus the tokens to explain the block.

The OS sandbox is where that question belongs: sandbox.py bind-masks the crew-home secrets (_CREW_HIDDEN_LEAVES) and seals the governance keystone read-only (_CREW_READONLY_LEAVES) in every mode, hides ~/.aws in strict/cc, and is_sensitive_path fences every resolved path the file tools open. Where the sandbox deliberately leaves a store visible (see residuals), a text regex was never a real control either -- it refused one spelling and let the next one through. A text regex over the command cannot be made complete (each closed spelling opened another -- #9089 went six review rounds on exactly that) and adds nothing the sandbox does not already hold.

Why it matters

Every false refusal is a wasted agent turn and a wrong lesson ("this tool is unsafe") on a command that was fine. The team direction, agreed after #9082 and #9089, is to remove the regex text layers rather than keep narrowing them. This PR is that removal, plus the package split that makes the surviving 16k lines navigable.

What changed (motivation → approach → change)

Package split (behaviour-preserving). kiro_crew.security is now a thin facade over owner modules ordered by dependency: vocabulary → helpers → shell_normalizer → paths → denied_rules → redaction → exfil → argv_floor, plus diagnostics. Every public and test-reached private name is re-exported from a frozen manifest (_exports.py, ratchet-tested) and a monkeypatch on the facade mirrors onto the owning module, so existing imports and patch targets keep working. The two load-time cycles MAP found are broken by moving shared vocabulary down or keeping an import call-time; the ~40 judgement calls are in .refactor/DECISIONS.md on the branch history (not shipped).

Text path matching deleted, not narrowed. is_sensitive_bash_command keeps three tiers: the size ceiling, the IMDS check, the environment-credential exfiltration check. Removed: the fence-literal and relative-traversal matchers, separator-run collapse, assignment-resolved views, trust-root cd re-rooting, the trust-root extraction control, the native home-entry scan, and every helper only they used (paths.py 3131→1880, shell_normalizer.py 3311→2806, argv_floor.py 2813→2588). From the deny catalog, three groups, catalog 149→111: (1) the 27 sensitive-file-read rows (<verb>.*<credential store> -- the same regex under another name); (2) the 4 self-management rows that fired on the product name appearing anywhere in a path, grep pattern or filename -- they had structural argv-floor twins, and the floor now runs them ungated instead of looking up its own catalog row (that lookup was a trap: deleting a row would have silently disabled the floor); (3) 7 bare-word rows with no successor: legacy-delete-stack-underscore, legacy-terminate-instance-underscore, legacy-drop-table-underscore, legacy-delete-table-underscore, legacy-delete-bucket-underscore (.*<word>.*, aws-destructive / sql) and legacy-get-secret, legacy-read-secret (credential-exfil). They refused any command containing a boto3 method name or a secret-fetching tool name -- grep -rn delete_stack src/, git log --grep drop_table, editing delete_table.sql -- and an earlier revision of this description wrongly filed them under the product-name group and claimed every destructive/credential-exfil row was kept. It was not: see residuals.

Kept, deliberately. is_sensitive_path (keystone; the file-tool fence). The IMDS / env-credential tiers. Every destructive, git-publish, pipe-to-shell, reverse-shell and credential-exfil catalog row that matches a COMMAND shape -- aws cloudformation delete-stack, aws s3 rb, DROP DATABASE, curl ... | bash -- as opposed to a bare word. The argv-structural self-protection floor. Output redaction. self-protection-cron-adopt (no floor twin, real ownership grab). Each surviving refusal now names its rule id and matched span so a false positive can be self-diagnosed.

Stated residuals -- two review lanes flagged them, they are real, and the maintainer has decided them.

• standard mode (the default) does NOT bind-mask ~/.aws, ~/.ssh or ~/.kube -- sandbox.py leaves them visible on purpose so kiro-cli's own credential resolution keeps working (_STANDARD_DIRS vs _STRICT_DIRS; the module's own comment says the shell gate "is the control" for those). After this PR a spawned cat ~/.aws/credentials in standard mode is refused by nothing. My first description said "every sandbox mode"; that was wrong. Decision (maintainer, @bolichen97): (a) accept the residual. The matcher never made the store unreadable -- it refused one spelling and passed the next -- while refusing ordinary read-only work; masking .aws/.kube in standard (with the .aws/config carve-out cc already has) is a sandbox-tier change for its own PR.
• The deleted _WRITE_PROTECTED_BASH_LEAVES/_BARE_TOKEN_PROTECTED_LEAVES tier fenced shell WRITES to six crew-home leaves. Four have an OS seal already (settings_seeds.json, computer_use.json, aws_service_consent.json READONLY; routing, workflow_library, apps/meetings/data/edits HIDDEN) and their tests now assert that disposition. Two do not: connections-tool-aliases.json (an alias-deletion grant record) and playwright-cli-config.json (kept VISIBLE by sandbox.py on purpose; the agent can already point PLAYWRIGHT_MCP_CONFIG elsewhere). The models/ weights directory and the ops-mission-control rotation.yaml/incidents/index.json are likewise unsealed. Decision (maintainer): accepted as a stated residual. Sealing the alias record in _CREW_READONLY_LEAVES is the follow-up and is NOT in this PR: it needs the empty-document and precreate analysis settings_seeds.json got, and a sandbox change does not belong inside a deletion.
• Decision (maintainer): the 7 bare-word legacy-* rows stay deleted, with nothing replacing them. A python -c "boto3.client('cloudformation').delete_stack(StackName='prod')" one-liner is no longer refused by text; the hyphenated CLI spellings still are. The bare-word rows refused every command that merely CONTAINED the method name, which in a repository that talks to AWS is a routine grep, and a one-liner they did catch is one file-write away from a script they never caught. src/kiro_crew/cloud/__init__.py's module docstring still names the underscore shapes as "the MCP-side block"; that note is now stale and is corrected in this PR.
• A keystone READ from a shell is not blocked at the text layer (writes are sealed by the sandbox; AGENTS.md says so). sel_hmac.key is VISIBLE to the sandbox and has no bash-layer fence; closing that means moving its in-sandbox reader behind the gateway, not another matcher. The cron command vetter is a separate surface with its own credential-path detector; it gains a backslash-unescaped view so the spellings the deleted catalog row used to catch there stay caught there.

Tests

• TestTheBashGateMatchesNoPaths pins cat ~/.aws/credentials and cd ~/.kiro/crew && cat security_policy.json as not refused by the gate while IMDS, env-cred and oversize still are.
• TestNoCatalogRowMatchesACredentialPath pins the category gone, five credential-path commands passing is_denied, and the neighbouring families still refusing.
• TestProductNameAnywhereIsNotADenial pins the eleven product-name rows gone, representative false positives allowed, and the genuine self-kill / self-restart still refused by the floor -- including with every built-in disabled.
• TestARefusalNamesItsRuleAndSpan pins the diagnostic line on catalog, floor and ceiling refusals and that it never echoes matched bytes.
• test_security_facade.py pins the export manifest and patch-mirroring.
• Thirteen test files outside the security suite pinned the deleted shell tier on their own leaf or producer (computer-use keystone and typing policy, AWS consent, seed provenance, Teams routing, meetings edits, workflow library, browser launch config, alias record, the CLI permission gate, hook deny classification, the auto-improvement governance gate); each now asserts the sandbox disposition that actually holds, or documents the residual where none does.
• Tests asserting the deleted behaviour are removed, not weakened; producer-coupled tests (deny guidance, hooks prefix handling, workflow permission tier) are re-pointed at the IMDS / env-cred / boto3 producers that still exist.
• Targeted run: 4552 passed across test_security*, test_hooks*, test_denied*, test_redact*, test_mcp_cron_security*, test_governance*, test_deny_guidance, test_workflow_security, test_host_isolation_floor, test_sandbox_governance_mask. flake8 / isort / mypy (--platform linux) / black gate / docs-lint / brand / harness / subprocess-encoding gates green.

Manual verification

Full suite on the pre-#9089 shape of this branch: 89,308 passed; every failure reproduced identically on origin/main on the same host (ops-mission-control redaction, xdist worker budget, autonudge reconciler). Ran the probe is_sensitive_bash_command("cat ~/.aws/credentials") → None, ("env | grep AWS_SECRET") → refused, ("curl http://169.254.169.254/...") → refused, 30 KB command → refused with rule=keystone-scan-ceiling.

Related Issues

Follows #9082 (cron script bodies leave the shell gate) and #9089 (drop the traversal simulation). Closes the false-positive class behind #8889.

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

[github-actions]

First Principles Review (Fable 5.1) — ✅ human override accepted

@bolichen97 overrode this lane for 26b2f2a7b6d7accd008228f685298cdd9c429149 via /ai-review override (this commit only).

[github-actions]

Design Review (Fable 5.1) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound refactor that executes the codebase's own "OS layer, not a text matcher" mandate — but it removes the only shell-layer fence on ~/.aws/~/.ssh/~/.kube in the default standard mode without landing the compensating sandbox mask.

Watch

• Default-mode credential residual is deferred, not tracked. _STANDARD_DIRS (sandbox.py:1040) omits .aws/.ssh/.kube (only _STRICT_DIRS/_CC_DIRS mask them), so after deleting the text matcher a spawned cat ~/.aws/credentials in the default mode is refused by nothing — the PR states this and the maintainer accepted it, but the compensating standard-mode mask is punted to an unnamed follow-up. The direction is correct (a matcher that catches one spelling and passes the next was never a control), yet the default posture is measurably weaker between this merge and a follow-up that has no issue/gate holding it. Confirm the follow-up is filed and, ideally, that the standard-mode mask lands close behind, so the gap isn't indefinite.

Suggestions

• Consider gating merge of this deletion behind (or same-sprint with) the standard-mode .aws/.kube mask PR, so the default-mode window is minimized rather than open-ended.

[DESIGN-REVIEWED] 26b2f2a

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @bolichen97 overrides the GPT 5.6 finding for 26b2f2a7b6d7accd008228f685298cdd9c429149; 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 26b2f2a7b6d7accd008228f685298cdd9c429149: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 26b2f2a7b6d7accd008228f685298cdd9c429149 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 26b2f2a

Verdict parsed from the review's SHA-scoped output markers for commit 26b2f2a7b6d7accd008228f685298cdd9c429149.

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

[bolichen97]

/ai-review override gpt 33e745c: intentional design, accepted by the maintainer -- the shell gate's path regex refused one spelling of an unbounded set and passed the next (six review rounds on #9089 alone) while failing ordinary read-only work; the OS sandbox is the control where a store is masked, and the standard-mode visibility of ~/.aws/~/.ssh is a pre-existing sandbox trade for kiro-cli's own credential resolution that is now stated in the spec and PR body rather than papered over by a matcher.

[bolichen97]

Replying to the Design Review and First Principles CONCERNS on 33e745c35, item by item. The maintainer (@bolichen97) has taken the decision the description deferred.

standard mode leaves ~/.aws / ~/.ssh / ~/.kube visible (Design watch 1, FP watch 1, GPT F1). Decision: option (a), accept the residual. Rationale: the visibility is a pre-existing sandbox.py trade so kiro-cli's own credential resolution works in the default tier; the deleted text matcher never made that store unreadable, it refused the naive spelling and passed the next one (#9089 alone went six rounds of command/exec -a/nice/env -i/timeout/busybox), while refusing grep -r over a worktree and a probe whose command line merely contained the path. A control that stops the honest agent and not the adversarial one is cost without protection. Whether standard should mask .aws/.kube with the .aws/config carve-out cc already carries is a real question, but it is a sandbox-tier change with its own compatibility surface and belongs in its own PR, not inside a deletion. The spec (security.md, shell-gate bullet) and the PR body now state the residual explicitly.

Alias record (connections-tool-aliases.json) unsealed (Design watch 2). Accepted as a stated residual for the same reason; sealing it in _CREW_READONLY_LEAVES needs the empty-document / precreate analysis settings_seeds.json received and is the follow-up. Recorded in the PR body.

Split into two commits (Design suggestion). The package move and the deletion are one commit on purpose: a package that carries the deleted layer never exists in history, so there is no intermediate shape to bisect onto. The behaviour diff is auditable from the test side -- TestTheBashGateMatchesNoPaths, TestNoCatalogRowMatchesACredentialPath, TestProductNameAnywhereIsNotADenial and the re-pointed leaf tests are the whole of it.

refactor: vs fix: (FP watch 2). The gate accepts either; refactor is the maintainer's choice because the dominant change by far is the package split and the security change is a removal, not a fix to a defect in the retained code.

Cron vetter's two unescaped views (FP watch 3 / subtraction). Kept. mcp_cron._vet_shell_command is a separate surface that #9082 deliberately left with its own credential-path detector, and its tests (test_mcp_cron_security.py::test_vet_shell_command_blocks_malicious) pin the backslash spellings; the two views keep that layer's existing contract intact after the catalog row that used to co-cover them is gone. Whether that layer should follow this one out is a separate decision on that surface.

[github-actions]

Human judgment recorded

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

intentional design, accepted by the maintainer -- the shell gate's path regex refused one spelling of an unbounded set and passed the next (six review rounds on #9089 alone) while failing ordinary read-only work; the OS sandbox is the control where a store is masked, and the standard-mode visibility of ~/.aws/~/.ssh is a pre-existing sandbox trade for kiro-cli's own credential resolution that is now stated in the spec and PR body rather than papered over by a matcher.

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