feat: experimental env var: CODEX_SANDBOX_NETWORK_DISABLED (#879)

When using Codex to develop Codex itself, I noticed that sometimes it
would try to add `#[ignore]` to the following tests:

```
keeps_previous_response_id_between_tasks()
retries_on_early_close()
```

Both of these tests start a `MockServer` that launches an HTTP server on
an ephemeral port and requires network access to hit it, which the
Seatbelt policy associated with `--full-auto` correctly denies. If I
wasn't paying attention to the code that Codex was generating, one of
these `#[ignore]` annotations could have slipped into the codebase,
effectively disabling the test for everyone.

To that end, this PR enables an experimental environment variable named
`CODEX_SANDBOX_NETWORK_DISABLED` that is set to `1` if the
`SandboxPolicy` used to spawn the process does not have full network
access. I say it is "experimental" because I'm not convinced this API is
quite right, but we need to start somewhere. (It might be more
appropriate to have an env var like `CODEX_SANDBOX=full-auto`, but the
challenge is that our newer `SandboxPolicy` abstraction does not map to
a simple set of enums like in the TypeScript CLI.)

We leverage this new functionality by adding the following code to the
aforementioned tests as a way to "dynamically disable" them:

```rust
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;
}
```

We can use the `debug seatbelt --full-auto` command to verify that
`cargo test` fails when run under Seatbelt prior 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`
```

Though after this change, the above command succeeds! This means that,
going forward, when Codex operates on Codex itself, when it runs `cargo
test`, only "real failures" should cause the command to fail.

As part of this change, I decided to tighten up the codepaths for
running `exec()` for shell tool calls. In particular, we do it in `core`
for the main Codex business logic itself, but we also expose this logic
via `debug` subcommands in the CLI in the `cli` crate. The logic for the
`debug` subcommands was not quite as faithful to the true business logic
as I liked, so I:

* refactored a bit of the Linux code, splitting `linux.rs` into
`linux_exec.rs` and `landlock.rs` in the `core` crate.
* gating less code behind `#[cfg(target_os = "linux")]` because such
code does not get built by default when I develop on Mac, which means I
either have to build the code in Docker or wait for CI signal
* introduced `macro_rules! configure_command` in `exec.rs` so we can
have both sync and async versions of this code. The synchronous version
seems more appropriate for straight threads or potentially fork/exec.
This commit is contained in:
Michael Bolin
2025-05-09 18:29:34 -07:00
committed by GitHub
parent 7795272282
commit fde48aaa0d
12 changed files with 275 additions and 128 deletions

View File

@@ -0,0 +1,23 @@
#[cfg(unix)]
pub(crate) fn handle_exit_status(status: std::process::ExitStatus) -> ! {
use std::os::unix::process::ExitStatusExt;
// Use ExitStatus to derive the exit code.
if let Some(code) = status.code() {
std::process::exit(code);
} else if let Some(signal) = status.signal() {
std::process::exit(128 + signal);
} else {
std::process::exit(1);
}
}
#[cfg(windows)]
pub(crate) fn handle_exit_status(status: std::process::ExitStatus) -> ! {
if let Some(code) = status.code() {
std::process::exit(code);
} else {
// Rare on Windows, but if it happens: use fallback code.
std::process::exit(1);
}
}

View File

@@ -3,12 +3,14 @@
//! 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::exec_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;
use crate::exit_status::handle_exit_status;
/// Execute `command` in a Linux sandbox (Landlock + seccomp) the way Codex
/// would.
pub fn run_landlock(command: Vec<String>, sandbox_policy: SandboxPolicy) -> anyhow::Result<()> {
@@ -19,20 +21,15 @@ pub fn run_landlock(command: Vec<String>, sandbox_policy: SandboxPolicy) -> anyh
// Spawn a new thread and apply the sandbox policies there.
let handle = std::thread::spawn(move || -> anyhow::Result<ExitStatus> {
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
.join()
.map_err(|e| anyhow::anyhow!("Failed to join thread: {e:?}"))??;
// 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);
}
handle_exit_status(status);
}

View File

@@ -1,4 +1,5 @@
#[cfg(target_os = "linux")]
mod exit_status;
#[cfg(unix)]
pub mod landlock;
pub mod proto;
pub mod seatbelt;

View File

@@ -82,7 +82,7 @@ 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(unix)]
DebugCommand::Landlock(LandlockCommand {
command,
sandbox,
@@ -91,7 +91,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.");
}

View File

@@ -1,18 +1,16 @@
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 crate::exit_status::handle_exit_status;
pub async fn run_seatbelt(
command: Vec<String>,
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 cwd = std::env::current_dir()?;
let mut child =
spawn_command_under_seatbelt(command, &sandbox_policy, cwd, StdioPolicy::Inherit).await?;
let status = child.wait().await?;
handle_exit_status(status);
}

View File

@@ -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::exec_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<String>,
@@ -60,27 +72,6 @@ pub enum SandboxType {
LinuxSeccomp,
}
#[cfg(target_os = "linux")]
async fn exec_linux(
params: ExecParams,
ctrl_c: Arc<Notify>,
sandbox_policy: &SandboxPolicy,
) -> Result<RawExecToolCallOutput> {
crate::linux::exec_linux(params, ctrl_c, sandbox_policy).await
}
#[cfg(not(target_os = "linux"))]
async fn exec_linux(
_params: ExecParams,
_ctrl_c: Arc<Notify>,
_sandbox_policy: &SandboxPolicy,
) -> Result<RawExecToolCallOutput> {
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<String>,
sandbox_policy: &SandboxPolicy,
cwd: PathBuf,
stdio_policy: StdioPolicy,
) -> std::io::Result<Child> {
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<String>,
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<Notify>,
) -> Result<RawExecToolCallOutput> {
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<String>, cwd: PathBuf) -> std::io::Result<Child> {
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<String>,
cwd: PathBuf,
sandbox_policy: &SandboxPolicy,
stdio_policy: StdioPolicy,
) -> std::io::Result<Child> {
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<String>,
cwd: PathBuf,
sandbox_policy: &SandboxPolicy,
stdio_policy: StdioPolicy,
) -> std::io::Result<std::process::Child> {
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<Notify>,
timeout_ms: Option<u64>,

View File

@@ -0,0 +1,79 @@
use std::io;
use std::path::Path;
use std::sync::Arc;
use crate::error::CodexErr;
use crate::error::Result;
use crate::exec::ExecParams;
use crate::exec::RawExecToolCallOutput;
use crate::exec::StdioPolicy;
use crate::exec::consume_truncated_output;
use crate::exec::spawn_child_async;
use crate::protocol::SandboxPolicy;
use tokio::sync::Notify;
pub fn exec_linux(
params: ExecParams,
ctrl_c: Arc<Notify>,
sandbox_policy: &SandboxPolicy,
) -> Result<RawExecToolCallOutput> {
// Allow READ on /
// Allow WRITE on /dev/null
let ctrl_c_copy = ctrl_c.clone();
let sandbox_policy = sandbox_policy.clone();
// Isolate thread to run the sandbox from
let tool_call_output = std::thread::spawn(move || {
let rt = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.expect("Failed to create runtime");
rt.block_on(async {
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();
match tool_call_output {
Ok(Ok(output)) => Ok(output),
Ok(Err(e)) => Err(e),
Err(e) => Err(CodexErr::Io(io::Error::new(
io::ErrorKind::Other,
format!("thread join failed: {e:?}"),
))),
}
}
#[cfg(target_os = "linux")]
pub fn apply_sandbox_policy_to_current_thread(
sandbox_policy: &SandboxPolicy,
cwd: &Path,
) -> Result<()> {
crate::landlock::apply_sandbox_policy_to_current_thread(sandbox_policy, cwd)
}
#[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",
)))
}

View File

@@ -1,15 +1,10 @@
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::protocol::SandboxPolicy;
use landlock::ABI;
@@ -29,46 +24,11 @@ use seccompiler::SeccompFilter;
use seccompiler::SeccompRule;
use seccompiler::TargetArch;
use seccompiler::apply_filter;
use tokio::sync::Notify;
pub async fn exec_linux(
params: ExecParams,
ctrl_c: Arc<Notify>,
sandbox_policy: &SandboxPolicy,
) -> Result<RawExecToolCallOutput> {
// Allow READ on /
// Allow WRITE on /dev/null
let ctrl_c_copy = ctrl_c.clone();
let sandbox_policy = sandbox_policy.clone();
// Isolate thread to run the sandbox from
let tool_call_output = std::thread::spawn(move || {
let rt = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.expect("Failed to create runtime");
rt.block_on(async {
apply_sandbox_policy_to_current_thread(sandbox_policy, &params.cwd)?;
exec(params, ctrl_c_copy).await
})
})
.join();
match tool_call_output {
Ok(Ok(output)) => Ok(output),
Ok(Err(e)) => Err(e),
Err(e) => Err(CodexErr::Io(io::Error::new(
io::ErrorKind::Other,
format!("thread join failed: {e:?}"),
))),
}
}
/// Apply sandbox policies inside this thread so only the child inherits
/// them, not the entire CLI process.
pub fn apply_sandbox_policy_to_current_thread(
sandbox_policy: SandboxPolicy,
pub(crate) fn apply_sandbox_policy_to_current_thread(
sandbox_policy: &SandboxPolicy,
cwd: &Path,
) -> Result<()> {
if !sandbox_policy.has_full_network_access() {

View File

@@ -16,10 +16,11 @@ pub mod config;
mod conversation_history;
pub mod error;
pub mod exec;
pub mod exec_linux;
mod flags;
mod is_safe_command;
#[cfg(target_os = "linux")]
pub mod linux;
pub mod landlock;
mod mcp_connection_manager;
pub mod mcp_server_config;
mod mcp_tool_call;

View File

@@ -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;

View File

@@ -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;

View File

@@ -81,6 +81,7 @@ impl McpClient {
) -> std::io::Result<Self> {
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())