Apply metadata preflight consistently

Author: evawong-oai

codex-rs/cli/src/debug_sandbox.rs

@@ -35,10 +35,6 @@ use crate::exit_status::handle_exit_status;
 #[cfg(target_os = "macos")]
 use seatbelt::DenialLogger;
 
-#[cfg(target_os = "linux")]
-const LINUX_SANDBOX_FORWARDED_SIGNALS: &[libc::c_int] =
-    &[libc::SIGHUP, libc::SIGINT, libc::SIGQUIT, libc::SIGTERM];
-
 #[cfg(target_os = "macos")]
 pub async fn run_command_under_seatbelt(
     command: SeatbeltCommand,
@@ -149,6 +145,7 @@ async fn run_command_under_sandbox(
     if let Some(reason) = codex_shell_command::metadata_write_forbidden_reason(
         &command,
         cwd.as_path(),
+        sandbox_policy_cwd.as_path(),
         &config.permissions.file_system_sandbox_policy(),
     ) {
         anyhow::bail!("{reason}");
@@ -272,9 +269,6 @@ async fn run_command_under_sandbox(
         denial_logger.on_child_spawn(&child);
     }
 
-    #[cfg(target_os = "linux")]
-    let status = wait_for_debug_sandbox_child(&mut child).await?;
-    #[cfg(not(target_os = "linux"))]
     let status = child.wait().await?;
 
     #[cfg(target_os = "macos")]
@@ -452,96 +446,13 @@ async fn spawn_debug_sandbox_child(
         cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1");
     }
 
-    #[cfg(target_os = "linux")]
-    {
-        let parent_pid = unsafe { libc::getpid() };
-        // SAFETY: `pre_exec` runs in the child immediately before exec. The
-        // closure only adjusts the child signal mask, installs a parent-death
-        // signal, and checks the inherited parent pid to close the fork/exec
-        // race.
-        unsafe {
-            cmd.pre_exec(move || {
-                block_linux_sandbox_forwarded_signals()?;
-                if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) == -1 {
-                    return Err(std::io::Error::last_os_error());
-                }
-                if libc::getppid() != parent_pid {
-                    libc::raise(libc::SIGTERM);
-                }
-                Ok(())
-            });
-        }
-    }
-
     cmd.stdin(Stdio::inherit())
         .stdout(Stdio::inherit())
         .stderr(Stdio::inherit())
         .kill_on_drop(true)
         .spawn()
 }
 
-#[cfg(target_os = "linux")]
-async fn wait_for_debug_sandbox_child(
-    child: &mut Child,
-) -> std::io::Result<std::process::ExitStatus> {
-    let child_pid = child.id().map(|pid| pid as libc::pid_t);
-    tokio::select! {
-        status = child.wait() => status,
-        signal = recv_linux_sandbox_forwarded_signal() => {
-            let signal = signal?;
-            if let Some(child_pid) = child_pid {
-                signal_debug_sandbox_child(child_pid, signal)?;
-            }
-            child.wait().await
-        }
-    }
-}
-
-#[cfg(target_os = "linux")]
-async fn recv_linux_sandbox_forwarded_signal() -> std::io::Result<libc::c_int> {
-    use tokio::signal::unix::SignalKind;
-    use tokio::signal::unix::signal;
-
-    let mut sighup = signal(SignalKind::hangup())?;
-    let mut sigint = signal(SignalKind::interrupt())?;
-    let mut sigquit = signal(SignalKind::quit())?;
-    let mut sigterm = signal(SignalKind::terminate())?;
-
-    let signal = tokio::select! {
-        _ = sighup.recv() => libc::SIGHUP,
-        _ = sigint.recv() => libc::SIGINT,
-        _ = sigquit.recv() => libc::SIGQUIT,
-        _ = sigterm.recv() => libc::SIGTERM,
-    };
-    Ok(signal)
-}
-
-#[cfg(target_os = "linux")]
-fn signal_debug_sandbox_child(pid: libc::pid_t, signal: libc::c_int) -> std::io::Result<()> {
-    if unsafe { libc::kill(pid, signal) } < 0 {
-        let err = std::io::Error::last_os_error();
-        if err.raw_os_error() != Some(libc::ESRCH) {
-            return Err(err);
-        }
-    }
-    Ok(())
-}
-
-#[cfg(target_os = "linux")]
-fn block_linux_sandbox_forwarded_signals() -> std::io::Result<()> {
-    let mut blocked: libc::sigset_t = unsafe { std::mem::zeroed() };
-    unsafe {
-        libc::sigemptyset(&mut blocked);
-        for signal in LINUX_SANDBOX_FORWARDED_SIGNALS {
-            libc::sigaddset(&mut blocked, *signal);
-        }
-        if libc::sigprocmask(libc::SIG_BLOCK, &blocked, std::ptr::null_mut()) < 0 {
-            return Err(std::io::Error::last_os_error());
-        }
-    }
-    Ok(())
-}
-
 #[cfg(target_os = "windows")]
 mod windows_stdio_bridge {
     use std::io::Read;

codex-rs/core/src/tools/handlers/shell.rs

@@ -35,9 +35,11 @@ use crate::tools::runtimes::shell::ShellRequest;
 use crate::tools::runtimes::shell::ShellRuntime;
 use crate::tools::runtimes::shell::ShellRuntimeBackend;
 use crate::tools::sandboxing::ToolCtx;
+use crate::tools::sandboxing::apply_protected_metadata_write_preflight;
 use codex_features::Feature;
 use codex_protocol::models::AdditionalPermissionProfile;
 use codex_protocol::protocol::ExecCommandSource;
+use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
 use codex_shell_command::is_safe_command::is_known_safe_command;
 use codex_tools::ShellCommandBackendConfig;
 
@@ -513,7 +515,11 @@ impl ShellHandler {
         );
         emitter.begin(event_ctx).await;
 
-        let file_system_sandbox_policy = turn.file_system_sandbox_policy();
+        let base_file_system_sandbox_policy = turn.file_system_sandbox_policy();
+        let file_system_sandbox_policy = effective_file_system_sandbox_policy(
+            &base_file_system_sandbox_policy,
+            normalized_additional_permissions.as_ref(),
+        );
         let exec_approval_requirement = session
             .services
             .exec_policy
@@ -531,14 +537,14 @@ impl ShellHandler {
                 prefix_rule,
             })
             .await;
-        let exec_approval_requirement = codex_shell_command::metadata_write_forbidden_reason(
+        let exec_approval_requirement = apply_protected_metadata_write_preflight(
+            exec_approval_requirement,
+            effective_additional_permissions.sandbox_permissions,
             &exec_params.command,
             &exec_params.cwd,
+            turn.cwd.as_path(),
             &file_system_sandbox_policy,
-        )
-        .map_or(exec_approval_requirement, |reason| {
-            crate::tools::sandboxing::ExecApprovalRequirement::Forbidden { reason }
-        });
+        );
 
         let req = ShellRequest {
             command: exec_params.command.clone(),

codex-rs/core/src/tools/handlers/unified_exec.rs

@@ -1,3 +1,4 @@
+use crate::exec_policy::ExecApprovalRequest;
 use crate::function_tool::FunctionCallError;
 use crate::maybe_emit_implicit_skill_invocation;
 use crate::sandboxing::SandboxPermissions;
@@ -19,6 +20,7 @@ use crate::tools::registry::PostToolUsePayload;
 use crate::tools::registry::PreToolUsePayload;
 use crate::tools::registry::ToolHandler;
 use crate::tools::registry::ToolKind;
+use crate::tools::sandboxing::apply_protected_metadata_write_preflight;
 use crate::unified_exec::ExecCommandRequest;
 use crate::unified_exec::UnifiedExecContext;
 use crate::unified_exec::UnifiedExecError;
@@ -32,6 +34,7 @@ use codex_otel::TOOL_CALL_UNIFIED_EXEC_METRIC;
 use codex_protocol::models::AdditionalPermissionProfile;
 use codex_protocol::protocol::EventMsg;
 use codex_protocol::protocol::TerminalInteractionEvent;
+use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
 use codex_shell_command::is_safe_command::is_known_safe_command;
 use codex_tools::UnifiedExecShellMode;
 use codex_utils_output_truncation::TruncationPolicy;
@@ -330,6 +333,40 @@ impl ToolHandler for UnifiedExecHandler {
                     });
                 }
 
