[buluoray]

Problem / Motivation

Goal: Give argv_floor.py real headroom under the 4,500-line per-module cap by moving its largest self-contained cluster, the ssh self-target floor, into its own module with no behaviour change.

src/kiro_crew/security/argv_floor.py was at 4,454 lines on main against _MODULE_LINE_CAP = 4_500 (test_security_gate_liveness.py::test_no_single_module_grows_back_into_a_monolith). With that little room, a security fix that adds code to the module has to move some unrelated function out first, picked because it is easy to move that day. The PR closing #10205 did exactly that with _backtick_closer.

Why it matters

This module gets multi-round lexer hardening all the time. Without headroom, each of those PRs pays a forced-relocation toll and wears down the layer boundaries the cap exists to protect. #10284 is already waiting on this headroom.

Not a goal

• Splitting the rest of argv_floor.py (self-kill, subcommand, credential-mint, git-publish floors). How to partition those is the owner's decision recorded on security: argv_floor.py rides the 4,500-line anti-monolith cap; plan a deliberate split #10283. This PR is the first slice and says Refs, not Closes.
• Any change to what the ssh self-target floor allows or denies.
• Moving the two argv-floor helpers the ssh floor reads (_self_token_frames, _static_substitution_output) down into shell_normalizer.py.

What changed (motivation → approach → change)

• Cap pressure → split by cohesion, not by convenience → the # ── Sandbox-escape floor section (lines 1245–3449 on main: _is_ssh_to_self, the own-host name/address cache, the hosts-file verdict cache, the DNS verdict layer and the operand walk) moves into src/kiro_crew/security/ssh_self_floor.py. I picked this cluster first because, measured on main, none of its 89 top-level names are used anywhere else in argv_floor.py. It is also the cluster the earlier hosts_file.py and host_addresses.py splits were already peeling. Finally, none of the four open PRs that edit argv_floor.py (fix(security): consolidate the git-publish scan onto a bash-faithful word lexer #17442, fix: deny a git push whose subcommand hides behind a non-plain word #17374, fix(security): quote-aware argv windows for the mint and pkill self-protection floors #16496, fix(security): keep a quoted newline inside its word on the push path #15438) touch these lines.
• The moved body is byte-identical to main, with three exceptions. Two calls now read _argv_floor._self_token_frames(...) and _argv_floor._static_substitution_output(...) through the module attribute, following the patch-visibility convention argv_floor already uses for the reader. Two comments say ssh devbox instead of an internal host name, because the commit leak guard refuses that name in newly added lines.
• ssh_self_floor imports argv_floor, and argv_floor never imports it back, so the edge runs one way.
• Facade: the 24 moved names in _EXPORTS now map to ssh_self_floor, and the TYPE_CHECKING import block is split the same way. is_denied reads _is_ssh_to_self from its new owner.
• dashboard/server.py imports warm_own_host_names from the new module.
• Docstrings in hosts_file.py and host_addresses.py, the module list in docs/system-specs/modules/security.md, and two references in testing-conventions.md now name the new owner.
• Result: argv_floor.py drops from 4,454 to 2,210 lines, and ssh_self_floor.py is 2,279.

Backwards compatibility

Compatible: every name stays reachable on the kiro_crew.security facade, now routed to the module that owns it, and the floor's verdicts are unchanged.

The diff removes the moved names from argv_floor's module namespace. These are their readers on origin/main @ 0d885e6:

Reader: src/kiro_crew/security/init.py:is_denied -- chat -- reads _is_ssh_to_self from _submodule("ssh_self_floor"); test_denied_commands_security.py (1316 passed)
Reader: src/kiro_crew/security/init.py:_EXPORTS -- chat -- the 24 names now map to ssh_self_floor; test_security_facade.py::test_a_facade_name_is_the_owning_submodules_object
Reader: src/kiro_crew/dashboard/server.py:warm_own_host_names -- release -- imports from ssh_self_floor
Reader: test/integration/test_boot_smoke.py:_KNOWN_SECOND_BOOT_CHANGES -- release -- module path updated to kiro_crew.security.ssh_self_floor
Reader: test/test_denied_commands_security.py:_ssh_self_floor -- release -- patch and read targets updated
Reader: test/test_security.py:ssh_self_floor -- release -- patch and read targets updated
Reader: test/test_deny_guidance.py:_ssh_hook_result -- release -- patch and read targets updated
Reader: test/test_argv_floor_inline_and_brace_scope.py:ssh_self_floor -- release -- patch and read targets updated

