idd-all Phase 5: PR body heredoc 改 quoted (預防未來 RCE)

[kiki830621]

Problem

From verification of #1 (P3 finding 17 — security reviewer):
Phase 5 PR body 用 unquoted heredoc:

PR_BODY=$(cat <<EOF
Refs #${N}

## Summary
{從 issue title + diagnosis 的 Strategy 摘要}
...
EOF
)

<<EOF(非 <<'EOF')讓 shell 對內容做 variable expansion + command substitution。目前 ${TITLE} / {...} placeholder 是註解性質沒實際插入,但若未來 maintainer 真的把 ${TITLE} 寫進 body,issue title 含 backticks (`rm -rf /`) 或 $(...) 就會 RCE。

Pre-existing — <<EOF 在 v2.40.0 之前就這樣寫;#1 verify 順手 catch。

Type

fix / security defensive (future-proofing)

Why P3 (latent)

• 目前 placeholder 是字面 {...} 不是 ${...},沒實際 expansion
• 但 placeholder 用法看起來像 template,maintainer 容易誤以為可以直接塞 ${TITLE}
• 用 quoted heredoc 是純 win-win:預防誤用 + 標示 "這裡的 $ 是字面"

Recommendation

改成 quoted heredoc,然後在外面用 printf 或 envsubst 做 controlled substitution:

TEMPLATE=$(cat <<'EOF'
Refs #__N__

## Summary
__SUMMARY__

## Verification
6-AI cross-model verification PASS。詳見 issue #__N__ 的 Verify comment。
...
EOF
)

PR_BODY=$(echo "$TEMPLATE" | sed -e "s/__N__/$N/g" -e "s/__SUMMARY__/$(escape_sed "$SUMMARY")/g")

或繼續用 expansion-allowed heredoc 但加註釋警告未來 maintainer:

# WARNING: <<EOF (not <<'EOF') — variable expansion happens.
# DO NOT insert untrusted strings (e.g. issue titles) without sanitization.
PR_BODY=$(cat <<EOF
...
EOF
)

Source

• Verify FAIL on commit 98b7a71 (idd-all: 增加 HITL（attended）模式 — direct-commit + 允許 sub-skill AskUserQuestion #1)
• Security reviewer finding 4 (pre-existing, latent)
• Master comment: idd-all: 增加 HITL（attended）模式 — direct-commit + 允許 sub-skill AskUserQuestion #1 (comment)

Related: #1

[kiki830621]

Closing Summary

Problem

Phase 5 PR body 用 unquoted heredoc:

PR_BODY=$(cat <<EOF
Refs #${N}
...
EOF
)

<<EOF(非 <<'EOF')讓 shell 對內容做 variable expansion + command substitution。目前 ${TITLE} / {...} placeholder 是 markdown 註解性質沒實際插入,但若未來 maintainer 真的 substitute issue title 含 backticks (`rm -rf /`) 或 $(...) 進 body,就 RCE。

Pre-existing latent risk。

Root Cause

Heredoc 模式選擇沒有 reflect 「不該有 user-input expansion」的設計意圖。

Solution

在 heredoc 上方加 inline WARNING comment:

# WARNING (#7): heredoc below is NOT quoted — variable expansion happens.
# Do NOT insert untrusted strings (e.g. issue titles) into the body without
# sanitization. Current placeholders {...} are markdown-style note text, not
# ${...} shell expansions, so safe today; future maintainers must keep it that way.

不改 heredoc 本身(改 quoted 會影響 ${N} 真實 substitution),但讓 hazard 對 future maintainer 顯眼。

Verification

Smoke test 不涉及 Phase 5(那是 destructive op);comment-only change 對其他流程零影響。

Changes

• a7a4e5e fix: idd-all/idd-issue P2/P3 follow-up batch (Refs idd-all: direct-commit mode 加 passive warning (dirty tree / on default branch) #2 idd-all SKILL.md: pseudocode quality (goto bash / branch-exists no-op / stale Failure Modes table) #3 idd-all Phase 4: ${MAX_ROUND} substitution missing in abort message #4 idd-all Phase 0.5: fork detection 改 fail-closed (gh repo view 失敗時預設 PR mode) #5 idd-all SLUG derivation: BSD/GNU sed alternation (\| not portable) #6 idd-all Phase 5: PR body heredoc 改 quoted (預防未來 RCE) #7 idd-all Phase 0.2: GITHUB_REPO post-derive shape assertion #8 idd-issue Step 2: 強制 sanitize incoming title (length cap / strip control chars) #9 idd-all Examples: 跨 repo invocation 段重複描述,合併 #10)