From 3724bb8ba421f427839821fcade6512cb2e7d22b Mon Sep 17 00:00:00 2001
From: A <258483684+la14-1@users.noreply.github.com>
Date: Tue, 10 Mar 2026 09:27:47 -0700
Subject: [PATCH] fix: address SSH command injection risks in e2e cloud drivers
 (#2447)

Add defense-in-depth validation across all e2e cloud driver scripts:

- Validate IP addresses match IPv4 format before use in SSH commands
  (aws, digitalocean, gcp, hetzner)
- Validate SSH username contains only safe characters (gcp)
- Validate resource IDs are numeric before interpolating into API URLs
  (digitalocean droplet IDs, hetzner server IDs)
- URL-encode app name in Hetzner API query parameter to prevent
  query parameter injection
- Validate numeric env vars (INPUT_TEST_TIMEOUT, PROVISION_TIMEOUT,
  INSTALL_WAIT) that get interpolated into remote command strings

Fixes #2432, #2433, #2434, #2435, #2442

Agent: security-auditor

Co-authored-by: B <6723574+louisgv@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 sh/e2e/lib/clouds/aws.sh          |  7 +++++++
 sh/e2e/lib/clouds/digitalocean.sh | 12 ++++++++++++
 sh/e2e/lib/clouds/gcp.sh          | 13 +++++++++++++
 sh/e2e/lib/clouds/hetzner.sh      | 23 ++++++++++++++++++++++-
 sh/e2e/lib/common.sh              |  5 +++++
 5 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/sh/e2e/lib/clouds/aws.sh b/sh/e2e/lib/clouds/aws.sh
index 60d340e2..335453c8 100644
--- a/sh/e2e/lib/clouds/aws.sh
+++ b/sh/e2e/lib/clouds/aws.sh
@@ -136,6 +136,13 @@ _aws_exec() {
       log_err "Could not resolve IP for instance ${app}"
       return 1
     fi
+    # Validate IP looks like an IPv4 address (defense-in-depth against API/file tampering)
+    if ! printf '%s' "${_AWS_INSTANCE_IP}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$'; then
+      log_err "Invalid IP address for instance ${app}: ${_AWS_INSTANCE_IP}"
+      _AWS_INSTANCE_IP=""
+      _AWS_INSTANCE_APP=""
+      return 1
+    fi
   fi
 
   ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
diff --git a/sh/e2e/lib/clouds/digitalocean.sh b/sh/e2e/lib/clouds/digitalocean.sh
index 58aa9a0f..fcb09b68 100644
--- a/sh/e2e/lib/clouds/digitalocean.sh
+++ b/sh/e2e/lib/clouds/digitalocean.sh
@@ -149,6 +149,12 @@ _digitalocean_exec() {
     return 1
   fi
 
+  # Validate IP looks like an IPv4 address (defense-in-depth against file tampering)
+  if ! printf '%s' "${ip}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$'; then
+    log_err "Invalid IP address in ${ip_file}: ${ip}"
+    return 1
+  fi
+
   ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
       -o ConnectTimeout=10 -o LogLevel=ERROR -o BatchMode=yes \
       "root@${ip}" "${cmd}"
@@ -183,6 +189,9 @@ _digitalocean_teardown() {
     return 0
   fi
 
+  # Validate droplet ID is numeric (defense-in-depth against metadata tampering)
+  case "${droplet_id}" in ''|*[!0-9]*) log_warn "Non-numeric droplet ID: ${droplet_id}"; untrack_app "${app}"; return 0 ;; esac
+
   # Retry DELETE up to 3 times with --max-time to prevent hangs
   local attempt=0
   local delete_accepted=0
@@ -281,6 +290,9 @@ _digitalocean_cleanup_stale() {
     local droplet_name
     droplet_name=$(printf '%s' "${line}" | cut -d' ' -f2)
 
+    # Validate droplet ID is numeric before using it in API URL
+    case "${droplet_id}" in ''|*[!0-9]*) log_warn "Skipping ${line} — non-numeric droplet ID"; skipped=$((skipped + 1)); continue ;; esac
+
     # Extract timestamp from name: e2e-AGENT-TIMESTAMP
     # The timestamp is the last dash-separated segment
     local ts
diff --git a/sh/e2e/lib/clouds/gcp.sh b/sh/e2e/lib/clouds/gcp.sh
index 8c20013e..4294dcc4 100644
--- a/sh/e2e/lib/clouds/gcp.sh
+++ b/sh/e2e/lib/clouds/gcp.sh
@@ -126,6 +126,12 @@ _gcp_exec() {
   local cmd="$2"
   local ssh_user="${GCP_SSH_USER:-$(whoami)}"
 
+  # Validate SSH user contains only safe characters (defense-in-depth)
+  if ! printf '%s' "${ssh_user}" | grep -qE '^[a-zA-Z0-9._-]+$'; then
+    log_err "Invalid SSH user for instance ${app}: ${ssh_user}"
+    return 1
+  fi
+
   # Resolve instance IP (cached per app)
   if [ "${_GCP_INSTANCE_APP}" != "${app}" ] || [ -z "${_GCP_INSTANCE_IP}" ]; then
     # Try reading from the IP file first (written by _gcp_provision_verify)
@@ -143,6 +149,13 @@ _gcp_exec() {
       log_err "Could not resolve IP for instance ${app}"
       return 1
     fi
+    # Validate IP looks like an IPv4 address (defense-in-depth against API/file tampering)
+    if ! printf '%s' "${_GCP_INSTANCE_IP}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$'; then
+      log_err "Invalid IP address for instance ${app}: ${_GCP_INSTANCE_IP}"
+      _GCP_INSTANCE_IP=""
+      _GCP_INSTANCE_APP=""
+      return 1
+    fi
   fi
 
   ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
diff --git a/sh/e2e/lib/clouds/hetzner.sh b/sh/e2e/lib/clouds/hetzner.sh
index 10d674df..80eb0a89 100644
--- a/sh/e2e/lib/clouds/hetzner.sh
+++ b/sh/e2e/lib/clouds/hetzner.sh
@@ -54,10 +54,14 @@ _hetzner_provision_verify() {
   local app="$1"
   local log_dir="$2"
 
+  # URL-encode the app name to prevent query parameter injection
+  local encoded_app
+  encoded_app=$(jq -rn --arg v "${app}" '$v|@uri')
+
   local response
   response=$(curl -sf \
     -H "Authorization: Bearer ${HCLOUD_TOKEN}" \
-    "${_HETZNER_API}/servers?name=${app}" 2>/dev/null || true)
+    "${_HETZNER_API}/servers?name=${encoded_app}" 2>/dev/null || true)
 
   if [ -z "${response}" ]; then
     log_err "Failed to query Hetzner API for server ${app}"
@@ -120,6 +124,17 @@ _hetzner_exec() {
   local ip
   ip=$(cat "${ip_file}")
 
+  if [ -z "${ip}" ]; then
+    log_err "Empty IP in ${ip_file}"
+    return 1
+  fi
+
+  # Validate IP looks like an IPv4 address (defense-in-depth against file tampering)
+  if ! printf '%s' "${ip}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$'; then
+    log_err "Invalid IP address in ${ip_file}: ${ip}"
+    return 1
+  fi
+
   ssh -o StrictHostKeyChecking=no \
       -o UserKnownHostsFile=/dev/null \
       -o LogLevel=ERROR \
@@ -153,6 +168,9 @@ _hetzner_teardown() {
     return 0
   fi
 
+  # Validate server ID is numeric (defense-in-depth against metadata tampering)
+  case "${server_id}" in ''|*[!0-9]*) log_warn "Non-numeric server ID: ${server_id}"; untrack_app "${app}"; return 0 ;; esac
+
   log_step "Deleting Hetzner server ${app} (id=${server_id})"
 
   local http_code
@@ -220,6 +238,9 @@ _hetzner_cleanup_stale() {
     local server_name
     server_name=$(printf '%s' "${entry}" | cut -d: -f2-)
 
+    # Validate server ID is numeric before using it in API URL
+    case "${server_id}" in ''|*[!0-9]*) log_warn "Skipping ${entry} — non-numeric server ID"; skipped=$((skipped + 1)); continue ;; esac
+
     # Extract timestamp from name: e2e-AGENT-TIMESTAMP
     local ts
     ts=$(printf '%s' "${server_name}" | sed 's/.*-//')
diff --git a/sh/e2e/lib/common.sh b/sh/e2e/lib/common.sh
index a9c2ec3c..3d55a7e0 100644
--- a/sh/e2e/lib/common.sh
+++ b/sh/e2e/lib/common.sh
@@ -9,6 +9,11 @@ ALL_AGENTS="claude openclaw zeroclaw codex opencode kilocode hermes junie"
 PROVISION_TIMEOUT="${PROVISION_TIMEOUT:-720}"
 INSTALL_WAIT="${INSTALL_WAIT:-600}"
 INPUT_TEST_TIMEOUT="${INPUT_TEST_TIMEOUT:-120}"
+# Validate numeric env vars that get interpolated into remote command strings.
+# A non-numeric value here could lead to shell injection via SSH commands.
+case "${PROVISION_TIMEOUT}" in ''|*[!0-9]*) PROVISION_TIMEOUT=720 ;; esac
+case "${INSTALL_WAIT}" in ''|*[!0-9]*) INSTALL_WAIT=600 ;; esac
+case "${INPUT_TEST_TIMEOUT}" in ''|*[!0-9]*) INPUT_TEST_TIMEOUT=120 ;; esac
 
 # Active cloud (set by load_cloud_driver)
 ACTIVE_CLOUD=""