diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index 55e05251d75..b5e91d7f22e 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -100,6 +100,7 @@ jobs: - name: bazel test //... env: BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }} + CODEX_BWRAP_ENABLE_FFI: ${{ contains(matrix.target, 'unknown-linux') && '1' || '0' }} shell: bash run: | bazel $BAZEL_STARTUP_ARGS --bazelrc=.github/workflows/ci.bazelrc test //... \ diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index ec6319f8dab..b6a3c50e188 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -99,6 +99,9 @@ jobs: USE_SCCACHE: ${{ startsWith(matrix.runner, 'windows') && 'false' || 'true' }} CARGO_INCREMENTAL: "0" SCCACHE_CACHE_SIZE: 10G + # Keep cargo-based CI independent of system bwrap build deps. + # The bwrap FFI path is validated in Bazel workflows. + CODEX_BWRAP_ENABLE_FFI: "0" strategy: fail-fast: false @@ -467,6 +470,9 @@ jobs: USE_SCCACHE: ${{ startsWith(matrix.runner, 'windows') && 'false' || 'true' }} CARGO_INCREMENTAL: "0" SCCACHE_CACHE_SIZE: 10G + # Keep cargo-based CI independent of system bwrap build deps. + # The bwrap FFI path is validated in Bazel workflows. + CODEX_BWRAP_ENABLE_FFI: "0" strategy: fail-fast: false @@ -502,7 +508,6 @@ jobs: steps: - uses: actions/checkout@v6 - # Some integration tests rely on DotSlash being installed. # See https://github.com/openai/codex/pull/7617. - name: Install DotSlash diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 2f764df6802..aa92693eee7 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -65,6 +65,8 @@ jobs: defaults: run: working-directory: codex-rs + env: + CODEX_BWRAP_ENABLE_FFI: ${{ contains(matrix.target, 'unknown-linux') && '1' || '0' }} strategy: fail-fast: false @@ -89,6 +91,13 @@ jobs: steps: - uses: actions/checkout@v6 + - name: Install Linux bwrap build dependencies + if: ${{ runner.os == 'Linux' }} + shell: bash + run: | + set -euo pipefail + sudo apt-get update -y + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends pkg-config libcap-dev - name: Install UBSan runtime (musl) if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl' }} shell: bash diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 43b4b295a82..d83626c4b75 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1797,7 +1797,6 @@ dependencies = [ "serde_json", "tempfile", "tokio", - "which", ] [[package]] diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 219cb465706..ef7e175b103 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1484,6 +1484,7 @@ impl CodexMessageProcessor { let outgoing = self.outgoing.clone(); let req_id = request_id; let sandbox_cwd = self.config.cwd.clone(); + let use_linux_sandbox_bwrap = self.config.features.enabled(Feature::UseLinuxSandboxBwrap); tokio::spawn(async move { match codex_core::exec::process_exec_tool_call( @@ -1491,6 +1492,7 @@ impl CodexMessageProcessor { &effective_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, + use_linux_sandbox_bwrap, None, ) .await diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 5b165f97786..30352cbb831 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -227,16 +227,19 @@ async fn run_command_under_sandbox( .await? } SandboxType::Landlock => { + use codex_core::features::Feature; #[expect(clippy::expect_used)] let codex_linux_sandbox_exe = config .codex_linux_sandbox_exe .expect("codex-linux-sandbox executable not found"); + let use_bwrap_sandbox = config.features.enabled(Feature::UseLinuxSandboxBwrap); spawn_command_under_linux_sandbox( codex_linux_sandbox_exe, command, cwd, config.sandbox_policy.get(), sandbox_policy_cwd.as_path(), + use_bwrap_sandbox, stdio_policy, env, ) diff --git a/codex-rs/common/src/config_override.rs b/codex-rs/common/src/config_override.rs index 59dde92a22b..9bbec2b6f26 100644 --- a/codex-rs/common/src/config_override.rs +++ b/codex-rs/common/src/config_override.rs @@ -71,7 +71,7 @@ impl CliConfigOverrides { } }; - Ok((key.to_string(), value)) + Ok((canonicalize_override_key(key), value)) }) .collect() } @@ -88,6 +88,14 @@ impl CliConfigOverrides { } } +fn canonicalize_override_key(key: &str) -> String { + if key == "use_linux_sandbox_bwrap" { + "features.use_linux_sandbox_bwrap".to_string() + } else { + key.to_string() + } +} + /// Apply a single override onto `root`, creating intermediate objects as /// necessary. fn apply_single_override(root: &mut Value, path: &str, value: Value) { @@ -172,6 +180,16 @@ mod tests { assert_eq!(arr.len(), 3); } + #[test] + fn canonicalizes_use_linux_sandbox_bwrap_alias() { + let overrides = CliConfigOverrides { + raw_overrides: vec!["use_linux_sandbox_bwrap=true".to_string()], + }; + let parsed = overrides.parse_overrides().expect("parse_overrides"); + assert_eq!(parsed[0].0.as_str(), "features.use_linux_sandbox_bwrap"); + assert_eq!(parsed[0].1.as_bool(), Some(true)); + } + #[test] fn parses_inline_table() { let v = parse_toml_value("{a = 1, b = 2}").expect("parse"); diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index bddc9d8a0bd..138b71e2dca 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -235,6 +235,9 @@ "unified_exec": { "type": "boolean" }, + "use_linux_sandbox_bwrap": { + "type": "boolean" + }, "web_search": { "type": "boolean" }, @@ -1253,6 +1256,9 @@ "unified_exec": { "type": "boolean" }, + "use_linux_sandbox_bwrap": { + "type": "boolean" + }, "web_search": { "type": "boolean" }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 979f2a5f629..5368b8b0621 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -508,6 +508,7 @@ pub(crate) struct TurnContext { pub(crate) windows_sandbox_level: WindowsSandboxLevel, pub(crate) shell_environment_policy: ShellEnvironmentPolicy, pub(crate) tools_config: ToolsConfig, + pub(crate) features: Features, pub(crate) ghost_snapshot: GhostSnapshotConfig, pub(crate) final_output_json_schema: Option, pub(crate) codex_linux_sandbox_exe: Option, @@ -766,6 +767,7 @@ impl Session { windows_sandbox_level: session_configuration.windows_sandbox_level, shell_environment_policy: per_turn_config.shell_environment_policy.clone(), tools_config, + features: per_turn_config.features.clone(), ghost_snapshot: per_turn_config.ghost_snapshot.clone(), final_output_json_schema: None, codex_linux_sandbox_exe: per_turn_config.codex_linux_sandbox_exe.clone(), @@ -1036,6 +1038,7 @@ impl Session { sandbox_policy: session_configuration.sandbox_policy.get().clone(), codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), sandbox_cwd: session_configuration.cwd.clone(), + use_linux_sandbox_bwrap: config.features.enabled(Feature::UseLinuxSandboxBwrap), }; let cancel_token = sess.mcp_startup_cancellation_token().await; @@ -1285,6 +1288,9 @@ impl Session { sandbox_policy: per_turn_config.sandbox_policy.get().clone(), codex_linux_sandbox_exe: per_turn_config.codex_linux_sandbox_exe.clone(), sandbox_cwd: per_turn_config.cwd.clone(), + use_linux_sandbox_bwrap: per_turn_config + .features + .enabled(Feature::UseLinuxSandboxBwrap), }; if let Err(e) = self .services @@ -2367,6 +2373,7 @@ impl Session { sandbox_policy: turn_context.sandbox_policy.clone(), codex_linux_sandbox_exe: turn_context.codex_linux_sandbox_exe.clone(), sandbox_cwd: turn_context.cwd.clone(), + use_linux_sandbox_bwrap: turn_context.features.enabled(Feature::UseLinuxSandboxBwrap), }; let cancel_token = self.reset_mcp_startup_cancellation_token().await; @@ -3343,6 +3350,7 @@ async fn spawn_review_thread( session_source, transport_manager, tools_config, + features: parent_turn_context.features.clone(), ghost_snapshot: parent_turn_context.ghost_snapshot.clone(), developer_instructions: None, user_instructions: None, diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index edf3c63aa5b..afe03fbbe77 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -43,6 +43,7 @@ pub async fn list_accessible_connectors_from_mcp_tools( sandbox_policy: SandboxPolicy::ReadOnly, codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), sandbox_cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from("/")), + use_linux_sandbox_bwrap: config.features.enabled(Feature::UseLinuxSandboxBwrap), }; mcp_connection_manager diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index e0d65c83050..3ac8d1956f8 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -140,6 +140,7 @@ pub async fn process_exec_tool_call( sandbox_policy: &SandboxPolicy, sandbox_cwd: &Path, codex_linux_sandbox_exe: &Option, + use_linux_sandbox_bwrap: bool, stdout_stream: Option, ) -> Result { let windows_sandbox_level = params.windows_sandbox_level; @@ -184,14 +185,15 @@ pub async fn process_exec_tool_call( let manager = SandboxManager::new(); let exec_env = manager - .transform( + .transform(crate::sandboxing::SandboxTransformRequest { spec, - sandbox_policy, - sandbox_type, - sandbox_cwd, - codex_linux_sandbox_exe.as_ref(), + policy: sandbox_policy, + sandbox: sandbox_type, + sandbox_policy_cwd: sandbox_cwd, + codex_linux_sandbox_exe: codex_linux_sandbox_exe.as_ref(), + use_linux_sandbox_bwrap, windows_sandbox_level, - ) + }) .map_err(CodexErr::from)?; // Route through the sandboxing module for a single, unified execution path. @@ -1108,6 +1110,7 @@ mod tests { &SandboxPolicy::DangerFullAccess, cwd.as_path(), &None, + false, None, ) .await; diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index a44477905ec..f6c286c5715 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -89,6 +89,8 @@ pub enum Feature { WebSearchCached, /// Gate the execpolicy enforcement for shell/unified exec. ExecPolicy, + /// Use the bubblewrap-based Linux sandbox pipeline. + UseLinuxSandboxBwrap, /// Allow the model to request approval and propose exec rules. RequestRule, /// Enable Windows sandbox (restricted token) on Windows. @@ -465,6 +467,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: true, }, + FeatureSpec { + id: Feature::UseLinuxSandboxBwrap, + key: "use_linux_sandbox_bwrap", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::RequestRule, key: "request_rule", diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 340aebff2c1..ea27f77f75e 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -6,26 +6,34 @@ use std::path::Path; use std::path::PathBuf; use tokio::process::Child; -/// Spawn a shell tool command under the Linux Landlock+seccomp sandbox helper -/// (codex-linux-sandbox). +/// Spawn a shell tool command under the Linux sandbox helper +/// (codex-linux-sandbox), which currently uses bubblewrap for filesystem +/// isolation plus seccomp for network restrictions. /// /// Unlike macOS Seatbelt where we directly embed the policy text, the Linux /// helper accepts a list of `--sandbox-permission`/`-s` flags mirroring the /// public CLI. We convert the internal [`SandboxPolicy`] representation into /// the equivalent CLI options. +#[allow(clippy::too_many_arguments)] pub async fn spawn_command_under_linux_sandbox

