From b8b5c208edd8d2cd112eaa842aac8b2b55d39a7f Mon Sep 17 00:00:00 2001 From: ajabaabaa <59121300+ajabaabaa@users.noreply.github.com> Date: Sun, 4 Oct 2026 23:16:46 -0400 Subject: [PATCH 1/8] fix(windows): forward process bootstrap env into sandboxed children The sandbox environment allow-list in sandbox/ops.rs carried only Unix variable names, so every child spawned after env_clear() lost SystemRoot, WINDIR, COMSPEC, PATHEXT, TEMP, TMP, USERPROFILE, APPDATA and LOCALAPPDATA. A Windows child cannot initialise the OS crypto provider without them, and it fails in a way that does not look like a missing environment variable: - node.exe aborts at startup with "Assertion failed: ncrypto::CSPRNG(nullptr, 0)" (exit 134) - powershell.exe exits 0xffff0000 with "Internal Windows PowerShell error. Loading managed Windows PowerShell failed with error 8009001d" - cmd.exe leaves %SystemRoot% and %USERPROFILE% unexpanded, and falls back to its built-in PATHEXT, so .cmd shims (npm, npx) stop resolving The four tool launchers (shell, node_exec, npm_exec, python_exec) already listed these names, yet the bug survived that fix: all four divert to crate::sandbox before ever reaching their own allow-list whenever the active agent is SandboxMode::Sandboxed -- which the built-in orchestrator is -- so the sandbox list was the only one the main agent actually used. Adds platform_shell::WINDOWS_PROCESS_ENV_VARS as the canonical list, with assert_forwards_windows_bootstrap() and a guard per launcher, so a future sixth allow-list cannot be introduced without it. Values are inherited from the parent environment, never synthesised or hard-coded; on Linux and macOS the names do not resolve, so those platforms are unchanged. Secrets handling is unchanged -- this is still a named allow-list, not inheritance. Tests: sandbox spawn coverage lives in tests/windows_sandbox_env_e2e.rs because release lib tests cannot compile on this branch (event_bus_tests.rs calls a #[cfg(debug_assertions)] function). The spawn probe must be a Rust CreateProcess: node's own child_process.spawn injects SystemRoot, so a JavaScript probe reports a stripped environment as healthy. --- crates/openhuman-cli/Cargo.toml | 4 + .../src/agent/platform_shell.rs | 65 ++++++ crates/openhuman-core/src/sandbox/ops.rs | 45 +++- .../openhuman-core/src/sandbox/ops_tests.rs | 26 +++ .../src/tools/impl/system/node_exec_tests.rs | 11 + .../tools/impl/system/python_exec_tests.rs | 10 + tests/windows_sandbox_env_e2e.rs | 214 ++++++++++++++++++ 7 files changed, 374 insertions(+), 1 deletion(-) create mode 100644 tests/windows_sandbox_env_e2e.rs diff --git a/crates/openhuman-cli/Cargo.toml b/crates/openhuman-cli/Cargo.toml index 121e2ef738a..73854b9550a 100644 --- a/crates/openhuman-cli/Cargo.toml +++ b/crates/openhuman-cli/Cargo.toml @@ -209,6 +209,10 @@ path = "../../tests/config_auth_app_state_connectivity_e2e.rs" name = "cwd_jail_e2e" path = "../../tests/cwd_jail_e2e.rs" +[[test]] +name = "windows_sandbox_env_e2e" +path = "../../tests/windows_sandbox_env_e2e.rs" + [[test]] name = "domain_modules_e2e" path = "../../tests/domain_modules_e2e.rs" diff --git a/crates/openhuman-core/src/agent/platform_shell.rs b/crates/openhuman-core/src/agent/platform_shell.rs index ffc36df4e99..148bc35d474 100644 --- a/crates/openhuman-core/src/agent/platform_shell.rs +++ b/crates/openhuman-core/src/agent/platform_shell.rs @@ -29,6 +29,71 @@ use std::path::Path; +/// Environment variables a Windows child process needs before it can run at +/// all, beyond the functional allow-list each launcher already forwards. +/// +/// Every sanitized spawn path in the core calls `env_clear()` and re-forwards +/// only what it names, so this list is what makes a cleared Windows environment +/// bootable. These are forwarded from the parent environment, never synthesised +/// or hard-coded: a name the parent does not have is simply not set. +/// +/// Measured consequences of omitting them (Windows 11, `SystemRoot` absent from +/// an otherwise-valid child environment): +/// +/// - `node.exe` aborts during startup with +/// `Assertion failed: ncrypto::CSPRNG(nullptr, 0)` (exit 134), because the +/// OS random provider cannot initialise without a system directory. +/// - `powershell.exe` exits with `Internal Windows PowerShell error. Loading +/// managed Windows PowerShell failed with error 8009001d.` +/// - `cmd.exe` leaves `%SystemRoot%`/`%TEMP%`/`%USERPROFILE%` unexpanded. It +/// does *not* repair them for its own children, so a wrapper shell cannot +/// rescue a stripped environment. +/// +/// `COMSPEC` and `PATHEXT` are here because `cmd.exe` needs `PATHEXT` to resolve +/// the `.cmd`/`.bat` shims that npm, npx and the Git toolchain are installed +/// as; `TEMP`/`TMP`/`USERPROFILE`/`APPDATA`/`LOCALAPPDATA` because tooling +/// writes scratch and reads config from them; the `ProgramFiles*` trio because +/// installers and SDK locators probe them. +/// +/// Single source of truth: keep every launcher's allow-list a superset of this +/// (enforced by `windows_bootstrap_vars_are_forwarded_everywhere`), and add a +/// launcher to that test rather than inventing a sixth list. +pub const WINDOWS_PROCESS_ENV_VARS: &[&str] = &[ + "SystemRoot", + "WINDIR", + "COMSPEC", + "PATHEXT", + "TEMP", + "TMP", + "USERPROFILE", + "APPDATA", + "LOCALAPPDATA", + "ProgramFiles", + "ProgramFiles(x86)", + "ProgramW6432", +]; + +/// Assert that a launcher's environment allow-list covers every Windows +/// process-bootstrap variable. +/// +/// Each launcher guards its own list with this, so a new launcher cannot +/// silently ship a `env_clear()` that produces an unbootable Windows child. +/// The failure this prevents is not a clean error: the child aborts inside the +/// OS crypto provider (`node` → `ncrypto::CSPRNG` assertion, `powershell` → +/// `8009001d`), and the harness reports it as a mysterious exit code rather +/// than a missing environment variable. +#[track_caller] +pub fn assert_forwards_windows_bootstrap(allowlist: &[&str], launcher: &str) { + for var in WINDOWS_PROCESS_ENV_VARS { + assert!( + allowlist.contains(var), + "{launcher} clears the child environment but does not forward \ + `{var}`; a Windows child spawned without it aborts during crypto \ + init. Add it to the allow-list." + ); + } +} + /// Whether the Unix arm prefixes `set -o pipefail`. #[derive(Clone, Copy, PartialEq, Eq)] enum PipeFail { diff --git a/crates/openhuman-core/src/sandbox/ops.rs b/crates/openhuman-core/src/sandbox/ops.rs index d3a091d5e1d..d4b7b05e664 100644 --- a/crates/openhuman-core/src/sandbox/ops.rs +++ b/crates/openhuman-core/src/sandbox/ops.rs @@ -16,8 +16,51 @@ use std::path::Path; use std::time::Duration; /// Safe environment variables forwarded into sandboxed execution. +/// +/// This list is the *entire* environment a sandboxed child gets: both +/// [`execute_unsandboxed`] and [`execute_local_jail`] call `env_clear()` and +/// re-forward only what is named here. The `WINDOWS_PROCESS_ENV_VARS` entries +/// are therefore not decoration — without them a Windows child cannot +/// initialise the OS crypto provider, and it fails in a way that looks nothing +/// like a missing environment variable (`node` aborts with the +/// `ncrypto::CSPRNG` assertion, `powershell` with `8009001d`). +/// +/// They were missing when this defect was found, and it survived the first +/// round of fixes because the four tool launchers (`shell`, `node_exec`, +/// `npm_exec`, `python_exec`) each carry their own copy of the allow-list and +/// had already been patched: the built-in `orchestrator` runs with +/// `sandbox_mode = "sandboxed"`, and all four tools divert to +/// [`crate::sandbox`] *before* reaching those lists, so this was the only list +/// that mattered for the main agent. Keep it a superset of +/// [`crate::agent::platform_shell::WINDOWS_PROCESS_ENV_VARS`] — +/// `sandbox_env_forwards_windows_bootstrap_vars` and +/// `tests/windows_sandbox_env_e2e.rs` enforce that. +/// +/// On Linux and macOS these names simply do not resolve in the parent +/// environment, so forwarding them is a no-op there. pub const SANDBOX_ENV_PASSTHROUGH: &[&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 bootstrap — see `crate::agent::platform_shell`. + "SystemRoot", + "WINDIR", + "COMSPEC", + "PATHEXT", + "TEMP", + "TMP", + "USERPROFILE", + "APPDATA", + "LOCALAPPDATA", + "ProgramFiles", + "ProgramFiles(x86)", + "ProgramW6432", ]; /// Resolve a `SandboxPolicy` from the agent's `SandboxMode`, the diff --git a/crates/openhuman-core/src/sandbox/ops_tests.rs b/crates/openhuman-core/src/sandbox/ops_tests.rs index 50c67ae1f4d..ead2a40c7ef 100644 --- a/crates/openhuman-core/src/sandbox/ops_tests.rs +++ b/crates/openhuman-core/src/sandbox/ops_tests.rs @@ -308,3 +308,29 @@ fn local_status_is_ready_for_a_real_jail() { ); } } + +// ── Windows child environment ──────────────────────────────────────────────── +// +// `execute_unsandboxed` and `execute_local_jail` both call `env_clear()` and +// re-forward only `SANDBOX_ENV_PASSTHROUGH`. On Windows that list has to carry +// the process-bootstrap variables or the child cannot initialise the OS crypto +// provider — `node` dies with `Assertion failed: ncrypto::CSPRNG(nullptr, 0)` +// (exit 134) and `powershell` with `8009001d`. Both read as opaque failures to +// the agent, which is how the original defect shipped: the four tool launchers' +// allow-lists were fixed, the sandbox allow-list — the one the `sandboxed` +// orchestrator actually routes through — was not. + +#[test] +fn sandbox_env_forwards_windows_bootstrap_vars() { + crate::agent::platform_shell::assert_forwards_windows_bootstrap( + SANDBOX_ENV_PASSTHROUGH, + "sandbox::ops::SANDBOX_ENV_PASSTHROUGH", + ); +} + +// What a real child actually receives is covered end-to-end in +// `tests/windows_sandbox_env_e2e.rs`, which spawns through `execute_in_sandbox` +// and asserts on the child's own environment. It lives there because the probe +// has to be a Rust `CreateProcess` spawn: `node`'s own `child_process.spawn` +// silently injects `SystemRoot`, so a JavaScript probe reports a stripped +// environment as healthy. diff --git a/crates/openhuman-core/src/tools/impl/system/node_exec_tests.rs b/crates/openhuman-core/src/tools/impl/system/node_exec_tests.rs index 62d4187f49b..9615701f339 100644 --- a/crates/openhuman-core/src/tools/impl/system/node_exec_tests.rs +++ b/crates/openhuman-core/src/tools/impl/system/node_exec_tests.rs @@ -150,3 +150,14 @@ fn resolve_script_path_targets_action_dir_not_workspace_dir() { resolved.display() ); } + +/// `node_exec` clears the child environment, so its allow-list must carry the +/// Windows process-bootstrap set — without `SystemRoot` a spawned `node.exe` +/// aborts with `Assertion failed: ncrypto::CSPRNG(nullptr, 0)`. +#[test] +fn safe_env_vars_cover_windows_bootstrap() { + crate::agent::platform_shell::assert_forwards_windows_bootstrap( + SAFE_ENV_VARS, + "node_exec::SAFE_ENV_VARS", + ); +} diff --git a/crates/openhuman-core/src/tools/impl/system/python_exec_tests.rs b/crates/openhuman-core/src/tools/impl/system/python_exec_tests.rs index b8b87da3f89..94bdbe59a24 100644 --- a/crates/openhuman-core/src/tools/impl/system/python_exec_tests.rs +++ b/crates/openhuman-core/src/tools/impl/system/python_exec_tests.rs @@ -37,3 +37,13 @@ fn resolve_script_path_rejects_escapes() { std::path::Path::new("/ws/scripts/run.py") ); } + +/// `python_exec` clears the child environment, so its allow-list must carry the +/// Windows process-bootstrap set — a child without them fails to start. +#[test] +fn safe_env_vars_cover_windows_bootstrap() { + crate::agent::platform_shell::assert_forwards_windows_bootstrap( + SAFE_ENV_VARS, + "python_exec::SAFE_ENV_VARS", + ); +} diff --git a/tests/windows_sandbox_env_e2e.rs b/tests/windows_sandbox_env_e2e.rs new file mode 100644 index 00000000000..408584e94c5 --- /dev/null +++ b/tests/windows_sandbox_env_e2e.rs @@ -0,0 +1,214 @@ +//! Windows child-environment e2e for OpenHuman's own sandbox execution path. +//! +//! Both `sandbox::ops` exec functions call `Command::env_clear()` and re-forward +//! only `SANDBOX_ENV_PASSTHROUGH`, so that list is the entire environment a +//! sandboxed child gets. This test measures what a real child sees when spawned +//! through `execute_in_sandbox` — the same function the `shell`, `node_exec`, +//! `npm_exec` and `python_exec` tools route into whenever the active agent is +//! `SandboxMode::Sandboxed` (which the built-in `orchestrator` is). +//! +//! The defect this pins: the list carried no Windows process-bootstrap +//! variables. A child without `SystemRoot` cannot initialise the OS crypto +//! provider, and the failures are opaque to the agent rather than diagnostic: +//! +//! - `node.exe` aborts at startup — `Assertion failed: ncrypto::CSPRNG(nullptr, 0)`, exit 134. +//! - `powershell.exe` exits with `Internal Windows PowerShell error. Loading +//! managed Windows PowerShell failed with error 8009001d.` +//! +//! Note on method: these must be spawned from Rust. `node`'s own +//! `child_process.spawn` (libuv) injects `SystemRoot` into the child, and a +//! probe driven through `cmd.exe` from JavaScript reports it as present, so a +//! JS harness cannot observe the stripping at all. + +use std::collections::HashMap; +use std::time::Duration; + +use openhuman_core::agent::harness::definition::SandboxMode; +use openhuman_core::config::RuntimeConfig; +use openhuman_core::sandbox::ops::{ + execute_in_sandbox, resolve_sandbox_policy, SANDBOX_ENV_PASSTHROUGH, +}; + +/// Run `command` through OpenHuman's sandbox execution path under `mode`. +async fn run_in_sandbox( + mode: SandboxMode, + command: &str, +) -> openhuman_core::sandbox::types::SandboxExecResult { + let tempdir = tempfile::tempdir().expect("tempdir"); + let policy = resolve_sandbox_policy(mode, tempdir.path(), &RuntimeConfig::default(), false); + execute_in_sandbox( + &policy, + command, + tempdir.path(), + HashMap::new(), + Duration::from_secs(120), + ) + .await + .unwrap_or_else(|e| panic!("execute_in_sandbox({mode:?}) failed to run the command: {e}")) +} + +/// Every allow-list that clears the child environment must cover the Windows +/// bootstrap set. Runs on every OS: the list is platform-independent by design, +/// and on Unix the extra names simply never resolve in the parent environment. +#[test] +fn sandbox_allowlist_covers_windows_bootstrap() { + openhuman_core::agent::platform_shell::assert_forwards_windows_bootstrap( + SANDBOX_ENV_PASSTHROUGH, + "sandbox::ops::SANDBOX_ENV_PASSTHROUGH", + ); +} + +#[cfg(windows)] +fn env_probe_command() -> &'static str { + "echo SR=[%SystemRoot%] TEMP=[%TEMP%] UP=[%USERPROFILE%] PATH=[%PATH%]" +} + +/// `cmd.exe` prints `%SystemRoot%` *verbatim* when the variable is absent, which +/// makes non-expansion the assertion. A resolved value is also required, so a +/// child that echoes nothing cannot pass. +#[cfg(windows)] +fn assert_bootstrap_env_resolved(result: &openhuman_core::sandbox::types::SandboxExecResult) { + assert!( + result.success(), + "child exited {} with stderr {:?}", + result.exit_code, + result.stderr + ); + for name in ["SystemRoot", "TEMP", "USERPROFILE", "PATH"] { + assert!( + !result.stdout.contains(&format!("%{name}%")), + "`{name}` reached the child unexpanded, so it is not in the \ + forwarded environment; stdout {:?}", + result.stdout + ); + } + let system_root = bracketed(&result.stdout, "SR=").unwrap_or_default(); + assert!( + system_root.contains(':'), + "`SystemRoot` did not resolve to a directory; stdout {:?}", + result.stdout + ); + assert!( + !bracketed(&result.stdout, "UP=") + .unwrap_or_default() + .is_empty(), + "`USERPROFILE` resolved to nothing; stdout {:?}", + result.stdout + ); +} + +#[cfg(windows)] +fn bracketed(stdout: &str, prefix: &str) -> Option { + let tail = stdout.split(prefix).nth(1)?; + Some(tail.split(']').next()?.trim().to_string()) +} + +/// The unsandboxed exec path (`SandboxMode::None`), which shares the +/// allow-list with the jail path and has no OS-jail dependency. +#[cfg(windows)] +#[tokio::test] +async fn unsandboxed_child_receives_windows_bootstrap_env() { + let result = run_in_sandbox(SandboxMode::None, env_probe_command()).await; + assert_bootstrap_env_resolved(&result); +} + +/// The route the built-in orchestrator actually takes: +/// `SandboxMode::Sandboxed` → `SandboxBackendKind::Local` → +/// `execute_local_jail`. +#[cfg(windows)] +#[tokio::test] +async fn sandboxed_child_receives_windows_bootstrap_env() { + let result = run_in_sandbox(SandboxMode::Sandboxed, env_probe_command()).await; + assert_bootstrap_env_resolved(&result); +} + +/// The reported Node failure, end to end through OpenHuman's spawn code: +/// `node -e` must reach the crypto provider and print random bytes. +#[cfg(windows)] +#[tokio::test] +async fn node_crypto_runs_through_sandbox_path() { + if !tool_available("node") { + eprintln!("skipping node_crypto_runs_through_sandbox_path: `node` is not on PATH"); + return; + } + + let result = run_in_sandbox( + SandboxMode::None, + r#"node -e "console.log('SR=' + process.env.SystemRoot); console.log(require('crypto').randomBytes(8).toString('hex'))""#, + ) + .await; + + assert!( + !result.stderr.contains("ncrypto"), + "node's OS random provider failed to initialise — the child \ + environment is missing Windows bootstrap variables; stderr {:?}", + result.stderr + ); + assert!( + result.success(), + "node aborted through the sandbox path (exit {}); stderr {:?}", + result.exit_code, + result.stderr + ); + assert!( + result + .stdout + .lines() + .any(|line| line.trim().len() == 16 + && line.trim().chars().all(|c| c.is_ascii_hexdigit())), + "no random bytes were printed; stdout {:?}", + result.stdout + ); +} + +/// The reported PowerShell failure: `8009001d` when the child has no system +/// directory to load its crypto provider from. +#[cfg(windows)] +#[tokio::test] +async fn powershell_runs_through_sandbox_path() { + if !tool_available("powershell") { + eprintln!("skipping powershell_runs_through_sandbox_path: `powershell` is not on PATH"); + return; + } + + let result = run_in_sandbox( + SandboxMode::None, + "powershell -NoProfile -Command [guid]::NewGuid().ToString()", + ) + .await; + + assert!( + !result.stderr.contains("8009001d"), + "PowerShell could not load managed PowerShell — the child environment \ + is missing Windows bootstrap variables; stderr {:?}", + result.stderr + ); + assert!( + result.success(), + "powershell failed through the sandbox path (exit {}); stderr {:?}", + result.exit_code, + result.stderr + ); + let guid: String = result + .stdout + .trim() + .chars() + .filter(|c| c.is_ascii_hexdigit() || *c == '-') + .collect(); + assert!( + guid.len() >= 36, + "no GUID was produced by the child; stdout {:?}", + result.stdout + ); +} + +#[cfg(windows)] +fn tool_available(program: &str) -> bool { + std::process::Command::new(program) + .arg("--version") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false) +} From 5ca37fdec9316f3c72c5625a4ad63c3c6a654f1e Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 6 Oct 2026 21:56:55 +0300 Subject: [PATCH 2/8] fix: keep Windows bootstrap env host-only Co-authored-by: Medulla --- crates/openhuman-cli/Cargo.toml | 8 ------- .../src/agent/platform_shell.rs | 20 ++++++++++++++-- crates/openhuman-core/src/sandbox/ops.rs | 12 +++------- tests/windows_sandbox_env_e2e.rs | 24 +++++-------------- 4 files changed, 27 insertions(+), 37 deletions(-) diff --git a/crates/openhuman-cli/Cargo.toml b/crates/openhuman-cli/Cargo.toml index 0450166998e..24c80d722b0 100644 --- a/crates/openhuman-cli/Cargo.toml +++ b/crates/openhuman-cli/Cargo.toml @@ -199,14 +199,6 @@ path = "../../tests/cwd_jail_e2e.rs" name = "windows_sandbox_env_e2e" path = "../../tests/windows_sandbox_env_e2e.rs" -[[test]] -name = "domain_modules_e2e" -path = "../../tests/in_process/domain_modules_e2e.rs" - -[[test]] -name = "embeddings_rpc_e2e" -path = "../../tests/in_process/embeddings_rpc_e2e.rs" - [[test]] name = "inference_provider_e2e" path = "../../tests/inference_provider_e2e.rs" diff --git a/crates/openhuman-core/src/agent/platform_shell.rs b/crates/openhuman-core/src/agent/platform_shell.rs index 01dc475baaf..59793af3bfd 100644 --- a/crates/openhuman-core/src/agent/platform_shell.rs +++ b/crates/openhuman-core/src/agent/platform_shell.rs @@ -56,8 +56,8 @@ use std::path::Path; /// installers and SDK locators probe them. /// /// Single source of truth: keep every launcher's allow-list a superset of this -/// (enforced by `windows_bootstrap_vars_are_forwarded_everywhere`), and add a -/// launcher to that test rather than inventing a sixth list. +/// (enforced by the launcher unit tests), and add a launcher to those tests +/// rather than inventing a sixth list. pub const WINDOWS_PROCESS_ENV_VARS: &[&str] = &[ "SystemRoot", "WINDIR", @@ -96,6 +96,7 @@ pub fn assert_forwards_windows_bootstrap(allowlist: &[&str], launcher: &str) { /// Add Windows bootstrap variables to a host child. Keep these out of the /// sandbox policy because that policy is also forwarded into Linux containers. +#[cfg(windows)] pub fn forward_windows_bootstrap_env(cmd: &mut tokio::process::Command) { for var in WINDOWS_PROCESS_ENV_VARS { if let Ok(val) = std::env::var(var) { @@ -104,6 +105,21 @@ pub fn forward_windows_bootstrap_env(cmd: &mut tokio::process::Command) { } } +#[cfg(not(windows))] +pub fn forward_windows_bootstrap_env(_cmd: &mut tokio::process::Command) {} + +#[cfg(windows)] +pub fn forward_windows_bootstrap_env_std(cmd: &mut std::process::Command) { + for var in WINDOWS_PROCESS_ENV_VARS { + if let Ok(val) = std::env::var(var) { + cmd.env(var, val); + } + } +} + +#[cfg(not(windows))] +pub fn forward_windows_bootstrap_env_std(_cmd: &mut std::process::Command) {} + /// Whether the Unix arm prefixes `set -o pipefail`. #[derive(Clone, Copy, PartialEq, Eq)] enum PipeFail { diff --git a/crates/openhuman-core/src/sandbox/ops.rs b/crates/openhuman-core/src/sandbox/ops.rs index 99ed1fd7135..f1588b6c008 100644 --- a/crates/openhuman-core/src/sandbox/ops.rs +++ b/crates/openhuman-core/src/sandbox/ops.rs @@ -29,14 +29,8 @@ use std::time::Duration; /// `npm_exec`, `python_exec`) each carry their own copy of the allow-list and /// had already been patched: the built-in `orchestrator` runs with /// `sandbox_mode = "sandboxed"`, and all four tools divert to -/// [`crate::sandbox`] *before* reaching those lists, so this was the only list -/// that mattered for the main agent. Keep it a superset of -/// [`crate::agent::platform_shell::WINDOWS_PROCESS_ENV_VARS`] — -/// `sandbox_env_forwards_windows_bootstrap_vars` and -/// `tests/windows_sandbox_env_e2e.rs` enforce that. -/// -/// On Linux and macOS these names simply do not resolve in the parent -/// environment, so forwarding them is a no-op there. +/// [`crate::sandbox`] *before* reaching those lists, so the host spawn paths +/// below are the ones that must add the Windows-only bootstrap set. pub const SANDBOX_ENV_PASSTHROUGH: &[&str] = &[ "PATH", "HOME", "TERM", "LANG", "LC_ALL", "LC_CTYPE", "USER", "SHELL", "TMPDIR", ]; @@ -411,7 +405,7 @@ async fn execute_local_jail( cmd.env(var, val); } } - platform_shell::forward_windows_bootstrap_env(&mut cmd); + platform_shell::forward_windows_bootstrap_env_std(&mut cmd); cmd.env("TMPDIR", &scratch.path); for (k, v) in extra_env { cmd.env(k, v); diff --git a/tests/windows_sandbox_env_e2e.rs b/tests/windows_sandbox_env_e2e.rs index a7a8fad532c..1dfdfc3799d 100644 --- a/tests/windows_sandbox_env_e2e.rs +++ b/tests/windows_sandbox_env_e2e.rs @@ -25,11 +25,10 @@ use std::time::Duration; use openhuman_core::agent::harness::definition::SandboxMode; use openhuman_core::config::RuntimeConfig; -use openhuman_core::sandbox::ops::{ - execute_in_sandbox, resolve_sandbox_policy, SANDBOX_ENV_PASSTHROUGH, -}; +use openhuman_core::sandbox::ops::{execute_in_sandbox, resolve_sandbox_policy}; /// Run `command` through OpenHuman's sandbox execution path under `mode`. +#[cfg(windows)] async fn run_in_sandbox( mode: SandboxMode, command: &str, @@ -53,17 +52,6 @@ async fn run_in_sandbox( .unwrap_or_else(|e| panic!("execute_in_sandbox({mode:?}) failed to run the command: {e}")) } -/// Every allow-list that clears the child environment must cover the Windows -/// bootstrap set. Runs on every OS: the list is platform-independent by design, -/// and on Unix the extra names simply never resolve in the parent environment. -#[test] -fn sandbox_allowlist_covers_windows_bootstrap() { - openhuman_core::agent::platform_shell::assert_forwards_windows_bootstrap( - SANDBOX_ENV_PASSTHROUGH, - "sandbox::ops::SANDBOX_ENV_PASSTHROUGH", - ); -} - #[cfg(windows)] fn env_probe_command() -> &'static str { "echo SR=[%SystemRoot%] TEMP=[%TEMP%] UP=[%USERPROFILE%] PATH=[%PATH%]" @@ -172,14 +160,14 @@ async fn node_crypto_runs_through_sandbox_path() { #[cfg(windows)] #[tokio::test] async fn powershell_runs_through_sandbox_path() { - if !tool_available("powershell") { - eprintln!("skipping powershell_runs_through_sandbox_path: `powershell` is not on PATH"); + if !tool_available("powershell.exe") { + eprintln!("skipping powershell_runs_through_sandbox_path: `powershell.exe` is not on PATH"); return; } let result = run_in_sandbox( SandboxMode::None, - "powershell -NoProfile -Command [guid]::NewGuid().ToString()", + "powershell.exe -NoProfile -Command [guid]::NewGuid().ToString()", ) .await; @@ -210,7 +198,7 @@ async fn powershell_runs_through_sandbox_path() { #[cfg(windows)] fn tool_available(program: &str) -> bool { - let args = if program.eq_ignore_ascii_case("powershell") { + let args = if program.eq_ignore_ascii_case("powershell.exe") { vec![ "-NoProfile", "-NonInteractive", From 35b11ad1fabaf88f1519a7e7d4f4c9199f24d272 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 6 Oct 2026 22:20:39 +0300 Subject: [PATCH 3/8] fix: keep local jail temp files inside scratch Co-authored-by: Medulla --- crates/openhuman-core/src/sandbox/ops.rs | 7 ++++++- tests/windows_sandbox_env_e2e.rs | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/crates/openhuman-core/src/sandbox/ops.rs b/crates/openhuman-core/src/sandbox/ops.rs index f1588b6c008..99d134d6759 100644 --- a/crates/openhuman-core/src/sandbox/ops.rs +++ b/crates/openhuman-core/src/sandbox/ops.rs @@ -406,10 +406,15 @@ async fn execute_local_jail( } } platform_shell::forward_windows_bootstrap_env_std(&mut cmd); - cmd.env("TMPDIR", &scratch.path); for (k, v) in extra_env { cmd.env(k, v); } + // Keep every Windows spelling of the temporary directory inside this + // per-call grant. `TEMP`/`TMP` are the variables used by Windows tools; + // `TMPDIR` covers Unix-oriented tools running on the same host. + cmd.env("TMPDIR", &scratch.path); + cmd.env("TEMP", &scratch.path); + cmd.env("TMP", &scratch.path); let os_backend = cwd_jail::default_backend(); let spawn_result = if os_backend.is_available() { diff --git a/tests/windows_sandbox_env_e2e.rs b/tests/windows_sandbox_env_e2e.rs index 1dfdfc3799d..554f4956c80 100644 --- a/tests/windows_sandbox_env_e2e.rs +++ b/tests/windows_sandbox_env_e2e.rs @@ -89,6 +89,15 @@ fn assert_bootstrap_env_resolved(result: &openhuman_core::sandbox::types::Sandbo "`USERPROFILE` resolved to nothing; stdout {:?}", result.stdout ); + for name in ["TEMP", "PATH"] { + assert!( + !bracketed(&result.stdout, &format!("{name}=")) + .unwrap_or_default() + .is_empty(), + "`{name}` resolved to nothing; stdout {:?}", + result.stdout + ); + } } #[cfg(windows)] @@ -122,7 +131,7 @@ async fn sandboxed_child_receives_windows_bootstrap_env() { #[tokio::test] async fn node_crypto_runs_through_sandbox_path() { if !tool_available("node") { - eprintln!("skipping node_crypto_runs_through_sandbox_path: `node` is not on PATH"); + eprintln!("test skipped: node_crypto_runs_through_sandbox_path (`node` is not on PATH)"); return; } @@ -161,7 +170,9 @@ async fn node_crypto_runs_through_sandbox_path() { #[tokio::test] async fn powershell_runs_through_sandbox_path() { if !tool_available("powershell.exe") { - eprintln!("skipping powershell_runs_through_sandbox_path: `powershell.exe` is not on PATH"); + eprintln!( + "test skipped: powershell_runs_through_sandbox_path (`powershell.exe` is not on PATH)" + ); return; } From ea3acbdacf923e0a65787a2104e0343ecf206c85 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 6 Oct 2026 22:38:50 +0300 Subject: [PATCH 4/8] fix: reject empty sandbox environment values Co-authored-by: Medulla --- .../openhuman-core/src/agent/platform_shell.rs | 8 ++++++-- crates/openhuman-core/src/sandbox/ops.rs | 8 ++++++-- tests/windows_sandbox_env_e2e.rs | 18 ++++++++---------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/crates/openhuman-core/src/agent/platform_shell.rs b/crates/openhuman-core/src/agent/platform_shell.rs index 59793af3bfd..6be83e15a55 100644 --- a/crates/openhuman-core/src/agent/platform_shell.rs +++ b/crates/openhuman-core/src/agent/platform_shell.rs @@ -100,7 +100,9 @@ pub fn assert_forwards_windows_bootstrap(allowlist: &[&str], launcher: &str) { pub fn forward_windows_bootstrap_env(cmd: &mut tokio::process::Command) { for var in WINDOWS_PROCESS_ENV_VARS { if let Ok(val) = std::env::var(var) { - cmd.env(var, val); + if !val.is_empty() { + cmd.env(var, val); + } } } } @@ -112,7 +114,9 @@ pub fn forward_windows_bootstrap_env(_cmd: &mut tokio::process::Command) {} pub fn forward_windows_bootstrap_env_std(cmd: &mut std::process::Command) { for var in WINDOWS_PROCESS_ENV_VARS { if let Ok(val) = std::env::var(var) { - cmd.env(var, val); + if !val.is_empty() { + cmd.env(var, val); + } } } } diff --git a/crates/openhuman-core/src/sandbox/ops.rs b/crates/openhuman-core/src/sandbox/ops.rs index 99d134d6759..6c4f83bfdf3 100644 --- a/crates/openhuman-core/src/sandbox/ops.rs +++ b/crates/openhuman-core/src/sandbox/ops.rs @@ -273,7 +273,9 @@ async fn execute_unsandboxed( cmd.env_clear(); for var in SANDBOX_ENV_PASSTHROUGH { if let Ok(val) = std::env::var(var) { - cmd.env(var, val); + if !val.is_empty() { + cmd.env(var, val); + } } } platform_shell::forward_windows_bootstrap_env(&mut cmd); @@ -402,7 +404,9 @@ async fn execute_local_jail( cmd.env_clear(); for var in SANDBOX_ENV_PASSTHROUGH { if let Ok(val) = std::env::var(var) { - cmd.env(var, val); + if !val.is_empty() { + cmd.env(var, val); + } } } platform_shell::forward_windows_bootstrap_env_std(&mut cmd); diff --git a/tests/windows_sandbox_env_e2e.rs b/tests/windows_sandbox_env_e2e.rs index 554f4956c80..8fa86994590 100644 --- a/tests/windows_sandbox_env_e2e.rs +++ b/tests/windows_sandbox_env_e2e.rs @@ -130,10 +130,10 @@ async fn sandboxed_child_receives_windows_bootstrap_env() { #[cfg(windows)] #[tokio::test] async fn node_crypto_runs_through_sandbox_path() { - if !tool_available("node") { - eprintln!("test skipped: node_crypto_runs_through_sandbox_path (`node` is not on PATH)"); - return; - } + assert!( + tool_available("node"), + "node is required for node_crypto_runs_through_sandbox_path; missing tooling must not silently pass" + ); let result = run_in_sandbox( SandboxMode::None, @@ -169,12 +169,10 @@ async fn node_crypto_runs_through_sandbox_path() { #[cfg(windows)] #[tokio::test] async fn powershell_runs_through_sandbox_path() { - if !tool_available("powershell.exe") { - eprintln!( - "test skipped: powershell_runs_through_sandbox_path (`powershell.exe` is not on PATH)" - ); - return; - } + assert!( + tool_available("powershell.exe"), + "powershell.exe is required for powershell_runs_through_sandbox_path; missing tooling must not silently pass" + ); let result = run_in_sandbox( SandboxMode::None, From 72dbec885557d4bb8f8f867d25d671cdcb55729b Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 6 Oct 2026 23:02:30 +0300 Subject: [PATCH 5/8] fix: reject empty sandbox environment values Co-authored-by: Medulla --- .../src/agent/platform_shell.rs | 24 ++++++++++++------- crates/openhuman-core/src/sandbox/ops.rs | 14 ++++++----- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/crates/openhuman-core/src/agent/platform_shell.rs b/crates/openhuman-core/src/agent/platform_shell.rs index 6be83e15a55..36e804b87e5 100644 --- a/crates/openhuman-core/src/agent/platform_shell.rs +++ b/crates/openhuman-core/src/agent/platform_shell.rs @@ -97,32 +97,40 @@ pub fn assert_forwards_windows_bootstrap(allowlist: &[&str], launcher: &str) { /// Add Windows bootstrap variables to a host child. Keep these out of the /// sandbox policy because that policy is also forwarded into Linux containers. #[cfg(windows)] -pub fn forward_windows_bootstrap_env(cmd: &mut tokio::process::Command) { +pub fn forward_windows_bootstrap_env(cmd: &mut tokio::process::Command) -> anyhow::Result<()> { for var in WINDOWS_PROCESS_ENV_VARS { if let Ok(val) = std::env::var(var) { - if !val.is_empty() { - cmd.env(var, val); + if val.is_empty() { + anyhow::bail!("Windows bootstrap environment variable {var} is empty"); } + cmd.env(var, val); } } + Ok(()) } #[cfg(not(windows))] -pub fn forward_windows_bootstrap_env(_cmd: &mut tokio::process::Command) {} +pub fn forward_windows_bootstrap_env(_cmd: &mut tokio::process::Command) -> anyhow::Result<()> { + Ok(()) +} #[cfg(windows)] -pub fn forward_windows_bootstrap_env_std(cmd: &mut std::process::Command) { +pub fn forward_windows_bootstrap_env_std(cmd: &mut std::process::Command) -> anyhow::Result<()> { for var in WINDOWS_PROCESS_ENV_VARS { if let Ok(val) = std::env::var(var) { - if !val.is_empty() { - cmd.env(var, val); + if val.is_empty() { + anyhow::bail!("Windows bootstrap environment variable {var} is empty"); } + cmd.env(var, val); } } + Ok(()) } #[cfg(not(windows))] -pub fn forward_windows_bootstrap_env_std(_cmd: &mut std::process::Command) {} +pub fn forward_windows_bootstrap_env_std(_cmd: &mut std::process::Command) -> anyhow::Result<()> { + Ok(()) +} /// Whether the Unix arm prefixes `set -o pipefail`. #[derive(Clone, Copy, PartialEq, Eq)] diff --git a/crates/openhuman-core/src/sandbox/ops.rs b/crates/openhuman-core/src/sandbox/ops.rs index 6c4f83bfdf3..344719b9c1d 100644 --- a/crates/openhuman-core/src/sandbox/ops.rs +++ b/crates/openhuman-core/src/sandbox/ops.rs @@ -273,12 +273,13 @@ async fn execute_unsandboxed( cmd.env_clear(); for var in SANDBOX_ENV_PASSTHROUGH { if let Ok(val) = std::env::var(var) { - if !val.is_empty() { - cmd.env(var, val); + if val.is_empty() { + anyhow::bail!("sandbox passthrough environment variable {var} is empty"); } + cmd.env(var, val); } } - platform_shell::forward_windows_bootstrap_env(&mut cmd); + platform_shell::forward_windows_bootstrap_env(&mut cmd)?; for (k, v) in extra_env { cmd.env(k, v); } @@ -404,12 +405,13 @@ async fn execute_local_jail( cmd.env_clear(); for var in SANDBOX_ENV_PASSTHROUGH { if let Ok(val) = std::env::var(var) { - if !val.is_empty() { - cmd.env(var, val); + if val.is_empty() { + anyhow::bail!("sandbox passthrough environment variable {var} is empty"); } + cmd.env(var, val); } } - platform_shell::forward_windows_bootstrap_env_std(&mut cmd); + platform_shell::forward_windows_bootstrap_env_std(&mut cmd)?; for (k, v) in extra_env { cmd.env(k, v); } From e09aefebe5979afcf5a21a28c1a1b361eeaa3700 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 6 Oct 2026 23:27:13 +0300 Subject: [PATCH 6/8] fix: preserve Windows bootstrap environment Co-authored-by: Medulla --- crates/openhuman-core/src/sandbox/ops.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/openhuman-core/src/sandbox/ops.rs b/crates/openhuman-core/src/sandbox/ops.rs index 344719b9c1d..9cf37bdd831 100644 --- a/crates/openhuman-core/src/sandbox/ops.rs +++ b/crates/openhuman-core/src/sandbox/ops.rs @@ -271,6 +271,9 @@ async fn execute_unsandboxed( let mut cmd = platform_shell::build_tokio_command(command); cmd.current_dir(working_dir); cmd.env_clear(); + for (k, v) in extra_env { + cmd.env(k, v); + } for var in SANDBOX_ENV_PASSTHROUGH { if let Ok(val) = std::env::var(var) { if val.is_empty() { @@ -280,9 +283,6 @@ async fn execute_unsandboxed( } } platform_shell::forward_windows_bootstrap_env(&mut cmd)?; - for (k, v) in extra_env { - cmd.env(k, v); - } let result = tokio::time::timeout(timeout, cmd.output()).await; match result { @@ -403,6 +403,9 @@ async fn execute_local_jail( let mut cmd = platform_shell::build_std_command(&wrapped); cmd.current_dir(working_dir); cmd.env_clear(); + for (k, v) in extra_env { + cmd.env(k, v); + } for var in SANDBOX_ENV_PASSTHROUGH { if let Ok(val) = std::env::var(var) { if val.is_empty() { @@ -412,9 +415,6 @@ async fn execute_local_jail( } } platform_shell::forward_windows_bootstrap_env_std(&mut cmd)?; - for (k, v) in extra_env { - cmd.env(k, v); - } // Keep every Windows spelling of the temporary directory inside this // per-call grant. `TEMP`/`TMP` are the variables used by Windows tools; // `TMPDIR` covers Unix-oriented tools running on the same host. From 2cdba4cc2f55664f02701563eb5da3d578266168 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 6 Oct 2026 23:50:50 +0300 Subject: [PATCH 7/8] fix: preserve sandbox environment precedence Co-authored-by: Medulla --- crates/openhuman-core/src/sandbox/ops.rs | 12 +++---- tests/windows_sandbox_env_e2e.rs | 41 ++++++++++++++++++++---- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/crates/openhuman-core/src/sandbox/ops.rs b/crates/openhuman-core/src/sandbox/ops.rs index 9cf37bdd831..ccf0a7ffa90 100644 --- a/crates/openhuman-core/src/sandbox/ops.rs +++ b/crates/openhuman-core/src/sandbox/ops.rs @@ -271,9 +271,6 @@ async fn execute_unsandboxed( let mut cmd = platform_shell::build_tokio_command(command); cmd.current_dir(working_dir); cmd.env_clear(); - for (k, v) in extra_env { - cmd.env(k, v); - } for var in SANDBOX_ENV_PASSTHROUGH { if let Ok(val) = std::env::var(var) { if val.is_empty() { @@ -282,6 +279,9 @@ async fn execute_unsandboxed( cmd.env(var, val); } } + for (k, v) in extra_env { + cmd.env(k, v); + } platform_shell::forward_windows_bootstrap_env(&mut cmd)?; let result = tokio::time::timeout(timeout, cmd.output()).await; @@ -403,9 +403,6 @@ async fn execute_local_jail( let mut cmd = platform_shell::build_std_command(&wrapped); cmd.current_dir(working_dir); cmd.env_clear(); - for (k, v) in extra_env { - cmd.env(k, v); - } for var in SANDBOX_ENV_PASSTHROUGH { if let Ok(val) = std::env::var(var) { if val.is_empty() { @@ -414,6 +411,9 @@ async fn execute_local_jail( cmd.env(var, val); } } + for (k, v) in extra_env { + cmd.env(k, v); + } platform_shell::forward_windows_bootstrap_env_std(&mut cmd)?; // Keep every Windows spelling of the temporary directory inside this // per-call grant. `TEMP`/`TMP` are the variables used by Windows tools; diff --git a/tests/windows_sandbox_env_e2e.rs b/tests/windows_sandbox_env_e2e.rs index 8fa86994590..c93b50f0ae3 100644 --- a/tests/windows_sandbox_env_e2e.rs +++ b/tests/windows_sandbox_env_e2e.rs @@ -20,11 +20,16 @@ //! probe driven through `cmd.exe` from JavaScript reports it as present, so a //! JS harness cannot observe the stripping at all. +#[cfg(windows)] use std::collections::HashMap; +#[cfg(windows)] use std::time::Duration; +#[cfg(windows)] use openhuman_core::agent::harness::definition::SandboxMode; +#[cfg(windows)] use openhuman_core::config::RuntimeConfig; +#[cfg(windows)] use openhuman_core::sandbox::ops::{execute_in_sandbox, resolve_sandbox_policy}; /// Run `command` through OpenHuman's sandbox execution path under `mode`. @@ -32,8 +37,12 @@ use openhuman_core::sandbox::ops::{execute_in_sandbox, resolve_sandbox_policy}; async fn run_in_sandbox( mode: SandboxMode, command: &str, -) -> openhuman_core::sandbox::types::SandboxExecResult { +) -> ( + openhuman_core::sandbox::types::SandboxExecResult, + std::path::PathBuf, +) { let tempdir = tempfile::tempdir().expect("tempdir"); + let root = tempdir.path().to_path_buf(); let policy = resolve_sandbox_policy( mode, tempdir.path(), @@ -41,7 +50,7 @@ async fn run_in_sandbox( &RuntimeConfig::default(), false, ); - execute_in_sandbox( + let result = execute_in_sandbox( &policy, command, tempdir.path(), @@ -49,7 +58,8 @@ async fn run_in_sandbox( Duration::from_secs(120), ) .await - .unwrap_or_else(|e| panic!("execute_in_sandbox({mode:?}) failed to run the command: {e}")) + .unwrap_or_else(|e| panic!("execute_in_sandbox({mode:?}) failed to run the command: {e}")); + (result, root) } #[cfg(windows)] @@ -100,6 +110,22 @@ fn assert_bootstrap_env_resolved(result: &openhuman_core::sandbox::types::Sandbo } } +#[cfg(windows)] +fn assert_temp_is_in_scratch( + result: &openhuman_core::sandbox::types::SandboxExecResult, + root: &std::path::Path, +) { + let temp = bracketed(&result.stdout, "TEMP=").unwrap_or_default(); + let expected_prefix = root.join("artifacts").join("sandbox-scratch"); + let normalize = |path: &str| path.replace('/', "\\").to_ascii_lowercase(); + assert!( + normalize(&temp).starts_with(&normalize(&expected_prefix.to_string_lossy())), + "local-jail TEMP escaped its per-test scratch tree: TEMP={temp:?}, expected under {:?}; stdout {:?}", + expected_prefix, + result.stdout + ); +} + #[cfg(windows)] fn bracketed(stdout: &str, prefix: &str) -> Option { let tail = stdout.split(prefix).nth(1)?; @@ -111,7 +137,7 @@ fn bracketed(stdout: &str, prefix: &str) -> Option { #[cfg(windows)] #[tokio::test] async fn unsandboxed_child_receives_windows_bootstrap_env() { - let result = run_in_sandbox(SandboxMode::None, env_probe_command()).await; + let (result, _) = run_in_sandbox(SandboxMode::None, env_probe_command()).await; assert_bootstrap_env_resolved(&result); } @@ -121,8 +147,9 @@ async fn unsandboxed_child_receives_windows_bootstrap_env() { #[cfg(windows)] #[tokio::test] async fn sandboxed_child_receives_windows_bootstrap_env() { - let result = run_in_sandbox(SandboxMode::Sandboxed, env_probe_command()).await; + let (result, root) = run_in_sandbox(SandboxMode::Sandboxed, env_probe_command()).await; assert_bootstrap_env_resolved(&result); + assert_temp_is_in_scratch(&result, &root); } /// The reported Node failure, end to end through OpenHuman's spawn code: @@ -135,7 +162,7 @@ async fn node_crypto_runs_through_sandbox_path() { "node is required for node_crypto_runs_through_sandbox_path; missing tooling must not silently pass" ); - let result = run_in_sandbox( + let (result, _) = run_in_sandbox( SandboxMode::None, r#"node -e "console.log('SR=' + process.env.SystemRoot); console.log(require('crypto').randomBytes(8).toString('hex'))""#, ) @@ -174,7 +201,7 @@ async fn powershell_runs_through_sandbox_path() { "powershell.exe is required for powershell_runs_through_sandbox_path; missing tooling must not silently pass" ); - let result = run_in_sandbox( + let (result, _) = run_in_sandbox( SandboxMode::None, "powershell.exe -NoProfile -Command [guid]::NewGuid().ToString()", ) From 664251659a08a767bd7b46ae24333b0b287599ed Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 7 Oct 2026 00:14:44 +0300 Subject: [PATCH 8/8] fix: preserve sandbox temp overrides Co-authored-by: Medulla --- crates/openhuman-core/src/sandbox/ops.rs | 19 +++++++--- tests/windows_sandbox_env_e2e.rs | 46 +++++++++++++++++++++++- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/crates/openhuman-core/src/sandbox/ops.rs b/crates/openhuman-core/src/sandbox/ops.rs index ccf0a7ffa90..2ca9b0de421 100644 --- a/crates/openhuman-core/src/sandbox/ops.rs +++ b/crates/openhuman-core/src/sandbox/ops.rs @@ -279,10 +279,10 @@ async fn execute_unsandboxed( cmd.env(var, val); } } + platform_shell::forward_windows_bootstrap_env(&mut cmd)?; for (k, v) in extra_env { cmd.env(k, v); } - platform_shell::forward_windows_bootstrap_env(&mut cmd)?; let result = tokio::time::timeout(timeout, cmd.output()).await; match result { @@ -396,6 +396,9 @@ async fn execute_local_jail( jail = jail.add_read_write(&scratch.path); let stdout_file = capture.stdout(); let stderr_file = capture.stderr(); + let caller_sets_tmpdir = extra_env.contains_key(std::ffi::OsStr::new("TMPDIR")); + let caller_sets_temp = extra_env.contains_key(std::ffi::OsStr::new("TEMP")); + let caller_sets_tmp = extra_env.contains_key(std::ffi::OsStr::new("TMP")); // Platform-aware output-capture wrap: `{ … ; } > … 2> …` on sh/bash, // trailing `> … 2> …` on cmd.exe (no brace grouping). Shell binary is // picked by `platform_shell` so this path is Windows-safe (#4705). @@ -411,16 +414,22 @@ async fn execute_local_jail( cmd.env(var, val); } } + platform_shell::forward_windows_bootstrap_env_std(&mut cmd)?; for (k, v) in extra_env { cmd.env(k, v); } - platform_shell::forward_windows_bootstrap_env_std(&mut cmd)?; // Keep every Windows spelling of the temporary directory inside this // per-call grant. `TEMP`/`TMP` are the variables used by Windows tools; // `TMPDIR` covers Unix-oriented tools running on the same host. - cmd.env("TMPDIR", &scratch.path); - cmd.env("TEMP", &scratch.path); - cmd.env("TMP", &scratch.path); + if !caller_sets_tmpdir { + cmd.env("TMPDIR", &scratch.path); + } + if !caller_sets_temp { + cmd.env("TEMP", &scratch.path); + } + if !caller_sets_tmp { + cmd.env("TMP", &scratch.path); + } let os_backend = cwd_jail::default_backend(); let spawn_result = if os_backend.is_available() { diff --git a/tests/windows_sandbox_env_e2e.rs b/tests/windows_sandbox_env_e2e.rs index c93b50f0ae3..300254113fd 100644 --- a/tests/windows_sandbox_env_e2e.rs +++ b/tests/windows_sandbox_env_e2e.rs @@ -40,6 +40,18 @@ async fn run_in_sandbox( ) -> ( openhuman_core::sandbox::types::SandboxExecResult, std::path::PathBuf, +) { + run_in_sandbox_with_env(mode, command, HashMap::new()).await +} + +#[cfg(windows)] +async fn run_in_sandbox_with_env( + mode: SandboxMode, + command: &str, + extra_env: HashMap, +) -> ( + openhuman_core::sandbox::types::SandboxExecResult, + std::path::PathBuf, ) { let tempdir = tempfile::tempdir().expect("tempdir"); let root = tempdir.path().to_path_buf(); @@ -54,7 +66,10 @@ async fn run_in_sandbox( &policy, command, tempdir.path(), - HashMap::new(), + extra_env + .into_iter() + .map(|(key, value)| (key.into(), value.into())) + .collect(), Duration::from_secs(120), ) .await @@ -152,6 +167,35 @@ async fn sandboxed_child_receives_windows_bootstrap_env() { assert_temp_is_in_scratch(&result, &root); } +/// Caller-provided temporary-directory values remain effective on both host +/// spawn paths; the local jail must not silently replace per-call overrides. +#[cfg(windows)] +#[tokio::test] +async fn caller_temp_overrides_survive_sandbox_paths() { + let expected = r"C:\openhuman-test-temp"; + for mode in [SandboxMode::None, SandboxMode::Sandboxed] { + let (result, _) = run_in_sandbox_with_env( + mode, + "echo TEMP=[%TEMP%] TMP=[%TMP%] TMPDIR=[%TMPDIR%]", + HashMap::from([ + ("TEMP".to_string(), expected.to_string()), + ("TMP".to_string(), expected.to_string()), + ("TMPDIR".to_string(), expected.to_string()), + ]), + ) + .await; + assert!(result.success(), "child failed: {:?}", result.stderr); + for name in ["TEMP", "TMP", "TMPDIR"] { + assert_eq!( + bracketed(&result.stdout, &format!("{name}=")).as_deref(), + Some(expected), + "caller override for {name} was replaced; stdout {:?}", + result.stdout + ); + } + } +} + /// The reported Node failure, end to end through OpenHuman's spawn code: /// `node -e` must reach the crypto provider and print random bytes. #[cfg(windows)]