A search of test/ and src/ for every remaining argv_floor.<name> and setattr(argv_floor, "<name>") finds each name on its target module. The one exception is _perm_verb_mention_only in a docstring, a stale reference that is already on main. This search caught two monkeypatch.setattr(_argv_floor, "open", ..., raising=False) calls that would otherwise have silently instrumented nothing. Both now target the new module.

Tests

• New: test_security_gate_liveness.py::test_the_argv_floor_never_imports_the_ssh_self_floor pins the one-way edge. Mutation check: adding from .ssh_self_floor import _is_ssh_to_self to argv_floor.py turns it red.
• _PACKAGE_LINE_BUDGET re-pinned from 28,572 to 28,612 (the measured total). The 40 lines are the new module's docstring and its own import block. The per-file cap is untouched.
• TestDataConsumerGuardIsChargedPerFrameNotPerTriggerToken (test_denied_commands_security.py) now patches _data_consumer_exempt on ssh_self_floor too, because one of its four frame loops moved there. Without that patch the instrument would no longer see that loop.
• Run locally against this worktree (PYTHONPATH=<worktree>/src, -n0): test_security_facade 11, test_security_single_storage 2846, test_security_gate_liveness 11, test_argv_floor_inline_and_brace_scope 187, test_deny_guidance 132, test_denied_commands_security 1316 (1 skipped), test_denied_commands_hooks 117, test_inline_payload_completion_regressions 48, test_inline_payload_scoped_regressions 134, test_redaction_mirror_parity 12, test_blocked_commands_disclosure 7, all passed. test/integration/test_boot_smoke.py skips locally.
• Not run locally: test/test_security.py. On this host, pytest is SIGKILLed during collection of that file. The same thing happens on an unmodified main checkout, so it is an environment problem and not this change. CI is the evidence for that file.

Manual verification

N/A — this is a pure move with no runtime-visible change. kiro_crew.security._is_ssh_to_self is kiro_crew.security.ssh_self_floor._is_ssh_to_self holds, and is_denied("ssh localhost") still refuses with rule=sandbox-escape-ssh-self. Windows and macOS address paths are moved code only; they were not exercised on this Linux host.

Related Issues

Refs #10283

Decision for the owner

#10283 asks how argv_floor.py should be partitioned into submodules, and when that split should happen relative to the in-flight floor PRs. This draft does not commit to a partition for the rest of the module. It moves only the ssh self-target floor, which is about half the file, has no internal callers left in argv_floor, and is untouched by the open floor PRs, so the other submodule boundaries stay your call.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|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

Contribution License Agreement

Contributions are accepted under the project's Apache-2.0 license; see the Licensing section of CONTRIBUTING.md.

[github-actions]

Sensitive Change Review

Sensitive files changed:

• src/kiro_crew/security/__init__.py

• src/kiro_crew/security/argv_floor.py

• src/kiro_crew/security/host_addresses.py

• src/kiro_crew/security/hosts_file.py

• src/kiro_crew/security/ssh_self_floor.py

• No approval on 4392cc5 yet.

A human reviewer with write access must approve the CURRENT head and
paste this template into the approval, every field filled
(N/A needs a reason):

## Sensitive change review
- What rule changed:
- Before -> after (what was allowed/blocked, now):
- Worst case if wrong (breaks commands / leaks secret / locks user out):
- Evidence checked (which sandbox result proves it):
- No regression:
  - Tools still work (which tools/commands were run, result):
  - Backward compatible (old config / old data / old callers still work, how checked):
  - Existing tests (which suites passed on this head):
- How to undo:

[github-actions]

GPT 6.1 Review — ✅ no blocking findings

GPT 6.1 completed its review of 4392cc5859f5e6099e1f4f21a4a517142e8ee1fe and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 4392cc5

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

[github-actions]

Design Review (Opus 5.5) — ✅ PASS

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

Design-Verdict: PASS

Confirm CI ran test/test_security.py green, since the author couldn't collect it locally and it holds patch targets this move retargets.

[DESIGN-REVIEWED] 4392cc5

Description read

[DESCRIPTION-READ] 90efab491052bb7e0a95d33de8390f5cd463ccc3277bc849fc41a029a3fe000e

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

First Principles Review (Opus 5.5) — ✅ PASS

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

First-Principles-Verdict: PASS

test/test_security.py was never run locally. Only CI shows that its re-pointed patch targets (ssh_self_floor.socket, the open shims) still reach the moved code.

What this change ships

