From a2f7820e27f9812504d632227ffdbad80342b626 Mon Sep 17 00:00:00 2001 From: Sayr Wolfridge <267323413+SayrWolfridge@users.noreply.github.com> Date: Tue, 6 Oct 2026 20:59:10 +0300 Subject: [PATCH 1/3] Preserve host Rust toolchain homes in native sandbox commands --- crates/openhuman-core/src/sandbox/ops.rs | 16 +++++++++-- .../openhuman-core/src/sandbox/ops_tests.rs | 27 +++++++++++++++++++ docs/RELEASE-MANUAL-SMOKE.md | 2 ++ docs/TEST-COVERAGE-MATRIX.md | 2 +- 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/crates/openhuman-core/src/sandbox/ops.rs b/crates/openhuman-core/src/sandbox/ops.rs index 84a0d60806a..cd8fe7c7fa0 100644 --- a/crates/openhuman-core/src/sandbox/ops.rs +++ b/crates/openhuman-core/src/sandbox/ops.rs @@ -21,6 +21,10 @@ pub const SANDBOX_ENV_PASSTHROUGH: &[&str] = &[ "PATH", "HOME", "TERM", "LANG", "LC_ALL", "LC_CTYPE", "USER", "SHELL", "TMPDIR", ]; +/// Native toolchain homes are available to host-local commands only. Docker +/// keeps its own image-provided Rust toolchain environment. +const HOST_TOOLCHAIN_ENV_PASSTHROUGH: &[&str] = &["RUSTUP_HOME", "CARGO_HOME"]; + /// Host switch that turns the agent sandbox off for the whole process. /// /// For hosts that already isolate the core (a container, a CI or benchmark @@ -257,7 +261,11 @@ async fn execute_unsandboxed( let mut cmd = platform_shell::build_tokio_command(command); cmd.current_dir(working_dir); cmd.env_clear(); - for var in SANDBOX_ENV_PASSTHROUGH { + for var in SANDBOX_ENV_PASSTHROUGH + .iter() + .chain(HOST_TOOLCHAIN_ENV_PASSTHROUGH.iter()) + .copied() + { if let Ok(val) = std::env::var(var) { cmd.env(var, val); } @@ -385,7 +393,11 @@ async fn execute_local_jail( let mut cmd = platform_shell::build_std_command(&wrapped); cmd.current_dir(working_dir); cmd.env_clear(); - for var in SANDBOX_ENV_PASSTHROUGH { + for var in SANDBOX_ENV_PASSTHROUGH + .iter() + .chain(HOST_TOOLCHAIN_ENV_PASSTHROUGH.iter()) + .copied() + { if let Ok(val) = std::env::var(var) { cmd.env(var, val); } diff --git a/crates/openhuman-core/src/sandbox/ops_tests.rs b/crates/openhuman-core/src/sandbox/ops_tests.rs index b9fe871c2ae..28b065bba3d 100644 --- a/crates/openhuman-core/src/sandbox/ops_tests.rs +++ b/crates/openhuman-core/src/sandbox/ops_tests.rs @@ -50,6 +50,13 @@ fn resolve_sandbox_policy_sandboxed_remote_uses_docker() { assert_eq!(policy.backend, SandboxBackendKind::Docker); assert!(!policy.allow_network); assert!(policy.docker_overrides.is_some()); + assert_eq!( + policy.env_passthrough, + ["PATH", "HOME", "TERM", "LANG", "LC_ALL", "LC_CTYPE", "USER", "SHELL", "TMPDIR"] + .map(str::to_owned) + .to_vec(), + "Docker keeps its established general environment allowlist" + ); } #[test] @@ -512,6 +519,26 @@ async fn landlock_jail_runs_cargo_and_mktemp_but_blocks_writes_outside() { let r = run_local(&policy, "cargo --version").await; assert!(r.success(), "cargo failed under the jail: {}", r.stderr); assert!(r.stdout.starts_with("cargo "), "stdout: {}", r.stdout); + + // The fixture uses the host's real toolchain, so compare the sandbox + // values with the environment that selected that toolchain. This avoids + // process-global env mutation and exercises the actual spawn path. + let expected_homes = ["RUSTUP_HOME", "CARGO_HOME"].map(|name| { + std::env::var_os(name) + .map(|value| value.to_string_lossy().into_owned()) + .unwrap_or_default() + }); + let r = run_local( + &policy, + "printf '%s\\n' \"${RUSTUP_HOME-}\" \"${CARGO_HOME-}\"", + ) + .await; + assert!(r.success(), "toolchain home probe failed: {}", r.stderr); + assert_eq!( + r.stdout, + format!("{}\n{}\n", expected_homes[0], expected_homes[1]), + "sandboxed commands must inherit explicitly configured Rust toolchain homes" + ); } else { eprintln!("SKIP cargo: not installed on this host"); } diff --git a/docs/RELEASE-MANUAL-SMOKE.md b/docs/RELEASE-MANUAL-SMOKE.md index bc34249d4fc..7af1e4fe994 100644 --- a/docs/RELEASE-MANUAL-SMOKE.md +++ b/docs/RELEASE-MANUAL-SMOKE.md @@ -129,3 +129,5 @@ Notes: ``` Paste the filled block as a commit comment on the `v-staging` tagged commit before promoting to production. + +- [ ] **Native sandbox preserves installed Rust toolchain homes** — In an isolated Linux profile with Rust installed through explicit `RUSTUP_HOME` and `CARGO_HOME`, run `cargo --version` through the sandboxed shell. Expected: the installed Cargo version is returned, temporary files use the sandbox scratch directory, workspace writes succeed, and a write to a separately prepared outside directory is denied. Verify the configured homes match the values observed inside the native sandbox. diff --git a/docs/TEST-COVERAGE-MATRIX.md b/docs/TEST-COVERAGE-MATRIX.md index ad4b32c44be..7c1072f11c4 100644 --- a/docs/TEST-COVERAGE-MATRIX.md +++ b/docs/TEST-COVERAGE-MATRIX.md @@ -272,7 +272,7 @@ End-to-end coverage of the agent harness via the web-chat RPC surface against an | ID | Feature | Layer | Test path(s) | Status | Notes | | ----- | ---------------------------- | ----- | --------------------------------------------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------ | -| 6.2.1 | Shell Command Execution | RU+WD | `crates/openhuman-core/src/tools/impl/system/shell.rs`, `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO asserts agent runtime + the shell-bearing agent registry contract (the orchestrator since `tools_agent` was removed); full LLM path tracked #68 | +| 6.2.1 | Shell Command Execution | RU+WD | `crates/openhuman-core/src/tools/impl/system/shell.rs`, `crates/openhuman-core/src/sandbox/ops_tests.rs`, `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO asserts agent runtime + the shell-bearing agent registry contract (the orchestrator since `tools_agent` was removed); full LLM path tracked #68; Rust toolchain startup is covered through the real Landlock Cargo fixture, including explicit host toolchain homes, scratch files and outside-workspace write denial | | 6.2.2 | Command Restriction Handling | RU+WD | `crates/openhuman-core/src/security/policy/policy_tests.rs` (+ `policy_injection_tests.rs`), `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO locks denial envelope shape `{ ok:false, error }` consumed by the React UI | | 6.2.3 | Git Read Operations | RU+WD | `vendor/tinyagents/vendor/tinytools/crates/tinytools-std/src/filesystem/git_operations/test.rs`, `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO seeds a fixture repo in OPENHUMAN_WORKSPACE and asserts read ops succeed | | 6.2.4 | Git Write Operations | RU+WD | `vendor/tinyagents/vendor/tinytools/crates/tinytools-std/src/filesystem/git_operations/test.rs`, `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO commits into the same fixture and asserts log advances | From 3ecf0d656d9cf7d74c8260a070c5c30924766630 Mon Sep 17 00:00:00 2001 From: Sayr Wolfridge <267323413+SayrWolfridge@users.noreply.github.com> Date: Wed, 7 Oct 2026 05:35:11 +0300 Subject: [PATCH 2/3] test(sandbox): verify toolchain homes through the agent shell --- docs/TEST-COVERAGE-MATRIX.md | 2 +- tests/agent_harness_e2e.rs | 192 +++++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 1 deletion(-) diff --git a/docs/TEST-COVERAGE-MATRIX.md b/docs/TEST-COVERAGE-MATRIX.md index b1e81b54419..69d84f38703 100644 --- a/docs/TEST-COVERAGE-MATRIX.md +++ b/docs/TEST-COVERAGE-MATRIX.md @@ -272,7 +272,7 @@ End-to-end coverage of the agent harness via the web-chat RPC surface against an | ID | Feature | Layer | Test path(s) | Status | Notes | | ----- | ---------------------------- | ----- | --------------------------------------------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------ | -| 6.2.1 | Shell Command Execution | RU+WD | `crates/openhuman-core/src/tools/impl/system/shell.rs`, `crates/openhuman-core/src/sandbox/ops_tests.rs`, `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO asserts agent runtime + the shell-bearing agent registry contract (the orchestrator since `tools_agent` was removed); full LLM path tracked #68; Rust toolchain startup is covered through the real Landlock Cargo fixture, including explicit host toolchain homes, scratch files and outside-workspace write denial | +| 6.2.1 | Shell Command Execution | RU+RI+WD | `crates/openhuman-core/src/tools/impl/system/shell.rs`, `crates/openhuman-core/src/sandbox/ops_tests.rs`, `tests/agent_harness_e2e.rs` (`sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes`), `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO asserts agent runtime + the shell-bearing agent registry contract (the orchestrator since `tools_agent` was removed); full LLM path tracked #68; Rust toolchain startup is covered through the real Landlock Cargo fixture and an agent-shell RPC/model-loop E2E, including observed host homes, Cargo startup, scratch files, workspace writes and outside-workspace write denial | | 6.2.2 | Command Restriction Handling | RU+WD | `crates/openhuman-core/src/security/policy/policy_tests.rs` (+ `policy_injection_tests.rs`), `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO locks denial envelope shape `{ ok:false, error }` consumed by the React UI | | 6.2.3 | Git Read Operations | RU+WD | `vendor/tinyagents/vendor/tinytools/crates/tinytools-std/src/filesystem/git_operations/test.rs`, `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO seeds a fixture repo in OPENHUMAN_WORKSPACE and asserts read ops succeed | | 6.2.4 | Git Write Operations | RU+WD | `vendor/tinyagents/vendor/tinytools/crates/tinytools-std/src/filesystem/git_operations/test.rs`, `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO commits into the same fixture and asserts log advances | diff --git a/tests/agent_harness_e2e.rs b/tests/agent_harness_e2e.rs index a1b73c7aed9..6154ef01c4c 100644 --- a/tests/agent_harness_e2e.rs +++ b/tests/agent_harness_e2e.rs @@ -5159,3 +5159,195 @@ async fn a_wrong_tool_guess_is_retried_not_treated_as_an_auth_blocker_inner() { stack.shutdown(); } + +/// The real sandboxed orchestrator shell receives the host toolchain homes, +/// can run Cargo and use its call-local scratch directory, and remains confined +/// to its configured action directory. +#[cfg(target_os = "linux")] +#[test] +fn sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes() { + run_on_agent_stack( + "sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes", + sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes_inner, + ); +} + +#[cfg(target_os = "linux")] +async fn sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes_inner() { + let _lock = env_lock(); + + // Give this real orchestrator turn a private action directory and make the + // host sandbox switch explicit. HOME is separately replaced by the stack + // fixture, so these guards pin the actual host Rust toolchain locations. + let action_dir = tempdir().expect("action directory"); + let outside_dir = tempdir().expect("outside directory"); + let host_home = std::env::var_os("HOME") + .map(std::path::PathBuf::from) + .expect("host HOME must be set before the stack fixture replaces it"); + let rustup_home = std::env::var_os("RUSTUP_HOME") + .map(std::path::PathBuf::from) + .unwrap_or_else(|| host_home.join(".rustup")); + let cargo_home = std::env::var_os("CARGO_HOME") + .map(std::path::PathBuf::from) + .unwrap_or_else(|| host_home.join(".cargo")); + assert!( + rustup_home.is_absolute(), + "host RUSTUP_HOME must be absolute" + ); + assert!(cargo_home.is_absolute(), "host CARGO_HOME must be absolute"); + assert!(rustup_home.is_dir(), "host RUSTUP_HOME must exist"); + assert!(cargo_home.is_dir(), "host CARGO_HOME must exist"); + let cargo_path = std::env::split_paths( + &std::env::var_os("PATH").expect("host PATH must be set before the stack fixture"), + ) + .map(|dir| dir.join("cargo")) + .find(|candidate| candidate.is_file()) + .expect("cargo must be present on the pinned Linux test PATH"); + let cargo_bin = cargo_path + .parent() + .expect("cargo executable must have a parent directory") + .canonicalize() + .expect("cargo executable directory must exist"); + let marker_name = format!("agent-harness-shell-{}.txt", std::process::id()); + let marker_path = action_dir.path().join(&marker_name); + let outside_path = outside_dir.path().join("must-stay-unwritable.txt"); + let command = format!( + "set -eu; export PATH={}:$PATH; printf 'RUSTUP_HOME=%s\\nCARGO_HOME=%s\\n' \"$RUSTUP_HOME\" \"$CARGO_HOME\"; printf 'CARGO_EXE=%s\\n' \"$(command -v cargo)\"; cargo --version; printf 'workspace-write-ok\\n' > '{marker_name}'; test \"$(cat '{marker_name}')\" = workspace-write-ok; printf 'WORKSPACE_WRITE=ok\\n'; if printf 'outside-write\\n' > {}; then printf 'OUTSIDE_WRITE=allowed\\n'; else printf 'OUTSIDE_WRITE=blocked\\n'; fi; scratch=$(mktemp); test -f \"$scratch\"; printf 'MKTEMP=ok\\n'; rm -f \"$scratch\"", + shell_single_quote(&cargo_bin.to_string_lossy()), + shell_single_quote(&outside_path.to_string_lossy()) + ); + reset_script(vec![ + tool_call_completion("shell", json!({ "command": command, "category": "write" })), + text_completion("SANDBOXED_SHELL_TURN_COMPLETED"), + ]); + + let _action_dir_guard = EnvVarGuard::set_to_path("OPENHUMAN_ACTION_DIR", action_dir.path()); + let _sandbox_guard = EnvVarGuard::set("OPENHUMAN_SANDBOX", "on"); + let _rustup_home_guard = EnvVarGuard::set_to_path("RUSTUP_HOME", &rustup_home); + let _cargo_home_guard = EnvVarGuard::set_to_path("CARGO_HOME", &cargo_home); + let stack = boot_stack().await; + // The shell's default local jail grants HOME/.rustup and HOME/.cargo/bin. + // Link only those temporary-home entries to the captured host locations; + // the default grant resolver canonicalizes them before spawning the jail. + std::os::unix::fs::symlink(&rustup_home, stack._tmp.path().join(".rustup")) + .expect("link host Rust home into temporary HOME"); + let fixture_cargo_home = stack._tmp.path().join(".cargo"); + std::fs::create_dir_all(&fixture_cargo_home).expect("create temporary Cargo home"); + std::os::unix::fs::symlink(&cargo_bin, fixture_cargo_home.join("bin")) + .expect("link host Cargo executable directory into temporary HOME"); + let mut events = spawn_sse_collector(format!( + "{}/events?client_id=harness-sandboxed-shell", + stack.rpc_base + )) + .await; + let request_id = send_web_chat( + &stack.rpc_base, + 920, + "harness-sandboxed-shell", + "thread-sandboxed-shell", + "Run the requested shell check.", + ) + .await; + let (terminal, results) = + collect_turn_tool_results_request(&mut events, Duration::from_secs(120), Some(&request_id)) + .await; + assert_eq!( + terminal.get("event").and_then(Value::as_str), + Some("chat_done"), + "sandboxed shell turn should complete: {terminal}" + ); + let shell_result = results + .iter() + .find(|frame| frame.get("tool_call_id").and_then(Value::as_str) == Some("call_shell")) + .unwrap_or_else(|| panic!("missing shell tool_result event: {results:?}")); + assert_eq!( + shell_result.get("success"), + Some(&json!(true)), + "shell command should succeed: {shell_result}" + ); + assert!(marker_path.is_file(), "workspace write did not persist"); + assert!( + !outside_path.exists(), + "the local jail allowed a write outside action_dir: {}", + outside_path.display() + ); + + // The second model request must carry the actual shell result. This proves + // the assertions observe the real tool response returned through the agent + // loop, rather than a scripted model answer or an out-of-band probe. + let requests = with_captured(|captured| captured.clone()); + assert!( + requests.len() >= 2, + "shell result was not sent to the model" + ); + let call_id = "call_shell"; + let marker = format!(""); + let model_saw_result = requests.iter().skip(1).any(|request| { + request + .pointer("/body/messages") + .and_then(Value::as_array) + .is_some_and(|messages| { + messages.iter().any(|message| { + if message.get("role").and_then(Value::as_str) == Some("tool") + && message.get("tool_call_id").and_then(Value::as_str) == Some(call_id) + { + let content = message + .get("content") + .and_then(Value::as_str) + .unwrap_or_default(); + return shell_result_contains_all_markers( + content, + &rustup_home, + &cargo_home, + &cargo_bin, + ); + } + message + .get("content") + .and_then(Value::as_str) + .and_then(|content| { + content + .split_once(&marker) + .and_then(|(_, result)| result.split_once("")) + .map(|(result, _)| { + shell_result_contains_all_markers( + result, + &rustup_home, + &cargo_home, + &cargo_bin, + ) + }) + }) + .unwrap_or(false) + }) + }) + }); + assert!( + model_saw_result, + "the subsequent model request did not contain the successful shell result: {}", + serde_json::to_string_pretty(&requests).unwrap_or_default() + ); + + stack.shutdown(); +} + +#[cfg(target_os = "linux")] +fn shell_result_contains_all_markers( + result: &str, + rustup_home: &std::path::Path, + cargo_home: &std::path::Path, + cargo_bin: &std::path::Path, +) -> bool { + result.contains(format!("RUSTUP_HOME={}", rustup_home.display()).as_str()) + && result.contains(format!("CARGO_HOME={}", cargo_home.display()).as_str()) + && result.contains(format!("CARGO_EXE={}", cargo_bin.join("cargo").display()).as_str()) + && result.contains("cargo ") + && result.contains("WORKSPACE_WRITE=ok") + && result.contains("OUTSIDE_WRITE=blocked") + && result.contains("MKTEMP=ok") +} + +#[cfg(target_os = "linux")] +fn shell_single_quote(value: &str) -> String { + format!("'{}'", value.replace('\'', "'\\''")) +} From 2285292f7f81f17622ef2b6d2092dbc102e72baf Mon Sep 17 00:00:00 2001 From: Sayr Wolfridge <267323413+SayrWolfridge@users.noreply.github.com> Date: Wed, 7 Oct 2026 06:28:18 +0300 Subject: [PATCH 3/3] fix(sandbox): admit selective custom Rust toolchain homes --- crates/openhuman-core/src/sandbox/grants.rs | 130 +++++++- .../src/sandbox/grants_tests.rs | 248 +++++++++++++++ .../openhuman-core/src/sandbox/ops_tests.rs | 25 +- .../src/sandbox/ops_toolchain_tests.rs | 104 +++++++ docs/RELEASE-MANUAL-SMOKE.md | 4 +- docs/TEST-COVERAGE-MATRIX.md | 2 +- tests/agent_harness_e2e.rs | 287 +++++++++++++++--- 7 files changed, 737 insertions(+), 63 deletions(-) create mode 100644 crates/openhuman-core/src/sandbox/ops_toolchain_tests.rs diff --git a/crates/openhuman-core/src/sandbox/grants.rs b/crates/openhuman-core/src/sandbox/grants.rs index 7aaecac3978..580d3af9b34 100644 --- a/crates/openhuman-core/src/sandbox/grants.rs +++ b/crates/openhuman-core/src/sandbox/grants.rs @@ -30,6 +30,9 @@ const HOME_READ_ONLY_DIRS: &[&str] = &[".rustup", ".nvm", ".npm"]; /// holds registry credentials and so cannot be granted whole. const CARGO_READ_ONLY_FILES: &[&str] = &["config.toml", "config", "env"]; +/// Cargo credential locations excluded by the selective Cargo-home helper. +const CARGO_CREDENTIAL_PATHS: &[&str] = &["credentials", "credentials.toml"]; + /// Git config files read at startup, relative to `$HOME`. const GITCONFIG_FILES: &[&str] = &[".gitconfig", ".config/git/config", ".config/git/ignore"]; @@ -56,6 +59,7 @@ pub fn resolve_local_jail_grants(home: Option<&Path>, cfg: &LocalJailConfig) -> b.read_only(&h.join(dir), "toolchain"); } } + b.add_host_toolchain_homes(); for dir in SYSTEM_TOOLCHAIN_DIRS { b.read_only(Path::new(dir), "toolchain"); } @@ -167,19 +171,133 @@ impl<'a> Builder<'a> { } } - /// Grant only the parts of `~/.cargo` that jailed Cargo needs. The root is + /// Known Cargo roots, including their canonical targets when available. + /// This is intentionally local to Rust-home grants: system and explicit + /// extra grants retain their existing policy. + fn cargo_roots(&self) -> Vec { + let mut roots = Vec::new(); + if let Some(home) = self.home { + roots.push(home.join(".cargo")); + } + if let Ok(raw) = std::env::var("CARGO_HOME") { + let configured = PathBuf::from(raw); + if configured.is_absolute() { + roots.push(configured); + } + } + let mut normalized = Vec::new(); + for root in roots { + let root = root.canonicalize().unwrap_or(root); + if !normalized.contains(&root) { + normalized.push(root); + } + } + normalized + } + + /// Cargo's piecewise grants may include children of Cargo roots, but may + /// not collapse through a symlink onto a root or credentials. A custom + /// Rustup root is broader and therefore may overlap neither. + fn cargo_grant_overlaps( + candidate: &Path, + roots: &[PathBuf], + strict_root: bool, + writable: bool, + ) -> bool { + roots.iter().any(|root| { + (candidate == root || root.starts_with(candidate)) + || (strict_root && candidate.starts_with(root)) + }) || roots.iter().any(|root| { + CARGO_CREDENTIAL_PATHS.iter().any(|name| { + let credential = root.join(name); + let credential = credential.canonicalize().unwrap_or(credential); + candidate == credential + || candidate.starts_with(&credential) + || credential.starts_with(candidate) + }) + }) || (writable + && roots.iter().any(|root| { + std::iter::once("bin") + .chain(CARGO_READ_ONLY_FILES.iter().copied()) + .any(|name| { + let protected = root.join(name); + let protected = protected.canonicalize().unwrap_or(protected); + candidate == protected + || candidate.starts_with(&protected) + || protected.starts_with(candidate) + }) + })) + } + + /// Admit explicitly selected host Rust homes when the built-in toolchain + /// grants are enabled. Match `ops`' Unicode environment-value semantics; + /// jail admission additionally requires absolute paths to existing dirs. + fn add_host_toolchain_homes(&mut self) { + if let Ok(raw) = std::env::var("RUSTUP_HOME") { + let rustup = PathBuf::from(raw); + if rustup.is_absolute() && rustup.is_dir() { + if let Some(path) = self.admit(&rustup, "RUSTUP_HOME") { + if !Self::cargo_grant_overlaps(&path, &self.cargo_roots(), true, false) { + self.record_read_only(path); + } + } + } + } + + if let Ok(raw) = std::env::var("CARGO_HOME") { + let cargo = PathBuf::from(raw); + if cargo.is_absolute() { + self.add_cargo_home(&cargo); + } + } + } + + /// Grant only the parts of a Cargo home that jailed Cargo needs. Its root is /// never writable: `bin` and Cargo configuration run later outside the /// jail, so writable access there would persist an escape for host tools. fn add_cargo_home(&mut self, cargo: &Path) { + let Ok(cargo) = cargo.canonicalize() else { + return; + }; if !cargo.is_dir() { return; } - tracing::debug!("[sandbox:grants] granting ~/.cargo parts with host code read-only"); - self.read_only(&cargo.join("bin"), "toolchain"); - self.read_write(&cargo.join("registry"), "toolchain"); - self.read_write(&cargo.join("git"), "toolchain"); + tracing::debug!("[sandbox:grants] granting Cargo home parts with host code read-only"); + self.cargo_read_only(&cargo.join("bin"), "toolchain"); + self.cargo_read_write(&cargo.join("registry"), "toolchain"); + self.cargo_read_write(&cargo.join("git"), "toolchain"); for f in CARGO_READ_ONLY_FILES { - self.read_only(&cargo.join(f), "toolchain"); + self.cargo_read_only(&cargo.join(f), "toolchain"); + } + } + + fn cargo_read_only(&mut self, path: &Path, source: &str) { + if let Some(path) = self.admit(path, source) { + if !Self::cargo_grant_overlaps(&path, &self.cargo_roots(), false, false) { + self.record_read_only(path); + } + } + } + + fn cargo_read_write(&mut self, path: &Path, source: &str) { + if let Some(path) = self.admit(path, source) { + if !Self::cargo_grant_overlaps(&path, &self.cargo_roots(), false, true) { + self.record_read_write(path); + } + } + } + + fn record_read_only(&mut self, path: PathBuf) { + if self.seen.insert(path.clone()) { + self.grants.read_only.push(path); + } + } + + fn record_read_write(&mut self, path: PathBuf) { + self.grants.read_only.retain(|grant| grant != &path); + self.seen.insert(path.clone()); + if !self.grants.read_write.contains(&path) { + self.grants.read_write.push(path); } } diff --git a/crates/openhuman-core/src/sandbox/grants_tests.rs b/crates/openhuman-core/src/sandbox/grants_tests.rs index e490e6c464a..2256b5f1f33 100644 --- a/crates/openhuman-core/src/sandbox/grants_tests.rs +++ b/crates/openhuman-core/src/sandbox/grants_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::config::test_env::EnvVarGuard; use std::fs; use std::path::{Path, PathBuf}; @@ -24,6 +25,12 @@ fn all(g: &JailGrants) -> Vec { g.read_only.iter().chain(&g.read_write).cloned().collect() } +fn isolated_toolchain_env() -> EnvVarGuard { + EnvVarGuard::locked() + .without("RUSTUP_HOME") + .without("CARGO_HOME") +} + /// A grant reaches a credential store if it IS one, is inside one, or contains /// one (Landlock grants are recursive, so a parent grant exposes the child). fn reaches_credentials(grants: &[PathBuf], home: &Path) -> Option { @@ -40,6 +47,7 @@ fn reaches_credentials(grants: &[PathBuf], home: &Path) -> Option { #[test] fn credential_dirs_are_never_granted_by_default() { + let _env = isolated_toolchain_env(); let home = fake_home(); let g = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); assert_eq!(reaches_credentials(&all(&g), home.path()), None); @@ -47,6 +55,7 @@ fn credential_dirs_are_never_granted_by_default() { #[test] fn credential_dirs_are_never_granted_even_when_extras_ask() { + let _env = isolated_toolchain_env(); let home = fake_home(); let cfg = LocalJailConfig { extra_read_only: vec![ @@ -67,6 +76,7 @@ fn credential_dirs_are_never_granted_even_when_extras_ask() { #[test] fn credential_floor_holds_for_a_symlink_into_a_credential_dir() { + let _env = isolated_toolchain_env(); let home = fake_home(); std::os::unix::fs::symlink(home.path().join(".ssh"), home.path().join("innocent")).unwrap(); let cfg = LocalJailConfig { @@ -79,6 +89,7 @@ fn credential_floor_holds_for_a_symlink_into_a_credential_dir() { #[test] fn toolchain_homes_are_granted_when_they_exist() { + let _env = isolated_toolchain_env(); let home = fake_home(); let cargo = home.path().join(".cargo"); for dir in ["bin", "registry", "git"] { @@ -117,8 +128,228 @@ fn toolchain_homes_are_granted_when_they_exist() { } } +#[test] +fn configured_absolute_rust_homes_outside_home_use_selective_grants() { + let home = fake_home(); + let install = tempfile::tempdir().unwrap(); + let rustup = install.path().join("rustup-home"); + let cargo = install.path().join("cargo-home"); + fs::create_dir_all(&rustup).unwrap(); + for dir in ["bin", "registry", "git"] { + fs::create_dir_all(cargo.join(dir)).unwrap(); + } + for file in ["config.toml", "config", "env", "credentials.toml"] { + fs::write(cargo.join(file), "").unwrap(); + } + + let _env = EnvVarGuard::locked() + .with("RUSTUP_HOME", &rustup) + .with("CARGO_HOME", &cargo); + let grants = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); + let rustup = canon(&rustup); + let cargo = canon(&cargo); + + assert!(has(&grants.read_only, &rustup)); + assert!(!has(&grants.read_write, &rustup)); + assert!(!all(&grants).iter().any(|path| path == &cargo)); + assert!(has(&grants.read_only, &cargo.join("bin"))); + for file in ["config.toml", "config", "env"] { + assert!( + has(&grants.read_only, &cargo.join(file)), + "{file} should be read-only" + ); + } + assert!(has(&grants.read_write, &cargo.join("registry"))); + assert!(has(&grants.read_write, &cargo.join("git"))); + assert!(!all(&grants) + .iter() + .any(|path| path == &cargo.join("credentials.toml"))); + assert_eq!(reaches_credentials(&all(&grants), home.path()), None); +} + +#[cfg(unix)] +#[test] +fn cargo_symlink_aliases_cannot_reach_root_or_credentials() { + let home = fake_home(); + let install = tempfile::tempdir().unwrap(); + let cargo = install.path().join("cargo-home"); + let cache = install.path().join("dedicated-cache"); + fs::create_dir_all(cargo.join("bin")).unwrap(); + fs::create_dir_all(&cache).unwrap(); + fs::write(cargo.join("credentials.toml"), "token = \"secret\"\n").unwrap(); + std::os::unix::fs::symlink(&cargo, cargo.join("registry")).unwrap(); + std::os::unix::fs::symlink(cargo.join("credentials.toml"), cargo.join("config.toml")).unwrap(); + std::os::unix::fs::symlink(&cache, cargo.join("git")).unwrap(); + + // The parent also contains Cargo credentials, so a broad explicit + // RUSTUP_HOME grant must be refused while the narrow cache remains usable. + let _env = EnvVarGuard::locked() + .with("RUSTUP_HOME", install.path()) + .with("CARGO_HOME", &cargo); + let grants = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); + let cargo = canon(&cargo); + let cache = canon(&cache); + let credential = canon(&cargo.join("credentials.toml")); + + assert!(!has(&grants.read_write, &cargo)); + assert!(!has(&grants.read_only, install.path())); + assert!(!all(&grants) + .iter() + .any(|path| path == &credential || path.starts_with(&credential))); + assert!(has(&grants.read_only, &cargo.join("bin"))); + assert!(has(&grants.read_write, &cache)); + + drop(_env); + let mode_cargo = install.path().join("mode-cargo-home"); + fs::create_dir_all(mode_cargo.join("bin")).unwrap(); + std::os::unix::fs::symlink(mode_cargo.join("bin"), mode_cargo.join("registry")).unwrap(); + let _env = EnvVarGuard::locked() + .without("RUSTUP_HOME") + .with("CARGO_HOME", &mode_cargo); + let grants = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); + let bin = canon(&mode_cargo.join("bin")); + assert!(has(&grants.read_only, &bin)); + assert!(!has(&grants.read_write, &bin)); +} + +#[test] +fn custom_rust_homes_inside_credential_stores_are_refused() { + let home = fake_home(); + let rustup = home.path().join(".ssh").join("rustup"); + let cargo = home.path().join(".aws").join("cargo"); + fs::create_dir_all(&rustup).unwrap(); + for dir in ["bin", "registry", "git"] { + fs::create_dir_all(cargo.join(dir)).unwrap(); + } + for file in ["config.toml", "config", "env"] { + fs::write(cargo.join(file), "").unwrap(); + } + + let _env = EnvVarGuard::locked() + .with("RUSTUP_HOME", &rustup) + .with("CARGO_HOME", &cargo); + let grants = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); + + assert!(!all(&grants).iter().any(|path| path.starts_with(&rustup))); + assert!(!all(&grants).iter().any(|path| path.starts_with(&cargo))); + assert_eq!(reaches_credentials(&all(&grants), home.path()), None); +} + +#[test] +fn custom_rust_homes_skip_relative_and_missing_paths() { + let home = fake_home(); + let cwd = std::env::current_dir().unwrap(); + let relative_rustup = tempfile::Builder::new() + .prefix("relative-rustup-home-") + .tempdir_in(&cwd) + .unwrap(); + let relative_cargo = tempfile::Builder::new() + .prefix("relative-cargo-home-") + .tempdir_in(&cwd) + .unwrap(); + for dir in ["bin", "registry", "git"] { + fs::create_dir_all(relative_cargo.path().join(dir)).unwrap(); + } + let relative_rustup_name = relative_rustup.path().file_name().unwrap(); + let relative_cargo_name = relative_cargo.path().file_name().unwrap(); + assert!(Path::new(relative_rustup_name).is_relative()); + assert!(Path::new(relative_cargo_name).is_relative()); + + { + let _env = EnvVarGuard::locked() + .with("RUSTUP_HOME", relative_rustup_name) + .with("CARGO_HOME", relative_cargo_name); + let grants = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); + let rustup = canon(relative_rustup.path()); + let cargo = canon(relative_cargo.path()); + assert!(!all(&grants).iter().any(|path| path.starts_with(&rustup))); + assert!(!all(&grants).iter().any(|path| path.starts_with(&cargo))); + } + + let missing_root = tempfile::tempdir().unwrap(); + let missing_rustup = missing_root.path().join("missing-rustup"); + let missing_cargo = missing_root.path().join("missing-cargo"); + { + let _env = EnvVarGuard::locked() + .with("RUSTUP_HOME", &missing_rustup) + .with("CARGO_HOME", &missing_cargo); + let grants = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); + assert!(!all(&grants) + .iter() + .any(|path| path.starts_with(&missing_rustup))); + assert!(!all(&grants) + .iter() + .any(|path| path.starts_with(&missing_cargo))); + } +} + +#[test] +fn configured_rust_homes_deduplicate_existing_home_grants() { + let home = fake_home(); + let cargo = home.path().join(".cargo"); + for dir in ["bin", "registry", "git"] { + fs::create_dir_all(cargo.join(dir)).unwrap(); + } + for file in ["config.toml", "config", "env"] { + fs::write(cargo.join(file), "").unwrap(); + } + let rustup = home.path().join(".rustup"); + + let _env = EnvVarGuard::locked() + .with("RUSTUP_HOME", &rustup) + .with("CARGO_HOME", &cargo); + let grants = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); + let rustup = canon(&rustup); + let cargo = canon(&cargo); + + assert_eq!( + grants + .read_only + .iter() + .filter(|path| *path == &rustup) + .count(), + 1 + ); + assert_eq!( + grants + .read_only + .iter() + .filter(|path| *path == &cargo.join("bin")) + .count(), + 1 + ); + for file in ["config.toml", "config", "env"] { + assert_eq!( + grants + .read_only + .iter() + .filter(|path| *path == &cargo.join(file)) + .count(), + 1, + "{file} should appear once" + ); + } + assert_eq!( + grants + .read_write + .iter() + .filter(|path| *path == &cargo.join("registry")) + .count(), + 1 + ); + assert_eq!( + grants + .read_write + .iter() + .filter(|path| *path == &cargo.join("git")) + .count(), + 1 + ); +} + #[test] fn absent_toolchain_homes_are_not_granted() { + let _env = isolated_toolchain_env(); let home = tempfile::tempdir().unwrap(); fs::create_dir_all(home.path().join(".nvm")).unwrap(); let g = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); @@ -132,6 +363,16 @@ fn absent_toolchain_homes_are_not_granted() { fn toolchain_homes_can_be_switched_off() { let home = fake_home(); fs::write(home.path().join(".gitconfig"), "[user]\n name = x\n").unwrap(); + let install = tempfile::tempdir().unwrap(); + let rustup = install.path().join("rustup-home"); + let cargo = install.path().join("cargo-home"); + fs::create_dir_all(&rustup).unwrap(); + for dir in ["bin", "registry", "git"] { + fs::create_dir_all(cargo.join(dir)).unwrap(); + } + let _env = EnvVarGuard::locked() + .with("RUSTUP_HOME", &rustup) + .with("CARGO_HOME", &cargo); let cfg = LocalJailConfig { toolchain_homes: false, ..LocalJailConfig::default() @@ -142,6 +383,7 @@ fn toolchain_homes_can_be_switched_off() { #[test] fn cargo_home_with_registry_credentials_is_granted_piecewise() { + let _env = isolated_toolchain_env(); let home = fake_home(); let cargo = home.path().join(".cargo"); for d in ["bin", "registry", "git"] { @@ -168,6 +410,7 @@ fn cargo_home_with_registry_credentials_is_granted_piecewise() { #[test] fn a_writable_grant_subsumes_the_same_path_read_only() { + let _env = isolated_toolchain_env(); let home = fake_home(); let cfg = LocalJailConfig { extra_read_only: vec!["~/.npm".into()], @@ -182,6 +425,7 @@ fn a_writable_grant_subsumes_the_same_path_read_only() { #[test] fn proc_is_off_by_default_and_only_the_toggle_grants_it() { + let _env = isolated_toolchain_env(); let home = fake_home(); let g = resolve_local_jail_grants(Some(home.path()), &LocalJailConfig::default()); assert!(!all(&g).iter().any(|p| p.starts_with("/proc"))); @@ -205,6 +449,7 @@ fn proc_is_off_by_default_and_only_the_toggle_grants_it() { #[test] fn extras_expand_tilde_and_skip_missing_paths() { + let _env = isolated_toolchain_env(); let home = fake_home(); fs::create_dir_all(home.path().join("dotfiles")).unwrap(); fs::create_dir_all(home.path().join("data")).unwrap(); @@ -221,6 +466,7 @@ fn extras_expand_tilde_and_skip_missing_paths() { #[test] fn gitconfig_symlink_and_include_targets_are_canonicalized() { + let _env = isolated_toolchain_env(); let home = fake_home(); let dots = home.path().join("dotfiles"); fs::create_dir_all(&dots).unwrap(); @@ -247,6 +493,7 @@ fn gitconfig_symlink_and_include_targets_are_canonicalized() { #[test] fn gitconfig_include_cycles_terminate() { + let _env = isolated_toolchain_env(); let home = fake_home(); fs::write(home.path().join(".gitconfig"), "[include]\n path = ~/b\n").unwrap(); fs::write(home.path().join("b"), "[include]\n path = ~/.gitconfig\n").unwrap(); @@ -256,6 +503,7 @@ fn gitconfig_include_cycles_terminate() { #[test] fn no_home_yields_only_system_toolchain_dirs() { + let _env = isolated_toolchain_env(); let g = resolve_local_jail_grants(None, &LocalJailConfig::default()); assert!(g.read_write.is_empty()); assert!(g diff --git a/crates/openhuman-core/src/sandbox/ops_tests.rs b/crates/openhuman-core/src/sandbox/ops_tests.rs index fd98c5bdf66..2e952cd1f76 100644 --- a/crates/openhuman-core/src/sandbox/ops_tests.rs +++ b/crates/openhuman-core/src/sandbox/ops_tests.rs @@ -507,6 +507,7 @@ fn host_has(program: &str) -> bool { #[cfg(target_os = "linux")] #[tokio::test] async fn landlock_jail_runs_cargo_and_mktemp_but_blocks_writes_outside() { + let _env = crate::config::test_env::EnvVarGuard::locked_async().await; if !landlock_in_force() { return; } @@ -524,26 +525,6 @@ async fn landlock_jail_runs_cargo_and_mktemp_but_blocks_writes_outside() { let r = run_local(&policy, &format!("{quoted_cargo} --version")).await; assert!(r.success(), "cargo failed under the jail: {}", r.stderr); assert!(r.stdout.starts_with("cargo "), "stdout: {}", r.stdout); - - // The fixture uses the host's real toolchain, so compare the sandbox - // values with the environment that selected that toolchain. This avoids - // process-global env mutation and exercises the actual spawn path. - let expected_homes = ["RUSTUP_HOME", "CARGO_HOME"].map(|name| { - std::env::var_os(name) - .map(|value| value.to_string_lossy().into_owned()) - .unwrap_or_default() - }); - let r = run_local( - &policy, - "printf '%s\\n' \"${RUSTUP_HOME-}\" \"${CARGO_HOME-}\"", - ) - .await; - assert!(r.success(), "toolchain home probe failed: {}", r.stderr); - assert_eq!( - r.stdout, - format!("{}\n{}\n", expected_homes[0], expected_homes[1]), - "sandboxed commands must inherit explicitly configured Rust toolchain homes" - ); } else { eprintln!("SKIP cargo: build toolchain executable is absent on this host"); } @@ -668,3 +649,7 @@ fn sandbox_off_value_keeps_sandbox_on_otherwise() { assert!(!sandbox_off_value(v), "{v:?} must leave the sandbox on"); } } + +#[cfg(unix)] +#[path = "ops_toolchain_tests.rs"] +mod toolchain_homes; diff --git a/crates/openhuman-core/src/sandbox/ops_toolchain_tests.rs b/crates/openhuman-core/src/sandbox/ops_toolchain_tests.rs new file mode 100644 index 00000000000..97dd6f2dd03 --- /dev/null +++ b/crates/openhuman-core/src/sandbox/ops_toolchain_tests.rs @@ -0,0 +1,104 @@ +use super::*; + +#[cfg(unix)] +#[tokio::test] +async fn local_jail_forwards_nonempty_toolchain_homes_without_cargo() { + let action = tempfile::tempdir().unwrap(); + let state = tempfile::tempdir().unwrap(); + let rustup_home = tempfile::tempdir().unwrap(); + let cargo_home = tempfile::tempdir().unwrap(); + let rustup_value = rustup_home.path().to_string_lossy().into_owned(); + let cargo_value = cargo_home.path().to_string_lossy().into_owned(); + let _env = crate::config::test_env::EnvVarGuard::locked_async() + .await + .with("RUSTUP_HOME", &rustup_value) + .with("CARGO_HOME", &cargo_value); + let policy = local_policy(action.path(), state.path()); + + // These paths exercise forwarding alone. The child only prints them; no + // Cargo command consumes them as though they held a real toolchain. + let result = run_local( + &policy, + "printf '%s\\n' \"${RUSTUP_HOME-}\" \"${CARGO_HOME-}\"", + ) + .await; + assert!( + result.success(), + "toolchain-home probe failed: {}", + result.stderr + ); + assert_eq!( + result.stdout, + format!("{rustup_value}\n{cargo_value}\n"), + "local sandbox must forward explicitly configured nonempty toolchain homes" + ); +} + +#[cfg(target_os = "linux")] +#[tokio::test] +async fn landlock_custom_toolchain_homes_keep_selective_access() { + if !landlock_in_force() { + return; + } + let action = tempfile::tempdir().unwrap(); + let state = tempfile::tempdir().unwrap(); + let rustup_home = tempfile::tempdir().unwrap(); + let cargo_home = tempfile::tempdir().unwrap(); + for child in ["bin", "registry", "git"] { + std::fs::create_dir(cargo_home.path().join(child)).unwrap(); + } + std::fs::write(rustup_home.path().join("toolchain"), "rust-fixture").unwrap(); + std::fs::write(cargo_home.path().join("bin/tool"), "cargo-fixture").unwrap(); + std::fs::write(cargo_home.path().join("credentials.toml"), "fixture-secret").unwrap(); + let _env = crate::config::test_env::EnvVarGuard::locked_async() + .await + .with("RUSTUP_HOME", rustup_home.path().to_str().unwrap()) + .with("CARGO_HOME", cargo_home.path().to_str().unwrap()); + let policy = local_policy(action.path(), state.path()); + let reads = run_local( + &policy, + "cat \"$RUSTUP_HOME/toolchain\"; cat \"$CARGO_HOME/bin/tool\"", + ) + .await; + assert!( + reads.success(), + "custom toolchain reads failed: {}", + reads.stderr + ); + assert_eq!(reads.stdout, "rust-fixturecargo-fixture"); + let caches = run_local( + &policy, + "printf registry > \"$CARGO_HOME/registry/probe\" && printf git > \"$CARGO_HOME/git/probe\"", + ) + .await; + assert!( + caches.success(), + "Cargo cache writes failed: {}", + caches.stderr + ); + for command in [ + "printf denied > \"$RUSTUP_HOME/toolchain\"", + "printf denied > \"$CARGO_HOME/bin/tool\"", + "cat \"$CARGO_HOME/credentials.toml\"", + "printf denied > \"$CARGO_HOME/credentials.toml\"", + ] { + let result = run_local(&policy, command).await; + assert!(!result.success(), "selective access allowed {command}"); + } + assert_eq!( + std::fs::read_to_string(rustup_home.path().join("toolchain")).unwrap(), + "rust-fixture" + ); + assert_eq!( + std::fs::read_to_string(cargo_home.path().join("bin/tool")).unwrap(), + "cargo-fixture" + ); + assert_eq!( + std::fs::read_to_string(cargo_home.path().join("registry/probe")).unwrap(), + "registry" + ); + assert_eq!( + std::fs::read_to_string(cargo_home.path().join("git/probe")).unwrap(), + "git" + ); +} diff --git a/docs/RELEASE-MANUAL-SMOKE.md b/docs/RELEASE-MANUAL-SMOKE.md index 3723c4e3f4f..5ab08c2e99c 100644 --- a/docs/RELEASE-MANUAL-SMOKE.md +++ b/docs/RELEASE-MANUAL-SMOKE.md @@ -89,6 +89,8 @@ Applies to every release, all platforms. - [ ] **OS-native notification toasts fire** — Trigger a notification from inside the app (e.g. memory captured, agent finished). Expected: a libnotify-style toast appears outside the app window. (CI Linux sees only Xvfb; this surface verifies on a real desktop.) - [ ] **Headless supervisor update stages without self-exit** — On a Linux service deployment with `[update] restart_strategy = "supervisor"` and `rpc_mutations_enabled = false`, stage a new core binary through the documented operator flow. Expected: the running process stays up until the supervisor restart, the staged binary is present on disk, and `systemctl restart openhuman` (or equivalent) picks up the new version. +- [ ] **Native sandbox preserves installed Rust toolchain homes** — In an isolated Linux profile with Rust installed through explicit absolute `RUSTUP_HOME` and `CARGO_HOME` outside the profile HOME and system toolchain roots, run `cargo --version` through the sandboxed shell. Expected: the installed Cargo version is returned, temporary files use the sandbox scratch directory, workspace writes succeed, and a write to a separately prepared outside directory is denied. Verify the configured homes match the values observed inside the native sandbox. Confirm Cargo registry/git cache writes succeed and toolchain files and Cargo credentials retain their access restrictions. + ### Cross-platform - [ ] **Agent files land in a visible folder** — Ask the agent for a short document or deck. Expected: the file appears in `~/OpenHuman/projects/Files` under its title (not in `~/.openhuman`); **Show in folder** in the chat Files panel opens the file manager at it; Settings → Agent OS access → **Files folder** shows that path, **Show in folder** opens it, and choosing another folder sends the next file there while the earlier file still opens. On an upgraded install, files from before the upgrade have moved into the folder. @@ -130,5 +132,3 @@ Notes: ``` Paste the filled block as a commit comment on the `v-staging` tagged commit before promoting to production. - -- [ ] **Native sandbox preserves installed Rust toolchain homes** — In an isolated Linux profile with Rust installed through explicit `RUSTUP_HOME` and `CARGO_HOME`, run `cargo --version` through the sandboxed shell. Expected: the installed Cargo version is returned, temporary files use the sandbox scratch directory, workspace writes succeed, and a write to a separately prepared outside directory is denied. Verify the configured homes match the values observed inside the native sandbox. diff --git a/docs/TEST-COVERAGE-MATRIX.md b/docs/TEST-COVERAGE-MATRIX.md index 69d84f38703..bb2b791bd76 100644 --- a/docs/TEST-COVERAGE-MATRIX.md +++ b/docs/TEST-COVERAGE-MATRIX.md @@ -272,7 +272,7 @@ End-to-end coverage of the agent harness via the web-chat RPC surface against an | ID | Feature | Layer | Test path(s) | Status | Notes | | ----- | ---------------------------- | ----- | --------------------------------------------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------ | -| 6.2.1 | Shell Command Execution | RU+RI+WD | `crates/openhuman-core/src/tools/impl/system/shell.rs`, `crates/openhuman-core/src/sandbox/ops_tests.rs`, `tests/agent_harness_e2e.rs` (`sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes`), `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO asserts agent runtime + the shell-bearing agent registry contract (the orchestrator since `tools_agent` was removed); full LLM path tracked #68; Rust toolchain startup is covered through the real Landlock Cargo fixture and an agent-shell RPC/model-loop E2E, including observed host homes, Cargo startup, scratch files, workspace writes and outside-workspace write denial | +| 6.2.1 | Shell Command Execution | RU+RI+WD | `crates/openhuman-core/src/tools/impl/system/shell.rs`, `crates/openhuman-core/src/sandbox/ops_tests.rs`, `tests/agent_harness_e2e.rs` (`sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes`), `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO asserts agent runtime + the shell-bearing agent registry contract (the orchestrator since `tools_agent` was removed); full LLM path tracked #68; Rust toolchain startup is covered through the real Landlock Cargo fixture and an agent-shell RPC/model-loop E2E, including observed custom homes, Cargo startup, scratch files, workspace writes and outside-workspace write denial; `crates/openhuman-core/src/sandbox/ops_toolchain_tests.rs` (`landlock_custom_toolchain_homes_keep_selective_access`) verifies read-only toolchain files, registry/git cache writes and Cargo credential denial | | 6.2.2 | Command Restriction Handling | RU+WD | `crates/openhuman-core/src/security/policy/policy_tests.rs` (+ `policy_injection_tests.rs`), `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO locks denial envelope shape `{ ok:false, error }` consumed by the React UI | | 6.2.3 | Git Read Operations | RU+WD | `vendor/tinyagents/vendor/tinytools/crates/tinytools-std/src/filesystem/git_operations/test.rs`, `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO seeds a fixture repo in OPENHUMAN_WORKSPACE and asserts read ops succeed | | 6.2.4 | Git Write Operations | RU+WD | `vendor/tinyagents/vendor/tinytools/crates/tinytools-std/src/filesystem/git_operations/test.rs`, `app/test/e2e/specs/tool-shell-git-flow.spec.ts` | ✅ | Was 🟡 — WDIO commits into the same fixture and asserts log advances | diff --git a/tests/agent_harness_e2e.rs b/tests/agent_harness_e2e.rs index 6154ef01c4c..eb2ba2a35c2 100644 --- a/tests/agent_harness_e2e.rs +++ b/tests/agent_harness_e2e.rs @@ -36,7 +36,9 @@ use serde_json::{json, Value}; use tempfile::tempdir; use openhuman_core::agent::harness::AgentDefinitionRegistry; +use openhuman_core::config::RuntimeConfig; use openhuman_core::core::auth::{init_rpc_token, CORE_TOKEN_ENV_VAR}; +use openhuman_core::sandbox::grants::resolve_local_jail_grants; use openhuman_rpc::server::build_core_http_router; const TEST_RPC_TOKEN: &str = "json-rpc-e2e-local-token"; @@ -5177,44 +5179,221 @@ async fn sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes_inner let _lock = env_lock(); // Give this real orchestrator turn a private action directory and make the - // host sandbox switch explicit. HOME is separately replaced by the stack - // fixture, so these guards pin the actual host Rust toolchain locations. + // host sandbox switch explicit. Capture host toolchain inputs before the + // stack fixture replaces HOME, then stage their real executables externally. let action_dir = tempdir().expect("action directory"); let outside_dir = tempdir().expect("outside directory"); - let host_home = std::env::var_os("HOME") - .map(std::path::PathBuf::from) - .expect("host HOME must be set before the stack fixture replaces it"); - let rustup_home = std::env::var_os("RUSTUP_HOME") + let github_actions = std::env::var("GITHUB_ACTIONS").as_deref() == Ok("true"); + let host_home = std::env::var_os("HOME").map(std::path::PathBuf::from); + let host_path = std::env::var_os("PATH"); + let find_host_tool = |name: &str| { + host_path.as_ref().and_then(|path| { + std::env::split_paths(path) + .map(|dir| dir.join(name)) + .find(|candidate| candidate.is_file()) + }) + }; + let host_cargo_path = find_host_tool("cargo"); + let host_rustup_path = find_host_tool("rustup"); + let host_rustup_home = std::env::var_os("RUSTUP_HOME") .map(std::path::PathBuf::from) - .unwrap_or_else(|| host_home.join(".rustup")); - let cargo_home = std::env::var_os("CARGO_HOME") + .or_else(|| host_home.as_ref().map(|home| home.join(".rustup"))); + let host_cargo_home = std::env::var_os("CARGO_HOME") .map(std::path::PathBuf::from) - .unwrap_or_else(|| host_home.join(".cargo")); + .or_else(|| host_home.as_ref().map(|home| home.join(".cargo"))); + let mut missing_prerequisites = Vec::new(); + if host_home.is_none() { + missing_prerequisites.push("HOME is unset".to_owned()); + } + if host_cargo_path.is_none() { + missing_prerequisites.push("no cargo executable is present on PATH".to_owned()); + } + if host_rustup_path.is_none() { + missing_prerequisites.push("no rustup executable is present on PATH".to_owned()); + } + if !host_rustup_home + .as_ref() + .is_some_and(|home| home.join("settings.toml").is_file()) + { + missing_prerequisites + .push("Rustup settings.toml is absent from the selected RUSTUP_HOME".to_owned()); + } + if !missing_prerequisites.is_empty() { + let reason = missing_prerequisites.join("; "); + if github_actions { + panic!("pinned GitHub Actions agent-shell E2E requires initialized Rustup/Cargo prerequisites: {reason}"); + } + eprintln!("SKIP: agent-shell custom toolchain fixture prerequisites unavailable: {reason}"); + return; + } + let host_home = host_home.expect("prerequisite check established host HOME"); + let host_rustup_home = host_rustup_home.expect("prerequisite check established Rustup home"); + let host_cargo_home = host_cargo_home.expect("HOME or CARGO_HOME is required"); + // Rustup proxies dispatch by executable name: retain the PATH-selected + // Cargo name for invocation; staging resolves file targets separately. + let host_cargo_path = host_cargo_path.expect("prerequisite check found Cargo on PATH"); + let host_rustup_path = host_rustup_path.expect("prerequisite check found Rustup on PATH"); assert!( - rustup_home.is_absolute(), + host_rustup_home.is_absolute(), "host RUSTUP_HOME must be absolute" ); - assert!(cargo_home.is_absolute(), "host CARGO_HOME must be absolute"); - assert!(rustup_home.is_dir(), "host RUSTUP_HOME must exist"); - assert!(cargo_home.is_dir(), "host CARGO_HOME must exist"); - let cargo_path = std::env::split_paths( - &std::env::var_os("PATH").expect("host PATH must be set before the stack fixture"), + assert!( + host_cargo_home.is_absolute(), + "host CARGO_HOME must be absolute" + ); + let host_settings = std::fs::read_to_string(host_rustup_home.join("settings.toml")) + .expect("host Rustup settings must exist"); + let active_toolchain = host_settings + .lines() + .find_map(|line| { + let (key, value) = line.split_once('=')?; + (key.trim() == "default_toolchain").then(|| value.trim().trim_matches('"').to_owned()) + }) + .filter(|name| !name.is_empty()) + .expect("host Rustup settings must select a default toolchain"); + let host_toolchain_cargo = host_rustup_home + .join("toolchains") + .join(&active_toolchain) + .join("bin/cargo"); + let host_toolchain_rustc = host_rustup_home + .join("toolchains") + .join(&active_toolchain) + .join("bin/rustc"); + assert!( + host_toolchain_cargo.is_file(), + "the selected host toolchain must contain its real Cargo executable" + ); + assert!( + host_toolchain_rustc.is_file(), + "the selected host toolchain must contain its real rustc executable" + ); + let host_active_toolchain = std::process::Command::new(&host_rustup_path) + .args(["show", "active-toolchain"]) + .current_dir(action_dir.path()) + .env("RUSTUP_HOME", &host_rustup_home) + .env_remove("RUSTUP_TOOLCHAIN") + .output() + .expect("run host Rustup to verify the captured selection"); + assert!( + host_active_toolchain.status.success(), + "host Rustup could not select its default toolchain: {}", + String::from_utf8_lossy(&host_active_toolchain.stderr) + ); + let host_active_toolchain = String::from_utf8_lossy(&host_active_toolchain.stdout) + .trim() + .to_owned(); + assert!( + host_active_toolchain.starts_with(&active_toolchain), + "host Rustup selection disagrees with settings.toml: {host_active_toolchain}" + ); + let host_cargo_version = std::process::Command::new(&host_cargo_path) + .arg("--version") + .current_dir(action_dir.path()) + .env("RUSTUP_HOME", &host_rustup_home) + .env("CARGO_HOME", &host_cargo_home) + .env_remove("RUSTUP_TOOLCHAIN") + .output() + .expect("run host Cargo to capture its version"); + assert!( + host_cargo_version.status.success(), + "host Cargo version command failed: {}", + String::from_utf8_lossy(&host_cargo_version.stderr) + ); + let host_cargo_version = String::from_utf8_lossy(&host_cargo_version.stdout) + .trim() + .to_owned(); + assert!( + host_cargo_version.starts_with("cargo "), + "host Cargo returned an unexpected version string: {host_cargo_version}" + ); + + // Stage only real selection metadata and executable files outside HOME + // and system roots; the real compiler itself is never built or downloaded. + let staged_root = tempdir().expect("external toolchain fixture"); + let staged_root_path = staged_root + .path() + .canonicalize() + .expect("external toolchain fixture path"); + let canonical_host_home = host_home.canonicalize().expect("host HOME exists"); + assert!( + !staged_root_path.starts_with(&canonical_host_home) + && ["/usr/local", "/opt"] + .iter() + .all(|root| !staged_root_path.starts_with(root)), + "custom toolchain fixture must be outside host HOME and existing system grants" + ); + let rustup_home = staged_root.path().join("rustup-home"); + let cargo_home = staged_root.path().join("cargo-home"); + let cargo_bin = cargo_home.join("bin"); + std::fs::create_dir_all(&rustup_home).expect("create external Rustup home"); + std::fs::create_dir_all(&cargo_bin).expect("create external Cargo bin"); + std::fs::copy( + host_rustup_home.join("settings.toml"), + rustup_home.join("settings.toml"), ) - .map(|dir| dir.join("cargo")) - .find(|candidate| candidate.is_file()) - .expect("cargo must be present on the pinned Linux test PATH"); - let cargo_bin = cargo_path - .parent() - .expect("cargo executable must have a parent directory") + .expect("copy real Rustup selection metadata"); + // Rustup initializes these control directories even for read-only queries. + // Prepare the existing-install layout before the jail makes the home read-only. + for directory in ["downloads", "tmp", "update-hashes"] { + std::fs::create_dir_all(rustup_home.join(directory)) + .expect("create Rustup control-directory fixture"); + } + if let Ok(entries) = std::fs::read_dir(host_rustup_home.join("update-hashes")) { + for entry in entries { + let entry = entry.expect("read installed Rustup update-hash metadata"); + if entry + .file_type() + .expect("Rustup metadata file type") + .is_file() + { + std::fs::copy( + entry.path(), + rustup_home.join("update-hashes").join(entry.file_name()), + ) + .expect("copy installed Rustup update-hash metadata"); + } + } + } + let stage_executable = |source: &Path, destination: std::path::PathBuf| { + let source = source + .canonicalize() + .expect("host toolchain executable must resolve"); + if std::fs::hard_link(&source, &destination).is_err() { + std::fs::copy(&source, &destination).expect("copy real toolchain executable"); + } + }; + let staged_toolchain_bin = rustup_home + .join("toolchains") + .join(&active_toolchain) + .join("bin"); + std::fs::create_dir_all(&staged_toolchain_bin) + .expect("create staged selected toolchain bin directory"); + stage_executable(&host_toolchain_cargo, staged_toolchain_bin.join("cargo")); + stage_executable(&host_toolchain_rustc, staged_toolchain_bin.join("rustc")); + stage_executable(&host_cargo_path, cargo_bin.join("cargo")); + stage_executable(&host_rustup_path, cargo_bin.join("rustup")); + std::fs::write( + cargo_home.join("config.toml"), + "[term]\ncolor = \"never\"\n", + ) + .expect("write harmless Cargo config fixture"); + std::fs::create_dir_all(cargo_home.join("registry")).expect("create Cargo registry cache"); + std::fs::create_dir_all(cargo_home.join("git")).expect("create Cargo git cache"); + let cargo_registry_marker = cargo_home.join("registry/agent-shell-write.txt"); + let cargo_git_marker = cargo_home.join("git/agent-shell-write.txt"); + + let cargo_bin = cargo_bin .canonicalize() - .expect("cargo executable directory must exist"); + .expect("custom Cargo bin directory must exist"); let marker_name = format!("agent-harness-shell-{}.txt", std::process::id()); let marker_path = action_dir.path().join(&marker_name); let outside_path = outside_dir.path().join("must-stay-unwritable.txt"); + let expected_toolchain = shell_single_quote(&host_active_toolchain); + let expected_cargo_version = shell_single_quote(&host_cargo_version); let command = format!( - "set -eu; export PATH={}:$PATH; printf 'RUSTUP_HOME=%s\\nCARGO_HOME=%s\\n' \"$RUSTUP_HOME\" \"$CARGO_HOME\"; printf 'CARGO_EXE=%s\\n' \"$(command -v cargo)\"; cargo --version; printf 'workspace-write-ok\\n' > '{marker_name}'; test \"$(cat '{marker_name}')\" = workspace-write-ok; printf 'WORKSPACE_WRITE=ok\\n'; if printf 'outside-write\\n' > {}; then printf 'OUTSIDE_WRITE=allowed\\n'; else printf 'OUTSIDE_WRITE=blocked\\n'; fi; scratch=$(mktemp); test -f \"$scratch\"; printf 'MKTEMP=ok\\n'; rm -f \"$scratch\"", - shell_single_quote(&cargo_bin.to_string_lossy()), - shell_single_quote(&outside_path.to_string_lossy()) + "set -eu; export PATH={cargo_bin}:$PATH; printf 'RUSTUP_HOME=%s\\nCARGO_HOME=%s\\n' \"$RUSTUP_HOME\" \"$CARGO_HOME\"; printf 'CARGO_EXE=%s\\n' \"$(command -v cargo)\"; active_toolchain=$(rustup show active-toolchain); test -n \"$active_toolchain\"; test \"$active_toolchain\" = {expected_toolchain}; printf 'RUSTUP_ACTIVE_TOOLCHAIN=%s\\n' \"$active_toolchain\"; cargo_version=$(cargo --version); test -n \"$cargo_version\"; test \"$cargo_version\" = {expected_cargo_version}; printf 'CARGO_VERSION=%s\\n' \"$cargo_version\"; printf 'cache-write\\n' > \"$CARGO_HOME/registry/agent-shell-write.txt\"; printf 'cache-write\\n' > \"$CARGO_HOME/git/agent-shell-write.txt\"; printf 'CARGO_CACHE_WRITE=ok\\n'; printf 'workspace-write-ok\\n' > '{marker_name}'; test \"$(cat '{marker_name}')\" = workspace-write-ok; printf 'WORKSPACE_WRITE=ok\\n'; if printf 'outside-write\\n' > {outside_path}; then printf 'OUTSIDE_WRITE=allowed\\n'; else printf 'OUTSIDE_WRITE=blocked\\n'; fi; scratch=$(mktemp); test -f \"$scratch\"; printf 'MKTEMP=ok\\n'; rm -f \"$scratch\"", + cargo_bin = shell_single_quote(&cargo_bin.to_string_lossy()), + outside_path = shell_single_quote(&outside_path.to_string_lossy()), ); reset_script(vec![ tool_call_completion("shell", json!({ "command": command, "category": "write" })), @@ -5226,15 +5405,49 @@ async fn sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes_inner let _rustup_home_guard = EnvVarGuard::set_to_path("RUSTUP_HOME", &rustup_home); let _cargo_home_guard = EnvVarGuard::set_to_path("CARGO_HOME", &cargo_home); let stack = boot_stack().await; - // The shell's default local jail grants HOME/.rustup and HOME/.cargo/bin. - // Link only those temporary-home entries to the captured host locations; - // the default grant resolver canonicalizes them before spawning the jail. - std::os::unix::fs::symlink(&rustup_home, stack._tmp.path().join(".rustup")) - .expect("link host Rust home into temporary HOME"); - let fixture_cargo_home = stack._tmp.path().join(".cargo"); - std::fs::create_dir_all(&fixture_cargo_home).expect("create temporary Cargo home"); - std::os::unix::fs::symlink(&cargo_bin, fixture_cargo_home.join("bin")) - .expect("link host Cargo executable directory into temporary HOME"); + let fixture_home = stack + ._tmp + .path() + .canonicalize() + .expect("private fixture HOME exists"); + assert!( + !staged_root_path.starts_with(&fixture_home), + "custom toolchain fixture must be outside private HOME" + ); + let grants = resolve_local_jail_grants( + Some(stack._tmp.path()), + &RuntimeConfig::default().local_jail, + ); + let is_covered_by = |path: &Path, roots: &[std::path::PathBuf]| { + let canonical = path.canonicalize().expect("custom toolchain path exists"); + roots.iter().any(|grant| canonical.starts_with(grant)) + }; + assert!( + is_covered_by(&rustup_home, &grants.read_only), + "explicit custom RUSTUP_HOME must be read-only admitted" + ); + assert!( + is_covered_by(&cargo_bin, &grants.read_only), + "explicit custom CARGO_HOME/bin must be read-only admitted" + ); + assert!( + is_covered_by(&cargo_home.join("config.toml"), &grants.read_only), + "explicit custom Cargo config must be read-only admitted" + ); + assert!( + is_covered_by(&cargo_home.join("registry"), &grants.read_write) + && is_covered_by(&cargo_home.join("git"), &grants.read_write), + "explicit custom Cargo caches must be read-write admitted" + ); + assert!( + !is_covered_by(&cargo_home, &grants.read_only) + && !is_covered_by(&cargo_home, &grants.read_write), + "Cargo home root must retain selective child grants" + ); + assert!( + !stack._tmp.path().join(".rustup").exists() && !stack._tmp.path().join(".cargo").exists(), + "custom homes must be admitted directly without private-HOME aliases" + ); let mut events = spawn_sse_collector(format!( "{}/events?client_id=harness-sandboxed-shell", stack.rpc_base @@ -5266,6 +5479,10 @@ async fn sandboxed_shell_forwards_host_toolchain_homes_and_confines_writes_inner "shell command should succeed: {shell_result}" ); assert!(marker_path.is_file(), "workspace write did not persist"); + assert!( + cargo_registry_marker.is_file() && cargo_git_marker.is_file(), + "the custom Cargo cache grants did not permit shell writes" + ); assert!( !outside_path.exists(), "the local jail allowed a write outside action_dir: {}", @@ -5341,7 +5558,9 @@ fn shell_result_contains_all_markers( result.contains(format!("RUSTUP_HOME={}", rustup_home.display()).as_str()) && result.contains(format!("CARGO_HOME={}", cargo_home.display()).as_str()) && result.contains(format!("CARGO_EXE={}", cargo_bin.join("cargo").display()).as_str()) + && result.contains("RUSTUP_ACTIVE_TOOLCHAIN=") && result.contains("cargo ") + && result.contains("CARGO_CACHE_WRITE=ok") && result.contains("WORKSPACE_WRITE=ok") && result.contains("OUTSIDE_WRITE=blocked") && result.contains("MKTEMP=ok")