[bolichen97]

Problem / Motivation

Goal: Bring test/test_denied_commands_security.py (11,435 lines) under the 2,000-line cap by behaviour under test, in two PRs, without changing a single test; this first PR moves its last 4,800 lines.

The file is more than five times the size the line-cap campaign allows. A file that large is hard to navigate and review, and each PR that adds a deny-gate test to it pushes it further past the cap.

Why it matters

The maintainers' rule is that no hand-written file may exceed 2,000 lines. This file breaks it today, and every new deny-gate test makes the split bigger. Splitting it by behaviour gives each topic a file of its own that can grow, and makes a deny family's tests findable by file name.

Not a goal

• Changing, renaming, fixing, de-flaking or re-parametrizing any test. Every test moves with an identical AST.
• Changing production behaviour. No production code changes; the security-conductor corpus only gains rows that point at the new files.
• Fixing the pre-existing determinism shapes that move with their tests (they are marked, not rewritten).
• Bringing the original under 1,600 lines in this PR. A single PR would carry a ~1 MB diff, past the review lanes' size limit, so the rest moves in a second PR after this one merges.

What changed (motivation → approach → change)

The original keeps its header and every class before TestDenyMatchingIsQuoteNormalized; the contiguous run from that class through TestHostsFileWarmUp moves into five topic files. TestRecursiveForceRmFloor, which main appended after it, stays in the original until PR 2. The module autouse fixture, the packet-less probe socket, the two captured real functions and the guarded-literal block (_K .. Q, _rule_pattern, _denied_by) move to a non-test denied_commands_security_helpers.py, which every file imports by name. A recipe tool did the move from BASE, so it re-runs after a rebase instead of being hand-merged.

TestSandboxEscapeSshSelf was 1,806 lines, more than the 1,600 a file may hold, so it is cut into two topic classes. TestSandboxEscapeSshSelf keeps its name, its docstring and every test of how the floor reads an ssh-family command line; TestSandboxEscapeSshSelfOwnHost takes the tests of which hosts count as this machine. Both carry the class constant _RULE and the class autouse fixture _pin_own_host_cache, so every test still runs under the same pin.

File	Lines	What it pins
test_denied_commands_security.py	11,435 → 6,549	the catalog, resolver, dual-tier matching, ReDoS budget, and the rule families PR 2 moves
_quoting	1,256	re-spelt words still meet the catalog: quote normalization, nested payloads, empty quoted words
_cost	547	polynomial backtracking keeps the bounded engine; the data-consumer guard is charged per command and per frame
_ssh_self	945	TestSandboxEscapeSshSelf: the rule on both tiers, its note, destinations through quoting, options, user@, globs and expansions
_own_host	1,293	TestSandboxEscapeSshSelfOwnHost (interface seed, netlink publish, resolver worker and warm-up, DNS-alias verdicts, dotless names) and TestHostAddressesPlatformReaders
_hosts_file	934	the hosts-file alias layer and its background warm-up
denied_commands_security_helpers.py	118	the autouse _own_address_probe_stays_local, _PacketlessProbeSocket, _REAL_RESOLVED_HOST_VERDICT, _REAL_SCHEDULE_HOSTS_WARM, the guarded literals, _rule_pattern, _denied_by

The helpers module is under 150 lines because it holds only the module autouse fixture every file must import and the helpers two or more files read.

Glue, the only lines whose AST or comments change:

• The original's module docstring keeps its BASE text and gains one paragraph naming the topic modules and the helpers; each new file has its own docstring. Its pytestmark comment takes the family wording, since no test left in it patches ctypes.CDLL.
• Every part copies pytestmark = pytest.mark.usefixtures("fresh_darwin_libc_handles"), so each test keeps the fixture set it had; _own_host (which patches ctypes.CDLL) keeps the BASE comment, the others say every test of the family resets the caches.
• TestSandboxEscapeSshSelfOwnHost has a new class docstring; in TestSandboxEscapeSshSelf's copy of _pin_own_host_cache, the three comments that said "the ... tests below" name TestSandboxEscapeSshSelfOwnHost instead (comments only, AST identical).
• Locators that stopped being true: the helpers fixture docstring ("in this module" → "in a module importing this fixture") and the guarded-literal comment; TestDeniedReasonNotes' docstring and one comment in it, which pointed at a _denied_by "above" and "every id-based assertion in this file"; one quoting docstring that pointed at "the asymmetry this file already documents" now names TestNameAsDataIsNotAnInvocation.
• Own-line # nosemgrep: / # flake-ok: comments sit above the moved semgrep and timing sites (listed under Move audit).
• The security-conductor corpus (golden-paths.json) gains one kind: test row per new part file, with the same surface, platform and reason as the original's row plus "(part of the split test_denied_commands_security.py)". The original row stays, since the original still holds deny-fence assertions. That puts the corpus at 64 rows (69 after PR 2), so TestTheShippedCorpusIsALiveGate's size band moves its ceiling from 60 to 70, with one sentence on why. The band still rejects a stub or an unreviewed dump.
• test/wait_discipline_baseline.txt: the row's two waits are the gateway-startup polls now in _own_host, so the row moves there (2 → 2).
• test/test_dashboard_server_composition_contract.py:432: the docstring naming the module that polls _OWN_HOST_WARM_TASKS now names test_denied_commands_security_own_host.

Backwards compatibility

Compatible: test-only move. Every collected test keeps its name and parameters; the one class split shows as a new class name for the tests it took (TestSandboxEscapeSshSelfOwnHost::<same test>). Every test sees the same fixtures and markers. No test file imports from the origin, and no production code, import path, patch target or CLI changes.

Removes nothing: every test, fixture and helper still exists, moved with an identical AST. The corpus only gains rows.

Tests

No test was added or rewritten. These proofs show the move lost nothing:

Check	Result
pytest --collect-only at BASE vs HEAD (original + 5 parts)	1,371 = 1,371; the (class, name, params) multiset is identical with TestSandboxEscapeSshSelfOwnHost mapped back to TestSandboxEscapeSshSelf
pytest --fixtures-per-test at BASE vs HEAD	identical fixture set for every one of the 1,371 tests, autouse included
per-test markers (item.iter_markers()) at BASE vs HEAD	identical for every one of the 1,371 tests
the split files: BASE at -n0, HEAD at -n 3, and HEAD's 1,371 node ids at -n0 in a seeded shuffled order (seed 20261017)	1,321 passed, 1 skipped each time (the same skip: the netlink sweep is inert off Linux)
the related backend set (run_scoped_tests.plan: 10 files) + every corpus gate leaf_test_scope.corpus_gates() derives (543 files) + every test that scans the tree through source_corpus + every reader of golden-paths.json + the wait-discipline, darwin-libc and line-cap ratchets, -n 3 (604 files, at an earlier base)	43,275 passed, 131 skipped, 5 xfailed; the one red was the corpus size band, fixed below. At this base, the 25 directly affected files: 2,429 passed, 1 skipped
test/test_flake_pattern_ratchet.py (-n0, against the merge-base)	58 passed
semgrep 1.78 with the CI configs (--baseline-commit <BASE>) over the changed .py files	0 findings, no parse errors; with --disable-nosem exactly the 5 moved BASE findings, each marked in its new home, plus the kept one in the original
mutation proofs for the list edits	a corpus row naming a missing part, the ceiling back at 60, the wait row left on the origin, and _own_host without the libc reset request each turn their guard red
static floor: isort (the CI command), black, flake8, black/subprocess-encoding baselines, comment history, brand, docs-lint, line cap, harness parity, feature map, testpaths, cwd reads, woke	all exit 0

scripts/local-gate.py --base <BASE> --dry-run: backend related set 10 test files (7 in the diff, 3 referencing a changed file), all inside the run above; frontend 372 files, all from the cross-surface set (no website/ file changes).

Move audit