Inventory (7 items) — 7 justified

Intent: give argv_floor.py room under the 4,500-line per-module cap by moving the ssh self-target floor out unchanged. This is an ADDITION (refactor) with no behaviour change.

1. The ssh self-target floor, its caches and its DNS verdict layer now live in ssh_self_floor.py instead of argv_floor.py — justified
2. The facade maps the 24 moved names, and is_denied's _is_ssh_to_self, to the new owner — justified
3. The dashboard server imports warm_own_host_names from the new module — justified
4. A new liveness test checks that argv_floor never imports ssh_self_floor — justified
5. The package line budget goes up by 40 lines, from 28,572 to 28,612 — justified
6. Tests now patch the new module (socket, open, caches, _data_consumer_exempt), and the boot-smoke keys are renamed — justified
7. The security spec, the testing-conventions doc and the hosts_file/host_addresses docstrings now name the new owner — justified

[FIRST-PRINCIPLES-REVIEWED] 4392cc5

Description read

[DESCRIPTION-READ] a24ecea742e5c941f54de90d46a8216d21e2e8e31425dc2c43336440740757e0

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

[github-actions]

Opus 5.5 Review — ✅ no blocking findings

Reviewed 4392cc5859f5e6099e1f4f21a4a517142e8ee1fe — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 4392cc5

Verdict parsed from the review's SHA-scoped output markers for commit 4392cc5859f5e6099e1f4f21a4a517142e8ee1fe.

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

[github-actions]

Security Scope Review -- ✅ PASS

Which legitimate operations does 4392cc5859f5e6099e1f4f21a4a517142e8ee1fe newly refuse? Updated in place on each push. A script-confirmed regression blocks; the model proposes candidates and never decides a refusal.

Verdict basis: adjudicated, zero confirmed regressions.

Security scope review -- adjudicated candidates

• macos-latest (platform posix): 20 classified, 0 newly refused, 9 still allowed.
• ubuntu-latest (platform posix): 20 classified, 0 newly refused, 9 still allowed.
• windows-latest (platform windows): 12 classified, 0 newly refused, 4 still allowed.

No candidate flipped from allowed to refused.

Scope-Verdict: PASS

TIGHTENING: Nothing gets stricter. The ssh-to-self sandbox-escape floor (_is_ssh_to_self and its own-host name, address and hosts-file machinery) moves verbatim from argv_floor.py into a new ssh_self_floor.py. The floor is still wired into the same self-protection loop under the same rule id: - ("sandbox-escape-ssh-self", _argv._is_ssh_to_self), / + ("sandbox-escape-ssh-self", _ssh._is_ssh_to_self),. The only threat it refuses is unchanged: an ssh/scp/sftp/rsync connection whose target is this same host, which re-enters it outside the sandbox. Comparing the removed argv_floor lines with the new module, the logic is identical. Two comments change (ssh dev-dsk becomes ssh devbox), and two helper calls now go through the argv_floor module (_argv_floor._static_substitution_output and _argv_floor._self_token_frames). Every import removed from argv_floor is no longer referenced anywhere in that module.

ADJUDICATED: 24 candidates, 0 confirmed newly-refused

PLATFORM COVERAGE:

• posix: remote scp/rsync/sftp transfers and an rsync with no remote at all. The -e/--rsh payload is covered, plus $HOME and $(hostname) placed outside the target operand, and GIT_SSH_COMMAND. These probe the operand parser, the -e/--rsh parser, and the hostname-variable and command-substitution readers that moved.
• windows: ssh.exe by its .exe name, scp.exe with a C:\ source, a backslash drive-letter destination, and %USERPROFILE% in a key path. These probe the program-name, drive-letter (versus host:) and %VAR% readings.
• any: remote ssh with -p, with -o options, with -J jumps through remote hosts, and with an -L forward whose far end is localhost. Also ssh-add -l, a git clone over ssh, and two neighbouring floors in the same loop: a feature-branch git push and kill.

The diff itself adds no new platform assumption. It moves code that already handled the Windows hosts file and adapter-address paths, and both host spellings are probed above. No assumed platform is left uncovered.

[SCOPE-REVIEWED] 4392cc5

[iamwhatever]

Thanks for this refactor, @buluoray! Main has since moved the ssh self-target floor out of argv_floor.py in #21952 (commit d5eb10e), which split it into src/kiro_crew/security/argv_floor_ssh.py behind a facade. Closing this draft as superseded. If there's a part of this change that #21952 doesn't cover, please reopen and we'll take another look.