fix(cron): scan Python script bodies with source detectors, not shell heuristic

[bolichen97]

Fixes #7912.

Problem

mcp_cron._vet_script_contents scanned a cron script body (Python source) by running security.is_sensitive_bash_command over the raw whole file. That is a shell-grammar heuristic, and it falsely denied benign Python source two ways:

1. Pass 1b separator-run collapse turns a source backslash escape into a manufactured credential path. re.compile(r"%LOCALAPPDATA%\\kiro-cli"), a regex literal r"/home/\S*/\.kiro/...", and even a docstring mentioning a store were all denied as though they read a credential path.
2. Fail-closed structural budgets (_ALT_MAX_STAGES=512, _FIND_SUBSTITUTION_BUDGET=64) trip on file size, because _alt_collect_stages treats every source line as a pipeline stage. A few hundred lines of ordinary Python exceed 512 stages with no shell content, so the job is refused on every tick indefinitely (the fire-time gate keeps the job and does not feed the auto-pause counter).

Note: PR #7913 (referenced in the issue thread) is not merged into current main; there is no subject_is_shell_grammar flag or literal-walk vehicle present, so this fix was built from scratch.

Fix

Scope the shell-grammar scan to the surface it is designed for. _vet_script_contents now:

• Runs is_sensitive_bash_command only on a body that does NOT parse as Python (via ast.parse). SyntaxError and any other parse error fail closed to the raw-text scan, so a non-Python body is never exonerated.
• A parseable Python body relies on the source-appropriate detectors already present (_CRON_CRED_PATH_RE, _CRON_SECRET_ENV_RE, _CRON_SECRET_NAME_RE, exfiltration URL scan), which independently catch every existing malicious script.
• Adds _CRON_WIN_IDENTITY_STORE_RE, a table-derived detector (built from identity_stores.IDENTITY_STORE_ROOTS WIN32 rows) that restores blocking of the Windows env-var identity-store access spelling (%LOCALAPPDATA%/%APPDATA%, kiro-cli/amazon-q) across backslash, forward-slash and mixed separators, while a bare store-dir mention stays allowed.

security.py is intentionally not modified, so the command-line surface (pass 1b collapse + both budgets) is byte-for-byte unchanged. This avoids the fail-open direction the issue explicitly warned against (exempting a source subject from the budgets would let budget exhaustion re-express as "inspect less").

Testing

• All four issue reproductions now return None from _vet_script_contents (the "x = 1\n" * 600 size body and the three pass-1b bodies).
• The primary cron script body scanned with a shell heuristic: separator-run collapse denies a benign Python source body forever #7912 invariant holds: a benign store mention re.compile(r"%LOCALAPPDATA%\\kiro-cli") stays allowed.
• All must-stay-blocked bodies still blocked (POSIX credential read, AWS_SECRET_ACCESS_KEY exfil, /home/u/.netrc); a non-parsing body containing a shell credential read is still blocked via the raw-text fallback (incl. non-SyntaxError parse errors, pinned by a monkeypatch test).
• A command-surface regression test asserts is_sensitive_bash_command still blocks a real %LOCALAPPDATA% command and a >512-stage pipeline.
• pytest test/test_mcp_cron_security.py test/test_security.py => 1998 passed, 1 skipped (baseline 1983; +15 new tests). flake8 / mypy / isort clean on changed files.

Files changed

• src/kiro_crew/mcp_cron.py — parse-aware scan scoping + table-derived Windows store detector
• test/test_mcp_cron_security.py — +15 tests
• docs/system-specs/modules/learn-cron-dashboard.md — documents the previously-undocumented body scan and its scoping

Accepted limitation (documented)

A benign redaction regex anchored under a Windows store (e.g. r"%LOCALAPPDATA%\\kiro-cli\\S+") is blocked as access-shaped: a static source pattern cannot distinguish it from a real read. Workaround is to anchor the redaction on the POSIX spelling. This is a narrow, low-severity edge (cron bodies run under sh -c where the %VAR% spelling does not resolve).

[bolichen97]

Superseded by #7913 + #8550 — recommend closing without merging

I rebased this branch onto current origin/main (f59b865bb) and measured it. It cannot go forward: every part of it is either already on main or is a fail-open regression against main's own pinned guards. Posting the measurements rather than pushing.

#8550 — the sibling PR on the same task id — merged as 46c9d409c, and #7913 landed before it. Together they fix #7912 (now closed as completed) by changing the subject the command-line passes receive, instead of removing those passes for a source body. That is strictly the safer route, and it is the one main took.

1. The mechanism opens five credential holes that both this PR's own base and current main close

