From 1332fd5ebdb8707e4ae9df54c8b7825ecd550f1e Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 2 Feb 2026 13:06:22 -0800 Subject: [PATCH 01/14] feat(linux-sandbox): integrate vendored bwrap behind rollout flag --- .../app-server/src/codex_message_processor.rs | 2 + codex-rs/cli/src/debug_sandbox.rs | 3 + codex-rs/core/config.schema.json | 6 + codex-rs/core/src/codex.rs | 8 + codex-rs/core/src/connectors.rs | 1 + codex-rs/core/src/exec.rs | 15 +- codex-rs/core/src/features.rs | 8 + codex-rs/core/src/landlock.rs | 62 +++- codex-rs/core/src/mcp/mod.rs | 1 + codex-rs/core/src/mcp_connection_manager.rs | 2 + codex-rs/core/src/sandboxing/mod.rs | 37 +- codex-rs/core/src/tools/orchestrator.rs | 4 + codex-rs/core/src/tools/sandboxing.rs | 19 +- codex-rs/core/tests/suite/exec.rs | 2 +- codex-rs/docs/linux_sandbox.md | 59 +++ .../exec-server/src/posix/escalate_server.rs | 1 + codex-rs/exec-server/src/posix/mcp.rs | 1 + codex-rs/exec-server/tests/common/lib.rs | 2 + codex-rs/exec/tests/suite/sandbox.rs | 1 + codex-rs/linux-sandbox/Cargo.toml | 2 +- codex-rs/linux-sandbox/README.md | 25 ++ .../scripts/test_linux_sandbox.sh | 222 ++++++++++++ codex-rs/linux-sandbox/src/bwrap.rs | 39 -- codex-rs/linux-sandbox/src/landlock.rs | 29 +- codex-rs/linux-sandbox/src/linux_run_main.rs | 279 +++++++++----- codex-rs/linux-sandbox/src/mounts.rs | 339 ------------------ codex-rs/linux-sandbox/src/vendored_bwrap.rs | 30 +- .../linux-sandbox/tests/suite/landlock.rs | 102 +++++- 28 files changed, 792 insertions(+), 509 deletions(-) create mode 100644 codex-rs/docs/linux_sandbox.md create mode 100644 codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh delete mode 100644 codex-rs/linux-sandbox/src/mounts.rs diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 9e462ca4318d..99ad3fff041d 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1467,6 +1467,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( @@ -1474,6 +1475,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 13cb8cdd8633..af56977bd756 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/core/config.schema.json b/codex-rs/core/config.schema.json index 2315f75860f4..cc2eb8f786f9 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" }, @@ -1263,6 +1266,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 ba84b6b4321c..3584ea49846a 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -524,6 +524,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, @@ -725,6 +726,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(), @@ -995,6 +997,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; @@ -1244,6 +1247,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 @@ -2325,6 +2331,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; @@ -3198,6 +3205,7 @@ async fn spawn_review_thread( sub_id: sub_id.to_string(), client, 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 edf3c63aa5b4..afe03fbbe775 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 e0d65c83050a..3ac8d1956f88 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 12789f2c43c3..6047527a9d94 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. @@ -469,6 +471,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 340aebff2c14..13f270d619ad 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,51 @@ 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()); + linux_cmd.push("--use-vendored-bwrap".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 + ); + assert_eq!( + with_bwrap.contains(&"--use-vendored-bwrap".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 + ); + assert_eq!( + without_bwrap.contains(&"--use-vendored-bwrap".to_string()), + false + ); + } +} diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index cb0b5e2f262c..970fa5db401e 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 1a5ec559b8f9..def4f3169386 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 fca7adda2931..bc914abe5b1f 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 e9fdd6208be4..f67972bfef7b 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 a7d2bca62ac0..d50e59253009 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 cdf597a4e954..b70062d7ede4 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 ` re-enables writes for each writable root. +3. `--ro-bind ` re-applies read-only protections under + writable roots so protected paths win. +4. `--dev-bind /dev/null /dev/null` preserves the common sink. + +Writable roots and protected subpaths are derived from +`SandboxPolicy::get_writable_roots_with_cwd()`. + +Protected subpaths include: +- top-level `.git` (directory or pointer file), +- the resolved `gitdir:` target for worktrees and submodules, and +- top-level `.codex`. + +### Deny-path Hardening + +To reduce symlink and path-creation attacks inside writable roots: +- If any component of a protected path is a symlink within a writable root, the + helper mounts `/dev/null` on that symlink. +- If a protected path does not exist, the helper mounts `/dev/null` on the + first missing path component (when it is within a writable root). + +## Process and Network Semantics + +- In the bubblewrap pipeline, the helper isolates the PID namespace via + `--unshare-pid`. +- In the bubblewrap pipeline, it mounts a fresh `/proc` via `--proc /proc` by + default. +- In restrictive container environments, you can skip the `/proc` mount with + the helper flag `--no-proc` while still keeping PID isolation enabled. +- Network restrictions are enforced with seccomp when network access is + disabled. + +## Notes + +- The CLI still exposes legacy names such as `codex debug landlock`. +- Landlock remains the default filesystem enforcement pipeline when + `features.use_linux_sandbox_bwrap` is not enabled. diff --git a/codex-rs/exec-server/src/posix/escalate_server.rs b/codex-rs/exec-server/src/posix/escalate_server.rs index ef991ce8cd08..a2f247daaeb3 100644 --- a/codex-rs/exec-server/src/posix/escalate_server.rs +++ b/codex-rs/exec-server/src/posix/escalate_server.rs @@ -95,6 +95,7 @@ impl EscalateServer { &sandbox_state.sandbox_policy, &sandbox_state.sandbox_cwd, &sandbox_state.codex_linux_sandbox_exe, + sandbox_state.use_linux_sandbox_bwrap, None, ) .await?; diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/posix/mcp.rs index 620d332e71e5..e48b70a2a26d 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/posix/mcp.rs @@ -126,6 +126,7 @@ impl ExecTool { sandbox_policy: SandboxPolicy::ReadOnly, codex_linux_sandbox_exe: None, sandbox_cwd: PathBuf::from(¶ms.workdir), + use_linux_sandbox_bwrap: false, }); let escalate_server = EscalateServer::new( self.bash_path.clone(), diff --git a/codex-rs/exec-server/tests/common/lib.rs b/codex-rs/exec-server/tests/common/lib.rs index 562d3504f6ea..66bb4b7f9cb7 100644 --- a/codex-rs/exec-server/tests/common/lib.rs +++ b/codex-rs/exec-server/tests/common/lib.rs @@ -91,6 +91,7 @@ where sandbox_policy: SandboxPolicy::ReadOnly, codex_linux_sandbox_exe, sandbox_cwd: sandbox_cwd.as_ref().to_path_buf(), + use_linux_sandbox_bwrap: false, }; send_sandbox_state_update(sandbox_state, service).await } @@ -118,6 +119,7 @@ where }, codex_linux_sandbox_exe, sandbox_cwd: writable_folder.as_ref().to_path_buf(), + use_linux_sandbox_bwrap: false, }; send_sandbox_state_update(sandbox_state, service).await } diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index 303023b158b9..2d0ad42caa4f 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -50,6 +50,7 @@ async fn spawn_command_under_sandbox( command_cwd, sandbox_policy, sandbox_cwd, + false, stdio_policy, env, ) diff --git a/codex-rs/linux-sandbox/Cargo.toml b/codex-rs/linux-sandbox/Cargo.toml index 3feb1eb7b860..f937448f6b3b 100644 --- a/codex-rs/linux-sandbox/Cargo.toml +++ b/codex-rs/linux-sandbox/Cargo.toml @@ -23,7 +23,7 @@ landlock = { workspace = true } libc = { workspace = true } seccompiler = { workspace = true } serde_json = { workspace = true } -which = "8.0.0" +which = { workspace = true } [target.'cfg(target_os = "linux")'.dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 676f23495410..6e1aa9d86720 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -6,3 +6,28 @@ This crate is responsible for producing: - a lib crate that exposes the business logic of the executable as `run_main()` so that - the `codex-exec` CLI can check if its arg0 is `codex-linux-sandbox` and, if so, execute as if it were `codex-linux-sandbox` - this should also be true of the `codex` multitool CLI + +On Linux, the bubblewrap pipeline uses the vendored bubblewrap path compiled +into this binary. + +**Current Behavior** +- Legacy Landlock + mount protections remain available as the legacy pipeline. +- The bubblewrap pipeline is standardized on the vendored path. +- During rollout, the bubblewrap pipeline is gated by the temporary feature + flag `features.use_linux_sandbox_bwrap` (legacy remains default when off). +- When enabled, the bubblewrap pipeline applies `PR_SET_NO_NEW_PRIVS` and a + seccomp network filter in-process. +- When enabled, the filesystem is read-only by default via `--ro-bind / /`. +- When enabled, writable roots are layered with `--bind `. +- 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`. +- See `docs/linux_sandbox.md` for the full Linux sandbox semantics. diff --git a/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh b/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh new file mode 100644 index 000000000000..be15df8987a0 --- /dev/null +++ b/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh @@ -0,0 +1,222 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Linux sandbox smoke test script. +# +# This is designed for Linux devboxes where bwrap is available. It builds the +# codex-linux-sandbox binary and runs a small matrix of behavior checks: +# - workspace writes succeed +# - protected paths (.git, .codex) remain read-only +# - writes outside allowed roots fail +# - network_access=false blocks outbound sockets +# +# Usage: +# codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh +# +# Optional env vars: +# CODEX_BWRAP_ENABLE_FFI=1 # default: 1 (build vendored bwrap path) +# CODEX_LINUX_SANDBOX_NO_PROC=0 # default: 0 (let helper auto-retry with --no-proc) +# CODEX_LINUX_SANDBOX_DEBUG=1 # default: 0 (pass debug env var through) +# CODEX_LINUX_SANDBOX_USE_BWRAP=1 # default: 1 (run the bwrap suite) +# CODEX_LINUX_SANDBOX_USE_LEGACY=1 # default: 0 (run the legacy suite) +# CODEX_LINUX_SANDBOX_USE_CODEX_CLI=1 # default: 1 (run codex CLI bwrap smoke) + +if [[ "$(uname -s)" != "Linux" ]]; then + echo "This script is intended to run on Linux." >&2 + exit 1 +fi + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/../../.." && pwd)" +CODEX_RS_DIR="${REPO_ROOT}/codex-rs" + +BWRAP_ENABLE_FFI="${CODEX_BWRAP_ENABLE_FFI:-1}" +NO_PROC="${CODEX_LINUX_SANDBOX_NO_PROC:-0}" +DEBUG="${CODEX_LINUX_SANDBOX_DEBUG:-0}" +USE_BWRAP_SUITE="${CODEX_LINUX_SANDBOX_USE_BWRAP:-1}" +USE_LEGACY_SUITE="${CODEX_LINUX_SANDBOX_USE_LEGACY:-0}" +USE_CODEX_CLI_SMOKE="${CODEX_LINUX_SANDBOX_USE_CODEX_CLI:-1}" + +SANDBOX_BIN="${CODEX_RS_DIR}/target/debug/codex-linux-sandbox" +CODEX_BIN="${CODEX_RS_DIR}/target/debug/codex" +tmp_root="" + +build_binaries() { + echo "==> Building codex-linux-sandbox" + ( + cd "${CODEX_RS_DIR}" && \ + CODEX_BWRAP_ENABLE_FFI="${BWRAP_ENABLE_FFI}" cargo build -p codex-linux-sandbox >/dev/null + ) + + if [[ "${USE_CODEX_CLI_SMOKE}" == "1" ]]; then + echo "==> Building codex (local target/debug/codex)" + ( + cd "${CODEX_RS_DIR}" && \ + CODEX_BWRAP_ENABLE_FFI="${BWRAP_ENABLE_FFI}" cargo build -p codex-cli >/dev/null + ) + fi +} + +policy_json() { + local network_access="$1" + printf '{"type":"workspace-write","writable_roots":[],"network_access":%s}' "${network_access}" +} + +run_sandbox() { + local network_access="$1" + local use_bwrap="$2" + shift + shift + + local no_proc_flag=() + if [[ "${NO_PROC}" == "1" && "${use_bwrap}" == "1" ]]; then + no_proc_flag=(--no-proc) + fi + + local debug_env=() + if [[ "${DEBUG}" == "1" ]]; then + debug_env=(env CODEX_LINUX_SANDBOX_DEBUG=1) + fi + + local bwrap_flag=() + if [[ "${use_bwrap}" == "1" ]]; then + bwrap_flag=(--use-bwrap-sandbox --use-vendored-bwrap) + fi + + "${debug_env[@]}" "${SANDBOX_BIN}" \ + --sandbox-policy-cwd "${REPO_ROOT}" \ + --sandbox-policy "$(policy_json "${network_access}")" \ + "${bwrap_flag[@]}" \ + "${no_proc_flag[@]}" \ + -- "$@" +} + +expect_success() { + local label="$1" + local network_access="$2" + local use_bwrap="$3" + shift + shift + shift + echo "==> ${label}" + if run_sandbox "${network_access}" "${use_bwrap}" "$@"; then + echo " PASS" + else + echo " FAIL (expected success)" >&2 + exit 1 + fi +} + +expect_failure() { + local label="$1" + local network_access="$2" + local use_bwrap="$3" + shift + shift + shift + echo "==> ${label}" + if run_sandbox "${network_access}" "${use_bwrap}" "$@"; then + echo " FAIL (expected failure)" >&2 + exit 1 + else + echo " PASS (failed as expected)" + fi +} + +run_suite() { + local suite_name="$1" + local use_bwrap="$2" + + echo + echo "==== Suite: ${suite_name} (use_bwrap=${use_bwrap}) ====" + + # Create a disposable writable root for workspace-write checks. + if [[ -n "${tmp_root:-}" ]]; then + rm -rf -- "${tmp_root}" + fi + tmp_root="$(mktemp -d "${REPO_ROOT}/.codex-sandbox-test.XXXXXX")" + trap 'rm -rf -- "${tmp_root:-}"' EXIT + + mkdir -p "${REPO_ROOT}/.codex" + + expect_success \ + "workspace write succeeds inside repo" \ + true \ + "${use_bwrap}" \ + /usr/bin/bash -lc "cd '${tmp_root}' && touch OK_IN_WORKSPACE" + + expect_failure \ + "writes outside allowed roots fail" \ + true \ + "${use_bwrap}" \ + /usr/bin/bash -lc "touch /etc/SHOULD_FAIL" + + # Only the bwrap suite enforces `.git` and `.codex` as read-only. + if [[ "${use_bwrap}" == "1" ]]; then + expect_failure \ + ".git and .codex remain read-only (bwrap)" \ + true \ + "${use_bwrap}" \ + /usr/bin/bash -lc "cd '${REPO_ROOT}' && touch .git/SHOULD_FAIL && touch .codex/SHOULD_FAIL" + else + expect_success \ + ".git and .codex are NOT protected in legacy landlock path" \ + true \ + "${use_bwrap}" \ + /usr/bin/bash -lc "cd '${REPO_ROOT}' && mkdir -p .codex && touch .git/SHOULD_SUCCEED && touch .codex/SHOULD_SUCCEED" + fi + + expect_failure \ + "network_access=false blocks outbound sockets" \ + false \ + "${use_bwrap}" \ + /usr/bin/bash -lc "exec 3<>/dev/tcp/1.1.1.1/443" +} + +run_codex_cli_smoke() { + if [[ "${USE_CODEX_CLI_SMOKE}" != "1" ]]; then + return + fi + + echo + echo "==== codex CLI smoke (feature flag path) ====" + echo "==> codex -c features.use_linux_sandbox_bwrap=true sandbox linux ..." + + local output="" + if ! output=$( + "${CODEX_BIN}" \ + -c features.use_linux_sandbox_bwrap=true \ + sandbox linux --full-auto -- /usr/bin/bash -lc 'echo BWRAP_OK' 2>&1 + ); then + echo "${output}" >&2 + echo " FAIL (expected codex CLI bwrap smoke success)" >&2 + exit 1 + fi + + if [[ "${output}" != *"BWRAP_OK"* ]]; then + echo "${output}" >&2 + echo " FAIL (missing BWRAP_OK output)" >&2 + exit 1 + fi + + echo "${output}" + echo " PASS" +} + +main() { + build_binaries + run_codex_cli_smoke + + if [[ "${USE_BWRAP_SUITE}" == "1" ]]; then + run_suite "bwrap opt-in" "1" + fi + + if [[ "${USE_LEGACY_SUITE}" == "1" ]]; then + run_suite "legacy default" "0" + fi + + echo + echo "All requested linux-sandbox suites passed." +} + +main "$@" diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index c1e0732e0814..4a60c65dd3b8 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 772176fc9017..485301179213 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 3f6cd3ac86e4..f23c7a867ff3 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,12 +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. @@ -72,13 +69,12 @@ 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; + let use_bwrap_sandbox = use_bwrap_sandbox || use_vendored_bwrap; if command.is_empty() { panic!("No command specified to execute."); @@ -87,74 +83,208 @@ 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; - 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); + 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()) +} + +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() +} + +fn run_bwrap_in_child_capture_stderr(argv: Vec) -> String { + 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 { + libc::close(read_fd); + 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}"); + } + libc::close(write_fd); } - 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. + unsafe { + libc::close(write_fd); + } + + let mut stderr = String::new(); + // SAFETY: `read_fd` is a valid owned fd in the parent. + let mut read_file = unsafe { File::from_raw_fd(read_fd) }; + if let Err(err) = read_file.read_to_string(&mut stderr) { + 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}"); + } + + stderr +} + +fn is_proc_mount_failure(stderr: &str) -> bool { + stderr.contains("Can't mount proc") + && stderr.contains("/newroot/proc") + && stderr.contains("Invalid argument") +} + +#[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); + } + + #[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); + } + + #[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 }, + ); + let command_separator_index = argv + .iter() + .position(|arg| arg == "--") + .expect("expected command separator in bubblewrap argv"); + let argv0_flag_index = argv + .iter() + .position(|arg| arg == "--argv0") + .expect("expected --argv0 in bubblewrap argv"); + + assert_eq!(argv[0], "bwrap"); + assert_eq!(argv[argv0_flag_index + 1], "codex-linux-sandbox"); + assert_eq!(argv0_flag_index < command_separator_index, true); + } } /// Build the inner command that applies seccomp after bubblewrap. @@ -162,7 +292,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 +314,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 @@ -216,33 +341,3 @@ fn exec_or_panic(command: Vec) -> ! { let err = std::io::Error::last_os_error(); 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() - ); - } - if which::which("bwrap").is_ok() { - return; - } - - 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." - ); -} diff --git a/codex-rs/linux-sandbox/src/mounts.rs b/codex-rs/linux-sandbox/src/mounts.rs deleted file mode 100644 index 04f3651dfffe..000000000000 --- 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 ab4fb959ef23..c2816061ea9c 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 84f89a167822..171b3fd0e7af 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; @@ -47,12 +48,23 @@ 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(cmd, writable_roots, timeout_ms) + .await + .expect("sandboxed command should succeed") +} + +#[expect(clippy::expect_used)] +async fn run_cmd_result( + cmd: &[&str], + writable_roots: &[PathBuf], + timeout_ms: u64, +) -> Result { let cwd = std::env::current_dir().expect("cwd should exist"); let sandbox_cwd = cwd.clone(); let params = ExecParams { @@ -86,10 +98,24 @@ async fn run_cmd_output( &sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, + false, None, ) .await - .unwrap() +} + +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 +218,7 @@ async fn assert_network_blocked(cmd: &[&str]) { &sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, + false, None, ) .await; @@ -242,6 +269,77 @@ 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() { + 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( + &[ + "bash", + "-lc", + &format!("echo denied > {}", git_target.to_string_lossy()), + ], + &[tmpdir.path().to_path_buf()], + LONG_TIMEOUT_MS, + ) + .await, + ".git write should be denied under bubblewrap", + ); + + let codex_output = expect_denied( + run_cmd_result( + &[ + "bash", + "-lc", + &format!("echo denied > {}", codex_target.to_string_lossy()), + ], + &[tmpdir.path().to_path_buf()], + LONG_TIMEOUT_MS, + ) + .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() { + 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( + &[ + "bash", + "-lc", + &format!("echo denied > {}", codex_target.to_string_lossy()), + ], + &[tmpdir.path().to_path_buf()], + LONG_TIMEOUT_MS, + ) + .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` From b536599e6986991932f155502a3767c9b399fd23 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 2 Feb 2026 19:02:27 -0800 Subject: [PATCH 02/14] fix(linux-sandbox): satisfy clippy module order and shear deps --- codex-rs/Cargo.lock | 1 - codex-rs/linux-sandbox/Cargo.toml | 1 - codex-rs/linux-sandbox/src/linux_run_main.rs | 82 ++++++++++---------- 3 files changed, 41 insertions(+), 43 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 40a95cc3337b..cb2b3b0bd443 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1668,7 +1668,6 @@ dependencies = [ "serde_json", "tempfile", "tokio", - "which", ] [[package]] diff --git a/codex-rs/linux-sandbox/Cargo.toml b/codex-rs/linux-sandbox/Cargo.toml index f937448f6b3b..ff16a569fe06 100644 --- a/codex-rs/linux-sandbox/Cargo.toml +++ b/codex-rs/linux-sandbox/Cargo.toml @@ -23,7 +23,6 @@ landlock = { workspace = true } libc = { workspace = true } seccompiler = { workspace = true } serde_json = { workspace = true } -which = { workspace = true } [target.'cfg(target_os = "linux")'.dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index f23c7a867ff3..a477f19e509b 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -246,47 +246,6 @@ fn is_proc_mount_failure(stderr: &str) -> bool { && stderr.contains("Invalid argument") } -#[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); - } - - #[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); - } - - #[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 }, - ); - let command_separator_index = argv - .iter() - .position(|arg| arg == "--") - .expect("expected command separator in bubblewrap argv"); - let argv0_flag_index = argv - .iter() - .position(|arg| arg == "--argv0") - .expect("expected --argv0 in bubblewrap argv"); - - assert_eq!(argv[0], "bwrap"); - assert_eq!(argv[argv0_flag_index + 1], "codex-linux-sandbox"); - assert_eq!(argv0_flag_index < command_separator_index, true); - } -} - /// Build the inner command that applies seccomp after bubblewrap. fn build_inner_seccomp_command( sandbox_policy_cwd: &Path, @@ -341,3 +300,44 @@ fn exec_or_panic(command: Vec) -> ! { let err = std::io::Error::last_os_error(); panic!("Failed to execvp {}: {err}", command[0].as_str()); } + +#[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); + } + + #[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); + } + + #[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 }, + ); + let command_separator_index = argv + .iter() + .position(|arg| arg == "--") + .expect("expected command separator in bubblewrap argv"); + let argv0_flag_index = argv + .iter() + .position(|arg| arg == "--argv0") + .expect("expected --argv0 in bubblewrap argv"); + + assert_eq!(argv[0], "bwrap"); + assert_eq!(argv[argv0_flag_index + 1], "codex-linux-sandbox"); + assert_eq!(argv0_flag_index < command_separator_index, true); + } +} From 273c3055c24908e5f42631f377f742c8eb580def Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 2 Feb 2026 20:01:28 -0800 Subject: [PATCH 03/14] fix(linux-sandbox): run landlock suite via bwrap path --- codex-rs/linux-sandbox/tests/suite/landlock.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index 171b3fd0e7af..6187ae55f6ee 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -98,7 +98,7 @@ async fn run_cmd_result( &sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, - false, + true, None, ) .await @@ -218,7 +218,7 @@ async fn assert_network_blocked(cmd: &[&str]) { &sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, - false, + true, None, ) .await; From 1ae8d7f543d69b4e9949f6de53f5eb1246bd34f2 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 2 Feb 2026 20:10:19 -0800 Subject: [PATCH 04/14] fix(linux-sandbox): split legacy vs bwrap test paths --- .../linux-sandbox/tests/suite/landlock.rs | 53 ++++++++++++++++--- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index 6187ae55f6ee..fe69c754d8f1 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -33,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) @@ -59,11 +61,20 @@ async fn run_cmd_output( .expect("sandboxed command should succeed") } -#[expect(clippy::expect_used)] async fn run_cmd_result( cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64, +) -> Result { + run_cmd_result_with_bwrap(cmd, writable_roots, timeout_ms, false).await +} + +#[expect(clippy::expect_used)] +async fn run_cmd_result_with_bwrap( + 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(); @@ -98,12 +109,29 @@ async fn run_cmd_result( &sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, - true, + use_bwrap_sandbox, None, ) .await } +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_bwrap(&["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, @@ -218,7 +246,7 @@ async fn assert_network_blocked(cmd: &[&str]) { &sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, - true, + false, None, ) .await; @@ -271,6 +299,11 @@ async fn sandbox_blocks_nc() { #[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"); @@ -281,7 +314,7 @@ async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { let codex_target = dot_codex.join("config.toml"); let git_output = expect_denied( - run_cmd_result( + run_cmd_result_with_bwrap( &[ "bash", "-lc", @@ -289,13 +322,14 @@ async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { ], &[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( + run_cmd_result_with_bwrap( &[ "bash", "-lc", @@ -303,6 +337,7 @@ async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { ], &[tmpdir.path().to_path_buf()], LONG_TIMEOUT_MS, + true, ) .await, ".codex write should be denied under bubblewrap", @@ -313,6 +348,11 @@ async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { #[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"); @@ -325,7 +365,7 @@ async fn sandbox_blocks_codex_symlink_replacement_attack() { let codex_target = dot_codex.join("config.toml"); let codex_output = expect_denied( - run_cmd_result( + run_cmd_result_with_bwrap( &[ "bash", "-lc", @@ -333,6 +373,7 @@ async fn sandbox_blocks_codex_symlink_replacement_attack() { ], &[tmpdir.path().to_path_buf()], LONG_TIMEOUT_MS, + true, ) .await, ".codex symlink replacement should be denied", From a121b2518719f92a7cabf0297ffcfd9472714b12 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Feb 2026 00:13:39 -0800 Subject: [PATCH 05/14] ci(linux-sandbox): enable bwrap ffi in linux CI/release --- .github/workflows/bazel.yml | 9 +++++++++ .github/workflows/rust-ci.yml | 16 ++++++++++++++++ .github/workflows/rust-release.yml | 9 +++++++++ 3 files changed, 34 insertions(+) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index 55e05251d753..f2ca0a15542e 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -65,6 +65,14 @@ jobs: - name: Set up Bazel uses: bazelbuild/setup-bazelisk@v3 + - 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 + # TODO(mbolin): Bring this back once we have caching working. Currently, # we never seem to get a cache hit but we still end up paying the cost of # uploading at the end of the build, which takes over a minute! @@ -100,6 +108,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 ec6319f8dabd..752ec3282c22 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -99,6 +99,7 @@ jobs: USE_SCCACHE: ${{ startsWith(matrix.runner, 'windows') && 'false' || 'true' }} CARGO_INCREMENTAL: "0" SCCACHE_CACHE_SIZE: 10G + CODEX_BWRAP_ENABLE_FFI: ${{ contains(matrix.target, 'unknown-linux') && '1' || '0' }} strategy: fail-fast: false @@ -175,6 +176,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 @@ -467,6 +475,7 @@ jobs: USE_SCCACHE: ${{ startsWith(matrix.runner, 'windows') && 'false' || 'true' }} CARGO_INCREMENTAL: "0" SCCACHE_CACHE_SIZE: 10G + CODEX_BWRAP_ENABLE_FFI: ${{ contains(matrix.target, 'unknown-linux') && '1' || '0' }} strategy: fail-fast: false @@ -502,6 +511,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 # Some integration tests rely on DotSlash being installed. # See https://github.com/openai/codex/pull/7617. diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 2f764df6802b..aa92693eee74 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 From 6b9e24e5675c724be3ded43156d5d139ad61d2c2 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Feb 2026 09:16:09 -0800 Subject: [PATCH 06/14] refactor(linux-sandbox): standardize bwrap rollout flags --- codex-rs/common/src/config_override.rs | 20 ++++++++++++++++++- codex-rs/core/src/landlock.rs | 9 --------- codex-rs/docs/linux_sandbox.md | 3 ++- codex-rs/linux-sandbox/README.md | 3 ++- .../scripts/test_linux_sandbox.sh | 6 +++--- codex-rs/linux-sandbox/src/linux_run_main.rs | 8 -------- 6 files changed, 26 insertions(+), 23 deletions(-) diff --git a/codex-rs/common/src/config_override.rs b/codex-rs/common/src/config_override.rs index 59dde92a22b6..9bbec2b6f269 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/src/landlock.rs b/codex-rs/core/src/landlock.rs index 13f270d619ad..ea27f77f75e2 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -75,7 +75,6 @@ pub(crate) fn create_linux_sandbox_command_args( ]; if use_bwrap_sandbox { linux_cmd.push("--use-bwrap-sandbox".to_string()); - linux_cmd.push("--use-vendored-bwrap".to_string()); } // Separator so that command arguments starting with `-` are not parsed as @@ -104,19 +103,11 @@ mod tests { with_bwrap.contains(&"--use-bwrap-sandbox".to_string()), true ); - assert_eq!( - with_bwrap.contains(&"--use-vendored-bwrap".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 ); - assert_eq!( - without_bwrap.contains(&"--use-vendored-bwrap".to_string()), - false - ); } } diff --git a/codex-rs/docs/linux_sandbox.md b/codex-rs/docs/linux_sandbox.md index c665a6513267..be9c655375f0 100644 --- a/codex-rs/docs/linux_sandbox.md +++ b/codex-rs/docs/linux_sandbox.md @@ -11,7 +11,8 @@ pipelines: - The bubblewrap pipeline requires a Linux build that enables the vendored bubblewrap FFI path. - During rollout, enable bubblewrap with - `features.use_linux_sandbox_bwrap = true`. + `-c use_linux_sandbox_bwrap=true` (CLI override alias) or + `features.use_linux_sandbox_bwrap = true` in config. ## Filesystem Semantics (bubblewrap pipeline) diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 6e1aa9d86720..13298c13feb6 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -14,7 +14,8 @@ into this binary. - Legacy Landlock + mount protections remain available as the legacy pipeline. - The bubblewrap pipeline is standardized on the vendored path. - During rollout, the bubblewrap pipeline is gated by the temporary feature - flag `features.use_linux_sandbox_bwrap` (legacy remains default when off). + flag `use_linux_sandbox_bwrap` (CLI `-c` alias for + `features.use_linux_sandbox_bwrap`; legacy remains default when off). - When enabled, the bubblewrap pipeline applies `PR_SET_NO_NEW_PRIVS` and a seccomp network filter in-process. - When enabled, the filesystem is read-only by default via `--ro-bind / /`. diff --git a/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh b/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh index be15df8987a0..0bcaebecbe43 100644 --- a/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh +++ b/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh @@ -80,7 +80,7 @@ run_sandbox() { local bwrap_flag=() if [[ "${use_bwrap}" == "1" ]]; then - bwrap_flag=(--use-bwrap-sandbox --use-vendored-bwrap) + bwrap_flag=(--use-bwrap-sandbox) fi "${debug_env[@]}" "${SANDBOX_BIN}" \ @@ -180,12 +180,12 @@ run_codex_cli_smoke() { echo echo "==== codex CLI smoke (feature flag path) ====" - echo "==> codex -c features.use_linux_sandbox_bwrap=true sandbox linux ..." + echo "==> codex -c use_linux_sandbox_bwrap=true sandbox linux ..." local output="" if ! output=$( "${CODEX_BIN}" \ - -c features.use_linux_sandbox_bwrap=true \ + -c use_linux_sandbox_bwrap=true \ sandbox linux --full-auto -- /usr/bin/bash -lc 'echo BWRAP_OK' 2>&1 ); then echo "${output}" >&2 diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index a477f19e509b..1784f186bee5 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -32,12 +32,6 @@ pub struct LandlockCommand { #[arg(long = "use-bwrap-sandbox", hide = true, default_value_t = false)] pub use_bwrap_sandbox: bool, - /// 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. /// @@ -69,12 +63,10 @@ pub fn run_main() -> ! { sandbox_policy_cwd, sandbox_policy, use_bwrap_sandbox, - use_vendored_bwrap, apply_seccomp_then_exec, no_proc, command, } = LandlockCommand::parse(); - let use_bwrap_sandbox = use_bwrap_sandbox || use_vendored_bwrap; if command.is_empty() { panic!("No command specified to execute."); From 4689e773b7707b897dea4ffd217be2172d462460 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Feb 2026 11:16:51 -0800 Subject: [PATCH 07/14] ci(bazel): remove local bwrap deps install from remote bazel workflow --- .github/workflows/bazel.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index f2ca0a15542e..b5e91d7f22ef 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -65,14 +65,6 @@ jobs: - name: Set up Bazel uses: bazelbuild/setup-bazelisk@v3 - - 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 - # TODO(mbolin): Bring this back once we have caching working. Currently, # we never seem to get a cache hit but we still end up paying the cost of # uploading at the end of the build, which takes over a minute! From f010435799aecc2d95f01aba7f9d6df5e735c609 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Feb 2026 14:08:46 -0800 Subject: [PATCH 08/14] ci(rust-ci): keep bwrap ffi out of cargo workflow --- .github/workflows/rust-ci.yml | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 752ec3282c22..b6a3c50e1884 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -99,7 +99,9 @@ jobs: USE_SCCACHE: ${{ startsWith(matrix.runner, 'windows') && 'false' || 'true' }} CARGO_INCREMENTAL: "0" SCCACHE_CACHE_SIZE: 10G - CODEX_BWRAP_ENABLE_FFI: ${{ contains(matrix.target, 'unknown-linux') && '1' || '0' }} + # 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 @@ -176,13 +178,6 @@ 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 @@ -475,7 +470,9 @@ jobs: USE_SCCACHE: ${{ startsWith(matrix.runner, 'windows') && 'false' || 'true' }} CARGO_INCREMENTAL: "0" SCCACHE_CACHE_SIZE: 10G - CODEX_BWRAP_ENABLE_FFI: ${{ contains(matrix.target, 'unknown-linux') && '1' || '0' }} + # 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 @@ -511,14 +508,6 @@ 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 - # Some integration tests rely on DotSlash being installed. # See https://github.com/openai/codex/pull/7617. - name: Install DotSlash From 17c46b31d737a0b272e0dc33d9642a9b0feeb023 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Feb 2026 16:56:05 -0800 Subject: [PATCH 09/14] fix(linux-sandbox): bound preflight bwrap stderr capture --- codex-rs/linux-sandbox/src/linux_run_main.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 1784f186bee5..26bee82b498d 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -179,7 +179,20 @@ fn resolve_true_command() -> String { "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 { @@ -215,10 +228,11 @@ fn run_bwrap_in_child_capture_stderr(argv: Vec) -> String { libc::close(write_fd); } - let mut stderr = String::new(); // SAFETY: `read_fd` is a valid owned fd in the parent. let mut read_file = unsafe { File::from_raw_fd(read_fd) }; - if let Err(err) = read_file.read_to_string(&mut stderr) { + 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}"); } @@ -229,7 +243,7 @@ fn run_bwrap_in_child_capture_stderr(argv: Vec) -> String { panic!("waitpid failed for bubblewrap child: {err}"); } - stderr + String::from_utf8_lossy(&stderr_bytes).into_owned() } fn is_proc_mount_failure(stderr: &str) -> bool { From b5f25ec84a24894dd8490ff3f19dcd9f47d70f3d Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Feb 2026 16:58:35 -0800 Subject: [PATCH 10/14] fix(linux-sandbox): check close rc in bwrap preflight --- codex-rs/linux-sandbox/src/linux_run_main.rs | 21 +++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 26bee82b498d..4cb908862539 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -211,12 +211,12 @@ fn run_bwrap_in_child_capture_stderr(argv: Vec) -> String { if pid == 0 { // Child: redirect stderr to the pipe, then run bubblewrap. unsafe { - libc::close(read_fd); + 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}"); } - libc::close(write_fd); + close_fd_or_panic(write_fd, "close write end in bubblewrap child"); } let exit_code = run_vendored_bwrap_main(&argv); @@ -224,9 +224,7 @@ fn run_bwrap_in_child_capture_stderr(argv: Vec) -> String { } // Parent: close the write end and read stderr while the child runs. - unsafe { - libc::close(write_fd); - } + 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) }; @@ -246,6 +244,19 @@ fn run_bwrap_in_child_capture_stderr(argv: Vec) -> String { 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") From 98acefe57386618021d575533b087878f35f2560 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Feb 2026 18:40:48 -0800 Subject: [PATCH 11/14] docs(linux-sandbox): tighten rollout behavior docs --- codex-rs/docs/linux_sandbox.md | 7 ++++-- codex-rs/linux-sandbox/README.md | 43 +++++++++++--------------------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/codex-rs/docs/linux_sandbox.md b/codex-rs/docs/linux_sandbox.md index be9c655375f0..61d39a1f1a35 100644 --- a/codex-rs/docs/linux_sandbox.md +++ b/codex-rs/docs/linux_sandbox.md @@ -48,8 +48,9 @@ To reduce symlink and path-creation attacks inside writable roots: `--unshare-pid`. - In the bubblewrap pipeline, it mounts a fresh `/proc` via `--proc /proc` by default. -- In restrictive container environments, you can skip the `/proc` mount with - the helper flag `--no-proc` while still keeping PID isolation enabled. +- In restrictive container environments, the helper preflights `/proc` mount + support and retries internally with `--no-proc` on the known mount-failure + pattern. You can also force this path explicitly with `--no-proc`. - Network restrictions are enforced with seccomp when network access is disabled. @@ -58,3 +59,5 @@ To reduce symlink and path-creation attacks inside writable roots: - The CLI still exposes legacy names such as `codex debug landlock`. - Landlock remains the default filesystem enforcement pipeline when `features.use_linux_sandbox_bwrap` is not enabled. +- When `features.use_linux_sandbox_bwrap` is enabled, bubblewrap errors do not + fall back to legacy Landlock. diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 13298c13feb6..513fd84d4820 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -1,34 +1,21 @@ # codex-linux-sandbox -This crate is responsible for producing: +Linux sandbox helper used by Codex process execution on Linux. -- a `codex-linux-sandbox` standalone executable for Linux that is bundled with the Node.js version of the Codex CLI -- a lib crate that exposes the business logic of the executable as `run_main()` so that - - the `codex-exec` CLI can check if its arg0 is `codex-linux-sandbox` and, if so, execute as if it were `codex-linux-sandbox` - - this should also be true of the `codex` multitool CLI +This crate produces: +- `codex-linux-sandbox` binary +- library entrypoint (`run_main`) reused by `codex` / `codex-exec` arg0 dispatch -On Linux, the bubblewrap pipeline uses the vendored bubblewrap path compiled -into this binary. +## Runtime behavior -**Current Behavior** -- Legacy Landlock + mount protections remain available as the legacy pipeline. -- The bubblewrap pipeline is standardized on the vendored path. -- During rollout, the bubblewrap pipeline is gated by the temporary feature - flag `use_linux_sandbox_bwrap` (CLI `-c` alias for - `features.use_linux_sandbox_bwrap`; legacy remains default when off). -- When enabled, the bubblewrap pipeline applies `PR_SET_NO_NEW_PRIVS` and a - seccomp network filter in-process. -- When enabled, the filesystem is read-only by default via `--ro-bind / /`. -- When enabled, writable roots are layered with `--bind `. -- 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`. +- Default path (flag off): legacy Landlock + mount protections. +- Rollout path (flag on `features.use_linux_sandbox_bwrap`): vendored bubblewrap + in-process seccomp. +- Bubblewrap mode uses read-only root (`--ro-bind / /`) and rebinds writable roots. +- Bubblewrap mode re-protects `.git`, resolved `gitdir:`, and `.codex` under writable roots. +- Bubblewrap mode preflights `/proc` mount and retries with `--no-proc` on known mount failure. +- Bubblewrap mode does not fall back to legacy Landlock on bwrap errors. -**Notes** -- The CLI surface still uses legacy names like `codex debug landlock`. -- See `docs/linux_sandbox.md` for the full Linux sandbox semantics. +## Notes + +- CLI/debug surfaces still include legacy names such as `codex debug landlock`. +- See `codex-rs/docs/linux_sandbox.md` for full policy semantics. From a19ab5eceec6ade37752eae486f88383461ea79b Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Feb 2026 19:38:15 -0800 Subject: [PATCH 12/14] docs(linux-sandbox): remove obsolete sandbox doc --- codex-rs/docs/linux_sandbox.md | 63 -------------------------------- codex-rs/linux-sandbox/README.md | 42 +++++++++++++-------- 2 files changed, 27 insertions(+), 78 deletions(-) delete mode 100644 codex-rs/docs/linux_sandbox.md diff --git a/codex-rs/docs/linux_sandbox.md b/codex-rs/docs/linux_sandbox.md deleted file mode 100644 index 61d39a1f1a35..000000000000 --- a/codex-rs/docs/linux_sandbox.md +++ /dev/null @@ -1,63 +0,0 @@ -# Linux Sandbox (Vendored bubblewrap, Landlock legacy) - -The Linux sandbox helper (`codex-linux-sandbox`) supports two filesystem -pipelines: -- Vendored bubblewrap (feature-flag opt-in): bubblewrap (`bwrap`) for - filesystem isolation plus seccomp. -- Legacy: mount-based protections plus Landlock enforcement. - -## Requirements - -- The bubblewrap pipeline requires a Linux build that enables the vendored - bubblewrap FFI path. -- During rollout, enable bubblewrap with - `-c use_linux_sandbox_bwrap=true` (CLI override alias) or - `features.use_linux_sandbox_bwrap = true` in config. - -## Filesystem Semantics (bubblewrap pipeline) - -When the bubblewrap pipeline is enabled and disk writes are restricted -(`read-only` or `workspace-write`), the helper builds the filesystem view with -bubblewrap in this order: - -1. `--ro-bind / /` makes the entire filesystem read-only. -2. `--bind ` re-enables writes for each writable root. -3. `--ro-bind ` re-applies read-only protections under - writable roots so protected paths win. -4. `--dev-bind /dev/null /dev/null` preserves the common sink. - -Writable roots and protected subpaths are derived from -`SandboxPolicy::get_writable_roots_with_cwd()`. - -Protected subpaths include: -- top-level `.git` (directory or pointer file), -- the resolved `gitdir:` target for worktrees and submodules, and -- top-level `.codex`. - -### Deny-path Hardening - -To reduce symlink and path-creation attacks inside writable roots: -- If any component of a protected path is a symlink within a writable root, the - helper mounts `/dev/null` on that symlink. -- If a protected path does not exist, the helper mounts `/dev/null` on the - first missing path component (when it is within a writable root). - -## Process and Network Semantics - -- In the bubblewrap pipeline, the helper isolates the PID namespace via - `--unshare-pid`. -- In the bubblewrap pipeline, it mounts a fresh `/proc` via `--proc /proc` by - default. -- In restrictive container environments, the helper preflights `/proc` mount - support and retries internally with `--no-proc` on the known mount-failure - pattern. You can also force this path explicitly with `--no-proc`. -- Network restrictions are enforced with seccomp when network access is - disabled. - -## Notes - -- The CLI still exposes legacy names such as `codex debug landlock`. -- Landlock remains the default filesystem enforcement pipeline when - `features.use_linux_sandbox_bwrap` is not enabled. -- When `features.use_linux_sandbox_bwrap` is enabled, bubblewrap errors do not - fall back to legacy Landlock. diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 513fd84d4820..f14fc5f13d88 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -1,21 +1,33 @@ # codex-linux-sandbox -Linux sandbox helper used by Codex process execution on Linux. +This crate is responsible for producing: -This crate produces: -- `codex-linux-sandbox` binary -- library entrypoint (`run_main`) reused by `codex` / `codex-exec` arg0 dispatch +- a `codex-linux-sandbox` standalone executable for Linux that is bundled with the Node.js version of the Codex CLI +- a lib crate that exposes the business logic of the executable as `run_main()` so that + - the `codex-exec` CLI can check if its arg0 is `codex-linux-sandbox` and, if so, execute as if it were `codex-linux-sandbox` + - this should also be true of the `codex` multitool CLI -## Runtime behavior +On Linux, the bubblewrap pipeline uses the vendored bubblewrap path compiled +into this binary. -- Default path (flag off): legacy Landlock + mount protections. -- Rollout path (flag on `features.use_linux_sandbox_bwrap`): vendored bubblewrap + in-process seccomp. -- Bubblewrap mode uses read-only root (`--ro-bind / /`) and rebinds writable roots. -- Bubblewrap mode re-protects `.git`, resolved `gitdir:`, and `.codex` under writable roots. -- Bubblewrap mode preflights `/proc` mount and retries with `--no-proc` on known mount failure. -- Bubblewrap mode does not fall back to legacy Landlock on bwrap errors. +**Current Behavior** +- Legacy Landlock + mount protections remain available as the legacy pipeline. +- The bubblewrap pipeline is standardized on the vendored path. +- During rollout, the bubblewrap pipeline is gated by the temporary feature + flag `use_linux_sandbox_bwrap` (CLI `-c` alias for + `features.use_linux_sandbox_bwrap`; legacy remains default when off). +- When enabled, the bubblewrap pipeline applies `PR_SET_NO_NEW_PRIVS` and a + seccomp network filter in-process. +- When enabled, the filesystem is read-only by default via `--ro-bind / /`. +- When enabled, writable roots are layered with `--bind `. +- 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 - -- CLI/debug surfaces still include legacy names such as `codex debug landlock`. -- See `codex-rs/docs/linux_sandbox.md` for full policy semantics. +**Notes** +- The CLI surface still uses legacy names like `codex debug landlock`. From 0ef1a48b9acf9e47553626fba41a217db7710133 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 4 Feb 2026 09:38:08 -0800 Subject: [PATCH 13/14] fix(linux-sandbox): address review feedback and remove smoke script --- AGENTS.md | 4 + .../scripts/test_linux_sandbox.sh | 222 ------------------ codex-rs/linux-sandbox/src/linux_run_main.rs | 33 ++- .../linux-sandbox/tests/suite/landlock.rs | 29 ++- 4 files changed, 39 insertions(+), 249 deletions(-) delete mode 100644 codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh diff --git a/AGENTS.md b/AGENTS.md index 77c48ddba0f6..7b9773afdc20 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,8 @@ In the codex-rs folder where the rust code lives: - Always inline format! args when possible per https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args - Use method references over closures when possible per https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls - When possible, make `match` statements exhaustive and avoid wildcard arms. +- Name helpers by actual behavior, not by optional implementation details behind flags. +- For wrappers around third-party `main`/`exec*` flows, docstrings must match real control flow and return semantics; do not claim non-returning behavior unless guaranteed. - When writing tests, prefer comparing the equality of entire objects over fields one by one. - When making a change that adds or changes an API, ensure that the documentation in the `docs/` folder is up to date if applicable. - If you change `ConfigToml` or nested config types, run `just write-config-schema` to update `codex-rs/core/config.schema.json`. @@ -78,6 +80,8 @@ If you don’t have the tool: - Tests should use pretty_assertions::assert_eq for clearer diffs. Import this at the top of the test module if it isn't already. - Prefer deep equals comparisons whenever possible. Perform `assert_eq!()` on entire objects, rather than individual fields. +- Keep assertion failure messages precise about guarantees: use wording like “executes” unless the assertion is specifically about successful (`0`) exit status. +- Avoid one-off passthrough helper functions in tests; inline single-use wrappers unless they improve readability materially. - Avoid mutating process environment in tests; prefer passing environment-derived flags or dependencies from above. ### Spawning workspace binaries in tests (Cargo vs Bazel) diff --git a/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh b/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh deleted file mode 100644 index 0bcaebecbe43..000000000000 --- a/codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh +++ /dev/null @@ -1,222 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# Linux sandbox smoke test script. -# -# This is designed for Linux devboxes where bwrap is available. It builds the -# codex-linux-sandbox binary and runs a small matrix of behavior checks: -# - workspace writes succeed -# - protected paths (.git, .codex) remain read-only -# - writes outside allowed roots fail -# - network_access=false blocks outbound sockets -# -# Usage: -# codex-rs/linux-sandbox/scripts/test_linux_sandbox.sh -# -# Optional env vars: -# CODEX_BWRAP_ENABLE_FFI=1 # default: 1 (build vendored bwrap path) -# CODEX_LINUX_SANDBOX_NO_PROC=0 # default: 0 (let helper auto-retry with --no-proc) -# CODEX_LINUX_SANDBOX_DEBUG=1 # default: 0 (pass debug env var through) -# CODEX_LINUX_SANDBOX_USE_BWRAP=1 # default: 1 (run the bwrap suite) -# CODEX_LINUX_SANDBOX_USE_LEGACY=1 # default: 0 (run the legacy suite) -# CODEX_LINUX_SANDBOX_USE_CODEX_CLI=1 # default: 1 (run codex CLI bwrap smoke) - -if [[ "$(uname -s)" != "Linux" ]]; then - echo "This script is intended to run on Linux." >&2 - exit 1 -fi - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -REPO_ROOT="$(cd "${SCRIPT_DIR}/../../.." && pwd)" -CODEX_RS_DIR="${REPO_ROOT}/codex-rs" - -BWRAP_ENABLE_FFI="${CODEX_BWRAP_ENABLE_FFI:-1}" -NO_PROC="${CODEX_LINUX_SANDBOX_NO_PROC:-0}" -DEBUG="${CODEX_LINUX_SANDBOX_DEBUG:-0}" -USE_BWRAP_SUITE="${CODEX_LINUX_SANDBOX_USE_BWRAP:-1}" -USE_LEGACY_SUITE="${CODEX_LINUX_SANDBOX_USE_LEGACY:-0}" -USE_CODEX_CLI_SMOKE="${CODEX_LINUX_SANDBOX_USE_CODEX_CLI:-1}" - -SANDBOX_BIN="${CODEX_RS_DIR}/target/debug/codex-linux-sandbox" -CODEX_BIN="${CODEX_RS_DIR}/target/debug/codex" -tmp_root="" - -build_binaries() { - echo "==> Building codex-linux-sandbox" - ( - cd "${CODEX_RS_DIR}" && \ - CODEX_BWRAP_ENABLE_FFI="${BWRAP_ENABLE_FFI}" cargo build -p codex-linux-sandbox >/dev/null - ) - - if [[ "${USE_CODEX_CLI_SMOKE}" == "1" ]]; then - echo "==> Building codex (local target/debug/codex)" - ( - cd "${CODEX_RS_DIR}" && \ - CODEX_BWRAP_ENABLE_FFI="${BWRAP_ENABLE_FFI}" cargo build -p codex-cli >/dev/null - ) - fi -} - -policy_json() { - local network_access="$1" - printf '{"type":"workspace-write","writable_roots":[],"network_access":%s}' "${network_access}" -} - -run_sandbox() { - local network_access="$1" - local use_bwrap="$2" - shift - shift - - local no_proc_flag=() - if [[ "${NO_PROC}" == "1" && "${use_bwrap}" == "1" ]]; then - no_proc_flag=(--no-proc) - fi - - local debug_env=() - if [[ "${DEBUG}" == "1" ]]; then - debug_env=(env CODEX_LINUX_SANDBOX_DEBUG=1) - fi - - local bwrap_flag=() - if [[ "${use_bwrap}" == "1" ]]; then - bwrap_flag=(--use-bwrap-sandbox) - fi - - "${debug_env[@]}" "${SANDBOX_BIN}" \ - --sandbox-policy-cwd "${REPO_ROOT}" \ - --sandbox-policy "$(policy_json "${network_access}")" \ - "${bwrap_flag[@]}" \ - "${no_proc_flag[@]}" \ - -- "$@" -} - -expect_success() { - local label="$1" - local network_access="$2" - local use_bwrap="$3" - shift - shift - shift - echo "==> ${label}" - if run_sandbox "${network_access}" "${use_bwrap}" "$@"; then - echo " PASS" - else - echo " FAIL (expected success)" >&2 - exit 1 - fi -} - -expect_failure() { - local label="$1" - local network_access="$2" - local use_bwrap="$3" - shift - shift - shift - echo "==> ${label}" - if run_sandbox "${network_access}" "${use_bwrap}" "$@"; then - echo " FAIL (expected failure)" >&2 - exit 1 - else - echo " PASS (failed as expected)" - fi -} - -run_suite() { - local suite_name="$1" - local use_bwrap="$2" - - echo - echo "==== Suite: ${suite_name} (use_bwrap=${use_bwrap}) ====" - - # Create a disposable writable root for workspace-write checks. - if [[ -n "${tmp_root:-}" ]]; then - rm -rf -- "${tmp_root}" - fi - tmp_root="$(mktemp -d "${REPO_ROOT}/.codex-sandbox-test.XXXXXX")" - trap 'rm -rf -- "${tmp_root:-}"' EXIT - - mkdir -p "${REPO_ROOT}/.codex" - - expect_success \ - "workspace write succeeds inside repo" \ - true \ - "${use_bwrap}" \ - /usr/bin/bash -lc "cd '${tmp_root}' && touch OK_IN_WORKSPACE" - - expect_failure \ - "writes outside allowed roots fail" \ - true \ - "${use_bwrap}" \ - /usr/bin/bash -lc "touch /etc/SHOULD_FAIL" - - # Only the bwrap suite enforces `.git` and `.codex` as read-only. - if [[ "${use_bwrap}" == "1" ]]; then - expect_failure \ - ".git and .codex remain read-only (bwrap)" \ - true \ - "${use_bwrap}" \ - /usr/bin/bash -lc "cd '${REPO_ROOT}' && touch .git/SHOULD_FAIL && touch .codex/SHOULD_FAIL" - else - expect_success \ - ".git and .codex are NOT protected in legacy landlock path" \ - true \ - "${use_bwrap}" \ - /usr/bin/bash -lc "cd '${REPO_ROOT}' && mkdir -p .codex && touch .git/SHOULD_SUCCEED && touch .codex/SHOULD_SUCCEED" - fi - - expect_failure \ - "network_access=false blocks outbound sockets" \ - false \ - "${use_bwrap}" \ - /usr/bin/bash -lc "exec 3<>/dev/tcp/1.1.1.1/443" -} - -run_codex_cli_smoke() { - if [[ "${USE_CODEX_CLI_SMOKE}" != "1" ]]; then - return - fi - - echo - echo "==== codex CLI smoke (feature flag path) ====" - echo "==> codex -c use_linux_sandbox_bwrap=true sandbox linux ..." - - local output="" - if ! output=$( - "${CODEX_BIN}" \ - -c use_linux_sandbox_bwrap=true \ - sandbox linux --full-auto -- /usr/bin/bash -lc 'echo BWRAP_OK' 2>&1 - ); then - echo "${output}" >&2 - echo " FAIL (expected codex CLI bwrap smoke success)" >&2 - exit 1 - fi - - if [[ "${output}" != *"BWRAP_OK"* ]]; then - echo "${output}" >&2 - echo " FAIL (missing BWRAP_OK output)" >&2 - exit 1 - fi - - echo "${output}" - echo " PASS" -} - -main() { - build_binaries - run_codex_cli_smoke - - if [[ "${USE_BWRAP_SUITE}" == "1" ]]; then - run_suite "bwrap opt-in" "1" - fi - - if [[ "${USE_LEGACY_SUITE}" == "1" ]]; then - run_suite "legacy default" "0" - fi - - echo - echo "All requested linux-sandbox suites passed." -} - -main "$@" diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 4cb908862539..f5f0d9887aaf 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -344,17 +344,26 @@ mod tests { Path::new("/"), BwrapOptions { mount_proc: true }, ); - let command_separator_index = argv - .iter() - .position(|arg| arg == "--") - .expect("expected command separator in bubblewrap argv"); - let argv0_flag_index = argv - .iter() - .position(|arg| arg == "--argv0") - .expect("expected --argv0 in bubblewrap argv"); - - assert_eq!(argv[0], "bwrap"); - assert_eq!(argv[argv0_flag_index + 1], "codex-linux-sandbox"); - assert_eq!(argv0_flag_index < command_separator_index, 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/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index a859fd1e75a4..52cd402a134c 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -56,21 +56,13 @@ async fn run_cmd_output( writable_roots: &[PathBuf], timeout_ms: u64, ) -> codex_core::exec::ExecToolCallOutput { - run_cmd_result(cmd, writable_roots, timeout_ms) + run_cmd_result_with_writable_roots(cmd, writable_roots, timeout_ms, false) .await - .expect("sandboxed command should succeed") -} - -async fn run_cmd_result( - cmd: &[&str], - writable_roots: &[PathBuf], - timeout_ms: u64, -) -> Result { - run_cmd_result_with_bwrap(cmd, writable_roots, timeout_ms, false).await + .expect("sandboxed command should execute") } #[expect(clippy::expect_used)] -async fn run_cmd_result_with_bwrap( +async fn run_cmd_result_with_writable_roots( cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64, @@ -120,7 +112,14 @@ fn is_bwrap_unavailable_output(output: &codex_core::exec::ExecToolCallOutput) -> } async fn should_skip_bwrap_tests() -> bool { - match run_cmd_result_with_bwrap(&["bash", "-lc", "true"], &[], NETWORK_TIMEOUT_MS, true).await { + 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) @@ -314,7 +313,7 @@ async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { let codex_target = dot_codex.join("config.toml"); let git_output = expect_denied( - run_cmd_result_with_bwrap( + run_cmd_result_with_writable_roots( &[ "bash", "-lc", @@ -329,7 +328,7 @@ async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { ); let codex_output = expect_denied( - run_cmd_result_with_bwrap( + run_cmd_result_with_writable_roots( &[ "bash", "-lc", @@ -365,7 +364,7 @@ async fn sandbox_blocks_codex_symlink_replacement_attack() { let codex_target = dot_codex.join("config.toml"); let codex_output = expect_denied( - run_cmd_result_with_bwrap( + run_cmd_result_with_writable_roots( &[ "bash", "-lc", From abcbeae67ea7b4c0e6a19c11f95b4fa7c37f9139 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 4 Feb 2026 09:45:51 -0800 Subject: [PATCH 14/14] chore: revert repo-level AGENTS convention additions --- AGENTS.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7b9773afdc20..77c48ddba0f6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,8 +12,6 @@ In the codex-rs folder where the rust code lives: - Always inline format! args when possible per https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args - Use method references over closures when possible per https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls - When possible, make `match` statements exhaustive and avoid wildcard arms. -- Name helpers by actual behavior, not by optional implementation details behind flags. -- For wrappers around third-party `main`/`exec*` flows, docstrings must match real control flow and return semantics; do not claim non-returning behavior unless guaranteed. - When writing tests, prefer comparing the equality of entire objects over fields one by one. - When making a change that adds or changes an API, ensure that the documentation in the `docs/` folder is up to date if applicable. - If you change `ConfigToml` or nested config types, run `just write-config-schema` to update `codex-rs/core/config.schema.json`. @@ -80,8 +78,6 @@ If you don’t have the tool: - Tests should use pretty_assertions::assert_eq for clearer diffs. Import this at the top of the test module if it isn't already. - Prefer deep equals comparisons whenever possible. Perform `assert_eq!()` on entire objects, rather than individual fields. -- Keep assertion failure messages precise about guarantees: use wording like “executes” unless the assertion is specifically about successful (`0`) exit status. -- Avoid one-off passthrough helper functions in tests; inline single-use wrappers unless they improve readability materially. - Avoid mutating process environment in tests; prefer passing environment-derived flags or dependencies from above. ### Spawning workspace binaries in tests (Cargo vs Bazel)