fix(security): scan doctest examples in docstrings as code

[javenciu]

Problem / Motivation

doctest executes the >>> examples inside docstrings, but no pass of the source-body vet ever scans those examples as code. A dangerous command spelled inside an example sails through, while the identical string as a plain literal is blocked. Measured on main (3006935):

def helper():
    """Sync helper.

    Example:
        >>> subprocess.run("grep -r secret ~/.kiro/crew", shell=True)
    """                                          -> ALLOWED

    >>> os.system("find ~/.kiro/crew -name .env -exec cat {} +")
                                                 -> ALLOWED

cmd = "grep -r secret ~/.kiro/crew"              -> BLOCKED (the control: same
                                                    literal in body position)

Why it matters

The vet is a security gate: it exists so a cron/script body cannot smuggle a credential read or fenced-store traversal past review. Examples in docstrings are a runnable position (python -m doctest, pytest --doctest-modules), so the gap is a real bypass shape, not a formatting nit — and the inconsistency (blocked as a literal, allowed one docstring away) is exactly the kind of seam an author can trip accidentally or exploit deliberately.

What changed (motivation → approach → change)

• Symptom: the probes above — example code never convicts.
• Root cause: to the literal walk a docstring is ONE opaque string subject. The >>> subprocess.run(...) spelling is Python-call syntax, not shell grammar, so the traversal passes cannot read it; and the inner shell string is text-inside-text, never its own AST constant, so it never becomes a subject of its own.
• Change: _doctest_example_subjects() re-reads every string constant containing >>> with Python's own example parser (doctest.DocTestParser — the same PS1/PS2 grammar the runner uses, no hand-rolled regex) and hands the string literals inside each example's code to the same traversal passes as body-position literals, under the same subject cap. Every fallback errs toward denying: example code that is not valid Python (a shell transcript pasted after >>> ) becomes a raw-text subject; a malformed doctest directive (ValueError from the parser) routes each contiguous >>> /... block — continuation lines included, with the example's own indentation preserved — through the same literal extraction, so breaking doctest syntax cannot buy an exemption for any line of the example. Nested examples are followed to a fixed small depth.

Interplay with open #8811 (docstring subject treatment): this extraction hooks the constant walk and is additive — it does not depend on docstring-position constants remaining whole subjects, so it composes with either resolution of #8811. If #8811 lands first, this rebases cleanly; the example-literal subjects are unaffected.

Fence-scan wiring (review follow-up, folded into the single rebased commit): the subject extraction closed the asymmetry for the traversal passes, but the escape-aware fence pass (_sensitive_run_in_source_literals) walks tree constants directly, so a docstring stayed one raw string to it: an example literal spelled \x25LOCALAPPDATA\x25\x5c\x5ckiro-cli\x5c\x5cc.json matches no fence pattern as raw text, while the example's own parse decodes it into a read of the real store. Proven end-to-end fails-before against the subject-extraction-only tree: fence pass (True, None) and entrypoint None on the hidden spelling while the body-position twin convicts. The extracted example literals (same extractor — no second parser) now take the same fence check, wired at the constant after its own raw text is checked, so the two layers cannot disagree about which strings exist. The pattern-slot exoneration is not inherited: it is earned by the literal that occupies the slot, and an example literal sits inside that literal rather than in the slot — denying matches the module's direction everywhere an exoneration is in doubt. The spec doc (docs/system-specs/modules/security.md) gains the doctest-extraction paragraph covering both consumers.

Passes-after on the same probes:

PASS  doctest-subprocess       -> BLOCKED
PASS  doctest-os.system        -> BLOCKED
PASS  plain-literal-control    -> BLOCKED   (unchanged)
PASS  benign-doctest           -> ALLOWED   (no new denials)
PASS  benign-print             -> ALLOWED

Tests

Seven tests added to test/test_mcp_cron_security.py (module + seam-owner neighbor test_security_source_command_subject.py: 239 passed at the final tree):

