diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 45d77209c9..2e678f6780 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -16,12 +16,11 @@ use tokio::io::BufReader; use tokio::process::Child; use tokio_util::sync::CancellationToken; -use serde::Deserialize; - 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::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::ExecCommandOutputDeltaEvent; @@ -46,7 +45,6 @@ const SIGKILL_CODE: i32 = 9; const TIMEOUT_CODE: i32 = 64; const EXIT_CODE_SIGNAL_BASE: i32 = 128; // conventional shell: 128 + signal const EXEC_TIMEOUT_EXIT_CODE: i32 = 124; // conventional timeout exit code -const NETWORK_POLICY_DECISION_PREFIX: &str = "CODEX_NETWORK_POLICY_DECISION "; // I/O buffer sizing const READ_CHUNK_SIZE: usize = 8192; // bytes per read @@ -537,7 +535,7 @@ pub(crate) fn is_likely_sandbox_denied( return false; } - if has_network_policy_ask_decision(exec_output) { + if has_network_policy_blocking_decision(exec_output) { return true; } @@ -594,33 +592,15 @@ pub(crate) fn is_likely_sandbox_denied( false } -#[derive(Deserialize)] -struct NetworkPolicyDecisionPayload { - decision: String, - source: String, -} - -fn has_network_policy_ask_decision(exec_output: &ExecToolCallOutput) -> bool { +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() - .any(|section| section.lines().any(section_contains_network_policy_ask)) -} - -fn section_contains_network_policy_ask(line: &str) -> bool { - let payload = line.strip_prefix(NETWORK_POLICY_DECISION_PREFIX); - let Some(payload) = payload else { - return false; - }; - - let Ok(payload) = serde_json::from_str::(payload) else { - return false; - }; - - payload.decision == "ask" && payload.source == "decider" + .flat_map(|section| extract_network_policy_decisions(section)) + .any(|payload| payload.is_blocking_decision()) } #[derive(Debug, Clone)] @@ -1018,7 +998,7 @@ mod tests { } #[test] - fn sandbox_detection_ignores_network_policy_non_decider_with_zero_exit_code() { + fn sandbox_detection_flags_network_policy_non_decider_with_zero_exit_code() { let output = make_exec_output( 0, "", @@ -1026,12 +1006,36 @@ mod tests { 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}"#, + ); + assert!(!is_likely_sandbox_denied( SandboxType::LinuxSeccomp, &output )); } + #[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)."}"#, + ); + + assert!(is_likely_sandbox_denied(SandboxType::LinuxSeccomp, &output)); + } + #[tokio::test] async fn read_capped_limits_retained_bytes() { let (mut writer, reader) = tokio::io::duplex(1024); diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 53ac2e0d88..334438dbf5 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -41,6 +41,7 @@ pub mod landlock; pub mod mcp; mod mcp_connection_manager; pub mod models_manager; +mod network_policy_decision; pub mod network_proxy_loader; pub use mcp_connection_manager::MCP_SANDBOX_STATE_CAPABILITY; pub use mcp_connection_manager::MCP_SANDBOX_STATE_METHOD; diff --git a/codex-rs/core/src/network_policy_decision.rs b/codex-rs/core/src/network_policy_decision.rs new file mode 100644 index 0000000000..2f60dbd3bc --- /dev/null +++ b/codex-rs/core/src/network_policy_decision.rs @@ -0,0 +1,123 @@ +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 decision: String, + pub source: String, + pub protocol: Option, + pub host: Option, + pub reason: Option, + pub port: Option, +} + +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); + } + + if let Ok(value) = serde_json::from_str::(fragment) { + extract_policy_decisions_from_json_value(&value, &mut payloads); + } + + 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() +} + +#[cfg(test)] +mod tests { + use super::*; + 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}"#; + + let payloads = extract_network_policy_decisions(text); + 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), + }] + ); + } + + #[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/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index f393b01157..3bc2041295 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -10,6 +10,7 @@ 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::sandboxing::SandboxManager; use crate::tools::sandboxing::ApprovalCtx; use crate::tools::sandboxing::ExecApprovalRequirement; @@ -24,14 +25,11 @@ use codex_protocol::approvals::NetworkApprovalContext; use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; -use serde::Deserialize; pub(crate) struct ToolOrchestrator { sandbox: SandboxManager, } -const NETWORK_POLICY_DECISION_PREFIX: &str = "CODEX_NETWORK_POLICY_DECISION "; - impl ToolOrchestrator { pub fn new() -> Self { Self { @@ -273,21 +271,20 @@ fn extract_network_approval_context(output: &ExecToolCallOutput) -> Option Option { - text.lines() - .find_map(|line| line.strip_prefix(NETWORK_POLICY_DECISION_PREFIX)) - .and_then(|payload| serde_json::from_str::(payload).ok()) - .and_then(|payload| { - if payload.decision != "ask" || payload.source != "decider" { + extract_network_policy_decisions(text) + .into_iter() + .find_map(|payload| { + if !payload.is_ask_from_decider() { return None; } - let protocol = match payload.protocol.as_str() { - "http" => NetworkApprovalProtocol::Http, - "https" | "https_connect" => NetworkApprovalProtocol::Https, + let protocol = match payload.protocol.as_deref() { + Some("http") => NetworkApprovalProtocol::Http, + Some("https") | Some("https_connect") => NetworkApprovalProtocol::Https, _ => return None, }; - let host = payload.host.trim(); + let host = payload.host.as_deref()?.trim(); if host.is_empty() { return None; } @@ -299,15 +296,6 @@ fn extract_network_approval_context_from_text(text: &str) -> Option