feat(tools/system): route node_exec and npm_exec through sandbox backend like shell (#3309)

CLAUDE.md

@@ -130,6 +130,14 @@ PRs must meet **≥ 80% coverage on changed lines**. Enforced by [`.github/workf
 
 Overriding `OPENHUMAN_ACTION_DIR` does **not** weaken the internal denylist — `workspace_dir` is always blocked even if `action_dir` is relocated to overlap it. Adding new internal-state subdirectories under `workspace_dir` requires extending `WORKSPACE_INTERNAL_DIRS` (or `WORKSPACE_INTERNAL_FILES`) in the same change; otherwise an agent tool with a `trusted_root` grant could write into the new subtree.
 
+**Sandbox execution backends** — when an agent's `agent.toml` declares `sandbox_mode = "sandboxed"`, the three shell-family tools (`shell`, `node_exec`, `npm_exec`) route execution through `src/openhuman/sandbox/` instead of the plain `RuntimeAdapter`. The sandbox backend is picked by `sandbox::resolve_sandbox_policy` (`src/openhuman/sandbox/ops.rs`) based on the session origin + `RuntimeConfig`:
+
+- **Docker** — ephemeral `docker run --rm` containers with `--cap-drop ALL`, `--security-opt no-new-privileges`, `--network none` (remote sessions), read-only rootfs + tmpfs, the action sandbox mounted at the working directory, and labels for orphan cleanup. Preferred for remote / channel / cron / background sessions.
+- **Local OS jail (`cwd_jail`)** — Landlock on Linux, Seatbelt on macOS, AppContainer on Windows. Rooted at `Config.action_dir`, with output capture via temp files inside the jail (works around Seatbelt's stdio-forwarding limitation). Preferred for interactive desktop sessions.
+- **Noop** — on platforms where `cwd_jail` has no real backend (or when Docker is unavailable and the local backend is unsupported), the sandbox falls back to a documented passthrough that runs the command with `current_dir = action_dir` and `env_clear()` + the `SANDBOX_ENV_PASSTHROUGH` allow-list. The in-Rust path hardening (`is_path_string_allowed`, `is_workspace_internal_path`, structural `check_gated_command` guards, the unconditional `is_always_forbidden` system/credential block) **still applies** regardless of which sandbox backend is selected — defense-in-depth, not "noop = no defense."
+
+Default `sandbox_mode = "none"` agents keep the historical behavior (plain `runtime.build_shell_command` path), so the sandbox layer is strictly additive and opt-in per agent. `ShellTool::run_with_security`, `NodeExecTool::execute`, and `NpmExecTool::execute` all check `current_sandbox_mode()` (a `tokio` task-local set by the agent harness) and delegate to their `run_sandboxed()` helper when `SandboxMode::Sandboxed` is active. The security/rate-limit/audit checks run *before* the sandbox routing decision, so a denied command is still denied identically in both paths.
+
 **Command permission model (deterministic, fail-closed):** `classify_command` buckets a command into `CommandClass` (`Read` / `Write` / `Network` / `Install` / `Destructive`); an unrecognized command is **`Write`**, never `Read`. `gate_decision(class, tier)` → `Allow` / `Prompt` / `Block`: read-only allows only reads; ask-before-edit prompts every act (file *create* is free, *edit-existing* prompts); full runs read+write but **always-asks** Network/Install/Destructive. Acting tools (`shell`/`node_exec`/`npm_exec`/`file_write`/`edit_file`/`apply_patch`/`git_operations`/`curl`) return `external_effect_with_args() == true` for `Prompt` classes so the harness routes them through the `ApprovalGate` *before* `execute()`; read-only `Block` + structural guards (`check_gated_command`) are enforced in-tool. The LLM may pass a `category` (escalate-only: `max(rust_floor, declared)`). System/credential dirs are an **unconditional** cross-platform block (`is_always_forbidden`, trusted-root-proof). Enforcement is in Rust (`classify_command`/`gate_decision`/`check_gated_command`/`is_path_string_allowed`/`validate_path`), never the system prompt.
 
 > ⚠️ **The approval prompt is ON by default** (opt out with `OPENHUMAN_APPROVAL_GATE=0`/`false`, `jsonrpc.rs`). `ApprovalGate::init_global` installs unless disabled, so `try_global()` is `Some` and the prompt is wired end-to-end; with `OPENHUMAN_APPROVAL_GATE=0` the harness skips the intercept and `Prompt`-class calls **run unprompted**. The gate parks only for **interactive chat turns** (a `tokio` task-local chat context is set in `channels/providers/web.rs`; background triage/cron turns carry no context and are allowed through, not gated). It publishes `DomainEvent::ApprovalRequested`, which `ApprovalSurfaceSubscriber` bridges to the `approval_request` web-channel socket event; the frontend (`ChatApprovalRequestEvent` → `chatRuntime.pendingApprovalByThread` → `ApprovalRequestCard` above the composer) surfaces Approve/Deny, routing to the `openhuman.approval_decide` RPC. A typed `yes`/`no` chat reply is also honoured server-side (web.rs ingress router runs before the "newer request aborts the in-flight turn" path); any other text cancels the parked turn and is taken as a fresh message. Unanswered prompts still park to the 10-min TTL → Deny. Read-only blocking, path hardening, structural guards, and classification **are** live regardless of the flag. Full access ships as documented full-trust (not sandboxed).

src/openhuman/cwd_jail/noop.rs

@@ -24,3 +24,60 @@ impl JailBackend for NoopBackend {
         cmd.spawn()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Regression guard for #3235.
+    ///
+    /// On platforms where no OS-level jail backend is available
+    /// (Landlock / Seatbelt / AppContainer absent or unsupported),
+    /// `cwd_jail::detect::pick_backend()` returns `NoopBackend`. This
+    /// test pins the NoopBackend contract — it must always report
+    /// `is_available() == true` so it's a usable fallback, must
+    /// identify as `"noop"` so operators can see in logs which backend
+    /// is active, and `spawn` must be a passthrough that runs the
+    /// command without enforcement. The in-Rust path hardening in
+    /// `SecurityPolicy::is_path_string_allowed` /
+    /// `is_workspace_internal_path` still applies; the noop name
+    /// reflects "no OS-level isolation," not "no defense at all."
+    #[test]
+    fn noop_backend_identifies_as_noop_and_is_always_available() {
+        let backend = NoopBackend;
+        assert_eq!(backend.name(), "noop");
+        assert!(
+            backend.is_available(),
+            "NoopBackend must always be available so it can serve as the \
+             documented passthrough on unsupported platforms (see CLAUDE.md \
+             'Sandbox execution backends')."
+        );
+    }
+
+    #[test]
+    fn noop_backend_spawn_runs_command_passthrough() {
+        // On a POSIX host, `true` exits 0 immediately. The NoopBackend
+        // must spawn it successfully without injecting any jail flags,
+        // proving the passthrough contract.
+        let backend = NoopBackend;
+        let jail = Jail::new("/tmp", "noop-test-3235");
+        let cmd = if cfg!(windows) {
+            let mut c = Command::new("cmd");
+            c.args(["/C", "exit 0"]);
+            c
+        } else {
+            Command::new("true")
+        };
+        let child = backend
+            .spawn(&jail, cmd)
+            .expect("NoopBackend::spawn must passthrough-spawn the child");
+        let output = child
+            .wait_with_output()
+            .expect("child must run to completion");
+        assert!(
+            output.status.success(),
+            "passthrough child should succeed; got {:?}",
+            output.status
+        );
+    }
+}

src/openhuman/tools/impl/system/node_exec.rs

@@ -228,6 +228,21 @@ impl Tool for NodeExecTool {
             unreachable!("guarded above")
         };
 
+        // When the agent's sandbox mode is `Sandboxed`, route execution
+        // through the sandbox backend (Docker / OS-level `cwd_jail` /
+        // documented noop) instead of the native runtime path. Mirrors
+        // the wiring in `ShellTool::run_with_security` (PR #3261) so
+        // node_exec gets the same isolation guarantees as shell. The
+        // security/rate-limit checks above still apply.
+        if matches!(
+            crate::openhuman::agent::harness::current_sandbox_mode(),
+            Some(crate::openhuman::agent::harness::definition::SandboxMode::Sandboxed)
+        ) {
+            return Ok(self
+                .run_sandboxed(&command, &resolved.bin_dir, timeout_secs)
+                .await);
+        }
+
         let mut cmd = match self
             .runtime
             .build_shell_command(&command, &self.security.action_dir)
@@ -298,6 +313,108 @@ impl Tool for NodeExecTool {
     }
 }
 
+impl NodeExecTool {
+    /// Execute a node command through the sandbox backend. Called from
+    /// `execute()` when the agent's `SandboxMode` is `Sandboxed`.
+    ///
+    /// Mirrors `ShellTool::run_sandboxed`. The sandbox policy is resolved
+    /// from the current `RuntimeConfig` and rooted at
+    /// `security.action_dir`; on platforms without a real `cwd_jail`
+    /// backend the local backend falls back to a documented noop with
+    /// the in-Rust path-hardening guards from `SecurityPolicy` still
+    /// applying (see CLAUDE.md "Action sandbox vs internal workspace").
+    async fn run_sandboxed(
+        &self,
+        command: &str,
+        bin_dir: &std::path::Path,
+        timeout_secs: u64,
+    ) -> ToolResult {
+        use crate::openhuman::sandbox;
+
+        // Load the live `RuntimeConfig` so `resolve_sandbox_policy` derives
+        // the right backend (Docker / local / noop) from the operator's
+        // configuration instead of the unconfigured `RuntimeConfig::default()`.
+        // Falls back to defaults with a warning if the config load fails —
+        // a failed config read shouldn't block tool execution. (CodeRabbit
+        // finding on PR #3309.)
+        let runtime_cfg = match crate::openhuman::config::ops::load_config_with_timeout().await {
+            Ok(cfg) => cfg.runtime,
+            Err(err) => {
+                tracing::warn!(
+                    error = %err,
+                    "[node_exec] failed to load live RuntimeConfig — falling back to defaults"
+                );
+                crate::openhuman::config::RuntimeConfig::default()
+            }
+        };
+        // `is_remote_session = false` matches `ShellTool::run_sandboxed`'s
+        // current behavior (PR #3261). Threading the real session origin
+        // through requires a new `tokio::task_local!` next to
+        // `CURRENT_AGENT_SANDBOX_MODE` and is the same gap across all three
+        // shell-family tools; tracked separately so it can be fixed uniformly.
+        let policy = sandbox::resolve_sandbox_policy(
+            crate::openhuman::agent::harness::definition::SandboxMode::Sandboxed,
+            &self.security.action_dir,
+            &runtime_cfg,
+            false,
+        );
+
+        tracing::debug!(
+            backend = ?policy.backend,
+            runtime_kind = ?runtime_cfg.kind,
+            "[node_exec] routing to sandbox backend"
+        );
+
+        // Forward the managed Node.js bin dir on PATH so the child node
+        // process can resolve `node`, `npm`, `npx`, `corepack` consistently
+        // with the unsandboxed path.
+        let mut extra_env = std::collections::HashMap::new();
+        let host_path = std::env::var("PATH").unwrap_or_default();
+        let sep = if cfg!(windows) { ";" } else { ":" };
+        let prepended = if host_path.is_empty() {
+            bin_dir.to_string_lossy().into_owned()
+        } else {
+            format!("{}{}{}", bin_dir.display(), sep, host_path)
+        };
+        extra_env.insert("PATH".to_string(), prepended);
+
+        match sandbox::execute_in_sandbox(
+            &policy,
+            command,
+            &self.security.action_dir,
+            extra_env,
+            Duration::from_secs(timeout_secs),
+        )
+        .await
+        {
+            Ok(result) => {
+                if result.timed_out {
+                    ToolResult::error(format!(
+                        "node_exec timed out after {timeout_secs}s and was killed"
+                    ))
+                } else if result.success() {
+                    if result.stderr.is_empty() {
+                        ToolResult::success(result.stdout)
+                    } else {
+                        ToolResult::success(format!(
+                            "{}\n[stderr]\n{}",
+                            result.stdout, result.stderr
+                        ))
+                    }
+                } else {
+                    let err_msg = if result.stderr.is_empty() {
+                        result.stdout
+                    } else {
+                        result.stderr
+                    };
+                    ToolResult::error(err_msg)
+                }
+            }
+            Err(e) => ToolResult::error(format!("Sandbox execution failed: {e}")),
+        }
+    }
+}
+
 /// POSIX-safe single-quote escaping. Wraps `s` in `'…'`, turning any embedded
 /// single-quote into the four-char sequence `'\''`. Node bin paths and user
 /// code pass through untouched semantically, but no shell metacharacter can

src/openhuman/tools/impl/system/npm_exec.rs

@@ -241,6 +241,21 @@ impl Tool for NpmExecTool {
         }
         let command = parts.join(" ");
 
+        // When the agent's sandbox mode is `Sandboxed`, route execution
+        // through the sandbox backend (Docker / OS-level `cwd_jail` /
+        // documented noop) instead of the native runtime path. Mirrors
+        // the wiring in `ShellTool::run_with_security` (PR #3261) so
+        // npm_exec gets the same isolation guarantees as shell. The
+        // security/rate-limit checks above still apply.
+        if matches!(
+            crate::openhuman::agent::harness::current_sandbox_mode(),
+            Some(crate::openhuman::agent::harness::definition::SandboxMode::Sandboxed)
+        ) {
+            return Ok(self
+                .run_sandboxed(&command, &cwd, &resolved.bin_dir, timeout_secs)
+                .await);
+        }
+
         let mut cmd = match self.runtime.build_shell_command(&command, &cwd) {
             Ok(cmd) => cmd,
             Err(e) => {
@@ -308,6 +323,108 @@ impl Tool for NpmExecTool {
     }
 }
 
+impl NpmExecTool {
+    /// Execute an npm command through the sandbox backend. Called from
+    /// `execute()` when the agent's `SandboxMode` is `Sandboxed`.
+    ///
+    /// Mirrors `ShellTool::run_sandboxed` and `NodeExecTool::run_sandboxed`.
+    /// The sandbox policy is resolved from the current `RuntimeConfig` and
+    /// rooted at `security.action_dir` — note that the actual child-process
+    /// `working_dir` may be a sub-path of `action_dir` (the resolved `cwd`
+    /// from `cwd_override`), kept consistent with the unsandboxed path.
+    async fn run_sandboxed(
+        &self,
+        command: &str,
+        cwd: &std::path::Path,
+        bin_dir: &std::path::Path,
+        timeout_secs: u64,
+    ) -> ToolResult {
+        use crate::openhuman::sandbox;
+
+        // Load the live `RuntimeConfig` so `resolve_sandbox_policy` derives
+        // the right backend (Docker / local / noop) from the operator's
+        // configuration instead of the unconfigured `RuntimeConfig::default()`.
+        // Falls back to defaults with a warning if the config load fails —
+        // a failed config read shouldn't block tool execution. (CodeRabbit
+        // finding on PR #3309.)
+        let runtime_cfg = match crate::openhuman::config::ops::load_config_with_timeout().await {
+            Ok(cfg) => cfg.runtime,
+            Err(err) => {
+                tracing::warn!(
+                    error = %err,
+                    "[npm_exec] failed to load live RuntimeConfig — falling back to defaults"
+                );
+                crate::openhuman::config::RuntimeConfig::default()
+            }
+        };
+        // `is_remote_session = false` matches `ShellTool::run_sandboxed`'s
+        // current behavior (PR #3261). Threading the real session origin
+        // through requires a new `tokio::task_local!` next to
+        // `CURRENT_AGENT_SANDBOX_MODE` and is the same gap across all three
+        // shell-family tools; tracked separately so it can be fixed uniformly.
+        let policy = sandbox::resolve_sandbox_policy(
+            crate::openhuman::agent::harness::definition::SandboxMode::Sandboxed,
+            &self.security.action_dir,
+            &runtime_cfg,
+            false,
+        );
+
+        tracing::debug!(
+            backend = ?policy.backend,
+            runtime_kind = ?runtime_cfg.kind,
+            "[npm_exec] routing to sandbox backend"
+        );
+
+        // Forward the managed Node.js bin dir on PATH so npm child invocations
+        // (e.g. `npm run` spawning user scripts) resolve `node`/`npx`
+        // consistently with the unsandboxed path.
+        let mut extra_env = std::collections::HashMap::new();
+        let host_path = std::env::var("PATH").unwrap_or_default();
+        let sep = if cfg!(windows) { ";" } else { ":" };
+        let prepended = if host_path.is_empty() {
+            bin_dir.to_string_lossy().into_owned()
+        } else {
+            format!("{}{}{}", bin_dir.display(), sep, host_path)
+        };
+        extra_env.insert("PATH".to_string(), prepended);
+
+        match sandbox::execute_in_sandbox(
+            &policy,
+            command,
+            cwd,
+            extra_env,
+            Duration::from_secs(timeout_secs),
+        )
+        .await
+        {
+            Ok(result) => {
+                if result.timed_out {
+                    ToolResult::error(format!(
+                        "npm_exec timed out after {timeout_secs}s and was killed"
+                    ))
+                } else if result.success() {
+                    if result.stderr.is_empty() {
+                        ToolResult::success(result.stdout)
+                    } else {
+                        ToolResult::success(format!(
+                            "{}\n[stderr]\n{}",
+                            result.stdout, result.stderr
+                        ))
+                    }
+                } else {
+                    let err_msg = if result.stderr.is_empty() {
+                        result.stdout
+                    } else {
+                        result.stderr
+                    };
+                    ToolResult::error(err_msg)
+                }
+            }
+            Err(e) => ToolResult::error(format!("Sandbox execution failed: {e}")),
+        }
+    }
+}
+
 /// POSIX-safe single-quote escaping (mirrors the helper in `node_exec`).
 /// Wraps `s` in `'…'`, turning any embedded single-quote into `'\''` so no
 /// shell metacharacter can escape the quoted string.

src/openhuman/tools/impl/system/shell.rs

@@ -937,4 +937,50 @@ mod tests {
             result.output()
         );
     }
+
+    /// Regression guard for #3235 (cwd_jail wiring for shell-family tools).
+    ///
+    /// PR #3261 wired `ShellTool` to route through `sandbox::execute_in_sandbox`
+    /// (which uses `cwd_jail` for the local-OS-jail backend) when the
+    /// active agent's `SandboxMode::Sandboxed` is set. This PR extends the
+    /// same wiring to `NodeExecTool` and `NpmExecTool`. The behavioural
+    /// `shell_sandboxed_mode_routes_through_sandbox_backend` test above
+    /// proves the contract end-to-end for `shell` (no managed-Node
+    /// dependency); `node_exec` and `npm_exec` cannot run end-to-end in
+    /// unit tests without a resolved `NodeBootstrap`, so this source-grep
+    /// guard catches refactors that drop the sandbox check from either
+    /// tool's `execute()` body.
+    #[test]
+    fn shell_family_tools_route_to_sandbox_when_sandboxed_mode_active() {
+        const SHELL_SRC: &str = include_str!("shell.rs");
+        const NODE_EXEC_SRC: &str = include_str!("node_exec.rs");
+        const NPM_EXEC_SRC: &str = include_str!("npm_exec.rs");
+
+        for (name, src) in [
+            ("shell.rs", SHELL_SRC),
+            ("node_exec.rs", NODE_EXEC_SRC),
+            ("npm_exec.rs", NPM_EXEC_SRC),
+        ] {
+            assert!(
+                src.contains("current_sandbox_mode()"),
+                "{name} must check `current_sandbox_mode()` to detect SandboxMode::Sandboxed \
+                 sessions and route through the sandbox backend (see #3235)"
+            );
+            assert!(
+                src.contains("SandboxMode::Sandboxed"),
+                "{name} must compare against `SandboxMode::Sandboxed` to opt in to the \
+                 sandbox routing path (see #3235)"
+            );
+            // Use the call-site pattern `.run_sandboxed(` so the assertion
+            // doesn't trivially pass on the helper definition itself
+            // (`fn run_sandboxed(...)`). If `execute()` / `run_with_security()`
+            // stop delegating, this fires even though the helper still exists.
+            assert!(
+                src.contains(".run_sandboxed("),
+                "{name} must delegate to a `run_sandboxed` helper when the sandbox mode is \
+                 active (see #3235). Whitespace before `.run_sandboxed` is tolerated; the \
+                 helper call must appear in the source — *not* just the helper definition."
+            );
+        }
+    }
 }