mirror of
https://github.com/openai/codex.git
synced 2026-04-28 08:34:54 +00:00
Refactor network approvals to host/protocol/port scope (#12140)
## Summary Simplify network approvals by removing per-attempt proxy correlation and moving to session-level approval dedupe keyed by (host, protocol, port). Instead of encoding attempt IDs into proxy credentials/URLs, we now treat approvals as a destination policy decision. - Concurrent calls to the same destination share one approval prompt. - Different destinations (or same host on different ports) get separate prompts. - Allow once approves the current queued request group only. - Allow for session caches that (host, protocol, port) and auto-allows future matching requests. - Never policy continues to deny without prompting. Example: - 3 calls: - a.com (line 443) - b.com (line 443) - a.com (line 443) => 2 prompts total (a, b), second a waits on the first decision. - a.com:80 is treated separately from a.com line 443 ## Testing - `just fmt` (in `codex-rs`) - `cargo test -p codex-core tools::network_approval::tests` - `cargo test -p codex-core` (unit tests pass; existing integration-suite failures remain in this environment)
This commit is contained in:
@@ -1,5 +1,4 @@
|
||||
use crate::config::NetworkMode;
|
||||
use crate::metadata::attempt_id_from_proxy_authorization;
|
||||
use crate::network_policy::NetworkDecision;
|
||||
use crate::network_policy::NetworkDecisionSource;
|
||||
use crate::network_policy::NetworkPolicyDecider;
|
||||
@@ -160,8 +159,6 @@ async fn http_connect_accept(
|
||||
}
|
||||
|
||||
let client = client_addr(&req);
|
||||
let network_attempt_id = request_network_attempt_id(&req);
|
||||
|
||||
let enabled = app_state
|
||||
.enabled()
|
||||
.await
|
||||
@@ -188,7 +185,6 @@ async fn http_connect_accept(
|
||||
method: Some("CONNECT".to_string()),
|
||||
command: None,
|
||||
exec_policy_hint: None,
|
||||
attempt_id: network_attempt_id.clone(),
|
||||
});
|
||||
|
||||
match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await {
|
||||
@@ -213,7 +209,6 @@ async fn http_connect_accept(
|
||||
method: Some("CONNECT".to_string()),
|
||||
mode: None,
|
||||
protocol: "http-connect".to_string(),
|
||||
attempt_id: network_attempt_id.clone(),
|
||||
decision: Some(details.decision.as_str().to_string()),
|
||||
source: Some(details.source.as_str().to_string()),
|
||||
port: Some(authority.port),
|
||||
@@ -255,7 +250,6 @@ async fn http_connect_accept(
|
||||
method: Some("CONNECT".to_string()),
|
||||
mode: Some(NetworkMode::Limited),
|
||||
protocol: "http-connect".to_string(),
|
||||
attempt_id: network_attempt_id,
|
||||
decision: Some(details.decision.as_str().to_string()),
|
||||
source: Some(details.source.as_str().to_string()),
|
||||
port: Some(authority.port),
|
||||
@@ -374,8 +368,6 @@ async fn http_plain_proxy(
|
||||
}
|
||||
};
|
||||
let client = client_addr(&req);
|
||||
let network_attempt_id = request_network_attempt_id(&req);
|
||||
|
||||
let method_allowed = match app_state
|
||||
.method_allowed(req.method().as_str())
|
||||
.await
|
||||
@@ -504,7 +496,6 @@ async fn http_plain_proxy(
|
||||
method: Some(req.method().as_str().to_string()),
|
||||
command: None,
|
||||
exec_policy_hint: None,
|
||||
attempt_id: network_attempt_id.clone(),
|
||||
});
|
||||
|
||||
match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await {
|
||||
@@ -529,7 +520,6 @@ async fn http_plain_proxy(
|
||||
method: Some(req.method().as_str().to_string()),
|
||||
mode: None,
|
||||
protocol: "http".to_string(),
|
||||
attempt_id: network_attempt_id.clone(),
|
||||
decision: Some(details.decision.as_str().to_string()),
|
||||
source: Some(details.source.as_str().to_string()),
|
||||
port: Some(port),
|
||||
@@ -563,7 +553,6 @@ async fn http_plain_proxy(
|
||||
method: Some(req.method().as_str().to_string()),
|
||||
mode: Some(NetworkMode::Limited),
|
||||
protocol: "http".to_string(),
|
||||
attempt_id: network_attempt_id,
|
||||
decision: Some(details.decision.as_str().to_string()),
|
||||
source: Some(details.source.as_str().to_string()),
|
||||
port: Some(port),
|
||||
@@ -645,12 +634,6 @@ fn client_addr<T: ExtensionsRef>(input: &T) -> Option<String> {
|
||||
.map(|info| info.peer_addr().to_string())
|
||||
}
|
||||
|
||||
fn request_network_attempt_id(req: &Request) -> Option<String> {
|
||||
// Some HTTP stacks normalize proxy credentials into `authorization`; accept both.
|
||||
attempt_id_from_proxy_authorization(req.headers().get("proxy-authorization"))
|
||||
.or_else(|| attempt_id_from_proxy_authorization(req.headers().get("authorization")))
|
||||
}
|
||||
|
||||
fn remove_hop_by_hop_request_headers(headers: &mut HeaderMap) {
|
||||
while let Some(raw_connection) = headers.get(header::CONNECTION).cloned() {
|
||||
headers.remove(header::CONNECTION);
|
||||
@@ -738,7 +721,6 @@ async fn proxy_disabled_response(
|
||||
method,
|
||||
mode: None,
|
||||
protocol: protocol.as_policy_protocol().to_string(),
|
||||
attempt_id: None,
|
||||
decision: Some("deny".to_string()),
|
||||
source: Some("proxy_state".to_string()),
|
||||
port: Some(port),
|
||||
@@ -796,8 +778,6 @@ mod tests {
|
||||
use crate::config::NetworkMode;
|
||||
use crate::config::NetworkProxySettings;
|
||||
use crate::runtime::network_proxy_state_for_policy;
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rama_http::Method;
|
||||
use rama_http::Request;
|
||||
@@ -873,36 +853,6 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_network_attempt_id_reads_proxy_authorization_header() {
|
||||
let encoded = STANDARD.encode("codex-net-attempt-attempt-1:");
|
||||
let req = Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("http://example.com")
|
||||
.header("proxy-authorization", format!("Basic {encoded}"))
|
||||
.body(Body::empty())
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
request_network_attempt_id(&req),
|
||||
Some("attempt-1".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_network_attempt_id_reads_authorization_header_fallback() {
|
||||
let encoded = STANDARD.encode("codex-net-attempt-attempt-2:");
|
||||
let req = Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("http://example.com")
|
||||
.header("authorization", format!("Basic {encoded}"))
|
||||
.body(Body::empty())
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
request_network_attempt_id(&req),
|
||||
Some("attempt-2".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_hop_by_hop_request_headers_keeps_forwarding_headers() {
|
||||
let mut headers = HeaderMap::new();
|
||||
|
||||
Reference in New Issue
Block a user