guardian network allowlist review

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

Author: charley-oai

codex-rs/core/src/codex.rs

@@ -10510,25 +10510,30 @@ mod tests {
     }
 
     #[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![
@@ -10596,8 +10601,8 @@ mod tests {
         };
 
         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);
@@ -10656,7 +10661,7 @@ mod tests {
         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;
@@ -10670,6 +10675,52 @@ mod tests {
         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 {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 {
@@ -10682,7 +10733,7 @@ mod tests {
                     arguments: serde_json::json!({
                         "cmd": "echo hi",
                         "sandbox_permissions": SandboxPermissions::RequireEscalated,
-                        "justification": "need unsandboxed execution",
+                        "justification": "need escalated permissions",
                     })
                     .to_string(),
                 },
@@ -10694,7 +10745,7 @@ mod tests {
         };
 
         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()
         );
 

codex-rs/core/src/exec_policy.rs

@@ -552,7 +552,7 @@ pub fn render_decision_for_unmatched_command(
                     // In restricted sandboxes (ReadOnly/WorkspaceWrite), do not prompt for
                     // non‑escalated, non‑dangerous commands — let the sandbox enforce
                     // restrictions (e.g., block network/write) without a user prompt.
-                    if sandbox_permissions.uses_additional_permissions() {
+                    if sandbox_permissions.requests_sandbox_override() {
                         Decision::Prompt
                     } else {
                         Decision::Allow
@@ -567,7 +567,7 @@ pub fn render_decision_for_unmatched_command(
                 Decision::Allow
             }
             SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } => {
-                if sandbox_permissions.uses_additional_permissions() {
+                if sandbox_permissions.requests_sandbox_override() {
                     Decision::Prompt
                 } else {
                     Decision::Allow
@@ -1592,7 +1592,8 @@ prefix_rule(pattern=["git"], decision="prompt")
     }
 
     #[tokio::test]
-    async fn exec_approval_requirement_rejects_unmatched_prompt_when_sandbox_rejection_enabled() {
+    async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_sandbox_rejection_enabled()
+     {
         let command = vec!["madeup-cmd".to_string()];
 
         let requirement = ExecPolicyManager::default()

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()
+        && matches!(approval_policy, AskForApproval::Never)
+    {
+        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())
     {

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

@@ -33,7 +33,6 @@ use crate::tools::runtimes::shell::ShellRuntimeBackend;
 use crate::tools::sandboxing::ToolCtx;
 use crate::tools::spec::ShellCommandBackendConfig;
 use codex_protocol::models::PermissionProfile;
-use codex_protocol::models::SandboxPermissions;
 
 pub struct ShellHandler;
 

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

@@ -112,8 +112,12 @@ enum NetworkApprovalOutcome {
     DeniedByPolicy(String),
 }
 
-fn allows_network_prompt(policy: AskForApproval) -> bool {
-    !matches!(policy, AskForApproval::Never | AskForApproval::Guardian)
+/// Whether an allowlist miss may be reviewed instead of hard-denied.
+///
+/// Guardian uses the same inline decision path as user approvals, even though
+/// it is not an interactive prompt.
+fn allows_network_approval_flow(policy: AskForApproval) -> bool {
+    !matches!(policy, AskForApproval::Never)
 }
 
 impl PendingApprovalDecision {
@@ -316,7 +320,7 @@ impl NetworkApprovalService {
             .await;
             return NetworkDecision::deny(REASON_NOT_ALLOWED);
         };
-        if !allows_network_prompt(turn_context.approval_policy.value()) {
+        if !allows_network_approval_flow(turn_context.approval_policy.value()) {
             pending.set_decision(PendingApprovalDecision::Deny).await;
             let mut pending_approvals = self.pending_host_approvals.lock().await;
             pending_approvals.remove(&key);
@@ -666,12 +670,12 @@ mod tests {
     }
 
     #[test]
-    fn never_policy_disables_network_prompts() {
-        assert!(!allows_network_prompt(AskForApproval::Never));
-        assert!(allows_network_prompt(AskForApproval::OnRequest));
-        assert!(allows_network_prompt(AskForApproval::OnFailure));
-        assert!(!allows_network_prompt(AskForApproval::Guardian));
-        assert!(allows_network_prompt(AskForApproval::UnlessTrusted));
+    fn only_never_policy_disables_network_approval_flow() {
+        assert!(!allows_network_approval_flow(AskForApproval::Never));
+        assert!(allows_network_approval_flow(AskForApproval::OnRequest));
+        assert!(allows_network_approval_flow(AskForApproval::OnFailure));
+        assert!(allows_network_approval_flow(AskForApproval::Guardian));
+        assert!(allows_network_approval_flow(AskForApproval::UnlessTrusted));
     }
 
     fn denied_blocked_request(host: &str) -> BlockedRequest {