diff --git a/Dockerfile b/Dockerfile index 7388f3dc7..438462390 100644 --- a/Dockerfile +++ b/Dockerfile @@ -52,6 +52,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ libsqlite3-0 \ curl \ gh \ + bubblewrap \ && rm -rf /var/lib/apt/lists/* COPY --from=builder /usr/local/bin/spacebot /usr/local/bin/spacebot diff --git a/docs/content/docs/(configuration)/config.mdx b/docs/content/docs/(configuration)/config.mdx index acba06dfc..f098b6d73 100644 --- a/docs/content/docs/(configuration)/config.mdx +++ b/docs/content/docs/(configuration)/config.mdx @@ -119,6 +119,11 @@ cron_timezone = "America/Los_Angeles" # optional per-agent cron timezone overri [agents.routing] channel = "anthropic/claude-opus-4-20250514" +# Per-agent sandbox configuration. +[agents.sandbox] +mode = "enabled" # "enabled" (default) or "disabled" +writable_paths = ["/home/user/projects/myapp"] # additional writable directories + # Per-agent cron jobs. [[agents.cron]] id = "daily-check" @@ -516,6 +521,34 @@ When branch/worker/cron dispatch happens before readiness is satisfied, Spacebot Agent-specific routing is set via `[agents.routing]` with the same keys as `[defaults.routing]`. +### `[agents.sandbox]` + +OS-level filesystem containment for shell and exec tool subprocesses. Uses bubblewrap (Linux) or sandbox-exec (macOS) to enforce read-only access to everything outside the workspace. + +| Key | Type | Default | Description | +|-----|------|---------|-------------| +| `mode` | string | `"enabled"` | `"enabled"` for kernel-enforced containment, `"disabled"` for full host access | +| `writable_paths` | string[] | `[]` | Additional directories the agent can write to beyond its workspace | + +When `mode = "enabled"`, shell and exec commands run inside a mount namespace where the entire filesystem is read-only except: + +- The agent's workspace directory +- `/tmp` (private per invocation) +- `/dev` (standard device nodes) +- Any paths listed in `writable_paths` + +The agent's data directory (databases, config) is explicitly re-mounted read-only even if it would otherwise be writable due to path overlap. + +When `SPACEBOT_DEPLOYMENT=hosted`, sandbox mode is always `"enabled"` regardless of config. + +If the sandbox backend isn't available (e.g. bubblewrap not installed), processes run unsandboxed with a warning at startup. + +```toml +[agents.sandbox] +mode = "enabled" +writable_paths = ["/home/user/projects/myapp", "/var/data/shared"] +``` + ### `[[agents.cron]]` | Key | Type | Default | Description | diff --git a/docs/content/docs/(features)/tools.mdx b/docs/content/docs/(features)/tools.mdx index e714f8db8..f1def1fc5 100644 --- a/docs/content/docs/(features)/tools.mdx +++ b/docs/content/docs/(features)/tools.mdx @@ -92,7 +92,7 @@ Each worker gets its own isolated ToolServer, created at spawn time via `create_ └──────────────────────────────────────────┘ ``` -`shell`, `file`, and `exec` are stateless unit structs. `set_status` is bound to a specific worker's ID so status updates route to the right place in the channel's status block. `browser` is conditionally registered based on the agent's `browser.enabled` config. +`shell` and `exec` hold a shared `Sandbox` reference that wraps commands in OS-level containment (bubblewrap on Linux, sandbox-exec on macOS). `file` validates paths against the workspace boundary. `set_status` is bound to a specific worker's ID so status updates route to the right place in the channel's status block. `browser` is conditionally registered based on the agent's `browser.enabled` config. Workers don't get memory tools or channel tools. They can't talk to the user, can't recall memories, can't spawn branches. They execute their task and report status. @@ -171,9 +171,13 @@ async fn call(&self, args: Self::Args) -> Result { } ``` -### Protected paths +### Sandbox containment -The `file` tool rejects reads and writes to identity/memory paths (`prompts/`, `identity/`, `data/`, `SOUL.md`, `IDENTITY.md`, `USER.md`). Workers should use `memory_save` for that content, not raw file writes. +Shell and exec commands run inside an OS-level sandbox (bubblewrap on Linux, sandbox-exec on macOS). The entire filesystem is mounted read-only except the workspace, `/tmp`, and any configured `writable_paths`. The agent's data directory (databases, config files) is explicitly protected. See [Configuration](/docs/config#agentssandbox) for sandbox config options. + +The `file` tool independently validates paths against the workspace boundary and rejects writes to identity files (`SOUL.md`, `IDENTITY.md`, `USER.md`). The `exec` tool blocks dangerous environment variables (`LD_PRELOAD`, `DYLD_INSERT_LIBRARIES`, etc.) that enable library injection regardless of sandbox state. + +Leak detection (via `SpacebotHook`) scans all tool output for secret patterns (API keys, tokens, PEM keys) and terminates the process if a leak is found. ### Status reporting @@ -230,7 +234,7 @@ Reports the worker's current progress. The status string appears in the channel' ### shell -Runs a shell command via `sh -c` (Unix) or `cmd /C` (Windows). Captures stdout, stderr, exit code. Has a configurable timeout (default 60s). +Runs a shell command via `sh -c` (Unix) or `cmd /C` (Windows). Captures stdout, stderr, exit code. Has a configurable timeout (default 60s). Commands are wrapped in the sandbox when enabled — the filesystem is read-only except for the workspace and configured writable paths. ### file @@ -238,7 +242,7 @@ Read, write, or list files. Protects identity/memory paths. Creates parent direc ### exec -Runs a specific program with explicit arguments and environment variables. More precise than `shell` for running compilers, test runners, etc. Configurable timeout. +Runs a specific program with explicit arguments and environment variables. More precise than `shell` for running compilers, test runners, etc. Configurable timeout. Sandboxed like `shell`. Blocks dangerous env vars (`LD_PRELOAD`, `NODE_OPTIONS`, etc.) that enable code injection. ### browser diff --git a/docs/content/docs/(features)/workers.mdx b/docs/content/docs/(features)/workers.mdx index 30280a9ed..3e0a1096a 100644 --- a/docs/content/docs/(features)/workers.mdx +++ b/docs/content/docs/(features)/workers.mdx @@ -143,15 +143,19 @@ Worker execution logs are controlled by `worker_log_mode`: Logs include: worker ID, channel ID, timestamp, state, task, error (if any), and the full message history with tool calls and results. -## Protected Paths +## Sandbox and Protected Paths -The `file` and `shell` tools reject operations on protected paths: +Worker shell and exec commands run inside an OS-level sandbox (bubblewrap on Linux, sandbox-exec on macOS). The entire host filesystem is mounted read-only except: -- Identity files: `SOUL.md`, `IDENTITY.md`, `USER.md` -- System paths: `/prompts/`, `/data/spacebot.db`, `/data/lancedb/`, `/data/config.redb` -- Archives and ingest directories +- The agent's workspace directory (writable) +- `/tmp` (private per invocation) +- Any paths listed in `[agents.sandbox].writable_paths` -This prevents workers from accidentally corrupting system state. +The agent's data directory (databases, config files) is explicitly re-mounted read-only. This is enforced at the kernel level — no amount of command creativity can bypass it. + +The `file` tool additionally validates paths against the workspace boundary and blocks writes to identity files (`SOUL.md`, `IDENTITY.md`, `USER.md`). The `exec` tool blocks dangerous environment variables (`LD_PRELOAD`, `DYLD_INSERT_LIBRARIES`, etc.) that enable library injection. + +Sandbox mode is configurable per agent via `[agents.sandbox]`. See [Configuration](/docs/config#agentssandbox). ## Configuration @@ -160,11 +164,17 @@ This prevents workers from accidentally corrupting system state. max_concurrent_workers = 5 # per channel context_window = 128000 # tokens -[routing] +[defaults.routing] worker = "anthropic/claude-haiku-4.5-20250514" -[routing.task_overrides] +[defaults.routing.task_overrides] coding = "anthropic/claude-sonnet-4-20250514" + +# Sandbox is enabled by default. Disable for self-hosted/local setups +# that need full host filesystem access. +[agents.sandbox] +mode = "enabled" +writable_paths = [] ``` ## OpenCode Workers diff --git a/docs/content/docs/(getting-started)/docker.mdx b/docs/content/docs/(getting-started)/docker.mdx index 8b4c2e584..2302f5097 100644 --- a/docs/content/docs/(getting-started)/docker.mdx +++ b/docs/content/docs/(getting-started)/docker.mdx @@ -28,7 +28,7 @@ Minimal runtime. Everything works except the browser tool. - Base: `debian:bookworm-slim` - Size: ~150MB -- Includes: Spacebot binary, CA certs, SQLite libs, embedded frontend +- Includes: Spacebot binary, CA certs, SQLite libs, bubblewrap (process sandbox), embedded frontend ### `spacebot:full` diff --git a/docs/design-docs/sandbox.md b/docs/design-docs/sandbox.md new file mode 100644 index 000000000..62333fb19 --- /dev/null +++ b/docs/design-docs/sandbox.md @@ -0,0 +1,378 @@ +# Process Sandbox + +OS-level filesystem containment for shell and exec tool subprocesses. Replaces the current string-based command filtering with kernel-enforced boundaries that no amount of LLM creativity can bypass. + +## Context + +Workers execute arbitrary shell commands via `sh -c`. The current security model uses string inspection to block dangerous commands before execution — checking for references to the instance directory, sensitive filenames, secret env vars, subshells, `eval`, interpreter one-liners, `/proc` paths, and env dump builtins. Around 180 lines of pattern matching in `shell.rs` and 30 in `exec.rs`. + +This doesn't work. An LLM that hits a blocked command pivots to an equivalent that the string checks don't cover: `cp workspace/file /tmp/`, `cat > /tmp/file`, `find -exec`, heredocs, script files, language runtimes in non-`-c` mode, etc. The log from worker `022aa8f2` shows exactly this — the `file` tool correctly blocked a read outside workspace, so the worker used `shell` + `cp` instead and succeeded. + +The `file` tool's path canonicalization + `starts_with` check is solid for its own operations. But shell/exec spawn real OS processes with full host access. String filtering is whack-a-mole against an adversary that can try unlimited variations. + +## Current Security Inventory + +Everything that exists today, what it does, and what happens to it. + +### `shell.rs` — ShellTool + +| Item | Lines | What it does | Disposition | +|------|-------|-------------|-------------| +| `SENSITIVE_FILES` constant | 12-18 | Blocklist of 5 filenames: `config.toml`, `config.redb`, `settings.redb`, `.env`, `spacebot.db` | **Remove.** Sandbox makes these read-only via mount namespace. | +| `SECRET_ENV_VARS` constant | 21-30 | Blocklist of 8 env var names: API keys for Anthropic, OpenAI, OpenRouter, Discord, Slack (bot + app), Telegram, Brave Search | **Remove.** Sandbox inherits the parent's env, but the sandbox blocks reading `/proc/self/environ`. Leak detection in `SpacebotHook` catches any secrets that make it into tool output. | +| `instance_dir` field | 36 | Stored on `ShellTool` for path blocking comparisons | **Remove.** No longer needed — sandbox handles containment. | +| `check_command()` method | 50-232 | Pre-execution string inspection with 9 categories of checks (detailed below) | **Remove entirely.** All 182 lines. | +| — Instance dir blocking | 53-69 | Blocks commands containing the instance dir path (unless they also mention workspace) | Replaced by bwrap `--ro-bind` making the instance dir read-only | +| — Sensitive file blocking | 71-90 | Blocks commands mentioning any of the 5 `SENSITIVE_FILES` names unless clearly targeting workspace | Replaced by bwrap mount namespace — files aren't writable | +| — Secret env var expansion | 92-118 | Blocks `$VAR`, `${VAR}`, `printenv VAR` patterns for 8 secret var names, including unbraced `$VAR` at word boundaries | Replaced by sandbox env isolation + leak detection hook | +| — Broad env dumps | 120-144 | Blocks bare `printenv`, `env`, `env |`, `env >` | Replaced by sandbox env isolation + leak detection hook | +| — Shell builtin dumps | 146-163 | Blocks `set`, `declare -p`, `export -p`, `compgen -e`, `compgen -v` as standalone commands or in pipes/chains | Replaced by sandbox env isolation + leak detection hook | +| — Subshell blocking | 165-178 | Blocks backticks, `$(...)`, `<(...)`, `>(...)` | **Remove.** These were only blocked to prevent dynamic construction of blocked commands. With real sandboxing, there's nothing to dynamically bypass. | +| — eval/exec blocking | 180-186 | Blocks `eval` and `exec` builtins | **Remove.** Same rationale — no string checks to bypass. | +| — Interpreter one-liners | 188-205 | Blocks `python3 -c`, `python -c`, `perl -e`, `ruby -e`, `node -e`, `node --eval` | **Remove.** These were blocked to prevent bypassing the string checks via a different language. Sandbox makes this irrelevant. | +| — /proc and /dev paths | 207-217 | Blocks `/proc/self/environ`, `/proc/*/environ`, `/dev/fd/`, `/dev/stdin` | Replaced by bwrap `--proc /proc` (fresh procfs) + leak detection hook | +| — Shell state dumps | 219-229 | Blocks `set`, `declare -p`, `compgen`, `export` builtins (partial duplicate of lines 146-163) | Replaced by sandbox env isolation + leak detection hook | +| `contains_shell_builtin()` helper | 423-434 | Splits command on `|;& ` and checks if a segment starts with a given builtin | **Remove.** Only used by `check_command()`. | +| Working dir validation | 316-333 | Canonicalizes `working_dir` arg and checks `starts_with(workspace)` | **Keep.** Gives the LLM a descriptive error instead of a confusing cwd-not-found from bwrap. | +| `tools/bin` PATH prepend | 352-357 | Prepends `{instance_dir}/tools/bin` to PATH | **Keep**, but needs adjustment — the sandbox module will handle passing this path through since `instance_dir` is being removed from ShellTool. | +| System-internal `shell()` fn | 438-471 | Bypasses all checks, used by the system (not LLM-facing) | **Keep unchanged.** Not LLM-facing, no sandbox wrapping needed. | +| `ShellError`, `ShellArgs`, `ShellOutput` types | 236-275 | Tool types | **Keep unchanged.** | +| `format_shell_output()` | 399-419 | Output formatting | **Keep unchanged.** | +| `ShellResult` type + `format()` | 474-487 | System-internal result type | **Keep unchanged.** | + +### `exec.rs` — ExecTool + +| Item | Lines | What it does | Disposition | +|------|-------|-------------|-------------| +| `instance_dir` field | 15 | Stored on `ExecTool` for path blocking comparisons | **Remove.** | +| `check_args()` method | 29-59 | Pre-execution arg inspection with 2 checks | **Remove entirely.** All 31 lines. | +| — Instance dir blocking | 37-43 | Blocks if joined args contain instance dir but not workspace | Replaced by sandbox mount namespace | +| — Sensitive file blocking | 45-56 | Blocks args mentioning `SENSITIVE_FILES` unless clearly in workspace | Replaced by sandbox mount namespace | +| Secret env var blocking | 203-213 | Blocks setting any of the 8 `SECRET_ENV_VARS` via the `env` parameter | **Remove.** Sandbox prevents writing to protected paths. The env vars themselves are process-inherited and the LLM can't change them via this tool anyway — this check was blocking the LLM from *naming* a secret var as a key, not from accessing the value. | +| `DANGEROUS_ENV_VARS` blocking | 215-244 | Blocks setting 12 dangerous env vars: `LD_PRELOAD`, `LD_LIBRARY_PATH`, `DYLD_INSERT_LIBRARIES`, `DYLD_LIBRARY_PATH`, `PYTHONPATH`, `PYTHONSTARTUP`, `NODE_OPTIONS`, `RUBYOPT`, `PERL5OPT`, `PERL5LIB`, `BASH_ENV`, `ENV` | **Keep.** These enable code injection by altering how the child process loads libraries/modules. bwrap doesn't prevent env var injection — it only controls filesystem visibility. An LLM setting `LD_PRELOAD=/workspace/evil.so` is still dangerous even in a sandbox. | +| Working dir validation | 184-201 | Canonicalizes `working_dir` arg and checks `starts_with(workspace)` | **Keep.** Same rationale as ShellTool. | +| `tools/bin` PATH prepend | 256-261 | Prepends `{instance_dir}/tools/bin` to PATH | **Keep**, same adjustment needed as ShellTool. | +| System-internal `exec()` fn | 330-362 | Bypasses all checks, used by the system (not LLM-facing) | **Keep unchanged.** | +| All type definitions | 62-117 | `ExecError`, `ExecArgs`, `EnvVar`, `ExecOutput` | **Keep unchanged.** | +| `format_exec_output()` | 306-326 | Output formatting | **Keep unchanged.** | +| `ExecResult` type | 365-371 | System-internal result type | **Keep unchanged.** | + +### `file.rs` — FileTool + +| Item | Lines | What it does | Disposition | +|------|-------|-------------|-------------| +| `resolve_path()` method | 26-75 | Canonicalizes path, checks `starts_with(workspace)`, rejects symlinks | **Keep unchanged.** This is in-process I/O, not subprocess spawning. The sandbox doesn't apply here. | +| `best_effort_canonicalize()` | 81-106 | Walks up to deepest existing ancestor for paths that don't fully exist yet | **Keep unchanged.** Used by `resolve_path()`. | +| Protected identity files | 203-216 | Blocks writes to `SOUL.md`, `IDENTITY.md`, `USER.md` (case-insensitive) | **Keep unchanged.** Application-level protection, not security boundary. | +| System-internal `file_read/write/list` | 362-404 | Bypass workspace containment, used by the system | **Keep unchanged.** | + +### `send_file.rs` — SendFileTool + +| Item | Lines | What it does | Disposition | +|------|-------|-------------|-------------| +| `call()` method | 81-144 | Accepts any absolute path, reads file, sends as attachment. Only checks: is_absolute, is_file, max 25MB | **Fix.** Add workspace path validation (same `resolve_path` pattern from FileTool). Currently any readable file on disk can be exfiltrated via this tool. | + +### `hooks/spacebot.rs` — SpacebotHook (Leak Detection) + +| Item | Lines | What it does | Disposition | +|------|-------|-------------|-------------| +| `match_patterns()` | 47-75 | 11 regex patterns matching secret prefixes: `sk-*` (OpenAI), `sk-ant-*` (Anthropic), `sk-or-*` (OpenRouter), PEM private keys, `ghp_*` (GitHub), `AIza*` (Google), Discord bot tokens, `xoxb-*` (Slack bot), `xapp-*` (Slack app), Telegram bot tokens, `BSA*` (Brave) | **Keep unchanged.** | +| `scan_for_leaks()` | 82-135 | Multi-layer content scanning: raw plaintext, URL-decoded (`sk%2Dant%2D...`), base64-decoded (standard + URL-safe), hex-decoded. Min 24 chars for base64, 40 hex chars to reduce false positives | **Keep unchanged.** | +| `on_tool_call()` hook | 175-218 | Scans tool arguments before execution. If leak found, returns `ToolCallHookAction::Skip` (tool call is not executed) | **Keep unchanged.** | +| `on_tool_result()` hook | 220-293 | Scans tool output after execution. If leak found, returns `HookAction::Terminate` (agent is killed to prevent exfiltration via subsequent tool calls) | **Keep unchanged.** | + +### `browser.rs` — BrowserTool (SSRF Protection) + +| Item | Lines | What it does | Disposition | +|------|-------|-------------|-------------| +| `validate_url()` | 33-80 | Blocks non-http/https schemes, cloud metadata endpoints (169.254.169.254, metadata.google.internal, metadata.aws.internal), private/loopback/link-local/CGNAT/broadcast/unspecified IPs | **Keep unchanged.** Orthogonal to filesystem sandboxing. | +| `is_blocked_ip()` | 84-102 | Checks IPv4 loopback/private/link-local/broadcast/unspecified/CGNAT and IPv6 loopback/unique-local/link-local/v4-mapped ranges | **Keep unchanged.** | + +## Summary of Changes + +**Removed (~215 lines):** +- `shell.rs`: `check_command()` (182 lines), `contains_shell_builtin()` (12 lines), `SENSITIVE_FILES` (7 lines), `SECRET_ENV_VARS` (10 lines), `instance_dir` field, `check_command()` call site in `call()` +- `exec.rs`: `check_args()` (31 lines), `instance_dir` field, `check_args()` call site in `call()`, secret env var blocking (10 lines) + +**Kept:** +- `exec.rs`: `DANGEROUS_ENV_VARS` blocking (library injection protection) +- `shell.rs` + `exec.rs`: working dir validation, `tools/bin` PATH prepend, system-internal functions +- `file.rs`: everything (resolve_path, symlink check, identity file protection) +- `hooks/spacebot.rs`: everything (leak detection — plaintext, URL-encoded, base64, hex) +- `browser.rs`: everything (SSRF protection) + +**Fixed:** +- `send_file.rs`: add workspace path validation (currently has none) + +## Design + +### Config + +New `sandbox` section on the agent config: + +```toml +[[agents]] +id = "main" + +[agents.sandbox] +# "enabled" (default) - kernel-enforced filesystem containment +# "disabled" - full host access (self-hosted/local only) +mode = "enabled" + +# Additional directories the agent can write to beyond its workspace. +# The workspace itself is always writable when sandbox is enabled. +writable_paths = ["/home/user/projects/myapp"] +``` + +When `SPACEBOT_DEPLOYMENT=hosted`, the platform boot script forces `mode = "enabled"` regardless of user config. + +### Types + +```rust +pub struct SandboxConfig { + pub mode: SandboxMode, + pub writable_paths: Vec, +} + +pub enum SandboxMode { + Enabled, // OS-level containment (default) + Disabled, // No containment, full host access +} +``` + +Added to `AgentConfig` as `sandbox: Option`, resolved to `ResolvedAgentConfig` with defaults (`mode = Enabled`, `writable_paths = []`). Added to `RuntimeConfig` as `ArcSwap` for hot-reload. + +### Sandbox Module + +New module at `src/sandbox.rs`. + +```rust +pub struct Sandbox { + mode: SandboxMode, + workspace: PathBuf, + writable_paths: Vec, + backend: SandboxBackend, +} + +enum SandboxBackend { + Bubblewrap, // Linux: bwrap available + SandboxExec, // macOS: /usr/bin/sandbox-exec + None, // No sandbox support detected, or mode = Disabled +} +``` + +#### Detection + +At agent startup, probe for sandbox support: + +- **Linux:** Check if `bwrap --version` succeeds, then run a preflight: `bwrap --ro-bind / / --proc /proc -- /usr/bin/true`. If `--proc /proc` fails (nested container restrictions), retry without it and remember the fallback. +- **macOS:** Check if `/usr/bin/sandbox-exec` exists. Hardcode the full path to prevent PATH injection. +- **Either platform, mode = Disabled:** `SandboxBackend::None`. +- **Backend unavailable but mode = Enabled:** Log a warning at startup. Fall back to `None` rather than refusing to start — degraded security is better than a broken agent. + +#### Command Wrapping + +`Sandbox` exposes a single method that transforms a `tokio::process::Command` before spawning: + +```rust +impl Sandbox { + /// Wrap a command for sandboxed execution. + /// Returns the (possibly modified) Command ready to spawn. + pub fn wrap(&self, program: &str, args: &[&str], working_dir: &Path) -> Command +} +``` + +**Linux (bubblewrap):** + +``` +bwrap + --ro-bind / / # entire filesystem read-only + --dev /dev # writable /dev with standard nodes + --proc /proc # fresh /proc (skipped if preflight failed) + --tmpfs /tmp # private tmp per invocation + --bind # workspace writable + --bind # each configured writable path + --ro-bind # re-protect agent data dir + --unshare-pid # isolated PID namespace + --new-session # prevent TIOCSTI TTY injection + --die-with-parent # kill child if spacebot dies + --chdir # set cwd + -- sh -c "" # the actual command +``` + +Mount order matters — later mounts override earlier ones at the same path: +1. `--ro-bind / /` makes everything read-only +2. `--bind ` re-enables writes for the workspace +3. `--ro-bind ` re-applies read-only on the data directory (which lives under the instance dir, potentially overlapping with workspace's parent) + +The agent's `data_dir` (containing `spacebot.db`, `config.redb`, `settings.redb`, LanceDB) is explicitly re-mounted read-only even though `--ro-bind / /` already covers it. This ensures it stays protected if the workspace mount would otherwise make it writable (the default workspace is `{instance_dir}/agents/{id}/workspace`, and the data dir is `{instance_dir}/agents/{id}/data`). + +**macOS (sandbox-exec):** + +``` +/usr/bin/sandbox-exec + -p + -DWORKSPACE= + -DWRITABLE_0= + ... + -- sh -c "" +``` + +Generated SBPL profile: + +```scheme +(version 1) +(deny default) + +; process basics +(allow process-exec) +(allow process-fork) +(allow signal (target same-sandbox)) +(allow process-info* (target same-sandbox)) + +; filesystem: read everything, write workspace + configured paths +(allow file-read*) +(allow file-write* (subpath (param "WORKSPACE"))) +; one (allow file-write* (subpath (param "WRITABLE_N"))) per configured path + +; dev, sysctl, mach for basic operation +(allow file-write-data + (require-all (path "/dev/null") (vnode-type CHARACTER-DEVICE))) +(allow sysctl-read) +(allow mach-lookup + (global-name "com.apple.system.opendirectoryd.libinfo")) +(allow ipc-posix-sem) +(allow pseudo-tty) +``` + +All paths are canonicalized before being passed as params — `/var` on macOS is actually `/private/var`. + +**Fallback (no backend):** + +Pass through unchanged. The command runs unsandboxed. The warning at startup is the only indication. + +### Tool Integration + +`ShellTool` and `ExecTool` gain a `sandbox: Arc` field. Set during tool server creation. + +**ShellTool::call() changes:** + +```rust +// Before (current): +let mut cmd = Command::new("sh"); +cmd.arg("-c").arg(&args.command); +cmd.current_dir(&self.workspace); + +// After: +let cmd = self.sandbox.wrap("sh", &["-c", &args.command], &working_dir); +``` + +**ExecTool::call() changes:** + +```rust +// Before (current): +let mut cmd = Command::new(&args.program); +cmd.args(&args.args); +cmd.current_dir(&self.workspace); + +// After: +let arg_refs: Vec<&str> = args.args.iter().map(|s| s.as_str()).collect(); +let cmd = self.sandbox.wrap(&args.program, &arg_refs, &working_dir); +``` + +`ExecTool` retains the `DANGEROUS_ENV_VARS` check (blocking `LD_PRELOAD`, `DYLD_INSERT_LIBRARIES`, etc.) since these enable code injection regardless of filesystem sandbox state. + +Both tools still set `stdout(Stdio::piped())`, `stderr(Stdio::piped())`, timeout, PATH, and output truncation after getting the wrapped command. The sandbox only affects how the process is spawned. + +### Tool Server Wiring + +`create_worker_tool_server()` and `create_cortex_chat_tool_server()` gain a `sandbox: Arc` parameter and pass it to `ShellTool` and `ExecTool`: + +```rust +pub fn create_worker_tool_server( + // ... existing params ... + sandbox: Arc, +) -> ToolServerHandle { + ToolServer::new() + .tool(ShellTool::new(workspace.clone(), sandbox.clone())) + .tool(FileTool::new(workspace.clone())) + .tool(ExecTool::new(workspace, sandbox)) + // ... rest unchanged ... +} +``` + +The `Sandbox` is created once per agent during startup (in `initialize_agents()`) and shared via `Arc` across all workers for that agent. + +### `send_file` Fix + +The `send_file` tool (channel-only) currently reads any absolute path with no workspace check. Add the same `resolve_path` logic from `FileTool`: + +```rust +async fn call(&self, args: Self::Args) -> Result { + let path = PathBuf::from(&args.file_path); + let resolved = self.resolve_path(&path)?; // same canonicalize + starts_with check + // ... rest of existing logic using resolved path ... +} +``` + +### `tools/bin` Persistent Directory + +Currently `{instance_dir}/tools/bin` is prepended to PATH so LLM-installed binaries persist. Under the sandbox, this directory is read-only (it's under instance_dir). Binaries there are still executable — the LLM can run them, but can't install new ones from inside a sandboxed shell. + +Tool installation needs to happen through a dedicated non-sandboxed path (e.g., a `install_tool` tool or the skills system). This is a behavior change worth documenting but not a regression — the current system already has the same problem when `check_command()` blocks writes to the instance dir. + +## Dockerfile Change + +Add bubblewrap to both `slim` and `full` stages: + +```dockerfile +RUN apt-get update && apt-get install -y --no-install-recommends \ + ca-certificates libsqlite3-0 curl bubblewrap \ + && ... +``` + +bubblewrap is ~50KB installed. Available in Debian bookworm's default repos. + +## Dependencies + +```toml +# None for bubblewrap — it's an external binary invoked via Command +# None for sandbox-exec — it's a macOS system binary + +# Only if we later add Landlock as a secondary layer: +# landlock = "0.4" +``` + +No new Rust dependencies for the initial implementation. bubblewrap and sandbox-exec are invoked as subprocess wrappers via `tokio::process::Command`. + +## File Changes + +| File | Change | +|------|--------| +| `src/sandbox.rs` | New module: `Sandbox`, `SandboxConfig`, `SandboxMode`, `SandboxBackend`, detection, wrapping | +| `src/config.rs` | `SandboxConfig` on `AgentConfig`/`ResolvedAgentConfig`/`RuntimeConfig`, TOML parsing | +| `src/tools/shell.rs` | Remove `check_command()` (lines 50-232), `SENSITIVE_FILES` (lines 12-18), `SECRET_ENV_VARS` (lines 21-30), `contains_shell_builtin()` (lines 423-434), `instance_dir` field (line 36). Replace `instance_dir` in constructor with `sandbox: Arc`. Replace `Command::new("sh")` block (lines 335-343) with `sandbox.wrap()`. Move `tools/bin` PATH setup into sandbox. Keep: working dir validation, timeout, output truncation, format, system-internal `shell()` fn | +| `src/tools/exec.rs` | Remove `check_args()` (lines 29-59), `instance_dir` field (line 15), secret env var blocking (lines 203-213). Replace `instance_dir` in constructor with `sandbox: Arc`. Replace `Command::new(&args.program)` (line 246) with `sandbox.wrap()`. Move `tools/bin` PATH setup into sandbox. Keep: `DANGEROUS_ENV_VARS` blocking (lines 217-244), working dir validation, timeout, output truncation, format, system-internal `exec()` fn | +| `src/tools/send_file.rs` | Add `workspace: PathBuf` field, add `resolve_path()` method (reuse logic from `file.rs`), validate path in `call()` before reading | +| `src/tools.rs` | Update `create_worker_tool_server()` and `create_cortex_chat_tool_server()` signatures: drop `instance_dir` param, add `sandbox: Arc` param. Pass `sandbox` to `ShellTool::new()` and `ExecTool::new()` | +| `src/lib.rs` | Add `pub mod sandbox` | +| `src/main.rs` | Create `Sandbox` during `initialize_agents()`, run preflight probe at startup | +| `Dockerfile` | Add `bubblewrap` to apt-get install in both `slim` and `full` stages | + +## What This Doesn't Cover + +- **Network isolation.** A sandboxed process can still `curl` data out. bubblewrap's `--unshare-net` can block all networking, but many legitimate tasks need network access (git clone, npm install, curl). Network policy is a separate design decision — could be a future `network_isolated` config flag, or a proxy-based approach like Codex uses. +- **Resource limits.** CPU and memory abuse. bubblewrap doesn't do cgroups. The existing timeout mechanism and Fly VM resource limits cover this for now. +- **Landlock as a secondary layer.** The `landlock` crate (v0.4) could add defense-in-depth inside the bwrap namespace. Not needed for v1 — bwrap's mount namespace isolation is sufficient. Worth revisiting if we want to restrict filesystem access within the sandbox more granularly (e.g., read-only access to specific subdirectories of the workspace). + +## Phase Ordering + +``` +Phase 1 (sandbox module) — Sandbox struct, detection, wrapping logic, SBPL generation +Phase 2 (tool integration) — Wire into ShellTool, ExecTool, remove old checks +Phase 3 (config) — SandboxConfig parsing, hot-reload, hosted enforcement +Phase 4 (send_file) — Add workspace path validation +Phase 5 (Dockerfile) — Add bubblewrap, verify on Fly +``` + +Phases 1-3 are tightly coupled and should land together. Phase 4 is independent. Phase 5 requires a Docker image rebuild and Fly deployment. diff --git a/src/agent/worker.rs b/src/agent/worker.rs index 14f725e7c..6f941d7f9 100644 --- a/src/agent/worker.rs +++ b/src/agent/worker.rs @@ -202,7 +202,7 @@ impl Worker { self.screenshot_dir.clone(), self.brave_search_key.clone(), self.deps.runtime_config.workspace_dir.clone(), - self.deps.runtime_config.instance_dir.clone(), + self.deps.sandbox.clone(), mcp_tools, self.deps.runtime_config.clone(), ); diff --git a/src/api/agents.rs b/src/api/agents.rs index 29a27adcb..c139c4ffe 100644 --- a/src/api/agents.rs +++ b/src/api/agents.rs @@ -360,6 +360,7 @@ pub(super) async fn trigger_warmup( let memory_searches = state.memory_searches.load(); let mcp_managers = state.mcp_managers.load(); let pools = state.agent_pools.load(); + let sandboxes = state.sandboxes.load(); let runtime_config_ids = runtime_configs.keys().cloned().collect::>(); let memory_search_ids = memory_searches.keys().cloned().collect::>(); @@ -387,6 +388,9 @@ pub(super) async fn trigger_warmup( let Some(sqlite_pool) = pools.get(agent_id).cloned() else { continue; }; + let Some(sandbox) = sandboxes.get(agent_id).cloned() else { + continue; + }; let llm_manager = llm_manager.clone(); let force = request.force; @@ -403,6 +407,7 @@ pub(super) async fn trigger_warmup( event_tx, sqlite_pool: sqlite_pool.clone(), messaging_manager: None, + sandbox, links: Arc::new(arc_swap::ArcSwap::from_pointee(Vec::new())), agent_names: Arc::new(std::collections::HashMap::new()), }; @@ -525,6 +530,7 @@ pub(super) async fn create_agent( mcp: None, brave_search_key: None, cron_timezone: None, + sandbox: None, cron: Vec::new(), }; let agent_config = raw_config.resolve(&instance_dir, defaults); @@ -648,6 +654,16 @@ pub(super) async fn create_agent( let mcp_manager = std::sync::Arc::new(crate::mcp::McpManager::new(agent_config.mcp.clone())); mcp_manager.connect_all().await; + let sandbox = std::sync::Arc::new( + crate::sandbox::Sandbox::new( + &agent_config.sandbox, + agent_config.workspace.clone(), + &instance_dir, + agent_config.data_dir.clone(), + ) + .await, + ); + let deps = crate::AgentDeps { agent_id: arc_agent_id.clone(), memory_search: memory_search.clone(), @@ -661,6 +677,7 @@ pub(super) async fn create_agent( let guard = state.messaging_manager.read().await; guard.as_ref().cloned() }, + sandbox: sandbox.clone(), links: Arc::new(arc_swap::ArcSwap::from_pointee( (**state.agent_links.load()).clone(), )), @@ -713,6 +730,7 @@ pub(super) async fn create_agent( let conversation_logger = crate::conversation::history::ConversationLogger::new(db.sqlite.clone()); let channel_store = crate::conversation::ChannelStore::new(db.sqlite.clone()); + let cortex_tool_server = crate::tools::create_cortex_chat_tool_server( memory_search.clone(), conversation_logger, @@ -721,7 +739,7 @@ pub(super) async fn create_agent( agent_config.screenshot_dir(), brave_search_key, runtime_config.workspace_dir.clone(), - runtime_config.instance_dir.clone(), + sandbox.clone(), ); let cortex_store = crate::agent::cortex_chat::CortexChatStore::new(db.sqlite.clone()); let cortex_session = crate::agent::cortex_chat::CortexChatSession::new( @@ -778,6 +796,10 @@ pub(super) async fn create_agent( mcp_managers.insert(agent_id.clone(), mcp_manager); state.mcp_managers.store(std::sync::Arc::new(mcp_managers)); + let mut sandboxes = (**state.sandboxes.load()).clone(); + sandboxes.insert(agent_id.clone(), sandbox); + state.sandboxes.store(std::sync::Arc::new(sandboxes)); + let mut agent_infos = (**state.agent_configs.load()).clone(); agent_infos.push(AgentInfo { id: agent_config.id.clone(), @@ -1007,6 +1029,10 @@ pub(super) async fn delete_agent( configs.remove(&agent_id); state.runtime_configs.store(std::sync::Arc::new(configs)); + let mut sandboxes = (**state.sandboxes.load()).clone(); + sandboxes.remove(&agent_id); + state.sandboxes.store(std::sync::Arc::new(sandboxes)); + let mut agent_infos = (**state.agent_configs.load()).clone(); agent_infos.retain(|a| a.id != agent_id); state.agent_configs.store(std::sync::Arc::new(agent_infos)); diff --git a/src/api/state.rs b/src/api/state.rs index 7f7599c33..03914f079 100644 --- a/src/api/state.rs +++ b/src/api/state.rs @@ -69,6 +69,8 @@ pub struct ApiState { pub runtime_configs: ArcSwap>>, /// Per-agent MCP managers for status and reconnect APIs. pub mcp_managers: ArcSwap>>, + /// Per-agent sandbox instances for process containment. + pub sandboxes: ArcSwap>>, /// Shared reference to the Discord permissions ArcSwap (same instance used by the adapter and file watcher). pub discord_permissions: RwLock>>>, /// Shared reference to the Slack permissions ArcSwap (same instance used by the adapter and file watcher). @@ -221,6 +223,7 @@ impl ApiState { cron_schedulers: arc_swap::ArcSwap::from_pointee(HashMap::new()), runtime_configs: ArcSwap::from_pointee(HashMap::new()), mcp_managers: ArcSwap::from_pointee(HashMap::new()), + sandboxes: ArcSwap::from_pointee(HashMap::new()), discord_permissions: RwLock::new(None), slack_permissions: RwLock::new(None), bindings: RwLock::new(None), @@ -486,6 +489,11 @@ impl ApiState { self.mcp_managers.store(Arc::new(managers)); } + /// Set the sandbox instances for all agents. + pub fn set_sandboxes(&self, sandboxes: HashMap>) { + self.sandboxes.store(Arc::new(sandboxes)); + } + /// Share the Discord permissions ArcSwap with the API so reads get hot-reloaded values. pub async fn set_discord_permissions(&self, permissions: Arc>) { *self.discord_permissions.write().await = Some(permissions); diff --git a/src/config.rs b/src/config.rs index 6792ead12..098c78c23 100644 --- a/src/config.rs +++ b/src/config.rs @@ -764,6 +764,8 @@ pub struct AgentConfig { pub brave_search_key: Option, /// Optional timezone override for cron active-hours evaluation. pub cron_timezone: Option, + /// Sandbox configuration for process containment. + pub sandbox: Option, /// Cron job definitions for this agent. pub cron: Vec, } @@ -810,6 +812,8 @@ pub struct ResolvedAgentConfig { pub mcp: Vec, pub brave_search_key: Option, pub cron_timezone: Option, + /// Sandbox configuration for process containment. + pub sandbox: crate::sandbox::SandboxConfig, /// Number of messages to fetch from the platform when a new channel is created. pub history_backfill_count: usize, pub cron: Vec, @@ -892,6 +896,7 @@ impl AgentConfig { self.cron_timezone.as_deref(), defaults.cron_timezone.as_deref(), ), + sandbox: self.sandbox.clone().unwrap_or_default(), history_backfill_count: defaults.history_backfill_count, cron: self.cron.clone(), } @@ -1859,6 +1864,7 @@ struct TomlAgentConfig { mcp: Option>, brave_search_key: Option, cron_timezone: Option, + sandbox: Option, #[serde(default)] cron: Vec, } @@ -2539,6 +2545,7 @@ impl Config { mcp: None, brave_search_key: None, cron_timezone: None, + sandbox: None, cron: Vec::new(), }]; @@ -3300,6 +3307,7 @@ impl Config { }, brave_search_key: a.brave_search_key.as_deref().and_then(resolve_env_value), cron_timezone: a.cron_timezone.as_deref().and_then(resolve_env_value), + sandbox: a.sandbox, cron, }) }) @@ -3328,6 +3336,7 @@ impl Config { mcp: None, brave_search_key: None, cron_timezone: None, + sandbox: None, cron: Vec::new(), }); } @@ -3622,6 +3631,8 @@ pub struct RuntimeConfig { pub cron_scheduler: ArcSwap>>, /// Settings store for agent-specific configuration. pub settings: ArcSwap>>, + /// Sandbox configuration for process containment. + pub sandbox: ArcSwap, } impl RuntimeConfig { @@ -3672,6 +3683,7 @@ impl RuntimeConfig { cron_store: ArcSwap::from_pointee(None), cron_scheduler: ArcSwap::from_pointee(None), settings: ArcSwap::from_pointee(None), + sandbox: ArcSwap::from_pointee(agent_config.sandbox.clone()), } } @@ -3747,6 +3759,9 @@ impl RuntimeConfig { self.cron_timezone.store(Arc::new(resolved.cron_timezone)); self.cortex.store(Arc::new(resolved.cortex)); self.warmup.store(Arc::new(resolved.warmup)); + // sandbox config is not hot-reloaded here because the Sandbox instance + // is constructed once at startup and shared via Arc. Changing sandbox + // settings requires an agent restart. mcp_manager.reconcile(&old_mcp, &new_mcp).await; diff --git a/src/lib.rs b/src/lib.rs index 8649cebd1..572b84415 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,6 +19,7 @@ pub mod messaging; pub mod openai_auth; pub mod opencode; pub mod prompts; +pub mod sandbox; pub mod secrets; pub mod settings; pub mod skills; @@ -198,6 +199,7 @@ pub struct AgentDeps { pub event_tx: tokio::sync::broadcast::Sender, pub sqlite_pool: sqlx::SqlitePool, pub messaging_manager: Option>, + pub sandbox: Arc, pub links: Arc>>, /// Map of all agent IDs to display names, for inter-agent message routing. pub agent_names: Arc>, diff --git a/src/main.rs b/src/main.rs index f0c6f733a..451f89b93 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1449,6 +1449,16 @@ async fn initialize_agents( mcp_manager.clone(), )); + let sandbox = std::sync::Arc::new( + spacebot::sandbox::Sandbox::new( + &agent_config.sandbox, + agent_config.workspace.clone(), + &config.instance_dir, + agent_config.data_dir.clone(), + ) + .await, + ); + let deps = spacebot::AgentDeps { agent_id: agent_id.clone(), memory_search, @@ -1459,6 +1469,7 @@ async fn initialize_agents( event_tx, sqlite_pool: db.sqlite.clone(), messaging_manager: None, + sandbox, links: agent_links.clone(), agent_names: agent_name_map.clone(), }; @@ -1484,6 +1495,7 @@ async fn initialize_agents( let mut mcp_managers = std::collections::HashMap::new(); let mut agent_workspaces = std::collections::HashMap::new(); let mut runtime_configs = std::collections::HashMap::new(); + let mut sandboxes = std::collections::HashMap::new(); for (agent_id, agent) in agents.iter() { let event_rx = agent.deps.event_tx.subscribe(); api_state.register_agent_events(agent_id.to_string(), event_rx); @@ -1492,6 +1504,7 @@ async fn initialize_agents( mcp_managers.insert(agent_id.to_string(), agent.deps.mcp_manager.clone()); agent_workspaces.insert(agent_id.to_string(), agent.config.workspace.clone()); runtime_configs.insert(agent_id.to_string(), agent.deps.runtime_config.clone()); + sandboxes.insert(agent_id.to_string(), agent.deps.sandbox.clone()); agent_configs.push(spacebot::api::AgentInfo { id: agent.config.id.clone(), display_name: agent.config.display_name.clone(), @@ -1509,6 +1522,7 @@ async fn initialize_agents( api_state.set_mcp_managers(mcp_managers); api_state.set_runtime_configs(runtime_configs); api_state.set_agent_workspaces(agent_workspaces); + api_state.set_sandboxes(sandboxes); api_state.set_instance_dir(config.instance_dir.clone()); } @@ -1767,7 +1781,7 @@ async fn initialize_agents( agent.config.screenshot_dir(), brave_search_key, agent.deps.runtime_config.workspace_dir.clone(), - agent.deps.runtime_config.instance_dir.clone(), + agent.deps.sandbox.clone(), ); let store = spacebot::agent::cortex_chat::CortexChatStore::new(agent.db.sqlite.clone()); let session = spacebot::agent::cortex_chat::CortexChatSession::new( diff --git a/src/sandbox.rs b/src/sandbox.rs new file mode 100644 index 000000000..1a8953325 --- /dev/null +++ b/src/sandbox.rs @@ -0,0 +1,386 @@ +//! OS-level filesystem containment for shell and exec tool subprocesses. +//! +//! Replaces string-based command filtering with kernel-enforced boundaries. +//! On Linux, uses bubblewrap (bwrap) for mount namespace isolation. +//! On macOS, uses sandbox-exec with a generated SBPL profile. +//! Falls back to no sandboxing when neither backend is available. + +use serde::{Deserialize, Serialize}; +use std::path::{Path, PathBuf}; +use tokio::process::Command; + +/// Sandbox configuration from the agent config file. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SandboxConfig { + #[serde(default = "default_mode")] + pub mode: SandboxMode, + #[serde(default)] + pub writable_paths: Vec, +} + +impl Default for SandboxConfig { + fn default() -> Self { + Self { + mode: SandboxMode::Enabled, + writable_paths: Vec::new(), + } + } +} + +fn default_mode() -> SandboxMode { + SandboxMode::Enabled +} + +/// Sandbox enforcement mode. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum SandboxMode { + /// OS-level containment (default). + Enabled, + /// No containment, full host access. + Disabled, +} + +/// Detected sandbox backend. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SandboxBackend { + /// Linux: bubblewrap available. + Bubblewrap { proc_supported: bool }, + /// macOS: /usr/bin/sandbox-exec available. + SandboxExec, + /// No sandbox support detected, or mode = Disabled. + None, +} + +/// Filesystem sandbox for subprocess execution. +/// +/// Created once per agent at startup, shared via `Arc` across all workers. +/// Wraps `tokio::process::Command` to apply OS-level containment before spawning. +#[derive(Debug, Clone)] +pub struct Sandbox { + #[allow(dead_code)] + mode: SandboxMode, + workspace: PathBuf, + data_dir: PathBuf, + tools_bin: PathBuf, + writable_paths: Vec, + backend: SandboxBackend, +} + +impl Sandbox { + /// Create a sandbox with the given configuration. Probes for backend support. + pub async fn new( + config: &SandboxConfig, + workspace: PathBuf, + instance_dir: &Path, + data_dir: PathBuf, + ) -> Self { + let tools_bin = instance_dir.join("tools/bin"); + let backend = if config.mode == SandboxMode::Disabled { + SandboxBackend::None + } else { + detect_backend().await + }; + + match backend { + SandboxBackend::Bubblewrap { proc_supported } => { + tracing::info!(proc_supported, "sandbox enabled: bubblewrap backend"); + } + SandboxBackend::SandboxExec => { + tracing::info!("sandbox enabled: macOS sandbox-exec backend"); + } + SandboxBackend::None if config.mode == SandboxMode::Enabled => { + tracing::warn!( + "sandbox mode is enabled but no backend available — \ + processes will run unsandboxed" + ); + } + SandboxBackend::None => { + tracing::info!("sandbox disabled by config"); + } + } + + // Canonicalize paths at construction to resolve symlinks and validate existence. + let workspace = canonicalize_or_self(&workspace); + let data_dir = canonicalize_or_self(&data_dir); + let writable_paths: Vec = config + .writable_paths + .iter() + .filter_map(|path| match path.canonicalize() { + Ok(canonical) => Some(canonical), + Err(error) => { + tracing::warn!( + path = %path.display(), + %error, + "dropping writable_path entry (does not exist or is unresolvable)" + ); + None + } + }) + .collect(); + + Self { + mode: config.mode, + workspace, + data_dir, + tools_bin, + writable_paths, + backend, + } + } + + /// Wrap a command for sandboxed execution. + /// + /// Returns a `Command` ready to spawn, potentially prefixed with bwrap or + /// sandbox-exec depending on the detected backend. The caller still needs + /// to set stdout/stderr/timeout after this call. + pub fn wrap(&self, program: &str, args: &[&str], working_dir: &Path) -> Command { + // Prepend tools/bin to PATH for all commands + let path_env = match std::env::var_os("PATH") { + Some(current) => { + let mut paths = std::env::split_paths(¤t).collect::>(); + paths.insert(0, self.tools_bin.clone()); + std::env::join_paths(paths) + .unwrap_or(current) + .to_string_lossy() + .into_owned() + } + None => self.tools_bin.to_string_lossy().into_owned(), + }; + + match self.backend { + SandboxBackend::Bubblewrap { proc_supported } => { + self.wrap_bubblewrap(program, args, working_dir, proc_supported, &path_env) + } + SandboxBackend::SandboxExec => { + self.wrap_sandbox_exec(program, args, working_dir, &path_env) + } + SandboxBackend::None => self.wrap_passthrough(program, args, working_dir, &path_env), + } + } + + /// Linux: wrap with bubblewrap mount namespace. + fn wrap_bubblewrap( + &self, + program: &str, + args: &[&str], + working_dir: &Path, + proc_supported: bool, + path_env: &str, + ) -> Command { + let mut cmd = Command::new("bwrap"); + + // Mount order matters — later mounts override earlier ones. + // 1. Entire filesystem read-only + cmd.arg("--ro-bind").arg("/").arg("/"); + + // 2. Writable /dev with standard nodes + cmd.arg("--dev").arg("/dev"); + + // 3. Fresh /proc (if supported by the environment) + if proc_supported { + cmd.arg("--proc").arg("/proc"); + } + + // 4. Private /tmp per invocation + cmd.arg("--tmpfs").arg("/tmp"); + + // 5. Workspace writable + cmd.arg("--bind").arg(&self.workspace).arg(&self.workspace); + + // 6. Each configured writable path + for writable in &self.writable_paths { + cmd.arg("--bind").arg(writable).arg(writable); + } + + // 7. Re-protect agent data dir (may overlap with workspace parent) + cmd.arg("--ro-bind").arg(&self.data_dir).arg(&self.data_dir); + + // 8. Isolation flags + cmd.arg("--unshare-pid"); + cmd.arg("--new-session"); + cmd.arg("--die-with-parent"); + + // 9. Working directory + cmd.arg("--chdir").arg(working_dir); + + // 10. Set PATH inside the sandbox + cmd.arg("--setenv").arg("PATH").arg(path_env); + + // 11. The actual command + cmd.arg("--").arg(program); + for arg in args { + cmd.arg(arg); + } + + cmd + } + + /// macOS: wrap with sandbox-exec and a generated SBPL profile. + fn wrap_sandbox_exec( + &self, + program: &str, + args: &[&str], + working_dir: &Path, + path_env: &str, + ) -> Command { + let profile = self.generate_sbpl_profile(); + + let mut cmd = Command::new("/usr/bin/sandbox-exec"); + cmd.arg("-p").arg(profile); + cmd.arg(program); + for arg in args { + cmd.arg(arg); + } + cmd.current_dir(working_dir); + cmd.env("PATH", path_env); + + cmd + } + + /// No backend: pass through unchanged. + fn wrap_passthrough( + &self, + program: &str, + args: &[&str], + working_dir: &Path, + path_env: &str, + ) -> Command { + let mut cmd = Command::new(program); + for arg in args { + cmd.arg(arg); + } + cmd.current_dir(working_dir); + cmd.env("PATH", path_env); + + cmd + } + + /// Generate a macOS SBPL (Sandbox Profile Language) policy. + /// + /// Paths are canonicalized because /var on macOS is actually /private/var. + fn generate_sbpl_profile(&self) -> String { + let workspace = canonicalize_or_self(&self.workspace); + + let mut profile = String::from( + r#"(version 1) +(deny default) + +; process basics +(allow process-exec) +(allow process-fork) +(allow signal (target same-sandbox)) +(allow process-info* (target same-sandbox)) + +; filesystem: read everything +(allow file-read*) + +"#, + ); + + // Workspace writable + profile.push_str(&format!( + "; workspace writable\n(allow file-write* (subpath \"{}\"))\n\n", + workspace.display() + )); + + // Additional writable paths + for (index, writable) in self.writable_paths.iter().enumerate() { + let canonical = canonicalize_or_self(writable); + profile.push_str(&format!( + "; writable path {index}\n(allow file-write* (subpath \"{}\"))\n", + canonical.display() + )); + } + + // Protect data_dir even if it falls under the workspace subtree + let data_dir = canonicalize_or_self(&self.data_dir); + profile.push_str(&format!( + "\n; data dir read-only\n(deny file-write* (subpath \"{}\"))\n", + data_dir.display() + )); + + // /tmp writable + let tmp = canonicalize_or_self(Path::new("/tmp")); + profile.push_str(&format!( + "\n; tmp writable\n(allow file-write* (subpath \"{}\"))\n", + tmp.display() + )); + + profile.push_str( + r#" +; dev, sysctl, mach for basic operation +(allow file-write-data + (require-all (path "/dev/null") (vnode-type CHARACTER-DEVICE))) +(allow sysctl-read) +(allow mach-lookup + (global-name "com.apple.system.opendirectoryd.libinfo")) +(allow ipc-posix-sem) +(allow pseudo-tty) +(allow network*) +"#, + ); + + profile + } +} + +/// Canonicalize a path, falling back to the original if canonicalization fails. +fn canonicalize_or_self(path: &Path) -> PathBuf { + path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) +} + +/// Detect the best available sandbox backend for the current platform. +async fn detect_backend() -> SandboxBackend { + if cfg!(target_os = "linux") { + detect_bubblewrap().await + } else if cfg!(target_os = "macos") { + detect_sandbox_exec() + } else { + tracing::warn!("no sandbox backend available for this platform"); + SandboxBackend::None + } +} + +/// Linux: check if bwrap is available and whether --proc /proc works. +async fn detect_bubblewrap() -> SandboxBackend { + // Check if bwrap exists + let version_check = Command::new("bwrap").arg("--version").output().await; + + if version_check.is_err() { + tracing::debug!("bwrap not found in PATH"); + return SandboxBackend::None; + } + + // Preflight: test if --proc /proc works (may fail in nested containers) + let proc_check = Command::new("bwrap") + .args([ + "--ro-bind", + "/", + "/", + "--proc", + "/proc", + "--", + "/usr/bin/true", + ]) + .output() + .await; + + let proc_supported = proc_check.is_ok_and(|output| output.status.success()); + + if !proc_supported { + tracing::debug!("bwrap --proc /proc not supported, running without fresh procfs"); + } + + SandboxBackend::Bubblewrap { proc_supported } +} + +/// macOS: check if sandbox-exec exists at its known path. +fn detect_sandbox_exec() -> SandboxBackend { + if Path::new("/usr/bin/sandbox-exec").exists() { + SandboxBackend::SandboxExec + } else { + tracing::debug!("/usr/bin/sandbox-exec not found"); + SandboxBackend::None + } +} diff --git a/src/tools.rs b/src/tools.rs index 66f53234f..5dab9da1a 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -92,6 +92,7 @@ pub use web_search::{SearchResult, WebSearchArgs, WebSearchError, WebSearchOutpu use crate::agent::channel::ChannelState; use crate::config::{BrowserConfig, RuntimeConfig}; use crate::memory::MemorySearch; +use crate::sandbox::Sandbox; use crate::{AgentId, ChannelId, OutboundResponse, ProcessEvent, WorkerId}; use rig::tool::Tool as _; use rig::tool::server::{ToolServer, ToolServerHandle}; @@ -256,12 +257,15 @@ pub async fn add_channel_tools( )) .await?; } - handle.add_tool(CancelTool::new(state)).await?; handle - .add_tool(SkipTool::new(skip_flag.clone(), response_tx.clone())) + .add_tool(SendFileTool::new( + response_tx.clone(), + state.deps.runtime_config.workspace_dir.clone(), + )) .await?; + handle.add_tool(CancelTool::new(state)).await?; handle - .add_tool(SendFileTool::new(response_tx.clone())) + .add_tool(SkipTool::new(skip_flag.clone(), response_tx.clone())) .await?; handle.add_tool(ReactTool::new(response_tx.clone())).await?; if let Some(cron) = cron_tool { @@ -349,8 +353,8 @@ pub fn create_branch_tool_server( /// the specific worker's ID so status updates route correctly. The browser tool /// is included when browser automation is enabled in the agent config. /// -/// File operations are restricted to `workspace`. Shell and exec commands are -/// blocked from accessing sensitive files in `instance_dir`. +/// Shell and exec commands are sandboxed via the `Sandbox` backend. +/// File operations are restricted to `workspace` via path validation. #[allow(clippy::too_many_arguments)] pub fn create_worker_tool_server( agent_id: AgentId, @@ -361,14 +365,14 @@ pub fn create_worker_tool_server( screenshot_dir: PathBuf, brave_search_key: Option, workspace: PathBuf, - instance_dir: PathBuf, + sandbox: Arc, mcp_tools: Vec, runtime_config: Arc, ) -> ToolServerHandle { let mut server = ToolServer::new() - .tool(ShellTool::new(instance_dir.clone(), workspace.clone())) + .tool(ShellTool::new(workspace.clone(), sandbox.clone())) .tool(FileTool::new(workspace.clone())) - .tool(ExecTool::new(instance_dir, workspace)) + .tool(ExecTool::new(workspace, sandbox)) .tool(SetStatusTool::new( agent_id, worker_id, channel_id, event_tx, )) @@ -413,16 +417,16 @@ pub fn create_cortex_chat_tool_server( screenshot_dir: PathBuf, brave_search_key: Option, workspace: PathBuf, - instance_dir: PathBuf, + sandbox: Arc, ) -> ToolServerHandle { let mut server = ToolServer::new() .tool(MemorySaveTool::new(memory_search.clone())) .tool(MemoryRecallTool::new(memory_search.clone())) .tool(MemoryDeleteTool::new(memory_search)) .tool(ChannelRecallTool::new(conversation_logger, channel_store)) - .tool(ShellTool::new(instance_dir.clone(), workspace.clone())) + .tool(ShellTool::new(workspace.clone(), sandbox.clone())) .tool(FileTool::new(workspace.clone())) - .tool(ExecTool::new(instance_dir, workspace)); + .tool(ExecTool::new(workspace, sandbox)); if browser_config.enabled { server = server.tool(BrowserTool::new(browser_config, screenshot_dir)); diff --git a/src/tools/exec.rs b/src/tools/exec.rs index 1c5c9e42c..8cbd07ddb 100644 --- a/src/tools/exec.rs +++ b/src/tools/exec.rs @@ -1,61 +1,26 @@ //! Exec tool for running subprocesses (task workers only). +use crate::sandbox::Sandbox; use rig::completion::ToolDefinition; use rig::tool::Tool; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::path::PathBuf; use std::process::Stdio; +use std::sync::Arc; use tokio::process::Command; -/// Tool for executing subprocesses, with path restrictions to prevent -/// access to instance-level configuration and secrets. +/// Tool for executing subprocesses within a sandboxed environment. #[derive(Debug, Clone)] pub struct ExecTool { - instance_dir: PathBuf, workspace: PathBuf, + sandbox: Arc, } impl ExecTool { - /// Create a new exec tool with the given instance directory for path blocking. - pub fn new(instance_dir: PathBuf, workspace: PathBuf) -> Self { - Self { - instance_dir, - workspace, - } - } - - /// Check if program arguments reference sensitive instance paths. - fn check_args(&self, program: &str, args: &[String]) -> Result<(), ExecError> { - let instance_str = self.instance_dir.to_string_lossy(); - let workspace_str = self.workspace.to_string_lossy(); - let all_args = std::iter::once(program.to_string()) - .chain(args.iter().cloned()) - .collect::>() - .join(" "); - - // Block references to instance dir (unless targeting workspace within it) - if all_args.contains(instance_str.as_ref()) && !all_args.contains(workspace_str.as_ref()) { - return Err(ExecError { - message: "Cannot access the instance directory — it contains protected configuration and data.".to_string(), - exit_code: -1, - }); - } - - // Block references to sensitive files by name - for file in super::shell::SENSITIVE_FILES { - if all_args.contains(file) - && (all_args.contains(instance_str.as_ref()) - || !all_args.contains(workspace_str.as_ref())) - { - return Err(ExecError { - message: format!("Cannot access {file} — instance configuration is protected."), - exit_code: -1, - }); - } - } - - Ok(()) + /// Create a new exec tool with sandbox containment. + pub fn new(workspace: PathBuf, sandbox: Arc) -> Self { + Self { workspace, sandbox } } } @@ -116,6 +81,22 @@ pub struct ExecOutput { pub summary: String, } +/// Env vars that enable library injection or alter runtime loading behavior. +const DANGEROUS_ENV_VARS: &[&str] = &[ + "LD_PRELOAD", + "LD_LIBRARY_PATH", + "DYLD_INSERT_LIBRARIES", + "DYLD_LIBRARY_PATH", + "PYTHONPATH", + "PYTHONSTARTUP", + "NODE_OPTIONS", + "RUBYOPT", + "PERL5OPT", + "PERL5LIB", + "BASH_ENV", + "ENV", +]; + impl Tool for ExecTool { const NAME: &'static str = "exec"; @@ -178,11 +159,8 @@ impl Tool for ExecTool { } async fn call(&self, args: Self::Args) -> Result { - // Check for references to sensitive instance paths - self.check_args(&args.program, &args.args)?; - // Validate working_dir stays within workspace if specified - if let Some(ref dir) = args.working_dir { + let working_dir = if let Some(ref dir) = args.working_dir { let path = std::path::Path::new(dir); let canonical = path.canonicalize().unwrap_or_else(|_| path.to_path_buf()); let workspace_canonical = self @@ -198,36 +176,14 @@ impl Tool for ExecTool { exit_code: -1, }); } - } - - // Block passing secret env var values directly - for env_var in &args.env { - for secret in super::shell::SECRET_ENV_VARS { - if env_var.key == *secret { - return Err(ExecError { - message: "Cannot set secret environment variables.".to_string(), - exit_code: -1, - }); - } - } - } + canonical + } else { + self.workspace.clone() + }; // Block env vars that enable library injection or alter runtime - // loading behavior — these allow arbitrary code execution. - const DANGEROUS_ENV_VARS: &[&str] = &[ - "LD_PRELOAD", - "LD_LIBRARY_PATH", - "DYLD_INSERT_LIBRARIES", - "DYLD_LIBRARY_PATH", - "PYTHONPATH", - "PYTHONSTARTUP", - "NODE_OPTIONS", - "RUBYOPT", - "PERL5OPT", - "PERL5LIB", - "BASH_ENV", - "ENV", - ]; + // loading behavior — these allow arbitrary code execution regardless + // of filesystem sandbox state. for env_var in &args.env { if DANGEROUS_ENV_VARS .iter() @@ -243,23 +199,10 @@ impl Tool for ExecTool { } } - let mut cmd = Command::new(&args.program); - cmd.args(&args.args); - - // Default to workspace as working directory - if let Some(dir) = args.working_dir { - cmd.current_dir(dir); - } else { - cmd.current_dir(&self.workspace); - } - - // Prepend persistent tools directory to PATH so user-installed - // binaries survive container restarts. - let tools_bin = self.instance_dir.join("tools/bin"); - if let Ok(current_path) = std::env::var("PATH") { - cmd.env("PATH", format!("{}:{current_path}", tools_bin.display())); - } + let arg_refs: Vec<&str> = args.args.iter().map(|s| s.as_str()).collect(); + let mut cmd = self.sandbox.wrap(&args.program, &arg_refs, &working_dir); + // Apply user-specified env vars after sandbox wrapping for env_var in args.env { cmd.env(env_var.key, env_var.value); } diff --git a/src/tools/send_file.rs b/src/tools/send_file.rs index cfe556f4b..02437e829 100644 --- a/src/tools/send_file.rs +++ b/src/tools/send_file.rs @@ -13,14 +13,41 @@ use tokio::sync::mpsc; /// Reads a file from the local filesystem and sends it as an attachment /// in the conversation. The channel process creates a response sender per /// conversation turn and this tool routes file responses through it. +/// File access is restricted to the agent's workspace boundary. #[derive(Debug, Clone)] pub struct SendFileTool { response_tx: mpsc::Sender, + workspace: PathBuf, } impl SendFileTool { - pub fn new(response_tx: mpsc::Sender) -> Self { - Self { response_tx } + pub fn new(response_tx: mpsc::Sender, workspace: PathBuf) -> Self { + Self { + response_tx, + workspace, + } + } + + /// Validate that a path falls within the workspace boundary. + fn validate_workspace_path(&self, path: &std::path::Path) -> Result { + let workspace = &self.workspace; + + let canonical = path.canonicalize().map_err(|error| { + SendFileError(format!("can't resolve path '{}': {error}", path.display())) + })?; + let workspace_canonical = workspace + .canonicalize() + .unwrap_or_else(|_| workspace.clone()); + + if !canonical.starts_with(&workspace_canonical) { + return Err(SendFileError(format!( + "ACCESS DENIED: Path is outside the workspace boundary. \ + File operations are restricted to {}.", + workspace.display() + ))); + } + + Ok(canonical) } } @@ -79,12 +106,14 @@ impl Tool for SendFileTool { } async fn call(&self, args: Self::Args) -> Result { - let path = PathBuf::from(&args.file_path); + let raw_path = PathBuf::from(&args.file_path); - if !path.is_absolute() { + if !raw_path.is_absolute() { return Err(SendFileError("file_path must be an absolute path".into())); } + let path = self.validate_workspace_path(&raw_path)?; + let metadata = tokio::fs::metadata(&path).await.map_err(|error| { SendFileError(format!("can't read file '{}': {error}", path.display())) })?; diff --git a/src/tools/shell.rs b/src/tools/shell.rs index 7f4d57aed..60f161228 100644 --- a/src/tools/shell.rs +++ b/src/tools/shell.rs @@ -1,234 +1,26 @@ //! Shell tool for executing shell commands (task workers only). +use crate::sandbox::Sandbox; use rig::completion::ToolDefinition; use rig::tool::Tool; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::path::PathBuf; use std::process::Stdio; +use std::sync::Arc; use tokio::process::Command; -/// Sensitive filenames that should not be accessible via shell commands. -pub const SENSITIVE_FILES: &[&str] = &[ - "config.toml", - "config.redb", - "settings.redb", - ".env", - "spacebot.db", -]; - -/// Environment variable names that contain secrets. -pub const SECRET_ENV_VARS: &[&str] = &[ - "ANTHROPIC_API_KEY", - "OPENAI_API_KEY", - "OPENROUTER_API_KEY", - "DISCORD_BOT_TOKEN", - "SLACK_BOT_TOKEN", - "SLACK_APP_TOKEN", - "TELEGRAM_BOT_TOKEN", - "BRAVE_SEARCH_API_KEY", -]; - -/// Tool for executing shell commands, with path restrictions to prevent -/// access to instance-level configuration and secrets. +/// Tool for executing shell commands within a sandboxed environment. #[derive(Debug, Clone)] pub struct ShellTool { - instance_dir: PathBuf, workspace: PathBuf, + sandbox: Arc, } impl ShellTool { - /// Create a new shell tool with the given instance directory for path blocking. - pub fn new(instance_dir: PathBuf, workspace: PathBuf) -> Self { - Self { - instance_dir, - workspace, - } - } - - /// Check if a command references sensitive instance paths or secret env vars. - fn check_command(&self, command: &str) -> Result<(), ShellError> { - let instance_str = self.instance_dir.to_string_lossy(); - - // Block any command that directly references the instance directory. - // This prevents listing, reading, traversing, or archiving the instance - // dir and its contents (config.toml, databases, agent data, etc). - // Commands that reference the workspace (which is inside the instance dir) - // are allowed through. - if command.contains(instance_str.as_ref()) { - let workspace_str = self.workspace.to_string_lossy(); - if !command.contains(workspace_str.as_ref()) { - return Err(ShellError { - message: "ACCESS DENIED: Cannot access the instance directory — it contains \ - protected configuration and data. Do not attempt to reproduce or \ - guess its contents. Inform the user that this path is restricted." - .to_string(), - exit_code: -1, - }); - } - } - - // Block commands referencing sensitive files even without the full instance path - // (e.g. "cat config.toml" from a relative path, or via variable expansion) - for file in SENSITIVE_FILES { - if command.contains(file) { - let workspace_str = self.workspace.to_string_lossy(); - let mentions_workspace = command.contains(workspace_str.as_ref()); - let mentions_instance = command.contains(instance_str.as_ref()); - - // Block if referencing instance dir, or if not clearly targeting workspace - if mentions_instance || !mentions_workspace { - return Err(ShellError { - message: format!( - "ACCESS DENIED: Cannot access {file} — instance configuration is protected. \ - Do not attempt to reproduce or guess the file contents." - ), - exit_code: -1, - }); - } - } - } - - // Block access to secret environment variables via any expansion syntax - for var in SECRET_ENV_VARS { - if command.contains(&format!("${var}")) - || command.contains(&format!("${{{var}}}")) - || command.contains(&format!("printenv {var}")) - { - return Err(ShellError { - message: "Cannot access secret environment variables.".to_string(), - exit_code: -1, - }); - } - // Block unbraced $VAR at word boundaries (covers `echo $SECRET` patterns) - let dollar_var = format!("${var}"); - if let Some(pos) = command.find(&dollar_var) { - let after = pos + dollar_var.len(); - let next_char = command[after..].chars().next(); - // $VAR is a match if followed by non-alphanumeric/underscore or end-of-string - if next_char.is_none() - || (!next_char.unwrap().is_alphanumeric() && next_char.unwrap() != '_') - { - return Err(ShellError { - message: "Cannot access secret environment variables.".to_string(), - exit_code: -1, - }); - } - } - } - - // Block broad env dumps that would expose secrets - if command.contains("printenv") { - let trimmed = command.trim(); - if trimmed == "printenv" - || trimmed.ends_with("| printenv") - || trimmed.contains("printenv |") - || trimmed.contains("printenv >") - { - return Err(ShellError { - message: "Cannot dump all environment variables — they may contain secrets." - .to_string(), - exit_code: -1, - }); - } - } - if command.contains("env") { - let trimmed = command.trim(); - if trimmed == "env" || trimmed.starts_with("env |") || trimmed.starts_with("env >") { - return Err(ShellError { - message: "Cannot dump all environment variables — they may contain secrets." - .to_string(), - exit_code: -1, - }); - } - } - - // Block shell builtins that dump all variables - for dump_cmd in ["set", "declare -p", "export -p", "compgen -e", "compgen -v"] { - let trimmed = command.trim(); - if trimmed == dump_cmd - || trimmed.starts_with(&format!("{dump_cmd} ")) - || trimmed.starts_with(&format!("{dump_cmd}|")) - || trimmed.starts_with(&format!("{dump_cmd}>")) - || trimmed.contains(&format!("| {dump_cmd}")) - || trimmed.contains(&format!("; {dump_cmd}")) - || trimmed.contains(&format!("&& {dump_cmd}")) - { - return Err(ShellError { - message: "Cannot dump environment variables — they may contain secrets." - .to_string(), - exit_code: -1, - }); - } - } - - // Block subshell/command substitution that could bypass string-level checks. - // Backtick and $() let an attacker compose a blocked command dynamically. - if command.contains('`') - || command.contains("$(") - || command.contains("<(") - || command.contains(">(") - { - return Err(ShellError { - message: "Subshell and command substitution (`...`, $(...), <(...), >(...)) \ - are not allowed." - .to_string(), - exit_code: -1, - }); - } - - // Block eval/exec which can dynamically construct any command - if contains_shell_builtin(command, "eval") || contains_shell_builtin(command, "exec") { - return Err(ShellError { - message: "eval and exec are not allowed.".to_string(), - exit_code: -1, - }); - } - - // Block interpreter one-liners that can bypass shell-level restrictions - for interpreter in [ - "python3 -c", - "python -c", - "perl -e", - "ruby -e", - "node -e", - "node --eval", - ] { - if command.contains(interpreter) { - return Err(ShellError { - message: - "Inline interpreter execution is not permitted — use script files instead." - .to_string(), - exit_code: -1, - }); - } - } - - // Block /proc entries and /dev paths that expose environment or fd contents - if command.contains("/proc/self/environ") - || command.contains("/proc/*/environ") - || command.contains("/dev/fd/") - || command.contains("/dev/stdin") - { - return Err(ShellError { - message: "Cannot access process environment — it may contain secrets.".to_string(), - exit_code: -1, - }); - } - - // Block additional commands that dump environment state - if contains_shell_builtin(command, "set") - || command.contains("declare -p") - || contains_shell_builtin(command, "compgen") - || contains_shell_builtin(command, "export") - { - return Err(ShellError { - message: "Cannot dump shell state — it may contain secrets.".to_string(), - exit_code: -1, - }); - } - - Ok(()) + /// Create a new shell tool with sandbox containment. + pub fn new(workspace: PathBuf, sandbox: Arc) -> Self { + Self { workspace, sandbox } } } @@ -310,11 +102,8 @@ impl Tool for ShellTool { } async fn call(&self, args: Self::Args) -> Result { - // Check for commands targeting sensitive paths or env vars - self.check_command(&args.command)?; - // Validate working_dir stays within workspace if specified - if let Some(ref dir) = args.working_dir { + let working_dir = if let Some(ref dir) = args.working_dir { let path = std::path::Path::new(dir); let canonical = path.canonicalize().unwrap_or_else(|_| path.to_path_buf()); let workspace_canonical = self @@ -330,35 +119,23 @@ impl Tool for ShellTool { exit_code: -1, }); } - } + canonical + } else { + self.workspace.clone() + }; let mut cmd = if cfg!(target_os = "windows") { let mut c = Command::new("cmd"); c.arg("/C").arg(&args.command); + c.current_dir(&working_dir); c } else { - let mut c = Command::new("sh"); - c.arg("-c").arg(&args.command); - c + self.sandbox + .wrap("sh", &["-c", &args.command], &working_dir) }; - // Default to workspace as working directory - if let Some(dir) = args.working_dir { - cmd.current_dir(dir); - } else { - cmd.current_dir(&self.workspace); - } - - // Prepend persistent tools directory to PATH so user-installed - // binaries survive container restarts. - let tools_bin = self.instance_dir.join("tools/bin"); - if let Ok(current_path) = std::env::var("PATH") { - cmd.env("PATH", format!("{}:{current_path}", tools_bin.display())); - } - cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); - // Set timeout let timeout = tokio::time::Duration::from_secs(args.timeout_seconds); let output = tokio::time::timeout(timeout, cmd.output()) @@ -418,21 +195,6 @@ fn format_shell_output(exit_code: i32, stdout: &str, stderr: &str) -> String { output } -/// Check if a shell builtin appears as a standalone command -/// (not as a substring of another word). -fn contains_shell_builtin(command: &str, builtin: &str) -> bool { - for segment in command.split(['|', ';', '&']) { - let trimmed = segment.trim(); - if trimmed == builtin - || trimmed.starts_with(&format!("{builtin} ")) - || trimmed.starts_with(&format!("{builtin}\t")) - { - return true; - } - } - false -} - /// System-internal shell execution that bypasses path restrictions. /// Used by the system itself, not LLM-facing. pub async fn shell( diff --git a/tests/bulletin.rs b/tests/bulletin.rs index 81cf4b8bf..aaddb5eba 100644 --- a/tests/bulletin.rs +++ b/tests/bulletin.rs @@ -70,6 +70,16 @@ async fn bootstrap_deps() -> anyhow::Result { let agent_id: spacebot::AgentId = Arc::from(agent_config.id.as_str()); let mcp_manager = Arc::new(spacebot::mcp::McpManager::new(agent_config.mcp.clone())); + let sandbox = Arc::new( + spacebot::sandbox::Sandbox::new( + &agent_config.sandbox, + agent_config.workspace.clone(), + &config.instance_dir, + agent_config.data_dir.clone(), + ) + .await, + ); + Ok(spacebot::AgentDeps { agent_id, memory_search, @@ -80,6 +90,7 @@ async fn bootstrap_deps() -> anyhow::Result { event_tx, sqlite_pool: db.sqlite.clone(), messaging_manager: None, + sandbox, links: Arc::new(arc_swap::ArcSwap::from_pointee(Vec::new())), agent_names: Arc::new(std::collections::HashMap::new()), }) diff --git a/tests/context_dump.rs b/tests/context_dump.rs index 71812e49c..30780f261 100644 --- a/tests/context_dump.rs +++ b/tests/context_dump.rs @@ -70,6 +70,16 @@ async fn bootstrap_deps() -> anyhow::Result<(spacebot::AgentDeps, spacebot::conf let agent_id: spacebot::AgentId = Arc::from(agent_config.id.as_str()); let mcp_manager = Arc::new(spacebot::mcp::McpManager::new(agent_config.mcp.clone())); + let sandbox = Arc::new( + spacebot::sandbox::Sandbox::new( + &agent_config.sandbox, + agent_config.workspace.clone(), + &config.instance_dir, + agent_config.data_dir.clone(), + ) + .await, + ); + let deps = spacebot::AgentDeps { agent_id, memory_search, @@ -80,6 +90,7 @@ async fn bootstrap_deps() -> anyhow::Result<(spacebot::AgentDeps, spacebot::conf event_tx, sqlite_pool: db.sqlite.clone(), messaging_manager: None, + sandbox, links: Arc::new(arc_swap::ArcSwap::from_pointee(Vec::new())), agent_names: Arc::new(std::collections::HashMap::new()), }; @@ -326,7 +337,7 @@ async fn dump_worker_context() { std::path::PathBuf::from("/tmp/screenshots"), brave_search_key, std::path::PathBuf::from("/tmp"), - std::path::PathBuf::from("/tmp"), + deps.sandbox.clone(), vec![], deps.runtime_config.clone(), ); @@ -481,7 +492,7 @@ async fn dump_all_contexts() { std::path::PathBuf::from("/tmp/screenshots"), brave_search_key, std::path::PathBuf::from("/tmp"), - std::path::PathBuf::from("/tmp"), + deps.sandbox.clone(), vec![], deps.runtime_config.clone(), );