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>
This commit is contained in:
A 2026-04-02 21:24:33 -07:00 committed by GitHub
parent e157637ab8
commit 15df9dfae3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 39 additions and 10 deletions

View file

@ -42,12 +42,12 @@ const SSH_DETECT_CLOUD_HETZNER = (_host: string, _user: string, _keys: string[],
};
const SSH_DETECT_AGENT_VIA_WHICH = (_host: string, _user: string, _keys: string[], cmd: string) => {
// ps aux returns nothing, but which finds the binary
// ps aux returns nothing, but command -v finds the binary
if (cmd.includes("ps aux")) {
return null;
}
if (cmd.includes("which")) {
return "/usr/local/bin/claude\nclaude";
if (cmd === "command -v claude") {
return "/usr/local/bin/claude";
}
return null;
};

View file

@ -87,13 +87,11 @@ function detectAgent(host: string, user: string, keyOpts: string[], runCmd: SshC
}
}
// Second: check installed binaries
const whichCmd = KNOWN_AGENTS.map((b) => `(which ${b} 2>/dev/null && echo ${b})`).join(" || ");
const whichOut = runCmd(host, user, keyOpts, whichCmd);
if (whichOut) {
const match = KNOWN_AGENTS.find((b: KnownAgent) => whichOut.includes(b));
if (match) {
return match;
// Second: check installed binaries — one SSH call per agent to avoid shell injection
for (const agent of KNOWN_AGENTS) {
const whichOut = runCmd(host, user, keyOpts, `command -v ${agent}`);
if (whichOut) {
return agent;
}
}

View file

@ -14,6 +14,27 @@ set -eo pipefail
_GCP_INSTANCE_IP=""
_GCP_INSTANCE_APP=""
# ---------------------------------------------------------------------------
# _gcp_validate_instance_name NAME
#
# Validate that a GCP instance name contains only safe characters.
# GCP requires: lowercase letters, digits, and hyphens; must start with a
# letter and not end with a hyphen; max 63 chars.
# Returns 0 on valid, 1 on invalid.
# ---------------------------------------------------------------------------
_gcp_validate_instance_name() {
local name="$1"
if [ -z "${name}" ]; then
log_err "Instance name is empty"
return 1
fi
if ! printf '%s' "${name}" | grep -qE '^[a-z][a-z0-9-]{0,61}[a-z0-9]$'; then
log_err "Invalid GCP instance name: ${name} (must match [a-z][a-z0-9-]*[a-z0-9], max 63 chars)"
return 1
fi
return 0
}
# ---------------------------------------------------------------------------
# _gcp_validate_env
#
@ -105,6 +126,7 @@ process.stdout.write(d.GCP_ZONE || '');
_gcp_headless_env() {
local app="$1"
# $2 = agent (unused but part of the interface)
_gcp_validate_instance_name "${app}" || return 1
printf 'export GCP_INSTANCE_NAME="%s"\n' "${app}"
printf 'export GCP_PROJECT="%s"\n' "${GCP_PROJECT:-}"
@ -127,6 +149,7 @@ _gcp_provision_verify() {
local log_dir="$2"
local zone="${GCP_ZONE:-us-central1-a}"
local project="${GCP_PROJECT:-}"
_gcp_validate_instance_name "${app}" || return 1
# Check instance exists
if ! gcloud compute instances describe "${app}" \
@ -174,6 +197,7 @@ _gcp_exec() {
local app="$1"
local cmd="$2"
local ssh_user="${GCP_SSH_USER:-$(whoami)}"
_gcp_validate_instance_name "${app}" || return 1
# Validate SSH user contains only safe characters (defense-in-depth)
if ! printf '%s' "${ssh_user}" | grep -qE '^[a-zA-Z0-9._-]+$'; then
@ -238,6 +262,7 @@ _gcp_teardown() {
local app="$1"
local zone="${GCP_ZONE:-us-central1-a}"
local project="${GCP_PROJECT:-}"
_gcp_validate_instance_name "${app}" || return 1
# Try reading zone/project from metadata file
if [ -n "${LOG_DIR:-}" ] && [ -f "${LOG_DIR}/${app}.meta" ]; then
@ -330,6 +355,12 @@ _gcp_cleanup_stale() {
instance_name=$(printf '%s' "${entry}" | awk '{print $1}')
instance_zone_url=$(printf '%s' "${entry}" | awk '{print $2}')
if ! _gcp_validate_instance_name "${instance_name}"; then
log_warn "Skipping ${instance_name} — invalid name format"
skipped=$((skipped + 1))
continue
fi
# Extract zone name from full URL (zones/us-central1-a -> us-central1-a)
local instance_zone
instance_zone=$(printf '%s' "${instance_zone_url}" | sed 's|.*/||')