diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index e6709047b8..5bc9dfcb33 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -182,7 +182,7 @@ impl NetworkProxySpec { fn should_ask_on_allowlist_miss(sandbox_policy: &SandboxPolicy) -> bool { matches!( sandbox_policy, - SandboxPolicy::ReadOnly | SandboxPolicy::WorkspaceWrite { .. } + SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } ) } @@ -190,13 +190,17 @@ fn should_ask_on_allowlist_miss(sandbox_policy: &SandboxPolicy) -> bool { mod tests { use super::*; use codex_protocol::protocol::NetworkAccess; + use codex_protocol::protocol::ReadOnlyAccess; #[test] fn restricted_sandbox_modes_ask_on_allowlist_miss() { - assert!(should_ask_on_allowlist_miss(&SandboxPolicy::ReadOnly)); + assert!(should_ask_on_allowlist_miss(&SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + })); assert!(should_ask_on_allowlist_miss( &SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: ReadOnlyAccess::FullAccess, network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 01c4c23734..ea6bc4d31a 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -264,12 +264,24 @@ pub(crate) async fn execute_exec_env( Some(network) => network.blocked_requests_cursor().await.ok(), None => None, }; + match (network.as_ref(), blocked_cursor) { + (Some(_), Some(cursor)) => { + tracing::debug!("captured blocked telemetry cursor before exec: {cursor}"); + } + (Some(_), None) => { + tracing::debug!( + "managed network is present but failed to capture blocked telemetry cursor" + ); + } + (None, _) => {} + } let raw_output_result = exec(params, sandbox, sandbox_policy, stdout_stream).await; let duration = start.elapsed(); 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 + blocking_network_policy_decision_from_blocked_queue(network, cursor, sandbox_policy) + .await } (None, _) => None, (Some(_), None) => None, @@ -277,6 +289,14 @@ pub(crate) async fn execute_exec_env( match finalized { Ok(exec_output) => { if let Some(policy_decision) = telemetry_decision { + tracing::debug!( + "promoting successful exec result to sandbox denied based on structured network telemetry (decision={}, source={}, host={:?}, protocol={:?}, port={:?})", + policy_decision.decision, + policy_decision.source, + policy_decision.host, + policy_decision.protocol, + policy_decision.port + ); return Err(CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(exec_output), network_policy_decision: Some(policy_decision), @@ -287,10 +307,27 @@ pub(crate) async fn execute_exec_env( Err(CodexErr::Sandbox(SandboxErr::Denied { output, network_policy_decision, - })) => Err(CodexErr::Sandbox(SandboxErr::Denied { - output, - network_policy_decision: network_policy_decision.or(telemetry_decision), - })), + })) => { + let merged_decision = network_policy_decision.or(telemetry_decision); + if let Some(payload) = merged_decision.as_ref() { + tracing::debug!( + "sandbox-denied exec result includes structured network decision (decision={}, source={}, host={:?}, protocol={:?}, port={:?})", + payload.decision, + payload.source, + payload.host, + payload.protocol, + payload.port + ); + } else { + tracing::debug!( + "sandbox-denied exec result had no structured network decision payload" + ); + } + Err(CodexErr::Sandbox(SandboxErr::Denied { + output, + network_policy_decision: merged_decision, + })) + } Err(err) => Err(err), } } @@ -622,27 +659,65 @@ pub(crate) fn is_likely_sandbox_denied( async fn blocking_network_policy_decision_from_blocked_queue( network: &NetworkProxy, blocked_cursor: u64, + sandbox_policy: &SandboxPolicy, ) -> Option { - let blocked = network.blocked_requests_since(blocked_cursor).await.ok()?; - select_network_policy_decision_from_blocked_entries(blocked) + let blocked = match network.blocked_requests_since(blocked_cursor).await { + Ok(blocked) => blocked, + Err(err) => { + tracing::debug!( + "failed to load blocked telemetry since cursor {blocked_cursor}: {err:#}" + ); + return None; + } + }; + tracing::debug!( + "loaded {} blocked telemetry entries since cursor {blocked_cursor}", + blocked.len() + ); + let selected = select_network_policy_decision_from_blocked_entries(blocked, sandbox_policy); + if let Some(payload) = selected.as_ref() { + tracing::debug!( + "selected telemetry network decision (decision={}, source={}, host={:?}, protocol={:?}, port={:?})", + payload.decision, + payload.source, + payload.host, + payload.protocol, + payload.port + ); + } else { + tracing::debug!("no telemetry network decision selected since cursor {blocked_cursor}"); + } + selected } fn select_network_policy_decision_from_blocked_entries( blocked: Vec, + sandbox_policy: &SandboxPolicy, ) -> 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 { + let Some(payload) = network_policy_decision_from_blocked_entry(&entry, sandbox_policy) + 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() { + tracing::debug!( + "selected ask-from-decider telemetry decision for host {:?}", + payload.host + ); return Some(payload); } if latest_blocking_decision.is_none() { + tracing::debug!( + "recorded fallback telemetry decision candidate (decision={}, source={}, host={:?})", + payload.decision, + payload.source, + payload.host + ); latest_blocking_decision = Some(payload); } } @@ -652,13 +727,13 @@ fn select_network_policy_decision_from_blocked_entries( fn network_policy_decision_from_blocked_entry( entry: &codex_network_proxy::BlockedRequest, + sandbox_policy: &SandboxPolicy, ) -> Option { - let decision = entry.decision.as_deref()?; - if decision.eq_ignore_ascii_case("allow") { - return None; - } - - let source = entry.source.as_deref()?; + let (decision, source) = derive_network_policy_decision( + entry.reason.as_str(), + entry.source.as_deref(), + sandbox_policy, + )?; let protocol = match entry.protocol.as_str() { "http-connect" => Some("https_connect".to_string()), "socks5" => Some("socks5_tcp".to_string()), @@ -669,8 +744,8 @@ fn network_policy_decision_from_blocked_entry( _ => None, }?; Some(NetworkPolicyDecisionPayload { - decision: decision.to_string(), - source: source.to_string(), + decision, + source, protocol: Some(protocol), host: Some(entry.host.clone()), reason: Some(entry.reason.clone()), @@ -678,6 +753,37 @@ fn network_policy_decision_from_blocked_entry( }) } +fn derive_network_policy_decision( + reason: &str, + source: Option<&str>, + sandbox_policy: &SandboxPolicy, +) -> Option<(String, String)> { + const REASON_NOT_ALLOWED: &str = "not_allowed"; + const REASON_NOT_ALLOWED_LOCAL: &str = "not_allowed_local"; + const REASON_DENIED: &str = "denied"; + const REASON_METHOD_NOT_ALLOWED: &str = "method_not_allowed"; + + let source_or = |fallback: &str| source.unwrap_or(fallback).to_string(); + + match reason { + REASON_NOT_ALLOWED if should_ask_on_allowlist_miss(sandbox_policy) => { + Some(("ask".to_string(), "decider".to_string())) + } + REASON_NOT_ALLOWED + | REASON_NOT_ALLOWED_LOCAL + | REASON_DENIED + | REASON_METHOD_NOT_ALLOWED => Some(("deny".to_string(), source_or("baseline_policy"))), + _ => Some(("deny".to_string(), source_or("proxy_state"))), + } +} + +fn should_ask_on_allowlist_miss(sandbox_policy: &SandboxPolicy) -> bool { + matches!( + sandbox_policy, + SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } + ) +} + #[derive(Debug, Clone)] pub struct StreamOutput { pub text: T, @@ -997,10 +1103,21 @@ fn synthetic_exit_status(code: i32) -> ExitStatus { mod tests { use super::*; use codex_network_proxy::BlockedRequest; + use codex_protocol::protocol::ReadOnlyAccess; use pretty_assertions::assert_eq; use std::time::Duration; use tokio::io::AsyncWriteExt; + fn restricted_sandbox_policy() -> SandboxPolicy { + SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: ReadOnlyAccess::FullAccess, + network_access: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + } + } + fn make_exec_output( exit_code: i32, stdout: &str, @@ -1102,8 +1219,9 @@ mod tests { timestamp: 200, }; - let payload = network_policy_decision_from_blocked_entry(&entry) - .expect("blocked entry should map to structured decision"); + let payload = + network_policy_decision_from_blocked_entry(&entry, &restricted_sandbox_policy()) + .expect("blocked entry should map to structured decision"); assert_eq!( payload, NetworkPolicyDecisionPayload { @@ -1132,11 +1250,15 @@ mod tests { timestamp: 200, }; - assert_eq!(network_policy_decision_from_blocked_entry(&entry), None); + let payload = + network_policy_decision_from_blocked_entry(&entry, &restricted_sandbox_policy()) + .expect("restricted sandbox should derive ask decision on allowlist miss"); + assert_eq!(payload.decision, "ask"); + assert_eq!(payload.source, "decider"); } #[test] - fn network_policy_decision_ignores_entries_without_source() { + fn network_policy_decision_derives_source_when_missing() { let entry = BlockedRequest { host: "google.com".to_string(), reason: "not_allowed".to_string(), @@ -1150,7 +1272,11 @@ mod tests { timestamp: 100, }; - assert_eq!(network_policy_decision_from_blocked_entry(&entry), None); + let payload = + network_policy_decision_from_blocked_entry(&entry, &restricted_sandbox_policy()) + .expect("missing source should still map to structured decision"); + assert_eq!(payload.decision, "ask"); + assert_eq!(payload.source, "decider"); } #[test] @@ -1182,12 +1308,46 @@ mod tests { }, ]; - let decision = select_network_policy_decision_from_blocked_entries(blocked) - .expect("an ask decision should be selected"); + let decision = select_network_policy_decision_from_blocked_entries( + blocked, + &restricted_sandbox_policy(), + ) + .expect("an ask decision should be selected"); assert_eq!(decision.decision, "ask"); assert_eq!(decision.source, "decider"); } + #[test] + fn yolo_sandbox_denies_allowlist_miss() { + 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: Some("decider".to_string()), + port: Some(80), + timestamp: 100, + }; + + let payload = + network_policy_decision_from_blocked_entry(&entry, &SandboxPolicy::DangerFullAccess) + .expect("allowlist miss should still map to structured decision in yolo"); + assert_eq!( + payload, + NetworkPolicyDecisionPayload { + decision: "deny".to_string(), + source: "decider".to_string(), + protocol: Some("http".to_string()), + host: Some("google.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(80), + } + ); + } + #[tokio::test] async fn read_capped_limits_retained_bytes() { let (mut writer, reader) = tokio::io::duplex(1024); diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index e566a3e664..a4e9866dbb 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -130,6 +130,18 @@ impl ToolOrchestrator { output, network_policy_decision, }))) => { + if let Some(payload) = network_policy_decision.as_ref() { + tracing::debug!( + "sandbox denied with structured network decision (decision={}, source={}, host={:?}, protocol={:?}, port={:?})", + payload.decision, + payload.source, + payload.host, + payload.protocol, + payload.port + ); + } else { + tracing::debug!("sandbox denied without structured network decision payload"); + } if !tool.escalate_on_failure() { return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, @@ -140,6 +152,11 @@ impl ToolOrchestrator { network_policy_decision.as_ref(), should_prompt_for_network_approval(turn_ctx), ); + tracing::debug!( + "retry denial reason prepared (reason={}, has_network_context={})", + retry_details.reason, + retry_details.network_approval_context.is_some() + ); // Most tools disallow no-sandbox retry under OnRequest. However, for managed // network denials with extracted network approval context, we still allow @@ -150,6 +167,11 @@ impl ToolOrchestrator { &turn_ctx.sandbox_policy, retry_details.network_approval_context.is_some(), ) { + tracing::debug!( + "retry without sandbox blocked by approval gate (approval_policy={:?}, has_network_context={})", + approval_policy, + retry_details.network_approval_context.is_some() + ); return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, network_policy_decision, @@ -169,9 +191,15 @@ impl ToolOrchestrator { retry_reason: Some(retry_details.reason), network_approval_context: retry_details.network_approval_context.clone(), }; + tracing::debug!( + "requesting retry approval (reason={}, has_network_context={})", + approval_ctx.retry_reason.as_deref().unwrap_or(""), + approval_ctx.network_approval_context.is_some() + ); let decision = tool.start_approval_async(req, approval_ctx).await; otel.tool_decision(otel_tn, otel_ci, &decision, otel_user); + tracing::debug!("retry approval decision: {decision:?}"); match decision { ReviewDecision::Denied | ReviewDecision::Abort => { @@ -195,6 +223,11 @@ impl ToolOrchestrator { host = %network_approval_context.host, "failed to grant temporary network host allowance; retry may remain blocked" ); + } else { + tracing::debug!( + "granted temporary network host allowance for retry: {}", + network_approval_context.host + ); } granted_host.map(|host| (network.clone(), host)) } else { @@ -223,6 +256,7 @@ impl ToolOrchestrator { let second_attempt = (*tool).run(req, &escalated_attempt, tool_ctx).await; if let Some((network, host)) = temporary_allowed_host { network.revoke_temporary_allowed_host(&host).await; + tracing::debug!("revoked temporary network host allowance after retry: {host}"); } second_attempt } @@ -402,7 +436,9 @@ mod tests { assert!(!can_retry_without_sandbox( false, AskForApproval::OnRequest, - &codex_protocol::protocol::SandboxPolicy::ReadOnly, + &codex_protocol::protocol::SandboxPolicy::ReadOnly { + access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, + }, false )); } @@ -412,7 +448,9 @@ mod tests { assert!(can_retry_without_sandbox( false, AskForApproval::OnRequest, - &codex_protocol::protocol::SandboxPolicy::ReadOnly, + &codex_protocol::protocol::SandboxPolicy::ReadOnly { + access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, + }, true )); } @@ -432,7 +470,9 @@ mod tests { assert!(!can_retry_without_sandbox( false, AskForApproval::Never, - &codex_protocol::protocol::SandboxPolicy::ReadOnly, + &codex_protocol::protocol::SandboxPolicy::ReadOnly { + access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, + }, true )); } @@ -442,7 +482,9 @@ mod tests { assert!(can_retry_without_sandbox( true, AskForApproval::OnRequest, - &codex_protocol::protocol::SandboxPolicy::ReadOnly, + &codex_protocol::protocol::SandboxPolicy::ReadOnly { + access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, + }, false )); }