Skip to content

write-then-exec ask fallback can't scan non-heredoc content generation (grep/head/cut extraction) and message is misleading about target file #304

Description

@jamessoubry

Gap

Reported by James, follow-up to #302. Distinct from that bug — this is not a false positive, it's a real detection gap.

try_scan_heredoc_content (issue #216) can only find and scan content that's written via a literal << heredoc. Real-world scripts that generate a temp script another way — most commonly by slicing a portion of an existing file with grep/head/tail/sed/cut and redirecting the result to a new file, then source-ing or executing it — fall through to the unconditional-ask fallback: "Compound command writes to a script file then executes it — content cannot be scanned before execution."

Concrete example James hit in practice (paraphrased from a screenshot, not a heredoc at all):

END_LINE=$(grep -n '^build checkin' "$SCRIPT" | head -1 | cut -d: -f1)
head -n $((END_LINE - 1)) "$SCRIPT" > /tmp/checkin_readme_setup.sh
source /tmp/checkin_readme_setup.sh

Two problems with the current behavior here, not just one:

  1. No content scan at all — the ask fallback doesn't attempt to scan anything, even though in this case the source material ("$SCRIPT") usually already exists on disk and could in principle be read at hook-eval time (unlike a heredoc body, which only exists inline in the command text itself).
  2. Confusing message — the ask reason ("content cannot be scanned before execution") reads as if referring to the target file (/tmp/checkin_readme_setup.sh), which doesn't exist yet — it's about to be created by the very command being evaluated. This is misleading regardless of whether detection improves; worth fixing on its own even if scope is kept narrow.

Scope for a fix (needs a design pass, not a blind implementation)

This is a much harder problem than #302 — arbitrary shell pipelines (grep/head/sed/awk/cut in any combination) generating file content isn't something you can generically "scan" without actually simulating the pipeline, which risks false confidence (a wrong simulation could scan the wrong content and give a false sense of safety worse than just asking).

Recommend scoping down to a small, well-understood set of extraction shapes rather than a general solution:

  • head -n N FILE > TARGET / tail -n N FILE > TARGET where FILE exists and is readable at hook-eval time — these are simple enough to literally execute read-only and diff-check safely (just reading N lines, no side effects).
  • Explicitly decide whether sed/awk/arbitrary pipelines are in scope at all, or explicitly out of scope (like the AES string-literal and env-var-string cases were explicitly excluded in ast_guard: add crypto/TLS misconfiguration rules — AES-ECB, TLS verification disabled, Node createCipher #264) — pattern-matching an arbitrary sed expression's output without executing it isn't really AST- or regex-tractable.
  • At minimum, fix the misleading ask message to note the target file doesn't exist yet when that's the case (fs::metadata(target).is_err()), regardless of whether content-scanning itself gets extended — this is a small, safe, high-value fix on its own.

Reference

Sibling issue: #302 (heredoc-to-interpreter file-extension false positive — a different bug in an adjacent code path, already fixed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backlogIn the backlog queue

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions