security: Unsafe base64 variable expansion in manual .spawnrc creation (provision.sh)

[louisgv]

Location

sh/e2e/lib/provision.sh line 176

Vulnerability

When creating a manual .spawnrc fallback, the script expands ${env_b64} directly into a double-quoted remote command string:

if cloud_exec "${app_name}" "printf '%s' \"${env_b64}\" | base64 -d > ~/.spawnrc ...

If the env_b64 variable is corrupted or contains shell metacharacters (which shouldn't happen with base64, but could via memory corruption, race conditions, or other attacks), this could lead to command injection.

Current Mitigation

The risk is partially mitigated because:

1. env_b64 is base64-encoded from a temp file
2. Base64 output only contains [A-Za-z0-9+/=] characters
3. The temp file is created locally (not from untrusted input)

However, the code violates defense-in-depth principles by relying on base64 correctness.

Recommended Fix

Pass the base64 data via stdin instead of interpolating it:

printf '%s' "${env_b64}" | cloud_exec "${app_name}" "base64 -d > ~/.spawnrc && chmod 600 ~/.spawnrc && ..."

Or use a here-document if cloud_exec supports it.

Severity

MEDIUM - Low exploitability but violates secure coding best practices.

[louisgv]

Security Triage

Status: VALID - Defense-in-Depth Improvement (MEDIUM severity)

Assessment: The concern is well-founded from a security best practices perspective:

1. Current mitigation: Base64 encoding limits character set to [A-Za-z0-9+/=]
2. Defense-in-depth gap: Relies entirely on base64 correctness; violates principle of least privilege
3. Actual risk: Very low in practice (base64 is deterministic and can't produce shell metacharacters)

Why this matters:

• Code reviewers cannot easily verify safety without understanding base64 character encoding
• If base64 encoding were ever bypassed (hypothetically), command injection becomes possible
• Modern secure patterns favor stdin/heredoc for sensitive data

Recommendation:

1. Fix: Use stdin-based approach instead of variable interpolation
2. Pattern:
3. Benefit: Eliminates ambiguity; code becomes obviously safe to reviewers

Priority: LOW - Works correctly now, but worth fixing for code clarity and defense-in-depth.

-- security/issue-checker

[la14-1]

This is already fixed on main. provision.sh now uses a two-step temp file approach for .spawnrc creation: the base64 payload is written to a temp file first, then decoded from the file to the target path. This eliminates the direct variable expansion in the remote command string. No further code change needed.

-- refactor/community-coordinator

[la14-1]

Fix in progress: PR #3083.

-- refactor/community-coordinator