+                let base_file_system_sandbox_policy = context.turn.file_system_sandbox_policy();
+                let file_system_sandbox_policy = effective_file_system_sandbox_policy(
+                    &base_file_system_sandbox_policy,
+                    normalized_additional_permissions.as_ref(),
+                );
+                let exec_approval_requirement = context
+                    .session
+                    .services
+                    .exec_policy
+                    .create_exec_approval_requirement_for_command(ExecApprovalRequest {
+                        command: &command,
+                        approval_policy: context.turn.approval_policy.value(),
+                        permission_profile: context.turn.permission_profile(),
+                        file_system_sandbox_policy: &file_system_sandbox_policy,
+                        sandbox_cwd: context.turn.cwd.as_path(),
+                        sandbox_permissions: if effective_additional_permissions
+                            .permissions_preapproved
+                        {
+                            SandboxPermissions::UseDefault
+                        } else {
+                            effective_additional_permissions.sandbox_permissions
+                        },
+                        prefix_rule: prefix_rule.clone(),
+                    })
+                    .await;
+                let exec_approval_requirement = apply_protected_metadata_write_preflight(
+                    exec_approval_requirement,
+                    effective_additional_permissions.sandbox_permissions,
+                    &command,
+                    &cwd,
+                    context.turn.cwd.as_path(),
+                    &file_system_sandbox_policy,
+                );
+
                 emit_unified_exec_tty_metric(&turn.session_telemetry, tty);
                 match manager
                     .exec_command(
@@ -345,10 +382,11 @@ impl ToolHandler for UnifiedExecHandler {
                             sandbox_permissions: effective_additional_permissions
                                 .sandbox_permissions,
                             additional_permissions: normalized_additional_permissions,
+                            #[cfg(unix)]
                             additional_permissions_preapproved: effective_additional_permissions
                                 .permissions_preapproved,
                             justification,
-                            prefix_rule,
+                            exec_approval_requirement,
                         },
                         &context,
                     )

