diff --git a/src/openhuman/tools/impl/system/node_exec.rs b/src/openhuman/tools/impl/system/node_exec.rs index b0befe279..3486dc29d 100644 --- a/src/openhuman/tools/impl/system/node_exec.rs +++ b/src/openhuman/tools/impl/system/node_exec.rs @@ -40,7 +40,28 @@ const MAX_OUTPUT_BYTES: usize = 1_048_576; /// into spawned node processes. `PATH` gets a prepend of the managed bin /// dir before being forwarded. const SAFE_ENV_VARS: &[&str] = &[ - "HOME", "TERM", "LANG", "LC_ALL", "LC_CTYPE", "USER", "SHELL", "TMPDIR", + "HOME", + "TERM", + "LANG", + "LC_ALL", + "LC_CTYPE", + "USER", + "SHELL", + "TMPDIR", + // Windows process creation and child command lookup need these after env_clear(). + // PATH is rebuilt separately with the managed Node bin dir prepended. + "SystemRoot", + "WINDIR", + "COMSPEC", + "PATHEXT", + "TEMP", + "TMP", + "USERPROFILE", + "APPDATA", + "LOCALAPPDATA", + "ProgramFiles", + "ProgramFiles(x86)", + "ProgramW6432", ]; /// `node_exec` — execute JavaScript through the resolved Node.js runtime. @@ -355,4 +376,14 @@ mod tests { let resolved = resolve_script_path(ws, "scripts/run.js").unwrap(); assert_eq!(resolved, std::path::Path::new("/ws/scripts/run.js")); } + + #[test] + fn safe_env_vars_include_windows_process_essentials() { + for var in ["SystemRoot", "COMSPEC", "PATHEXT", "TEMP", "USERPROFILE"] { + assert!( + SAFE_ENV_VARS.contains(&var), + "{var} must be forwarded for Windows child processes" + ); + } + } } diff --git a/src/openhuman/tools/impl/system/npm_exec.rs b/src/openhuman/tools/impl/system/npm_exec.rs index a1d1b80ab..3a1cbf556 100644 --- a/src/openhuman/tools/impl/system/npm_exec.rs +++ b/src/openhuman/tools/impl/system/npm_exec.rs @@ -35,7 +35,28 @@ const NPM_TIMEOUT_MAX_SECS: u64 = 1800; const MAX_OUTPUT_BYTES: usize = 1_048_576; /// Env allow-list — matches the shell / node_exec tools. const SAFE_ENV_VARS: &[&str] = &[ - "HOME", "TERM", "LANG", "LC_ALL", "LC_CTYPE", "USER", "SHELL", "TMPDIR", + "HOME", + "TERM", + "LANG", + "LC_ALL", + "LC_CTYPE", + "USER", + "SHELL", + "TMPDIR", + // Windows process creation and child command lookup need these after env_clear(). + // PATH is rebuilt separately with the managed Node bin dir prepended. + "SystemRoot", + "WINDIR", + "COMSPEC", + "PATHEXT", + "TEMP", + "TMP", + "USERPROFILE", + "APPDATA", + "LOCALAPPDATA", + "ProgramFiles", + "ProgramFiles(x86)", + "ProgramW6432", ]; /// Subcommands we outright refuse to run. These either break the managed @@ -373,4 +394,14 @@ mod tests { let got = resolve_cwd(ws, Some("app")).unwrap(); assert_eq!(got, std::path::PathBuf::from("/tmp/ws/app")); } + + #[test] + fn safe_env_vars_include_windows_process_essentials() { + for var in ["SystemRoot", "COMSPEC", "PATHEXT", "TEMP", "USERPROFILE"] { + assert!( + SAFE_ENV_VARS.contains(&var), + "{var} must be forwarded for Windows child processes" + ); + } + } } diff --git a/src/openhuman/tools/impl/system/shell.rs b/src/openhuman/tools/impl/system/shell.rs index d9533c8d6..3093bc887 100644 --- a/src/openhuman/tools/impl/system/shell.rs +++ b/src/openhuman/tools/impl/system/shell.rs @@ -14,7 +14,28 @@ const MAX_OUTPUT_BYTES: usize = 1_048_576; /// Environment variables safe to pass to shell commands. /// Only functional variables are included — never API keys or secrets. const SAFE_ENV_VARS: &[&str] = &[ - "PATH", "HOME", "TERM", "LANG", "LC_ALL", "LC_CTYPE", "USER", "SHELL", "TMPDIR", + "PATH", + "HOME", + "TERM", + "LANG", + "LC_ALL", + "LC_CTYPE", + "USER", + "SHELL", + "TMPDIR", + // Windows process creation and child command lookup need these after env_clear(). + "SystemRoot", + "WINDIR", + "COMSPEC", + "PATHEXT", + "TEMP", + "TMP", + "USERPROFILE", + "APPDATA", + "LOCALAPPDATA", + "ProgramFiles", + "ProgramFiles(x86)", + "ProgramW6432", ]; /// Shell command execution tool with sandboxing @@ -616,6 +637,16 @@ mod tests { ); } + #[test] + fn shell_safe_env_vars_include_windows_process_essentials() { + for var in ["SystemRoot", "COMSPEC", "PATHEXT", "TEMP", "USERPROFILE"] { + assert!( + SAFE_ENV_VARS.contains(&var), + "{var} must be forwarded for Windows child processes" + ); + } + } + #[tokio::test] async fn shell_blocks_rate_limited() { let security = Arc::new(SecurityPolicy {