diff --git a/src/openhuman/runtime_python/bootstrap.rs b/src/openhuman/runtime_python/bootstrap.rs index a2307bc9f..beba97d29 100644 --- a/src/openhuman/runtime_python/bootstrap.rs +++ b/src/openhuman/runtime_python/bootstrap.rs @@ -27,6 +27,9 @@ pub enum PythonSource { /// Fully-resolved Python interpreter. #[derive(Debug, Clone)] pub struct ResolvedPython { + /// Directory that should be prepended to `PATH` for child processes so + /// `python`, `python3`, `pip`, and `pip3` resolve to the same toolchain. + pub bin_dir: std::path::PathBuf, /// Absolute path to the Python executable. pub python_bin: std::path::PathBuf, /// Normalized interpreter version, e.g. `3.12.4`. @@ -187,6 +190,7 @@ fn resolve_from_system(system: SystemPython) -> ResolvedPython { "[runtime_python::bootstrap] reusing compatible system python" ); ResolvedPython { + bin_dir: python_bin_dir(&system.path), python_bin: system.path, version: system.version, source: PythonSource::System, @@ -207,12 +211,20 @@ fn probe_managed_install(install_dir: &Path) -> Option { let version = super::resolver::probe_python_version_public(&python_bin)?; let version_info = super::resolver::parse_python_version(&version)?; Some(ResolvedPython { + bin_dir: python_bin_dir(&python_bin), python_bin, version: version_info.display(), source: PythonSource::Managed, }) } +fn python_bin_dir(python_bin: &Path) -> PathBuf { + python_bin + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(|| PathBuf::from(".")) +} + fn find_python_binary(install_dir: &Path) -> Option { let candidates = [ install_dir.join("bin").join("python3.12"), diff --git a/src/openhuman/tools/impl/system/shell.rs b/src/openhuman/tools/impl/system/shell.rs index a883b46d5..69eedf484 100644 --- a/src/openhuman/tools/impl/system/shell.rs +++ b/src/openhuman/tools/impl/system/shell.rs @@ -1,5 +1,6 @@ use crate::openhuman::agent::host_runtime::RuntimeAdapter; use crate::openhuman::javascript::NodeBootstrap; +use crate::openhuman::runtime_python::PythonBootstrap; use crate::openhuman::security::{AuditLogger, CommandExecutionLog, GateDecision, SecurityPolicy}; use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; use async_trait::async_trait; @@ -50,6 +51,12 @@ pub struct ShellTool { /// toolchain. Non-blocking: never triggers a download for unrelated /// commands (we use `try_cached()`). node_bootstrap: Option>, + /// Optional managed Python bootstrap. Unlike Node PATH injection, Python + /// shell support is the primary execution surface for skills, so + /// Python-looking commands resolve this lazily before spawn. That keeps + /// `pip install foo` and `python3 -m foo` on one interpreter instead of + /// mixing arbitrary host `pip` and `python3` binaries. + python_bootstrap: Option>, } impl ShellTool { @@ -63,6 +70,7 @@ impl ShellTool { runtime, audit, node_bootstrap: None, + python_bootstrap: None, } } @@ -80,6 +88,27 @@ impl ShellTool { runtime, audit, node_bootstrap: Some(bootstrap), + python_bootstrap: None, + } + } + + /// Attach managed language runtimes used by shell-invoked skills. Node is + /// injected only after a dedicated node/npm tool resolved it; Python is + /// resolved lazily for python/pip commands because shell is currently the + /// user-facing Python skill execution path. + pub fn with_language_bootstraps( + security: Arc, + runtime: Arc, + audit: Arc, + node_bootstrap: Option>, + python_bootstrap: Option>, + ) -> Self { + Self { + security, + runtime, + audit, + node_bootstrap, + python_bootstrap, } } @@ -264,25 +293,17 @@ impl ShellTool { } } - // If a managed Node.js install has already been resolved, transparently - // prepend its bin dir to PATH so this shell sees the managed toolchain. - // `try_cached()` never blocks and never triggers a download — unrelated - // commands (e.g. `ls`) stay fast and byte-identical to before. - if let Some(bootstrap) = self.node_bootstrap.as_ref() { - if let Some(resolved) = bootstrap.try_cached() { - let host_path = std::env::var("PATH").unwrap_or_default(); - let sep = if cfg!(windows) { ";" } else { ":" }; - let prepended = if host_path.is_empty() { - resolved.bin_dir.to_string_lossy().into_owned() - } else { - format!("{}{}{}", resolved.bin_dir.display(), sep, host_path) - }; - tracing::debug!( - bin_dir = %resolved.bin_dir.display(), - version = %resolved.version, - "[shell] prepending managed node bin to PATH" + match self.runtime_path_for_command(command).await { + Ok(Some(path)) => { + tracing::debug!(path = %path, "[shell] applying managed runtime PATH"); + cmd.env("PATH", path); + } + Ok(None) => {} + Err(error) => { + return ( + true, + ToolResult::error(format!("Failed to resolve command runtime: {error}")), ); - cmd.env("PATH", prepended); } } @@ -350,16 +371,16 @@ impl ShellTool { ); let mut extra_env = std::collections::HashMap::new(); - if let Some(bootstrap) = self.node_bootstrap.as_ref() { - if let Some(resolved) = bootstrap.try_cached() { - let host_path = std::env::var("PATH").unwrap_or_default(); - let sep = if cfg!(windows) { ";" } else { ":" }; - let prepended = if host_path.is_empty() { - resolved.bin_dir.to_string_lossy().into_owned() - } else { - format!("{}{}{}", resolved.bin_dir.display(), sep, host_path) - }; - extra_env.insert("PATH".to_string(), prepended); + match self.runtime_path_for_command(command).await { + Ok(Some(path)) => { + extra_env.insert("PATH".to_string(), path); + } + Ok(None) => {} + Err(error) => { + return ( + true, + ToolResult::error(format!("Failed to resolve command runtime: {error}")), + ); } } @@ -402,6 +423,109 @@ impl ShellTool { ), } } + + async fn runtime_path_for_command(&self, command: &str) -> anyhow::Result> { + let mut prepend_dirs = Vec::new(); + + // Node injection preserves the existing contract: shell only sees the + // managed Node bin directory after a previous node/npm tool resolved it. + if let Some(bootstrap) = self.node_bootstrap.as_ref() { + if let Some(resolved) = bootstrap.try_cached() { + tracing::debug!( + bin_dir = %resolved.bin_dir.display(), + version = %resolved.version, + "[shell] prepending managed node bin to PATH" + ); + prepend_dirs.push(resolved.bin_dir); + } + } + + if shell_command_needs_python_runtime(command) { + if let Some(bootstrap) = self.python_bootstrap.as_ref() { + let resolved = bootstrap.resolve().await?; + tracing::debug!( + bin_dir = %resolved.bin_dir.display(), + python_bin = %resolved.python_bin.display(), + version = %resolved.version, + source = ?resolved.source, + "[shell] prepending python runtime bin to PATH" + ); + prepend_dirs.push(resolved.bin_dir); + } + } + + if prepend_dirs.is_empty() { + Ok(None) + } else { + Ok(Some(prepend_path_dirs( + prepend_dirs.iter().map(|p| p.as_path()), + &std::env::var("PATH").unwrap_or_default(), + ))) + } + } +} + +fn prepend_path_dirs<'a>( + dirs: impl IntoIterator, + host_path: &str, +) -> String { + let sep = if cfg!(windows) { ";" } else { ":" }; + let mut parts: Vec = dirs + .into_iter() + .map(|path| path.to_string_lossy().into_owned()) + .collect(); + if !host_path.is_empty() { + parts.push(host_path.to_string()); + } + parts.join(sep) +} + +fn shell_command_needs_python_runtime(command: &str) -> bool { + let lower = command.to_ascii_lowercase(); + lower + .split(|ch| matches!(ch, ';' | '&' | '|' | '\n' | '\r')) + .any(segment_starts_with_python_command) +} + +fn segment_starts_with_python_command(segment: &str) -> bool { + let mut tokens = segment.split_whitespace().peekable(); + while let Some(token) = tokens.next() { + let token = token.trim_matches(|ch| matches!(ch, '(' | ')' | '<' | '>')); + if token.is_empty() { + continue; + } + if token.contains('=') && !token.starts_with('-') { + continue; + } + if matches!(token, "sudo" | "command" | "time" | "env") { + continue; + } + return is_python_executable_token(token); + } + false +} + +fn is_python_executable_token(token: &str) -> bool { + let executable = token.rsplit('/').next().unwrap_or(token); + matches!( + executable, + "python" + | "python3" + | "py" + | "pip" + | "pip3" + | "python.exe" + | "python3.exe" + | "pip.exe" + | "pip3.exe" + ) || versioned_executable(executable, "python3.") + || versioned_executable(executable, "pip3.") +} + +fn versioned_executable(executable: &str, prefix: &str) -> bool { + executable + .strip_prefix(prefix) + .is_some_and(|suffix| !suffix.is_empty() && suffix.chars().all(|c| c.is_ascii_digit())) } #[cfg(test)] @@ -894,6 +1018,54 @@ mod tests { } } + #[test] + fn shell_detects_python_runtime_commands() { + for command in [ + "python3 -m pyfiglet hello", + "python -m pip install pyfiglet", + "pip install pyfiglet", + "pip3.13 show pyfiglet", + "/opt/openhuman/python/bin/python3 script.py", + "echo hi && python3 -V", + ] { + assert!( + shell_command_needs_python_runtime(command), + "expected python runtime detection for {command}" + ); + } + + for command in [ + "echo python3", + "ls", + "cat ./pipelines.txt", + "node script.js", + ] { + assert!( + !shell_command_needs_python_runtime(command), + "did not expect python runtime detection for {command}" + ); + } + } + + #[test] + fn shell_runtime_path_prepends_managed_dirs_before_host_path() { + let python = std::path::Path::new("/opt/openhuman/python/bin"); + let node = std::path::Path::new("/opt/openhuman/node/bin"); + let joined = prepend_path_dirs([python, node], "/usr/local/bin:/usr/bin"); + let sep = if cfg!(windows) { ";" } else { ":" }; + assert_eq!( + joined, + format!( + "{}{}{}{}{}", + python.display(), + sep, + node.display(), + sep, + "/usr/local/bin:/usr/bin" + ) + ); + } + #[tokio::test] async fn shell_blocks_rate_limited() { let security = Arc::new(SecurityPolicy { diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index 6cd512374..ef793eba6 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -4,6 +4,7 @@ use crate::openhuman::agent::host_runtime::{NativeRuntime, RuntimeAdapter}; use crate::openhuman::config::{Config, DelegateAgentConfig}; use crate::openhuman::javascript::NodeBootstrap; use crate::openhuman::memory::Memory; +use crate::openhuman::runtime_python::PythonBootstrap; use crate::openhuman::security::{AuditLogger, SecurityPolicy}; use std::collections::HashMap; use std::sync::Arc; @@ -113,22 +114,30 @@ pub fn all_tools_with_runtime( ); None }; - - let shell: Box = if let Some(bootstrap) = node_bootstrap.as_ref() { - Box::new(ShellTool::with_node_bootstrap( - security.clone(), - Arc::clone(&runtime), - Arc::clone(&audit), - Arc::clone(bootstrap), - )) + let python_bootstrap: Option> = if root_config.runtime_python.enabled { + tracing::debug!( + minimum_version = %root_config.runtime_python.minimum_version, + prefer_system = root_config.runtime_python.prefer_system, + "[tools::ops] python runtime enabled — constructing shared PythonBootstrap" + ); + Some(Arc::new(PythonBootstrap::new( + root_config.runtime_python.clone(), + ))) } else { - Box::new(ShellTool::new( - security.clone(), - Arc::clone(&runtime), - Arc::clone(&audit), - )) + tracing::debug!( + "[tools::ops] python runtime disabled — shell python/pip PATH injection suppressed" + ); + None }; + let shell: Box = Box::new(ShellTool::with_language_bootstraps( + security.clone(), + Arc::clone(&runtime), + Arc::clone(&audit), + node_bootstrap.as_ref().map(Arc::clone), + python_bootstrap.as_ref().map(Arc::clone), + )); + let mut tools: Vec> = vec![ shell, Box::new(FileReadTool::new(security.clone())),