From 08bf18a078adc69a6e29f8cd229d3ee335695366 Mon Sep 17 00:00:00 2001 From: Qiyao Qin Date: Sat, 14 Mar 2026 15:31:19 -0700 Subject: [PATCH] deny override for network policy --- codex-rs/core/src/tools/network_approval.rs | 23 +++++++++++++++---- .../core/src/tools/network_approval_tests.rs | 21 +++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index f27c761acb..b47922ed65 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -118,6 +118,21 @@ fn allows_network_approval_flow(policy: AskForApproval) -> bool { !matches!(policy, AskForApproval::Never) } +#[cfg(test)] +fn pending_decision_for_network_review( + review_decision: &ReviewDecision, +) -> Option { + match review_decision { + ReviewDecision::Approved => Some(PendingApprovalDecision::AllowOnce), + ReviewDecision::ApprovedForSession => Some(PendingApprovalDecision::AllowForSession), + ReviewDecision::ApprovedOverrideCommand { .. } + | ReviewDecision::ApprovedExecpolicyAmendment { .. } + | ReviewDecision::Denied + | ReviewDecision::Abort => Some(PendingApprovalDecision::Deny), + ReviewDecision::NetworkPolicyAmendment { .. } => None, + } +} + impl PendingApprovalDecision { fn to_network_decision(self) -> NetworkDecision { match self { @@ -389,9 +404,7 @@ impl NetworkApprovalService { let mut cache_session_deny = false; let resolved = match approval_decision { - ReviewDecision::Approved - | ReviewDecision::ApprovedOverrideCommand { .. } - | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { + ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { PendingApprovalDecision::AllowOnce } ReviewDecision::ApprovedForSession => PendingApprovalDecision::AllowForSession, @@ -467,7 +480,9 @@ impl NetworkApprovalService { PendingApprovalDecision::Deny } }, - ReviewDecision::Denied | ReviewDecision::Abort => { + ReviewDecision::Denied + | ReviewDecision::Abort + | ReviewDecision::ApprovedOverrideCommand { .. } => { if routes_approval_to_guardian(&turn_context) { if let Some(owner_call) = owner_call.as_ref() { self.record_call_outcome( diff --git a/codex-rs/core/src/tools/network_approval_tests.rs b/codex-rs/core/src/tools/network_approval_tests.rs index 820fd4303b..44fef07f1f 100644 --- a/codex-rs/core/src/tools/network_approval_tests.rs +++ b/codex-rs/core/src/tools/network_approval_tests.rs @@ -1,6 +1,8 @@ use super::*; use codex_network_proxy::BlockedRequestArgs; +use codex_protocol::approvals::ExecPolicyAmendment; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::ReviewDecision; use pretty_assertions::assert_eq; #[tokio::test] @@ -137,6 +139,25 @@ fn only_never_policy_disables_network_approval_flow() { assert!(allows_network_approval_flow(AskForApproval::UnlessTrusted)); } +#[test] +fn network_review_rejects_command_and_execpolicy_overrides() { + assert_eq!( + pending_decision_for_network_review(&ReviewDecision::ApprovedOverrideCommand { + command: vec!["echo".to_string(), "override".to_string()], + }), + Some(PendingApprovalDecision::Deny) + ); + assert_eq!( + pending_decision_for_network_review(&ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: ExecPolicyAmendment::new(vec![ + "echo".to_string(), + "override".to_string(), + ]), + }), + Some(PendingApprovalDecision::Deny) + ); +} + fn denied_blocked_request(host: &str) -> BlockedRequest { BlockedRequest::new(BlockedRequestArgs { host: host.to_string(),