From bbf7814dce651163b404af0e47a0c2a6546d4d93 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 9 May 2025 12:08:13 -0700 Subject: [PATCH] feat: experimental env var: CODEX_SANDBOX_NETWORK_DISABLED Previous to this change: ``` $ cargo run --bin codex -- debug seatbelt --full-auto -- cargo test ---- keeps_previous_response_id_between_tasks stdout ---- thread 'keeps_previous_response_id_between_tasks' panicked at /Users/mbolin/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/wiremock-0.6.3/src/mock_server/builder.rs:107:46: Failed to bind an OS port for a mock server.: Os { code: 1, kind: PermissionDenied, message: "Operation not permitted" } note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace failures: keeps_previous_response_id_between_tasks test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s error: test failed, to rerun pass `-p codex-core --test previous_response_id` ``` --- codex-rs/cli/src/landlock.rs | 10 +- codex-rs/cli/src/lib.rs | 2 +- codex-rs/cli/src/main.rs | 10 +- codex-rs/cli/src/seatbelt.rs | 26 +- codex-rs/core/src/exec.rs | 187 +++++++---- codex-rs/core/src/landlock.rs | 319 ++++++++++++++++++ codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/linux.rs | 340 ++------------------ codex-rs/core/tests/previous_response_id.rs | 8 + codex-rs/core/tests/stream_no_completed.rs | 8 + codex-rs/mcp-client/src/mcp_client.rs | 1 + 11 files changed, 528 insertions(+), 384 deletions(-) create mode 100644 codex-rs/core/src/landlock.rs diff --git a/codex-rs/cli/src/landlock.rs b/codex-rs/cli/src/landlock.rs index bc43eb57cd..467cf17ae5 100644 --- a/codex-rs/cli/src/landlock.rs +++ b/codex-rs/cli/src/landlock.rs @@ -3,10 +3,12 @@ //! On Linux the command is executed inside a Landlock + seccomp sandbox by //! calling the low-level `exec_linux` helper from `codex_core::linux`. +use codex_core::exec::StdioPolicy; +use codex_core::exec::spawn_child_sync; +use codex_core::linux::apply_sandbox_policy_to_current_thread; use codex_core::protocol::SandboxPolicy; use std::os::unix::process::ExitStatusExt; use std::process; -use std::process::Command; use std::process::ExitStatus; /// Execute `command` in a Linux sandbox (Landlock + seccomp) the way Codex @@ -19,8 +21,10 @@ pub fn run_landlock(command: Vec, sandbox_policy: SandboxPolicy) -> anyh // Spawn a new thread and apply the sandbox policies there. let handle = std::thread::spawn(move || -> anyhow::Result { let cwd = std::env::current_dir()?; - codex_core::linux::apply_sandbox_policy_to_current_thread(sandbox_policy, &cwd)?; - let status = Command::new(&command[0]).args(&command[1..]).status()?; + + apply_sandbox_policy_to_current_thread(&sandbox_policy, &cwd)?; + let mut child = spawn_child_sync(command, cwd, &sandbox_policy, StdioPolicy::Inherit)?; + let status = child.wait()?; Ok(status) }); let status = handle diff --git a/codex-rs/cli/src/lib.rs b/codex-rs/cli/src/lib.rs index 82e434a0c8..40a1a5f881 100644 --- a/codex-rs/cli/src/lib.rs +++ b/codex-rs/cli/src/lib.rs @@ -1,4 +1,4 @@ -#[cfg(target_os = "linux")] +#[cfg(unix)] pub mod landlock; pub mod proto; pub mod seatbelt; diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 506c8d31d7..6484a4c4e4 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -3,6 +3,7 @@ use codex_cli::LandlockCommand; use codex_cli::SeatbeltCommand; use codex_cli::create_sandbox_policy; use codex_cli::proto; +#[cfg(target_os = "macos")] use codex_cli::seatbelt; use codex_exec::Cli as ExecCli; use codex_tui::Cli as TuiCli; @@ -74,6 +75,7 @@ async fn main() -> anyhow::Result<()> { proto::run_main(proto_cli).await?; } Some(Subcommand::Debug(debug_args)) => match debug_args.cmd { + #[cfg(target_os = "macos")] DebugCommand::Seatbelt(SeatbeltCommand { command, sandbox, @@ -82,7 +84,11 @@ async fn main() -> anyhow::Result<()> { let sandbox_policy = create_sandbox_policy(full_auto, sandbox); seatbelt::run_seatbelt(command, sandbox_policy).await?; } - #[cfg(target_os = "linux")] + #[cfg(not(target_os = "macos"))] + DebugCommand::Seatbelt(_) => { + anyhow::bail!("Seatbelt is only supported on macOS."); + } + #[cfg(unix)] DebugCommand::Landlock(LandlockCommand { command, sandbox, @@ -91,7 +97,7 @@ async fn main() -> anyhow::Result<()> { let sandbox_policy = create_sandbox_policy(full_auto, sandbox); codex_cli::landlock::run_landlock(command, sandbox_policy)?; } - #[cfg(not(target_os = "linux"))] + #[cfg(not(unix))] DebugCommand::Landlock(_) => { anyhow::bail!("Landlock is only supported on Linux."); } diff --git a/codex-rs/cli/src/seatbelt.rs b/codex-rs/cli/src/seatbelt.rs index 3c7ec2ba93..00a41fb739 100644 --- a/codex-rs/cli/src/seatbelt.rs +++ b/codex-rs/cli/src/seatbelt.rs @@ -1,18 +1,24 @@ -use codex_core::exec::create_seatbelt_command; +use codex_core::exec::StdioPolicy; +use codex_core::exec::spawn_command_under_seatbelt; use codex_core::protocol::SandboxPolicy; +use std::os::unix::process::ExitStatusExt; +use std::process; pub async fn run_seatbelt( command: Vec, sandbox_policy: SandboxPolicy, ) -> anyhow::Result<()> { let cwd = std::env::current_dir().expect("failed to get cwd"); - let seatbelt_command = create_seatbelt_command(command, &sandbox_policy, &cwd); - let status = tokio::process::Command::new(seatbelt_command[0].clone()) - .args(&seatbelt_command[1..]) - .spawn() - .map_err(|e| anyhow::anyhow!("Failed to spawn command: {}", e))? - .wait() - .await - .map_err(|e| anyhow::anyhow!("Failed to wait for command: {}", e))?; - std::process::exit(status.code().unwrap_or(1)); + let mut child = + spawn_command_under_seatbelt(command, &sandbox_policy, cwd, StdioPolicy::Inherit).await?; + let status = child.wait().await?; + + // Use ExitStatus to derive the exit code. + if let Some(code) = status.code() { + process::exit(code); + } else if let Some(signal) = status.signal() { + process::exit(128 + signal); + } else { + process::exit(1); + } } diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index aa761d2e7d..1a8a738d1b 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -1,6 +1,7 @@ -use std::io; -#[cfg(target_family = "unix")] +#[cfg(unix)] use std::os::unix::process::ExitStatusExt; + +use std::io; use std::path::Path; use std::path::PathBuf; use std::process::ExitStatus; @@ -19,6 +20,7 @@ use tokio::sync::Notify; use crate::error::CodexErr; use crate::error::Result; use crate::error::SandboxErr; +use crate::linux::exec_linux; use crate::protocol::SandboxPolicy; // Maximum we send for each stream, which is either: @@ -42,6 +44,16 @@ const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl /// already has root access. const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec"; +/// Experimental environment variable that will be set to some non-empty value +/// if both of the following are true: +/// +/// 1. The process was spawned by Codex as part of a shell tool call. +/// 2. SandboxPolicy.has_full_network_access() was false for the tool call. +/// +/// We may try to have just one environment variable for all sandboxing +/// attributes, so this may change in the future. +pub const CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR: &str = "CODEX_SANDBOX_NETWORK_DISABLED"; + #[derive(Debug, Clone)] pub struct ExecParams { pub command: Vec, @@ -60,27 +72,6 @@ pub enum SandboxType { LinuxSeccomp, } -#[cfg(target_os = "linux")] -async fn exec_linux( - params: ExecParams, - ctrl_c: Arc, - sandbox_policy: &SandboxPolicy, -) -> Result { - crate::linux::exec_linux(params, ctrl_c, sandbox_policy).await -} - -#[cfg(not(target_os = "linux"))] -async fn exec_linux( - _params: ExecParams, - _ctrl_c: Arc, - _sandbox_policy: &SandboxPolicy, -) -> Result { - Err(CodexErr::Io(io::Error::new( - io::ErrorKind::InvalidInput, - "linux sandbox is not supported on this platform", - ))) -} - pub async fn process_exec_tool_call( params: ExecParams, sandbox_type: SandboxType, @@ -90,25 +81,23 @@ pub async fn process_exec_tool_call( let start = Instant::now(); let raw_output_result = match sandbox_type { - SandboxType::None => exec(params, ctrl_c).await, + SandboxType::None => exec(params, sandbox_policy, ctrl_c).await, SandboxType::MacosSeatbelt => { let ExecParams { command, cwd, timeout_ms, } = params; - let seatbelt_command = create_seatbelt_command(command, sandbox_policy, &cwd); - exec( - ExecParams { - command: seatbelt_command, - cwd, - timeout_ms, - }, - ctrl_c, + let child = spawn_command_under_seatbelt( + command, + sandbox_policy, + cwd, + StdioPolicy::RedirectForShellTool, ) - .await + .await?; + consume_truncated_output(child, ctrl_c, timeout_ms).await } - SandboxType::LinuxSeccomp => exec_linux(params, ctrl_c, sandbox_policy).await, + SandboxType::LinuxSeccomp => exec_linux(params, ctrl_c, sandbox_policy), }; let duration = start.elapsed(); match raw_output_result { @@ -151,7 +140,17 @@ pub async fn process_exec_tool_call( } } -pub fn create_seatbelt_command( +pub async fn spawn_command_under_seatbelt( + command: Vec, + sandbox_policy: &SandboxPolicy, + cwd: PathBuf, + stdio_policy: StdioPolicy, +) -> std::io::Result { + let seatbelt_command = create_seatbelt_command(command, sandbox_policy, &cwd); + spawn_child_async(seatbelt_command, cwd, sandbox_policy, stdio_policy).await +} + +fn create_seatbelt_command( command: Vec, sandbox_policy: &SandboxPolicy, cwd: &Path, @@ -229,46 +228,118 @@ pub struct ExecToolCallOutput { pub duration: Duration, } -pub async fn exec( +async fn exec( ExecParams { command, cwd, timeout_ms, }: ExecParams, + sandbox_policy: &SandboxPolicy, ctrl_c: Arc, ) -> Result { - let child = spawn_child(command, cwd).await?; + let child = spawn_child_async( + command, + cwd, + sandbox_policy, + StdioPolicy::RedirectForShellTool, + ) + .await?; consume_truncated_output(child, ctrl_c, timeout_ms).await } -/// Spawns the appropriate child process for the ExecParams. -async fn spawn_child(command: Vec, cwd: PathBuf) -> std::io::Result { - if command.is_empty() { - return Err(std::io::Error::new( - io::ErrorKind::InvalidInput, - "command args are empty", - )); - } +#[derive(Debug, Clone, Copy)] +pub enum StdioPolicy { + RedirectForShellTool, + Inherit, +} - let mut cmd = Command::new(&command[0]); - cmd.args(&command[1..]); - cmd.current_dir(cwd); +macro_rules! configure_command { + ( + $cmd_type: path, + $command: expr, + $cwd: expr, + $sandbox_policy: expr, + $stdio_policy: expr + ) => {{ + // For now, we take `SandboxPolicy` as a parameter to spawn_child() because + // we need to determine whether to set the + // `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` environment variable. + // Ultimately, we should be stricter about the environment variables that + // are set for the command (as we are when spawning an MCP server), so + // instead of SandboxPolicy, we should take the exact env to use for the + // Command (i.e., `env_clear().envs(env)`). + if $command.is_empty() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "command args are empty", + )); + } - // Do not create a file descriptor for stdin because otherwise some - // commands may hang forever waiting for input. For example, ripgrep has - // a heuristic where it may try to read from stdin as explained here: - // https://github.com/BurntSushi/ripgrep/blob/e2362d4d5185d02fa857bf381e7bd52e66fafc73/crates/core/flags/hiargs.rs#L1101-L1103 - cmd.stdin(Stdio::null()); + let mut cmd = <$cmd_type>::new(&$command[0]); + cmd.args(&$command[1..]); + cmd.current_dir($cwd); - cmd.stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .kill_on_drop(true) - .spawn() + if !$sandbox_policy.has_full_network_access() { + cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1"); + } + + match $stdio_policy { + StdioPolicy::RedirectForShellTool => { + // Do not create a file descriptor for stdin because otherwise some + // commands may hang forever waiting for input. For example, ripgrep has + // a heuristic where it may try to read from stdin as explained here: + // https://github.com/BurntSushi/ripgrep/blob/e2362d4d5185d02fa857bf381e7bd52e66fafc73/crates/core/flags/hiargs.rs#L1101-L1103 + cmd.stdin(Stdio::null()); + + cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); + } + StdioPolicy::Inherit => { + // Inherit stdin, stdout, and stderr from the parent process. + cmd.stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()); + } + } + + std::io::Result::<$cmd_type>::Ok(cmd) + }}; +} + +/// Spawns the appropriate child process for the ExecParams and SandboxPolicy, +/// ensuring the args and environment variables used to create the `Command` +/// (and `Child`) honor the configuration. +pub(crate) async fn spawn_child_async( + command: Vec, + cwd: PathBuf, + sandbox_policy: &SandboxPolicy, + stdio_policy: StdioPolicy, +) -> std::io::Result { + let mut cmd = configure_command!(Command, command, cwd, sandbox_policy, stdio_policy)?; + cmd.kill_on_drop(true).spawn() +} + +/// Alternative verison of `spawn_child_async()` that returns +/// `std::process::Child` instead of `tokio::process::Child`. This is useful for +/// spawning a child process in a thread that is not running a Tokio runtime. +pub fn spawn_child_sync( + command: Vec, + cwd: PathBuf, + sandbox_policy: &SandboxPolicy, + stdio_policy: StdioPolicy, +) -> std::io::Result { + let mut cmd = configure_command!( + std::process::Command, + command, + cwd, + sandbox_policy, + stdio_policy + )?; + cmd.spawn() } /// Consumes the output of a child process, truncating it so it is suitable for /// use as the output of a `shell` tool call. Also enforces specified timeout. -async fn consume_truncated_output( +pub(crate) async fn consume_truncated_output( mut child: Child, ctrl_c: Arc, timeout_ms: Option, diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs new file mode 100644 index 0000000000..e8f5a4de9b --- /dev/null +++ b/codex-rs/core/src/landlock.rs @@ -0,0 +1,319 @@ +use std::collections::BTreeMap; +use std::path::Path; +use std::path::PathBuf; + +use crate::error::CodexErr; +use crate::error::Result; +use crate::error::SandboxErr; +use crate::protocol::SandboxPolicy; + +use landlock::ABI; +use landlock::Access; +use landlock::AccessFs; +use landlock::CompatLevel; +use landlock::Compatible; +use landlock::Ruleset; +use landlock::RulesetAttr; +use landlock::RulesetCreatedAttr; +use seccompiler::BpfProgram; +use seccompiler::SeccompAction; +use seccompiler::SeccompCmpArgLen; +use seccompiler::SeccompCmpOp; +use seccompiler::SeccompCondition; +use seccompiler::SeccompFilter; +use seccompiler::SeccompRule; +use seccompiler::TargetArch; +use seccompiler::apply_filter; + +/// Apply sandbox policies inside this thread so only the child inherits +/// them, not the entire CLI process. +pub(crate) fn apply_sandbox_policy_to_current_thread( + sandbox_policy: &SandboxPolicy, + cwd: &Path, +) -> Result<()> { + if !sandbox_policy.has_full_network_access() { + install_network_seccomp_filter_on_current_thread()?; + } + + if !sandbox_policy.has_full_disk_write_access() { + let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); + install_filesystem_landlock_rules_on_current_thread(writable_roots)?; + } + + // TODO(ragona): Add appropriate restrictions if + // `sandbox_policy.has_full_disk_read_access()` is `false`. + + Ok(()) +} + +/// Installs Landlock file-system rules on the current thread allowing read +/// access to the entire file-system while restricting write access to +/// `/dev/null` and the provided list of `writable_roots`. +/// +/// # Errors +/// Returns [`CodexErr::Sandbox`] variants when the ruleset fails to apply. +fn install_filesystem_landlock_rules_on_current_thread(writable_roots: Vec) -> Result<()> { + let abi = ABI::V5; + let access_rw = AccessFs::from_all(abi); + let access_ro = AccessFs::from_read(abi); + + let mut ruleset = Ruleset::default() + .set_compatibility(CompatLevel::BestEffort) + .handle_access(access_rw)? + .create()? + .add_rules(landlock::path_beneath_rules(&["/"], access_ro))? + .add_rules(landlock::path_beneath_rules(&["/dev/null"], access_rw))? + .set_no_new_privs(true); + + if !writable_roots.is_empty() { + ruleset = ruleset.add_rules(landlock::path_beneath_rules(&writable_roots, access_rw))?; + } + + let status = ruleset.restrict_self()?; + + if status.ruleset == landlock::RulesetStatus::NotEnforced { + return Err(CodexErr::Sandbox(SandboxErr::LandlockRestrict)); + } + + Ok(()) +} + +/// Installs a seccomp filter that blocks outbound network access except for +/// AF_UNIX domain sockets. +fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), SandboxErr> { + // Build rule map. + let mut rules: BTreeMap> = BTreeMap::new(); + + // Helper – insert unconditional deny rule for syscall number. + let mut deny_syscall = |nr: i64| { + rules.insert(nr, vec![]); // empty rule vec = unconditional match + }; + + deny_syscall(libc::SYS_connect); + deny_syscall(libc::SYS_accept); + deny_syscall(libc::SYS_accept4); + deny_syscall(libc::SYS_bind); + deny_syscall(libc::SYS_listen); + deny_syscall(libc::SYS_getpeername); + deny_syscall(libc::SYS_getsockname); + deny_syscall(libc::SYS_shutdown); + deny_syscall(libc::SYS_sendto); + deny_syscall(libc::SYS_sendmsg); + deny_syscall(libc::SYS_sendmmsg); + deny_syscall(libc::SYS_recvfrom); + deny_syscall(libc::SYS_recvmsg); + deny_syscall(libc::SYS_recvmmsg); + deny_syscall(libc::SYS_getsockopt); + deny_syscall(libc::SYS_setsockopt); + deny_syscall(libc::SYS_ptrace); + + // For `socket` we allow AF_UNIX (arg0 == AF_UNIX) and deny everything else. + let unix_only_rule = SeccompRule::new(vec![SeccompCondition::new( + 0, // first argument (domain) + SeccompCmpArgLen::Dword, + SeccompCmpOp::Eq, + libc::AF_UNIX as u64, + )?])?; + + rules.insert(libc::SYS_socket, vec![unix_only_rule]); + rules.insert(libc::SYS_socketpair, vec![]); // always deny (Unix can use socketpair but fine, keep open?) + + let filter = SeccompFilter::new( + rules, + SeccompAction::Allow, // default – allow + SeccompAction::Errno(libc::EPERM as u32), // when rule matches – return EPERM + if cfg!(target_arch = "x86_64") { + TargetArch::x86_64 + } else if cfg!(target_arch = "aarch64") { + TargetArch::aarch64 + } else { + unimplemented!("unsupported architecture for seccomp filter"); + }, + )?; + + let prog: BpfProgram = filter.try_into()?; + + apply_filter(&prog)?; + + Ok(()) +} + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + + use super::*; + use crate::exec::ExecParams; + use crate::exec::SandboxType; + use crate::exec::process_exec_tool_call; + use crate::protocol::SandboxPolicy; + use std::sync::Arc; + use tempfile::NamedTempFile; + use tokio::sync::Notify; + + #[allow(clippy::print_stdout)] + async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { + let params = ExecParams { + command: cmd.iter().map(|elm| elm.to_string()).collect(), + cwd: std::env::current_dir().expect("cwd should exist"), + timeout_ms: Some(timeout_ms), + }; + + let sandbox_policy = + SandboxPolicy::new_read_only_policy_with_writable_roots(writable_roots); + let ctrl_c = Arc::new(Notify::new()); + let res = + process_exec_tool_call(params, SandboxType::LinuxSeccomp, ctrl_c, &sandbox_policy) + .await + .unwrap(); + + if res.exit_code != 0 { + println!("stdout:\n{}", res.stdout); + println!("stderr:\n{}", res.stderr); + panic!("exit code: {}", res.exit_code); + } + } + + #[tokio::test] + async fn test_root_read() { + run_cmd(&["ls", "-l", "/bin"], &[], 200).await; + } + + #[tokio::test] + #[should_panic] + async fn test_root_write() { + let tmpfile = NamedTempFile::new().unwrap(); + let tmpfile_path = tmpfile.path().to_string_lossy(); + run_cmd( + &["bash", "-lc", &format!("echo blah > {}", tmpfile_path)], + &[], + 200, + ) + .await; + } + + #[tokio::test] + async fn test_dev_null_write() { + run_cmd(&["echo", "blah", ">", "/dev/null"], &[], 200).await; + } + + #[tokio::test] + async fn test_writable_root() { + let tmpdir = tempfile::tempdir().unwrap(); + let file_path = tmpdir.path().join("test"); + run_cmd( + &[ + "bash", + "-lc", + &format!("echo blah > {}", file_path.to_string_lossy()), + ], + &[tmpdir.path().to_path_buf()], + // We have seen timeouts when running this test in CI on GitHub, + // so we are using a generous timeout until we can diagnose further. + 1_000, + ) + .await; + } + + #[tokio::test] + #[should_panic(expected = "Sandbox(Timeout)")] + async fn test_timeout() { + run_cmd(&["sleep", "2"], &[], 50).await; + } + + /// Helper that runs `cmd` under the Linux sandbox and asserts that the command + /// does NOT succeed (i.e. returns a non‑zero exit code) **unless** the binary + /// is missing in which case we silently treat it as an accepted skip so the + /// suite remains green on leaner CI images. + async fn assert_network_blocked(cmd: &[&str]) { + let params = ExecParams { + command: cmd.iter().map(|s| s.to_string()).collect(), + cwd: std::env::current_dir().expect("cwd should exist"), + // Give the tool a generous 2‑second timeout so even slow DNS timeouts + // do not stall the suite. + timeout_ms: Some(2_000), + }; + + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let ctrl_c = Arc::new(Notify::new()); + let result = + process_exec_tool_call(params, SandboxType::LinuxSeccomp, ctrl_c, &sandbox_policy) + .await; + + let (exit_code, stdout, stderr) = match result { + Ok(output) => (output.exit_code, output.stdout, output.stderr), + Err(CodexErr::Sandbox(SandboxErr::Denied(exit_code, stdout, stderr))) => { + (exit_code, stdout, stderr) + } + _ => { + panic!("expected sandbox denied error, got: {:?}", result); + } + }; + + dbg!(&stderr); + dbg!(&stdout); + dbg!(&exit_code); + + // A completely missing binary exits with 127. Anything else should also + // be non‑zero (EPERM from seccomp will usually bubble up as 1, 2, 13…) + // If—*and only if*—the command exits 0 we consider the sandbox breached. + + if exit_code == 0 { + panic!( + "Network sandbox FAILED - {:?} exited 0\nstdout:\n{}\nstderr:\n{}", + cmd, stdout, stderr + ); + } + } + + #[tokio::test] + async fn sandbox_blocks_curl() { + assert_network_blocked(&["curl", "-I", "http://openai.com"]).await; + } + + #[cfg(target_os = "linux")] + #[tokio::test] + async fn sandbox_blocks_wget() { + assert_network_blocked(&["wget", "-qO-", "http://openai.com"]).await; + } + + #[tokio::test] + async fn sandbox_blocks_ping() { + // ICMP requires raw socket – should be denied quickly with EPERM. + assert_network_blocked(&["ping", "-c", "1", "8.8.8.8"]).await; + } + + #[tokio::test] + async fn sandbox_blocks_nc() { + // Zero‑length connection attempt to localhost. + assert_network_blocked(&["nc", "-z", "127.0.0.1", "80"]).await; + } + + #[tokio::test] + async fn sandbox_blocks_ssh() { + // Force ssh to attempt a real TCP connection but fail quickly. `BatchMode` + // avoids password prompts, and `ConnectTimeout` keeps the hang time low. + assert_network_blocked(&[ + "ssh", + "-o", + "BatchMode=yes", + "-o", + "ConnectTimeout=1", + "github.com", + ]) + .await; + } + + #[tokio::test] + async fn sandbox_blocks_getent() { + assert_network_blocked(&["getent", "ahosts", "openai.com"]).await; + } + + #[tokio::test] + async fn sandbox_blocks_dev_tcp_redirection() { + // This syntax is only supported by bash and zsh. We try bash first. + // Fallback generic socket attempt using /bin/sh with bash‑style /dev/tcp. Not + // all images ship bash, so we guard against 127 as well. + assert_network_blocked(&["bash", "-c", "echo hi > /dev/tcp/127.0.0.1/80"]).await; + } +} diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 7774e0f5cb..4e2258bb73 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -19,6 +19,7 @@ pub mod exec; mod flags; mod is_safe_command; #[cfg(target_os = "linux")] +pub mod landlock; pub mod linux; mod mcp_connection_manager; pub mod mcp_server_config; diff --git a/codex-rs/core/src/linux.rs b/codex-rs/core/src/linux.rs index 9928cfee4e..883a46a123 100644 --- a/codex-rs/core/src/linux.rs +++ b/codex-rs/core/src/linux.rs @@ -1,37 +1,19 @@ -use std::collections::BTreeMap; use std::io; use std::path::Path; -use std::path::PathBuf; use std::sync::Arc; use crate::error::CodexErr; use crate::error::Result; -use crate::error::SandboxErr; use crate::exec::ExecParams; use crate::exec::RawExecToolCallOutput; -use crate::exec::exec; +use crate::exec::StdioPolicy; +use crate::exec::consume_truncated_output; +use crate::exec::spawn_child_async; use crate::protocol::SandboxPolicy; -use landlock::ABI; -use landlock::Access; -use landlock::AccessFs; -use landlock::CompatLevel; -use landlock::Compatible; -use landlock::Ruleset; -use landlock::RulesetAttr; -use landlock::RulesetCreatedAttr; -use seccompiler::BpfProgram; -use seccompiler::SeccompAction; -use seccompiler::SeccompCmpArgLen; -use seccompiler::SeccompCmpOp; -use seccompiler::SeccompCondition; -use seccompiler::SeccompFilter; -use seccompiler::SeccompRule; -use seccompiler::TargetArch; -use seccompiler::apply_filter; use tokio::sync::Notify; -pub async fn exec_linux( +pub fn exec_linux( params: ExecParams, ctrl_c: Arc, sandbox_policy: &SandboxPolicy, @@ -49,8 +31,20 @@ pub async fn exec_linux( .expect("Failed to create runtime"); rt.block_on(async { - apply_sandbox_policy_to_current_thread(sandbox_policy, ¶ms.cwd)?; - exec(params, ctrl_c_copy).await + let ExecParams { + command, + cwd, + timeout_ms, + } = params; + apply_sandbox_policy_to_current_thread(&sandbox_policy, &cwd)?; + let child = spawn_child_async( + command, + cwd, + &sandbox_policy, + StdioPolicy::RedirectForShellTool, + ) + .await?; + consume_truncated_output(child, ctrl_c_copy, timeout_ms).await }) }) .join(); @@ -65,295 +59,21 @@ pub async fn exec_linux( } } -/// Apply sandbox policies inside this thread so only the child inherits -/// them, not the entire CLI process. +#[cfg(target_os = "linux")] pub fn apply_sandbox_policy_to_current_thread( - sandbox_policy: SandboxPolicy, + sandbox_policy: &SandboxPolicy, cwd: &Path, ) -> Result<()> { - if !sandbox_policy.has_full_network_access() { - install_network_seccomp_filter_on_current_thread()?; - } - - if !sandbox_policy.has_full_disk_write_access() { - let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); - install_filesystem_landlock_rules_on_current_thread(writable_roots)?; - } - - // TODO(ragona): Add appropriate restrictions if - // `sandbox_policy.has_full_disk_read_access()` is `false`. - - Ok(()) + crate::landlock::apply_sandbox_policy_to_current_thread(sandbox_policy, cwd) } -/// Installs Landlock file-system rules on the current thread allowing read -/// access to the entire file-system while restricting write access to -/// `/dev/null` and the provided list of `writable_roots`. -/// -/// # Errors -/// Returns [`CodexErr::Sandbox`] variants when the ruleset fails to apply. -fn install_filesystem_landlock_rules_on_current_thread(writable_roots: Vec) -> Result<()> { - let abi = ABI::V5; - let access_rw = AccessFs::from_all(abi); - let access_ro = AccessFs::from_read(abi); - - let mut ruleset = Ruleset::default() - .set_compatibility(CompatLevel::BestEffort) - .handle_access(access_rw)? - .create()? - .add_rules(landlock::path_beneath_rules(&["/"], access_ro))? - .add_rules(landlock::path_beneath_rules(&["/dev/null"], access_rw))? - .set_no_new_privs(true); - - if !writable_roots.is_empty() { - ruleset = ruleset.add_rules(landlock::path_beneath_rules(&writable_roots, access_rw))?; - } - - let status = ruleset.restrict_self()?; - - if status.ruleset == landlock::RulesetStatus::NotEnforced { - return Err(CodexErr::Sandbox(SandboxErr::LandlockRestrict)); - } - - Ok(()) -} - -/// Installs a seccomp filter that blocks outbound network access except for -/// AF_UNIX domain sockets. -fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), SandboxErr> { - // Build rule map. - let mut rules: BTreeMap> = BTreeMap::new(); - - // Helper – insert unconditional deny rule for syscall number. - let mut deny_syscall = |nr: i64| { - rules.insert(nr, vec![]); // empty rule vec = unconditional match - }; - - deny_syscall(libc::SYS_connect); - deny_syscall(libc::SYS_accept); - deny_syscall(libc::SYS_accept4); - deny_syscall(libc::SYS_bind); - deny_syscall(libc::SYS_listen); - deny_syscall(libc::SYS_getpeername); - deny_syscall(libc::SYS_getsockname); - deny_syscall(libc::SYS_shutdown); - deny_syscall(libc::SYS_sendto); - deny_syscall(libc::SYS_sendmsg); - deny_syscall(libc::SYS_sendmmsg); - deny_syscall(libc::SYS_recvfrom); - deny_syscall(libc::SYS_recvmsg); - deny_syscall(libc::SYS_recvmmsg); - deny_syscall(libc::SYS_getsockopt); - deny_syscall(libc::SYS_setsockopt); - deny_syscall(libc::SYS_ptrace); - - // For `socket` we allow AF_UNIX (arg0 == AF_UNIX) and deny everything else. - let unix_only_rule = SeccompRule::new(vec![SeccompCondition::new( - 0, // first argument (domain) - SeccompCmpArgLen::Dword, - SeccompCmpOp::Eq, - libc::AF_UNIX as u64, - )?])?; - - rules.insert(libc::SYS_socket, vec![unix_only_rule]); - rules.insert(libc::SYS_socketpair, vec![]); // always deny (Unix can use socketpair but fine, keep open?) - - let filter = SeccompFilter::new( - rules, - SeccompAction::Allow, // default – allow - SeccompAction::Errno(libc::EPERM as u32), // when rule matches – return EPERM - if cfg!(target_arch = "x86_64") { - TargetArch::x86_64 - } else if cfg!(target_arch = "aarch64") { - TargetArch::aarch64 - } else { - unimplemented!("unsupported architecture for seccomp filter"); - }, - )?; - - let prog: BpfProgram = filter.try_into()?; - - apply_filter(&prog)?; - - Ok(()) -} - -#[cfg(test)] -mod tests { - #![allow(clippy::unwrap_used)] - - use super::*; - use crate::exec::ExecParams; - use crate::exec::SandboxType; - use crate::exec::process_exec_tool_call; - use crate::protocol::SandboxPolicy; - use std::sync::Arc; - use tempfile::NamedTempFile; - use tokio::sync::Notify; - - #[allow(clippy::print_stdout)] - async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { - let params = ExecParams { - command: cmd.iter().map(|elm| elm.to_string()).collect(), - cwd: std::env::current_dir().expect("cwd should exist"), - timeout_ms: Some(timeout_ms), - }; - - let sandbox_policy = - SandboxPolicy::new_read_only_policy_with_writable_roots(writable_roots); - let ctrl_c = Arc::new(Notify::new()); - let res = - process_exec_tool_call(params, SandboxType::LinuxSeccomp, ctrl_c, &sandbox_policy) - .await - .unwrap(); - - if res.exit_code != 0 { - println!("stdout:\n{}", res.stdout); - println!("stderr:\n{}", res.stderr); - panic!("exit code: {}", res.exit_code); - } - } - - #[tokio::test] - async fn test_root_read() { - run_cmd(&["ls", "-l", "/bin"], &[], 200).await; - } - - #[tokio::test] - #[should_panic] - async fn test_root_write() { - let tmpfile = NamedTempFile::new().unwrap(); - let tmpfile_path = tmpfile.path().to_string_lossy(); - run_cmd( - &["bash", "-lc", &format!("echo blah > {}", tmpfile_path)], - &[], - 200, - ) - .await; - } - - #[tokio::test] - async fn test_dev_null_write() { - run_cmd(&["echo", "blah", ">", "/dev/null"], &[], 200).await; - } - - #[tokio::test] - async fn test_writable_root() { - let tmpdir = tempfile::tempdir().unwrap(); - let file_path = tmpdir.path().join("test"); - run_cmd( - &[ - "bash", - "-lc", - &format!("echo blah > {}", file_path.to_string_lossy()), - ], - &[tmpdir.path().to_path_buf()], - // We have seen timeouts when running this test in CI on GitHub, - // so we are using a generous timeout until we can diagnose further. - 1_000, - ) - .await; - } - - #[tokio::test] - #[should_panic(expected = "Sandbox(Timeout)")] - async fn test_timeout() { - run_cmd(&["sleep", "2"], &[], 50).await; - } - - /// Helper that runs `cmd` under the Linux sandbox and asserts that the command - /// does NOT succeed (i.e. returns a non‑zero exit code) **unless** the binary - /// is missing in which case we silently treat it as an accepted skip so the - /// suite remains green on leaner CI images. - async fn assert_network_blocked(cmd: &[&str]) { - let params = ExecParams { - command: cmd.iter().map(|s| s.to_string()).collect(), - cwd: std::env::current_dir().expect("cwd should exist"), - // Give the tool a generous 2‑second timeout so even slow DNS timeouts - // do not stall the suite. - timeout_ms: Some(2_000), - }; - - let sandbox_policy = SandboxPolicy::new_read_only_policy(); - let ctrl_c = Arc::new(Notify::new()); - let result = - process_exec_tool_call(params, SandboxType::LinuxSeccomp, ctrl_c, &sandbox_policy) - .await; - - let (exit_code, stdout, stderr) = match result { - Ok(output) => (output.exit_code, output.stdout, output.stderr), - Err(CodexErr::Sandbox(SandboxErr::Denied(exit_code, stdout, stderr))) => { - (exit_code, stdout, stderr) - } - _ => { - panic!("expected sandbox denied error, got: {:?}", result); - } - }; - - dbg!(&stderr); - dbg!(&stdout); - dbg!(&exit_code); - - // A completely missing binary exits with 127. Anything else should also - // be non‑zero (EPERM from seccomp will usually bubble up as 1, 2, 13…) - // If—*and only if*—the command exits 0 we consider the sandbox breached. - - if exit_code == 0 { - panic!( - "Network sandbox FAILED - {:?} exited 0\nstdout:\n{}\nstderr:\n{}", - cmd, stdout, stderr - ); - } - } - - #[tokio::test] - async fn sandbox_blocks_curl() { - assert_network_blocked(&["curl", "-I", "http://openai.com"]).await; - } - - #[cfg(target_os = "linux")] - #[tokio::test] - async fn sandbox_blocks_wget() { - assert_network_blocked(&["wget", "-qO-", "http://openai.com"]).await; - } - - #[tokio::test] - async fn sandbox_blocks_ping() { - // ICMP requires raw socket – should be denied quickly with EPERM. - assert_network_blocked(&["ping", "-c", "1", "8.8.8.8"]).await; - } - - #[tokio::test] - async fn sandbox_blocks_nc() { - // Zero‑length connection attempt to localhost. - assert_network_blocked(&["nc", "-z", "127.0.0.1", "80"]).await; - } - - #[tokio::test] - async fn sandbox_blocks_ssh() { - // Force ssh to attempt a real TCP connection but fail quickly. `BatchMode` - // avoids password prompts, and `ConnectTimeout` keeps the hang time low. - assert_network_blocked(&[ - "ssh", - "-o", - "BatchMode=yes", - "-o", - "ConnectTimeout=1", - "github.com", - ]) - .await; - } - - #[tokio::test] - async fn sandbox_blocks_getent() { - assert_network_blocked(&["getent", "ahosts", "openai.com"]).await; - } - - #[tokio::test] - async fn sandbox_blocks_dev_tcp_redirection() { - // This syntax is only supported by bash and zsh. We try bash first. - // Fallback generic socket attempt using /bin/sh with bash‑style /dev/tcp. Not - // all images ship bash, so we guard against 127 as well. - assert_network_blocked(&["bash", "-c", "echo hi > /dev/tcp/127.0.0.1/80"]).await; - } +#[cfg(not(target_os = "linux"))] +pub fn apply_sandbox_policy_to_current_thread( + _sandbox_policy: &SandboxPolicy, + _cwd: &Path, +) -> Result<()> { + Err(CodexErr::Io(io::Error::new( + io::ErrorKind::InvalidInput, + "linux sandbox is not supported on this platform", + ))) } diff --git a/codex-rs/core/tests/previous_response_id.rs b/codex-rs/core/tests/previous_response_id.rs index c318f38ba5..2c899df0e9 100644 --- a/codex-rs/core/tests/previous_response_id.rs +++ b/codex-rs/core/tests/previous_response_id.rs @@ -3,6 +3,7 @@ use std::time::Duration; use codex_core::Codex; use codex_core::ModelProviderInfo; use codex_core::config::Config; +use codex_core::exec::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_core::protocol::InputItem; use codex_core::protocol::Op; use serde_json::Value; @@ -50,6 +51,13 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"{}\",\"output\": async fn keeps_previous_response_id_between_tasks() { #![allow(clippy::unwrap_used)] + if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { + println!( + "Skipping test because it cannot execute when network is disabled in a Codex sandbox." + ); + return; + } + // Mock server let server = MockServer::start().await; diff --git a/codex-rs/core/tests/stream_no_completed.rs b/codex-rs/core/tests/stream_no_completed.rs index cfb7d44b2c..5b50d7ac26 100644 --- a/codex-rs/core/tests/stream_no_completed.rs +++ b/codex-rs/core/tests/stream_no_completed.rs @@ -6,6 +6,7 @@ use std::time::Duration; use codex_core::Codex; use codex_core::ModelProviderInfo; use codex_core::config::Config; +use codex_core::exec::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_core::protocol::InputItem; use codex_core::protocol::Op; use tokio::time::timeout; @@ -34,6 +35,13 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"{}\",\"output\": async fn retries_on_early_close() { #![allow(clippy::unwrap_used)] + if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { + println!( + "Skipping test because it cannot execute when network is disabled in a Codex sandbox." + ); + return; + } + let server = MockServer::start().await; struct SeqResponder; diff --git a/codex-rs/mcp-client/src/mcp_client.rs b/codex-rs/mcp-client/src/mcp_client.rs index 1c6a765c57..641de0e89a 100644 --- a/codex-rs/mcp-client/src/mcp_client.rs +++ b/codex-rs/mcp-client/src/mcp_client.rs @@ -81,6 +81,7 @@ impl McpClient { ) -> std::io::Result { let mut child = Command::new(program) .args(args) + .env_clear() .envs(create_env_for_mcp_server(env)) .stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped())