openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-30 19:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(core): use structured telemetry for network approval retries

Browse Source
This commit is contained in:
viyatb-oai
2026-02-12 16:32:36 -08:00
parent 7cc07443f8
commit 2481870c00
3 changed files with 235 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/config/network_proxy_spec.rs
+6 -2
View File
@@ -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,
codex-rs/core/src/exec.rs
+183 -23
View File
@@ -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<NetworkPolicyDecisionPayload> {
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<codex_network_proxy::BlockedRequest>,
sandbox_policy: &SandboxPolicy,
) -> 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 {
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<NetworkPolicyDecisionPayload> {
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<T: Clone> {
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);
codex-rs/core/src/tools/orchestrator.rs
+46 -4
View File
@@ -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("<none>"),
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
));
}