From cc58b66a0d8477b2fefd5cff930adfa334845977 Mon Sep 17 00:00:00 2001 From: Clifford Ressel Date: Fri, 2 Jan 2026 17:04:22 -0500 Subject: [PATCH 1/2] feat(acp): Add OS-level sandboxing for ACP agent subprocesses Implement platform-specific sandbox wrapping for ACP agent subprocesses: - macOS: Seatbelt via /usr/bin/sandbox-exec (reusing codex-core) - Linux: Landlock + seccomp via codex-linux-sandbox binary - Windows: Deferred (gracefully degrades to unsandboxed) This restricts what agent processes can access based on SandboxPolicy, providing OS-level security for Claude Code, Codex, and Gemini CLI agents. Key changes: - New sandbox.rs module with transform_command_for_sandbox() - AcpConnection::spawn() now accepts sandbox_policy parameter - Graceful degradation when Linux sandbox binary not provided - Defense-in-depth: application-level path restrictions retained --- codex-rs/Cargo.lock | 1 + codex-rs/acp/ACP_SANDBOX_IMPLEMENTATION.md | 305 +++++++++++++++++ codex-rs/acp/Cargo.toml | 3 + codex-rs/acp/docs.md | 52 ++- codex-rs/acp/src/backend.rs | 6 +- codex-rs/acp/src/connection.rs | 88 ++++- codex-rs/acp/src/lib.rs | 8 + codex-rs/acp/src/sandbox.rs | 370 +++++++++++++++++++++ codex-rs/core/docs.md | 2 + codex-rs/core/src/seatbelt.rs | 4 +- 10 files changed, 817 insertions(+), 22 deletions(-) create mode 100644 codex-rs/acp/ACP_SANDBOX_IMPLEMENTATION.md create mode 100644 codex-rs/acp/src/sandbox.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 7bd9cbefc..1af7728a9 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -872,6 +872,7 @@ dependencies = [ "agent-client-protocol", "anyhow", "async-trait", + "codex-core", "codex-protocol", "diffy", "dirs", diff --git a/codex-rs/acp/ACP_SANDBOX_IMPLEMENTATION.md b/codex-rs/acp/ACP_SANDBOX_IMPLEMENTATION.md new file mode 100644 index 000000000..bd25b7113 --- /dev/null +++ b/codex-rs/acp/ACP_SANDBOX_IMPLEMENTATION.md @@ -0,0 +1,305 @@ +# ACP Subprocess Sandboxing Implementation Plan + +This document describes the implementation plan for wrapping ACP agent subprocesses with OS-level sandboxing (Seatbelt on macOS, Landlock+seccomp on Linux, restricted tokens on Windows). + +## Background + +The ACP module spawns agent subprocesses (Claude Code CLI, Codex CLI, Gemini CLI) without OS-level sandboxing. Currently, there is only a temporary application-level path restriction in `write_text_file()` that checks paths are within the working directory or `/tmp`. + +The codex-core module has fully implemented platform-specific sandboxing that should be reused: +- **macOS**: Seatbelt via `/usr/bin/sandbox-exec` +- **Linux**: Landlock LSM + seccomp filters via `codex-linux-sandbox` binary +- **Windows**: Restricted tokens + ACL manipulation + +## Architecture + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ SandboxPolicy │ +│ (codex-protocol/src/protocol.rs) │ +│ ┌──────────────────┬──────────────────┬─────────────────────┐ │ +│ │ DangerFullAccess │ ReadOnly │ WorkspaceWrite │ │ +│ │ (no sandbox) │ (full read, no │ (cwd + extras + │ │ +│ │ │ write/network) │ configurable net) │ │ +│ └──────────────────┴──────────────────┴─────────────────────┘ │ +└─────────────────────────────────────────────────────────────────┘ + │ + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ ACP Connection │ +│ (acp/src/connection.rs) │ +│ - spawn_connection_internal() wraps subprocess in sandbox │ +│ - Platform-specific transformation at spawn time │ +└─────────────────────────────────────────────────────────────────┘ + │ + ┌────────────────────┼────────────────────┐ + ▼ ▼ ▼ +┌─────────────────┐ ┌─────────────────┐ ┌─────────────────────────┐ +│ Linux Landlock │ │ macOS Seatbelt │ │ Windows Restricted │ +│ + seccomp │ │ │ │ Tokens │ +│ (codex-linux- │ │ (/usr/bin/ │ │ (codex-windows-sandbox) │ +│ sandbox binary)│ │ sandbox-exec) │ │ │ +└─────────────────┘ └─────────────────┘ └─────────────────────────┘ +``` + +## Phase 1: Core Sandbox Wrapping + +### 1.1 Add sandbox module to ACP + +Create a new module `acp/src/sandbox.rs` that provides sandbox transformation utilities. + +**Key components:** +- `SandboxType` enum (reuse from codex-core or redefine) +- `get_platform_sandbox()` function for platform detection +- `transform_command_for_sandbox()` function that wraps commands + +### 1.2 Modify spawn_connection_internal() + +Transform the subprocess spawn in `connection.rs` to wrap the command with platform-specific sandbox: + +**Current code** (connection.rs:446-455): +```rust +let mut child = Command::new(&config.command) + .args(&config.args) + .current_dir(cwd) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; +``` + +**New approach:** +```rust +let (exe, args, arg0) = transform_command_for_sandbox( + &config.command, + &config.args, + &sandbox_policy, + cwd, + codex_linux_sandbox_exe.as_deref(), +)?; + +let mut cmd = Command::new(&exe); +cmd.args(&args) + .current_dir(cwd) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + +// For Linux, set argv[0] to "codex-linux-sandbox" for arg0 dispatch +if let Some(ref a0) = arg0 { + cmd.arg0(a0); +} + +let child = cmd.spawn()?; +``` + +### 1.3 Platform-specific transformations + +**macOS (Seatbelt):** +- Wrap command with `/usr/bin/sandbox-exec -p -D -- ` +- Reuse `create_seatbelt_command_args()` from codex-core +- Set env var `CODEX_SANDBOX=seatbelt` + +**Linux (Landlock+seccomp):** +- Wrap command with `codex-linux-sandbox --sandbox-policy --sandbox-policy-cwd -- ` +- Reuse `create_linux_sandbox_command_args()` from codex-core +- Set argv[0] to "codex-linux-sandbox" for arg0 dispatch + +**Windows:** +- More complex - requires in-process token restriction (see Phase 3) + +### 1.4 Remove temporary path restrictions + +After sandbox is implemented, remove the application-level path checking in `write_text_file()` (connection.rs:576-623). The OS-level sandbox will enforce write restrictions more robustly. + +## Phase 2: Configuration Threading + +### 2.1 Add sandbox configuration to AcpAgentConfig + +Modify `registry.rs` to include sandbox-related configuration: + +```rust +pub struct AcpAgentConfig { + pub agent: AgentKind, + pub provider_slug: String, + pub command: String, + pub args: Vec, + pub provider_info: AcpProviderInfo, + // NEW fields: + pub sandbox_policy: SandboxPolicy, + pub codex_linux_sandbox_exe: Option, +} +``` + +### 2.2 Thread sandbox policy through spawn + +Update `AcpConnection::spawn()` signature to accept sandbox configuration: + +```rust +pub async fn spawn( + config: &AcpAgentConfig, + cwd: &Path, + sandbox_policy: &SandboxPolicy, + codex_linux_sandbox_exe: Option<&Path>, +) -> Result +``` + +### 2.3 Update callers in backend.rs + +The `AcpBackend` must pass sandbox policy when spawning connections: + +```rust +// In backend.rs, when creating new AcpConnection: +let connection = AcpConnection::spawn( + &config, + cwd, + &self.sandbox_policy, // From session config + self.codex_linux_sandbox_exe.as_deref(), +).await?; +``` + +### 2.4 Resolve codex-linux-sandbox executable path + +On Linux, the `codex-linux-sandbox` binary must be located. Options: +1. Embedded in main binary via arg0 dispatch (check if main exe supports it) +2. Sibling executable next to main binary +3. Environment variable `CODEX_LINUX_SANDBOX_EXE` +4. Well-known path + +## Phase 3: Windows Support + +Windows sandbox uses a fundamentally different approach than command wrapping. + +### 3.1 Windows token-based sandboxing + +Windows uses `CreateProcessAsUserW` with restricted tokens and ACL manipulation: +- `CreateRestrictedToken()` with `DISABLE_MAX_PRIVILEGE`, `LUA_TOKEN`, `WRITE_RESTRICTED` +- Temporary ACEs added for allowed paths +- ACEs revoked after execution + +### 3.2 Integration options + +**Option A: Wrapper binary approach** +- Create a Windows wrapper similar to `codex-linux-sandbox` +- The wrapper creates restricted token and spawns the real process +- Simpler integration but requires separate binary + +**Option B: In-process token restriction** +- Use `codex-windows-sandbox` crate directly from ACP +- More complex as it requires different spawn path for Windows +- Matches how codex-core implements Windows sandbox + +**Option C: Defer Windows sandboxing** +- Document as unsupported for initial implementation +- ACP on Windows runs without sandbox (matches current behavior) + +### 3.3 Current implementation: Option C (Deferred) + +**Status:** Windows sandboxing is deferred for the initial implementation. + +The current implementation in `acp/src/sandbox.rs` handles Windows by: + +1. **Platform detection:** `get_platform_sandbox()` returns `None` on Windows +2. **No-op transformation:** `transform_command_for_sandbox()` returns the original command unchanged on Windows when `sandbox_type` is `None` +3. **Warning on restrictive policies:** When a non-`DangerFullAccess` policy is requested on Windows, the function returns the command unsandboxed but logs a warning + +**Code behavior on Windows:** +```rust +// In sandbox.rs - Windows returns None for sandbox type +#[cfg(target_os = "windows")] +{ None } + +// transform_command_for_sandbox handles this gracefully: +match get_platform_sandbox() { + Some(sandbox_type) => { /* wrap command */ } + None => { + // Windows or unsupported platform - return command unchanged + Ok(SandboxedCommand { + program: program.to_string(), + args: args.to_vec(), + arg0: None, + sandbox_type: SandboxType::None, + }) + } +} +``` + +**Rationale for deferral:** +1. Windows sandbox requires significant additional complexity (token manipulation, ACL changes) +2. The command-wrapping pattern used by macOS and Linux doesn't apply to Windows +3. Users on Windows can still use ACP with `DangerFullAccess` policy +4. Application-level path restrictions in `write_text_file()` provide basic safety until Windows sandbox is implemented + +**Future work for Windows:** +- Implement `codex-windows-sandbox` wrapper binary approach (Option A) +- Or integrate `windows-sandbox-rs` crate directly with special Windows spawn path (Option B) +- Remove the `SandboxType::None` fallback once Windows sandbox is implemented + +## Reusable Components from codex-core + +| Component | Location | Reusability | +|-----------|----------|-------------| +| `create_seatbelt_command_args()` | `core/src/seatbelt.rs:46` | Direct reuse | +| `create_linux_sandbox_command_args()` | `core/src/landlock.rs:43` | Direct reuse | +| `get_platform_sandbox()` | `core/src/safety.rs:99` | Direct reuse | +| `SandboxType` enum | `core/src/exec.rs:108` | Direct reuse | +| `SandboxPolicy` | `protocol/src/protocol.rs:258` | Already shared | +| `is_likely_sandbox_denied()` | `core/src/exec.rs:383` | Useful for error detection | +| Seatbelt policy files | `core/src/seatbelt_*.sbpl` | Included as strings | + +## Dependencies + +### New dependencies for ACP crate + +```toml +[dependencies] +codex-core = { path = "../core", default-features = false, features = ["sandbox"] } +``` + +Or alternatively, extract sandbox utilities to a new `codex-sandbox` crate that both `codex-core` and `codex-acp` can depend on. + +### Feature flags + +Consider adding a feature flag for sandbox support: +```toml +[features] +default = ["sandbox"] +sandbox = [] +``` + +## Testing + +### Unit tests +- Test `transform_command_for_sandbox()` produces correct command structure +- Test platform detection returns expected values +- Test sandbox policy serialization + +### Integration tests +- Spawn mock agent under sandbox, verify it can read files +- Spawn mock agent under sandbox, verify writes outside cwd fail +- Test sandbox denial detection from exit codes and stderr + +### Manual testing +- Test with real Claude Code CLI on macOS +- Test with real Claude Code CLI on Linux +- Verify agents work correctly under sandbox + +## Migration + +1. Implement sandbox wrapping behind feature flag +2. Test extensively on macOS and Linux +3. Remove temporary path restrictions from `write_text_file()` +4. Enable by default +5. Document Windows limitations + +## Files to modify + +1. `acp/Cargo.toml` - Add dependencies +2. `acp/src/lib.rs` - Export new sandbox module +3. `acp/src/sandbox.rs` - New file with sandbox transformation +4. `acp/src/connection.rs` - Modify spawn to use sandbox +5. `acp/src/registry.rs` - Add sandbox config fields +6. `acp/src/backend.rs` - Thread sandbox policy to connection +7. `core/src/seatbelt.rs` - Make functions pub(crate) -> pub +8. `core/src/landlock.rs` - Make functions pub(crate) -> pub +9. `core/src/safety.rs` - Make functions pub(crate) -> pub diff --git a/codex-rs/acp/Cargo.toml b/codex-rs/acp/Cargo.toml index 04c1263f7..a9d018fe0 100644 --- a/codex-rs/acp/Cargo.toml +++ b/codex-rs/acp/Cargo.toml @@ -29,6 +29,9 @@ toml = { workspace = true } dirs = { workspace = true } diffy = { workspace = true } +[target.'cfg(target_os = "macos")'.dependencies] +codex-core = { path = "../core", default-features = false } + [dev-dependencies] pretty_assertions = { workspace = true } serial_test = { workspace = true } diff --git a/codex-rs/acp/docs.md b/codex-rs/acp/docs.md index bccd77d01..e6a752cd2 100644 --- a/codex-rs/acp/docs.md +++ b/codex-rs/acp/docs.md @@ -160,6 +160,51 @@ The `init_rolling_file_tracing()` function in `@/codex-rs/acp/src/tracing_setup. - ANSI colors disabled for clean file output - Automatically initialized by the CLI (`@/codex-rs/cli/src/main.rs`) at startup, writing to `$NORI_HOME/log/nori-acp.YYYY-MM-DD` +### Subprocess Sandboxing + +The `sandbox` module (`@/codex-rs/acp/src/sandbox.rs`) provides OS-level sandboxing for ACP agent subprocesses. This restricts what the agent process can access at the operating system level. + +**Platform Support:** + +| Platform | Sandbox Type | Implementation | +|----------|--------------|----------------| +| macOS | Seatbelt | `/usr/bin/sandbox-exec` with SBPL profile | +| Linux | Landlock + seccomp | `codex-linux-sandbox` binary | +| Windows | Deferred | Returns command unchanged (TODO) | + +**Key Types:** + +- `SandboxType`: Enum identifying which sandbox to use (`None`, `MacosSeatbelt`, `LinuxSeccomp`, `WindowsRestrictedToken`) +- `SandboxedCommand`: Result struct containing transformed program, args, optional arg0 override, and sandbox type +- `SandboxError`: Error cases (currently only `MissingLinuxSandboxBinary`) + +**Sandbox Policy Mapping:** + +The sandbox respects `SandboxPolicy` from `codex-protocol`: +- `DangerFullAccess`: No sandbox applied, command passed through unchanged +- `ReadOnly`: Full read access, no writes or network +- `WorkspaceWrite`: Can write to workspace directory only + +**Command Transformation:** + +``` +┌─────────────────┐ ┌─────────────────────────────────┐ +│ Original Cmd │ transform_command_ │ Sandboxed Command │ +│ program: npx │ for_sandbox() │ program: /usr/bin/sandbox-exec │ +│ args: [claude] │─────────────────────────►│ args: [-p, , --, ...] │ +│ │ │ arg0: None (macOS) │ +└─────────────────┘ └─────────────────────────────────┘ +``` + +On macOS, the module delegates to `codex_core::seatbelt::create_seatbelt_command_args()` for SBPL profile generation. + +On Linux, args are structured as: `--sandbox-policy-cwd --sandbox-policy -- ` + +**Integration with Connection:** + +`AcpConnection::spawn()` accepts `sandbox_policy` and `codex_linux_sandbox_exe` parameters. The `spawn_connection_internal()` function calls `transform_command_for_sandbox()` before spawning the subprocess. + + ### Core Implementation **Thread-Safe Connection Wrapper (`connection.rs`):** @@ -183,7 +228,7 @@ The ACP library uses `LocalBoxFuture` which is `!Send`, preventing direct use in Model state is stored in `Arc>` shared between the main thread and worker thread for thread-safe access. -- `AcpConnection::spawn()` creates dedicated thread with `LocalSet` for `!Send` futures +- `AcpConnection::spawn(config, cwd, sandbox_policy, codex_linux_sandbox_exe)` creates dedicated thread with `LocalSet` for `!Send` futures, applying OS-level sandboxing to the subprocess - Commands sent via `mpsc::Sender` to worker thread - Responses returned via `oneshot` channels embedded in commands - Worker thread spawns subprocess, handles JSON-RPC handshake, runs command loop @@ -211,7 +256,7 @@ The `write_text_file` method implements file creation and modification for ACP a 1. **Relative Path Resolution**: Paths like `file.txt` are resolved against the workspace directory (`cwd`) before validation, so agents can use simple relative paths for workspace files -2. **Security Boundaries**: Application-level path restrictions (temporary until OS-level sandboxing is deployed): +2. **Security Boundaries (Defense-in-Depth)**: Application-level path restrictions complement OS-level sandboxing: - Workspace writes: Any path within or under the workspace directory - Temporary writes: Any path under `/tmp` directory - System paths: All other paths are rejected with an error @@ -222,6 +267,9 @@ The `write_text_file` method implements file creation and modification for ACP a The path validation canonicalizes paths to prevent symlink-based directory traversal attacks. + +The defense-in-depth approach layers protections: OS sandbox restricts the agent subprocess directly, while application-level checks control what the host writes on behalf of the agent via ACP protocol. This remains valuable because Windows lacks OS-level sandboxing currently. + **Session Transcript Parsing (`session_parser.rs`):** The `session_parser` module provides parsers to extract token usage and metadata from agent session transcript files. Each agent (Claude, Codex, Gemini) runs as an opaque subprocess and stores session data in different formats: diff --git a/codex-rs/acp/src/backend.rs b/codex-rs/acp/src/backend.rs index 871875f19..a71096883 100644 --- a/codex-rs/acp/src/backend.rs +++ b/codex-rs/acp/src/backend.rs @@ -92,8 +92,10 @@ impl AcpBackend { debug!("Spawning ACP backend for model: {}", config.model); - // Spawn the ACP connection - let mut connection = AcpConnection::spawn(&agent_config, &cwd).await?; + // Spawn the ACP connection with sandbox configuration + // TODO: Pass codex_linux_sandbox_exe path from configuration + let mut connection = + AcpConnection::spawn(&agent_config, &cwd, &config.sandbox_policy, None).await?; // Create a session let session_id = connection.create_session(&cwd).await?; diff --git a/codex-rs/acp/src/connection.rs b/codex-rs/acp/src/connection.rs index 71560effd..db34190ac 100644 --- a/codex-rs/acp/src/connection.rs +++ b/codex-rs/acp/src/connection.rs @@ -22,6 +22,7 @@ use anyhow::Result; use codex_protocol::approvals::ApplyPatchApprovalRequestEvent; use codex_protocol::approvals::ExecApprovalRequestEvent; use codex_protocol::protocol::ReviewDecision; +use codex_protocol::protocol::SandboxPolicy; use futures::AsyncBufReadExt; use futures::io::BufReader; use tokio::process::Child; @@ -34,6 +35,7 @@ use tracing::debug; use tracing::warn; use crate::registry::AcpAgentConfig; +use crate::sandbox::transform_command_for_sandbox; use crate::translator; /// The type of approval event to send to the UI. @@ -155,12 +157,21 @@ impl AcpConnection { /// # Arguments /// * `config` - Agent configuration (command, args, provider info) /// * `cwd` - Working directory for the agent subprocess + /// * `sandbox_policy` - Sandbox policy to apply to the subprocess + /// * `codex_linux_sandbox_exe` - Path to the `codex-linux-sandbox` binary (required on Linux) /// /// # Returns /// A connected `AcpConnection` ready for creating sessions. - pub async fn spawn(config: &AcpAgentConfig, cwd: &Path) -> Result { + pub async fn spawn( + config: &AcpAgentConfig, + cwd: &Path, + sandbox_policy: &SandboxPolicy, + codex_linux_sandbox_exe: Option<&Path>, + ) -> Result { let config = config.clone(); let cwd = cwd.to_path_buf(); + let sandbox_policy = sandbox_policy.clone(); + let codex_linux_sandbox_exe = codex_linux_sandbox_exe.map(|p| p.to_path_buf()); // Use a oneshot channel to receive the initialization result let (init_tx, init_rx) = oneshot::channel(); @@ -188,7 +199,15 @@ impl AcpConnection { let local = tokio::task::LocalSet::new(); local .run_until(async move { - match spawn_connection_internal(&config, &cwd, approval_tx).await { + match spawn_connection_internal( + &config, + &cwd, + &sandbox_policy, + codex_linux_sandbox_exe.as_deref(), + approval_tx, + ) + .await + { Ok((inner, capabilities)) => { let _ = init_tx.send(Ok(capabilities)); run_command_loop(inner, command_rx, model_state_for_worker).await; @@ -369,6 +388,8 @@ struct AcpConnectionInner { async fn spawn_connection_internal( config: &AcpAgentConfig, cwd: &Path, + sandbox_policy: &SandboxPolicy, + codex_linux_sandbox_exe: Option<&Path>, approval_tx: mpsc::Sender, ) -> Result<(AcpConnectionInner, acp::AgentCapabilities)> { debug!( @@ -378,12 +399,40 @@ async fn spawn_connection_internal( cwd.display() ); - let mut child = Command::new(&config.command) - .args(&config.args) + // Transform the command for sandbox execution based on the policy + let sandboxed = transform_command_for_sandbox( + &config.command, + &config.args, + sandbox_policy, + cwd, + codex_linux_sandbox_exe, + ) + .with_context(|| format!("Failed to set up sandbox for ACP agent: {}", config.command))?; + + debug!( + "Sandbox transformed command: {} {:?} (type: {:?})", + sandboxed.program, sandboxed.args, sandboxed.sandbox_type + ); + + let mut cmd = Command::new(&sandboxed.program); + cmd.args(&sandboxed.args) .current_dir(cwd) .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()) + .stderr(Stdio::piped()); + + // Set arg0 if specified (used by Linux sandbox for arg0 dispatch). + // The import may appear unused when sandbox gracefully degrades to unsandboxed execution. + #[cfg(unix)] + { + #[allow(unused_imports)] + use std::os::unix::process::CommandExt; + if let Some(ref arg0) = sandboxed.arg0 { + cmd.arg0(arg0); + } + } + + let mut child = cmd .spawn() .with_context(|| format!("Failed to spawn ACP agent: {}", config.command))?; @@ -746,16 +795,20 @@ impl acp::Client for ClientDelegate { path.to_path_buf() }; - // TEMPORARY PATH RESTRICTION: - // This application-level path check provides basic safety until the ACP agent - // subprocess is launched with OS-level sandboxing (Seatbelt on macOS, Landlock - // on Linux, restricted tokens on Windows) as implemented in codex-core's - // sandboxing module. Once subprocess sandboxing is in place, these checks - // should be removed as the OS will enforce write restrictions more robustly. + // PATH RESTRICTION (Defense-in-Depth): + // This application-level path check provides an additional layer of safety. + // While ACP agent subprocesses are now launched with OS-level sandboxing + // (Seatbelt on macOS, Landlock+seccomp on Linux), this restriction remains + // valuable because: // - // For now, restrict writes to: - // 1. Within the working directory (typical workspace operations) - // 2. Within /tmp (temporary files, common for agent workflows) + // 1. The OS sandbox restricts what the agent subprocess can directly access + // 2. This restriction controls what the HOST writes on behalf of the agent + // (when the agent calls write_text_file via ACP protocol) + // 3. Windows currently lacks OS-level sandboxing (deferred implementation) + // + // Restrict writes to: + // - Within the working directory (typical workspace operations) + // - Within /tmp (temporary files, common for agent workflows) let allowed = if let Ok(canonical) = resolved_path.canonicalize() { let in_cwd = self .cwd @@ -792,7 +845,7 @@ impl acp::Client for ClientDelegate { resolved_path.display() ))); } - // END TEMPORARY PATH RESTRICTION + // END PATH RESTRICTION // Create parent directories if they don't exist if let Some(parent) = resolved_path.parent() @@ -897,8 +950,11 @@ mod tests { let temp_dir = tempdir().expect("Failed to create temp dir"); + // Use DangerFullAccess for testing - sandbox transformation is tested in sandbox.rs + let sandbox_policy = SandboxPolicy::DangerFullAccess; + // Spawn connection - let conn = AcpConnection::spawn(&config, temp_dir.path()) + let conn = AcpConnection::spawn(&config, temp_dir.path(), &sandbox_policy, None) .await .expect("Failed to spawn ACP connection"); diff --git a/codex-rs/acp/src/lib.rs b/codex-rs/acp/src/lib.rs index d98958d32..eb0d10f7c 100644 --- a/codex-rs/acp/src/lib.rs +++ b/codex-rs/acp/src/lib.rs @@ -10,6 +10,7 @@ pub mod backend; pub mod config; pub mod connection; pub mod registry; +pub mod sandbox; pub mod session_parser; pub mod tracing_setup; pub mod translator; @@ -46,6 +47,13 @@ pub use tracing_setup::init_rolling_file_tracing; pub use translator::TranslatedEvent; pub use translator::translate_session_update; +// Sandbox types +pub use sandbox::SandboxError; +pub use sandbox::SandboxType; +pub use sandbox::SandboxedCommand; +pub use sandbox::get_platform_sandbox; +pub use sandbox::transform_command_for_sandbox; + // Re-export commonly used types from agent-client-protocol pub use agent_client_protocol::Agent; pub use agent_client_protocol::Client; diff --git a/codex-rs/acp/src/sandbox.rs b/codex-rs/acp/src/sandbox.rs new file mode 100644 index 000000000..a5f43b108 --- /dev/null +++ b/codex-rs/acp/src/sandbox.rs @@ -0,0 +1,370 @@ +//! Sandbox transformation for ACP agent subprocesses. +//! +//! This module provides platform-specific sandbox wrapping for ACP agent +//! subprocesses. On macOS, agents are wrapped with Seatbelt (`sandbox-exec`). +//! On Linux, agents are wrapped with Landlock+seccomp via `codex-linux-sandbox`. +//! On Windows, restricted tokens are used (handled separately). + +use std::path::Path; + +use codex_protocol::protocol::SandboxPolicy; + +/// The type of sandbox to apply to a subprocess. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SandboxType { + /// No sandboxing applied. + None, + /// macOS Seatbelt sandbox via `/usr/bin/sandbox-exec`. + MacosSeatbelt, + /// Linux Landlock + seccomp sandbox via `codex-linux-sandbox`. + LinuxSeccomp, + /// Windows restricted token sandbox (handled in-process, not via command wrapping). + WindowsRestrictedToken, +} + +/// Result of transforming a command for sandbox execution. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SandboxedCommand { + /// The executable to run (may be sandbox wrapper or original command). + pub program: String, + /// Arguments to pass to the executable. + pub args: Vec, + /// Optional override for argv[0] (used for Linux arg0 dispatch). + pub arg0: Option, + /// The sandbox type that was applied. + pub sandbox_type: SandboxType, +} + +/// Returns the appropriate sandbox type for the current platform. +/// +/// Returns `None` if no sandbox is available for the platform. +pub fn get_platform_sandbox() -> Option { + #[cfg(target_os = "macos")] + { + Some(SandboxType::MacosSeatbelt) + } + #[cfg(target_os = "linux")] + { + Some(SandboxType::LinuxSeccomp) + } + #[cfg(target_os = "windows")] + { + // Windows sandbox requires explicit enablement and uses a different + // execution path (in-process token restriction, not command wrapping). + // For now, return None to indicate command wrapping is not used. + None + } + #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "windows")))] + { + None + } +} + +/// Transforms a command to run under the appropriate platform sandbox. +/// +/// # Arguments +/// * `program` - The original program to execute +/// * `args` - Arguments to pass to the program +/// * `sandbox_policy` - The sandbox policy defining allowed operations +/// * `cwd` - The working directory for sandbox policy resolution +/// * `codex_linux_sandbox_exe` - Path to the `codex-linux-sandbox` binary (required on Linux) +/// +/// # Returns +/// A `SandboxedCommand` containing the transformed command, or an error if +/// sandboxing could not be applied (e.g., missing Linux sandbox binary). +pub fn transform_command_for_sandbox( + program: &str, + args: &[String], + sandbox_policy: &SandboxPolicy, + cwd: &Path, + codex_linux_sandbox_exe: Option<&Path>, +) -> Result { + // Build the full command vector (program + args) + let command: Vec = std::iter::once(program.to_string()) + .chain(args.iter().cloned()) + .collect(); + + // Check if sandbox should be bypassed + if matches!(sandbox_policy, SandboxPolicy::DangerFullAccess) { + return Ok(SandboxedCommand { + program: program.to_string(), + args: args.to_vec(), + arg0: None, + sandbox_type: SandboxType::None, + }); + } + + match get_platform_sandbox() { + None => { + // No sandbox available - run without sandboxing + Ok(SandboxedCommand { + program: program.to_string(), + args: args.to_vec(), + arg0: None, + sandbox_type: SandboxType::None, + }) + } + + Some(SandboxType::MacosSeatbelt) => { + #[cfg(target_os = "macos")] + { + let seatbelt_args = create_seatbelt_command_args(command, sandbox_policy, cwd); + Ok(SandboxedCommand { + program: MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string(), + args: seatbelt_args, + arg0: None, + sandbox_type: SandboxType::MacosSeatbelt, + }) + } + #[cfg(not(target_os = "macos"))] + { + // Should not reach here due to get_platform_sandbox() logic + Ok(SandboxedCommand { + program: program.to_string(), + args: args.to_vec(), + arg0: None, + sandbox_type: SandboxType::None, + }) + } + } + + Some(SandboxType::LinuxSeccomp) => { + match codex_linux_sandbox_exe { + Some(exe) => { + let linux_args = + create_linux_sandbox_command_args(command, sandbox_policy, cwd); + Ok(SandboxedCommand { + program: exe.to_string_lossy().to_string(), + args: linux_args, + arg0: Some("codex-linux-sandbox".to_string()), + sandbox_type: SandboxType::LinuxSeccomp, + }) + } + None => { + // Graceful degradation: if the Linux sandbox binary path is not provided, + // run without OS-level sandboxing. This allows ACP to work on Linux even + // before the codex-linux-sandbox binary path is properly configured. + // Application-level path restrictions in write_text_file() still provide + // defense-in-depth protection. + tracing::warn!( + "codex-linux-sandbox binary path not provided; running without OS-level sandbox" + ); + Ok(SandboxedCommand { + program: program.to_string(), + args: args.to_vec(), + arg0: None, + sandbox_type: SandboxType::None, + }) + } + } + } + + Some(SandboxType::WindowsRestrictedToken) => { + // Windows uses in-process token restriction, not command wrapping. + // Return the original command; the caller must handle Windows sandbox separately. + Ok(SandboxedCommand { + program: program.to_string(), + args: args.to_vec(), + arg0: None, + sandbox_type: SandboxType::WindowsRestrictedToken, + }) + } + + Some(SandboxType::None) => Ok(SandboxedCommand { + program: program.to_string(), + args: args.to_vec(), + arg0: None, + sandbox_type: SandboxType::None, + }), + } +} + +/// Errors that can occur during sandbox transformation. +/// +/// Note: Currently no errors are returned - the implementation uses graceful +/// degradation to unsandboxed execution when sandbox setup fails. This error +/// type is kept for future use when stricter sandbox enforcement may be added. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +#[non_exhaustive] +pub enum SandboxError { + // Currently empty - using graceful degradation instead of errors. + // Future variants might include: + // - SandboxBinaryNotFound + // - SandboxBinaryNotExecutable + // - PolicySerializationFailed +} + +// ============================================================================ +// Platform-specific sandbox command generation +// ============================================================================ + +/// Creates command arguments for running under macOS Seatbelt sandbox. +/// Delegates to codex-core's seatbelt implementation and uses its path constant. +#[cfg(target_os = "macos")] +fn create_seatbelt_command_args( + command: Vec, + sandbox_policy: &SandboxPolicy, + sandbox_policy_cwd: &Path, +) -> Vec { + codex_core::seatbelt::create_seatbelt_command_args(command, sandbox_policy, sandbox_policy_cwd) +} + +/// Re-export the seatbelt executable path from codex-core for macOS. +#[cfg(target_os = "macos")] +pub use codex_core::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; + +/// Creates command arguments for running under Linux Landlock+seccomp sandbox. +fn create_linux_sandbox_command_args( + command: Vec, + sandbox_policy: &SandboxPolicy, + sandbox_policy_cwd: &Path, +) -> Vec { + // Serialize sandbox policy to JSON for the helper binary + #[allow(clippy::expect_used)] + let sandbox_policy_cwd_str = sandbox_policy_cwd + .to_str() + .expect("cwd must be valid UTF-8") + .to_string(); + + #[allow(clippy::expect_used)] + let sandbox_policy_json = + serde_json::to_string(sandbox_policy).expect("Failed to serialize SandboxPolicy to JSON"); + + let mut linux_cmd: Vec = vec![ + "--sandbox-policy-cwd".to_string(), + sandbox_policy_cwd_str, + "--sandbox-policy".to_string(), + sandbox_policy_json, + "--".to_string(), + ]; + + linux_cmd.extend(command); + linux_cmd +} + +#[cfg(test)] +mod tests { + use super::*; + #[allow(unused_imports)] + use std::path::PathBuf; + + #[test] + fn test_transform_with_danger_full_access_bypasses_sandbox() { + let policy = SandboxPolicy::DangerFullAccess; + let cwd = PathBuf::from("/tmp/test"); + + let result = transform_command_for_sandbox( + "npx", + &["@anthropic/claude-code".to_string()], + &policy, + &cwd, + None, + ) + .unwrap(); + + assert_eq!(result.program, "npx"); + assert_eq!(result.args, vec!["@anthropic/claude-code"]); + assert_eq!(result.arg0, None); + assert_eq!(result.sandbox_type, SandboxType::None); + } + + #[test] + fn test_get_platform_sandbox_returns_expected_type() { + let sandbox = get_platform_sandbox(); + + #[cfg(target_os = "macos")] + assert_eq!(sandbox, Some(SandboxType::MacosSeatbelt)); + + #[cfg(target_os = "linux")] + assert_eq!(sandbox, Some(SandboxType::LinuxSeccomp)); + + #[cfg(target_os = "windows")] + assert_eq!(sandbox, None); // Windows uses different mechanism + + #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "windows")))] + assert_eq!(sandbox, None); + } + + #[test] + fn test_linux_sandbox_graceful_degradation_without_exe_path() { + // This test is only meaningful on Linux where we expect LinuxSeccomp sandbox + #[cfg(target_os = "linux")] + { + let policy = SandboxPolicy::new_workspace_write_policy(); + let cwd = PathBuf::from("/tmp/test"); + + // When no executable path is provided, should gracefully degrade + // to unsandboxed execution instead of failing + let result = transform_command_for_sandbox( + "npx", + &["@anthropic/claude-code".to_string()], + &policy, + &cwd, + None, // No executable path provided + ) + .unwrap(); + + // Should return original command with SandboxType::None + assert_eq!(result.program, "npx"); + assert_eq!(result.args, vec!["@anthropic/claude-code"]); + assert_eq!(result.sandbox_type, SandboxType::None); + } + } + + #[test] + fn test_linux_sandbox_wraps_command_correctly() { + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + let cwd = PathBuf::from("/home/user/project"); + + // Create the expected linux args directly to verify format + let command = vec!["npx".to_string(), "@anthropic/claude-code".to_string()]; + let linux_args = create_linux_sandbox_command_args(command, &policy, &cwd); + + // Verify the structure of linux args + assert_eq!(linux_args[0], "--sandbox-policy-cwd"); + assert_eq!(linux_args[1], "/home/user/project"); + assert_eq!(linux_args[2], "--sandbox-policy"); + // linux_args[3] is the JSON policy + assert_eq!(linux_args[4], "--"); + assert_eq!(linux_args[5], "npx"); + assert_eq!(linux_args[6], "@anthropic/claude-code"); + + // Verify the JSON policy can be parsed back + let parsed_policy: SandboxPolicy = serde_json::from_str(&linux_args[3]).unwrap(); + assert_eq!(parsed_policy, policy); + } + + #[test] + fn test_read_only_policy_enables_sandbox() { + let policy = SandboxPolicy::ReadOnly; + let cwd = PathBuf::from("/tmp/test"); + + // On platforms with sandbox, ReadOnly should enable it + #[cfg(target_os = "linux")] + { + let linux_exe = PathBuf::from("/usr/local/bin/codex-linux-sandbox"); + let result = transform_command_for_sandbox( + "echo", + &["hello".to_string()], + &policy, + &cwd, + Some(&linux_exe), + ) + .unwrap(); + assert_eq!(result.sandbox_type, SandboxType::LinuxSeccomp); + } + + #[cfg(target_os = "macos")] + { + let result = + transform_command_for_sandbox("echo", &["hello".to_string()], &policy, &cwd, None) + .unwrap(); + assert_eq!(result.sandbox_type, SandboxType::MacosSeatbelt); + } + } +} diff --git a/codex-rs/core/docs.md b/codex-rs/core/docs.md index 009b360a7..082720a61 100644 --- a/codex-rs/core/docs.md +++ b/codex-rs/core/docs.md @@ -90,6 +90,8 @@ Sandboxing is enforced through `safety.rs` and `sandboxing/`: The `SandboxMode` enum controls the policy: `ReadOnly`, `WorkspaceWrite`, `DangerFullAccess`. +The macOS seatbelt module (`@/codex-rs/core/src/seatbelt.rs`) exports `MACOS_PATH_TO_SEATBELT_EXECUTABLE` and `create_seatbelt_command_args()` as public for reuse by `@/codex-rs/acp` when sandboxing ACP agent subprocesses. + **Authentication:** The `auth/` module manages: diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 8ca7e4357..dc5959c43 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -18,7 +18,7 @@ const MACOS_SEATBELT_NETWORK_POLICY: &str = include_str!("seatbelt_network_polic /// to defend against an attacker trying to inject a malicious version on the /// PATH. If /usr/bin/sandbox-exec has been tampered with, then the attacker /// already has root access. -pub(crate) const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec"; +pub const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec"; pub async fn spawn_command_under_seatbelt( command: Vec, @@ -43,7 +43,7 @@ pub async fn spawn_command_under_seatbelt( .await } -pub(crate) fn create_seatbelt_command_args( +pub fn create_seatbelt_command_args( command: Vec, sandbox_policy: &SandboxPolicy, sandbox_policy_cwd: &Path, From 2262a29d60aefc6f55722461a80a2e317d07597c Mon Sep 17 00:00:00 2001 From: Clifford Ressel Date: Fri, 2 Jan 2026 19:03:06 -0500 Subject: [PATCH 2/2] Fix clippy --- codex-rs/acp/src/connection.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/acp/src/connection.rs b/codex-rs/acp/src/connection.rs index db34190ac..5d4c44732 100644 --- a/codex-rs/acp/src/connection.rs +++ b/codex-rs/acp/src/connection.rs @@ -171,7 +171,7 @@ impl AcpConnection { let config = config.clone(); let cwd = cwd.to_path_buf(); let sandbox_policy = sandbox_policy.clone(); - let codex_linux_sandbox_exe = codex_linux_sandbox_exe.map(|p| p.to_path_buf()); + let codex_linux_sandbox_exe = codex_linux_sandbox_exe.map(std::path::Path::to_path_buf); // Use a oneshot channel to receive the initialization result let (init_tx, init_rx) = oneshot::channel();