From 00acf1c76e6b3c302f9d435663040b94e356ce3e Mon Sep 17 00:00:00 2001 From: "Rodrigo Rodriguez (Pragmatismo)" Date: Fri, 9 Jan 2026 12:13:35 -0300 Subject: [PATCH] fix: Add trusted_shell_script_arg for internal scripts - shell_script_arg blocks $( and backticks for user input safety - trusted_shell_script_arg allows these for internal installer scripts - Internal scripts need shell features like command substitution - Updated bootstrap, installer, facade, and llm modules --- src/core/bootstrap/mod.rs | 2 +- src/core/package_manager/facade.rs | 2 +- src/core/package_manager/installer.rs | 4 ++-- src/llm/local.rs | 10 ++++----- src/security/command_guard.rs | 32 +++++++++++++++++++++++++++ 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/core/bootstrap/mod.rs b/src/core/bootstrap/mod.rs index 614d66e25..6885aaafd 100644 --- a/src/core/bootstrap/mod.rs +++ b/src/core/bootstrap/mod.rs @@ -38,7 +38,7 @@ fn safe_pgrep(args: &[&str]) -> Option { fn safe_sh_command(script: &str) -> Option { SafeCommand::new("sh") .and_then(|c| c.arg("-c")) - .and_then(|c| c.shell_script_arg(script)) + .and_then(|c| c.trusted_shell_script_arg(script)) .ok() .and_then(|cmd| cmd.execute().ok()) } diff --git a/src/core/package_manager/facade.rs b/src/core/package_manager/facade.rs index da0f2f8e6..81c250e72 100644 --- a/src/core/package_manager/facade.rs +++ b/src/core/package_manager/facade.rs @@ -1065,7 +1065,7 @@ Store credentials in Vault: trace!("Executing command: {}", rendered_cmd); let output = SafeCommand::new("bash") .and_then(|c| c.arg("-c")) - .and_then(|c| c.shell_script_arg(&rendered_cmd)) + .and_then(|c| c.trusted_shell_script_arg(&rendered_cmd)) .and_then(|c| c.working_dir(&bin_path)) .map_err(|e| anyhow::anyhow!("Failed to build bash command: {}", e))? .execute() diff --git a/src/core/package_manager/installer.rs b/src/core/package_manager/installer.rs index aa6d3a9c7..12b929b9c 100644 --- a/src/core/package_manager/installer.rs +++ b/src/core/package_manager/installer.rs @@ -17,7 +17,7 @@ fn safe_nvcc_version() -> Option { fn safe_sh_command(script: &str) -> Option { SafeCommand::new("sh") .and_then(|c| c.arg("-c")) - .and_then(|c| c.shell_script_arg(script)) + .and_then(|c| c.trusted_shell_script_arg(script)) .ok() .and_then(|cmd| cmd.execute().ok()) } @@ -1112,7 +1112,7 @@ EOF"#.to_string(), trace!("[START] Working dir: {}", bin_path.display()); let child = SafeCommand::new("sh") .and_then(|c| c.arg("-c")) - .and_then(|c| c.shell_script_arg(&rendered_cmd)) + .and_then(|c| c.trusted_shell_script_arg(&rendered_cmd)) .and_then(|c| c.working_dir(&bin_path)) .and_then(|cmd| cmd.spawn_with_envs(&evaluated_envs)) .map_err(|e| anyhow::anyhow!("Failed to spawn process: {}", e)); diff --git a/src/llm/local.rs b/src/llm/local.rs index d77fe4542..4c82834bb 100644 --- a/src/llm/local.rs +++ b/src/llm/local.rs @@ -90,7 +90,7 @@ pub async fn ensure_llama_servers_running( let pkill_result = SafeCommand::new("sh") .and_then(|c| c.arg("-c")) - .and_then(|c| c.shell_script_arg("pkill llama-server -9; true")); + .and_then(|c| c.trusted_shell_script_arg("pkill llama-server -9; true")); match pkill_result { Ok(cmd) => { @@ -366,7 +366,7 @@ pub fn start_llm_server( ); let cmd = SafeCommand::new("cmd") .and_then(|c| c.arg("/C")) - .and_then(|c| c.shell_script_arg(&cmd_arg)) + .and_then(|c| c.trusted_shell_script_arg(&cmd_arg)) .map_err(|e| Box::new(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())) as Box)?; cmd.execute().map_err(|e| Box::new(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())) as Box)?; } else { @@ -378,7 +378,7 @@ pub fn start_llm_server( ); let cmd = SafeCommand::new("sh") .and_then(|c| c.arg("-c")) - .and_then(|c| c.shell_script_arg(&cmd_arg)) + .and_then(|c| c.trusted_shell_script_arg(&cmd_arg)) .map_err(|e| Box::new(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())) as Box)?; cmd.execute().map_err(|e| Box::new(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())) as Box)?; } @@ -410,7 +410,7 @@ pub async fn start_embedding_server( ); let cmd = SafeCommand::new("cmd") .and_then(|c| c.arg("/c")) - .and_then(|c| c.shell_script_arg(&cmd_arg)) + .and_then(|c| c.trusted_shell_script_arg(&cmd_arg)) .map_err(|e| Box::new(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())) as Box)?; cmd.execute().map_err(|e| Box::new(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())) as Box)?; } else { @@ -422,7 +422,7 @@ pub async fn start_embedding_server( ); let cmd = SafeCommand::new("sh") .and_then(|c| c.arg("-c")) - .and_then(|c| c.shell_script_arg(&cmd_arg)) + .and_then(|c| c.trusted_shell_script_arg(&cmd_arg)) .map_err(|e| Box::new(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())) as Box)?; cmd.execute().map_err(|e| Box::new(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())) as Box)?; } diff --git a/src/security/command_guard.rs b/src/security/command_guard.rs index 675188702..198e62306 100644 --- a/src/security/command_guard.rs +++ b/src/security/command_guard.rs @@ -176,6 +176,38 @@ impl SafeCommand { Ok(self) } + pub fn trusted_shell_script_arg(mut self, script: &str) -> Result { + let is_unix_shell = self.command == "bash" || self.command == "sh"; + let is_windows_cmd = self.command == "cmd"; + if !is_unix_shell && !is_windows_cmd { + return Err(CommandGuardError::InvalidArgument( + "trusted_shell_script_arg only allowed for bash/sh/cmd commands".to_string(), + )); + } + let valid_flag = if is_unix_shell { + self.args.last().is_some_and(|a| a == "-c") + } else { + self.args.last().is_some_and(|a| a == "/C" || a == "/c") + }; + if !valid_flag { + return Err(CommandGuardError::InvalidArgument( + "trusted_shell_script_arg requires -c (unix) or /C (windows) flag to be set first".to_string(), + )); + } + if script.is_empty() { + return Err(CommandGuardError::InvalidArgument( + "Empty script".to_string(), + )); + } + if script.len() > 16384 { + return Err(CommandGuardError::InvalidArgument( + "Script too long".to_string(), + )); + } + self.args.push(script.to_string()); + Ok(self) + } + pub fn args(mut self, args: &[&str]) -> Result { for arg in args { validate_argument(arg)?;