From 3834c47ccb91adeef82e7d95f7ecdec5cde6dda8 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 29 Jul 2026 22:23:29 +0000 Subject: [PATCH] Deny network access when an allow amendment fails (#36037) ## Why A failed network policy amendment must not grant access to the requested host or approve it for the rest of the session. ## What changed - Only approve the host for the session after the allow amendment is applied successfully. - Otherwise deny the pending request and record a policy-denial outcome for its owning call. ## Testing Added a managed-network regression test that submits an invalid allow amendment, verifies that the request is blocked, and confirms that retrying the host prompts for approval again. GitOrigin-RevId: b2014d19128133abd5e19b8a7e4eb27810a45306 --- codex-rs/core/src/tools/network_approval.rs | 23 ++++-- codex-rs/core/tests/suite/network_approval.rs | 79 +++++++++++++++++++ 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 3a858a0b5f..056d29b88a 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -920,12 +920,25 @@ impl NetworkApprovalService { .await; } } - { - let mut denied_hosts = self.session_denied_hosts.lock().await; - denied_hosts.remove(&key); + if pending_owner.decision_on_drop == PendingApprovalDecision::AllowForSession { + { + let mut denied_hosts = self.session_denied_hosts.lock().await; + denied_hosts.remove(&key); + } + self.session_approved_hosts.lock().await.insert(key.clone()); + PendingApprovalDecision::AllowForSession + } else { + if let Some(owner_call) = owner_call.as_ref() { + self.record_call_outcome( + &owner_call.registration_id, + NetworkApprovalOutcome::DeniedByPolicy( + policy_denial_message.clone(), + ), + ) + .await; + } + PendingApprovalDecision::Deny } - self.session_approved_hosts.lock().await.insert(key.clone()); - PendingApprovalDecision::AllowForSession } NetworkPolicyRuleAction::Deny => { match session diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index f98edf25c0..4eb2383250 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -682,6 +682,85 @@ async fn allowing_network_policy_amendment_persists_context_and_bypasses_prompt( Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[cfg_attr( + not(target_os = "linux"), + ignore = "requires the trusted Linux proxy bridge" +)] +async fn failed_network_policy_amendment_denies_request_and_does_not_approve_host() -> Result<()> { + skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture"); + skip_if_host_windows!(Ok(())); + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + + let server = start_mock_server().await; + let test = managed_network_unified_exec_test(&server).await?; + let environments = vec![local(test.config.cwd.clone())]; + let first_responses = mount_exec_network_turn( + &server, + "resp-network-failed-amendment-1", + "network-failed-amendment-1", + network_fetch_args(LOCAL_ENVIRONMENT_ID), + ) + .await?; + submit_managed_network_turn( + &test, + "reject an invalid network policy amendment", + environments.clone(), + ApprovalsReviewer::User, + AskForApproval::OnRequest, + ) + .await?; + let approval = expect_network_approval(&test, LOCAL_ENVIRONMENT_ID).await?; + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: Some(approval.turn_id), + decision: ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: NetworkPolicyAmendment { + host: "not-the-approved-host.invalid".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, + }, + }) + .await?; + wait_for_turn_complete(&test).await; + + let denied_output = first_responses + .requests() + .iter() + .find_map(|request| request.function_call_output_text("network-failed-amendment-1")) + .context("expected the failed policy amendment to reject the network request")?; + assert!(denied_output.contains("blocked by policy")); + + mount_exec_network_turn( + &server, + "resp-network-failed-amendment-2", + "network-failed-amendment-2", + network_fetch_args(LOCAL_ENVIRONMENT_ID), + ) + .await?; + submit_managed_network_turn( + &test, + "a failed policy amendment must not approve the host for the session", + environments, + ApprovalsReviewer::User, + AskForApproval::OnRequest, + ) + .await?; + let approval = expect_network_approval(&test, LOCAL_ENVIRONMENT_ID).await?; + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: Some(approval.turn_id), + decision: ReviewDecision::denied("reject the retried network request"), + }) + .await?; + wait_for_turn_complete(&test).await; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[cfg_attr( not(target_os = "linux"),