From 8ba3e97ed6e60973ca90aebc98cb0f66ae94abbd Mon Sep 17 00:00:00 2001
From: A <258483684+la14-1@users.noreply.github.com>
Date: Mon, 16 Feb 2026 17:24:30 -0800
Subject: [PATCH] fix: add critical error handling and input validation (#1356)

- Fix race condition in cleanup_oauth_session: Kill process group to prevent zombie OAuth server processes
- Add mktemp failure handling in _init_oauth_session: Prevents undefined behavior when /tmp is full or inaccessible
- Add env var name validation in generate_env_config: Prevents shell injection via malformed KEY=value pairs

Agent: code-health

Co-authored-by: test-engineer <agent@spawn.local>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
---
 shared/common.sh | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/shared/common.sh b/shared/common.sh
index d0442699..47973394 100644
--- a/shared/common.sh
+++ b/shared/common.sh
@@ -757,7 +757,12 @@ cleanup_oauth_session() {
     local oauth_dir="${2}"
 
     if [[ -n "${server_pid}" ]]; then
-        kill "${server_pid}" 2>/dev/null || true
+        # Kill process group to catch any child processes (netcat listeners, etc)
+        kill -TERM "-${server_pid}" 2>/dev/null || kill "${server_pid}" 2>/dev/null || true
+        # Give it time to shut down gracefully
+        sleep 0.5
+        # Force kill if still running
+        kill -KILL "-${server_pid}" 2>/dev/null || true
         wait "${server_pid}" 2>/dev/null || true
     fi
 
@@ -906,12 +911,20 @@ _generate_csrf_state() {
 # Create temp directory with OAuth session files and CSRF state
 _init_oauth_session() {
     local oauth_dir
-    oauth_dir=$(mktemp -d)
+    oauth_dir=$(mktemp -d) || {
+        log_error "Failed to create temporary directory for OAuth session"
+        log_error "Check disk space and /tmp permissions"
+        return 1
+    }
 
     # SECURITY: Generate random CSRF state token (32 hex chars = 128 bits)
     local csrf_state
     csrf_state=$(_generate_csrf_state)
-    printf '%s' "${csrf_state}" > "${oauth_dir}/state"
+    printf '%s' "${csrf_state}" > "${oauth_dir}/state" || {
+        rm -rf "${oauth_dir}"
+        log_error "Failed to write OAuth state file"
+        return 1
+    }
     chmod 600 "${oauth_dir}/state"
 
     echo "${oauth_dir}"
@@ -1066,6 +1079,14 @@ generate_env_config() {
     for env_pair in "$@"; do
         local key="${env_pair%%=*}"
         local value="${env_pair#*=}"
+
+        # SECURITY: Validate environment variable names to prevent injection
+        # Only allow uppercase letters, numbers, and underscores (standard env var format)
+        if [[ ! "${key}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then
+            log_error "SECURITY: Invalid environment variable name rejected: ${key}"
+            continue
+        fi
+
         # Escape any single quotes in the value: replace ' with '\''
         local escaped_value="${value//\'/\'\\\'\'}"
         echo "export ${key}='${escaped_value}'"