What happened
PR fullsend-ai#2979 modified post-code.sh (self-described as 'the most security-sensitive component in the pipeline') to add pr_body handling. The fullsend-ai-review bot ran 3 successful reviews producing findings on ADR coherence, duplicate Closes, test gaps, and documentation style — but missed two shell-safety bugs. The waynesun09 Review Squad (3-agent consensus: Claude, Gemini, Codex) identified on Jul 6: (1) echo "${PR_BODY_FROM_RESULT}" interprets leading dashes as flags — if pr_body starts with -e, escape sequences are silently interpreted, corrupting the PR description; (2) unquoted <<RESULT heredoc in SKILL.md examples allows shell expansion of $ and backticks in pr_body content, risking JSON corruption that silently discards the pr_body. Both findings were addressed in the author's final commit.
What could go better
The Review Squad's multi-model consensus caught genuine shell-safety issues that the single-model fullsend-ai-review missed. These are not style nits — echo flag interpretation can silently corrupt data, and heredoc quoting can cause silent JSON corruption in a security-sensitive script. The review agent's correctness sub-agent should include shell-safety heuristics for scripts in the harness/scripts directory, particularly: (a) flagging echo with variable interpolation that could contain user-controlled content, and (b) flagging unquoted heredocs writing structured data (JSON/YAML) with embedded variables. Confidence is medium: these are real bug classes, but the false-positive rate of a general 'echo is unsafe' rule could be high — the rule needs scoping to user-controlled content in security-sensitive scripts. Issue fullsend-ai#3445 already tracks a related shell correctness rule (top-level return in executed scripts), suggesting appetite for this class of improvement.
Proposed change
Add shell-safety heuristics to the review agent's correctness analysis for scripts under harness/scripts directories: (1) Flag echo "${VAR}" patterns where the variable may contain user- or agent-controlled content, recommending printf '%s\n' "${VAR}" instead; (2) Flag unquoted heredocs (<<TOKEN without quotes) that generate JSON or YAML with embedded variable interpolation, recommending jq -n or quoted heredocs (<<'TOKEN'). Scope these rules to post-code.sh, post-fix.sh, post-review.sh and similar harness scripts to avoid false positives on general shell scripts.
Validation criteria
On a test PR that uses echo "${user_content}" in a harness script, the review agent flags the shell-safety concern. On a PR using unquoted heredoc for JSON generation with embedded variables in a harness script, the agent flags the quoting risk. No false positives on echo usage with hardcoded strings or in non-harness scripts.
Generated by retro agent from fullsend-ai#2979
What happened
PR fullsend-ai#2979 modified
post-code.sh(self-described as 'the most security-sensitive component in the pipeline') to add pr_body handling. The fullsend-ai-review bot ran 3 successful reviews producing findings on ADR coherence, duplicate Closes, test gaps, and documentation style — but missed two shell-safety bugs. The waynesun09 Review Squad (3-agent consensus: Claude, Gemini, Codex) identified on Jul 6: (1)echo "${PR_BODY_FROM_RESULT}"interprets leading dashes as flags — if pr_body starts with-e, escape sequences are silently interpreted, corrupting the PR description; (2) unquoted<<RESULTheredoc in SKILL.md examples allows shell expansion of$and backticks in pr_body content, risking JSON corruption that silently discards the pr_body. Both findings were addressed in the author's final commit.What could go better
The Review Squad's multi-model consensus caught genuine shell-safety issues that the single-model fullsend-ai-review missed. These are not style nits — echo flag interpretation can silently corrupt data, and heredoc quoting can cause silent JSON corruption in a security-sensitive script. The review agent's correctness sub-agent should include shell-safety heuristics for scripts in the harness/scripts directory, particularly: (a) flagging
echowith variable interpolation that could contain user-controlled content, and (b) flagging unquoted heredocs writing structured data (JSON/YAML) with embedded variables. Confidence is medium: these are real bug classes, but the false-positive rate of a general 'echo is unsafe' rule could be high — the rule needs scoping to user-controlled content in security-sensitive scripts. Issue fullsend-ai#3445 already tracks a related shell correctness rule (top-level return in executed scripts), suggesting appetite for this class of improvement.Proposed change
Add shell-safety heuristics to the review agent's correctness analysis for scripts under harness/scripts directories: (1) Flag
echo "${VAR}"patterns where the variable may contain user- or agent-controlled content, recommendingprintf '%s\n' "${VAR}"instead; (2) Flag unquoted heredocs (<<TOKENwithout quotes) that generate JSON or YAML with embedded variable interpolation, recommendingjq -nor quoted heredocs (<<'TOKEN'). Scope these rules to post-code.sh, post-fix.sh, post-review.sh and similar harness scripts to avoid false positives on general shell scripts.Validation criteria
On a test PR that uses
echo "${user_content}"in a harness script, the review agent flags the shell-safety concern. On a PR using unquoted heredoc for JSON generation with embedded variables in a harness script, the agent flags the quoting risk. No false positives on echo usage with hardcoded strings or in non-harness scripts.Generated by retro agent from fullsend-ai#2979