test/test_denied_commands_security.py: 11435 -> 6549 lines; moved 98.15% (python-ast)
   changed: TestDeniedReasonNotes [ClassDef] 114 lines      (class docstring: "``_denied_by`` above" -> "the shared ``_denied_by`` helper"; stays in the original)
   changed: TestDenyMatchingIsQuoteNormalized.test_two_accepted_over_blocks_are_pinned_not_implied [FunctionDef] 26 lines   (docstring: "this file already documents" -> ``TestNameAsDataIsNotAnInvocation``)
   changed: _own_address_probe_stays_local [FunctionDef] 24 lines   (docstring: "in this module" -> "in a module importing this fixture")
   changed: Expr [Expr] 6 lines        (module docstring)

Every other test function, method and helper moved with an identical AST. Both copies of _pin_own_host_cache are AST-identical to BASE (their comments name the sibling class where BASE said "this class's" or "below"). git diff BASE...HEAD is 535,637 bytes.

Moved sites with an own-line marker (the bodies are unchanged):

• kirocrew.test-autouse-shared-monkeypatch (marker between decorator and def): _own_address_probe_stays_local (helpers), both copies of _pin_own_host_cache (_ssh_self, _own_host), TestHostsFileWarmUp._isolate (_hosts_file).
• kirocrew.test-bare-monkeypatch-undo: the per-target monkeypatch.undo() in test_view_construction_never_raises (_quoting).
• kirocrew.test-stdlib-clock-rebound + # flake-ok:: the frozen _argv_floor.time.monotonic in test_negative_dns_verdict_is_revalidated_after_ttl (_own_host); the rule is newer than the code, so the move is its first sight of it.
• # flake-ok: only: the 1 s wall-clock ceilings in _quoting and _cost, the two bounded 10 ms startup polls and the 0.5 s ceiling against a 5 s hold in _own_host, and the 10 ms step of _poll_until in _own_host.

No credential-shaped literal, PEM header, internal host or internal ticket id is on any added line; the only home path is the /home/x placeholder of the empty-word tests.

Diff alignment (L36): for every changed file, git diff BASE -- <file> and git diff --minimal BASE -- <file> add and delete the same lines; the original is 22/4908 (docstring, import and comment glue only).

Logger names (L25): no new module defines a logger.

Path-list edits (L14)

• src/kiro_crew/builtin_skills/security-conductor/golden-paths.json: five kind: test rows, one per part file (test/test_denied_commands_security_{quoting,cost,ssh_self,own_host,hosts_file}.py), surface "S1 bash classifier fence", platform any, the original row's reason plus "(part of the split test_denied_commands_security.py)". The corpus goes from 59 to 64 rows; PR 2 adds five more (69).
• test/test_security_conductor_verify_fix.py:1047: the corpus size band's ceiling 60 → 70, so it admits 64 now and 69 after PR 2; the comment gains one sentence on why. A test row takes one pytest selector, so the rows cannot be folded together.
• test/wait_discipline_baseline.txt: 2 test_denied_commands_security.py → 2 test_denied_commands_security_own_host.py.

Citations

• test/test_dashboard_server_composition_contract.py:432: the module that polls _OWN_HOST_WARM_TASKS → test_denied_commands_security_own_host.
• Left as they are because they stay true: AGENTS.md's pointer to the denied-rule count pin (TestCatalog stays), the count-assertion and pin-style references in test_cli_commands_coverage_agent_security.py and test_spawn_admission_verdict_census.py, test_denied_rule_seam.py's _CATASTROPHIC pointer and security/denied_rules.py's ReDoS-tests pointer (both classes stay in PR 1), and docs/system-specs/modules/security.md's TestSelfProtectionFloorIsAdditive pointer (stays in PR 1); class-name citations of moved classes keep resolving because the class names are unchanged.

CI path references

None: no workflow filter, explicit file list, AUTOSDE.yaml glob or sensitive-change glob names this file.

Manual verification

N/A: unit coverage is sufficient. This is a test-only move, and the collected-test multiset plus the run of every moved test prove it.

Related Issues

Part of #20192

Follow-up: #20383 (pre-existing shapes moved verbatim; de-flake there)

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

[bolichen97]

Sensitive change review

