From 9919a8321c1d49f2c22dcd16e60491bd09bdc970 Mon Sep 17 00:00:00 2001 From: "Rodrigo Rodriguez (Pragmatismo)" Date: Tue, 31 Mar 2026 21:34:04 -0300 Subject: [PATCH] fix: Use SafeCommand directly for vault health check to avoid shell injection false positive - Replace safe_sh_command with SafeCommand::new("curl").args() in vault_health_check() - The URL contains https:// which triggered '//' pattern detection in shell command - Direct SafeCommand bypasses shell parsing, URL passed as single argument - Add vault data directory existence check before recovery attempt - Prevents 'Dangerous pattern // detected' errors during bootstrap --- src/core/bootstrap/bootstrap_utils.rs | 41 +++++++++------- src/core/package_manager/installer.rs | 67 ++++++++++++++++----------- 2 files changed, 62 insertions(+), 46 deletions(-) diff --git a/src/core/bootstrap/bootstrap_utils.rs b/src/core/bootstrap/bootstrap_utils.rs index 31b6c329..f919af59 100644 --- a/src/core/bootstrap/bootstrap_utils.rs +++ b/src/core/bootstrap/bootstrap_utils.rs @@ -90,25 +90,30 @@ pub fn vault_health_check() -> bool { let vault_addr = std::env::var("VAULT_ADDR").unwrap_or_else(|_| "https://localhost:8200".to_string()); - let cmd = format!( - "curl -f -s --connect-timeout 2 -k {}/v1/sys/health", - vault_addr - ); + let health_url = format!("{}/v1/sys/health", vault_addr); - let output = safe_sh_command(&cmd); - if output.is_empty() { - return false; - } - - if let Ok(json) = serde_json::from_str::(&output) { - let sealed = json.get("sealed").and_then(|v| v.as_bool()).unwrap_or(true); - let initialized = json - .get("initialized") - .and_then(|v| v.as_bool()) - .unwrap_or(false); - !sealed && initialized - } else { - false + match SafeCommand::new("curl") + .and_then(|c| c.args(&["-f", "-s", "--connect-timeout", "2", "-k", &health_url])) + .and_then(|c| c.execute()) + { + Ok(output) => { + if output.status.success() { + let stdout = String::from_utf8_lossy(&output.stdout); + if let Ok(json) = serde_json::from_str::(&stdout) { + let sealed = json.get("sealed").and_then(|v| v.as_bool()).unwrap_or(true); + let initialized = json + .get("initialized") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + return !sealed && initialized; + } + } + // Health endpoint returns 503 when sealed but initialized + let stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + stdout.contains("\"initialized\":true") || stderr.contains("\"initialized\":true") + } + Err(_) => false, } } diff --git a/src/core/package_manager/installer.rs b/src/core/package_manager/installer.rs index b6a1234b..26c20df6 100644 --- a/src/core/package_manager/installer.rs +++ b/src/core/package_manager/installer.rs @@ -1387,6 +1387,15 @@ EOF"#.to_string(), let vault_bin = bin_path.join("vault"); let vault_data = self.base_path.join("data/vault"); + // Check if Vault data directory exists (real indicator of initialized state) + let vault_data_exists = vault_data.exists(); + + if !vault_data_exists { + info!("Vault data directory not found, will initialize fresh"); + } else { + info!("Vault data directory found, checking health..."); + } + // Wait for Vault to be ready info!("Waiting for Vault to start..."); std::thread::sleep(std::time::Duration::from_secs(3)); @@ -1395,39 +1404,41 @@ EOF"#.to_string(), std::env::var("VAULT_ADDR").unwrap_or_else(|_| "https://localhost:8200".to_string()); let ca_cert = conf_path.join("system/certificates/ca/ca.crt"); - // Check if Vault is already initialized via health endpoint - let health_cmd = format!( - "curl -f -s --connect-timeout 2 -k {}/v1/sys/health", - vault_addr - ); - let health_output = safe_sh_command(&health_cmd); + // Only attempt recovery if data directory exists + if vault_data_exists { + // Check if Vault is already initialized via health endpoint + let health_cmd = format!( + "curl -f -s --connect-timeout 2 -k {}/v1/sys/health", + vault_addr + ); + let health_output = safe_sh_command(&health_cmd); - let already_initialized = if let Some(ref output) = health_output { - if output.status.success() { - if let Ok(json) = serde_json::from_str::( - &String::from_utf8_lossy(&output.stdout), - ) { - json.get("initialized") - .and_then(|v| v.as_bool()) - .unwrap_or(false) + let already_initialized = if let Some(ref output) = health_output { + if output.status.success() { + if let Ok(json) = serde_json::from_str::( + &String::from_utf8_lossy(&output.stdout), + ) { + json.get("initialized") + .and_then(|v| v.as_bool()) + .unwrap_or(false) + } else { + false + } } else { - false + // Health endpoint returns 503 when sealed but initialized + let stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + stdout.contains("\"initialized\":true") + || stderr.contains("\"initialized\":true") } } else { - // Health endpoint returns 503 when sealed but initialized - let stderr = String::from_utf8_lossy(&output.stderr); - let stdout = String::from_utf8_lossy(&output.stdout); - stdout.contains("\"initialized\":true") - || stderr.contains("\"initialized\":true") - || vault_data.exists() - } - } else { - vault_data.exists() - }; + false + }; - if already_initialized { - info!("Vault already initialized (detected via health/data), skipping init"); - return self.recover_existing_vault(); + if already_initialized { + info!("Vault already initialized (detected via health/data), skipping init"); + return self.recover_existing_vault(); + } } // Initialize Vault