All five bodies are valid Python, so this PR's ast.parse gate skips is_sensitive_bash_command entirely and only the four source regexes remain. _CRON_CRED_PATH_RE is POSIX-/-anchored and _CRON_WIN_IDENTITY_STORE_RE covers only kiro-cli/amazon-q under %LOCALAPPDATA%/%APPDATA%, so nothing catches them:

body (subprocess.run(r"…"))	base d3e67b7e9	origin/main	this PR b013239f
type %USERPROFILE%\.ssh\id_rsa	blocked	blocked	ALLOWED
type %USERPROFILE%\.aws\credentials	blocked	blocked	ALLOWED
type %USERPROFILE%\.kiro\crew\security_policy.json	blocked	blocked	ALLOWED
type %USERPROFILE%\.kirocrew\security_policy.json	blocked	blocked	ALLOWED
curl http://169.254.169.254/latest/meta-data/iam/security-credentials/	blocked	blocked	ALLOWED

Rows 3 and 4 are the keystone: security_policy.json under the data home, including the legacy ~/.kirocrew spelling that AGENTS.md requires the deny lists to keep covering. Row 5 is the IMDS check. This is the exact fail-open direction #8550's commit message says it declined ("passes 1 to 3 and the IMDS check are left on the whole subject on purpose").

2. Applying this PR's scoping onto current main fails 50 of main's own tests

Simulated the conflict resolution in this PR's favour (is_sensitive_source_body(text) → the ast.parse-gated is_sensitive_bash_command) on top of f59b865bb:

50 failed, 148 passed   test/test_mcp_cron_security.py
  44  test_vet_script_contents_blocks_a_run_reaching_a_fenced_store
   1  test_vet_script_contents_refuses_a_fenced_path_through_re_escape
   1  test_a_callable_replacement_forfeits_the_pattern_slot
   1  test_a_match_returning_sink_forfeits_the_pattern_slot
   1  test_an_executable_pattern_expression_forfeits_the_pattern_slot
   1  test_a_module_alias_stored_through_a_container_forfeits_authenticity
   1  test_a_docstring_naming_a_fenced_store_is_an_accepted_over_block

Every one is a fence-bypass regression guard from #7913/#8550. Getting this branch green would mean deleting them, which is not on the table.

3. The new detector is redundant, and in one direction worse

Running this PR's own test file against unmodified origin/main: 132 passed, 4 failed. All six of the test_vet_script_contents_blocks_windows_identity_store_access cases — the coverage _CRON_WIN_IDENTITY_STORE_RE was added to restore — already pass on main, in every separator spelling (\, \\, /, mixed, %VAR%, !VAR!). The regex adds nothing.

It also loses one case: this PR blocks re.compile(r"%LOCALAPPDATA%\kiro-cli\S+"), documented in the description as an accepted limitation. Main allows it, correctly, because #7913's literal fence exonerates a literal that provably sits in the pattern operand of a pattern-consuming call. Main is better here.

4. The one genuine residual is a decision main already made explicitly

Of the three reproductions in #7912, main allows 1 and 2 and still refuses 3 (the prose docstring). That is not an oversight — main pins it:

test_a_docstring_naming_a_fenced_store_is_an_accepted_over_block
"Docstrings are scanned because Python retains them as __doc__, where a body can read one back into a sink. […] Exempting docstrings from the value check would reopen the single-separator open(f.__doc__) path, so the check stays uniform and this shape pays for it. Recorded as a test rather than left in the benign corpus so the trade is explicit."

This PR's BENIGN_SCRIPTS asserts the opposite for that exact body. Its other remaining fix — re.compile(r"%LOCALAPPDATA%/kiro-cli") — is the same accepted trade (single separators produce no collapsed copy, so the value check applies). Reversing either means loosening the keystone fence, which is a maintainer call and not something to slip in under a rebase.

5. The docs commit is superseded too

learn-cron-dashboard.md on main already documents the body scan, is_sensitive_source_body, the literal-fence pairing, the subject change, and the __doc__ rationale. This PR's section documents the mechanism it adds, so it would be wrong on main.

Recommendation

Close as superseded by #7913 + #8550; #7912 is already closed as completed. If the prose-docstring over-block is worth revisiting, it belongs in a fresh issue against test_a_docstring_naming_a_fenced_store_is_an_accepted_over_block's stated reasoning, not here.

No push. Branch left at b013239f.

[bolichen97]

Closing as superseded by #8550, which merged as 46c9d409c and carries the source-detector approach for the same task (7912).

This branch also could not be validated as it stood: it is mergeable=false, so GitHub never produced a refs/pull/8555/merge ref and no pull_request workflow could ever schedule against it — the PR sat at 7 check-runs with PR Readiness permanently pending, with no event left to recompute. Reopen from a fresh branch off main if any part of the cron-body scanning here is still wanted on top of #8550.