• Head commit: e36a1b7
• What rule changed: no rule changes; the security-conductor corpus gains five kind: test rows, one per new part file of the split deny-fence test file, so the fence's own assertions stay in the corpus verify_fix runs.
• Before -> after: before, one row ran test/test_denied_commands_security.py, which held all 1,371 deny-fence tests; after, that row runs the 908 that stay and five new rows run the 463 that moved to test/test_denied_commands_security_{quoting,cost,ssh_self,own_host,hosts_file}.py.
• Worst case if wrong: a row naming a file that does not exist would make verify_fix answer unverifiable for every fix; TestTheShippedCorpusIsALiveGate refuses such a row at PR time, and a mutation run that misnamed one row turned it red.
• Evidence checked: the five paths exist and collect (1,371 tests in total, identical to BASE), every row has the same surface, platform and reason as the original row, and the corpus loader and verify_fix tests pass with 64 rows.
• No regression:
  • Tools still work: the deny fence is untouched; only test rows are added, and the shell, flow and cron rows are byte-identical.
  • Backward compatible: the original row stays, and verify_fix runs every row it ran before plus the five new ones.
  • Existing tests: test_security_conductor_verify_fix.py, test_security_conductor_golden_paths_about.py, test_deny_diff.py and test_scope_candidates.py pass; the corpus size band now allows 70 rows under a Master grant.
• How to undo: delete the five added rows from golden-paths.json; the original row still runs the tests that stay in test_denied_commands_security.py.

[github-actions]

Sensitive Change Review

Sensitive files changed:

• src/kiro_crew/builtin_skills/security-conductor/golden-paths.json

• @bolichen97 posted a complete reasoning template for e36a1b7.

[github-actions]

UX Review (Opus 5.5) — ⏭️ skipped

Revision e36a1b7270d5be273762f9f28733118c47cd5f9d touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.

[github-actions]

Design Review (Opus 5.5) — ✅ PASS

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

Design-Verdict: PASS

Before merge, check that .github/file-line-cap-baseline.txt (still 11203 test/test_denied_commands_security.py) passes the line-cap ratchet now that the file is 6,549 lines.

[DESIGN-REVIEWED] e36a1b7

Description read

[DESCRIPTION-READ] c76563d4dcdc8909668d0c748eefefbd216db43ab7f19b37c759ce68e1dda95c

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) — 🟡 CONCERNS

Premise-level review of e36a1b7270d5be273762f9f28733118c47cd5f9d — 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: CONCERNS

Splitting TestSandboxEscapeSshSelf into two classes leaves two copies of the 35-line _pin_own_host_cache fixture, justified only by an undocumented 1,600-line limit.

Not justified as shipped

• Item 3 — inherited: the description says the class "was 1,806 lines, more than the 1,600 a file may hold". The only documented cap is CAP = 2000 (scripts/check_file_line_cap.py:114), and a grep for 1,600|1600-line|1_600 finds no such limit. A ~1,850-line file holding the whole class fits under the cap. The split leaves _pin_own_host_cache in both test_denied_commands_security_ssh_self.py:49 and test_denied_commands_security_own_host.py:42, two copies of one pin that will drift apart. It also renames the node IDs of every test it moves.
  Subtraction: delete TestSandboxEscapeSshSelfOwnHost and its copy of the fixture. Keep the whole class in one file, and move TestHostAddressesPlatformReaders to its own part if that file needs room.
  Clears when: the class stays whole under the 2,000 cap, or a documented 1,600 limit is cited.

What this change ships

Inventory (6 items) — 5 justified

Intent: bring an 11,435-line deny-fence test file toward the 2,000-line cap without changing any test. This is an ADDITION (a refactor that only moves code).

1. About 4,800 lines of tests move into five topic files — justified
2. Shared fixtures and literals move to denied_commands_security_helpers.py — justified
3. The ssh-self class is split in two; the moved tests get new class names in their node IDs — inherited (rests on the 1,600 figure)
4. Five new corpus rows, so verify_fix still runs the moved tests — justified
5. The corpus size band ceiling goes from 60 to 70 — justified
6. The wait-baseline row and one docstring now point at the new files — justified