codex-rs/core/src/tools/sandboxing.rs

@@ -34,6 +34,7 @@ use serde::Serialize;
 use std::collections::HashMap;
 use std::fmt::Debug;
 use std::hash::Hash;
+use std::path::Path;
 use std::sync::Arc;
 
 #[derive(Clone, Default, Debug)]
@@ -194,6 +195,50 @@ impl ExecApprovalRequirement {
     }
 }
 
+/// Applies an early UX check for protected metadata writes only to commands
+/// that would otherwise run in the sandbox. Explicit sandbox-bypass approval
+/// paths keep their existing approval semantics.
+pub(crate) fn apply_protected_metadata_write_preflight(
+    exec_approval_requirement: ExecApprovalRequirement,
+    sandbox_permissions: SandboxPermissions,
+    command: &[String],
+    command_cwd: &Path,
+    sandbox_policy_cwd: &Path,
+    file_system_sandbox_policy: &FileSystemSandboxPolicy,
+) -> ExecApprovalRequirement {
+    let bypasses_sandbox = matches!(
+        &exec_approval_requirement,
+        ExecApprovalRequirement::Skip {
+            bypass_sandbox: true,
+            ..
+        }
+    );
+    let approval_is_for_sandbox_override = sandbox_permissions.requests_sandbox_override()
+        && matches!(
+            &exec_approval_requirement,
+            ExecApprovalRequirement::NeedsApproval { .. }
+        );
+    if bypasses_sandbox
+        || approval_is_for_sandbox_override
+        || matches!(
+            &exec_approval_requirement,
+            ExecApprovalRequirement::Forbidden { .. }
+        )
+    {
+        return exec_approval_requirement;
+    }
+
+    codex_shell_command::metadata_write_forbidden_reason(
+        command,
+        command_cwd,
+        sandbox_policy_cwd,
+        file_system_sandbox_policy,
+    )
+    .map_or(exec_approval_requirement, |reason| {
+        ExecApprovalRequirement::Forbidden { reason }
+    })
+}
+
 /// - Never, OnFailure: do not ask
 /// - OnRequest: ask unless filesystem access is unrestricted
 /// - Granular: ask unless filesystem access is unrestricted, but auto-reject