mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Resolve outbound proxy routes explicitly (#34435)
## Why System proxy discovery can block, and delegating fallback to each transport can repeat discovery or apply inconsistent environment proxy behavior. ## What changed - Resolve unavailable system proxy decisions to an explicit environment proxy or direct route, including `NO_PROXY` settings and WebSocket-specific fallbacks. - Add asynchronous system proxy resolution that uses cached decisions first and serializes blocking platform lookups on Windows and macOS. - Preserve `NO_PROXY` handling for WebSocket connections, including HTTP and HTTPS proxies, and hash proxy cache keys on every platform. ## Testing - Add coverage for explicit environment fallback, cached asynchronous resolution, and proxied or bypassed WebSocket connections. GitOrigin-RevId: 3b7cf170dcfe639eec53c3c1514f92bfcf13e7e9
This commit is contained in:
@@ -16,15 +16,13 @@ rustls-native-certs = { workspace = true }
|
||||
rustls-pki-types = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
sha2 = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
tokio = { workspace = true, features = ["macros", "rt", "time", "sync"] }
|
||||
tracing = { workspace = true }
|
||||
tracing-opentelemetry = { workspace = true }
|
||||
zstd = { workspace = true }
|
||||
|
||||
[target.'cfg(any(target_os = "windows", target_os = "macos"))'.dependencies]
|
||||
sha2 = { workspace = true }
|
||||
|
||||
[target.'cfg(target_os = "macos")'.dependencies]
|
||||
system-configuration = { workspace = true }
|
||||
|
||||
|
||||
@@ -12,19 +12,21 @@ use std::sync::Mutex;
|
||||
use std::sync::OnceLock;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
#[cfg(any(target_os = "windows", target_os = "macos"))]
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use crate::custom_ca::BuildCustomCaTransportError;
|
||||
use crate::custom_ca::build_reqwest_client_with_custom_ca;
|
||||
use crate::default_client::HttpClient;
|
||||
#[cfg(any(target_os = "windows", target_os = "macos"))]
|
||||
use sha2::Digest;
|
||||
#[cfg(any(target_os = "windows", target_os = "macos"))]
|
||||
use sha2::Sha256;
|
||||
use thiserror::Error;
|
||||
|
||||
const SYSTEM_PROXY_SUCCESS_CACHE_TTL: Duration = Duration::from_secs(60);
|
||||
const SYSTEM_PROXY_UNAVAILABLE_CACHE_TTL: Duration = Duration::from_secs(5);
|
||||
const SYSTEM_PROXY_CACHE_MAX_ENTRIES: usize = 256;
|
||||
#[cfg(any(target_os = "windows", target_os = "macos"))]
|
||||
static ASYNC_SYSTEM_PROXY_RESOLUTION_PERMIT: Semaphore = Semaphore::const_new(1);
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
mod macos;
|
||||
@@ -100,16 +102,22 @@ pub enum OutboundProxyPolicy {
|
||||
|
||||
/// Resolved proxy route for a concrete outbound destination.
|
||||
///
|
||||
/// `TransportDefault` delegates environment-proxy handling to the underlying transport. Proxy
|
||||
/// URLs are intentionally redacted from `Debug` output because they may contain credentials.
|
||||
#[derive(Clone, PartialEq, Eq)]
|
||||
/// `TransportDefault` preserves the underlying transport behavior only when system-proxy support
|
||||
/// is disabled. When system resolution is enabled, environment and direct fallbacks are resolved
|
||||
/// explicitly so the transport cannot repeat system discovery. Proxy URLs and no-proxy settings
|
||||
/// are intentionally redacted from `Debug` output because they may contain credentials or private
|
||||
/// hostnames.
|
||||
#[derive(Clone, Hash, PartialEq, Eq)]
|
||||
pub enum OutboundProxyRoute {
|
||||
/// Preserve the underlying transport's existing proxy behavior.
|
||||
TransportDefault,
|
||||
/// Connect directly and bypass transport-level proxy discovery.
|
||||
Direct,
|
||||
/// Connect through the selected proxy URL.
|
||||
Proxy { url: String },
|
||||
Proxy {
|
||||
url: String,
|
||||
no_proxy: Option<String>,
|
||||
},
|
||||
}
|
||||
|
||||
impl fmt::Debug for OutboundProxyRoute {
|
||||
@@ -117,7 +125,11 @@ impl fmt::Debug for OutboundProxyRoute {
|
||||
match self {
|
||||
Self::TransportDefault => f.write_str("TransportDefault"),
|
||||
Self::Direct => f.write_str("Direct"),
|
||||
Self::Proxy { .. } => f.debug_struct("Proxy").field("url", &"<redacted>").finish(),
|
||||
Self::Proxy { .. } => f
|
||||
.debug_struct("Proxy")
|
||||
.field("url", &"<redacted>")
|
||||
.field("no_proxy", &"<redacted>")
|
||||
.finish(),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -149,16 +161,64 @@ impl HttpClientFactory {
|
||||
///
|
||||
/// WebSocket schemes are resolved through their HTTP equivalents so platform PAC and system
|
||||
/// proxy APIs apply the same policy to `ws`/`wss` and `http`/`https` destinations. When system
|
||||
/// resolution is unavailable, the transport retains responsibility for environment-proxy
|
||||
/// fallback.
|
||||
/// resolution is unavailable, explicit environment settings are resolved before falling back
|
||||
/// to a direct route.
|
||||
pub fn resolve_proxy_route(&self, request_url: &str) -> OutboundProxyRoute {
|
||||
resolve_proxy_route(
|
||||
&ProcessEnv,
|
||||
request_url,
|
||||
self.outbound_proxy_policy,
|
||||
resolve_system_proxy,
|
||||
)
|
||||
}
|
||||
|
||||
/// Resolves the proxy route for a concrete destination without blocking a Tokio worker.
|
||||
pub async fn resolve_proxy_route_async(
|
||||
&self,
|
||||
request_url: String,
|
||||
) -> io::Result<OutboundProxyRoute> {
|
||||
if matches!(
|
||||
self.outbound_proxy_policy,
|
||||
OutboundProxyPolicy::ReqwestDefault
|
||||
) {
|
||||
return Ok(OutboundProxyRoute::TransportDefault);
|
||||
}
|
||||
|
||||
if let Some(route) = self.cached_proxy_route(&request_url) {
|
||||
return Ok(route);
|
||||
}
|
||||
|
||||
#[cfg(not(any(target_os = "windows", target_os = "macos")))]
|
||||
return Ok(self.resolve_proxy_route(&request_url));
|
||||
|
||||
#[cfg(any(target_os = "windows", target_os = "macos"))]
|
||||
{
|
||||
let permit = ASYNC_SYSTEM_PROXY_RESOLUTION_PERMIT
|
||||
.acquire()
|
||||
.await
|
||||
.map_err(io::Error::other)?;
|
||||
let factory = self.clone();
|
||||
tokio::task::spawn_blocking(move || {
|
||||
// Keep the permit with the blocking task: cancelling the caller must not allow a
|
||||
// second PAC/WinHTTP lookup to start while this one is still running.
|
||||
let _permit = permit;
|
||||
factory.resolve_proxy_route(&request_url)
|
||||
})
|
||||
.await
|
||||
.map_err(io::Error::other)
|
||||
}
|
||||
}
|
||||
|
||||
fn cached_proxy_route(&self, request_url: &str) -> Option<OutboundProxyRoute> {
|
||||
let env_proxy_kind = EnvProxyKind::from_request_url(request_url);
|
||||
let request_url = proxy_resolution_url(request_url);
|
||||
if RequestOrigin::parse(&request_url).is_none() {
|
||||
return Some(OutboundProxyRoute::Direct);
|
||||
}
|
||||
cached_system_proxy_decision(&request_url)
|
||||
.map(|decision| route_from_system_decision(&ProcessEnv, env_proxy_kind, decision))
|
||||
}
|
||||
|
||||
/// Builds an HTTP client for a concrete outbound route.
|
||||
pub fn build_client(
|
||||
&self,
|
||||
@@ -196,6 +256,7 @@ impl HttpClientFactory {
|
||||
}
|
||||
|
||||
fn resolve_proxy_route(
|
||||
env: &dyn EnvSource,
|
||||
request_url: &str,
|
||||
outbound_proxy_policy: OutboundProxyPolicy,
|
||||
resolve_system_proxy: impl FnOnce(&str, &RequestOrigin) -> SystemProxyDecision,
|
||||
@@ -204,15 +265,79 @@ fn resolve_proxy_route(
|
||||
return OutboundProxyRoute::TransportDefault;
|
||||
}
|
||||
|
||||
let env_proxy_kind = EnvProxyKind::from_request_url(request_url);
|
||||
let request_url = proxy_resolution_url(request_url);
|
||||
let Some(origin) = RequestOrigin::parse(&request_url) else {
|
||||
return OutboundProxyRoute::TransportDefault;
|
||||
return OutboundProxyRoute::Direct;
|
||||
};
|
||||
|
||||
match resolve_system_proxy(&request_url, &origin) {
|
||||
route_from_system_decision(
|
||||
env,
|
||||
env_proxy_kind,
|
||||
resolve_system_proxy(&request_url, &origin),
|
||||
)
|
||||
}
|
||||
|
||||
fn route_from_system_decision(
|
||||
env: &dyn EnvSource,
|
||||
env_proxy_kind: EnvProxyKind,
|
||||
decision: SystemProxyDecision,
|
||||
) -> OutboundProxyRoute {
|
||||
match decision {
|
||||
SystemProxyDecision::Direct => OutboundProxyRoute::Direct,
|
||||
SystemProxyDecision::Proxy { url } => OutboundProxyRoute::Proxy { url },
|
||||
SystemProxyDecision::Unavailable { .. } => OutboundProxyRoute::TransportDefault,
|
||||
SystemProxyDecision::Proxy { url } => OutboundProxyRoute::Proxy {
|
||||
url,
|
||||
no_proxy: None,
|
||||
},
|
||||
SystemProxyDecision::Unavailable { .. } => resolve_env_proxy_route(env, env_proxy_kind),
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_env_proxy_route(
|
||||
env: &dyn EnvSource,
|
||||
env_proxy_kind: EnvProxyKind,
|
||||
) -> OutboundProxyRoute {
|
||||
let proxy_url = match env_proxy_kind {
|
||||
EnvProxyKind::Https => {
|
||||
proxy_env_value(env, "HTTPS_PROXY").or_else(|| proxy_env_value(env, "ALL_PROXY"))
|
||||
}
|
||||
EnvProxyKind::SecureWebSocket => proxy_env_value(env, "HTTPS_PROXY")
|
||||
.or_else(|| proxy_env_value(env, "HTTP_PROXY"))
|
||||
.or_else(|| proxy_env_value(env, "ALL_PROXY")),
|
||||
EnvProxyKind::Http => {
|
||||
proxy_env_value(env, "HTTP_PROXY").or_else(|| proxy_env_value(env, "ALL_PROXY"))
|
||||
}
|
||||
EnvProxyKind::Other => proxy_env_value(env, "ALL_PROXY"),
|
||||
};
|
||||
match proxy_url {
|
||||
Some(url) => OutboundProxyRoute::Proxy {
|
||||
url,
|
||||
no_proxy: proxy_env_value(env, "NO_PROXY"),
|
||||
},
|
||||
None => OutboundProxyRoute::Direct,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum EnvProxyKind {
|
||||
Http,
|
||||
Https,
|
||||
SecureWebSocket,
|
||||
Other,
|
||||
}
|
||||
|
||||
impl EnvProxyKind {
|
||||
fn from_request_url(request_url: &str) -> Self {
|
||||
let scheme = request_url
|
||||
.parse::<http::Uri>()
|
||||
.ok()
|
||||
.and_then(|uri| uri.scheme_str().map(str::to_ascii_lowercase));
|
||||
match scheme.as_deref() {
|
||||
Some("http" | "ws") => Self::Http,
|
||||
Some("https") => Self::Https,
|
||||
Some("wss") => Self::SecureWebSocket,
|
||||
Some(_) | None => Self::Other,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -276,22 +401,26 @@ fn configure_proxy_for_route(
|
||||
outbound_proxy_policy: OutboundProxyPolicy,
|
||||
resolve_system_proxy: impl FnOnce(&str, &RequestOrigin) -> SystemProxyDecision,
|
||||
) -> Result<reqwest::ClientBuilder, BuildRouteAwareHttpClientError> {
|
||||
if matches!(outbound_proxy_policy, OutboundProxyPolicy::ReqwestDefault) {
|
||||
return Ok(builder);
|
||||
}
|
||||
let origin = RequestOrigin::parse(request_url);
|
||||
let route = resolve_proxy_route(
|
||||
env,
|
||||
request_url,
|
||||
outbound_proxy_policy,
|
||||
resolve_system_proxy,
|
||||
);
|
||||
configure_builder_for_resolved_route(builder, route_class, &route)
|
||||
}
|
||||
|
||||
let Some(origin) = origin.as_ref() else {
|
||||
return configure_env_proxy_handling(env, builder, /*origin*/ None, route_class);
|
||||
};
|
||||
|
||||
match resolve_system_proxy(request_url, origin) {
|
||||
SystemProxyDecision::Direct => Ok(builder.no_proxy()),
|
||||
SystemProxyDecision::Proxy { url } => {
|
||||
configure_concrete_proxy(builder, route_class, &url, /*no_proxy*/ None)
|
||||
}
|
||||
SystemProxyDecision::Unavailable { .. } => {
|
||||
configure_env_proxy_handling(env, builder, Some(origin), route_class)
|
||||
fn configure_builder_for_resolved_route(
|
||||
builder: reqwest::ClientBuilder,
|
||||
route_class: ClientRouteClass,
|
||||
route: &OutboundProxyRoute,
|
||||
) -> Result<reqwest::ClientBuilder, BuildRouteAwareHttpClientError> {
|
||||
match route {
|
||||
OutboundProxyRoute::TransportDefault => Ok(builder),
|
||||
OutboundProxyRoute::Direct => Ok(builder.no_proxy()),
|
||||
OutboundProxyRoute::Proxy { url, no_proxy } => {
|
||||
let no_proxy = no_proxy.as_deref().and_then(reqwest::NoProxy::from_string);
|
||||
configure_concrete_proxy(builder, route_class, url, no_proxy)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -311,31 +440,6 @@ fn configure_concrete_proxy(
|
||||
Ok(builder.proxy(proxy.no_proxy(no_proxy)))
|
||||
}
|
||||
|
||||
fn configure_env_proxy_handling(
|
||||
env: &dyn EnvSource,
|
||||
builder: reqwest::ClientBuilder,
|
||||
origin: Option<&RequestOrigin>,
|
||||
route_class: ClientRouteClass,
|
||||
) -> Result<reqwest::ClientBuilder, BuildRouteAwareHttpClientError> {
|
||||
if let Some(origin) = origin {
|
||||
let proxy_url = match origin.scheme.as_str() {
|
||||
"https" => {
|
||||
proxy_env_value(env, "HTTPS_PROXY").or_else(|| proxy_env_value(env, "ALL_PROXY"))
|
||||
}
|
||||
"http" => {
|
||||
proxy_env_value(env, "HTTP_PROXY").or_else(|| proxy_env_value(env, "ALL_PROXY"))
|
||||
}
|
||||
_ => proxy_env_value(env, "ALL_PROXY"),
|
||||
};
|
||||
if let Some(proxy_url) = proxy_url {
|
||||
let no_proxy = proxy_env_value(env, "NO_PROXY")
|
||||
.and_then(|value| reqwest::NoProxy::from_string(&value));
|
||||
return configure_concrete_proxy(builder, route_class, &proxy_url, no_proxy);
|
||||
}
|
||||
}
|
||||
Ok(builder.no_proxy())
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
#[allow(dead_code)]
|
||||
struct RequestOrigin {
|
||||
@@ -431,7 +535,6 @@ struct CachedSystemProxyDecision {
|
||||
static SYSTEM_PROXY_CACHE: OnceLock<Mutex<HashMap<String, CachedSystemProxyDecision>>> =
|
||||
OnceLock::new();
|
||||
|
||||
#[cfg(test)]
|
||||
fn cached_system_proxy_decision(request_url: &str) -> Option<SystemProxyDecision> {
|
||||
let cache = SYSTEM_PROXY_CACHE.get_or_init(|| Mutex::new(HashMap::new()));
|
||||
let mut cache = cache.lock().ok()?;
|
||||
@@ -494,17 +597,11 @@ fn insert_system_proxy_cache_entry(
|
||||
}
|
||||
|
||||
fn system_proxy_cache_key(request_url: &str) -> String {
|
||||
#[cfg(any(target_os = "windows", target_os = "macos"))]
|
||||
{
|
||||
// Keep URL-specific PAC decisions without retaining the raw routed URL.
|
||||
let mut hasher = Sha256::new();
|
||||
hasher.update(b"system-proxy-cache-v1\0");
|
||||
hasher.update(request_url.as_bytes());
|
||||
format!("{:x}", hasher.finalize())
|
||||
}
|
||||
|
||||
#[cfg(not(any(target_os = "windows", target_os = "macos")))]
|
||||
request_url.to_string()
|
||||
// Keep URL-specific PAC decisions without retaining the raw routed URL.
|
||||
let mut hasher = Sha256::new();
|
||||
hasher.update(b"system-proxy-cache-v1\0");
|
||||
hasher.update(request_url.as_bytes());
|
||||
format!("{:x}", hasher.finalize())
|
||||
}
|
||||
|
||||
#[cfg(any(test, target_os = "windows"))]
|
||||
|
||||
@@ -12,7 +12,11 @@ struct MapEnv {
|
||||
|
||||
#[test]
|
||||
fn websocket_route_uses_http_equivalent_for_system_resolution() {
|
||||
let env = MapEnv {
|
||||
values: HashMap::new(),
|
||||
};
|
||||
let route = resolve_proxy_route(
|
||||
&env,
|
||||
"wss://api.openai.com/v1/responses",
|
||||
OutboundProxyPolicy::RespectSystemProxy,
|
||||
|request_url, origin| {
|
||||
@@ -30,13 +34,18 @@ fn websocket_route_uses_http_equivalent_for_system_resolution() {
|
||||
route,
|
||||
OutboundProxyRoute::Proxy {
|
||||
url: "http://proxy.example:8080".to_string(),
|
||||
no_proxy: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reqwest_default_route_preserves_transport_proxy_behavior() {
|
||||
let env = MapEnv {
|
||||
values: HashMap::new(),
|
||||
};
|
||||
let route = resolve_proxy_route(
|
||||
&env,
|
||||
"wss://api.openai.com/v1/responses",
|
||||
OutboundProxyPolicy::ReqwestDefault,
|
||||
|_, _| panic!("default policy should not resolve system proxy settings"),
|
||||
@@ -79,12 +88,11 @@ fn environment_fallback_reads_injected_proxy_environment() {
|
||||
let env = MapEnv {
|
||||
values: HashMap::from([("HTTPS_PROXY".to_string(), "://invalid".to_string())]),
|
||||
};
|
||||
let origin = RequestOrigin::parse("https://auth.openai.com/oauth/token").expect("valid URL");
|
||||
let result = configure_env_proxy_handling(
|
||||
&env,
|
||||
let route = resolve_env_proxy_route(&env, EnvProxyKind::Https);
|
||||
let result = configure_builder_for_resolved_route(
|
||||
reqwest::Client::builder(),
|
||||
Some(&origin),
|
||||
ClientRouteClass::Auth,
|
||||
&route,
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
@@ -95,6 +103,95 @@ fn environment_fallback_reads_injected_proxy_environment() {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unavailable_system_route_resolves_environment_or_direct_explicitly() {
|
||||
let env = MapEnv {
|
||||
values: HashMap::from([
|
||||
(
|
||||
"HTTPS_PROXY".to_string(),
|
||||
"http://proxy.example:8080".to_string(),
|
||||
),
|
||||
("NO_PROXY".to_string(), "localhost,.internal".to_string()),
|
||||
]),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
route_from_system_decision(
|
||||
&env,
|
||||
EnvProxyKind::Https,
|
||||
SystemProxyDecision::Unavailable {
|
||||
failure: RouteFailureClass::ProxyResolutionUnavailable,
|
||||
},
|
||||
),
|
||||
OutboundProxyRoute::Proxy {
|
||||
url: "http://proxy.example:8080".to_string(),
|
||||
no_proxy: Some("localhost,.internal".to_string()),
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
route_from_system_decision(
|
||||
&MapEnv {
|
||||
values: HashMap::new(),
|
||||
},
|
||||
EnvProxyKind::Https,
|
||||
SystemProxyDecision::Unavailable {
|
||||
failure: RouteFailureClass::ProxyResolutionUnavailable,
|
||||
},
|
||||
),
|
||||
OutboundProxyRoute::Direct
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unavailable_system_route_preserves_wss_http_proxy_fallback() {
|
||||
let env = MapEnv {
|
||||
values: HashMap::from([(
|
||||
"HTTP_PROXY".to_string(),
|
||||
"http://proxy.example:8080".to_string(),
|
||||
)]),
|
||||
};
|
||||
|
||||
let route = resolve_proxy_route(
|
||||
&env,
|
||||
"wss://api.openai.com/v1/responses",
|
||||
OutboundProxyPolicy::RespectSystemProxy,
|
||||
|_, _| SystemProxyDecision::Unavailable {
|
||||
failure: RouteFailureClass::ProxyResolutionUnavailable,
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
route,
|
||||
OutboundProxyRoute::Proxy {
|
||||
url: "http://proxy.example:8080".to_string(),
|
||||
no_proxy: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "windows", target_os = "macos"))]
|
||||
#[tokio::test]
|
||||
async fn async_resolution_uses_cached_route_before_global_permit() {
|
||||
let request_url = "https://cached-fast-path.test/request";
|
||||
cache_system_proxy_decision(request_url, SystemProxyDecision::Direct);
|
||||
let factory = HttpClientFactory::new(OutboundProxyPolicy::RespectSystemProxy);
|
||||
let permit = ASYNC_SYSTEM_PROXY_RESOLUTION_PERMIT
|
||||
.acquire()
|
||||
.await
|
||||
.expect("global proxy permit should stay open");
|
||||
|
||||
let route = tokio::time::timeout(
|
||||
Duration::from_secs(2),
|
||||
factory.resolve_proxy_route_async(request_url.to_string()),
|
||||
)
|
||||
.await
|
||||
.expect("cached resolution should not wait for the global permit")
|
||||
.expect("cached route should resolve");
|
||||
drop(permit);
|
||||
|
||||
assert_eq!(route, OutboundProxyRoute::Direct);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn enabled_environment_proxy_routes_request_through_proxy() {
|
||||
let listener =
|
||||
@@ -282,6 +379,5 @@ fn system_proxy_cache_key_preserves_url_specific_pac_decisions() {
|
||||
cache_key,
|
||||
system_proxy_cache_key("https://auth.openai.com/oauth/revoke")
|
||||
);
|
||||
#[cfg(any(target_os = "windows", target_os = "macos"))]
|
||||
assert!(!cache_key.contains(request_url));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user