• test_doctest_examples_in_docstrings_are_scanned_as_code — both attack spellings convict, with the body-position literal pinned as the control.
• test_benign_doctest_examples_stay_allowed — the fix adds subjects, not grammar; ordinary examples must not new-deny.
• test_doctest_shell_transcript_example_still_denies — unparseable example code is scanned as raw text (deny-direction fallback).
• test_malformed_doctest_directive_cannot_exonerate — a directive the parser refuses still yields subjects via the same extraction.
• test_malformed_doctest_continuation_lines_cannot_exonerate — a payload on a ... continuation line under a malformed directive convicts at deny-parity with the identical valid-syntax example (malformed input must never be safer than well-formed input).
• test_escape_spelled_fenced_read_in_example_convicts_via_fence_path — fence wiring: the decoded example literal convicts on the fence pass at body-position parity (twin pinned as control), and the entrypoint denies.
• test_benign_examples_stay_clear_of_the_fence_path — fence wiring: the wiring adds a check, not grammar; an escape-spelled innocent path in an example must not new-deny.

Manual verification

N/A — unit coverage sufficient: the seam is a pure function (source text in → verdict out) and the probes above are the manual scenario, pasted with their outputs.

Related Issues

Fixes #8824

Pattern harvest

Pattern: executable content in a documentation position treated as prose by a scanner (docstring doctest examples here).
Rule candidate: review-prompt — "any text position the runtime can execute (doctest examples, doctest.testfile targets, eval'd templates) must reach the same scanner passes as body code."
Adjacent same-class candidate, deferred (one topic per PR): text-file doctests (doctest.testfile / README examples) never pass through this vet either; that is a separate ingestion seam and would be its own issue+PR.

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) — N/A: behavior change is internal to the vet; docstring of the new helper documents it
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

Per the template placeholder (CLA text pending): offered under the same terms as my prior merged contributions to this repository (#8835).

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of f81afe3e3d8171b48800086b5fc329fed1d75ff0 via the fork AI-review pipeline — 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.

Both wiring sites check out against the base: the fence-path loop lands as a sibling of the raw-value check inside _sensitive_run_in_source_literals (deny-only addition), _parse_source_body returns None on unparseable input as the fallback assumes, and the base has no prior doctest-example scanning (3 doctest hits in security.py, all the reflection-audit name). One gap between the description and the diff: the description's "What changed" names only the traversal-pass wiring and "five tests added", while the diff ships a second wiring (the fence check on decoded example literals) plus seven tests. I verified the second wiring is not redundant with the first — traversal subjects never take the pass-1b fence check, so both are load-bearing.

First-Principles-Verdict: CONCERNS

The fix is real and every piece earns its place, but half the shipped wiring — the fence-path check and two of seven tests — appears nowhere in the description.

What this change ships

Intent: a dangerous command inside a docstring's >>> doctest example must fail the script vet the way the same command fails in body position — a FIX (#8824).

1. Doctest example literals now convict via the traversal/env subject walk — justified (declared, probe-measured)
2. Escape-spelled fenced reads inside examples now convict via the literal fence check — undeclared ("hands the string literals … to the same traversal passes"; "Five tests added" vs seven shipped)
3. A malformed doctest directive no longer exempts the example's lines — justified (deny parity, pinned by control test)
4. Unparseable example code is scanned as raw text — justified (deny direction)
5. Nested examples followed to depth 3, inner text kept as a plain subject past it — justified bound
6. Example literals count toward the same 1024-subject cap, so example-dense bodies can now refuse — declared, justified (else the cap is the bypass)
7. Extraction also runs on non-docstring literals containing >>> — undeclared nuance, deny-only, needed for the withdrawn-exclusion branch
8. security.md section in the same commit — mandated by AGENTS.md

Watch

• Item 2 is a second scanner wiring the description never mentions. Not duplication (grepped doctest in base src/: 3 hits, all the _DOC_REFLECTION_NAMES audit) and it is not covered by wiring 1 — but a human should confirm the escape-spelling half matches doctest >>> examples in docstrings are executable code but are never scanned as code (pre-existing, all treatments) #8824's reported scope rather than widening it silently.
• The author's own harvest rule names 3 same-cause positions (doctest examples, doctest.testfile targets, eval'd code strings): 1 fixed here, testfile explicitly deferred, the eval'd-string seam named in the rule but neither fixed nor deferred as an issue.

