[bolichen97]

Problem / Motivation

Goal: Finish bringing test/test_denied_commands_security.py under the 2,000-line cap by behaviour under test, without changing a single test; this second PR moves the 4,990 lines the first PR left.

After the first PR of this series (#21858) the file is still 6,549 lines, more than three times the cap the line-cap campaign enforces, and every deny-gate test added to it pushes it further past.

Why it matters

The maintainers' rule is that no hand-written file may exceed 2,000 lines. This PR finishes the split, so the file leaves the line-cap baseline and each deny family's tests can be found by file name and grow in a file of their own.

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).
• Re-splitting the parts the first PR created; only _cost gains classes, inserted above its own.

What changed (motivation → approach → change)

The original keeps its header and every class up to TestIsDeniedReDoSResistance: the catalog and its count pin, flag interposition, row scoping, the resolver, dual-tier matching, gap splitting, user-pattern semantics and the ReDoS budget. Everything after it moves, by the same recipe as the first PR, into five new topic files, and the remaining cost classes go into the existing _cost part above its first class, in their BASE order. The shared helpers already live in denied_commands_security_helpers.py, so it does not change.

File	Lines	What it pins
test_denied_commands_security.py	6,549 → 1,560	catalog, resolver, dual-tier matching, ReDoS budget
_rows	566 (new)	refusal notes, floor additivity, rule identity, reverse-shell-nc, the dev-mode confirm flag, the recursive-force rm floor
_mentions	790 (new)	a product name or permission verb as data is a mention, not an invocation
_kill	1,271 (new)	the self-kill floor: command boundaries, kill-target scoping, the argv and raw windows
_mint	856 (new)	the credential-mint floor: interpreter argv literals and segment scoping
_stdin	755 (new)	a stdin-reading interpreter's program, output redirects, redirect-target quoting
_cost	547 → 1,418	gains the user-regex ReDoS gate, the built-in matcher shape, the floor short-circuit and the two linear-scan classes

Glue, the only lines whose AST or comments change:

• Each new file has its own module docstring, and _cost's docstring is rewritten to cover what it now holds. Every new file copies pytestmark = pytest.mark.usefixtures("fresh_darwin_libc_handles") with the family wording, so each test keeps the fixture set it had.
• 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 file, as in the first PR, which takes it from 64 to 69 rows (the size band's ceiling is 70 since test: split test_denied_commands_security.py by behaviour (1 of 2) #21858).
• The file's row in .github/file-line-cap-baseline.txt is deleted, because the file is now under the cap.
• Two citations name the file that now holds what they point at: test_denied_rule_seam.py (_CATASTROPHIC, in _cost) and docs/system-specs/modules/security.md (TestSelfProtectionFloorIsAdditive, in _rows).

Backwards compatibility

Compatible: test-only move. Every collected test keeps its class, name and parameters (the multiset is identical before and after), and 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 over the whole family (original + every part)	1,371 = 1,371; the (class, name, params) multiset is identical
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 family at BASE (-n0) and HEAD's 1,371 node ids at -n0 in a seeded shuffled order	1,370 passed, 1 skipped each (the same skip)
the related backend set (run_scoped_tests.plan: 39 files) + every reader of golden-paths.json + the wait-discipline, darwin-libc and line-cap ratchets, -n 3 (44 files)	7,026 passed, 2 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 one moved BASE finding, marked in its new home
mutation proofs for the list edits	a new corpus row naming a missing part, and the line-cap row left in place, 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, the campaign's verify_test_split.py	all exit 0

Move audit

test/test_denied_commands_security.py: 6549 -> 1560 lines; moved 100.0% (python-ast)
   changed: (none)

Every test function, method and helper moved with an identical AST; verify_test_split.py finds every BASE unit unchanged at HEAD. git diff BASE...HEAD is 530,923 bytes.

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

• kirocrew.test-autouse-shared-monkeypatch (marker between decorator and def): TestReverseShellNcIsCommandTokenAnchored._remote_rsync_targets_are_not_this_host (_rows).
• # flake-ok: only: the three 5 s ReDoS-budget ceilings of TestUserRegexReDoSGate and the 0.5 s backstop of TestSelfModuleIndexIsLinear (_cost), and the 0.2 s backstop of TestPythonStdinDetectorStepsOverOutputRedirects (_stdin).

No credential-shaped literal, PEM header, internal host, home path or internal ticket id is on any added line.

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 1/4990, _cost 874/3 (its docstring and imports), every new file, the row edits and the citations match too.

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 new part file (test/test_denied_commands_security_{rows,mentions,kill,mint,stdin}.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 64 to 69 rows, inside the band's ceiling of 70.
• .github/file-line-cap-baseline.txt: the row 11203 test/test_denied_commands_security.py is deleted (the file is 1,560 lines).

Citations

• test/test_denied_rule_seam.py:123: _CATASTROPHIC and the ReDoS budget → test_denied_commands_security_cost.py.
• docs/system-specs/modules/security.md: test_denied_commands_security.py::TestSelfProtectionFloorIsAdditive → test_denied_commands_security_rows.py::TestSelfProtectionFloorIsAdditive.
• 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, security/denied_rules.py's pointer to the ReDoS tests (TestIsDeniedReDoSResistance stays); class-name citations 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: fe1c174
• What rule changed: no rule changes; the security-conductor corpus gains five kind: test rows, one per new part file of the second half of the split deny-fence test file, so the fence's own assertions stay in the corpus verify_fix runs.
• Before -> after: before, six rows ran the family of 1,371 deny-fence tests, 908 of them through the original row; after, that row runs the 76 that stay, and the 832 that moved run under the five new rows for test/test_denied_commands_security_{rows,mentions,kill,mint,stdin}.py and the existing _cost row.
• 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 69 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 allows 70 rows since test: split test_denied_commands_security.py by behaviour (1 of 2) #21858.
• How to undo: delete the five added rows from golden-paths.json; the original row and the first five part rows still run their tests.

[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 fe1c174.

[github-actions]

UX Review (Opus 5.5) — ⏭️ skipped

Revision fe1c174410ec1aa9bd88f170942003445a539172 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]

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of fe1c174410ec1aa9bd88f170942003445a539172 — 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

Confirm that the five new golden-paths.json rows keep the security-conductor running every moved deny-fence test that the original single row used to cover.

What this change ships

Inventory (6 items) — 6 justified

Intent: bring test_denied_commands_security.py under the 2,000-line cap that AGENTS.md requires, without changing any test. This is a FIX (a cap violation).

1. The deny-gate tests that were in one file now live in five topic files plus the existing _cost file. — justified
2. The file's row is removed from the line-cap baseline. — justified
3. The security-conductor corpus gains one test row for each new file (64 → 69). — justified
4. The pointer in security.md now names TestSelfProtectionFloorIsAdditive's new file. — justified
5. The test_denied_rule_seam.py comment now points at _cost for the ReDoS budget. — justified
6. Own-line # nosemgrep: / # flake-ok: markers sit above the moved timing and semgrep sites. — justified

[FIRST-PRINCIPLES-REVIEWED] fe1c174

Description read

[DESCRIPTION-READ] bb5f5d4ba83812f06bb7a202753e3bfb0812171eb23082a09e99678663d7e7b3

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]

Design Review (Opus 5.5) — ✅ PASS

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

Design-Verdict: PASS

Nothing to check.

[DESIGN-REVIEWED] fe1c174

Description read

[DESCRIPTION-READ] fa88395f3f9092bf3166b474a22f1d1db160dd807353f6f645d351e628d014ba

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]

