From 20df0dfa4c8d777377dabd336bbf83b2d6c86d29 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Tue, 23 Jun 2026 02:23:17 -0400 Subject: [PATCH] Surface triggerless network denials to Codex --- .../context/guardian_network_access_denied.rs | 48 +++++++++++++++++++ codex-rs/core/src/context/mod.rs | 2 + codex-rs/core/src/guardian/review.rs | 42 +++++++++++++++- codex-rs/core/tests/suite/network_approval.rs | 19 +++++++- 4 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 codex-rs/core/src/context/guardian_network_access_denied.rs diff --git a/codex-rs/core/src/context/guardian_network_access_denied.rs b/codex-rs/core/src/context/guardian_network_access_denied.rs new file mode 100644 index 0000000000..b70483f221 --- /dev/null +++ b/codex-rs/core/src/context/guardian_network_access_denied.rs @@ -0,0 +1,48 @@ +use super::ContextualUserFragment; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::truncate_text; + +const MAX_NETWORK_TARGET_TOKENS: usize = 128; +const MAX_GUARDIAN_REJECTION_TOKENS: usize = 512; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct GuardianNetworkAccessDenied { + target: String, + rejection: String, +} + +impl GuardianNetworkAccessDenied { + pub(crate) fn new(target: &str, rejection: &str) -> Self { + Self { + target: truncate_text(target, TruncationPolicy::Tokens(MAX_NETWORK_TARGET_TOKENS)), + rejection: truncate_text( + rejection, + TruncationPolicy::Tokens(MAX_GUARDIAN_REJECTION_TOKENS), + ), + } + } +} + +impl ContextualUserFragment for GuardianNetworkAccessDenied { + fn role(&self) -> &'static str { + "developer" + } + + fn markers(&self) -> (&'static str, &'static str) { + Self::type_markers() + } + + fn type_markers() -> (&'static str, &'static str) { + ( + "", + "", + ) + } + + fn body(&self) -> String { + format!( + "\nAutomatic approval review denied network access to {:?}.\n{}\n", + self.target, self.rejection + ) + } +} diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index 4ba6acf029..a40b436d1e 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -9,6 +9,7 @@ mod contextual_user_message; mod current_time_reminder; mod environment_context; mod guardian_followup_review_reminder; +mod guardian_network_access_denied; mod hook_additional_context; mod image_generation_instructions; mod inter_agent_completion_message; @@ -49,6 +50,7 @@ pub(crate) use contextual_user_message::is_contextual_user_fragment; pub(crate) use contextual_user_message::parse_visible_hook_prompt_message; pub(crate) use current_time_reminder::CurrentTimeReminder; pub(crate) use guardian_followup_review_reminder::GuardianFollowupReviewReminder; +pub(crate) use guardian_network_access_denied::GuardianNetworkAccessDenied; pub(crate) use hook_additional_context::HookAdditionalContext; pub(crate) use image_generation_instructions::ImageGenerationInstructions; pub use image_generation_instructions::extension_image_generation_output_hint; diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 4756cc1fc4..4580e03da3 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -24,6 +24,8 @@ use tokio::time::Instant; use tokio::time::sleep_until; use tokio_util::sync::CancellationToken; +use crate::context::ContextualUserFragment; +use crate::context::GuardianNetworkAccessDenied; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use crate::turn_timing::now_unix_timestamp_ms; @@ -80,10 +82,17 @@ pub(crate) async fn guardian_rejection_message(session: &Session, review_id: &st rationale: "Auto-reviewer denied the action without a specific rationale.".to_string(), source: GuardianAssessmentDecisionSource::Agent, }); - match rejection.source { + format_guardian_rejection_message(&rejection.rationale, rejection.source) +} + +fn format_guardian_rejection_message( + rationale: &str, + source: GuardianAssessmentDecisionSource, +) -> String { + match source { GuardianAssessmentDecisionSource::Agent => format!( "This action was rejected due to unacceptable risk.\nReason: {}\n{}", - rejection.rationale.trim(), + rationale.trim(), GUARDIAN_REJECTION_INSTRUCTIONS ), } @@ -281,6 +290,23 @@ async fn run_guardian_review( approval_request_source: GuardianApprovalRequestSource, external_cancel: Option, ) -> ReviewDecision { + let unattributed_network_target = match &request { + GuardianApprovalRequest::NetworkAccess { + target, + trigger: None, + .. + } => Some(target.clone()), + GuardianApprovalRequest::Shell { .. } + | GuardianApprovalRequest::ExecCommand { .. } + | GuardianApprovalRequest::ApplyPatch { .. } + | GuardianApprovalRequest::NetworkAccess { + trigger: Some(_), .. + } + | GuardianApprovalRequest::McpToolCall { .. } + | GuardianApprovalRequest::RequestPermissions { .. } => None, + #[cfg(unix)] + GuardianApprovalRequest::Execve { .. } => None, + }; let target_item_id = guardian_request_target_item_id(&request).map(str::to_string); let assessment_turn_id = guardian_request_turn_id(&request, &turn.sub_id).to_string(); let action_summary = guardian_assessment_action(&request); @@ -577,6 +603,18 @@ async fn run_guardian_review( ) .await; + if !approved && let Some(target) = unattributed_network_target { + let rejection = format_guardian_rejection_message( + &assessment.rationale, + GuardianAssessmentDecisionSource::Agent, + ); + let denial: codex_protocol::models::ResponseItem = + ContextualUserFragment::into(GuardianNetworkAccessDenied::new(&target, &rejection)); + session + .record_conversation_items(turn.as_ref(), std::slice::from_ref(&denial)) + .await; + } + if count_denial_for_circuit_breaker { record_guardian_denial(&session, &turn, &assessment_turn_id).await; } else { diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index c6f54c69d8..d5446f74e2 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -87,6 +87,7 @@ async fn guardian_receives_all_possible_triggers_for_concurrent_network_requests let second_command = network_command(&second_marker, &first_marker); let first_args = network_exec_args(LOCAL_ENVIRONMENT_ID, &first_command); let second_args = network_exec_args(LOCAL_ENVIRONMENT_ID, &second_command); + let denial_rationale = "The destination is not authorized by the user."; let responses = mount_sse_sequence( &server, vec![ @@ -106,7 +107,16 @@ async fn guardian_receives_all_possible_triggers_for_concurrent_network_requests ]), sse(vec![ ev_response_created("resp-network-guardian"), - ev_assistant_message("msg-network-guardian", r#"{"outcome":"allow"}"#), + ev_assistant_message( + "msg-network-guardian", + &json!({ + "risk_level": "high", + "user_authorization": "low", + "outcome": "deny", + "rationale": denial_rationale, + }) + .to_string(), + ), ev_completed("resp-network-guardian"), ]), sse(vec![ @@ -173,6 +183,13 @@ async fn guardian_receives_all_possible_triggers_for_concurrent_network_requests .concat() .contains("Approve if at least one candidate") ); + assert!(responses.requests().into_iter().any(|request| { + request.message_input_texts("developer").iter().any(|text| { + text.contains("") + && text.contains(NETWORK_TEST_TARGET) + && text.contains(denial_rationale) + }) + })); Ok(()) }