[FIRST-PRINCIPLES-REVIEWED] f81afe3

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of f81afe3e3d8171b48800086b5fc329fed1d75ff0 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The seams check out: doctest is already in _DOC_REFLECTION_NAMES (so a body that could self-execute its examples already withdrew the docstring exclusion), but the escape-spelled fenced read inside a raw docstring's example — decoded only by the example's own parse — was invisible to every raw-text treatment on main, so the gap is real and the parser-based extraction is the only shape that closes it. The extraction hooks the existing constant walk in both consumers, inherits the subject cap deny-direction, fails toward denying on unparseable/malformed input, withholds the pattern-slot exoneration, and the spec is updated in the same commit. The stronger alternative (withdraw the docstring exclusion for any >>>-bearing docstring) would reintroduce the #8643 prose false positive and still miss the escape-decode case, so this design wins. No design-level findings survive.

Design-Verdict: PASS

Closes a real decode-gap bypass at the right seam, deny-direction throughout, and beats the whole-docstring alternative on both false positives and coverage.

[DESIGN-REVIEWED] f81afe3

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed c73a82bc05bce66abc64e460951d64e7a55ae63b via the fork AI-review pipeline; updated in place on each push.

1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- src/kiro_crew/security.py:11356 -- Depth exhaustion reopens the doctest credential-path bypass
if _depth >= 3 or ">>>" not in value:
return []
Three nested doctest wrappers -> extraction returns empty before decoding the payload -> cron vet allows a protected-path read.
Anchor: backend-security-controls
Fix: Signal depth exhaustion and deny the source body instead of returning an empty list.
[BLOCK-MERGE] c73a82b
[GPT-REVIEWED] c73a82b

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

I've read the prompt, the findings, and the diff. The adjudicable block is empty (0 findings). One fenced finding, F1, requires a ruling.

F1 analysis — _doctest_example_subjects at security.py:11356 caps recursion with if _depth >= 3 or ">>>" not in value: return []. Tracing the mechanism from the diff: at each level a discovered literal is appended and fence-checked as its own value/collapsed copies, and recursion only descends to find further nested doctest examples inside it. A payload escape-spelled (\x25LOCALAPPDATA...) inside a literal only decodes to a fence-matching path when that literal's example is parsed. At _depth==3 the value is returned unparsed, so a literal C found at depth 2 is fence-checked only as raw text (where \x25 matches no pattern) and its inner example literal D is never extracted/decoded. The doc claim "inner text was already appended one level up" does not defend against this: the appended value C is the escape-encoded text, not the decoded path.

The conditions (a body with ~4 levels of nested doctest examples, escape-spelled at the deepest) are entirely producible by the author of a malicious cron script — no timing contradiction, no legacy-only artifact, no input the system's writer cannot generate. Hiding a payload behind added nesting is exactly what a motivated attacker does. Harm is a governance-ceiling / protected-path read via cron vet — unbounded, security-class. I cannot complete a rarity record showing the combination is unproducible, so the FLAG bar is not met.

Fenced harm: UNBOUNDED (ceiling bypass, protected-path read). Conditions confirmed at security.py:11352 (_depth >= 3 skip) and the fence-check-of-raw-value path at security.py:11588 (_fence_hit_in_collapsed(example_value)), neither of which decodes the deeper level. Attacker-producible, so no acceptable-residual-risk argument exists → UPHOLD-FENCED.