Opus 5.5 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] fe1c174

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

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

[github-actions]

GPT 6.1 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] fe1c174

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

[github-actions]

Security Scope Review -- ✅ PASS

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

No candidate flipped from allowed to refused.

Scope-Verdict: PASS

TIGHTENING: None. This change is a test-file split, not a stricter rule. The patch changes no file under src/kiro_crew/security, hooks.py or any other guard. The touched files are: .github/file-line-cap-baseline.txt (the -11203 test/test_denied_commands_security.py row is removed), one doc reference in docs/system-specs/modules/security.md (test_denied_commands_security.py::TestSelfProtectionFloorIsAdditive becomes test_denied_commands_security_rows.py::TestSelfProtectionFloorIsAdditive), six new "kind": "test" rows in golden-paths.json for the split files (_rows, _mentions, _kill, _mint, _stdin, plus the existing _cost), and test classes moved out of test/test_denied_commands_security.py into those files. The patch also prunes now-unused imports (_HYPH, _KA, Q, _rule_pattern, is_safe_user_regex, _argv_floor, _perm_verb_mention, _GIT_PUBLISH_RULE_PATTERNS) and updates a comment path in test/test_denied_rule_seam.py. None of the three checks (sensitive-bash, exfil, deny-rules) changes, so no operation can be newly refused. I found no embedded instructions in the diff.

ADJUDICATED: 14 candidates, 0 confirmed newly-refused. The harness returned no confirmed rows. The candidates target the area whose tests moved: local pytest runs of the new _kill, _mint, _rows, _stdin and _mentions files; read-only searches of those files for the quoted kirocrew token and pkill -f kirocrew fixture strings; history and diff checks on the moved paths; a pkill -f aimed at a hung pytest run, not the product; and lint over the pruned imports. Since the composite is byte-identical between base and head, every one of them should classify the same at both refs.

PLATFORM COVERAGE: The diff makes no new assumption about paths, separators, argv shape, interpreter, executable name or environment variables, because no matcher changes. I still wrote spellings for two platforms:

• posix: .venv/bin/python -m pytest test/..., grep, git, wc, pkill -f and flake8 rows.
• windows: .venv\Scripts\python.exe -m pytest test\... rows with backslash separators and the .exe name, plus a PowerShell Select-String -Path test\... row.

There are no any rows, because every candidate carries a path or an executable name. No platform the diff assumes is left without a candidate.

[SCOPE-REVIEWED] fe1c174

[bolichen97]

• First Principles punchline: every moved deny-fence test still has a corpus row

Confirmed at this head. Every file of the family has exactly one kind: test row in golden-paths.json: the original plus all ten parts. That is 11 of the corpus's 69 rows, and no part file is left without a row.
Those 11 files collect 1,371 tests, the same (class, name, params) multiset as the single file before the split, so the rows together select every test the original row selected.
TestTheShippedCorpusIsALiveGate::test_every_shipped_test_row_resolves_in_this_repository passes, and a row naming a missing part turns it red (mutation proof in the PR body).