Skip to content

Commit 15df9df

Browse files
la14-1louisgvclaude
authored
fix(security): array-based agent detection and GCP instance name validation (#3158)
* fix(security): array-based agent detection and GCP instance name validation Replace shell string concatenation in detectAgent() with individual `command -v` calls per agent, eliminating the compound shell command. Add _gcp_validate_instance_name() to validate GCP instance names match [a-z][a-z0-9-]*[a-z0-9] before passing to gcloud commands. Fixes #3151 Fixes #3149 Agent: security-auditor Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: add instance name validation in _gcp_cleanup_stale() Defense-in-depth: validate instance names from GCP API before passing to gcloud delete, consistent with validation at other call sites. Agent: pr-maintainer Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent e157637 commit 15df9df

3 files changed

Lines changed: 39 additions & 10 deletions

File tree

packages/cli/src/__tests__/cmd-link-cov.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ const SSH_DETECT_CLOUD_HETZNER = (_host: string, _user: string, _keys: string[],
4242
};
4343

4444
const SSH_DETECT_AGENT_VIA_WHICH = (_host: string, _user: string, _keys: string[], cmd: string) => {
45-
// ps aux returns nothing, but which finds the binary
45+
// ps aux returns nothing, but command -v finds the binary
4646
if (cmd.includes("ps aux")) {
4747
return null;
4848
}
49-
if (cmd.includes("which")) {
50-
return "/usr/local/bin/claude\nclaude";
49+
if (cmd === "command -v claude") {
50+
return "/usr/local/bin/claude";
5151
}
5252
return null;
5353
};

packages/cli/src/commands/link.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,11 @@ function detectAgent(host: string, user: string, keyOpts: string[], runCmd: SshC
8787
}
8888
}
8989

90-
// Second: check installed binaries
91-
const whichCmd = KNOWN_AGENTS.map((b) => `(which ${b} 2>/dev/null && echo ${b})`).join(" || ");
92-
const whichOut = runCmd(host, user, keyOpts, whichCmd);
93-
if (whichOut) {
94-
const match = KNOWN_AGENTS.find((b: KnownAgent) => whichOut.includes(b));
95-
if (match) {
96-
return match;
90+
// Second: check installed binaries — one SSH call per agent to avoid shell injection
91+
for (const agent of KNOWN_AGENTS) {
92+
const whichOut = runCmd(host, user, keyOpts, `command -v ${agent}`);
93+
if (whichOut) {
94+
return agent;
9795
}
9896
}
9997

sh/e2e/lib/clouds/gcp.sh

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,27 @@ set -eo pipefail
1414
_GCP_INSTANCE_IP=""
1515
_GCP_INSTANCE_APP=""
1616

17+
# ---------------------------------------------------------------------------
18+
# _gcp_validate_instance_name NAME
19+
#
20+
# Validate that a GCP instance name contains only safe characters.
21+
# GCP requires: lowercase letters, digits, and hyphens; must start with a
22+
# letter and not end with a hyphen; max 63 chars.
23+
# Returns 0 on valid, 1 on invalid.
24+
# ---------------------------------------------------------------------------
25+
_gcp_validate_instance_name() {
26+
local name="$1"
27+
if [ -z "${name}" ]; then
28+
log_err "Instance name is empty"
29+
return 1
30+
fi
31+
if ! printf '%s' "${name}" | grep -qE '^[a-z][a-z0-9-]{0,61}[a-z0-9]$'; then
32+
log_err "Invalid GCP instance name: ${name} (must match [a-z][a-z0-9-]*[a-z0-9], max 63 chars)"
33+
return 1
34+
fi
35+
return 0
36+
}
37+
1738
# ---------------------------------------------------------------------------
1839
# _gcp_validate_env
1940
#
@@ -105,6 +126,7 @@ process.stdout.write(d.GCP_ZONE || '');
105126
_gcp_headless_env() {
106127
local app="$1"
107128
# $2 = agent (unused but part of the interface)
129+
_gcp_validate_instance_name "${app}" || return 1
108130

109131
printf 'export GCP_INSTANCE_NAME="%s"\n' "${app}"
110132
printf 'export GCP_PROJECT="%s"\n' "${GCP_PROJECT:-}"
@@ -127,6 +149,7 @@ _gcp_provision_verify() {
127149
local log_dir="$2"
128150
local zone="${GCP_ZONE:-us-central1-a}"
129151
local project="${GCP_PROJECT:-}"
152+
_gcp_validate_instance_name "${app}" || return 1
130153

131154
# Check instance exists
132155
if ! gcloud compute instances describe "${app}" \
@@ -174,6 +197,7 @@ _gcp_exec() {
174197
local app="$1"
175198
local cmd="$2"
176199
local ssh_user="${GCP_SSH_USER:-$(whoami)}"
200+
_gcp_validate_instance_name "${app}" || return 1
177201

178202
# Validate SSH user contains only safe characters (defense-in-depth)
179203
if ! printf '%s' "${ssh_user}" | grep -qE '^[a-zA-Z0-9._-]+$'; then
@@ -238,6 +262,7 @@ _gcp_teardown() {
238262
local app="$1"
239263
local zone="${GCP_ZONE:-us-central1-a}"
240264
local project="${GCP_PROJECT:-}"
265+
_gcp_validate_instance_name "${app}" || return 1
241266

242267
# Try reading zone/project from metadata file
243268
if [ -n "${LOG_DIR:-}" ] && [ -f "${LOG_DIR}/${app}.meta" ]; then
@@ -330,6 +355,12 @@ _gcp_cleanup_stale() {
330355
instance_name=$(printf '%s' "${entry}" | awk '{print $1}')
331356
instance_zone_url=$(printf '%s' "${entry}" | awk '{print $2}')
332357

358+
if ! _gcp_validate_instance_name "${instance_name}"; then
359+
log_warn "Skipping ${instance_name} — invalid name format"
360+
skipped=$((skipped + 1))
361+
continue
362+
fi
363+
333364
# Extract zone name from full URL (zones/us-central1-a -> us-central1-a)
334365
local instance_zone
335366
instance_zone=$(printf '%s' "${instance_zone_url}" | sed 's|.*/||')

0 commit comments

Comments
 (0)