fix: cleanup the contract of the general-purpose exec() function

codex-rs/core/src/exec.rs

@@ -251,8 +251,24 @@ pub fn build_exec_request(
     codex_linux_sandbox_exe: &Option<PathBuf>,
     use_legacy_landlock: bool,
 ) -> Result<ExecRequest> {
-    let windows_sandbox_level = params.windows_sandbox_level;
-    let enforce_managed_network = params.network.is_some();
+    let ExecParams {
+        command,
+        cwd,
+        mut env,
+        expiration,
+        capture_policy,
+        network,
+        windows_sandbox_level,
+        windows_sandbox_private_desktop,
+
+        // TODO: Should arg0 be set on the ExecRequest that is returned?
+        arg0: _,
+        // These fields are related to approvals, so can be ignored here.
+        justification: _,
+        sandbox_permissions: _,
+    } = params;
+
+    let enforce_managed_network = network.is_some();
     let sandbox_type = select_process_exec_tool_sandbox_type(
         file_system_sandbox_policy,
         network_sandbox_policy,
@@ -261,19 +277,6 @@ pub fn build_exec_request(
     );
     tracing::debug!("Sandbox type: {sandbox_type:?}");
 
-    let ExecParams {
-        command,
-        cwd,
-        mut env,
-        expiration,
-        capture_policy,
-        network,
-        sandbox_permissions: _,
-        windows_sandbox_level,
-        windows_sandbox_private_desktop,
-        justification: _,
-        arg0: _,
-    } = params;
     if let Some(network) = network.as_ref() {
         network.apply_to_env(&mut env);
     }
@@ -357,7 +360,8 @@ pub(crate) async fn execute_exec_request(
         windows_sandbox_level,
         windows_sandbox_private_desktop,
         sandbox_policy,
-        file_system_sandbox_policy,
+        // TODO(mbolin): Use file_system_sandbox_policy instead of sandbox_policy.
+        file_system_sandbox_policy: _,
         network_sandbox_policy,
         windows_sandbox_filesystem_overrides,
         arg0,
@@ -378,17 +382,19 @@ pub(crate) async fn execute_exec_request(
     };
 
     let start = Instant::now();
-    let raw_output_result = exec(
-        params,
-        sandbox,
-        &sandbox_policy,
-        &file_system_sandbox_policy,
-        windows_sandbox_filesystem_overrides.as_ref(),
-        network_sandbox_policy,
-        stdout_stream,
-        after_spawn,
-    )
-    .await;
+
+    //     #[cfg(target_os = "windows")]
+
+    let raw_output_result = if sandbox == SandboxType::WindowsRestrictedToken {
+        exec_windows_sandbox(
+            params,
+            &sandbox_policy,
+            windows_sandbox_filesystem_overrides.as_ref(),
+        )
+        .await
+    } else {
+        exec(params, network_sandbox_policy, stdout_stream, after_spawn).await
+    };
     let duration = start.elapsed();
     finalize_exec_result(raw_output_result, sandbox, duration)
 }
@@ -459,6 +465,15 @@ fn record_windows_sandbox_spawn_failure(
     }
 }
 
+#[cfg(not(target_os = "windows"))]
+async fn exec_windows_sandbox(
+    _params: ExecParams,
+    _sandbox_policy: &SandboxPolicy,
+    _windows_sandbox_filesystem_overrides: Option<&WindowsSandboxFilesystemOverrides>,
+) -> Result<RawExecToolCallOutput> {
+    unimplemented!("exec_windows_sandbox is only implemented on windows builds");
+}
+
 #[cfg(target_os = "windows")]
 async fn exec_windows_sandbox(
     params: ExecParams,
@@ -799,26 +814,24 @@ fn aggregate_output(
     }
 }
 
-#[allow(clippy::too_many_arguments)]
+/// This is a general-purpose function for executing a command specified by
+/// [ExecParams]. Events are reported via `stdout_stream`, if specified, and
+/// `after_spawn` is invoked once the child process has been spawned, before
+/// output consumption begins.
+///
+/// `network_sandbox_policy` is used to determine whether
+/// CODEX_SANDBOX_NETWORK_DISABLED=1 is added to the environment of the spawned
+/// process.
+///
+/// Note this command does not apply any sandboxing logic. The caller is
+/// responsible for constructing [ExecParams::command] to include any sandboxing
+/// wrapper args, as appropriate.
 async fn exec(
     params: ExecParams,
-    _sandbox: SandboxType,
-    _sandbox_policy: &SandboxPolicy,
-    _file_system_sandbox_policy: &FileSystemSandboxPolicy,
-    _windows_sandbox_filesystem_overrides: Option<&WindowsSandboxFilesystemOverrides>,
     network_sandbox_policy: NetworkSandboxPolicy,
     stdout_stream: Option<StdoutStream>,
     after_spawn: Option<Box<dyn FnOnce() + Send>>,
 ) -> Result<RawExecToolCallOutput> {
-    #[cfg(target_os = "windows")]
-    if _sandbox == SandboxType::WindowsRestrictedToken {
-        return exec_windows_sandbox(
-            params,
-            _sandbox_policy,
-            _windows_sandbox_filesystem_overrides,
-        )
-        .await;
-    }
     let ExecParams {
         command,
         cwd,
@@ -827,8 +840,14 @@ async fn exec(
         arg0,
         expiration,
         capture_policy,
+
+        // If applicable, these fields should have been honored upstream of
+        // this exec call.
         windows_sandbox_level: _,
-        ..
+        windows_sandbox_private_desktop: _,
+        // These fields are related to approvals, so can be ignored here.
+        sandbox_permissions: _,
+        justification: _,
     } = params;
     if let Some(network) = network.as_ref() {
         network.apply_to_env(&mut env);

codex-rs/core/src/exec_tests.rs

@@ -277,10 +277,6 @@ async fn exec_full_buffer_capture_ignores_expiration() -> Result<()> {
             justification: None,
             arg0: None,
         },
-        SandboxType::None,
-        &SandboxPolicy::DangerFullAccess,
-        &FileSystemSandboxPolicy::unrestricted(),
-        /*windows_sandbox_filesystem_overrides*/ None,
         NetworkSandboxPolicy::Enabled,
         /*stdout_stream*/ None,
         /*after_spawn*/ None,
@@ -317,10 +313,6 @@ async fn exec_full_buffer_capture_keeps_io_drain_timeout_when_descendant_holds_p
                 justification: None,
                 arg0: None,
             },
-            SandboxType::None,
-            &SandboxPolicy::DangerFullAccess,
-            &FileSystemSandboxPolicy::unrestricted(),
-            /*windows_sandbox_filesystem_overrides*/ None,
             NetworkSandboxPolicy::Enabled,
             /*stdout_stream*/ None,
             /*after_spawn*/ None,
@@ -931,10 +923,6 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()>
 
     let output = exec(
         params,
-        SandboxType::None,
-        &SandboxPolicy::new_read_only_policy(),
-        &FileSystemSandboxPolicy::from(&SandboxPolicy::new_read_only_policy()),
-        /*windows_sandbox_filesystem_overrides*/ None,
         NetworkSandboxPolicy::Restricted,
         /*stdout_stream*/ None,
         /*after_spawn*/ None,