split sandbox permission validation modes

Co-authored-by: Codex <noreply@openai.com>

codex-rs/core/src/codex_tests.rs

@@ -3759,25 +3759,30 @@ async fn sample_rollout(
 }
 
 #[tokio::test]
-async fn rejects_escalated_permissions_when_policy_not_on_request() {
+async fn rejects_additional_permissions_when_policy_not_on_request() {
     use crate::exec::ExecParams;
+    use crate::features::Feature;
     use crate::protocol::AskForApproval;
     use crate::protocol::SandboxPolicy;
     use crate::sandboxing::SandboxPermissions;
     use crate::turn_diff_tracker::TurnDiffTracker;
     use std::collections::HashMap;
 
-    let (session, mut turn_context_raw) = make_session_and_context().await;
+    let (mut session, mut turn_context_raw) = make_session_and_context().await;
     // Ensure policy is NOT OnRequest so the early rejection path triggers
     turn_context_raw
         .approval_policy
         .set(AskForApproval::OnFailure)
         .expect("test setup should allow updating approval policy");
+    session
+        .features
+        .enable(Feature::RequestPermissions)
+        .expect("test setup should allow enabling request permissions");
     let session = Arc::new(session);
     let mut turn_context = Arc::new(turn_context_raw);
 
     let timeout_ms = 1000;
-    let sandbox_permissions = SandboxPermissions::RequireEscalated;
+    let sandbox_permissions = SandboxPermissions::WithAdditionalPermissions;
     let params = ExecParams {
         command: if cfg!(windows) {
             vec![
@@ -3845,8 +3850,8 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
     };
 
     let expected = format!(
-        "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}",
-        policy = turn_context.approval_policy.value()
+        "approval policy is {policy:?}; reject command — you cannot request additional permissions unless the approval policy is OnRequest",
+        policy = turn_context.approval_policy.value(),
     );
 
     pretty_assertions::assert_eq!(output, expected);
@@ -3909,7 +3914,126 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
     assert!(exec_output.output.contains("hi"));
 }
 #[tokio::test]
-async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request() {
+async fn unified_exec_rejects_additional_permissions_when_policy_not_on_request() {
+    use crate::protocol::AskForApproval;
+    use crate::sandboxing::SandboxPermissions;
+    use crate::turn_diff_tracker::TurnDiffTracker;
+
+    let (session, mut turn_context_raw) = make_session_and_context().await;
+    turn_context_raw
+        .approval_policy
+        .set(AskForApproval::OnFailure)
+        .expect("test setup should allow updating approval policy");
+    let session = Arc::new(session);
+    let turn_context = Arc::new(turn_context_raw);
+    let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
+
+    let handler = UnifiedExecHandler;
+    let resp = handler
+        .handle(ToolInvocation {
+            session: Arc::clone(&session),
+            turn: Arc::clone(&turn_context),
+            tracker: Arc::clone(&tracker),
+            call_id: "exec-call".to_string(),
+            tool_name: "exec_command".to_string(),
+            payload: ToolPayload::Function {
+                arguments: serde_json::json!({
+                    "cmd": "echo hi",
+                    "sandbox_permissions": SandboxPermissions::WithAdditionalPermissions,
+                    "justification": "need additional sandbox permissions",
+                })
+                .to_string(),
+            },
+        })
+        .await;
+
+    let Err(FunctionCallError::RespondToModel(output)) = resp else {
+        panic!("expected error result");
+    };
+
+    let expected = format!(
+        "approval policy is {policy:?}; reject command — you cannot ask for additional sandbox permissions if the approval policy is not OnRequest",
+        policy = turn_context.approval_policy.value()
+    );
+
+    pretty_assertions::assert_eq!(output, expected);
+}
+
+#[tokio::test]
+async fn rejects_escalated_permissions_when_policy_is_on_failure() {
+    use crate::exec::ExecParams;
+    use crate::protocol::AskForApproval;
+    use crate::sandboxing::SandboxPermissions;
+    use crate::turn_diff_tracker::TurnDiffTracker;
+    use std::collections::HashMap;
+
+    let (session, mut turn_context_raw) = make_session_and_context().await;
+    turn_context_raw
+        .approval_policy
+        .set(AskForApproval::OnFailure)
+        .expect("test setup should allow updating approval policy");
+    let session = Arc::new(session);
+    let turn_context = Arc::new(turn_context_raw);
+
+    let params = ExecParams {
+        command: if cfg!(windows) {
+            vec![
+                "cmd.exe".to_string(),
+                "/C".to_string(),
+                "echo hi".to_string(),
+            ]
+        } else {
+            vec![
+                "/bin/sh".to_string(),
+                "-c".to_string(),
+                "echo hi".to_string(),
+            ]
+        },
+        cwd: turn_context.cwd.clone(),
+        expiration: 1000.into(),
+        env: HashMap::new(),
+        network: None,
+        sandbox_permissions: SandboxPermissions::RequireEscalated,
+        windows_sandbox_level: turn_context.windows_sandbox_level,
+        justification: Some("test".to_string()),
+        arg0: None,
+    };
+
+    let handler = ShellHandler;
+    let resp = handler
+        .handle(ToolInvocation {
+            session: Arc::clone(&session),
+            turn: Arc::clone(&turn_context),
+            tracker: Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())),
+            call_id: "test-call".to_string(),
+            tool_name: "shell".to_string(),
+            payload: ToolPayload::Function {
+                arguments: serde_json::json!({
+                    "command": params.command.clone(),
+                    "workdir": Some(turn_context.cwd.to_string_lossy().to_string()),
+                    "timeout_ms": params.expiration.timeout_ms(),
+                    "sandbox_permissions": params.sandbox_permissions,
+                    "justification": params.justification.clone(),
+                })
+                .to_string(),
+            },
+        })
+        .await;
+
+    let Err(FunctionCallError::RespondToModel(output)) = resp else {
+        panic!("expected error result");
+    };
+
+    let expected = format!(
+        "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}",
+        policy = turn_context.approval_policy.value()
+    );
+
+    pretty_assertions::assert_eq!(output, expected);
+}
+
+#[tokio::test]
+async fn unified_exec_rejects_escalated_permissions_when_policy_is_on_failure() {
     use crate::protocol::AskForApproval;
     use crate::sandboxing::SandboxPermissions;
     use crate::turn_diff_tracker::TurnDiffTracker;
@@ -3935,7 +4059,7 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request()
                 arguments: serde_json::json!({
                     "cmd": "echo hi",
                     "sandbox_permissions": SandboxPermissions::RequireEscalated,
-                    "justification": "need unsandboxed execution",
+                    "justification": "need escalated permissions",
                 })
                 .to_string(),
             },
@@ -3947,7 +4071,53 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request()
     };
 
     let expected = format!(
-        "approval policy is {policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {policy:?}",
+        "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}",
+        policy = turn_context.approval_policy.value()
+    );
+
+    pretty_assertions::assert_eq!(output, expected);
+}
+
+#[tokio::test]
+async fn unified_exec_rejects_escalated_permissions_when_policy_is_never() {
+    use crate::protocol::AskForApproval;
+    use crate::sandboxing::SandboxPermissions;
+    use crate::turn_diff_tracker::TurnDiffTracker;
+
+    let (session, mut turn_context_raw) = make_session_and_context().await;
+    turn_context_raw
+        .approval_policy
+        .set(AskForApproval::Never)
+        .expect("test setup should allow updating approval policy");
+    let session = Arc::new(session);
+    let turn_context = Arc::new(turn_context_raw);
+    let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
+
+    let handler = UnifiedExecHandler;
+    let resp = handler
+        .handle(ToolInvocation {
+            session: Arc::clone(&session),
+            turn: Arc::clone(&turn_context),
+            tracker: Arc::clone(&tracker),
+            call_id: "exec-call".to_string(),
+            tool_name: "exec_command".to_string(),
+            payload: ToolPayload::Function {
+                arguments: serde_json::json!({
+                    "cmd": "echo hi",
+                    "sandbox_permissions": SandboxPermissions::RequireEscalated,
+                    "justification": "need escalated permissions",
+                })
+                .to_string(),
+            },
+        })
+        .await;
+
+    let Err(FunctionCallError::RespondToModel(output)) = resp else {
+        panic!("expected error result");
+    };
+
+    let expected = format!(
+        "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}",
         policy = turn_context.approval_policy.value()
     );
 

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

@@ -102,6 +102,14 @@ pub(super) fn normalize_and_validate_additional_permissions(
         SandboxPermissions::WithAdditionalPermissions
     );
 
+    if sandbox_permissions.requires_escalated_permissions()
+        && approval_policy != AskForApproval::OnRequest
+    {
+        return Err(format!(
+            "approval policy is {approval_policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {approval_policy:?}"
+        ));
+    }
+
     if !request_permission_enabled
         && (uses_additional_permissions || additional_permissions.is_some())
     {
@@ -112,7 +120,7 @@ pub(super) fn normalize_and_validate_additional_permissions(
     }
 
     if uses_additional_permissions {
-        if !matches!(approval_policy, AskForApproval::OnRequest) {
+        if approval_policy != AskForApproval::OnRequest {
             return Err(format!(
                 "approval policy is {approval_policy:?}; reject command — you cannot request additional permissions unless the approval policy is OnRequest"
             ));

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

@@ -341,16 +341,15 @@ impl ShellHandler {
         )
         .map_err(FunctionCallError::RespondToModel)?;
 
-        // Approval policy guard for explicit escalation in non-OnRequest modes.
-        if exec_params.sandbox_permissions.requests_sandbox_override()
-            && !matches!(
-                turn.approval_policy.value(),
-                codex_protocol::protocol::AskForApproval::OnRequest
-            )
+        // Approval policy guard for the sandboxed additional-permissions flow.
+        if exec_params
+            .sandbox_permissions
+            .uses_additional_permissions()
+            && turn.approval_policy.value() != codex_protocol::protocol::AskForApproval::OnRequest
         {
             let approval_policy = turn.approval_policy.value();
             return Err(FunctionCallError::RespondToModel(format!(
-                "approval policy is {approval_policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {approval_policy:?}"
+                "approval policy is {approval_policy:?}; reject command — you should not ask for additional sandbox permissions if the approval policy is not OnRequest"
             )));
         }
 

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

@@ -171,16 +171,14 @@ impl ToolHandler for UnifiedExecHandler {
                 let request_permission_enabled =
                     session.features().enabled(Feature::RequestPermissions);
 
-                if sandbox_permissions.requests_sandbox_override()
-                    && !matches!(
-                        context.turn.approval_policy.value(),
-                        codex_protocol::protocol::AskForApproval::OnRequest
-                    )
+                if sandbox_permissions.uses_additional_permissions()
+                    && context.turn.approval_policy.value()
+                        != codex_protocol::protocol::AskForApproval::OnRequest
                 {
                     let approval_policy = context.turn.approval_policy.value();
                     manager.release_process_id(&process_id).await;
                     return Err(FunctionCallError::RespondToModel(format!(
-                        "approval policy is {approval_policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {approval_policy:?}"
+                        "approval policy is {approval_policy:?}; reject command — you cannot ask for additional sandbox permissions if the approval policy is not OnRequest"
                     )));
                 }