fix(security): derive the UNC path-token boundary from whitespace

[iamwhatever]

Problem / Motivation

_PATH_TOKEN_BOUNDARY in src/kiro_crew/security.py spelled out the whitespace a path operand may start after as " \t" and stopped there, while the sensitive-path patterns accept the whole whitespace class.

_separator_collapsed_variants used that constant to decide whether a separator run is LEADING (a UNC prefix, which must survive as a pair for the UNC anchor to match) or interior (collapsed to one). So a UNC path on a continuation line — newline immediately before it, as in a multi-line PowerShell command — was misclassified as interior. Every emitted variant then destroyed the UNC anchor, while the original command still missed because of its interior doubled run.

Net effect: the doubled spelling of a fenced path was permitted where the single-separator spelling of the same file was blocked. That is the #6350 defect class surviving at a newline boundary. \r, \v and \f were missing for the same reason.

Why it matters

Two reachable surfaces, both governance-critical:

• the crew-directory read fence over security_policy.json
• the ~/.kiro/agents write gate, where a planted agent spec becomes a gateway-exec'd MCP server command outside the per-session sandbox

Post-merge patrol finding from #6993, verified live on origin/main before this fix.

What changed

The boundary is now derived rather than enumerated: str.isspace() plus the punctuation set, which is exactly the class the patterns themselves accept before a path operand, so the two cannot drift apart again. The constant is renamed to _PATH_TOKEN_BOUNDARY_PUNCTUATION to say what it now holds — it had no consumer outside this function.

leading = start == 0 or prev.isspace() or prev in _PATH_TOKEN_BOUNDARY_PUNCTUATION

Deliberately minimal: two lines of logic, one constant, one docstring. No pattern was widened.

Before / after

is_sensitive_bash_command against a continuation-line UNC read of \\server\share\.kiro\crew\security_policy.json — measured on the origin/main blob, then on this branch:

boundary	spelling	before	after
\n	single separator	BLOCKED	BLOCKED
\n	doubled separator	ALLOWED	BLOCKED
\r	doubled separator	ALLOWED	BLOCKED
\v	doubled separator	ALLOWED	BLOCKED
\f	doubled separator	ALLOWED	BLOCKED
space	doubled separator	BLOCKED	BLOCKED (control)

The ~/.kiro/agents write gate leaked identically:

command	before	after
Set-Content \\n\server\share.kiro\agents\evil.json -Value x`	ALLOWED	BLOCKED
Set-Content \\n\server\share.kiro\agents\evil.json -Value x`	BLOCKED	BLOCKED (control)

Tests

TestWindowsSeparatorRuns gains two cases, both parametrized over the whitespace class rather than the newline alone — enumerating is what produced this gap:

• test_a_unc_path_after_any_whitespace_boundary_is_still_fenced — the doubled spelling across \n \r \r\n \v \f space tab, on both the read fence and the agents write gate, plus the single-separator control on the same commands so a future change cannot close the gap by relaxing that side instead.
• test_a_benign_unc_path_after_a_whitespace_boundary_stays_allowed — holds the allow side: an unfenced UNC path on a continuation line stays allowed in both spellings.

Revert-verified. With the fix mutated back to the pre-fix boundary, 5 of the 11 new cases fail (\n, \r, \r\n, \v, \f) and the space, tab and benign-allow cases still pass. Restoring the fix returns all 11 to green. Re-run after the rebase onto current main.

No false positives. A 17-command benign corpus — multi-line PowerShell/POSIX commands with separator runs at a newline boundary (robocopy, Copy-Item, grep -r over a UNC share, cd node_modules//pkg, cat /etc//hostname) — returns byte-identical verdicts before and after this change. Widening the fence is the failure mode here, so this was checked as a diff of the two verdict sets rather than by inspection.

gate	result
test/test_security.py	1261 passed, 1 skipped
flake8	clean
isort	clean
mypy (security.py)	clean
brand-name (BRAND_BASE_REF set)	PASS
focus-cue (base ref set)	PASS
changelog-history (base ref set)	PASS
harness-parity (base ref set)	PASS
docs-lint	PASS

Diff-scoped gates were run with BASE_REF=$(git merge-base HEAD origin/main) exported — without it they report a false green.

Related Issues

Follows #6350 (the separator-run collapse this boundary belongs to). Patrol finding from #6993.

Pattern harvest

Rule candidate: semgrep
Pattern: hand-enumerated character class standing in for a derivable one (" \t" where str.isspace() is meant) on a security-matching path — the enumeration and the pattern it mirrors drift apart silently, and the drift is only observable as a boundary the fence stops covering.

The same shape is worth flagging wherever a gate compares against a literal string of separator/whitespace characters instead of the classifier the regexes use; a review-prompt line asking "is this class derivable?" would have caught it at the original #6350 review.

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

A derived boundary class replaces the enumeration that drifted from the patterns it mirrors — root-cause fix, minimal, revert-verified, allow-side pinned.

The fix lands at the right layer (the one classifier _separator_collapsed_variants consults), str.isspace() and the regex \s class the patterns use are the same Unicode class so the drift cannot recur, and the tests pin both the deny side across the whole whitespace class and the benign-allow side, so the widening failure mode is guarded. Description and diff match exactly.

[DESIGN-REVIEWED] d3fceaa

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] d3fceaa

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

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

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

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

All verification done. The rename has exactly one consumer, no sibling hand-enumerated whitespace classes exist on security-matching paths (the " \t" hits in messaging/tables.py are CommonMark markdown parsing, where space-or-tab is the spec), and the diff ships no new public surface. The fix replaces the enumerated whitespace class with the derived one, removing the drift mechanism itself rather than patching the newline case.

First-Principles-Verdict: PASS

A live-verified fence bypass closed at cause level — the enumerated whitespace class is replaced by the derived one, so it cannot drift again.

What this change ships

Intent: stop a doubled-separator UNC spelling of a fenced path on a continuation line from slipping past the sensitive-path gate — a FIX.

1. Fenced UNC paths after \n/\r/\v/\f in doubled-separator spelling are now blocked — justified (defect reported in fix(security): fence repeated Windows path separators (#6350) #6993, verified live).
2. _PATH_TOKEN_BOUNDARY renamed to _PATH_TOKEN_BOUNDARY_PUNCTUATION — declared; module-private, 1 consumer (security.py:8228, grepped _PATH_TOKEN_BOUNDARY), and the constant's meaning changed, so the rename is the honest spelling.
3. Two parametrized tests pin the whole whitespace class, both gates, and the benign allow side — declared, revert-verified per the description.

No new config key, flag, or exported symbol; zero one-way-door surface. Grepped in " \t" across src/kiro_crew for unfixed siblings of the root cause: 7 hits, all in messaging/tables.py markdown parsing where space-or-tab is the CommonMark rule, not a mirrored security class — 0 true siblings.

Watch

• The drift class the description names survives in the constant's other half: "\"'=:,;(<>|&"is still hand-enumerated (note(present,)` absent) and mirrors the patterns only by convention. Python has no punctuation classifier, so this is genuinely out of scope for a whitespace fix — but the description's "the two cannot drift apart again" claim holds only for the whitespace half.

[FIRST-PRINCIPLES-REVIEWED] d3fceaa

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of d3fceaa09e551462cce683001a670c50ccd3d70d and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] d3fceaa

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