From 81ab237efebdc1004ca58edb0d4ad6f0b3bf5559 Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Mon, 23 Mar 2026 23:30:47 -0700 Subject: [PATCH] fix(e2e): harden shell scripts against injection in SSH commands (#2945) - hetzner.sh: Pipe base64-encoded command via stdin to SSH instead of embedding it in the SSH command string via variable expansion. The remote bash reads stdin, base64-decodes, and executes. - verify.sh: Add remote-side re-validation of base64 and timeout values in _stage_prompt_remotely and _stage_timeout_remotely. Values are assigned to remote shell variables and validated before writing to temp files, providing defense-in-depth against injection. - provision.sh: Add explicit early rejection of dangerous shell chars ($, `, \) in env var values from cloud_headless_env, and add remote-side re-validation of base64 payload before writing. Fixes #2937 Fixes #2938 Fixes #2939 Agent: security-auditor Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 --- sh/e2e/lib/clouds/hetzner.sh | 17 +++++++++-------- sh/e2e/lib/provision.sh | 14 +++++++++++++- sh/e2e/lib/verify.sh | 36 ++++++++++++++++-------------------- 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/sh/e2e/lib/clouds/hetzner.sh b/sh/e2e/lib/clouds/hetzner.sh index 0d4c2353..21db5e3d 100644 --- a/sh/e2e/lib/clouds/hetzner.sh +++ b/sh/e2e/lib/clouds/hetzner.sh @@ -151,27 +151,28 @@ _hetzner_exec() { return 1 fi - # Base64-encode the command to prevent shell injection when passed as an - # SSH argument. The encoded string contains only [A-Za-z0-9+/=] characters, - # making it safe to embed in single quotes. Stdin is preserved for callers - # that pipe data into cloud_exec. + # Base64-encode the command and pipe the payload via stdin to SSH. + # This eliminates variable expansion of the encoded command in the SSH + # command string, preventing injection even if base64 validation is bypassed. local encoded_cmd encoded_cmd=$(printf '%s' "${cmd}" | base64 | tr -d '\n') # Validate base64 output contains only safe characters (defense-in-depth). - # Standard base64 only produces [A-Za-z0-9+/=]. This rejects any corruption - # and ensures the value cannot break out of single quotes in the SSH command. + # Standard base64 only produces [A-Za-z0-9+/=]. This rejects any corruption. if ! printf '%s' "${encoded_cmd}" | grep -qE '^[A-Za-z0-9+/=]+$'; then log_err "Invalid base64 encoding of command for SSH exec" return 1 fi - ssh -o StrictHostKeyChecking=no \ + # Pipe the base64 payload via stdin to the remote host. The remote bash + # reads stdin, base64-decodes it, and executes the result. No user-controlled + # data is interpolated into the SSH command string. + printf '%s' "${encoded_cmd}" | ssh -o StrictHostKeyChecking=no \ -o UserKnownHostsFile=/dev/null \ -o LogLevel=ERROR \ -o BatchMode=yes \ -o ConnectTimeout=10 \ - "root@${ip}" "printf '%s' '${encoded_cmd}' | base64 -d | bash" + "root@${ip}" "base64 -d | bash" } # --------------------------------------------------------------------------- diff --git a/sh/e2e/lib/provision.sh b/sh/e2e/lib/provision.sh index 181d4da5..83b9c0de 100644 --- a/sh/e2e/lib/provision.sh +++ b/sh/e2e/lib/provision.sh @@ -102,6 +102,16 @@ provision_agent() { continue ;; esac + # Defense-in-depth: reject values containing shell injection characters + # ($, `, \) early, before the broader whitelist check. This explicit + # check makes the security intent clear and catches dangerous patterns + # even if the whitelist regex below is ever relaxed. + case "${_env_val}" in + *'$'*|*'`'*|*'\\'*) + log_err "SECURITY: Dangerous characters in env value for ${_env_name} — rejecting" + continue + ;; + esac # Validate value: only allow characters that appear in cloud resource names # (server names, regions, sizes). This strict whitelist rejects all shell # metacharacters ($, `, ', ", ;, |, &, etc.) preventing command injection @@ -312,13 +322,15 @@ CLOUD_ENV # Step 1: Create a temp file and write base64 data to it on the remote host. # env_b64 is validated above to contain only [A-Za-z0-9+/=] (base64 alphabet), # which cannot break out of single quotes or cause shell injection. + # The remote command re-validates the data as defense-in-depth. local b64_tmp b64_tmp=$(cloud_exec "${app_name}" "mktemp -t spawnrc.b64.XXXXXX" 2>/dev/null | tr -d '[:space:]') if [ -z "${b64_tmp}" ]; then log_err "Failed to create remote temp file for .spawnrc payload" return 1 fi - if ! cloud_exec "${app_name}" "printf '%s' '${env_b64}' > '${b64_tmp}'" >/dev/null 2>&1; then + # Assign to remote variable and re-validate base64 on remote side before writing. + if ! cloud_exec "${app_name}" "_B64='${env_b64}'; printf '%s' \"\$_B64\" | grep -qE '^[A-Za-z0-9+/=]+$' && printf '%s' \"\$_B64\" > '${b64_tmp}' || exit 1" >/dev/null 2>&1; then log_err "Failed to write .spawnrc payload to remote temp file" return 1 fi diff --git a/sh/e2e/lib/verify.sh b/sh/e2e/lib/verify.sh index e3e133e0..c157715c 100644 --- a/sh/e2e/lib/verify.sh +++ b/sh/e2e/lib/verify.sh @@ -48,40 +48,36 @@ _validate_base64() { # _stage_prompt_remotely APP ENCODED_PROMPT # # Writes the base64-encoded prompt to a temp file on the remote host. -# Uses stdin piping so the encoded prompt is never interpolated into a -# command string — eliminating command injection risk entirely. +# The encoded_prompt is validated by _validate_base64 to contain only +# [A-Za-z0-9+/=] characters. The value is assigned to a shell variable +# on the remote side and re-validated there before writing to the file, +# providing defense-in-depth against injection even if local validation +# is bypassed. # --------------------------------------------------------------------------- _stage_prompt_remotely() { local app="$1" local encoded_prompt="$2" - # Write the base64-encoded prompt to a remote temp file. - # The encoded_prompt is validated to contain only [A-Za-z0-9+/=] characters - # (by _validate_base64), so embedding it in a printf command is safe — it - # cannot break out of single quotes or inject shell metacharacters. - # We do NOT use stdin piping here: _hetzner_exec runs commands via - # "printf ... | base64 -d | bash", which connects bash's stdin to the - # base64 pipe rather than to SSH's outer stdin, so piped data never reaches - # the subcommand. - cloud_exec "${app}" "printf '%s' '${encoded_prompt}' > /tmp/.e2e-prompt" + # Assign the validated base64 value to a remote variable, re-validate it + # on the remote side (defense-in-depth), then write to the temp file. + # Base64 chars [A-Za-z0-9+/=] cannot break out of single quotes. + cloud_exec "${app}" "_EP='${encoded_prompt}'; printf '%s' \"\$_EP\" | grep -qE '^[A-Za-z0-9+/=]*$' && printf '%s' \"\$_EP\" > /tmp/.e2e-prompt || exit 1" } # --------------------------------------------------------------------------- # _stage_timeout_remotely APP TIMEOUT # # Writes the validated timeout value to a temp file on the remote host. -# Like _stage_prompt_remotely, this avoids interpolating the value into -# any remote command string — eliminating injection surface entirely. +# The value is assigned to a shell variable on the remote side and +# re-validated there before writing to the file, providing defense-in-depth +# against injection even if local validation is bypassed. # --------------------------------------------------------------------------- _stage_timeout_remotely() { local app="$1" local timeout_val="$2" - # timeout_val is validated by _validate_timeout to contain only [0-9] digits, - # so embedding it directly in the command string is safe — no injection risk. - # We do NOT use stdin piping here: _hetzner_exec runs commands via - # "printf ... | base64 -d | bash", which connects bash's stdin to the - # base64 pipe rather than to SSH's outer stdin, so piped data never reaches - # the subcommand. - cloud_exec "${app}" "printf '%s' '${timeout_val}' > /tmp/.e2e-timeout" + # Assign the validated digits-only value to a remote variable, re-validate + # it on the remote side (defense-in-depth), then write to the temp file. + # Digits [0-9] cannot break out of single quotes or inject shell metacharacters. + cloud_exec "${app}" "_TV='${timeout_val}'; printf '%s' \"\$_TV\" | grep -qE '^[0-9]+$' && printf '%s' \"\$_TV\" > /tmp/.e2e-timeout || exit 1" } # ---------------------------------------------------------------------------