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

`exec()` had a number of arguments that were unused, making the function
signature misleading. This PR aims to clean things up to clarify the
role of this function and to clarify which fields of `ExecParams` are
unused and why.

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,21 +382,40 @@ pub(crate) async fn execute_exec_request(
     };
 
     let start = Instant::now();
-    let raw_output_result = exec(
+    let raw_output_result = get_raw_output_result(
         params,
-        sandbox,
-        &sandbox_policy,
-        &file_system_sandbox_policy,
-        windows_sandbox_filesystem_overrides.as_ref(),
         network_sandbox_policy,
         stdout_stream,
         after_spawn,
+        sandbox,
+        &sandbox_policy,
+        windows_sandbox_filesystem_overrides.as_ref(),
     )
     .await;
     let duration = start.elapsed();
     finalize_exec_result(raw_output_result, sandbox, duration)
 }
 
+async fn get_raw_output_result(
+    params: ExecParams,
+    network_sandbox_policy: NetworkSandboxPolicy,
+    stdout_stream: Option<StdoutStream>,
+    after_spawn: Option<Box<dyn FnOnce() + Send>>,
+    #[cfg_attr(not(windows), allow(unused_variables))] sandbox: SandboxType,
+    #[cfg_attr(not(windows), allow(unused_variables))] sandbox_policy: &SandboxPolicy,
+    #[cfg_attr(not(windows), allow(unused_variables))] windows_sandbox_filesystem_overrides: Option<
+        &WindowsSandboxFilesystemOverrides,
+    >,
+) -> Result<RawExecToolCallOutput> {
+    #[cfg(target_os = "windows")]
+    if sandbox == SandboxType::WindowsRestrictedToken {
+        return exec_windows_sandbox(params, sandbox_policy, windows_sandbox_filesystem_overrides)
+            .await;
+    }
+
+    exec(params, network_sandbox_policy, stdout_stream, after_spawn).await
+}
+
 #[cfg(target_os = "windows")]
 fn extract_create_process_as_user_error_code(err: &str) -> Option<String> {
     let marker = "CreateProcessAsUserW failed: ";
@@ -799,26 +822,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 +848,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,