[ADJUDICATION] c73a82bc05bce66abc64e460951d64e7a55ae63b total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] c73a82bc05bce66abc64e460951d64e7a55ae63b
[ADJUDICATION-FENCED] c73a82bc05bce66abc64e460951d64e7a55ae63b fenced=1 flagged=0
UPHOLD-FENCED F1 src/kiro_crew/security.py:11356 -- Depth-3 cap leaves an escape-spelled fenced read inside a nested doctest example unparsed and only raw-text-checked, and the deep nesting is fully producible by a malicious cron-script author, so the ceiling-bypass risk is not acceptable residual risk.
[GPT-ADJUDICATED-FENCED] c73a82bc05bce66abc64e460951d64e7a55ae63b

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed f81afe3e3d8171b48800086b5fc329fed1d75ff0 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] f81afe3

[javenciu]

CI note: the Backend Tests shard-1 failure is test_approval_threading.py:219 (the event-loop-closed concurrency class), which has no overlap with this diff (doctest subject extraction in security.py plus its tests only). It passes 5/5 locally at this head, including with current main merged in. Happy to rebase to re-roll CI if that is preferred.

[javenciu]

Thanks — this residual was real, and the fence-scan wiring now on the branch (f81afe3e3) closes it.

• Fence scan wired: each extracted example literal now takes the decoded-literal fence treatment it would get in body position — after the literal's own raw-text check it runs through the collapsed-fence check, deny-direction only. Slot exoneration is deliberately not inherited: that status is earned by the slot occupant, not by an example literal that happens to sit inside it.
• Conviction pinned with fails-before: an escape-spelled fenced-leaf read inside a raw docstring's example (the exact escape-class asymmetry you traced) is now BLOCKED with body-position parity. Fails-before proven at commit 1: the extractor already yielded the decoded constant — it just never reached the fence check. A benign escape-spelled control stays ALLOWED.
• Spec doc updated in the same commit: docs/system-specs/modules/security.md now documents the example-literal subject collection for both consumers, so the "documented exhaustively" convention holds and the earlier N/A checklist claim is corrected.

The deferred doctest.testfile seam noted in the spec remains out of scope for this PR, as documented.

[javenciu]

Closing this as superseded by upstream direction rather than rebasing.

This PR hardened the docstring-prose exclusion inside security.py's source-body scanning (_reads_dunder_doc and the doctest-example subject extraction), so that executable doctest examples embedded in docstrings were still scanned as code while prose stayed excluded.

Two merged changes have since removed the layer this fix lives in:

• fix(security): stop scanning cron script bodies with the shell command gate #9082 stopped handing cron script bodies to the shell command gate entirely — script bodies are now vetted by linear whole-body detectors (credential-path, secret-env, exfiltration URL), with the sandbox as the runtime fence for what a script may open.
• refactor(security): split security.py into a package and drop path regex #9183 split security.py into a package and deleted the source-body analysis family (is_sensitive_source_body, _source_command_subjects, _sensitive_run_in_source_literals, _parse_source_body). test_the_shell_gate_has_no_source_body_entry_point now ratchets those names as permanently absent, on the reasoning that static text analysis of a Turing-complete body cannot be the fence (cron script body scanned with a shell heuristic: separator-run collapse denies a benign Python source body forever #7912, cron script body scanned as one shell command line: stage-budget refusal permanently blocks every ~512+ line script cron #8563, Docstrings drawn into #8550's traversal subjects re-block real cron scripts #8643, Docstring prose still refused by the tool-input scan (_first_tool_input_denial) — same class as #8643, different entry point #8812 all being false-denial classes it produced).

The gap this PR fixed (doctest examples executing literals that the prose exclusion hid from the scanner) is real in the old architecture, but the old architecture is gone by design: there is no _reads_dunder_doc to harden, and reintroducing any source-body subject extraction would trip the ratchet test. The new mcp_cron._vet_script_contents detectors are whole-body linear matches, so they see doctest literals the same as any other text — the prose/code distinction that created this gap no longer exists in the scan path.

Thanks to the reviewers for the earlier rounds on this one.