Merge 63ab6984ed into sapling-pr-archive-bolinfest

codex-rs/cli/src/landlock.rs

@@ -3,6 +3,7 @@
 //! 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::spawn_child;
 use codex_core::protocol::SandboxPolicy;
 use std::os::unix::process::ExitStatusExt;
 use std::process;
@@ -19,8 +20,9 @@ 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()?;
+        codex_core::linux::apply_sandbox_policy_to_current_thread(&sandbox_policy, &cwd)?;
+        let child = spawn_child(command, cwd, sandbox_policy)?;
+        let status = child.status()?;
         Ok(status)
     });
     let status = handle

codex-rs/cli/src/seatbelt.rs

@@ -1,4 +1,4 @@
-use codex_core::exec::create_seatbelt_command;
+use codex_core::exec::spawn_command_under_seatbelt;
 use codex_core::protocol::SandboxPolicy;
 
 pub async fn run_seatbelt(
@@ -6,10 +6,8 @@ pub async fn run_seatbelt(
     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()
+    let child = spawn_command_under_seatbelt(command, &sandbox_policy, cwd).await;
+    let status = child
         .map_err(|e| anyhow::anyhow!("Failed to spawn command: {}", e))?
         .wait()
         .await

codex-rs/core/src/exec.rs

@@ -12,6 +12,7 @@ use std::time::Instant;
 use tokio::io::AsyncRead;
 use tokio::io::AsyncReadExt;
 use tokio::io::BufReader;
+use tokio::process::Child;
 use tokio::process::Command;
 use tokio::sync::Notify;
 
@@ -41,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>,
@@ -89,23 +100,15 @@ 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,
-            )
-            .await
+            let child = spawn_command_under_seatbelt(command, sandbox_policy, cwd).await?;
+            consume_truncated_output(child, ctrl_c, timeout_ms).await
         }
         SandboxType::LinuxSeccomp => exec_linux(params, ctrl_c, sandbox_policy).await,
     };
@@ -150,7 +153,16 @@ 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,
+) -> std::io::Result<Child> {
+    let seatbelt_command = create_seatbelt_command(command, sandbox_policy, &cwd);
+    spawn_child(seatbelt_command, cwd, sandbox_policy).await
+}
+
+fn create_seatbelt_command(
     command: Vec<String>,
     sandbox_policy: &SandboxPolicy,
     cwd: &Path,
@@ -229,39 +241,65 @@ pub struct ExecToolCallOutput {
 }
 
 pub async fn exec(
-    ExecParams {
+    params: ExecParams,
+    sandbox_policy: &SandboxPolicy,
+    ctrl_c: Arc<Notify>,
+) -> Result<RawExecToolCallOutput> {
+    let ExecParams {
         command,
         cwd,
         timeout_ms,
-    }: ExecParams,
+    } = params;
+    let child = spawn_child(command, cwd, sandbox_policy).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,
+    sandbox_policy: &SandboxPolicy,
+) -> std::io::Result<Child> {
+    // For now, we take `SandboxPolicy` as a parameter to exec() 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",
+        ));
+    }
+
+    let mut cmd = Command::new(&command[0]);
+    cmd.args(&command[1..]);
+    cmd.current_dir(cwd);
+
+    if !sandbox_policy.has_full_network_access() {
+        cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1");
+    }
+
+    // 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())
+        .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(
+    mut child: Child,
     ctrl_c: Arc<Notify>,
+    timeout_ms: Option<u64>,
 ) -> Result<RawExecToolCallOutput> {
-    let mut child = {
-        if command.is_empty() {
-            return Err(CodexErr::Io(io::Error::new(
-                io::ErrorKind::InvalidInput,
-                "command args are empty",
-            )));
-        }
-
-        let mut cmd = Command::new(&command[0]);
-        if command.len() > 1 {
-            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());
-
-        cmd.stdout(Stdio::piped())
-            .stderr(Stdio::piped())
-            .kill_on_drop(true)
-            .spawn()?
-    };
-
     let stdout_handle = tokio::spawn(read_capped(
         BufReader::new(child.stdout.take().expect("stdout is not piped")),
         MAX_STREAM_OUTPUT,

codex-rs/core/src/linux.rs

@@ -49,8 +49,8 @@ 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
+            apply_sandbox_policy_to_current_thread(&sandbox_policy, &params.cwd)?;
+            exec(params, sandbox_policy, ctrl_c_copy).await
         })
     })
     .join();
@@ -68,7 +68,7 @@ pub async fn exec_linux(
 /// 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,
+    sandbox_policy: &SandboxPolicy,
     cwd: &Path,
 ) -> Result<()> {
     if !sandbox_policy.has_full_network_access() {

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;
 

codex-rs/mcp-client/src/mcp_client.rs

@@ -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())