diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 404fbc5a7a..ad49c611fc 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -1,4 +1,5 @@ use crate::exec::ExecToolCallOutput; +use crate::network_policy_decision::NetworkPolicyDecisionPayload; use crate::token_data::KnownPlan; use crate::token_data::PlanType; use crate::truncate::TruncationPolicy; @@ -31,7 +32,10 @@ pub enum SandboxErr { "sandbox denied exec error, exit code: {}, stdout: {}, stderr: {}", .output.exit_code, .output.stdout.text, .output.stderr.text )] - Denied { output: Box }, + Denied { + output: Box, + network_policy_decision: Option, + }, /// Error from linux seccomp filter setup #[cfg(target_os = "linux")] @@ -616,7 +620,7 @@ impl CodexErr { pub fn get_error_message_ui(e: &CodexErr) -> String { let message = match e { - CodexErr::Sandbox(SandboxErr::Denied { output }) => { + CodexErr::Sandbox(SandboxErr::Denied { output, .. }) => { let aggregated = output.aggregated_output.text.trim(); if !aggregated.is_empty() { output.aggregated_output.text.clone() @@ -736,6 +740,7 @@ mod tests { }; let err = CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, }); assert_eq!(get_error_message_ui(&err), "aggregate detail"); } @@ -752,6 +757,7 @@ mod tests { }; let err = CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, }); assert_eq!(get_error_message_ui(&err), "stderr detail\nstdout detail"); } @@ -768,6 +774,7 @@ mod tests { }; let err = CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, }); assert_eq!(get_error_message_ui(&err), "stdout only"); } @@ -811,6 +818,7 @@ mod tests { }; let err = CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, }); assert_eq!( get_error_message_ui(&err), diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index ebb7d8ca1b..01c4c23734 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -20,7 +20,7 @@ use crate::error::CodexErr; use crate::error::Result; use crate::error::SandboxErr; use crate::get_platform_sandbox; -use crate::network_policy_decision::extract_network_policy_decisions; +use crate::network_policy_decision::NetworkPolicyDecisionPayload; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::ExecCommandOutputDeltaEvent; @@ -252,7 +252,7 @@ pub(crate) async fn execute_exec_env( cwd, expiration, env, - network, + network: network.clone(), sandbox_permissions, windows_sandbox_level, justification, @@ -260,9 +260,39 @@ pub(crate) async fn execute_exec_env( }; let start = Instant::now(); + let blocked_cursor = match network.as_ref() { + Some(network) => network.blocked_requests_cursor().await.ok(), + None => None, + }; let raw_output_result = exec(params, sandbox, sandbox_policy, stdout_stream).await; let duration = start.elapsed(); - finalize_exec_result(raw_output_result, sandbox, duration) + let finalized = finalize_exec_result(raw_output_result, sandbox, duration); + let telemetry_decision = match (network.as_ref(), blocked_cursor) { + (Some(network), Some(cursor)) => { + blocking_network_policy_decision_from_blocked_queue(network, cursor).await + } + (None, _) => None, + (Some(_), None) => None, + }; + match finalized { + Ok(exec_output) => { + if let Some(policy_decision) = telemetry_decision { + return Err(CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(exec_output), + network_policy_decision: Some(policy_decision), + })); + } + Ok(exec_output) + } + Err(CodexErr::Sandbox(SandboxErr::Denied { + output, + network_policy_decision, + })) => Err(CodexErr::Sandbox(SandboxErr::Denied { + output, + network_policy_decision: network_policy_decision.or(telemetry_decision), + })), + Err(err) => Err(err), + } } #[cfg(target_os = "windows")] @@ -491,6 +521,7 @@ fn finalize_exec_result( if is_likely_sandbox_denied(sandbox_type, &exec_output) { return Err(CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(exec_output), + network_policy_decision: None, })); } @@ -535,10 +566,6 @@ pub(crate) fn is_likely_sandbox_denied( return false; } - if has_network_policy_blocking_decision(exec_output) { - return true; - } - if exec_output.exit_code == 0 { return false; } @@ -592,15 +619,63 @@ pub(crate) fn is_likely_sandbox_denied( false } -fn has_network_policy_blocking_decision(exec_output: &ExecToolCallOutput) -> bool { - [ - &exec_output.stderr.text, - &exec_output.stdout.text, - &exec_output.aggregated_output.text, - ] - .into_iter() - .flat_map(|section| extract_network_policy_decisions(section)) - .any(|payload| payload.is_blocking_decision()) +async fn blocking_network_policy_decision_from_blocked_queue( + network: &NetworkProxy, + blocked_cursor: u64, +) -> Option { + let blocked = network.blocked_requests_since(blocked_cursor).await.ok()?; + select_network_policy_decision_from_blocked_entries(blocked) +} + +fn select_network_policy_decision_from_blocked_entries( + blocked: Vec, +) -> Option { + let mut latest_blocking_decision = None; + + for entry in blocked.into_iter().rev() { + let Some(payload) = network_policy_decision_from_blocked_entry(&entry) else { + continue; + }; + + // If the command produced an ask-from-decider block, prefer that for retry + // prompting even if a later deny was also recorded. + if payload.is_ask_from_decider() { + return Some(payload); + } + if latest_blocking_decision.is_none() { + latest_blocking_decision = Some(payload); + } + } + + latest_blocking_decision +} + +fn network_policy_decision_from_blocked_entry( + entry: &codex_network_proxy::BlockedRequest, +) -> Option { + let decision = entry.decision.as_deref()?; + if decision.eq_ignore_ascii_case("allow") { + return None; + } + + let source = entry.source.as_deref()?; + let protocol = match entry.protocol.as_str() { + "http-connect" => Some("https_connect".to_string()), + "socks5" => Some("socks5_tcp".to_string()), + "socks5-udp" => Some("socks5_udp".to_string()), + "http" | "https" | "https_connect" | "socks5_tcp" | "socks5_udp" => { + Some(entry.protocol.clone()) + } + _ => None, + }?; + Some(NetworkPolicyDecisionPayload { + decision: decision.to_string(), + source: source.to_string(), + protocol: Some(protocol), + host: Some(entry.host.clone()), + reason: Some(entry.reason.clone()), + port: entry.port, + }) } #[derive(Debug, Clone)] @@ -921,6 +996,7 @@ fn synthetic_exit_status(code: i32) -> ExitStatus { #[cfg(test)] mod tests { use super::*; + use codex_network_proxy::BlockedRequest; use pretty_assertions::assert_eq; use std::time::Duration; use tokio::io::AsyncWriteExt; @@ -971,6 +1047,17 @@ mod tests { assert!(!is_likely_sandbox_denied(SandboxType::None, &output)); } + #[test] + fn sandbox_detection_ignores_network_policy_text_in_non_sandbox_mode() { + let output = make_exec_output( + 0, + "", + "", + r#"CODEX_NETWORK_POLICY_DECISION {"decision":"ask","reason":"not_allowed","source":"decider","protocol":"http","host":"google.com","port":80}"#, + ); + assert!(!is_likely_sandbox_denied(SandboxType::None, &output)); + } + #[test] fn sandbox_detection_uses_aggregated_output() { let output = make_exec_output( @@ -986,36 +1073,12 @@ mod tests { } #[test] - fn sandbox_detection_flags_network_policy_ask_with_zero_exit_code() { + fn sandbox_detection_ignores_network_policy_text_with_zero_exit_code() { let output = make_exec_output( 0, "", "", - r#"CODEX_NETWORK_POLICY_DECISION {"decision":"ask","reason":"not_allowed","source":"decider","protocol":"http","host":"google.com","port":80}"#, - ); - - assert!(is_likely_sandbox_denied(SandboxType::LinuxSeccomp, &output)); - } - - #[test] - fn sandbox_detection_flags_network_policy_non_decider_with_zero_exit_code() { - let output = make_exec_output( - 0, - "", - "", - r#"CODEX_NETWORK_POLICY_DECISION {"decision":"ask","reason":"not_allowed","source":"baseline_policy","protocol":"http","host":"google.com","port":80}"#, - ); - - assert!(is_likely_sandbox_denied(SandboxType::LinuxSeccomp, &output)); - } - - #[test] - fn sandbox_detection_ignores_network_policy_allow_with_zero_exit_code() { - let output = make_exec_output( - 0, - "", - "", - r#"CODEX_NETWORK_POLICY_DECISION {"decision":"allow","source":"decider","protocol":"http","host":"google.com","port":80}"#, + r#"CODEX_NETWORK_POLICY_DECISION {"decision":"ask","source":"decider","protocol":"http","host":"google.com","port":80}"#, ); assert!(!is_likely_sandbox_denied( @@ -1025,15 +1088,104 @@ mod tests { } #[test] - fn sandbox_detection_flags_network_policy_ask_from_json_blocked_response() { - let output = make_exec_output( - 0, - "", - "", - r#"{"status":"blocked","host":"google.com","reason":"not_allowed","policy_decision_prefix":"CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"reason\":\"not_allowed\",\"source\":\"decider\",\"protocol\":\"http\",\"host\":\"google.com\",\"port\":80}","message":"CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"reason\":\"not_allowed\",\"source\":\"decider\",\"protocol\":\"http\",\"host\":\"google.com\",\"port\":80}\nCodex blocked this request: domain not in allowlist (this is not a denylist block)."}"#, - ); + fn network_policy_decision_maps_fresh_entries() { + let entry = BlockedRequest { + host: "google.com".to_string(), + reason: "not_allowed".to_string(), + client: None, + method: Some("GET".to_string()), + mode: None, + protocol: "http-connect".to_string(), + decision: Some("ask".to_string()), + source: Some("decider".to_string()), + port: Some(443), + timestamp: 200, + }; - assert!(is_likely_sandbox_denied(SandboxType::LinuxSeccomp, &output)); + let payload = network_policy_decision_from_blocked_entry(&entry) + .expect("blocked entry should map to structured decision"); + assert_eq!( + payload, + NetworkPolicyDecisionPayload { + decision: "ask".to_string(), + source: "decider".to_string(), + protocol: Some("https_connect".to_string()), + host: Some("google.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + } + ); + } + + #[test] + fn network_policy_decision_ignores_allow_entries() { + let entry = BlockedRequest { + host: "google.com".to_string(), + reason: "not_allowed".to_string(), + client: None, + method: Some("GET".to_string()), + mode: None, + protocol: "http".to_string(), + decision: Some("allow".to_string()), + source: Some("decider".to_string()), + port: Some(80), + timestamp: 200, + }; + + assert_eq!(network_policy_decision_from_blocked_entry(&entry), None); + } + + #[test] + fn network_policy_decision_ignores_entries_without_source() { + let entry = BlockedRequest { + host: "google.com".to_string(), + reason: "not_allowed".to_string(), + client: None, + method: Some("GET".to_string()), + mode: None, + protocol: "http".to_string(), + decision: Some("ask".to_string()), + source: None, + port: Some(80), + timestamp: 100, + }; + + assert_eq!(network_policy_decision_from_blocked_entry(&entry), None); + } + + #[test] + fn select_network_policy_decision_prefers_ask_from_decider_over_newer_deny() { + let blocked = vec![ + BlockedRequest { + host: "google.com".to_string(), + reason: "not_allowed".to_string(), + client: None, + method: Some("GET".to_string()), + mode: None, + protocol: "http".to_string(), + decision: Some("ask".to_string()), + source: Some("decider".to_string()), + port: Some(80), + timestamp: 200, + }, + BlockedRequest { + host: "google.com".to_string(), + reason: "not_allowed".to_string(), + client: None, + method: Some("GET".to_string()), + mode: None, + protocol: "http".to_string(), + decision: Some("deny".to_string()), + source: Some("baseline_policy".to_string()), + port: Some(80), + timestamp: 201, + }, + ]; + + let decision = select_network_policy_decision_from_blocked_entries(blocked) + .expect("an ask decision should be selected"); + assert_eq!(decision.decision, "ask"); + assert_eq!(decision.source, "decider"); } #[tokio::test] diff --git a/codex-rs/core/src/network_policy_decision.rs b/codex-rs/core/src/network_policy_decision.rs index 2f60dbd3bc..a918f18175 100644 --- a/codex-rs/core/src/network_policy_decision.rs +++ b/codex-rs/core/src/network_policy_decision.rs @@ -1,11 +1,10 @@ +use codex_protocol::approvals::NetworkApprovalContext; +use codex_protocol::approvals::NetworkApprovalProtocol; use serde::Deserialize; -use serde_json::Value; - -pub(crate) const NETWORK_POLICY_DECISION_PREFIX: &str = "CODEX_NETWORK_POLICY_DECISION "; #[derive(Debug, Clone, Deserialize, PartialEq, Eq)] #[serde(rename_all = "camelCase")] -pub(crate) struct NetworkPolicyDecisionPayload { +pub struct NetworkPolicyDecisionPayload { pub decision: String, pub source: String, pub protocol: Option, @@ -18,57 +17,30 @@ impl NetworkPolicyDecisionPayload { pub(crate) fn is_ask_from_decider(&self) -> bool { self.decision.eq_ignore_ascii_case("ask") && self.source.eq_ignore_ascii_case("decider") } - - pub(crate) fn is_blocking_decision(&self) -> bool { - !self.decision.eq_ignore_ascii_case("allow") - } } -pub(crate) fn extract_network_policy_decisions(text: &str) -> Vec { - text.lines() - .flat_map(extract_policy_decisions_from_fragment) - .collect() -} - -fn extract_policy_decisions_from_fragment(fragment: &str) -> Vec { - let mut payloads = Vec::new(); - - if let Some(payload) = parse_prefixed_payload(fragment) { - payloads.push(payload); +pub(crate) fn network_approval_context_from_payload( + payload: &NetworkPolicyDecisionPayload, +) -> Option { + if !payload.is_ask_from_decider() { + return None; } - if let Ok(value) = serde_json::from_str::(fragment) { - extract_policy_decisions_from_json_value(&value, &mut payloads); + let protocol = match payload.protocol.as_deref() { + Some("http") => NetworkApprovalProtocol::Http, + Some("https") | Some("https_connect") => NetworkApprovalProtocol::Https, + _ => return None, + }; + + let host = payload.host.as_deref()?.trim(); + if host.is_empty() { + return None; } - payloads -} - -fn extract_policy_decisions_from_json_value( - value: &Value, - payloads: &mut Vec, -) { - match value { - Value::String(text) => { - payloads.extend(text.lines().filter_map(parse_prefixed_payload)); - } - Value::Array(values) => { - for value in values { - extract_policy_decisions_from_json_value(value, payloads); - } - } - Value::Object(map) => { - for value in map.values() { - extract_policy_decisions_from_json_value(value, payloads); - } - } - Value::Null | Value::Bool(_) | Value::Number(_) => {} - } -} - -fn parse_prefixed_payload(text: &str) -> Option { - let payload = text.strip_prefix(NETWORK_POLICY_DECISION_PREFIX)?; - serde_json::from_str::(payload).ok() + Some(NetworkApprovalContext { + host: host.to_string(), + protocol, + }) } #[cfg(test)] @@ -77,47 +49,51 @@ mod tests { use pretty_assertions::assert_eq; #[test] - fn extracts_payload_from_prefixed_line() { - let text = r#"CODEX_NETWORK_POLICY_DECISION {"decision":"ask","source":"decider","protocol":"http","host":"example.com","port":80}"#; + fn network_approval_context_requires_ask_from_decider() { + let payload = NetworkPolicyDecisionPayload { + decision: "deny".to_string(), + source: "decider".to_string(), + protocol: Some("https_connect".to_string()), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + }; - let payloads = extract_network_policy_decisions(text); + assert_eq!(network_approval_context_from_payload(&payload), None); + } + + #[test] + fn network_approval_context_maps_http_and_https_protocols() { + let http_payload = NetworkPolicyDecisionPayload { + decision: "ask".to_string(), + source: "decider".to_string(), + protocol: Some("http".to_string()), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(80), + }; assert_eq!( - payloads, - vec![NetworkPolicyDecisionPayload { - decision: "ask".to_string(), - source: "decider".to_string(), - protocol: Some("http".to_string()), - host: Some("example.com".to_string()), - reason: None, - port: Some(80), - }] + network_approval_context_from_payload(&http_payload), + Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Http, + }) + ); + + let https_payload = NetworkPolicyDecisionPayload { + decision: "ask".to_string(), + source: "decider".to_string(), + protocol: Some("https_connect".to_string()), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + }; + assert_eq!( + network_approval_context_from_payload(&https_payload), + Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + }) ); } - - #[test] - fn extracts_payload_from_generic_json_string_field() { - let text = r#"{"unexpected":"CODEX_NETWORK_POLICY_DECISION {\"decision\":\"deny\",\"source\":\"baseline_policy\",\"protocol\":\"https_connect\",\"host\":\"google.com\",\"port\":443}"}"#; - - let payloads = extract_network_policy_decisions(text); - assert_eq!(payloads.len(), 1); - assert_eq!(payloads[0].decision, "deny"); - assert_eq!(payloads[0].source, "baseline_policy"); - assert_eq!(payloads[0].host.as_deref(), Some("google.com")); - } - - #[test] - fn extracts_payload_from_nested_json_values() { - let text = r#"{"data":[{"meta":{"message":"CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"decider\",\"protocol\":\"https_connect\",\"host\":\"api.example.com\",\"port\":443}\nblocked"}}]}"#; - - let payloads = extract_network_policy_decisions(text); - assert_eq!(payloads.len(), 1); - assert_eq!(payloads[0].decision, "ask"); - assert_eq!(payloads[0].host.as_deref(), Some("api.example.com")); - } - - #[test] - fn ignores_lines_without_policy_prefix() { - let text = r#"{"status":"blocked","message":"domain not in allowlist"}"#; - assert!(extract_network_policy_decisions(text).is_empty()); - } } diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 65972f5a5a..2bacd188d5 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -314,7 +314,7 @@ impl ToolEmitter { (event, result) } Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { output }))) - | Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output }))) => { + | Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, .. }))) => { let response = self.format_exec_output_for_model(&output, ctx); let event = ToolEventStage::Failure(ToolEventFailure::Output(*output)); let result = Err(FunctionCallError::RespondToModel(response)); diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 74beca5700..e566a3e664 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -8,9 +8,9 @@ caching). */ use crate::error::CodexErr; use crate::error::SandboxErr; -use crate::exec::ExecToolCallOutput; use crate::features::Feature; -use crate::network_policy_decision::extract_network_policy_decisions; +use crate::network_policy_decision::NetworkPolicyDecisionPayload; +use crate::network_policy_decision::network_approval_context_from_payload; use crate::sandboxing::SandboxManager; use crate::tools::sandboxing::ApprovalCtx; use crate::tools::sandboxing::ExecApprovalRequirement; @@ -22,7 +22,6 @@ use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::default_exec_approval_requirement; use codex_otel::ToolDecisionSource; use codex_protocol::approvals::NetworkApprovalContext; -use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; @@ -127,14 +126,18 @@ impl ToolOrchestrator { // We have a successful initial result Ok(out) } - Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output }))) => { + Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { + output, + network_policy_decision, + }))) => { if !tool.escalate_on_failure() { return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, + network_policy_decision, }))); } - let retry_details = build_denial_reason_from_output( - output.as_ref(), + let retry_details = build_denial_reason( + network_policy_decision.as_ref(), should_prompt_for_network_approval(turn_ctx), ); @@ -149,6 +152,7 @@ impl ToolOrchestrator { ) { return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, + network_policy_decision, }))); } @@ -233,12 +237,12 @@ struct RetryApprovalDetails { network_approval_context: Option, } -fn build_denial_reason_from_output( - output: &ExecToolCallOutput, +fn build_denial_reason( + network_policy_decision: Option<&NetworkPolicyDecisionPayload>, network_prompting_enabled: bool, ) -> RetryApprovalDetails { let network_approval_context = if network_prompting_enabled { - extract_network_approval_context(output) + network_policy_decision.and_then(network_approval_context_from_payload) } else { None }; @@ -248,8 +252,7 @@ fn build_denial_reason_from_output( network_approval_context.host ) } else { - // Keep approval reason terse and stable for UX/tests, but accept the - // output so we can evolve heuristics later without touching call sites. + // Keep approval reason terse and stable for UX/tests. "command failed; retry without sandbox?".to_string() }; RetryApprovalDetails { @@ -259,16 +262,10 @@ fn build_denial_reason_from_output( } fn should_prompt_for_network_approval(turn_ctx: &crate::codex::TurnContext) -> bool { - matches!( - turn_ctx - .config - .config_layer_stack - .requirements_toml() - .network - .as_ref() - .and_then(|network| network.enabled), - Some(true) - ) + // Network retry prompting should follow the active managed proxy at runtime, not only + // requirements.toml. This keeps behavior consistent for default sandbox sessions that still + // route through a managed proxy. + turn_ctx.network.is_some() } fn can_retry_without_sandbox( @@ -300,62 +297,25 @@ fn should_bypass_retry_approval( tool_wants_to_bypass_approval && !has_network_approval_context } -fn extract_network_approval_context(output: &ExecToolCallOutput) -> Option { - [ - output.stderr.text.as_str(), - output.stdout.text.as_str(), - output.aggregated_output.text.as_str(), - ] - .into_iter() - .find_map(extract_network_approval_context_from_text) -} - -fn extract_network_approval_context_from_text(text: &str) -> Option { - extract_network_policy_decisions(text) - .into_iter() - .find_map(|payload| { - if !payload.is_ask_from_decider() { - return None; - } - - let protocol = match payload.protocol.as_deref() { - Some("http") => NetworkApprovalProtocol::Http, - Some("https") | Some("https_connect") => NetworkApprovalProtocol::Https, - _ => return None, - }; - - let host = payload.host.as_deref()?.trim(); - if host.is_empty() { - return None; - } - - Some(NetworkApprovalContext { - host: host.to_string(), - protocol, - }) - }) -} - #[cfg(test)] mod tests { use super::*; - use crate::exec::StreamOutput; + use crate::network_policy_decision::NetworkPolicyDecisionPayload; + use codex_protocol::approvals::NetworkApprovalProtocol; use pretty_assertions::assert_eq; - fn output_with_stderr(stderr: &str) -> ExecToolCallOutput { - ExecToolCallOutput { - stderr: StreamOutput::new(stderr.to_string()), - ..ExecToolCallOutput::default() - } - } - #[test] fn build_denial_reason_extracts_network_context_when_enabled() { - let output = output_with_stderr( - "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"decider\",\"protocol\":\"https_connect\",\"host\":\"example.com\",\"port\":443}\nblocked", - ); + let decision = NetworkPolicyDecisionPayload { + decision: "ask".to_string(), + source: "decider".to_string(), + protocol: Some("https_connect".to_string()), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + }; - let details = build_denial_reason_from_output(&output, true); + let details = build_denial_reason(Some(&decision), true); assert_eq!( details.network_approval_context, @@ -372,39 +332,67 @@ mod tests { #[test] fn build_denial_reason_skips_network_context_when_disabled() { - let output = output_with_stderr( - "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"decider\",\"protocol\":\"https_connect\",\"host\":\"example.com\",\"port\":443}\nblocked", - ); + let decision = NetworkPolicyDecisionPayload { + decision: "ask".to_string(), + source: "decider".to_string(), + protocol: Some("https_connect".to_string()), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + }; - let details = build_denial_reason_from_output(&output, false); + let details = build_denial_reason(Some(&decision), false); assert_eq!(details.network_approval_context, None); assert_eq!(details.reason, "command failed; retry without sandbox?"); } #[test] - fn extract_network_approval_context_ignores_non_ask_payloads() { - let text = "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"deny\",\"source\":\"decider\",\"protocol\":\"http\",\"host\":\"example.com\",\"port\":80}"; + fn build_denial_reason_ignores_non_ask_payloads() { + let decision = NetworkPolicyDecisionPayload { + decision: "deny".to_string(), + source: "decider".to_string(), + protocol: Some("http".to_string()), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(80), + }; - assert_eq!(extract_network_approval_context_from_text(text), None); + let details = build_denial_reason(Some(&decision), true); + assert_eq!(details.network_approval_context, None); } #[test] - fn extract_network_approval_context_ignores_non_decider_payloads() { - let text = "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"baseline_policy\",\"protocol\":\"http\",\"host\":\"example.com\",\"port\":80}"; + fn build_denial_reason_ignores_non_decider_payloads() { + let decision = NetworkPolicyDecisionPayload { + decision: "ask".to_string(), + source: "baseline_policy".to_string(), + protocol: Some("http".to_string()), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(80), + }; - assert_eq!(extract_network_approval_context_from_text(text), None); + let details = build_denial_reason(Some(&decision), true); + assert_eq!(details.network_approval_context, None); } #[test] - fn extract_network_approval_context_from_json_blocked_response() { - let text = r#"{"status":"blocked","host":"example.com","reason":"not_allowed","policy_decision_prefix":"CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"reason\":\"not_allowed\",\"source\":\"decider\",\"protocol\":\"https_connect\",\"host\":\"example.com\",\"port\":443}","message":"CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"reason\":\"not_allowed\",\"source\":\"decider\",\"protocol\":\"https_connect\",\"host\":\"example.com\",\"port\":443}\nCodex blocked this request: domain not in allowlist (this is not a denylist block)."}"#; + fn build_denial_reason_extracts_http_protocol() { + let decision = NetworkPolicyDecisionPayload { + decision: "ask".to_string(), + source: "decider".to_string(), + protocol: Some("http".to_string()), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(80), + }; assert_eq!( - extract_network_approval_context_from_text(text), + build_denial_reason(Some(&decision), true).network_approval_context, Some(NetworkApprovalContext { host: "example.com".to_string(), - protocol: NetworkApprovalProtocol::Https, + protocol: NetworkApprovalProtocol::Http, }) ); } diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 71593c6685..fbd72db7d3 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -187,6 +187,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt UnifiedExecError::SandboxDenied { output, .. } => { ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, })) } other => ToolError::Rejected(other.to_string()),