Skip to content

Commit 3724bb8

Browse files
la14-1louisgvclaude
authored
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>
1 parent 0380ad3 commit 3724bb8

5 files changed

Lines changed: 59 additions & 1 deletion

File tree

sh/e2e/lib/clouds/aws.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,13 @@ _aws_exec() {
136136
log_err "Could not resolve IP for instance ${app}"
137137
return 1
138138
fi
139+
# Validate IP looks like an IPv4 address (defense-in-depth against API/file tampering)
140+
if ! printf '%s' "${_AWS_INSTANCE_IP}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$'; then
141+
log_err "Invalid IP address for instance ${app}: ${_AWS_INSTANCE_IP}"
142+
_AWS_INSTANCE_IP=""
143+
_AWS_INSTANCE_APP=""
144+
return 1
145+
fi
139146
fi
140147

141148
ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \

sh/e2e/lib/clouds/digitalocean.sh

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,12 @@ _digitalocean_exec() {
149149
return 1
150150
fi
151151

152+
# Validate IP looks like an IPv4 address (defense-in-depth against file tampering)
153+
if ! printf '%s' "${ip}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$'; then
154+
log_err "Invalid IP address in ${ip_file}: ${ip}"
155+
return 1
156+
fi
157+
152158
ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
153159
-o ConnectTimeout=10 -o LogLevel=ERROR -o BatchMode=yes \
154160
"root@${ip}" "${cmd}"
@@ -183,6 +189,9 @@ _digitalocean_teardown() {
183189
return 0
184190
fi
185191

192+
# Validate droplet ID is numeric (defense-in-depth against metadata tampering)
193+
case "${droplet_id}" in ''|*[!0-9]*) log_warn "Non-numeric droplet ID: ${droplet_id}"; untrack_app "${app}"; return 0 ;; esac
194+
186195
# Retry DELETE up to 3 times with --max-time to prevent hangs
187196
local attempt=0
188197
local delete_accepted=0
@@ -281,6 +290,9 @@ _digitalocean_cleanup_stale() {
281290
local droplet_name
282291
droplet_name=$(printf '%s' "${line}" | cut -d' ' -f2)
283292

293+
# Validate droplet ID is numeric before using it in API URL
294+
case "${droplet_id}" in ''|*[!0-9]*) log_warn "Skipping ${line} — non-numeric droplet ID"; skipped=$((skipped + 1)); continue ;; esac
295+
284296
# Extract timestamp from name: e2e-AGENT-TIMESTAMP
285297
# The timestamp is the last dash-separated segment
286298
local ts

sh/e2e/lib/clouds/gcp.sh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,12 @@ _gcp_exec() {
126126
local cmd="$2"
127127
local ssh_user="${GCP_SSH_USER:-$(whoami)}"
128128

129+
# Validate SSH user contains only safe characters (defense-in-depth)
130+
if ! printf '%s' "${ssh_user}" | grep -qE '^[a-zA-Z0-9._-]+$'; then
131+
log_err "Invalid SSH user for instance ${app}: ${ssh_user}"
132+
return 1
133+
fi
134+
129135
# Resolve instance IP (cached per app)
130136
if [ "${_GCP_INSTANCE_APP}" != "${app}" ] || [ -z "${_GCP_INSTANCE_IP}" ]; then
131137
# Try reading from the IP file first (written by _gcp_provision_verify)
@@ -143,6 +149,13 @@ _gcp_exec() {
143149
log_err "Could not resolve IP for instance ${app}"
144150
return 1
145151
fi
152+
# Validate IP looks like an IPv4 address (defense-in-depth against API/file tampering)
153+
if ! printf '%s' "${_GCP_INSTANCE_IP}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$'; then
154+
log_err "Invalid IP address for instance ${app}: ${_GCP_INSTANCE_IP}"
155+
_GCP_INSTANCE_IP=""
156+
_GCP_INSTANCE_APP=""
157+
return 1
158+
fi
146159
fi
147160

148161
ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \

sh/e2e/lib/clouds/hetzner.sh

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,14 @@ _hetzner_provision_verify() {
5454
local app="$1"
5555
local log_dir="$2"
5656

57+
# URL-encode the app name to prevent query parameter injection
58+
local encoded_app
59+
encoded_app=$(jq -rn --arg v "${app}" '$v|@uri')
60+
5761
local response
5862
response=$(curl -sf \
5963
-H "Authorization: Bearer ${HCLOUD_TOKEN}" \
60-
"${_HETZNER_API}/servers?name=${app}" 2>/dev/null || true)
64+
"${_HETZNER_API}/servers?name=${encoded_app}" 2>/dev/null || true)
6165

6266
if [ -z "${response}" ]; then
6367
log_err "Failed to query Hetzner API for server ${app}"
@@ -120,6 +124,17 @@ _hetzner_exec() {
120124
local ip
121125
ip=$(cat "${ip_file}")
122126

127+
if [ -z "${ip}" ]; then
128+
log_err "Empty IP in ${ip_file}"
129+
return 1
130+
fi
131+
132+
# Validate IP looks like an IPv4 address (defense-in-depth against file tampering)
133+
if ! printf '%s' "${ip}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$'; then
134+
log_err "Invalid IP address in ${ip_file}: ${ip}"
135+
return 1
136+
fi
137+
123138
ssh -o StrictHostKeyChecking=no \
124139
-o UserKnownHostsFile=/dev/null \
125140
-o LogLevel=ERROR \
@@ -153,6 +168,9 @@ _hetzner_teardown() {
153168
return 0
154169
fi
155170

171+
# Validate server ID is numeric (defense-in-depth against metadata tampering)
172+
case "${server_id}" in ''|*[!0-9]*) log_warn "Non-numeric server ID: ${server_id}"; untrack_app "${app}"; return 0 ;; esac
173+
156174
log_step "Deleting Hetzner server ${app} (id=${server_id})"
157175

158176
local http_code
@@ -220,6 +238,9 @@ _hetzner_cleanup_stale() {
220238
local server_name
221239
server_name=$(printf '%s' "${entry}" | cut -d: -f2-)
222240

241+
# Validate server ID is numeric before using it in API URL
242+
case "${server_id}" in ''|*[!0-9]*) log_warn "Skipping ${entry} — non-numeric server ID"; skipped=$((skipped + 1)); continue ;; esac
243+
223244
# Extract timestamp from name: e2e-AGENT-TIMESTAMP
224245
local ts
225246
ts=$(printf '%s' "${server_name}" | sed 's/.*-//')

sh/e2e/lib/common.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ ALL_AGENTS="claude openclaw zeroclaw codex opencode kilocode hermes junie"
99
PROVISION_TIMEOUT="${PROVISION_TIMEOUT:-720}"
1010
INSTALL_WAIT="${INSTALL_WAIT:-600}"
1111
INPUT_TEST_TIMEOUT="${INPUT_TEST_TIMEOUT:-120}"
12+
# Validate numeric env vars that get interpolated into remote command strings.
13+
# A non-numeric value here could lead to shell injection via SSH commands.
14+
case "${PROVISION_TIMEOUT}" in ''|*[!0-9]*) PROVISION_TIMEOUT=720 ;; esac
15+
case "${INSTALL_WAIT}" in ''|*[!0-9]*) INSTALL_WAIT=600 ;; esac
16+
case "${INPUT_TEST_TIMEOUT}" in ''|*[!0-9]*) INPUT_TEST_TIMEOUT=120 ;; esac
1217

1318
# Active cloud (set by load_cloud_driver)
1419
ACTIVE_CLOUD=""

0 commit comments

Comments
 (0)