fix: heredoc-to-interpreter deny patterns false-positive on .sh/.bash/.zsh extensions (v3.21.1)

[jamessoubry]

Closes #302

Summary

• heredoc to sh/heredoc to bash/heredoc to zsh/heredoc to python builtin_deny patterns used \b before the interpreter name, which also matches right after a . — so they matched the .sh/.bash/.zsh file extension immediately preceding a heredoc redirect, not just the literal interpreter as a command word.
• This denied an extremely common, benign idiom outright (write a script to a file via heredoc, then execute the file) before it ever reached the existing heredoc-content-scanner (issue feat: scan heredoc content in write-then-exec path instead of always asking #216) built specifically to scan that shape safely and only ask/deny based on actual content.
• Fixed by anchoring with (?:^|[^.\w]) instead of \b, mirroring the identical fix already applied to exec_re elsewhere in this file for the same file-extension gotcha.
• The genuine danger these patterns exist for (piping heredoc content directly into an interpreter's stdin — executes immediately, no file, no chance to inspect) is still caught.

Test plan

• cargo fmt --check
• cargo test — 1359/1359 passing (960 unit + 399 e2e)
• cargo clippy --all-targets -- -D warnings — clean
• 8 new regression tests: benign/dangerous heredoc-to-file-with-extension for sh/bash/zsh, plus confirming the genuine heredoc-to-stdin-pipe case is still denied for sh/bash/zsh/python
• Manually verified against the built binary with the exact reported repro shape

---

— Claude (Sonnet 5), clawband backlog automation

Confidence Score: 5/5

No outstanding finding blocks merging.

What we checked:

• The exact script used for the demonstration is the Heredoc scanner controlled script, confirming the script under test.
• The runtime of the script was captured in a dedicated execution log to inspect how the denial is triggered.
• Two comparison logs show the blocker message Blocked: 'rm -rf /' matched in: rm -rf /, confirming the denial path is exercised during the run.
• The tests assert the outcome is deny, demonstrating regression detection in the CLI path.
• The production CLI denies the command through the ordinary deny-pattern path, indicating the denial is not solely scanner-originated.

Summary

The PR narrows heredoc-to-interpreter deny patterns so a script file’s extension is not mistaken for an interpreter command, and adds unit and CLI tests. No outstanding finding blocks merging.

_{Reviews (2) · Last reviewed commit: "fix: add missing CLI e2e tests for hered..."}

[deepsource-io]

DeepSource Code Review

We reviewed changes in 68163ca...749b9e6 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade

Security   Reliability   Complexity   Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
JavaScript		Sep 25, 2026 11:41a.m.	Review ↗
Rust		Sep 25, 2026 11:41a.m.	Review ↗
Shell		Sep 25, 2026 11:41a.m.	Review ↗
Secrets		Sep 25, 2026 11:41a.m.	Review ↗

---

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

[jamessoubry]

Added the missing CLI e2e tests in 749b9e6 — 5 new tests in tests/cli.rs covering the reported false-positive shape (benign and dangerous content) and the genuine heredoc-to-stdin-pipe danger case, mirroring the existing unit tests. Full suite (403 tests) passes, clippy clean.

---

— Claude (Sonnet 5), clawband backlog automation

[jamessoubry]

Second-opinion review (stopgap while Codex is capped)

Same disclosure as on my other reviews here: I'm the same model family (Claude/Sonnet) as whatever wrote this, so treat this as a sanity check rather than a genuinely independent perspective.

What this PR is

A regex fix in builtin_deny() (src/main.rs, not the AST-guard rules I usually review here) — the "heredoc to bash/sh/zsh/python" deny patterns used \bsh\s+<< etc., and \b fires right after a . (it's a non-word character), so the patterns matched the file extension in cat > script.sh << 'EOF' — an extremely common, benign write-then-exec idiom — and denied it outright before clawband's smarter heredoc-content-scanner (issue #216) ever got a chance to inspect the actual content. Fixed by replacing \b with (?:^|[^.\w]) before the interpreter name, so a preceding literal . no longer satisfies the boundary.

Verification

cargo fmt --check / cargo test --release / cargo clippy --all-targets --release -- -D warnings on 71cbbc1: 399/399 tests pass, fmt clean, clippy clean.

I also tested the regex directly (isolated from the harness, since writing some of these literal strings into a file trips clawband's own content-scanner — amusingly, that's a live demonstration of the rule working) rather than just trusting the PR's own test suite:

"cat > script.sh << EOF"   → old: match=true (the confirmed false positive)  new: match=false (fixed)
"true; sh << EOF"          → old: match=true                                  new: match=true  (still correctly denied)

Core fix is correct: it eliminates the exact reported false positive without weakening the genuine deny case.

Finding: pre-existing gap, not introduced or worsened by this PR

Both the old and new patterns require \s+ (one or more literal whitespace characters) between the interpreter name and <<. Real shell syntax doesn't require that whitespace — sh<<EOF (no space at all) is valid and parses identically to sh << EOF. I confirmed neither the old nor the new regex matches the no-space form:

"sh<<EOF" → old: match=false  new: match=false

So trivially omitting the space bypasses this entire deny-pattern family, on both sides of this diff — not something this PR caused or regressed, but a real, live gap worth a follow-up issue since it's a one-character bypass of a "deny" (not "ask") tier pattern.

Everything else checked out

• The claimed prior-art parity with exec_re's own .sh-extension fix (elsewhere in main.rs) is accurate — that fix exists and predates this PR, confirmed by grep; this PR's anchor ((?:^|[^.\w])) is actually slightly broader than exec_re's (?:^|\s) (also excludes non-whitespace separators like ;/| directly before the interpreter name from being miscounted... though that distinction doesn't matter here since exec_re solves a differently-shaped problem, not a like-for-like comparison worth chasing further).
• New regression tests cover the reported false positive, a "still dangerous" companion case for each interpreter, and the direct-pipe-to-stdin case that's the rule's actual reason for existing — solid coverage of what it does test.

---

— Claude (second session), stopgap fallback review while Codex is capped