fix(windows): forward process bootstrap env into sandboxed children

crates/openhuman-cli/Cargo.toml

@@ -195,6 +195,10 @@ path = "../../tests/composio_list_tools_stack_overflow_regression.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 = "inference_provider_e2e"
 path = "../../tests/inference_provider_e2e.rs"

crates/openhuman-core/src/agent/platform_shell.rs

@@ -29,6 +29,109 @@
 
 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 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",
+    "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."
+        );
+    }
+}
+
+/// 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) -> anyhow::Result<()> {
+    for var in WINDOWS_PROCESS_ENV_VARS {
+        if let Ok(val) = std::env::var(var) {
+            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) -> anyhow::Result<()> {
+    Ok(())
+}
+
+#[cfg(windows)]
+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() {
+                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) -> anyhow::Result<()> {
+    Ok(())
+}
+
 /// Whether the Unix arm prefixes `set -o pipefail`.
 #[derive(Clone, Copy, PartialEq, Eq)]
 enum PipeFail {

crates/openhuman-core/src/sandbox/ops.rs

@@ -17,6 +17,20 @@ use std::path::{Path, PathBuf};
 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. Windows process-bootstrap variables are
+/// added only in the host spawn paths below; keeping them out of this policy
+/// prevents Windows paths from being passed into Linux Docker containers.
+///
+/// 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 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",
 ];
@@ -259,9 +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() {
+                anyhow::bail!("sandbox passthrough environment variable {var} is empty");
+            }
             cmd.env(var, val);
         }
     }
+    platform_shell::forward_windows_bootstrap_env(&mut cmd)?;
     for (k, v) in extra_env {
         cmd.env(k, v);
     }
@@ -378,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).
@@ -387,13 +408,28 @@ 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() {
+                anyhow::bail!("sandbox passthrough environment variable {var} is empty");
+            }
             cmd.env(var, val);
         }
     }
-    cmd.env("TMPDIR", &scratch.path);
+    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.
+    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() {

crates/openhuman-core/src/sandbox/ops_tests.rs

@@ -322,6 +322,23 @@ 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.
+
+// 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.
 // ── #6961: local-jail output capture stays out of the user's project ─────────
 
 #[cfg(unix)]

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",
+    );
+}

crates/openhuman-core/src/tools/impl/system/npm_exec_tests.rs

@@ -75,10 +75,8 @@ fn resolve_cwd_allows_relative_subdir() {
 
 #[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"
-        );
-    }
+    crate::agent::platform_shell::assert_forwards_windows_bootstrap(
+        SAFE_ENV_VARS,
+        "npm_exec::SAFE_ENV_VARS",
+    );
 }

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",
+    );
+}

crates/openhuman-core/src/tools/impl/system/shell_tests_schema_and_env_tests.rs

@@ -618,10 +618,8 @@ fn shell_safe_env_vars_includes_essentials() {
 
 #[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"
-        );
-    }
+    crate::agent::platform_shell::assert_forwards_windows_bootstrap(
+        SAFE_ENV_VARS,
+        "shell::SAFE_ENV_VARS",
+    );
 }