From 539c0e1100512bbdaaeafdfa3dafc1c8954eb288 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 22 Jul 2026 00:45:41 +0000 Subject: [PATCH] Migrate login HTTP construction to `HttpClient` (#34643) ## Why `codex-http-client` is the intended owner of direct `reqwest` usage. ## What changed - Build Codex's default and route-aware clients with `HttpClientBuilder`, and return `HttpClient` directly to API, model discovery, auth, remote control, skills, and extension callers. - Preserve custom CA, proxy, Cloudflare cookie, sandbox, and request-logging policies while removing `codex-login`'s direct `reqwest` dependency. - Use shared HTTP types for auth headers, status codes, transport errors, and login-server tests. ## Testing - Add a Linux integration test verifying that a regular Responses API turn traverses the configured system proxy. GitOrigin-RevId: 714695fb80762d2083e2f7a6414183e2b3b63854 --- codex-rs/Cargo.lock | 1 - .../src/transport/remote_control/clients.rs | 4 +- .../src/transport/remote_control/enroll.rs | 6 +- .../transport/remote_control/server_api.rs | 4 +- codex-rs/cli/src/doctor.rs | 6 +- codex-rs/core-skills/src/remote.rs | 6 +- codex-rs/core/src/client.rs | 6 +- codex-rs/core/tests/suite/mod.rs | 2 + .../tests/suite/responses_system_proxy.rs | 120 ++++++++++++++++++ codex-rs/deny.toml | 1 - codex-rs/ext/image-generation/src/backend.rs | 4 +- codex-rs/ext/web-search/src/tool.rs | 4 +- codex-rs/login/Cargo.toml | 1 - codex-rs/login/src/auth/auth_headers.rs | 2 +- codex-rs/login/src/auth/auth_tests.rs | 11 +- codex-rs/login/src/auth/default_client.rs | 112 ++++++---------- codex-rs/login/src/auth/manager.rs | 2 +- codex-rs/login/src/auth/revoke.rs | 4 +- codex-rs/login/src/server.rs | 6 +- .../login/tests/suite/login_server_e2e.rs | 23 ++-- .../model-provider/src/models_endpoint.rs | 16 +-- 21 files changed, 210 insertions(+), 131 deletions(-) create mode 100644 codex-rs/core/tests/suite/responses_system_proxy.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 41e7822f2b..f3e84a90ca 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3343,7 +3343,6 @@ dependencies = [ "pretty_assertions", "rand 0.9.3", "regex-lite", - "reqwest 0.12.28", "serde", "serde_json", "serial_test", diff --git a/codex-rs/app-server-transport/src/transport/remote_control/clients.rs b/codex-rs/app-server-transport/src/transport/remote_control/clients.rs index 121a7685dd..92050049a4 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/clients.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/clients.rs @@ -12,7 +12,7 @@ use codex_app_server_protocol::RemoteControlClientsListResponse; use codex_app_server_protocol::RemoteControlClientsRevokeParams; use codex_app_server_protocol::RemoteControlClientsRevokeResponse; use codex_login::AuthManager; -use codex_login::default_client::build_reqwest_client; +use codex_login::default_client::create_client_without_request_logging; use serde::Deserialize; use std::io; use std::io::ErrorKind; @@ -186,7 +186,7 @@ async fn send_client_management_request_once( request: &ClientManagementRequest<'_>, action: &str, ) -> io::Result { - let client = build_reqwest_client(); + let client = create_client_without_request_logging(); let auth_headers = auth.request_headers()?; let request = match request { ClientManagementRequest::List { url, params } => { diff --git a/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs b/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs index be64072476..c991913781 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs @@ -7,7 +7,7 @@ use super::protocol::StartRemoteControlPairingResponse; use axum::http::HeaderMap; use codex_app_server_protocol::RemoteControlPairingStartResponse; use codex_app_server_protocol::RemoteControlPairingStatusResponse; -use codex_login::default_client::build_reqwest_client; +use codex_login::default_client::create_client_without_request_logging; use codex_state::RemoteControlEnrollmentRecord; use codex_state::StateRuntime; use std::io; @@ -59,7 +59,7 @@ impl RemoteControlEnrollment { .as_deref() .ok_or_else(pairing_unavailable_error)?; - let response = build_reqwest_client() + let response = create_client_without_request_logging() .post(&self.remote_control_target.pair_url) .timeout(REMOTE_CONTROL_PAIRING_TIMEOUT) .bearer_auth(remote_control_token) @@ -154,7 +154,7 @@ impl RemoteControlEnrollment { .as_deref() .ok_or_else(pairing_unavailable_error)?; - let response = build_reqwest_client() + let response = create_client_without_request_logging() .post(&self.remote_control_target.pair_status_url) .timeout(REMOTE_CONTROL_PAIRING_TIMEOUT) .bearer_auth(remote_control_token) diff --git a/codex-rs/app-server-transport/src/transport/remote_control/server_api.rs b/codex-rs/app-server-transport/src/transport/remote_control/server_api.rs index def38907c7..fcd4ae9a42 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/server_api.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/server_api.rs @@ -9,7 +9,7 @@ use super::protocol::RefreshRemoteServerRequest; use super::protocol::RemoteControlTarget; use axum::http::HeaderMap; use axum::http::StatusCode; -use codex_login::default_client::build_reqwest_client; +use codex_login::default_client::create_client_without_request_logging; use rand::Rng; use serde::Serialize; use serde::de::DeserializeOwned; @@ -223,7 +223,7 @@ where Request: Serialize, Response: DeserializeOwned, { - let client = build_reqwest_client(); + let client = create_client_without_request_logging(); let auth_headers = auth.request_headers()?; let response = client .post(url) diff --git a/codex-rs/cli/src/doctor.rs b/codex-rs/cli/src/doctor.rs index 7f06a611bc..ed93f43ed5 100644 --- a/codex-rs/cli/src/doctor.rs +++ b/codex-rs/cli/src/doctor.rs @@ -49,7 +49,7 @@ use codex_login::CODEX_ACCESS_TOKEN_ENV_VAR; use codex_login::CODEX_API_KEY_ENV_VAR; use codex_login::CodexAuth; use codex_login::OPENAI_API_KEY_ENV_VAR; -use codex_login::default_client::build_reqwest_client; +use codex_login::default_client::create_client_without_request_logging; use codex_login::default_client::default_headers; use codex_login::load_auth_dot_json; use codex_model_provider::create_model_provider; @@ -2885,7 +2885,7 @@ async fn mcp_http_probe_url_with_timeout(url: &str, timeout: Duration) -> Result } async fn http_probe_url_with_timeout(url: &str, timeout: Duration) -> Result { - let response = build_reqwest_client() + let response = create_client_without_request_logging() .head(url) .timeout(timeout) .send() @@ -2911,7 +2911,7 @@ async fn http_get_probe_url_with_timeout(url: &str, timeout: Duration) -> Result } async fn http_get_probe_status_with_timeout(url: &str, timeout: Duration) -> Result { - let response = build_reqwest_client() + let response = create_client_without_request_logging() .get(url) .timeout(timeout) .send() diff --git a/codex-rs/core-skills/src/remote.rs b/codex-rs/core-skills/src/remote.rs index 1ca7cd0cb7..b7e27e34f7 100644 --- a/codex-rs/core-skills/src/remote.rs +++ b/codex-rs/core-skills/src/remote.rs @@ -7,7 +7,7 @@ use std::path::PathBuf; use std::time::Duration; use codex_login::CodexAuth; -use codex_login::default_client::build_reqwest_client; +use codex_login::default_client::create_client_without_request_logging; const REMOTE_SKILLS_API_TIMEOUT: Duration = Duration::from_secs(30); @@ -107,7 +107,7 @@ pub async fn list_remote_skills( query_params.push(("enabled", enabled)); } - let client = build_reqwest_client(); + let client = create_client_without_request_logging(); let request = client .get(&url) .timeout(REMOTE_SKILLS_API_TIMEOUT) @@ -146,7 +146,7 @@ pub async fn export_remote_skill( ) -> Result { let auth = ensure_codex_backend_auth(auth)?; - let client = build_reqwest_client(); + let client = create_client_without_request_logging(); let base_url = chatgpt_base_url.trim_end_matches('/'); let url = format!("{base_url}/hazelnuts/{skill_id}/export"); let request = client diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 9b0592d35a..7dde771cf7 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -70,7 +70,7 @@ use codex_login::CodexAuth; use codex_login::RefreshTokenError; use codex_login::UnauthorizedRecovery; use codex_login::default_client::add_originator_header; -use codex_login::default_client::build_default_reqwest_client_for_route; +use codex_login::default_client::create_client_for_route; use codex_otel::SessionTelemetry; use codex_otel::current_span_w3c_trace_context; use codex_protocol::auth::AuthMode; @@ -967,13 +967,13 @@ impl ModelClient { endpoint: &str, ) -> Result { let request_url = api_provider.url_for_path(endpoint); - let client = build_default_reqwest_client_for_route( + let client = create_client_for_route( &self.http_client_factory, &request_url, ClientRouteClass::Api, ) .map_err(std::io::Error::from)?; - Ok(ReqwestTransport::new(client)) + Ok(ReqwestTransport::from_http_client(client)) } pub(crate) async fn prewarm_auth(&self) -> Result<()> { diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index e7b9cea4b6..aeb570c032 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -116,6 +116,8 @@ mod request_plugin_install; mod request_user_input; mod responses_api_proxy_headers; mod responses_lite; +#[cfg(target_os = "linux")] +mod responses_system_proxy; mod resume; mod resume_warning; mod review; diff --git a/codex-rs/core/tests/suite/responses_system_proxy.rs b/codex-rs/core/tests/suite/responses_system_proxy.rs new file mode 100644 index 0000000000..daa385d09b --- /dev/null +++ b/codex-rs/core/tests/suite/responses_system_proxy.rs @@ -0,0 +1,120 @@ +use anyhow::Result; +use codex_features::Feature; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::sse; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use pretty_assertions::assert_eq; +use std::time::Duration; +use tokio::io::AsyncReadExt; +use tokio::io::AsyncWriteExt; +use tokio::net::TcpListener; +use tokio::process::Command; +use wiremock::MockServer; + +const SYSTEM_PROXY_TEST_SUBPROCESS_ENV_VAR: &str = "CODEX_SYSTEM_PROXY_TEST_SUBPROCESS"; +const TEST_NAME: &str = + "suite::responses_system_proxy::regular_responses_turn_honors_respect_system_proxy"; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn regular_responses_turn_honors_respect_system_proxy() -> Result<()> { + skip_if_no_network!(Ok(())); + + if std::env::var_os(SYSTEM_PROXY_TEST_SUBPROCESS_ENV_VAR).is_none() { + let proxy = TcpListener::bind("127.0.0.1:0").await?; + let proxy_url = format!("http://{}", proxy.local_addr()?); + let response_body = sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]); + let proxy_task = tokio::spawn(async move { + let (mut stream, _) = proxy.accept().await?; + let mut request = Vec::new(); + let mut chunk = [0_u8; 1024]; + let header_end = loop { + let bytes_read = stream.read(&mut chunk).await?; + if bytes_read == 0 { + anyhow::bail!("proxy client closed before sending request headers"); + } + request.extend_from_slice(&chunk[..bytes_read]); + if request.len() > 64 * 1024 { + anyhow::bail!("proxy request headers exceeded 64 KiB"); + } + if let Some(header_end) = + request.windows(4).position(|window| window == b"\r\n\r\n") + { + break header_end + 4; + } + }; + let headers = String::from_utf8_lossy(&request[..header_end]).into_owned(); + let content_length = headers + .lines() + .filter_map(|line| line.split_once(':')) + .find_map(|(name, value)| { + name.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse::().ok()) + .flatten() + }) + .unwrap_or_default(); + while request.len() < header_end + content_length { + let bytes_read = stream.read(&mut chunk).await?; + if bytes_read == 0 { + anyhow::bail!("proxy client closed before sending the request body"); + } + request.extend_from_slice(&chunk[..bytes_read]); + } + + let response = format!( + "HTTP/1.1 200 OK\r\ncontent-type: text/event-stream\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{response_body}", + response_body.len() + ); + stream.write_all(response.as_bytes()).await?; + stream.shutdown().await?; + Ok::<_, anyhow::Error>(headers.lines().next().unwrap_or_default().to_string()) + }); + + let mut command = Command::new(std::env::current_exe()?); + command.arg("--exact").arg(TEST_NAME); + for &key in codex_network_proxy::PROXY_ENV_KEYS { + command.env_remove(key); + } + // Keep the test harness's loopback HTTP and WebSocket traffic out of the proxy. The fake + // API origin is not covered, so the inference request must still traverse the proxy. + command + .env(SYSTEM_PROXY_TEST_SUBPROCESS_ENV_VAR, "1") + .env("HTTP_PROXY", &proxy_url) + .env("http_proxy", proxy_url) + .env("NO_PROXY", codex_network_proxy::DEFAULT_NO_PROXY_VALUE) + .env("no_proxy", codex_network_proxy::DEFAULT_NO_PROXY_VALUE); + + let output = command.output().await?; + if !output.status.success() { + proxy_task.abort(); + } + assert!( + output.status.success(), + "subprocess test `{TEST_NAME}` failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + let proxy_request = tokio::time::timeout(Duration::from_secs(1), proxy_task).await???; + assert_eq!( + proxy_request, + "POST http://responses-proxy.invalid/v1/responses HTTP/1.1" + ); + return Ok(()); + } + + let server = MockServer::start().await; + let mut builder = test_codex().with_config(|config| { + config.model_provider.base_url = Some("http://responses-proxy.invalid/v1".to_string()); + config + .features + .enable(Feature::RespectSystemProxy) + .expect("test config should allow feature update"); + config.respect_system_proxy = true; + }); + let test = builder.build_with_auto_env(&server).await?; + + test.submit_turn("hello through the system proxy").await?; + + Ok(()) +} diff --git a/codex-rs/deny.toml b/codex-rs/deny.toml index 633bad28e8..94ca4d28a8 100644 --- a/codex-rs/deny.toml +++ b/codex-rs/deny.toml @@ -244,7 +244,6 @@ deny = [ "codex-core", "codex-exec-server", "codex-lmstudio", - "codex-login", "codex-ollama", "codex-otel", "codex-protocol", diff --git a/codex-rs/ext/image-generation/src/backend.rs b/codex-rs/ext/image-generation/src/backend.rs index 58df602426..761bb1f27d 100644 --- a/codex-rs/ext/image-generation/src/backend.rs +++ b/codex-rs/ext/image-generation/src/backend.rs @@ -4,7 +4,7 @@ use codex_api::ImageResponse; use codex_api::ImagesClient; use codex_api::ReqwestTransport; use codex_login::default_client::add_originator_header; -use codex_login::default_client::build_reqwest_client; +use codex_login::default_client::create_client; use codex_model_provider::SharedModelProvider; use http::HeaderMap; @@ -36,7 +36,7 @@ impl CodexImagesBackend { .await .map_err(|err| err.to_string())?; Ok(ImagesClient::new( - ReqwestTransport::new(build_reqwest_client()), + ReqwestTransport::from_http_client(create_client()), provider, auth, )) diff --git a/codex-rs/ext/web-search/src/tool.rs b/codex-rs/ext/web-search/src/tool.rs index 9b7924b09d..9747b75edd 100644 --- a/codex-rs/ext/web-search/src/tool.rs +++ b/codex-rs/ext/web-search/src/tool.rs @@ -19,7 +19,7 @@ use codex_extension_items::ExtensionItem; use codex_extension_items::web_search::WebSearchAction; use codex_extension_items::web_search::WebSearchItem; use codex_login::default_client::add_originator_header; -use codex_login::default_client::build_reqwest_client; +use codex_login::default_client::create_client; use codex_model_provider::SharedModelProvider; use codex_protocol::models::WebSearchAction as CoreWebSearchAction; use codex_protocol::protocol::EventMsg; @@ -103,7 +103,7 @@ impl WebSearchTool { .await .map_err(|err| FunctionCallError::Fatal(err.to_string()))?; let client = SearchClient::new( - ReqwestTransport::new(build_reqwest_client()), + ReqwestTransport::from_http_client(create_client()), provider, auth, ); diff --git a/codex-rs/login/Cargo.toml b/codex-rs/login/Cargo.toml index f50648c3af..424e7383b3 100644 --- a/codex-rs/login/Cargo.toml +++ b/codex-rs/login/Cargo.toml @@ -24,7 +24,6 @@ http = { workspace = true } once_cell = { workspace = true } os_info = { workspace = true } rand = { workspace = true } -reqwest = { workspace = true, features = ["json", "blocking"] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } sha2 = { workspace = true } diff --git a/codex-rs/login/src/auth/auth_headers.rs b/codex-rs/login/src/auth/auth_headers.rs index 802f00c4d3..ad27cc03c8 100644 --- a/codex-rs/login/src/auth/auth_headers.rs +++ b/codex-rs/login/src/auth/auth_headers.rs @@ -1,6 +1,6 @@ use std::fmt; -use reqwest::header::HeaderMap; +use http::HeaderMap; /// Request headers returned by an external auth provider. /// diff --git a/codex-rs/login/src/auth/auth_tests.rs b/codex-rs/login/src/auth/auth_tests.rs index 780cd86009..5699421394 100644 --- a/codex-rs/login/src/auth/auth_tests.rs +++ b/codex-rs/login/src/auth/auth_tests.rs @@ -1108,15 +1108,12 @@ impl ExternalAuth for StaticExternalAuth { #[tokio::test] async fn external_auth_provider_can_install_headers() { - let mut headers = reqwest::header::HeaderMap::new(); + let mut headers = http::HeaderMap::new(); headers.insert( - reqwest::header::AUTHORIZATION, - reqwest::header::HeaderValue::from_static("Bearer external"), - ); - headers.insert( - "x-external-auth", - reqwest::header::HeaderValue::from_static("enabled"), + http::header::AUTHORIZATION, + http::HeaderValue::from_static("Bearer external"), ); + headers.insert("x-external-auth", http::HeaderValue::from_static("enabled")); let auth = CodexAuth::Headers(AuthHeaders::new(headers)); let codex_home = tempdir().expect("tempdir"); let manager = AuthManager::new( diff --git a/codex-rs/login/src/auth/default_client.rs b/codex-rs/login/src/auth/default_client.rs index 630bcdb6e1..ef3722dad8 100644 --- a/codex-rs/login/src/auth/default_client.rs +++ b/codex-rs/login/src/auth/default_client.rs @@ -1,22 +1,20 @@ //! Default Codex HTTP client: shared `User-Agent`, `originator`, optional residency header, and -//! reqwest/`HttpClient` construction. +//! `HttpClient` construction. //! //! Use [`crate::default_client`] or [`codex_login::default_client`] from other crates in this //! workspace. -use codex_http_client::BuildCustomCaTransportError; use codex_http_client::BuildRouteAwareHttpClientError; use codex_http_client::ClientRouteClass; use codex_http_client::HttpClient; +use codex_http_client::HttpClientBuilder; use codex_http_client::HttpClientFactory; use codex_http_client::OutboundProxyPolicy; pub use codex_http_client::RequestBuilder as CodexRequestBuilder; -use codex_http_client::build_reqwest_client_with_custom_ca; -use codex_http_client::with_chatgpt_cloudflare_cookie_store; use codex_terminal_detection::user_agent; -use reqwest::header::HeaderMap; -use reqwest::header::HeaderValue; -use reqwest::header::USER_AGENT; +use http::HeaderMap; +use http::HeaderValue; +use http::header::USER_AGENT; use std::sync::LazyLock; use std::sync::Mutex; use std::sync::RwLock; @@ -218,110 +216,84 @@ fn sanitize_user_agent(candidate: String, fallback: &str) -> String { /// Create an HTTP client with default `originator` and `User-Agent` headers set. /// -/// This supported default path preserves reqwest's existing proxy behavior and does not opt into +/// This supported default path preserves the transport's existing proxy behavior and does not opt into /// Codex's route-aware system/PAC resolution. pub fn create_client() -> HttpClient { - let inner = build_reqwest_client(); - HttpClient::new(inner) + build_default_client(default_http_client_builder()) } /// Create the default HTTP client without request URL or response-header diagnostics. /// -/// This preserves the default client's legacy custom-CA fallback and reqwest proxy behavior while +/// This preserves the default client's legacy custom-CA fallback and transport proxy behavior while /// avoiding diagnostics that could expose credentials embedded in request URLs or headers. pub fn create_client_without_request_logging() -> HttpClient { - let inner = build_reqwest_client(); - HttpClient::new_without_request_logging(inner) + build_default_client(default_http_client_builder().without_request_logging()) } -/// Builds the default reqwest client used for ordinary Codex HTTP traffic. -/// -/// This starts from the standard Codex user agent, default headers, and sandbox-specific proxy -/// policy, then layers in shared custom CA handling from `CODEX_CA_CERTIFICATE` / -/// `SSL_CERT_FILE`. The function remains infallible for compatibility with existing call sites, so -/// a custom-CA or builder failure is logged and falls back to `reqwest::Client::new()`. -/// -/// This supported default path preserves reqwest's existing proxy behavior and does not opt into -/// Codex's route-aware system/PAC resolution. Auth callers with route settings must use -/// `build_default_auth_reqwest_client` or `create_default_auth_client`. -pub fn build_reqwest_client() -> reqwest::Client { - try_build_reqwest_client().unwrap_or_else(|error| { - tracing::warn!(error = %error, "failed to build default reqwest client"); - with_chatgpt_cloudflare_cookie_store(reqwest::Client::builder()) - .build() - .unwrap_or_else(|fallback_error| { - tracing::warn!( - error = %fallback_error, - "failed to build fallback reqwest client with ChatGPT Cloudflare cookie store" - ); - reqwest::Client::new() - }) - }) -} - -/// Tries to build the default reqwest client used for ordinary Codex HTTP traffic. -/// -/// Callers that need a structured CA-loading failure instead of the legacy logged fallback can use -/// this method directly. -pub fn try_build_reqwest_client() -> Result { - build_reqwest_client_with_custom_ca(default_reqwest_client_builder()) -} - -/// Builds the default Codex reqwest client for a concrete outbound route. +/// Builds the default Codex HTTP client for a concrete outbound route. /// /// When route-aware proxy handling is disabled, or the client is running inside the Codex /// sandbox, this preserves the default client's existing proxy behavior. Otherwise it resolves /// the destination through the shared system/PAC-aware routing policy. -pub fn build_default_reqwest_client_for_route( +pub fn create_client_for_route( http_client_factory: &HttpClientFactory, request_url: &str, route_class: ClientRouteClass, -) -> Result { +) -> Result { if matches!( http_client_factory.outbound_proxy_policy(), OutboundProxyPolicy::ReqwestDefault ) { - return Ok(build_reqwest_client()); + return Ok(create_client()); } if is_sandboxed() { // Preserve the sandbox's existing no-proxy policy; sandboxed command egress is routed // separately through network-proxy. - return Ok(build_reqwest_client()); + return Ok(create_client()); } - http_client_factory.build_reqwest_client( - default_reqwest_client_builder(), + default_http_client_builder().build_respecting_outbound_proxy_policy( + http_client_factory, request_url, route_class, ) } -/// Builds the default Codex reqwest client for a concrete outbound route without blocking the +/// Builds the default Codex HTTP client for a concrete outbound route without blocking the /// async runtime worker that initiated the request. -pub async fn build_default_reqwest_client_for_route_async( +pub async fn create_client_for_route_async( http_client_factory: HttpClientFactory, request_url: String, route_class: ClientRouteClass, -) -> std::io::Result { +) -> std::io::Result { let permit = ROUTE_AWARE_CLIENT_BUILD_PERMIT .acquire() .await .map_err(std::io::Error::other)?; tokio::task::spawn_blocking(move || { let _permit = permit; - build_default_reqwest_client_for_route(&http_client_factory, &request_url, route_class) + create_client_for_route(&http_client_factory, &request_url, route_class) .map_err(std::io::Error::from) }) .await .map_err(std::io::Error::other)? } -fn default_reqwest_client_builder() -> reqwest::ClientBuilder { - let mut builder = reqwest::Client::builder().default_headers(default_headers()); +fn default_http_client_builder() -> HttpClientBuilder { + HttpClientBuilder::new() + .default_headers(default_headers()) + .with_chatgpt_cloudflare_cookie_store() +} + +// These legacy constructors intentionally preserve the infallible behavior of `create_client`. +// New endpoint-aware call sites use `create_client_for_route` and propagate construction errors. +#[allow(deprecated)] +fn build_default_client(builder: HttpClientBuilder) -> HttpClient { if is_sandboxed() { - builder = builder.no_proxy(); + builder.build_direct_with_custom_ca_fallback() + } else { + builder.build_with_transport_default_proxy_and_custom_ca_fallback() } - with_chatgpt_cloudflare_cookie_store(builder) } /// Builds an HTTP client for an auth endpoint without Codex default headers. @@ -333,24 +305,16 @@ pub(crate) fn create_raw_auth_client( .build_client_without_request_logging(endpoint, ClientRouteClass::Auth) } -/// Builds the default Codex reqwest client for an auth endpoint. -pub(crate) fn build_default_auth_reqwest_client( - endpoint: &str, - auth_route_config: Option<&AuthRouteConfig>, -) -> Result { - build_default_reqwest_client_for_route( - &auth_http_client_factory(auth_route_config), - endpoint, - ClientRouteClass::Auth, - ) -} - /// Builds the default Codex HTTP client wrapper for an auth endpoint. pub(crate) fn create_default_auth_client( endpoint: &str, auth_route_config: Option<&AuthRouteConfig>, ) -> Result { - build_default_auth_reqwest_client(endpoint, auth_route_config).map(HttpClient::new) + create_client_for_route( + &auth_http_client_factory(auth_route_config), + endpoint, + ClientRouteClass::Auth, + ) } fn auth_http_client_factory(auth_route_config: Option<&AuthRouteConfig>) -> HttpClientFactory { diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index 9509efc348..72163bcb1c 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -1,5 +1,5 @@ use chrono::Utc; -use reqwest::StatusCode; +use http::StatusCode; use serde::Deserialize; use serde::Serialize; #[cfg(test)] diff --git a/codex-rs/login/src/auth/revoke.rs b/codex-rs/login/src/auth/revoke.rs index 6b2b21dcf8..1b90020938 100644 --- a/codex-rs/login/src/auth/revoke.rs +++ b/codex-rs/login/src/auth/revoke.rs @@ -200,8 +200,8 @@ mod tests { let reqwest_error = error .get_ref() - .and_then(|error| error.downcast_ref::()) - .expect("timeout error should preserve reqwest error"); + .and_then(|error| error.downcast_ref::()) + .expect("timeout error should preserve HTTP client error"); assert!(reqwest_error.is_timeout()); } } diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 804d05434e..2b05e1df48 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -754,8 +754,10 @@ fn redact_sensitive_url_parts(url: &mut url::Url) { url.set_query(Some(&redacted_query)); } -/// Redacts any URL attached to a reqwest transport error before it is logged or returned. -fn redact_sensitive_error_url(mut err: reqwest::Error) -> reqwest::Error { +/// Redacts any URL attached to an HTTP transport error before it is logged or returned. +fn redact_sensitive_error_url( + mut err: codex_http_client::HttpError, +) -> codex_http_client::HttpError { if let Some(url) = err.url_mut() { redact_sensitive_url_parts(url); } diff --git a/codex-rs/login/tests/suite/login_server_e2e.rs b/codex-rs/login/tests/suite/login_server_e2e.rs index dbcd396ef1..ae55ae998b 100644 --- a/codex-rs/login/tests/suite/login_server_e2e.rs +++ b/codex-rs/login/tests/suite/login_server_e2e.rs @@ -9,6 +9,7 @@ use std::time::Duration; use anyhow::Result; use base64::Engine; use codex_config::types::AuthCredentialsStoreMode; +use codex_http_client::HttpClientBuilder; use codex_login::AuthKeyringBackendKind; use codex_login::LoginSuccessPage; use codex_login::LoginSuccessPageBrand; @@ -145,9 +146,9 @@ async fn end_to_end_login_flow_persists_auth_json() -> Result<()> { let login_port = server.actual_port; // Simulate browser callback and assert the local success redirect before following it. - let client = reqwest::Client::builder() - .redirect(reqwest::redirect::Policy::none()) - .build()?; + let client = HttpClientBuilder::new() + .without_redirects() + .build_direct()?; let url = format!("http://127.0.0.1:{login_port}/auth/callback?code=abc&state=test_state_123"); let resp = client.get(&url).send().await?; assert_eq!(resp.status(), 302); @@ -204,9 +205,9 @@ async fn hosted_login_redirects_to_configured_open_app_url() -> Result<()> { auth_keyring_backend_kind: AuthKeyringBackendKind::Direct, })?; let login_port = server.actual_port; - let client = reqwest::Client::builder() - .redirect(reqwest::redirect::Policy::none()) - .build()?; + let client = HttpClientBuilder::new() + .without_redirects() + .build_direct()?; let response = client .get(format!( @@ -256,7 +257,7 @@ async fn creates_missing_codex_home_dir() -> Result<()> { let server = run_login_server(opts)?; let login_port = server.actual_port; - let client = reqwest::Client::new(); + let client = HttpClientBuilder::new().build_direct()?; let url = format!("http://127.0.0.1:{login_port}/auth/callback?code=abc&state=state2"); let resp = client.get(&url).send().await?; assert!(resp.status().is_success()); @@ -349,7 +350,7 @@ async fn forced_chatgpt_workspace_id_mismatch_blocks_login() -> Result<()> { ); let login_port = server.actual_port; - let client = reqwest::Client::new(); + let client = HttpClientBuilder::new().build_direct()?; let url = format!("http://127.0.0.1:{login_port}/auth/callback?code=abc&state={state}"); let resp = client.get(&url).send().await?; assert!(resp.status().is_success()); @@ -406,7 +407,7 @@ async fn oauth_access_denied_missing_entitlement_blocks_login_with_clear_error() let server = run_login_server(opts)?; let login_port = server.actual_port; - let client = reqwest::Client::new(); + let client = HttpClientBuilder::new().build_direct()?; let url = format!( "http://127.0.0.1:{login_port}/auth/callback?state={state}&error=access_denied&error_description=missing_codex_entitlement" ); @@ -477,7 +478,7 @@ async fn oauth_access_denied_unknown_reason_uses_generic_error_page() -> Result< let server = run_login_server(opts)?; let login_port = server.actual_port; - let client = reqwest::Client::new(); + let client = HttpClientBuilder::new().build_direct()?; let url = format!( "http://127.0.0.1:{login_port}/auth/callback?state={state}&error=access_denied&error_description=some_other_reason" ); @@ -658,7 +659,7 @@ async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> { .expect_err("login server should report cancellation"); assert_eq!(cancel_result.kind(), io::ErrorKind::Interrupted); - let client = reqwest::Client::new(); + let client = HttpClientBuilder::new().build_direct()?; let cancel_url = format!("http://127.0.0.1:{login_port}/cancel"); let resp = client.get(cancel_url).send().await?; assert!(resp.status().is_success()); diff --git a/codex-rs/model-provider/src/models_endpoint.rs b/codex-rs/model-provider/src/models_endpoint.rs index 622cb8ec80..49ed6979b7 100644 --- a/codex-rs/model-provider/src/models_endpoint.rs +++ b/codex-rs/model-provider/src/models_endpoint.rs @@ -19,7 +19,7 @@ use codex_login::AuthEnvTelemetry; use codex_login::AuthManager; use codex_login::CodexAuth; use codex_login::collect_auth_env_telemetry; -use codex_login::default_client::build_default_reqwest_client_for_route_async; +use codex_login::default_client::create_client_for_route_async; use codex_model_provider_info::ModelProviderInfo; use codex_models_manager::manager::ModelsEndpointClient; use codex_models_manager::manager::ModelsEndpointFuture; @@ -169,13 +169,9 @@ impl ModelsTransportBuilder for RouteAwareModelsTransportBuilder { request_url: String, ) -> ModelsTransportFuture<'_> { Box::pin(async move { - build_default_reqwest_client_for_route_async( - http_client_factory, - request_url, - ClientRouteClass::Api, - ) - .await - .map(ReqwestTransport::new) + create_client_for_route_async(http_client_factory, request_url, ClientRouteClass::Api) + .await + .map(ReqwestTransport::from_http_client) }) } } @@ -284,7 +280,7 @@ mod tests { use super::*; use codex_http_client::OutboundProxyPolicy; - use codex_login::default_client::build_reqwest_client; + use codex_login::default_client::create_client; use codex_protocol::config_types::ModelProviderAuthInfo; use codex_protocol::openai_models::ModelsResponse; use pretty_assertions::assert_eq; @@ -312,7 +308,7 @@ mod tests { .lock() .expect("observed request lock should not be poisoned") = Some((http_client_factory.outbound_proxy_policy(), request_url)); - Ok(ReqwestTransport::new(build_reqwest_client())) + Ok(ReqwestTransport::from_http_client(create_client())) }) } }