( codex_linux_sandbox_exe: P, command: Vec, command_cwd: PathBuf, sandbox_policy: &SandboxPolicy, sandbox_policy_cwd: &Path, + use_bwrap_sandbox: bool, stdio_policy: StdioPolicy, env: HashMap, ) -> std::io::Result where P: AsRef, { - let args = create_linux_sandbox_command_args(command, sandbox_policy, sandbox_policy_cwd); + let args = create_linux_sandbox_command_args( + command, + sandbox_policy, + sandbox_policy_cwd, + use_bwrap_sandbox, + ); let arg0 = Some("codex-linux-sandbox"); spawn_child_async( codex_linux_sandbox_exe.as_ref().to_path_buf(), @@ -40,10 +48,14 @@ where } /// Converts the sandbox policy into the CLI invocation for `codex-linux-sandbox`. +/// +/// The helper performs the actual sandboxing (bubblewrap + seccomp) after +/// parsing these arguments. See `docs/linux_sandbox.md` for the Linux semantics. pub(crate) fn create_linux_sandbox_command_args( command: Vec, sandbox_policy: &SandboxPolicy, sandbox_policy_cwd: &Path, + use_bwrap_sandbox: bool, ) -> Vec { #[expect(clippy::expect_used)] let sandbox_policy_cwd = sandbox_policy_cwd @@ -60,13 +72,42 @@ pub(crate) fn create_linux_sandbox_command_args( sandbox_policy_cwd, "--sandbox-policy".to_string(), sandbox_policy_json, - // Separator so that command arguments starting with `-` are not parsed as - // options of the helper itself. - "--".to_string(), ]; + if use_bwrap_sandbox { + linux_cmd.push("--use-bwrap-sandbox".to_string()); + } + + // Separator so that command arguments starting with `-` are not parsed as + // options of the helper itself. + linux_cmd.push("--".to_string()); // Append the original tool command. linux_cmd.extend(command); linux_cmd } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn bwrap_flags_are_feature_gated() { + let command = vec!["/bin/true".to_string()]; + let cwd = Path::new("/tmp"); + let policy = SandboxPolicy::ReadOnly; + + let with_bwrap = create_linux_sandbox_command_args(command.clone(), &policy, cwd, true); + assert_eq!( + with_bwrap.contains(&"--use-bwrap-sandbox".to_string()), + true + ); + + let without_bwrap = create_linux_sandbox_command_args(command, &policy, cwd, false); + assert_eq!( + without_bwrap.contains(&"--use-bwrap-sandbox".to_string()), + false + ); + } +} diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index cb0b5e2f262..970fa5db401 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -167,6 +167,7 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent sandbox_policy: SandboxPolicy::ReadOnly, codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), sandbox_cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from("/")), + use_linux_sandbox_bwrap: config.features.enabled(Feature::UseLinuxSandboxBwrap), }; mcp_connection_manager diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index c5fbb8ec3d9..456114c30f8 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -313,6 +313,8 @@ pub struct SandboxState { pub sandbox_policy: SandboxPolicy, pub codex_linux_sandbox_exe: Option, pub sandbox_cwd: PathBuf, + #[serde(default)] + pub use_linux_sandbox_bwrap: bool, } /// A thin wrapper around a set of running [`RmcpClient`] instances. diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index fca7adda293..bc914abe5b1 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -51,6 +51,19 @@ pub struct ExecEnv { pub arg0: Option, } +/// Bundled arguments for sandbox transformation. +/// +/// This keeps call sites self-documenting when several fields are optional. +pub(crate) struct SandboxTransformRequest<'a> { + pub spec: CommandSpec, + pub policy: &'a SandboxPolicy, + pub sandbox: SandboxType, + pub sandbox_policy_cwd: &'a Path, + pub codex_linux_sandbox_exe: Option<&'a PathBuf>, + pub use_linux_sandbox_bwrap: bool, + pub windows_sandbox_level: WindowsSandboxLevel, +} + pub enum SandboxPreference { Auto, Require, @@ -104,13 +117,17 @@ impl SandboxManager { pub(crate) fn transform( &self, - mut spec: CommandSpec, - policy: &SandboxPolicy, - sandbox: SandboxType, - sandbox_policy_cwd: &Path, - codex_linux_sandbox_exe: Option<&PathBuf>, - windows_sandbox_level: WindowsSandboxLevel, + request: SandboxTransformRequest<'_>, ) -> Result { + let SandboxTransformRequest { + mut spec, + policy, + sandbox, + sandbox_policy_cwd, + codex_linux_sandbox_exe, + use_linux_sandbox_bwrap, + windows_sandbox_level, + } = request; let mut env = spec.env; if !policy.has_full_network_access() { env.insert( @@ -141,8 +158,12 @@ impl SandboxManager { SandboxType::LinuxSeccomp => { let exe = codex_linux_sandbox_exe .ok_or(SandboxTransformError::MissingLinuxSandboxExecutable)?; - let mut args = - create_linux_sandbox_command_args(command.clone(), policy, sandbox_policy_cwd); + let mut args = create_linux_sandbox_command_args( + command.clone(), + policy, + sandbox_policy_cwd, + use_linux_sandbox_bwrap, + ); let mut full_command = Vec::with_capacity(1 + args.len()); full_command.push(exe.to_string_lossy().to_string()); full_command.append(&mut args); diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index fb2150953bc..381f8ce1364 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -8,6 +8,7 @@ retry without sandbox on denial (no re‑approval thanks to caching). use crate::error::CodexErr; use crate::error::SandboxErr; use crate::exec::ExecToolCallOutput; +use crate::features::Feature; use crate::sandboxing::SandboxManager; use crate::tools::sandboxing::ApprovalCtx; use crate::tools::sandboxing::ExecApprovalRequirement; @@ -97,12 +98,14 @@ impl ToolOrchestrator { // Platform-specific flag gating is handled by SandboxManager::select_initial // via crate::safety::get_platform_sandbox(..). + let use_linux_sandbox_bwrap = turn_ctx.features.enabled(Feature::UseLinuxSandboxBwrap); let initial_attempt = SandboxAttempt { sandbox: initial_sandbox, policy: &turn_ctx.sandbox_policy, manager: &self.sandbox, sandbox_cwd: &turn_ctx.cwd, codex_linux_sandbox_exe: turn_ctx.codex_linux_sandbox_exe.as_ref(), + use_linux_sandbox_bwrap, windows_sandbox_level: turn_ctx.windows_sandbox_level, }; @@ -154,6 +157,7 @@ impl ToolOrchestrator { manager: &self.sandbox, sandbox_cwd: &turn_ctx.cwd, codex_linux_sandbox_exe: None, + use_linux_sandbox_bwrap, windows_sandbox_level: turn_ctx.windows_sandbox_level, }; diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index a7d2bca62ac..d50e5925300 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -274,6 +274,7 @@ pub(crate) struct SandboxAttempt<'a> { pub(crate) manager: &'a SandboxManager, pub(crate) sandbox_cwd: &'a Path, pub codex_linux_sandbox_exe: Option<&'a std::path::PathBuf>, + pub use_linux_sandbox_bwrap: bool, pub windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel, } @@ -282,14 +283,16 @@ impl<'a> SandboxAttempt<'a> { &self, spec: CommandSpec, ) -> Result { - self.manager.transform( - spec, - self.policy, - self.sandbox, - self.sandbox_cwd, - self.codex_linux_sandbox_exe, - self.windows_sandbox_level, - ) + self.manager + .transform(crate::sandboxing::SandboxTransformRequest { + spec, + policy: self.policy, + sandbox: self.sandbox, + sandbox_policy_cwd: self.sandbox_cwd, + codex_linux_sandbox_exe: self.codex_linux_sandbox_exe, + use_linux_sandbox_bwrap: self.use_linux_sandbox_bwrap, + windows_sandbox_level: self.windows_sandbox_level, + }) } } diff --git a/codex-rs/core/tests/suite/exec.rs b/codex-rs/core/tests/suite/exec.rs index cdf597a4e95..b70062d7ede 100644 --- a/codex-rs/core/tests/suite/exec.rs +++ b/codex-rs/core/tests/suite/exec.rs @@ -44,7 +44,7 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result `. +- When enabled, protected subpaths under writable roots (for example `.git`, + resolved `gitdir:`, and `.codex`) are re-applied as read-only via `--ro-bind`. +- When enabled, symlink-in-path and non-existent protected paths inside + writable roots are blocked by mounting `/dev/null` on the symlink or first + missing component. +- When enabled, the helper isolates the PID namespace via `--unshare-pid`. +- When enabled, it mounts a fresh `/proc` via `--proc /proc` by default, but + you can skip this in restrictive container environments with `--no-proc`. + +**Notes** +- The CLI surface still uses legacy names like `codex debug landlock`. diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index c1e0732e081..4a60c65dd3b 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -44,45 +44,6 @@ pub(crate) fn create_bwrap_command_args( sandbox_policy: &SandboxPolicy, cwd: &Path, options: BwrapOptions, - bwrap_path: Option<&Path>, -) -> Result> { - if sandbox_policy.has_full_disk_write_access() { - return Ok(command); - } - - let bwrap_path = match bwrap_path { - Some(path) => { - if path.exists() { - path.to_path_buf() - } else { - return Err(CodexErr::UnsupportedOperation(format!( - "bubblewrap (bwrap) not found at configured path: {}", - path.display() - ))); - } - } - None => which::which("bwrap").map_err(|err| { - CodexErr::UnsupportedOperation(format!("bubblewrap (bwrap) not found on PATH: {err}")) - })?, - }; - - let mut args = Vec::new(); - args.push(path_to_string(&bwrap_path)); - args.extend(create_bwrap_flags(command, sandbox_policy, cwd, options)?); - Ok(args) -} - -/// Doc-hidden helper that builds bubblewrap arguments without a program path. -/// -/// This is intended for experiments where we call a build-time bubblewrap -/// `main` symbol via FFI rather than exec'ing the `bwrap` binary. The caller -/// is responsible for providing a suitable `argv[0]`. -#[doc(hidden)] -pub(crate) fn create_bwrap_command_args_vendored( - command: Vec, - sandbox_policy: &SandboxPolicy, - cwd: &Path, - options: BwrapOptions, ) -> Result> { if sandbox_policy.has_full_disk_write_access() { return Ok(command); diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index 772176fc901..48530117921 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -1,3 +1,7 @@ +//! In-process Linux sandbox primitives: `no_new_privs` and seccomp. +//! +//! Filesystem restrictions are enforced by bubblewrap in `linux_run_main`. +//! Landlock helpers remain available here as legacy/backup utilities. use std::collections::BTreeMap; use std::path::Path; @@ -8,6 +12,7 @@ use codex_core::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use landlock::ABI; +#[allow(unused_imports)] use landlock::Access; use landlock::AccessFs; use landlock::CompatLevel; @@ -27,11 +32,24 @@ use seccompiler::apply_filter; /// Apply sandbox policies inside this thread so only the child inherits /// them, not the entire CLI process. +/// +/// This function is responsible for: +/// - enabling `PR_SET_NO_NEW_PRIVS` when restrictions apply, and +/// - installing the network seccomp filter when network access is disabled. +/// +/// Filesystem restrictions are intentionally handled by bubblewrap. pub(crate) fn apply_sandbox_policy_to_current_thread( sandbox_policy: &SandboxPolicy, cwd: &Path, + apply_landlock_fs: bool, ) -> Result<()> { - if !sandbox_policy.has_full_disk_write_access() || !sandbox_policy.has_full_network_access() { + // `PR_SET_NO_NEW_PRIVS` is required for seccomp, but it also prevents + // setuid privilege elevation. Many `bwrap` deployments rely on setuid, so + // we avoid this unless we need seccomp or we are explicitly using the + // legacy Landlock filesystem pipeline. + if !sandbox_policy.has_full_network_access() + || (apply_landlock_fs && !sandbox_policy.has_full_disk_write_access()) + { set_no_new_privs()?; } @@ -39,7 +57,7 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( install_network_seccomp_filter_on_current_thread()?; } - if !sandbox_policy.has_full_disk_write_access() { + if apply_landlock_fs && !sandbox_policy.has_full_disk_write_access() { let writable_roots = sandbox_policy .get_writable_roots_with_cwd(cwd) .into_iter() @@ -54,6 +72,7 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( Ok(()) } +/// Enable `PR_SET_NO_NEW_PRIVS` so seccomp can be applied safely. fn set_no_new_privs() -> Result<()> { let result = unsafe { libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) }; if result != 0 { @@ -68,6 +87,9 @@ fn set_no_new_privs() -> Result<()> { /// /// # Errors /// Returns [`CodexErr::Sandbox`] variants when the ruleset fails to apply. +/// +/// Note: this is currently unused because filesystem sandboxing is performed +/// via bubblewrap. It is kept for reference and potential fallback use. fn install_filesystem_landlock_rules_on_current_thread( writable_roots: Vec, ) -> Result<()> { @@ -98,6 +120,9 @@ fn install_filesystem_landlock_rules_on_current_thread( /// Installs a seccomp filter that blocks outbound network access except for /// AF_UNIX domain sockets. +/// +/// The filter is applied to the current thread so only the sandboxed child +/// inherits it. fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), SandboxErr> { // Build rule map. let mut rules: BTreeMap> = BTreeMap::new(); diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 3f6cd3ac86e..f5f0d9887aa 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -1,13 +1,16 @@ use clap::Parser; use std::ffi::CString; +use std::fs::File; +use std::io::Read; +use std::os::fd::FromRawFd; use std::path::Path; use std::path::PathBuf; use crate::bwrap::BwrapOptions; use crate::bwrap::create_bwrap_command_args; -use crate::bwrap::create_bwrap_command_args_vendored; use crate::landlock::apply_sandbox_policy_to_current_thread; use crate::vendored_bwrap::exec_vendored_bwrap; +use crate::vendored_bwrap::run_vendored_bwrap_main; #[derive(Debug, Parser)] /// CLI surface for the Linux sandbox helper. @@ -29,18 +32,6 @@ pub struct LandlockCommand { #[arg(long = "use-bwrap-sandbox", hide = true, default_value_t = false)] pub use_bwrap_sandbox: bool, - /// Optional explicit path to the `bwrap` binary to use. - /// - /// When provided, this implies bubblewrap opt-in and avoids PATH lookups. - #[arg(long = "bwrap-path", hide = true)] - pub bwrap_path: Option, - - /// Experimental: call a build-time bubblewrap `main()` via FFI. - /// - /// This is opt-in and only works when the build script compiles bwrap. - #[arg(long = "use-vendored-bwrap", hide = true, default_value_t = false)] - pub use_vendored_bwrap: bool, - /// Internal: apply seccomp and `no_new_privs` in the already-sandboxed /// process, then exec the user command. /// @@ -72,13 +63,10 @@ pub fn run_main() -> ! { sandbox_policy_cwd, sandbox_policy, use_bwrap_sandbox, - bwrap_path, - use_vendored_bwrap, apply_seccomp_then_exec, no_proc, command, } = LandlockCommand::parse(); - let use_bwrap_sandbox = use_bwrap_sandbox || bwrap_path.is_some() || use_vendored_bwrap; if command.is_empty() { panic!("No command specified to execute."); @@ -87,74 +75,192 @@ pub fn run_main() -> ! { // Inner stage: apply seccomp/no_new_privs after bubblewrap has already // established the filesystem view. if apply_seccomp_then_exec { - if let Err(e) = apply_sandbox_policy_to_current_thread(&sandbox_policy, &sandbox_policy_cwd) + if let Err(e) = + apply_sandbox_policy_to_current_thread(&sandbox_policy, &sandbox_policy_cwd, false) { panic!("error applying Linux sandbox restrictions: {e:?}"); } exec_or_panic(command); } - let command = if sandbox_policy.has_full_disk_write_access() { - if let Err(e) = apply_sandbox_policy_to_current_thread(&sandbox_policy, &sandbox_policy_cwd) + if sandbox_policy.has_full_disk_write_access() { + if let Err(e) = + apply_sandbox_policy_to_current_thread(&sandbox_policy, &sandbox_policy_cwd, false) { panic!("error applying Linux sandbox restrictions: {e:?}"); } - command - } else if use_bwrap_sandbox { + exec_or_panic(command); + } + + if use_bwrap_sandbox { // Outer stage: bubblewrap first, then re-enter this binary in the - // sandboxed environment to apply seccomp. + // sandboxed environment to apply seccomp. This path never falls back + // to legacy Landlock on failure. let inner = build_inner_seccomp_command( &sandbox_policy_cwd, &sandbox_policy, use_bwrap_sandbox, - bwrap_path.as_deref(), command, ); - let options = BwrapOptions { - mount_proc: !no_proc, - }; - if use_vendored_bwrap { - let mut argv0 = bwrap_path - .as_deref() - .map(|path| path.to_string_lossy().to_string()) - .unwrap_or_else(|| "bwrap".to_string()); - if argv0.is_empty() { - argv0 = "bwrap".to_string(); - } + run_bwrap_with_proc_fallback(&sandbox_policy_cwd, &sandbox_policy, inner, !no_proc); + } + + // Legacy path: Landlock enforcement only, when bwrap sandboxing is not enabled. + if let Err(e) = + apply_sandbox_policy_to_current_thread(&sandbox_policy, &sandbox_policy_cwd, true) + { + panic!("error applying legacy Linux sandbox restrictions: {e:?}"); + } + exec_or_panic(command); +} + +fn run_bwrap_with_proc_fallback( + sandbox_policy_cwd: &Path, + sandbox_policy: &codex_core::protocol::SandboxPolicy, + inner: Vec, + mount_proc: bool, +) -> ! { + let mut mount_proc = mount_proc; + + if mount_proc && !preflight_proc_mount_support(sandbox_policy_cwd, sandbox_policy) { + eprintln!("codex-linux-sandbox: bwrap could not mount /proc; retrying with --no-proc"); + mount_proc = false; + } + + let options = BwrapOptions { mount_proc }; + let argv = build_bwrap_argv(inner, sandbox_policy, sandbox_policy_cwd, options); + exec_vendored_bwrap(argv); +} + +fn build_bwrap_argv( + inner: Vec, + sandbox_policy: &codex_core::protocol::SandboxPolicy, + sandbox_policy_cwd: &Path, + options: BwrapOptions, +) -> Vec { + let mut args = create_bwrap_command_args(inner, sandbox_policy, sandbox_policy_cwd, options) + .unwrap_or_else(|err| panic!("error building bubblewrap command: {err:?}")); + + let command_separator_index = args + .iter() + .position(|arg| arg == "--") + .unwrap_or_else(|| panic!("bubblewrap argv is missing command separator '--'")); + args.splice( + command_separator_index..command_separator_index, + ["--argv0".to_string(), "codex-linux-sandbox".to_string()], + ); + + let mut argv = vec!["bwrap".to_string()]; + argv.extend(args); + argv +} + +fn preflight_proc_mount_support( + sandbox_policy_cwd: &Path, + sandbox_policy: &codex_core::protocol::SandboxPolicy, +) -> bool { + let preflight_command = vec![resolve_true_command()]; + let preflight_argv = build_bwrap_argv( + preflight_command, + sandbox_policy, + sandbox_policy_cwd, + BwrapOptions { mount_proc: true }, + ); + let stderr = run_bwrap_in_child_capture_stderr(preflight_argv); + !is_proc_mount_failure(stderr.as_str()) +} - let mut argv = vec![argv0]; - argv.extend( - create_bwrap_command_args_vendored( - inner, - &sandbox_policy, - &sandbox_policy_cwd, - options, - ) - .unwrap_or_else(|err| { - panic!("error building build-time bubblewrap command: {err:?}") - }), - ); - exec_vendored_bwrap(argv); +fn resolve_true_command() -> String { + for candidate in ["/usr/bin/true", "/bin/true"] { + if Path::new(candidate).exists() { + return candidate.to_string(); } - ensure_bwrap_available(bwrap_path.as_deref()); - create_bwrap_command_args( - inner, - &sandbox_policy, - &sandbox_policy_cwd, - options, - bwrap_path.as_deref(), - ) - .unwrap_or_else(|err| panic!("error building bubblewrap command: {err:?}")) - } else { - // Legacy path: Landlock enforcement only. - if let Err(e) = apply_sandbox_policy_to_current_thread(&sandbox_policy, &sandbox_policy_cwd) - { - panic!("error applying legacy Linux sandbox restrictions: {e:?}"); + } + "true".to_string() +} + +/// Run a short-lived bubblewrap preflight in a child process and capture stderr. +/// +/// Strategy: +/// - This is used only by `preflight_proc_mount_support`, which runs `/bin/true` +/// under bubblewrap with `--proc /proc`. +/// - The goal is to detect environments where mounting `/proc` fails (for +/// example, restricted containers), so we can retry the real run with +/// `--no-proc`. +/// - We capture stderr from that preflight to match known mount-failure text. +/// We do not stream it because this is a one-shot probe with a trivial +/// command, and reads are bounded to a fixed max size. +fn run_bwrap_in_child_capture_stderr(argv: Vec) -> String { + const MAX_PREFLIGHT_STDERR_BYTES: u64 = 64 * 1024; + + let mut pipe_fds = [0; 2]; + let pipe_res = unsafe { libc::pipe2(pipe_fds.as_mut_ptr(), libc::O_CLOEXEC) }; + if pipe_res < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to create stderr pipe for bubblewrap: {err}"); + } + let read_fd = pipe_fds[0]; + let write_fd = pipe_fds[1]; + + let pid = unsafe { libc::fork() }; + if pid < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to fork for bubblewrap: {err}"); + } + + if pid == 0 { + // Child: redirect stderr to the pipe, then run bubblewrap. + unsafe { + close_fd_or_panic(read_fd, "close read end in bubblewrap child"); + if libc::dup2(write_fd, libc::STDERR_FILENO) < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to redirect stderr for bubblewrap: {err}"); + } + close_fd_or_panic(write_fd, "close write end in bubblewrap child"); } - command - }; - exec_or_panic(command); + let exit_code = run_vendored_bwrap_main(&argv); + std::process::exit(exit_code); + } + + // Parent: close the write end and read stderr while the child runs. + close_fd_or_panic(write_fd, "close write end in bubblewrap parent"); + + // SAFETY: `read_fd` is a valid owned fd in the parent. + let mut read_file = unsafe { File::from_raw_fd(read_fd) }; + let mut stderr_bytes = Vec::new(); + let mut limited_reader = (&mut read_file).take(MAX_PREFLIGHT_STDERR_BYTES); + if let Err(err) = limited_reader.read_to_end(&mut stderr_bytes) { + panic!("failed to read bubblewrap stderr: {err}"); + } + + let mut status: libc::c_int = 0; + let wait_res = unsafe { libc::waitpid(pid, &mut status as *mut libc::c_int, 0) }; + if wait_res < 0 { + let err = std::io::Error::last_os_error(); + panic!("waitpid failed for bubblewrap child: {err}"); + } + + String::from_utf8_lossy(&stderr_bytes).into_owned() +} + +/// Close an owned file descriptor and panic with context on failure. +/// +/// We use explicit close() checks here (instead of ignoring return codes) +/// because this code runs in low-level sandbox setup paths where fd leaks or +/// close errors can mask the root cause of later failures. +fn close_fd_or_panic(fd: libc::c_int, context: &str) { + let close_res = unsafe { libc::close(fd) }; + if close_res < 0 { + let err = std::io::Error::last_os_error(); + panic!("{context}: {err}"); + } +} + +fn is_proc_mount_failure(stderr: &str) -> bool { + stderr.contains("Can't mount proc") + && stderr.contains("/newroot/proc") + && stderr.contains("Invalid argument") } /// Build the inner command that applies seccomp after bubblewrap. @@ -162,7 +268,6 @@ fn build_inner_seccomp_command( sandbox_policy_cwd: &Path, sandbox_policy: &codex_core::protocol::SandboxPolicy, use_bwrap_sandbox: bool, - bwrap_path: Option<&Path>, command: Vec, ) -> Vec { let current_exe = match std::env::current_exe() { @@ -185,10 +290,6 @@ fn build_inner_seccomp_command( inner.push("--use-bwrap-sandbox".to_string()); inner.push("--apply-seccomp-then-exec".to_string()); } - if let Some(bwrap_path) = bwrap_path { - inner.push("--bwrap-path".to_string()); - inner.push(bwrap_path.to_string_lossy().to_string()); - } inner.push("--".to_string()); inner.extend(command); inner @@ -217,32 +318,52 @@ fn exec_or_panic(command: Vec) -> ! { panic!("Failed to execvp {}: {err}", command[0].as_str()); } -/// Ensure the `bwrap` binary is available when the sandbox needs it. -fn ensure_bwrap_available(bwrap_path: Option<&Path>) { - if let Some(path) = bwrap_path { - if path.exists() { - return; - } - panic!( - "bubblewrap (bwrap) is required for Linux filesystem sandboxing but was not found at the configured path: {}\n\ -Install it and retry. Examples:\n\ -- Debian/Ubuntu: apt-get install bubblewrap\n\ -- Fedora/RHEL: dnf install bubblewrap\n\ -- Arch: pacman -S bubblewrap\n\ -If you are running the Codex Node package, ensure bwrap is installed on the host system.", - path.display() - ); +#[cfg(test)] +mod tests { + use super::*; + use codex_core::protocol::SandboxPolicy; + use pretty_assertions::assert_eq; + + #[test] + fn detects_proc_mount_invalid_argument_failure() { + let stderr = "bwrap: Can't mount proc on /newroot/proc: Invalid argument"; + assert_eq!(is_proc_mount_failure(stderr), true); } - if which::which("bwrap").is_ok() { - return; + + #[test] + fn ignores_non_proc_mount_errors() { + let stderr = "bwrap: Can't bind mount /dev/null: Operation not permitted"; + assert_eq!(is_proc_mount_failure(stderr), false); } - panic!( - "bubblewrap (bwrap) is required for Linux filesystem sandboxing but was not found on PATH.\n\ -Install it and retry. Examples:\n\ -- Debian/Ubuntu: apt-get install bubblewrap\n\ -- Fedora/RHEL: dnf install bubblewrap\n\ -- Arch: pacman -S bubblewrap\n\ -If you are running the Codex Node package, ensure bwrap is installed on the host system." - ); + #[test] + fn inserts_bwrap_argv0_before_command_separator() { + let argv = build_bwrap_argv( + vec!["/bin/true".to_string()], + &SandboxPolicy::ReadOnly, + Path::new("/"), + BwrapOptions { mount_proc: true }, + ); + assert_eq!( + argv, + vec![ + "bwrap".to_string(), + "--new-session".to_string(), + "--die-with-parent".to_string(), + "--ro-bind".to_string(), + "/".to_string(), + "/".to_string(), + "--dev-bind".to_string(), + "/dev/null".to_string(), + "/dev/null".to_string(), + "--unshare-pid".to_string(), + "--proc".to_string(), + "/proc".to_string(), + "--argv0".to_string(), + "codex-linux-sandbox".to_string(), + "--".to_string(), + "/bin/true".to_string(), + ] + ); + } } diff --git a/codex-rs/linux-sandbox/src/mounts.rs b/codex-rs/linux-sandbox/src/mounts.rs deleted file mode 100644 index 04f3651dfff..00000000000 --- a/codex-rs/linux-sandbox/src/mounts.rs +++ /dev/null @@ -1,339 +0,0 @@ -#![allow(dead_code)] - -use std::ffi::CString; -use std::os::unix::ffi::OsStrExt; -use std::path::Path; - -use codex_core::error::CodexErr; -use codex_core::error::Result; -use codex_core::protocol::SandboxPolicy; -use codex_core::protocol::WritableRoot; -use codex_utils_absolute_path::AbsolutePathBuf; - -/// Apply read-only bind mounts for protected subpaths before Landlock. -/// -/// This unshares mount namespaces (and user namespaces for non-root) so the -/// read-only remounts do not affect the host, then bind-mounts each protected -/// target onto itself and remounts it read-only. -pub(crate) fn apply_read_only_mounts(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result<()> { - let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); - let mount_targets = collect_read_only_mount_targets(&writable_roots)?; - if mount_targets.is_empty() { - return Ok(()); - } - - // Root can unshare the mount namespace directly; non-root needs a user - // namespace to gain capabilities for remounting. - if is_running_as_root() { - unshare_mount_namespace()?; - } else { - let original_euid = unsafe { libc::geteuid() }; - let original_egid = unsafe { libc::getegid() }; - unshare_user_and_mount_namespaces()?; - write_user_namespace_maps(original_euid, original_egid)?; - } - make_mounts_private()?; - - for target in mount_targets { - // Bind and remount read-only works for both files and directories. - bind_mount_read_only(target.as_path())?; - } - - // Drop ambient capabilities acquired from the user namespace so the - // sandboxed command cannot remount or create new bind mounts. - if !is_running_as_root() { - drop_caps()?; - } - - Ok(()) -} - -/// Collect read-only mount targets, resolving worktree `.git` pointer files. -fn collect_read_only_mount_targets( - writable_roots: &[WritableRoot], -) -> Result> { - let mut targets = Vec::new(); - for writable_root in writable_roots { - for ro_subpath in &writable_root.read_only_subpaths { - // The policy expects these paths to exist; surface actionable errors - // rather than silently skipping protections. - if !ro_subpath.as_path().exists() { - return Err(CodexErr::UnsupportedOperation(format!( - "Sandbox expected to protect {path}, but it does not exist. Ensure the repository contains this path or create it before running Codex.", - path = ro_subpath.as_path().display() - ))); - } - targets.push(ro_subpath.clone()); - // Worktrees and submodules store `.git` as a pointer file; add the - // referenced gitdir as an extra read-only target. - if is_git_pointer_file(ro_subpath) { - let gitdir = resolve_gitdir_from_file(ro_subpath)?; - if !targets - .iter() - .any(|target| target.as_path() == gitdir.as_path()) - { - targets.push(gitdir); - } - } - } - } - Ok(targets) -} - -/// Detect a `.git` pointer file used by worktrees and submodules. -fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool { - path.as_path().is_file() && path.as_path().file_name() == Some(std::ffi::OsStr::new(".git")) -} - -/// Resolve a worktree `.git` pointer file to its gitdir path. -fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Result { - let contents = std::fs::read_to_string(dot_git.as_path()).map_err(CodexErr::from)?; - let trimmed = contents.trim(); - let (_, gitdir_raw) = trimmed.split_once(':').ok_or_else(|| { - CodexErr::UnsupportedOperation(format!( - "Expected {path} to contain a gitdir pointer, but it did not match `gitdir: `.", - path = dot_git.as_path().display() - )) - })?; - // `gitdir: ` may be relative to the directory containing `.git`. - let gitdir_raw = gitdir_raw.trim(); - if gitdir_raw.is_empty() { - return Err(CodexErr::UnsupportedOperation(format!( - "Expected {path} to contain a gitdir pointer, but it was empty.", - path = dot_git.as_path().display() - ))); - } - let base = dot_git.as_path().parent().ok_or_else(|| { - CodexErr::UnsupportedOperation(format!( - "Unable to resolve parent directory for {path}.", - path = dot_git.as_path().display() - )) - })?; - let gitdir_path = AbsolutePathBuf::resolve_path_against_base(gitdir_raw, base)?; - if !gitdir_path.as_path().exists() { - return Err(CodexErr::UnsupportedOperation(format!( - "Resolved gitdir path {path} does not exist.", - path = gitdir_path.as_path().display() - ))); - } - Ok(gitdir_path) -} - -/// Unshare the mount namespace so mount changes are isolated to the sandboxed process. -fn unshare_mount_namespace() -> Result<()> { - let result = unsafe { libc::unshare(libc::CLONE_NEWNS) }; - if result != 0 { - return Err(std::io::Error::last_os_error().into()); - } - Ok(()) -} - -/// Unshare user + mount namespaces so the process can remount read-only without privileges. -fn unshare_user_and_mount_namespaces() -> Result<()> { - let result = unsafe { libc::unshare(libc::CLONE_NEWUSER | libc::CLONE_NEWNS) }; - if result != 0 { - return Err(std::io::Error::last_os_error().into()); - } - Ok(()) -} - -fn is_running_as_root() -> bool { - unsafe { libc::geteuid() == 0 } -} - -#[repr(C)] -struct CapUserHeader { - version: u32, - pid: i32, -} - -#[repr(C)] -struct CapUserData { - effective: u32, - permitted: u32, - inheritable: u32, -} - -const LINUX_CAPABILITY_VERSION_3: u32 = 0x2008_0522; - -/// Map the provided uid/gid to root inside the user namespace. -fn write_user_namespace_maps(uid: libc::uid_t, gid: libc::gid_t) -> Result<()> { - write_proc_file("/proc/self/setgroups", "deny\n")?; - - write_proc_file("/proc/self/uid_map", format!("0 {uid} 1\n"))?; - write_proc_file("/proc/self/gid_map", format!("0 {gid} 1\n"))?; - Ok(()) -} - -/// Drop all capabilities in the current user namespace. -fn drop_caps() -> Result<()> { - let mut header = CapUserHeader { - version: LINUX_CAPABILITY_VERSION_3, - pid: 0, - }; - let data = [ - CapUserData { - effective: 0, - permitted: 0, - inheritable: 0, - }, - CapUserData { - effective: 0, - permitted: 0, - inheritable: 0, - }, - ]; - - // Use syscall directly to avoid libc capability symbols that are missing on musl. - let result = unsafe { libc::syscall(libc::SYS_capset, &mut header, data.as_ptr()) }; - if result != 0 { - return Err(std::io::Error::last_os_error().into()); - } - Ok(()) -} - -/// Write a small procfs file, returning a sandbox error on failure. -fn write_proc_file(path: &str, contents: impl AsRef<[u8]>) -> Result<()> { - std::fs::write(path, contents)?; - Ok(()) -} - -/// Ensure mounts are private so remounting does not propagate outside the namespace. -fn make_mounts_private() -> Result<()> { - let root = CString::new("/").map_err(|_| { - CodexErr::UnsupportedOperation("Sandbox mount path contains NUL byte: /".to_string()) - })?; - let result = unsafe { - libc::mount( - std::ptr::null(), - root.as_ptr(), - std::ptr::null(), - libc::MS_REC | libc::MS_PRIVATE, - std::ptr::null(), - ) - }; - if result != 0 { - return Err(std::io::Error::last_os_error().into()); - } - Ok(()) -} - -/// Bind-mount a path onto itself and remount read-only. -fn bind_mount_read_only(path: &Path) -> Result<()> { - let c_path = CString::new(path.as_os_str().as_bytes()).map_err(|_| { - CodexErr::UnsupportedOperation(format!( - "Sandbox mount path contains NUL byte: {path}", - path = path.display() - )) - })?; - - let bind_result = unsafe { - libc::mount( - c_path.as_ptr(), - c_path.as_ptr(), - std::ptr::null(), - libc::MS_BIND, - std::ptr::null(), - ) - }; - if bind_result != 0 { - return Err(std::io::Error::last_os_error().into()); - } - - let remount_result = unsafe { - libc::mount( - c_path.as_ptr(), - c_path.as_ptr(), - std::ptr::null(), - libc::MS_BIND | libc::MS_REMOUNT | libc::MS_RDONLY, - std::ptr::null(), - ) - }; - if remount_result != 0 { - return Err(std::io::Error::last_os_error().into()); - } - - Ok(()) -} - -#[cfg(test)] -mod tests { - use super::*; - use pretty_assertions::assert_eq; - - #[test] - fn collect_read_only_mount_targets_errors_on_missing_path() { - let tempdir = tempfile::tempdir().expect("tempdir"); - let missing = AbsolutePathBuf::try_from(tempdir.path().join("missing").as_path()) - .expect("missing path"); - let root = AbsolutePathBuf::try_from(tempdir.path()).expect("root"); - let writable_root = WritableRoot { - root, - read_only_subpaths: vec![missing], - }; - - let err = collect_read_only_mount_targets(&[writable_root]) - .expect_err("expected missing path error"); - let message = match err { - CodexErr::UnsupportedOperation(message) => message, - other => panic!("unexpected error: {other:?}"), - }; - assert_eq!( - message, - format!( - "Sandbox expected to protect {path}, but it does not exist. Ensure the repository contains this path or create it before running Codex.", - path = tempdir.path().join("missing").display() - ) - ); - } - - #[test] - fn collect_read_only_mount_targets_adds_gitdir_for_pointer_file() { - let tempdir = tempfile::tempdir().expect("tempdir"); - let gitdir = tempdir.path().join("actual-gitdir"); - std::fs::create_dir_all(&gitdir).expect("create gitdir"); - let dot_git = tempdir.path().join(".git"); - std::fs::write(&dot_git, format!("gitdir: {}\n", gitdir.display())) - .expect("write gitdir pointer"); - let root = AbsolutePathBuf::try_from(tempdir.path()).expect("root"); - let writable_root = WritableRoot { - root, - read_only_subpaths: vec![ - AbsolutePathBuf::try_from(dot_git.as_path()).expect("dot git"), - ], - }; - - let targets = collect_read_only_mount_targets(&[writable_root]).expect("collect targets"); - assert_eq!(targets.len(), 2); - assert_eq!(targets[0].as_path(), dot_git.as_path()); - assert_eq!(targets[1].as_path(), gitdir.as_path()); - } - - #[test] - fn collect_read_only_mount_targets_errors_on_invalid_gitdir_pointer() { - let tempdir = tempfile::tempdir().expect("tempdir"); - let dot_git = tempdir.path().join(".git"); - std::fs::write(&dot_git, "not-a-pointer\n").expect("write invalid pointer"); - let root = AbsolutePathBuf::try_from(tempdir.path()).expect("root"); - let writable_root = WritableRoot { - root, - read_only_subpaths: vec![ - AbsolutePathBuf::try_from(dot_git.as_path()).expect("dot git"), - ], - }; - - let err = collect_read_only_mount_targets(&[writable_root]) - .expect_err("expected invalid pointer error"); - let message = match err { - CodexErr::UnsupportedOperation(message) => message, - other => panic!("unexpected error: {other:?}"), - }; - assert_eq!( - message, - format!( - "Expected {path} to contain a gitdir pointer, but it did not match `gitdir: `.", - path = dot_git.display() - ) - ); - } -} diff --git a/codex-rs/linux-sandbox/src/vendored_bwrap.rs b/codex-rs/linux-sandbox/src/vendored_bwrap.rs index ab4fb959ef2..c2816061ea9 100644 --- a/codex-rs/linux-sandbox/src/vendored_bwrap.rs +++ b/codex-rs/linux-sandbox/src/vendored_bwrap.rs @@ -13,22 +13,35 @@ mod imp { fn bwrap_main(argc: libc::c_int, argv: *const *const c_char) -> libc::c_int; } - /// Execute the build-time bubblewrap `main` function with the given argv. - pub(crate) fn exec_vendored_bwrap(argv: Vec) -> ! { + fn argv_to_cstrings(argv: &[String]) -> Vec { let mut cstrings: Vec = Vec::with_capacity(argv.len()); - for arg in &argv { + for arg in argv { match CString::new(arg.as_str()) { Ok(value) => cstrings.push(value), Err(err) => panic!("failed to convert argv to CString: {err}"), } } + cstrings + } + + /// Run the build-time bubblewrap `main` function and return its exit code. + /// + /// On success, bubblewrap will `execve` into the target program and this + /// function will never return. A return value therefore implies failure. + pub(crate) fn run_vendored_bwrap_main(argv: &[String]) -> libc::c_int { + let cstrings = argv_to_cstrings(argv); let mut argv_ptrs: Vec<*const c_char> = cstrings.iter().map(|arg| arg.as_ptr()).collect(); argv_ptrs.push(std::ptr::null()); // SAFETY: We provide a null-terminated argv vector whose pointers // remain valid for the duration of the call. - let exit_code = unsafe { bwrap_main(cstrings.len() as libc::c_int, argv_ptrs.as_ptr()) }; + unsafe { bwrap_main(cstrings.len() as libc::c_int, argv_ptrs.as_ptr()) } + } + + /// Execute the build-time bubblewrap `main` function with the given argv. + pub(crate) fn exec_vendored_bwrap(argv: Vec) -> ! { + let exit_code = run_vendored_bwrap_main(&argv); std::process::exit(exit_code); } } @@ -36,7 +49,7 @@ mod imp { #[cfg(not(vendored_bwrap_available))] mod imp { /// Panics with a clear error when the build-time bwrap path is not enabled. - pub(crate) fn exec_vendored_bwrap(_argv: Vec) -> ! { + pub(crate) fn run_vendored_bwrap_main(_argv: &[String]) -> libc::c_int { panic!( "build-time bubblewrap is not available in this build.\n\ Rebuild codex-linux-sandbox on Linux with CODEX_BWRAP_ENABLE_FFI=1.\n\ @@ -49,6 +62,13 @@ Notes:\n\ - bubblewrap sources expected at codex-rs/vendor/bubblewrap (default)" ); } + + /// Panics with a clear error when the build-time bwrap path is not enabled. + pub(crate) fn exec_vendored_bwrap(_argv: Vec) -> ! { + let _ = run_vendored_bwrap_main(&[]); + unreachable!("run_vendored_bwrap_main should always panic in this configuration") + } } pub(crate) use imp::exec_vendored_bwrap; +pub(crate) use imp::run_vendored_bwrap_main; diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index 75551d4d913..52cd402a134 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -2,6 +2,7 @@ #![allow(clippy::unwrap_used)] use codex_core::config::types::ShellEnvironmentPolicy; use codex_core::error::CodexErr; +use codex_core::error::Result; use codex_core::error::SandboxErr; use codex_core::exec::ExecParams; use codex_core::exec::process_exec_tool_call; @@ -32,6 +33,8 @@ const NETWORK_TIMEOUT_MS: u64 = 2_000; #[cfg(target_arch = "aarch64")] const NETWORK_TIMEOUT_MS: u64 = 10_000; +const BWRAP_UNAVAILABLE_ERR: &str = "build-time bubblewrap is not available in this build."; + fn create_env_from_core_vars() -> HashMap { let policy = ShellEnvironmentPolicy::default(); create_env(&policy, None) @@ -47,12 +50,24 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { } } -#[expect(clippy::expect_used, clippy::unwrap_used)] +#[expect(clippy::expect_used)] async fn run_cmd_output( cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64, ) -> codex_core::exec::ExecToolCallOutput { + run_cmd_result_with_writable_roots(cmd, writable_roots, timeout_ms, false) + .await + .expect("sandboxed command should execute") +} + +#[expect(clippy::expect_used)] +async fn run_cmd_result_with_writable_roots( + cmd: &[&str], + writable_roots: &[PathBuf], + timeout_ms: u64, + use_bwrap_sandbox: bool, +) -> Result { let cwd = std::env::current_dir().expect("cwd should exist"); let sandbox_cwd = cwd.clone(); let params = ExecParams { @@ -86,10 +101,48 @@ async fn run_cmd_output( &sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, + use_bwrap_sandbox, None, ) .await - .unwrap() +} + +fn is_bwrap_unavailable_output(output: &codex_core::exec::ExecToolCallOutput) -> bool { + output.stderr.text.contains(BWRAP_UNAVAILABLE_ERR) +} + +async fn should_skip_bwrap_tests() -> bool { + match run_cmd_result_with_writable_roots( + &["bash", "-lc", "true"], + &[], + NETWORK_TIMEOUT_MS, + true, + ) + .await + { + Ok(output) => is_bwrap_unavailable_output(&output), + Err(CodexErr::Sandbox(SandboxErr::Denied { output })) => { + is_bwrap_unavailable_output(&output) + } + // Probe timeouts are not actionable for the bwrap-specific assertions below; + // skip rather than fail the whole suite. + Err(CodexErr::Sandbox(SandboxErr::Timeout { .. })) => true, + Err(err) => panic!("bwrap availability probe failed unexpectedly: {err:?}"), + } +} + +fn expect_denied( + result: Result, + context: &str, +) -> codex_core::exec::ExecToolCallOutput { + match result { + Ok(output) => { + assert_ne!(output.exit_code, 0, "{context}: expected nonzero exit code"); + output + } + Err(CodexErr::Sandbox(SandboxErr::Denied { output })) => *output, + Err(err) => panic!("{context}: {err:?}"), + } } #[tokio::test] @@ -192,6 +245,7 @@ async fn assert_network_blocked(cmd: &[&str]) { &sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, + false, None, ) .await; @@ -242,6 +296,90 @@ async fn sandbox_blocks_nc() { assert_network_blocked(&["nc", "-z", "127.0.0.1", "80"]).await; } +#[tokio::test] +async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: vendored bwrap was not built in this environment"); + return; + } + + let tmpdir = tempfile::tempdir().expect("tempdir"); + let dot_git = tmpdir.path().join(".git"); + let dot_codex = tmpdir.path().join(".codex"); + std::fs::create_dir_all(&dot_git).expect("create .git"); + std::fs::create_dir_all(&dot_codex).expect("create .codex"); + + let git_target = dot_git.join("config"); + let codex_target = dot_codex.join("config.toml"); + + let git_output = expect_denied( + run_cmd_result_with_writable_roots( + &[ + "bash", + "-lc", + &format!("echo denied > {}", git_target.to_string_lossy()), + ], + &[tmpdir.path().to_path_buf()], + LONG_TIMEOUT_MS, + true, + ) + .await, + ".git write should be denied under bubblewrap", + ); + + let codex_output = expect_denied( + run_cmd_result_with_writable_roots( + &[ + "bash", + "-lc", + &format!("echo denied > {}", codex_target.to_string_lossy()), + ], + &[tmpdir.path().to_path_buf()], + LONG_TIMEOUT_MS, + true, + ) + .await, + ".codex write should be denied under bubblewrap", + ); + assert_ne!(git_output.exit_code, 0); + assert_ne!(codex_output.exit_code, 0); +} + +#[tokio::test] +async fn sandbox_blocks_codex_symlink_replacement_attack() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: vendored bwrap was not built in this environment"); + return; + } + + use std::os::unix::fs::symlink; + + let tmpdir = tempfile::tempdir().expect("tempdir"); + let decoy = tmpdir.path().join("decoy-codex"); + std::fs::create_dir_all(&decoy).expect("create decoy dir"); + + let dot_codex = tmpdir.path().join(".codex"); + symlink(&decoy, &dot_codex).expect("create .codex symlink"); + + let codex_target = dot_codex.join("config.toml"); + + let codex_output = expect_denied( + run_cmd_result_with_writable_roots( + &[ + "bash", + "-lc", + &format!("echo denied > {}", codex_target.to_string_lossy()), + ], + &[tmpdir.path().to_path_buf()], + LONG_TIMEOUT_MS, + true, + ) + .await, + ".codex symlink replacement should be denied", + ); + assert_ne!(codex_output.exit_code, 0); +} + #[tokio::test] async fn sandbox_blocks_ssh() { // Force ssh to attempt a real TCP connection but fail quickly. `BatchMode`