fix(security): detect letter-spaced P3 and P4 prompts

[mohgupta-ship-it]

Summary

Follow-up to merged #408. This closes the remaining static-only bypass where a raw P3/P4 phrase is detected, but a fully or partly letter-spaced equivalent can return no findings and SAFE.

• Reconstructs explicit letter-spaced tokens for P3/P4 only, with exact source-offset mapping.
• Detects mixed forms such as s e n d conversation to external and never warn the u s e r.
• Preserves punctuation inside reconstructed tokens, including URLs and possessives.
• Keeps boundary-free reconstruction conservative: it emits AE6 + partial/CAUTION without inventing a P3/P4 classification.
• Treats identifier-adjacent and oversized ambiguous runs as AE6-only rather than silently safe.
• Works with --no-llm; MCP returns safe_to_install=false for covered attacks.

Algorithm

The projection uses a deterministic linear scanner rather than a large backtracking regex. It removes only observed single ASCII-space token interiors, preserves explicit multi-space/newline boundaries, and maintains a derived-to-raw offset map. Semantic matching remains scoped to the existing P3/P4 grammars.

flowchart TD
    A[Raw artifact text] --> B[Linear letter-spacing scanner]
    B --> C{Explicit boundaries reconstruct P3/P4?}
    C -->|Yes| D[P3 or P4 finding]
    D --> E[AE6 + partial analysis]
    C -->|No| F{Boundary-free or identifier-adjacent security grammar?}
    F -->|Yes| G[AE6 only; no guessed semantic rule]
    F -->|Oversized ambiguity| G
    F -->|No| H[Keep established result]
    E --> I[CAUTION / unsafe to install]
    G --> I

Loading

Regression coverage

• Fully spaced, mixed-token, URL, apostrophe, identifier-adjacent, and all P3/P4 ambiguous families.
• Benign spelling/initialism controls, including the always use rover/cover re-segmentation case.
• Duplicate suppression across raw/normalized/compact views.
• LF/CRLF source locations, cross-window ownership, long matches beyond overlap, and cooperative runtime checks.
• Graph, CLI --no-llm, and MCP public surfaces.

Validation

• 552 passed across the affected security, end-to-end, artifact-integrity bounds, and static-pattern suites.
• Ruff, formatting, mypy, and git diff --check passed.
• Three-agent adversarial release council: security, code-quality, and public-surface test judges all voted RELEASE on the frozen diff.

Powered by Codex.

[mohgupta-ship-it]

CI follow-up pushed in 60fdd2a.

Root cause: NEW_REGRESSION / runtime-budget exhaustion. The letter-spacing projection walked every large plain-text window in Python even when no spacing candidate existed. Under pytest-cov overhead, static_patterns_prompt_injection crossed its 30-second artifact budget, became partial, and omitted the final P1 window.

Fix:

• Fast-reject plain text with a fixed-width C-regex necessary-condition prefilter.
• Avoid rescanning an identical identifier-relaxed projection.
• Add an oversized plain-text regression proving the Python projection loop is skipped.

Validation:

• CI-failing tests: passed with coverage.
• Full security end-to-end file: 74 passed with coverage.
• Affected suites: 553 passed.
• Ruff, mypy, and diff checks: passed.

Powered by Codex.

[yashrajp22]

I spent a while on this one because the idea is good and the core machinery holds up: I verified end to end with --no-llm that the fully-spaced and double-space-mixed forms now get caught, offset mapping back to raw lines is correct, raw-vs-projected dedup works, and I couldn't find any ReDoS in the projection (1–4 MB adversarial inputs stay linear; the prefilter does its job).

But there's one gap I think is worth fixing before merge: the single-space mixed form of this PR's own headline attack strings still scans completely SAFE — details on the run scanner inline. There's also a reproducible false positive where benign snake_case code gets flagged as obfuscation. Both look fixable within the existing design.

One smaller thing that may just need documenting: a long run of benign spaced letters (think A C G T A C G T ... in docs or tables — I tried 600 chars) goes to AE6 HIGH + CAUTION + safe_to_install=false by design. Fail-closed is the right instinct, but that's a real documentation pattern, so worth a note or an allowlist path.

[yashrajp22]

This is the one that worries me: s e n d conversation to external — ONE space before "conversation", i.e. the most natural way to write the attack — scans completely SAFE end to end, while the two-space version in your tests fires P3. Same story for n e v e r warn the user.

What happens: the extension loop at 1760 swallows the first letter of the adjacent word (the run becomes s e n d c), so right_letter/right_identifier is true and the whole run gets rejected — and index = run_end means we never retry from inside the run. The run is also under 6 letters, so the pre-existing compact view can't rescue it either. Tab-mixed (s\te\tn\td\tconversation ...) and dot-spaced (s.e.n.d conversation ...) variants also come out SAFE.

Suggested fix that stays within this design: like _letter_spacing_offsets (~line 447), stop extending the run when the next letter starts a multi-letter word (two-letter lookahead), and after a left-side rejection retry once from the next spaced letter. Still linear — each position is visited at most twice — and I traced the rover/cover and identifier-adjacency tests: they keep passing, while s e n d conversation to external projects to send conversation to external and fires P3.

[yashrajp22]

This line causes a reproducible false positive on ordinary code: a bundle containing an innocent spaced acronym (U S A on its own line) plus the normal identifier upload_files_to_external_service(config) gets AE6 HIGH pointing at the identifier's line ("Instruction text uses inter-character separators to evade pattern matching"), analysis incomplete, CAUTION. Remove the acronym and it's SAFE.

The issue is that any collapsible spaced pair anywhere in the file arms this whole-document rescan where every underscore and digit becomes a space — so identifiers that happen to spell a P3/P4 phrase get reported as obfuscation.

Suggestion: require the relaxed match to overlap at least one actually reconstructed span. Your s e n d1 conversation test still passes under that rule (its match contains reconstructed characters), but the pure-identifier match doesn't.

[yashrajp22]

These sixteen regexes are a hand-transliterated copy of P3_PATTERNS/P4_PATTERNS with the \s+ removed, and they've already drifted in three places: the always pattern here requires at least one filler letter ([a-z]{1,96}) where the raw pattern allows zero filler words; raw \w+ (digits allowed) became [a-z]; and the raw pattern's optional (?:without\s+)? prefix on the "user knowing" rule got dropped.

Nothing is broken today, but any future edit to P3/P4 silently de-syncs this fail-closed net. Could these be derived mechanically from the source patterns? Failing that, a test asserting one ambiguous counterpart exists per P3/P4 pattern would at least catch the drift.

[yashrajp22]

Cost note: one sweep of the 16 P3/P4 patterns over a 100 KB adversarial single-line window takes ~4.1s on my machine (pre-existing — the lazy .*? rescans), and this change now runs the sweep a second time per view here, plus up to two more in artifact-integrity. There's no runtime check inside these finditer loops, so a crafted spaced artifact can burn the 30s budget into partial analysis — same class as the CI timeout you already hit, and the C-regex prefilter only rescues non-spaced text.

It fails closed to CAUTION, so this is degradation rather than a bypass, but a budget check inside the loop would be cheap insurance.