mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
fix(core): surface denylist network blocks as structured rejections
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
use codex_network_proxy::BlockedRequest;
|
||||
use codex_protocol::approvals::NetworkApprovalContext;
|
||||
use codex_protocol::approvals::NetworkApprovalProtocol;
|
||||
use serde::Deserialize;
|
||||
@@ -43,9 +44,38 @@ pub(crate) fn network_approval_context_from_payload(
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn denied_network_policy_message(blocked: &BlockedRequest) -> Option<String> {
|
||||
if !blocked
|
||||
.decision
|
||||
.as_deref()
|
||||
.is_some_and(|decision| decision.eq_ignore_ascii_case("deny"))
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
let host = blocked.host.trim();
|
||||
if host.is_empty() {
|
||||
return Some("Network access was blocked by policy.".to_string());
|
||||
}
|
||||
|
||||
let detail = match blocked.reason.as_str() {
|
||||
"denied" => "domain is explicitly denied by policy and cannot be approved from this prompt",
|
||||
"not_allowed" => "domain is not on the allowlist for the current sandbox mode",
|
||||
"not_allowed_local" => "local/private network addresses are blocked by policy",
|
||||
"method_not_allowed" => "request method is blocked by the current network mode",
|
||||
"proxy_disabled" => "managed network proxy is disabled",
|
||||
_ => "request is blocked by network policy",
|
||||
};
|
||||
|
||||
Some(format!(
|
||||
"Network access to \"{host}\" was blocked: {detail}."
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_network_proxy::BlockedRequest;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
@@ -96,4 +126,45 @@ mod tests {
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn denied_network_policy_message_requires_deny_decision() {
|
||||
let blocked = BlockedRequest {
|
||||
host: "google.com".to_string(),
|
||||
reason: "not_allowed".to_string(),
|
||||
client: None,
|
||||
method: Some("GET".to_string()),
|
||||
mode: None,
|
||||
protocol: "http".to_string(),
|
||||
attempt_id: Some("attempt-1".to_string()),
|
||||
decision: Some("ask".to_string()),
|
||||
source: Some("decider".to_string()),
|
||||
port: Some(80),
|
||||
timestamp: 0,
|
||||
};
|
||||
assert_eq!(denied_network_policy_message(&blocked), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn denied_network_policy_message_for_denylist_block_is_explicit() {
|
||||
let blocked = BlockedRequest {
|
||||
host: "google.com".to_string(),
|
||||
reason: "denied".to_string(),
|
||||
client: None,
|
||||
method: Some("GET".to_string()),
|
||||
mode: None,
|
||||
protocol: "http".to_string(),
|
||||
attempt_id: Some("attempt-1".to_string()),
|
||||
decision: Some("deny".to_string()),
|
||||
source: Some("baseline_policy".to_string()),
|
||||
port: Some(80),
|
||||
timestamp: 0,
|
||||
};
|
||||
assert_eq!(
|
||||
denied_network_policy_message(&blocked),
|
||||
Some(
|
||||
"Network access to \"google.com\" was blocked: domain is explicitly denied by policy and cannot be approved from this prompt.".to_string()
|
||||
)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ use crate::codex::NetworkApprovalOutcome;
|
||||
use crate::command_canonicalization::canonicalize_command_for_approval;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::features::Feature;
|
||||
use crate::network_policy_decision::denied_network_policy_message;
|
||||
use crate::powershell::prefix_powershell_script_with_utf8;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::sandboxing::execute_env;
|
||||
@@ -196,6 +197,22 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let network_policy_denial_message = if let (Some(network), Some(attempt_id)) =
|
||||
(req.network.as_ref(), network_attempt_id.as_deref())
|
||||
{
|
||||
match network.latest_blocked_request_for_attempt(attempt_id).await {
|
||||
Ok(Some(blocked)) => denied_network_policy_message(&blocked),
|
||||
Ok(None) => None,
|
||||
Err(err) => {
|
||||
tracing::debug!(
|
||||
"failed to read blocked network telemetry for attempt {attempt_id}: {err}"
|
||||
);
|
||||
None
|
||||
}
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
if let Some(attempt_id) = network_attempt_id.as_deref() {
|
||||
ctx.session
|
||||
@@ -206,6 +223,9 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
if approval_outcome == Some(NetworkApprovalOutcome::DeniedByUser) {
|
||||
return Err(ToolError::Rejected("rejected by user".to_string()));
|
||||
}
|
||||
if let Some(message) = network_policy_denial_message {
|
||||
return Err(ToolError::Rejected(message));
|
||||
}
|
||||
|
||||
let out = out?;
|
||||
Ok(out)
|
||||
|
||||
@@ -16,6 +16,7 @@ use uuid::Uuid;
|
||||
use crate::codex::NetworkApprovalOutcome;
|
||||
use crate::exec_env::create_env;
|
||||
use crate::exec_policy::ExecApprovalRequest;
|
||||
use crate::network_policy_decision::denied_network_policy_message;
|
||||
use crate::protocol::ExecCommandSource;
|
||||
use crate::sandboxing::ExecRequest;
|
||||
use crate::tools::events::ToolEmitter;
|
||||
@@ -90,6 +91,25 @@ fn apply_unified_exec_env(mut env: HashMap<String, String>) -> HashMap<String, S
|
||||
env
|
||||
}
|
||||
|
||||
async fn network_policy_denial_message_for_attempt(
|
||||
network: Option<&codex_network_proxy::NetworkProxy>,
|
||||
attempt_id: Option<&str>,
|
||||
) -> Option<String> {
|
||||
let (Some(network), Some(attempt_id)) = (network, attempt_id) else {
|
||||
return None;
|
||||
};
|
||||
match network.latest_blocked_request_for_attempt(attempt_id).await {
|
||||
Ok(Some(blocked)) => denied_network_policy_message(&blocked),
|
||||
Ok(None) => None,
|
||||
Err(err) => {
|
||||
tracing::debug!(
|
||||
"failed to read blocked network telemetry for attempt {attempt_id}: {err}"
|
||||
);
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
struct PreparedProcessHandles {
|
||||
writer_tx: mpsc::Sender<Vec<u8>>,
|
||||
output_buffer: OutputBuffer,
|
||||
@@ -272,6 +292,14 @@ impl UnifiedExecProcessManager {
|
||||
"rejected by user".to_string(),
|
||||
));
|
||||
}
|
||||
if let Some(message) = network_policy_denial_message_for_attempt(
|
||||
request.network.as_ref(),
|
||||
network_attempt_id.as_deref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
return Err(UnifiedExecError::create_process(message));
|
||||
}
|
||||
process.check_for_sandbox_denial_with_text(&text).await?;
|
||||
|
||||
let original_token_count = approx_token_count(&text);
|
||||
@@ -311,6 +339,16 @@ impl UnifiedExecProcessManager {
|
||||
"rejected by user".to_string(),
|
||||
));
|
||||
}
|
||||
if let Some(message) = network_policy_denial_message_for_attempt(
|
||||
request.network.as_ref(),
|
||||
network_attempt_id.as_deref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
process.terminate();
|
||||
self.release_process_id(&request.process_id).await;
|
||||
return Err(UnifiedExecError::create_process(message));
|
||||
}
|
||||
|
||||
// Long‑lived command: persist the process so write_stdin can reuse
|
||||
// it, and register a background watcher that will emit
|
||||
|
||||
Reference in New Issue
Block a user