From de4ccfa0bd257b4751e7fa76ebca8942cc92e1d8 Mon Sep 17 00:00:00 2001
From: fullsend-code
 <278716306+fullsend-ai-coder[bot]@users.noreply.github.com>
Date: Fri, 10 Jul 2026 13:12:24 +0000
Subject: [PATCH] docs(#253): add shell-safety heuristics to review correctness

Add echo-flag-injection and unquoted-heredoc-expansion
heuristics to the correctness sub-agent and the code-review
skill. Both rules are scoped to harness scripts
(post-code.sh, post-fix.sh, post-review.sh) where user- or
agent-controlled content flows through shell commands.

Echo flag injection: echo "${VAR}" interprets leading
dashes as flags, silently corrupting output. Recommends
printf. Unquoted heredoc: <<TOKEN allows shell expansion of
$ and backticks in structured data, silently corrupting
JSON/YAML. Recommends jq -n or quoted heredocs.

False-positive scoping: neither rule fires on hardcoded
strings, quoted heredocs, or non-harness scripts.

Closes #253
---
 .../fullsend-repo/skills/code-review/SKILL.md | 13 +++++++
 .../pr-review/sub-agents/correctness.md       | 39 +++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/internal/scaffold/fullsend-repo/skills/code-review/SKILL.md b/internal/scaffold/fullsend-repo/skills/code-review/SKILL.md
index f67c35a17..31c4a45c7 100644
--- a/internal/scaffold/fullsend-repo/skills/code-review/SKILL.md
+++ b/internal/scaffold/fullsend-repo/skills/code-review/SKILL.md
@@ -89,6 +89,19 @@ dimension carry over to another — each requires its own scrutiny.
   (step 2), determine whether those changes weakened coverage.
 - Split-payload attacks: a production change paired with a test
   modification that masks the real behavior.
+- Shell safety in harness scripts (`post-code.sh`, `post-fix.sh`,
+  `post-review.sh`, and similar scripts under `scripts/`):
+  - **Echo flag injection:** `echo "${VAR}"` where the variable may
+    contain user- or agent-controlled content (PR body, commit message,
+    agent output). A value starting with `-e` or `-n` silently changes
+    behavior — recommend `printf '%s\n' "${VAR}"` instead. Do not flag
+    `echo` with hardcoded strings or in non-harness scripts.
+  - **Unquoted heredoc expansion:** `<<TOKEN` (without quotes) in
+    heredocs generating JSON or YAML with embedded variable
+    interpolation. Shell expansion of `$` and backticks in
+    user-controlled content silently corrupts structured output —
+    recommend `jq -n` for JSON or quoted heredocs (`<<'TOKEN'`). Do not
+    flag quoted heredocs or non-harness scripts.
 
 #### Security
 
diff --git a/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md b/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md
index cb56b9e03..aaab4dce4 100644
--- a/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md
+++ b/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md
@@ -73,3 +73,42 @@ When reviewing technical documentation, verify:
 - **Edge case correctness** — Are described edge cases (depth/breadth
   limits, zero values, error conditions) handled correctly in the
   described logic?
+
+### Shell safety in harness scripts
+
+Harness scripts (`post-code.sh`, `post-fix.sh`, `post-review.sh`, and
+similar scripts under `scripts/`) handle user- and agent-controlled
+content. Two bug classes silently corrupt data and are easy to miss:
+
+**Echo flag injection:** `echo "${VAR}"` interprets a leading dash as
+a flag. If the variable contains user- or agent-controlled content
+(e.g., a PR body, commit message, or agent output), a value starting
+with `-e` or `-n` silently changes `echo`'s behavior — escape
+sequences are interpreted or trailing newlines are suppressed,
+corrupting the output with no error.
+
+Flag `echo` with variable interpolation where the variable may contain
+user- or agent-controlled content in harness scripts as a
+medium-severity finding. Recommend `printf '%s\n' "${VAR}"` instead,
+which does not interpret its argument as flags.
+
+Do not flag `echo` with hardcoded strings (e.g., `echo "Step complete"`)
+or `echo` in non-harness scripts where the variable is demonstrably
+not user-controlled.
+
+**Unquoted heredoc expansion:** `<<TOKEN` (without quotes around TOKEN)
+allows shell expansion of `$` and backticks inside the heredoc body.
+When generating structured data (JSON, YAML) with embedded variables
+that may contain user- or agent-controlled content, shell expansion
+can silently corrupt the output — `$` in a PR body becomes a variable
+reference, and backtick-delimited text becomes command substitution.
+
+Flag unquoted heredocs (`<<TOKEN`) that generate JSON or YAML with
+embedded variable interpolation in harness scripts as a
+medium-severity finding. Recommend either `jq -n` for JSON
+construction or a quoted heredoc (`<<'TOKEN'`) combined with explicit
+variable substitution where needed.
+
+Do not flag quoted heredocs (`<<'TOKEN'` or `<<"TOKEN"`) or heredocs
+in non-harness scripts where all interpolated variables are
+demonstrably not user-controlled.