security: Fix command injection vulnerability in env var exports (#1086)

CRITICAL: Add validation to prevent command injection via malicious environment variable names in `export "${var_name}=..."` patterns.

Vulnerability Details:
- All instances of `export "${var_name}=${value}"` where var_name is derived from external sources (manifest.json auth fields, user input, API responses) were vulnerable to command injection
- If var_name contained shell metacharacters like `;`, `$()`, or backticks, arbitrary code could be executed
- Example exploit: var_name=`FOO; rm -rf /` would execute the rm command

Affected Files:
- shared/key-request.sh: _try_load_env_var() - var_name from manifest.json
- shared/common.sh: _load_token_from_config(), ensure_api_token_with_provider(), _multi_creds_load_config(), _multi_creds_prompt(), _poll_instance_once() - var_name from function parameters
- test/record.sh: _load_multi_config_from_file(), _try_load_cloud_config(), _prompt_cloud_creds_interactive() - var_name from test fixtures

Fix Applied:
- Added regex validation before all export statements: `^[A-Z_][A-Z0-9_]*$`
- This allowlist enforces standard POSIX environment variable naming (uppercase letters, digits, underscores only, must start with letter or underscore)
- Returns error if validation fails, preventing injection

Impact:
- While current usage passes hardcoded env var names (e.g., "HCLOUD_TOKEN"), the vulnerability existed in the implementation
- manifest.json is currently trusted, but defense-in-depth prevents supply chain attacks or accidental malformed entries
- Test infrastructure was also vulnerable to malicious fixture data

Agent: security-auditor

Co-authored-by: Spawn Refactor Service <refactor@spawn.service>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
A 2026-02-14 01:01:25 -08:00 committed by GitHub
parent ecdfc5fa9b
commit f3ee7e271a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 52 additions and 1 deletions

View file

@ -1794,6 +1794,11 @@ _poll_instance_once() {
local ip
ip=$(_extract_json_field "${response}" "${ip_py}")
if [[ -n "${ip}" ]]; then
# SECURITY: Validate ip_var to prevent command injection
if [[ ! "${ip_var}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
log_error "SECURITY: Invalid env var name rejected: ${ip_var}"
return 1
fi
export "${ip_var}=${ip}"
log_info "${description} ready (IP: ${ip})"
return 0
@ -1870,6 +1875,12 @@ _load_token_from_config() {
local env_var_name="${2}"
local provider_name="${3}"
# SECURITY: Validate env_var_name to prevent command injection
if [[ ! "${env_var_name}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
log_error "SECURITY: Invalid env var name rejected: ${env_var_name}"
return 1
fi
if [[ ! -f "${config_file}" ]]; then
return 1
fi
@ -1959,6 +1970,12 @@ ensure_api_token_with_provider() {
local token
token=$(validated_read "Enter your ${provider_name} API token: " validate_api_token) || return 1
# SECURITY: Validate env_var_name to prevent command injection
if [[ ! "${env_var_name}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
log_error "SECURITY: Invalid env var name rejected: ${env_var_name}"
return 1
fi
export "${env_var_name}=${token}"
# Validate with provider API
@ -2057,6 +2074,11 @@ _multi_creds_load_config() {
if [[ -z "${value}" ]]; then
return 1
fi
# SECURITY: Validate env var name before export
if [[ ! "${env_vars[$i]}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
log_error "SECURITY: Invalid env var name rejected: ${env_vars[$i]}"
return 1
fi
export "${env_vars[$i]}=${value}"
i=$((i + 1))
done <<< "${creds}"
@ -2084,6 +2106,11 @@ _multi_creds_prompt() {
local idx
for idx in $(seq 0 $((${#env_vars[@]} - 1))); do
# SECURITY: Validate env var name before export
if [[ ! "${env_vars[$idx]}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
log_error "SECURITY: Invalid env var name rejected: ${env_vars[$idx]}"
return 1
fi
local val
val=$(safe_read "Enter ${provider_name} ${labels[$idx]}: ") || return 1
if [[ -z "${val}" ]]; then

View file

@ -60,6 +60,13 @@ _try_load_env_var() {
local var_name="${1}"
local config_file="${2}"
# SECURITY: Validate var_name to prevent command injection via export
# Only allow uppercase letters, numbers, and underscores (standard env var naming)
if [[ ! "${var_name}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
log "SECURITY: Invalid env var name rejected: ${var_name}"
return 1
fi
# Already set in environment?
local current_val="${!var_name:-}"
if [[ -n "${current_val}" ]]; then

View file

@ -233,7 +233,14 @@ except: pass
read -ra fields <<< "$vals"
local i
for i in "${!env_vars[@]}"; do
[[ -n "${fields[$i]:-}" ]] && export "${env_vars[$i]}=${fields[$i]}"
# SECURITY: Validate env var name before export
if [[ -n "${fields[$i]:-}" ]]; then
if [[ ! "${env_vars[$i]}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
echo "SECURITY: Invalid env var name rejected: ${env_vars[$i]}" >&2
return 1
fi
export "${env_vars[$i]}=${fields[$i]}"
fi
done
return 0
}
@ -317,6 +324,11 @@ try_load_config() {
# Standard single-token config
if [[ -f "$config_file" ]]; then
# SECURITY: Validate env var name before export
if [[ ! "${env_var}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
echo "SECURITY: Invalid env var name rejected: ${env_var}" >&2
return 1
fi
local token
token=$(python3 -c "import json,sys; d=json.load(open(sys.argv[1])); print(d.get('api_key','') or d.get('token',''))" "$config_file" 2>/dev/null) || true
if [[ -n "${token:-}" ]]; then
@ -394,6 +406,11 @@ prompt_credentials() {
fi
for var_name in $vars_needed; do
# SECURITY: Validate env var name before using in eval or export
if [[ ! "${var_name}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
echo "SECURITY: Invalid env var name rejected: ${var_name}" >&2
return 1
fi
eval "local current=\"\${${var_name}:-}\""
if [[ -n "$current" ]]; then
continue