mirror of
https://github.com/openai/codex.git
synced 2026-09-16 12:13:30 +00:00
fix(core): finalize network approval retry and allow-once flow
This commit is contained in:
@@ -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<ExecToolCallOutput> },
|
||||
Denied {
|
||||
output: Box<ExecToolCallOutput>,
|
||||
network_policy_decision: Option<NetworkPolicyDecisionPayload>,
|
||||
},
|
||||
|
||||
/// 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),
|
||||
|
||||
@@ -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<NetworkPolicyDecisionPayload> {
|
||||
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<codex_network_proxy::BlockedRequest>,
|
||||
) -> Option<NetworkPolicyDecisionPayload> {
|
||||
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<NetworkPolicyDecisionPayload> {
|
||||
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]
|
||||
|
||||
@@ -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<String>,
|
||||
@@ -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<NetworkPolicyDecisionPayload> {
|
||||
text.lines()
|
||||
.flat_map(extract_policy_decisions_from_fragment)
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn extract_policy_decisions_from_fragment(fragment: &str) -> Vec<NetworkPolicyDecisionPayload> {
|
||||
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<NetworkApprovalContext> {
|
||||
if !payload.is_ask_from_decider() {
|
||||
return None;
|
||||
}
|
||||
|
||||
if let Ok(value) = serde_json::from_str::<Value>(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<NetworkPolicyDecisionPayload>,
|
||||
) {
|
||||
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<NetworkPolicyDecisionPayload> {
|
||||
let payload = text.strip_prefix(NETWORK_POLICY_DECISION_PREFIX)?;
|
||||
serde_json::from_str::<NetworkPolicyDecisionPayload>(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());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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<NetworkApprovalContext>,
|
||||
}
|
||||
|
||||
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<NetworkApprovalContext> {
|
||||
[
|
||||
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<NetworkApprovalContext> {
|
||||
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,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -187,6 +187,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> 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()),
|
||||
|
||||
Reference in New Issue
Block a user