mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
fix(core): generalize network policy decision extraction
This commit is contained in:
@@ -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::<NetworkPolicyDecisionPayload>(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);
|
||||
|
||||
@@ -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;
|
||||
|
||||
123
codex-rs/core/src/network_policy_decision.rs
Normal file
123
codex-rs/core/src/network_policy_decision.rs
Normal file
@@ -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<String>,
|
||||
pub host: Option<String>,
|
||||
pub reason: Option<String>,
|
||||
pub port: Option<u16>,
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
if let Ok(value) = serde_json::from_str::<Value>(fragment) {
|
||||
extract_policy_decisions_from_json_value(&value, &mut payloads);
|
||||
}
|
||||
|
||||
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()
|
||||
}
|
||||
|
||||
#[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());
|
||||
}
|
||||
}
|
||||
@@ -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<Netwo
|
||||
}
|
||||
|
||||
fn extract_network_approval_context_from_text(text: &str) -> Option<NetworkApprovalContext> {
|
||||
text.lines()
|
||||
.find_map(|line| line.strip_prefix(NETWORK_POLICY_DECISION_PREFIX))
|
||||
.and_then(|payload| serde_json::from_str::<NetworkPolicyDecisionPayload>(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<NetworkAppro
|
||||
})
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
struct NetworkPolicyDecisionPayload {
|
||||
decision: String,
|
||||
source: String,
|
||||
protocol: String,
|
||||
host: String,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -368,6 +356,19 @@ mod tests {
|
||||
assert_eq!(extract_network_approval_context_from_text(text), 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)."}"#;
|
||||
|
||||
assert_eq!(
|
||||
extract_network_approval_context_from_text(text),
|
||||
Some(NetworkApprovalContext {
|
||||
host: "example.com".to_string(),
|
||||
protocol: NetworkApprovalProtocol::Https,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn can_retry_without_sandbox_respects_default_on_request_gate() {
|
||||
assert!(!can_retry_without_sandbox(
|
||||
|
||||
Reference in New Issue
Block a user