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`
```
This commit is contained in:
Michael Bolin
2025-05-09 12:15:05 -07:00
parent 78843c3940
commit e0cded2f7e
8 changed files with 149 additions and 43 deletions
+81 -25
View File
@@ -42,6 +42,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>,
@@ -90,23 +100,21 @@ 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,
};
@@ -151,7 +159,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(seatbelt_command, cwd, sandbox_policy, stdio_policy).await
}
fn create_seatbelt_command(
command: Vec<String>,
sandbox_policy: &SandboxPolicy,
cwd: &Path,
@@ -229,22 +247,47 @@ 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(
command,
cwd,
sandbox_policy,
StdioPolicy::RedirectForShellTool,
)
.await?;
consume_truncated_output(child, ctrl_c, timeout_ms).await
}
#[derive(Debug, Clone, Copy)]
pub enum StdioPolicy {
RedirectForShellTool,
Inherit,
}
/// Spawns the appropriate child process for the ExecParams.
async fn spawn_child(command: Vec<String>, cwd: PathBuf) -> std::io::Result<Child> {
pub(crate) async fn spawn_child(
command: Vec<String>,
cwd: PathBuf,
sandbox_policy: &SandboxPolicy,
stdio_policy: StdioPolicy,
) -> std::io::Result<Child> {
// 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(std::io::Error::new(
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"command args are empty",
));
@@ -254,21 +297,34 @@ async fn spawn_child(command: Vec<String>, cwd: PathBuf) -> std::io::Result<Chil
cmd.args(&command[1..]);
cmd.current_dir(cwd);
// 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());
if !sandbox_policy.has_full_network_access() {
cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1");
}
cmd.stdout(Stdio::piped())
.stderr(Stdio::piped())
.kill_on_drop(true)
.spawn()
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());
}
}
cmd.kill_on_drop(true).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>,
+24 -5
View File
@@ -9,7 +9,9 @@ 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;
use crate::protocol::SandboxPolicy;
use landlock::ABI;
@@ -49,8 +51,15 @@ pub async fn exec_linux(
.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
let ExecParams {
command,
cwd,
timeout_ms,
} = params;
let child =
spawn_command_under_landlock(command, &sandbox_policy, cwd, StdioPolicy::Inherit)
.await?;
consume_truncated_output(child, ctrl_c_copy, timeout_ms).await
})
})
.join();
@@ -65,10 +74,20 @@ pub async fn exec_linux(
}
}
pub async fn spawn_command_under_landlock(
command: Vec<String>,
sandbox_policy: &SandboxPolicy,
cwd: PathBuf,
stdio_policy: StdioPolicy,
) -> Result<tokio::process::Child> {
apply_sandbox_policy_to_current_thread(&sandbox_policy, &cwd)?;
spawn_child(command, cwd, &sandbox_policy, stdio_policy).await
}
/// 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,
fn apply_sandbox_policy_to_current_thread(
sandbox_policy: &SandboxPolicy,
cwd: &Path,
) -> Result<()> {
if !sandbox_policy.has_full_network_access() {