mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
PAC 2 - Add shared auth system proxy contract (#26707)
## Summary Stacked on #26706. Adds the shared auth/system-proxy contract that later platform resolver PRs plug into. This PR moves Codex-owned auth and startup HTTP clients through a common route-aware boundary, but does not yet add Windows or macOS system proxy resolution. The default path remains unchanged when `respect_system_proxy` is absent or disabled. ## Implementation - Adds `codex-client/src/outbound_proxy.rs` with the shared route-selection model: - `OutboundProxyConfig`; - `ClientRouteClass`; - `RouteFailureClass`; - `build_reqwest_client_for_route`. - Preserves the existing reqwest/default-client behavior when no route config is supplied. - Uses the fixed MVP routing policy when route config is supplied: platform system/PAC/WPAD discovery, then explicit env proxy variables, then direct connection. - Keeps platform-specific system discovery behind the shared client boundary. This PR provides the contract and fallback behavior; later resolver PRs plug in Windows and macOS discovery. - Adds `login::AuthRouteConfig` so auth call sites depend on a small policy type instead of platform resolver details. - Maps the resolved `Config.respect_system_proxy` boolean into `AuthRouteConfig` for auth-owned clients. - Wires the route config through browser login, device-code login, access-token login, login status, logout/revoke, token refresh, API-key exchange, app-server account login, TUI/app startup, cloud-config bootstrap, cloud tasks, plugin auth, and exec startup config loading. ## End-user behavior - No behavior changes by default. - When `respect_system_proxy = true`, auth-owned clients opt into the shared route-aware client path. - On platforms without a resolver implementation in this PR, system discovery is unavailable and the route-aware path falls back to explicit env proxy handling, then direct connection. - Custom CA handling remains separate from proxy route selection and still runs through the shared client builder. - No proxy URLs, PAC contents, or resolved platform details are exposed through the public config surface introduced here. ## Tests Adds or updates coverage for: - preserving default auth-client fallback behavior when no route config is provided; - injected environment-proxy fallback without mutating process environment; - existing login-server E2E flows using explicit `auth_route_config: None` to guard unchanged default behavior; - updated auth manager, login, logout, cloud-config, startup, and plugin-auth call sites passing route config explicitly.
This commit is contained in:
@@ -3,6 +3,7 @@ mod chatgpt_hosts;
|
||||
mod custom_ca;
|
||||
mod default_client;
|
||||
mod error;
|
||||
mod outbound_proxy;
|
||||
mod request;
|
||||
mod retry;
|
||||
mod sse;
|
||||
@@ -25,6 +26,11 @@ pub use crate::default_client::CodexHttpClient;
|
||||
pub use crate::default_client::CodexRequestBuilder;
|
||||
pub use crate::error::StreamError;
|
||||
pub use crate::error::TransportError;
|
||||
pub use crate::outbound_proxy::BuildRouteAwareHttpClientError;
|
||||
pub use crate::outbound_proxy::ClientRouteClass;
|
||||
pub use crate::outbound_proxy::OutboundProxyConfig;
|
||||
pub use crate::outbound_proxy::RouteFailureClass;
|
||||
pub use crate::outbound_proxy::build_reqwest_client_for_route;
|
||||
pub use crate::request::EncodedJsonBody;
|
||||
pub use crate::request::PreparedRequestBody;
|
||||
pub use crate::request::Request;
|
||||
|
||||
@@ -0,0 +1,322 @@
|
||||
//! Conservative outbound proxy selection for resolver-aware clients.
|
||||
//!
|
||||
//! When enabled, platform system discovery is tried first, explicit environment
|
||||
//! proxies are the fallback, and the final fallback is a direct connection.
|
||||
//! When disabled, callers retain the existing reqwest builder behavior.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::fmt;
|
||||
use std::io;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::OnceLock;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use crate::custom_ca::BuildCustomCaTransportError;
|
||||
use crate::custom_ca::build_reqwest_client_with_custom_ca;
|
||||
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;
|
||||
|
||||
/// Coarse semantic bucket for the HTTP or WebSocket client being constructed.
|
||||
///
|
||||
/// This is not the selected proxy route or a concrete endpoint. It labels the
|
||||
/// product path that owns the client so proxy-resolution diagnostics can
|
||||
/// distinguish auth, API, WebSocket, and miscellaneous traffic without exposing
|
||||
/// endpoint details.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum ClientRouteClass {
|
||||
/// Login, token refresh/revoke, PAT, and agent identity auth traffic.
|
||||
Auth,
|
||||
/// First-party API traffic that is not part of the auth flow.
|
||||
Api,
|
||||
/// WebSocket traffic.
|
||||
WebSocket,
|
||||
/// Call sites without a more specific route class.
|
||||
Other,
|
||||
}
|
||||
|
||||
impl fmt::Display for ClientRouteClass {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.write_str(match self {
|
||||
Self::Auth => "auth",
|
||||
Self::Api => "api",
|
||||
Self::WebSocket => "wss",
|
||||
Self::Other => "other",
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// Coarse failure class for route selection errors.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum RouteFailureClass {
|
||||
ProxyResolutionUnavailable,
|
||||
ConnectTimeout,
|
||||
ProxyAuthenticationRequired,
|
||||
TlsError,
|
||||
InvalidProxyConfig,
|
||||
UnsupportedProxyScheme,
|
||||
ResolverError,
|
||||
}
|
||||
|
||||
impl fmt::Display for RouteFailureClass {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.write_str(match self {
|
||||
Self::ProxyResolutionUnavailable => "proxy_resolution_unavailable",
|
||||
Self::ConnectTimeout => "connect_timeout",
|
||||
Self::ProxyAuthenticationRequired => "proxy_407",
|
||||
Self::TlsError => "tls_error",
|
||||
Self::InvalidProxyConfig => "invalid_proxy_config",
|
||||
Self::UnsupportedProxyScheme => "unsupported_proxy_scheme",
|
||||
Self::ResolverError => "resolver_error",
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// Marker enabling fixed system/PAC/WPAD, environment, then direct routing.
|
||||
/// Resolved endpoints and platform details remain internal to the client builder.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct OutboundProxyConfig;
|
||||
|
||||
impl OutboundProxyConfig {
|
||||
pub const fn respect_system_proxy() -> Self {
|
||||
Self
|
||||
}
|
||||
}
|
||||
|
||||
/// Error while building a resolver-aware reqwest client.
|
||||
#[derive(Debug, Error)]
|
||||
pub enum BuildRouteAwareHttpClientError {
|
||||
#[error(transparent)]
|
||||
CustomCa(#[from] BuildCustomCaTransportError),
|
||||
|
||||
#[error("Failed to configure outbound proxy selected for {route_class}")]
|
||||
InvalidProxyConfig { route_class: ClientRouteClass },
|
||||
}
|
||||
|
||||
impl From<BuildRouteAwareHttpClientError> for io::Error {
|
||||
fn from(error: BuildRouteAwareHttpClientError) -> Self {
|
||||
match error {
|
||||
BuildRouteAwareHttpClientError::CustomCa(error) => error.into(),
|
||||
BuildRouteAwareHttpClientError::InvalidProxyConfig { .. } => io::Error::other(error),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Builds a reqwest client with conservative route selection and shared CA handling.
|
||||
///
|
||||
/// Unavailable platform resolution falls back to environment proxies and then direct. Errors after
|
||||
/// a route is selected are returned without trying another route.
|
||||
pub fn build_reqwest_client_for_route(
|
||||
builder: reqwest::ClientBuilder,
|
||||
request_url: &str,
|
||||
route_class: ClientRouteClass,
|
||||
config: Option<&OutboundProxyConfig>,
|
||||
) -> Result<reqwest::Client, BuildRouteAwareHttpClientError> {
|
||||
let builder =
|
||||
configure_proxy_for_route(&ProcessEnv, builder, request_url, route_class, config)?;
|
||||
build_reqwest_client_with_custom_ca(builder).map_err(Into::into)
|
||||
}
|
||||
|
||||
fn configure_proxy_for_route(
|
||||
env: &dyn EnvSource,
|
||||
builder: reqwest::ClientBuilder,
|
||||
request_url: &str,
|
||||
route_class: ClientRouteClass,
|
||||
config: Option<&OutboundProxyConfig>,
|
||||
) -> Result<reqwest::ClientBuilder, BuildRouteAwareHttpClientError> {
|
||||
if config.is_none() {
|
||||
return Ok(builder);
|
||||
}
|
||||
let origin = RequestOrigin::parse(request_url);
|
||||
|
||||
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_concrete_proxy(
|
||||
builder: reqwest::ClientBuilder,
|
||||
route_class: ClientRouteClass,
|
||||
proxy_url: &str,
|
||||
no_proxy: Option<reqwest::NoProxy>,
|
||||
) -> Result<reqwest::ClientBuilder, BuildRouteAwareHttpClientError> {
|
||||
let proxy = match reqwest::Proxy::all(proxy_url) {
|
||||
Ok(proxy) => proxy,
|
||||
Err(_source) => {
|
||||
return Err(BuildRouteAwareHttpClientError::InvalidProxyConfig { route_class });
|
||||
}
|
||||
};
|
||||
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 {
|
||||
scheme: String,
|
||||
host: String,
|
||||
port: u16,
|
||||
}
|
||||
|
||||
impl RequestOrigin {
|
||||
fn parse(request_url: &str) -> Option<Self> {
|
||||
let uri = request_url.parse::<http::Uri>().ok()?;
|
||||
let scheme = uri.scheme_str()?.to_ascii_lowercase();
|
||||
let host = uri.host()?.trim_matches(['[', ']']).to_ascii_lowercase();
|
||||
let port = uri.port_u16().or(match scheme.as_str() {
|
||||
"http" => Some(80),
|
||||
"https" => Some(443),
|
||||
_ => None,
|
||||
})?;
|
||||
Some(Self { scheme, host, port })
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
#[allow(
|
||||
dead_code,
|
||||
reason = "Direct and Proxy are constructed by platform resolvers added in later PRs"
|
||||
)]
|
||||
enum SystemProxyDecision {
|
||||
Direct,
|
||||
Proxy { url: String },
|
||||
Unavailable { failure: RouteFailureClass },
|
||||
}
|
||||
|
||||
fn resolve_system_proxy(request_url: &str, origin: &RequestOrigin) -> SystemProxyDecision {
|
||||
if let Some(decision) = cached_system_proxy_decision(request_url) {
|
||||
return decision;
|
||||
}
|
||||
|
||||
let decision = resolve_platform_system_proxy(request_url, origin);
|
||||
cache_system_proxy_decision(request_url, decision.clone());
|
||||
decision
|
||||
}
|
||||
|
||||
fn resolve_platform_system_proxy(
|
||||
_request_url: &str,
|
||||
_origin: &RequestOrigin,
|
||||
) -> SystemProxyDecision {
|
||||
SystemProxyDecision::Unavailable {
|
||||
failure: RouteFailureClass::ProxyResolutionUnavailable,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
struct CachedSystemProxyDecision {
|
||||
decision: SystemProxyDecision,
|
||||
expires_at: Instant,
|
||||
}
|
||||
|
||||
static SYSTEM_PROXY_CACHE: OnceLock<Mutex<HashMap<String, CachedSystemProxyDecision>>> =
|
||||
OnceLock::new();
|
||||
|
||||
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()?;
|
||||
let cached = cache.get(request_url)?;
|
||||
if cached.expires_at > Instant::now() {
|
||||
return Some(cached.decision.clone());
|
||||
}
|
||||
cache.remove(request_url);
|
||||
None
|
||||
}
|
||||
|
||||
fn cache_system_proxy_decision(request_url: &str, decision: SystemProxyDecision) {
|
||||
let cache = SYSTEM_PROXY_CACHE.get_or_init(|| Mutex::new(HashMap::new()));
|
||||
if let Ok(mut cache) = cache.lock() {
|
||||
insert_system_proxy_cache_entry(&mut cache, request_url, decision, Instant::now());
|
||||
}
|
||||
}
|
||||
|
||||
fn insert_system_proxy_cache_entry(
|
||||
cache: &mut HashMap<String, CachedSystemProxyDecision>,
|
||||
request_url: &str,
|
||||
decision: SystemProxyDecision,
|
||||
now: Instant,
|
||||
) {
|
||||
let ttl = match &decision {
|
||||
SystemProxyDecision::Direct | SystemProxyDecision::Proxy { .. } => {
|
||||
SYSTEM_PROXY_SUCCESS_CACHE_TTL
|
||||
}
|
||||
SystemProxyDecision::Unavailable { .. } => SYSTEM_PROXY_UNAVAILABLE_CACHE_TTL,
|
||||
};
|
||||
|
||||
cache.retain(|_, cached| cached.expires_at > now);
|
||||
if cache.len() >= SYSTEM_PROXY_CACHE_MAX_ENTRIES
|
||||
&& !cache.contains_key(request_url)
|
||||
&& let Some(request_url_to_evict) = cache
|
||||
.iter()
|
||||
.min_by_key(|(_, cached)| cached.expires_at)
|
||||
.map(|(request_url, _)| request_url.clone())
|
||||
{
|
||||
cache.remove(&request_url_to_evict);
|
||||
}
|
||||
cache.insert(
|
||||
request_url.to_string(),
|
||||
CachedSystemProxyDecision {
|
||||
decision,
|
||||
expires_at: now + ttl,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
trait EnvSource {
|
||||
fn var(&self, key: &str) -> Option<String>;
|
||||
}
|
||||
|
||||
struct ProcessEnv;
|
||||
|
||||
impl EnvSource for ProcessEnv {
|
||||
fn var(&self, key: &str) -> Option<String> {
|
||||
std::env::var(key).ok()
|
||||
}
|
||||
}
|
||||
|
||||
fn proxy_env_value(env: &dyn EnvSource, upper: &str) -> Option<String> {
|
||||
let lower = upper.to_ascii_lowercase();
|
||||
env.var(upper)
|
||||
.or_else(|| env.var(&lower))
|
||||
.filter(|value| !value.is_empty())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "outbound_proxy_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,133 @@
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::io::Read;
|
||||
use std::io::Write;
|
||||
|
||||
struct MapEnv {
|
||||
values: HashMap<String, String>,
|
||||
}
|
||||
|
||||
impl EnvSource for MapEnv {
|
||||
fn var(&self, key: &str) -> Option<String> {
|
||||
self.values.get(key).cloned()
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn proxy_env_value_matches_reqwest_casing_precedence() {
|
||||
let env = MapEnv {
|
||||
values: HashMap::from([
|
||||
("HTTPS_PROXY".to_string(), "upper".to_string()),
|
||||
("https_proxy".to_string(), "lower".to_string()),
|
||||
("http_proxy".to_string(), "lower-only".to_string()),
|
||||
("ALL_PROXY".to_string(), String::new()),
|
||||
("all_proxy".to_string(), "masked".to_string()),
|
||||
]),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
proxy_env_value(&env, "HTTPS_PROXY"),
|
||||
Some("upper".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
proxy_env_value(&env, "HTTP_PROXY"),
|
||||
Some("lower-only".to_string())
|
||||
);
|
||||
assert_eq!(proxy_env_value(&env, "ALL_PROXY"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
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,
|
||||
reqwest::Client::builder(),
|
||||
Some(&origin),
|
||||
ClientRouteClass::Auth,
|
||||
);
|
||||
|
||||
assert!(matches!(
|
||||
result,
|
||||
Err(BuildRouteAwareHttpClientError::InvalidProxyConfig {
|
||||
route_class: ClientRouteClass::Auth,
|
||||
})
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn enabled_environment_proxy_routes_request_through_proxy() {
|
||||
let listener =
|
||||
std::net::TcpListener::bind(("127.0.0.1", 0)).expect("local proxy listener should bind");
|
||||
let proxy_addr = listener
|
||||
.local_addr()
|
||||
.expect("local proxy listener should have an address");
|
||||
let proxy_thread = std::thread::spawn(move || {
|
||||
let (mut stream, _) = listener.accept().expect("proxy should accept a request");
|
||||
let mut buffer = [0_u8; 4096];
|
||||
let size = stream.read(&mut buffer).expect("proxy should read request");
|
||||
stream
|
||||
.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok")
|
||||
.expect("proxy should write response");
|
||||
String::from_utf8_lossy(&buffer[..size]).into_owned()
|
||||
});
|
||||
let env = MapEnv {
|
||||
values: HashMap::from([("HTTP_PROXY".to_string(), format!("http://{proxy_addr}"))]),
|
||||
};
|
||||
let request_url = "http://enabled-proxy.test/proxy-check";
|
||||
let config = OutboundProxyConfig::respect_system_proxy();
|
||||
let builder = configure_proxy_for_route(
|
||||
&env,
|
||||
reqwest::Client::builder().timeout(Duration::from_secs(2)),
|
||||
request_url,
|
||||
ClientRouteClass::Auth,
|
||||
Some(&config),
|
||||
)
|
||||
.expect("enabled proxy route should configure");
|
||||
|
||||
let response = builder
|
||||
.build()
|
||||
.expect("proxy client should build")
|
||||
.get(request_url)
|
||||
.send()
|
||||
.await
|
||||
.expect("request should use local proxy");
|
||||
let proxy_request = proxy_thread.join().expect("proxy thread should finish");
|
||||
|
||||
assert_eq!(response.status(), reqwest::StatusCode::OK);
|
||||
assert_eq!(
|
||||
proxy_request.lines().next(),
|
||||
Some("GET http://enabled-proxy.test/proxy-check HTTP/1.1")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unavailable_system_proxy_decision_is_cached() {
|
||||
let request_url = "https://unavailable-cache.test/oauth/token";
|
||||
let decision = SystemProxyDecision::Unavailable {
|
||||
failure: RouteFailureClass::ProxyResolutionUnavailable,
|
||||
};
|
||||
|
||||
cache_system_proxy_decision(request_url, decision.clone());
|
||||
|
||||
assert_eq!(cached_system_proxy_decision(request_url), Some(decision));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn system_proxy_cache_is_bounded() {
|
||||
let mut cache = HashMap::new();
|
||||
let now = Instant::now();
|
||||
|
||||
for index in 0..=SYSTEM_PROXY_CACHE_MAX_ENTRIES {
|
||||
insert_system_proxy_cache_entry(
|
||||
&mut cache,
|
||||
&format!("https://bounded-cache.test/{index}"),
|
||||
SystemProxyDecision::Direct,
|
||||
now,
|
||||
);
|
||||
}
|
||||
|
||||
assert_eq!(cache.len(), SYSTEM_PROXY_CACHE_MAX_ENTRIES);
|
||||
}
|
||||
Reference in New Issue
Block a user