[FIRST-PRINCIPLES-REVIEWED] e36a1b7

Description read

[DESCRIPTION-READ] 31ff4e88c66bd5d794b6c89a759fdfcb339685c95cfe0fac3244db6dd957732d

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 e36a1b7270d5be273762f9f28733118c47cd5f9d — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] e36a1b7

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

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

[github-actions]

GPT 6.1 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] e36a1b7

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

[github-actions]

Security Scope Review -- ✅ PASS

Which legitimate operations does e36a1b7270d5be273762f9f28733118c47cd5f9d 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): 6 classified, 0 newly refused, 6 still allowed.
• ubuntu-latest (platform posix): 6 classified, 0 newly refused, 6 still allowed.
• windows-latest (platform windows): 4 classified, 0 newly refused, 4 still allowed.

No candidate flipped from allowed to refused.

Scope-Verdict: PASS

TIGHTENING: none -- this diff doesn't make any guard stricter. The only non-test file it changes is the corpus, which gets six new kind: test rows: "command_or_flow": "test/test_denied_commands_security_quoting.py" and five sibling parts, added after the existing test/test_denied_commands_security.py row. Everything else moves code within test/. test_denied_commands_security.py is split into five topic parts (_quoting, _cost, _ssh_self, _own_host, _hosts_file), and a new test/denied_commands_security_helpers.py holds the moved autouse _own_address_probe_stays_local fixture, _PacketlessProbeSocket and the guarded-literal helpers. Other changes: one docstring rename, a wait_discipline_baseline.txt rename, and the corpus size ceiling in test_security_conductor_verify_fix.py going from <= 60 to <= 70. Nothing in src/kiro_crew/security, hooks.py, or any of the three deny tiers (sensitive-bash, exfil, deny-rules) changes. So base and head should classify every shell command the same way.

ADJUDICATED: 9 candidates written; the harness returned no confirmed newly-refused rows.

PLATFORM COVERAGE: Rows cover posix, windows and any. The diff adds no new path, separator, interpreter or environment-variable logic to any guard. The candidates check that the maintainers' daily commands on the moved test files still pass the gate in both host spellings:

• posix: .venv/bin/python -m pytest …, cat, grep -n, flake8.
• windows: .venv\Scripts\python.exe -m pytest test\… and PowerShell Get-Content test\….
• any: a read-only git diff --stat scoped to the moved files.

These rows are aimed at the new file names that contain gate-sensitive words (ssh, hosts, own_host), so reading or running those files from an agent shell is checked on both hosts. No platform the diff assumes is left without a candidate.

[SCOPE-REVIEWED] e36a1b7

[bolichen97]

• span=bd2218c68819 rebutted: the topic-class split of TestSandboxEscapeSshSelf

The 1,600-line ceiling is the line-cap campaign's headroom target (#20192): each split file stays well under the 2,000 cap so the next feature PR does not cross it. This family is hot: #21726 added 117 lines to the hosts warm-up class while this PR was open.
The cut follows a real seam rather than arithmetic. TestSandboxEscapeSshSelf keeps how the floor reads an ssh-family command line; TestSandboxEscapeSshSelfOwnHost holds which hosts count as this machine (seed, netlink, resolver worker, DNS-alias verdicts).
The copy of _pin_own_host_cache is AST-identical to BASE, so every test runs under the same pin. A shared base class would be the mixin shape the split doctrine rules out.
The duplication is 35 lines and keeps each class self-contained.

[bolichen97]

• Design punchline: the line-cap baseline row

Checked: scripts/check_file_line_cap.py exits 0 at this head. The file is 6,549 lines, still over 2,000 and under its row (11,203), so the row is neither stale nor exceeded; a shrink is only noted.
Per the line-cap rules the row is deleted by the PR that brings the file under 2,000, which is PR 2 of this series (the original ends near 1,560 lines there).

[mbriones98]

Checks green and readiness passed at e36a1b7; no behavior change.