From 9bf3c216e86cb9c033ebca8a04b802ad98f86ea2 Mon Sep 17 00:00:00 2001
From: A <258483684+la14-1@users.noreply.github.com>
Date: Tue, 10 Mar 2026 10:07:23 -0700
Subject: [PATCH] fix: harden provision.sh against command injection in env_b64
 and app_name (#2444)

- Validate app_name at function entry (alphanumeric, dots, hyphens, underscores
  only) before it's used in file paths or passed to cloud_exec
- Add trap-based cleanup for the temp file used during .spawnrc fallback creation
- Add security comments documenting the three-layer defense model: printf %q
  quoting, base64 encoding, and stdin piping (no interpolation into command
  strings)

The core vulnerability (env_b64 interpolated into the cloud_exec command string)
was already fixed in a prior commit that switched to stdin piping. This change
adds defense-in-depth and documentation.

Fixes #2437, #2441

Agent: code-health

Co-authored-by: B <6723574+louisgv@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 sh/e2e/lib/provision.sh | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/sh/e2e/lib/provision.sh b/sh/e2e/lib/provision.sh
index 35442cb6..e993fba0 100644
--- a/sh/e2e/lib/provision.sh
+++ b/sh/e2e/lib/provision.sh
@@ -20,6 +20,13 @@ provision_agent() {
   local app_name="$2"
   local log_dir="$3"
 
+  # Validate app_name early — it's used in file paths and passed to cloud_exec.
+  # Only allow alphanumeric, dots, hyphens, and underscores.
+  if [ -z "${app_name}" ] || ! printf '%s' "${app_name}" | grep -qE '^[A-Za-z0-9._-]+$'; then
+    log_err "Invalid app_name: must be non-empty and contain only [A-Za-z0-9._-]"
+    return 1
+  fi
+
   local exit_file="${log_dir}/${app_name}.exit"
   local stdout_file="${log_dir}/${app_name}.stdout"
   local stderr_file="${log_dir}/${app_name}.stderr"
@@ -176,8 +183,13 @@ CLOUD_ENV
   # Build env lines in a temp file to avoid interpolating api_key into shell
   # strings directly (prevents command injection if the key contains shell
   # metacharacters like single quotes, backticks, or dollar signs).
+  # printf %q shell-quotes each value; base64 encodes the result; the encoded
+  # payload is piped via stdin to cloud_exec (never interpolated into the
+  # remote command string). This three-layer approach (quoting + encoding +
+  # stdin piping) ensures no user-controlled data enters shell evaluation.
   local env_tmp
   env_tmp=$(mktemp)
+  trap 'rm -f "${env_tmp}"' RETURN
   {
     printf '%s\n' "# [spawn:env]"
     printf 'export IS_SANDBOX=%q\n' "1"
@@ -220,13 +232,17 @@ CLOUD_ENV
   local env_b64
   env_b64=$(base64 < "${env_tmp}" | tr -d '\n')
 
-  # Validate base64 output contains only safe characters (defense-in-depth)
+  # Validate base64 output contains only safe characters (defense-in-depth).
+  # Standard base64 only produces [A-Za-z0-9+/=]. This rejects any corruption.
   if ! printf '%s' "${env_b64}" | grep -qE '^[A-Za-z0-9+/=]+$'; then
     log_err "Invalid base64 encoding"
-    rm -f "${env_tmp}"
     return 1
   fi
 
+  # SECURITY: env_b64 is piped via stdin — it is NOT interpolated into the
+  # remote command string. The command argument to cloud_exec is a fixed
+  # string with no variable substitution from user-controlled data.
+  # The \$ escapes below are for remote shell variables, not local ones.
   if printf '%s' "${env_b64}" | cloud_exec "${app_name}" "base64 -d > ~/.spawnrc && chmod 600 ~/.spawnrc && \
     for _rc in ~/.bashrc ~/.profile ~/.bash_profile; do \
     grep -q 'source ~/.spawnrc' \"\$_rc\" 2>/dev/null || printf '%s\n' '[ -f ~/.spawnrc ] && source ~/.spawnrc' >> \"\$_rc\"; done" >/dev/null 2>&1; then
@@ -234,6 +250,5 @@ CLOUD_ENV
   else
     log_err "Failed to create manual .spawnrc"
   fi
-  rm -f "${env_tmp}"
   return 0
 }