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>
This commit is contained in:
A 2026-03-10 10:07:23 -07:00 committed by GitHub
parent a22fe9010c
commit 9bf3c216e8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -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
}