diff --git a/AGENTS.md b/AGENTS.md index df6c3df3a..db3216b4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,10 @@ In the codex-rs folder where the rust code lives: repo root to refresh `MODULE.bazel.lock`, and include that lockfile update in the same change. - After dependency changes, run `just bazel-lock-check` from the repo root so lockfile drift is caught locally before CI. +- Bazel does not automatically make source-tree files available to compile-time Rust file access. If + you add `include_str!`, `include_bytes!`, `sqlx::migrate!`, or similar build-time file or + directory reads, update the crate's `BUILD.bazel` (`compile_data`, `build_script_data`, or test + data) or Bazel may fail even when Cargo passes. - Do not create small helper methods that are referenced only once. - Avoid large modules: - Prefer adding new modules instead of growing existing ones. diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index fbb3450e9..7c5b8c7ce 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1596,6 +1596,7 @@ version = "0.0.0" dependencies = [ "anyhow", "codex-backend-openapi-models", + "codex-client", "codex-core", "codex-protocol", "pretty_assertions", @@ -1683,15 +1684,22 @@ version = "0.0.0" dependencies = [ "async-trait", "bytes", + "codex-utils-cargo-bin", + "codex-utils-rustls-provider", "eventsource-stream", "futures", "http 1.4.0", "opentelemetry", "opentelemetry_sdk", + "pretty_assertions", "rand 0.9.2", "reqwest", + "rustls", + "rustls-native-certs", + "rustls-pki-types", "serde", "serde_json", + "tempfile", "thiserror 2.0.18", "tokio", "tracing", @@ -1732,6 +1740,7 @@ dependencies = [ "base64 0.22.1", "chrono", "clap", + "codex-client", "codex-cloud-tasks-client", "codex-core", "codex-login", @@ -2133,18 +2142,16 @@ dependencies = [ "base64 0.22.1", "chrono", "codex-app-server-protocol", + "codex-client", "codex-core", - "codex-utils-cargo-bin", "core_test_support", "pretty_assertions", "rand 0.9.2", "reqwest", - "rustls-pki-types", "serde", "serde_json", "sha2", "tempfile", - "thiserror 2.0.18", "tiny_http", "tokio", "tracing", @@ -2338,6 +2345,7 @@ version = "0.0.0" dependencies = [ "anyhow", "axum", + "codex-client", "codex-keyring-store", "codex-protocol", "codex-utils-cargo-bin", @@ -2492,6 +2500,7 @@ dependencies = [ "codex-backend-client", "codex-chatgpt", "codex-cli", + "codex-client", "codex-cloud-requirements", "codex-core", "codex-feedback", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 1b8303d6a..bc79242b2 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -240,6 +240,7 @@ rustls = { version = "0.23", default-features = false, features = [ "ring", "std", ] } +rustls-native-certs = "0.8.3" rustls-pki-types = "1.14.0" schemars = "0.8.22" seccompiler = "0.5.0" diff --git a/codex-rs/backend-client/Cargo.toml b/codex-rs/backend-client/Cargo.toml index ec5546a67..8279dba63 100644 --- a/codex-rs/backend-client/Cargo.toml +++ b/codex-rs/backend-client/Cargo.toml @@ -14,6 +14,7 @@ serde = { version = "1", features = ["derive"] } serde_json = "1" reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] } codex-backend-openapi-models = { path = "../codex-backend-openapi-models" } +codex-client = { workspace = true } codex-protocol = { workspace = true } codex-core = { workspace = true } diff --git a/codex-rs/backend-client/src/client.rs b/codex-rs/backend-client/src/client.rs index c5a0e637e..02ea98344 100644 --- a/codex-rs/backend-client/src/client.rs +++ b/codex-rs/backend-client/src/client.rs @@ -4,6 +4,7 @@ use crate::types::PaginatedListTaskListItem; use crate::types::RateLimitStatusPayload; use crate::types::TurnAttemptsSiblingTurnsResponse; use anyhow::Result; +use codex_client::build_reqwest_client_with_custom_ca; use codex_core::auth::CodexAuth; use codex_core::default_client::get_codex_user_agent; use codex_protocol::account::PlanType as AccountPlanType; @@ -120,7 +121,7 @@ impl Client { { base_url = format!("{base_url}/backend-api"); } - let http = reqwest::Client::builder().build()?; + let http = build_reqwest_client_with_custom_ca(reqwest::Client::builder())?; let path_style = PathStyle::from_base_url(&base_url); Ok(Self { base_url, diff --git a/codex-rs/cloud-tasks/Cargo.toml b/codex-rs/cloud-tasks/Cargo.toml index 717f49903..a80c455de 100644 --- a/codex-rs/cloud-tasks/Cargo.toml +++ b/codex-rs/cloud-tasks/Cargo.toml @@ -20,6 +20,7 @@ codex-cloud-tasks-client = { path = "../cloud-tasks-client", features = [ "mock", "online", ] } +codex-client = { workspace = true } codex-core = { path = "../core" } codex-login = { path = "../login" } codex-tui = { path = "../tui" } diff --git a/codex-rs/cloud-tasks/src/env_detect.rs b/codex-rs/cloud-tasks/src/env_detect.rs index e7e8fb6b1..cd38c7f34 100644 --- a/codex-rs/cloud-tasks/src/env_detect.rs +++ b/codex-rs/cloud-tasks/src/env_detect.rs @@ -1,3 +1,4 @@ +use codex_client::build_reqwest_client_with_custom_ca; use reqwest::header::CONTENT_TYPE; use reqwest::header::HeaderMap; use std::collections::HashMap; @@ -73,7 +74,7 @@ pub async fn autodetect_environment_id( }; crate::append_error_log(format!("env: GET {list_url}")); // Fetch and log the full environments JSON for debugging - let http = reqwest::Client::builder().build()?; + let http = build_reqwest_client_with_custom_ca(reqwest::Client::builder())?; let res = http.get(&list_url).headers(headers.clone()).send().await?; let status = res.status(); let ct = res @@ -147,7 +148,7 @@ async fn get_json( url: &str, headers: &HeaderMap, ) -> anyhow::Result { - let http = reqwest::Client::builder().build()?; + let http = build_reqwest_client_with_custom_ca(reqwest::Client::builder())?; let res = http.get(url).headers(headers.clone()).send().await?; let status = res.status(); let ct = res diff --git a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs index 60cb5d2c3..141f57d9d 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs @@ -14,6 +14,7 @@ use crate::endpoint::realtime_websocket::protocol::SessionUpdateSession; use crate::endpoint::realtime_websocket::protocol::parse_realtime_event; use crate::error::ApiError; use crate::provider::Provider; +use codex_client::maybe_build_rustls_client_config_with_custom_ca; use codex_utils_rustls_provider::ensure_rustls_crypto_provider; use futures::SinkExt; use futures::StreamExt; @@ -474,12 +475,19 @@ impl RealtimeWebsocketClient { request.headers_mut().extend(headers); info!("connecting realtime websocket: {ws_url}"); - let (stream, response) = - tokio_tungstenite::connect_async_with_config(request, Some(websocket_config()), false) - .await - .map_err(|err| { - ApiError::Stream(format!("failed to connect realtime websocket: {err}")) - })?; + // Realtime websocket TLS should honor the same custom-CA env vars as the rest of Codex's + // outbound HTTPS and websocket traffic. + let connector = maybe_build_rustls_client_config_with_custom_ca() + .map_err(|err| ApiError::Stream(format!("failed to configure websocket TLS: {err}")))? + .map(tokio_tungstenite::Connector::Rustls); + let (stream, response) = tokio_tungstenite::connect_async_tls_with_config( + request, + Some(websocket_config()), + false, + connector, + ) + .await + .map_err(|err| ApiError::Stream(format!("failed to connect realtime websocket: {err}")))?; info!( ws_url = %ws_url, status = %response.status(), diff --git a/codex-rs/codex-api/src/endpoint/responses_websocket.rs b/codex-rs/codex-api/src/endpoint/responses_websocket.rs index 30af9783a..d5bc6fd4b 100644 --- a/codex-rs/codex-api/src/endpoint/responses_websocket.rs +++ b/codex-rs/codex-api/src/endpoint/responses_websocket.rs @@ -10,6 +10,7 @@ use crate::sse::responses::ResponsesStreamEvent; use crate::sse::responses::process_responses_event; use crate::telemetry::WebsocketTelemetry; use codex_client::TransportError; +use codex_client::maybe_build_rustls_client_config_with_custom_ca; use codex_utils_rustls_provider::ensure_rustls_crypto_provider; use futures::SinkExt; use futures::StreamExt; @@ -30,6 +31,7 @@ use tokio::sync::oneshot; use tokio::time::Instant; use tokio_tungstenite::MaybeTlsStream; use tokio_tungstenite::WebSocketStream; +use tokio_tungstenite::connect_async_tls_with_config; use tokio_tungstenite::tungstenite::Error as WsError; use tokio_tungstenite::tungstenite::Message; use tokio_tungstenite::tungstenite::client::IntoClientRequest; @@ -331,10 +333,18 @@ async fn connect_websocket( .map_err(|err| ApiError::Stream(format!("failed to build websocket request: {err}")))?; request.headers_mut().extend(headers); - let response = tokio_tungstenite::connect_async_with_config( + // Secure websocket traffic needs the same custom-CA policy as reqwest-based HTTPS traffic. + // If a Codex-specific CA bundle is configured, build an explicit rustls connector so this + // websocket path does not fall back to tungstenite's default native-roots-only behavior. + let connector = maybe_build_rustls_client_config_with_custom_ca() + .map_err(|err| ApiError::Stream(format!("failed to configure websocket TLS: {err}")))? + .map(tokio_tungstenite::Connector::Rustls); + + let response = connect_async_tls_with_config( request, Some(websocket_config()), false, // `false` means "do not disable Nagle", which is tungstenite's recommended default. + connector, ) .await; diff --git a/codex-rs/codex-client/BUILD.bazel b/codex-rs/codex-client/BUILD.bazel index dd7e50463..b1b1ef765 100644 --- a/codex-rs/codex-client/BUILD.bazel +++ b/codex-rs/codex-client/BUILD.bazel @@ -3,4 +3,5 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "codex-client", crate_name = "codex_client", + compile_data = glob(["tests/fixtures/**"]), ) diff --git a/codex-rs/codex-client/Cargo.toml b/codex-rs/codex-client/Cargo.toml index 233bea408..2ef31ac82 100644 --- a/codex-rs/codex-client/Cargo.toml +++ b/codex-rs/codex-client/Cargo.toml @@ -13,17 +13,24 @@ http = { workspace = true } opentelemetry = { workspace = true } rand = { workspace = true } reqwest = { workspace = true, features = ["json", "stream"] } +rustls = { workspace = true } +rustls-native-certs = { workspace = true } +rustls-pki-types = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, features = ["macros", "rt", "time", "sync"] } tracing = { workspace = true } tracing-opentelemetry = { workspace = true } +codex-utils-rustls-provider = { workspace = true } zstd = { workspace = true } [lints] workspace = true [dev-dependencies] +codex-utils-cargo-bin = { workspace = true } opentelemetry_sdk = { workspace = true } +pretty_assertions = { workspace = true } +tempfile = { workspace = true } tracing-subscriber = { workspace = true } diff --git a/codex-rs/codex-client/src/bin/custom_ca_probe.rs b/codex-rs/codex-client/src/bin/custom_ca_probe.rs new file mode 100644 index 000000000..164f1054b --- /dev/null +++ b/codex-rs/codex-client/src/bin/custom_ca_probe.rs @@ -0,0 +1,29 @@ +//! Helper binary for exercising shared custom CA environment handling in tests. +//! +//! The shared reqwest client honors `CODEX_CA_CERTIFICATE` and `SSL_CERT_FILE`, but those +//! environment variables are process-global and unsafe to mutate in parallel test execution. This +//! probe keeps the behavior under test while letting integration tests (`tests/ca_env.rs`) set +//! env vars per-process, proving: +//! +//! - env precedence is respected, +//! - multi-cert PEM bundles load, +//! - error messages guide users when CA files are invalid. +//! +//! The detailed explanation of what "hermetic" means here lives in `codex_client::custom_ca`. +//! This binary exists so the tests can exercise +//! [`codex_client::build_reqwest_client_for_subprocess_tests`] in a separate process without +//! duplicating client-construction logic. + +use std::process; + +fn main() { + match codex_client::build_reqwest_client_for_subprocess_tests(reqwest::Client::builder()) { + Ok(_) => { + println!("ok"); + } + Err(error) => { + eprintln!("{error}"); + process::exit(1); + } + } +} diff --git a/codex-rs/login/src/custom_ca.rs b/codex-rs/codex-client/src/custom_ca.rs similarity index 61% rename from codex-rs/login/src/custom_ca.rs rename to codex-rs/codex-client/src/custom_ca.rs index 7d401b619..7e0a6dbee 100644 --- a/codex-rs/login/src/custom_ca.rs +++ b/codex-rs/codex-client/src/custom_ca.rs @@ -1,9 +1,11 @@ -//! Custom CA handling for login HTTP clients. +//! Custom CA handling for Codex outbound HTTP and websocket clients. //! -//! Login flows are the only place this crate constructs ad hoc outbound HTTP clients, so this -//! module centralizes the trust-store behavior that those clients must share. Enterprise networks -//! often terminate TLS with an internal root CA, which means system roots alone cannot validate -//! the OAuth and device-code endpoints that the login flows call. +//! Codex constructs outbound reqwest clients and secure websocket connections in a few crates, but +//! they all need the same trust-store policy when enterprise proxies or gateways intercept TLS. +//! This module centralizes that policy so callers can start from an ordinary +//! `reqwest::ClientBuilder` or rustls client config, layer in custom CA support, and either get +//! back a configured transport or a user-facing error that explains how to fix a misconfigured CA +//! bundle. //! //! The module intentionally has a narrow responsibility: //! @@ -13,24 +15,41 @@ //! - return user-facing errors that explain how to fix misconfigured CA files //! //! It does not validate certificate chains or perform a handshake in tests. Its contract is -//! narrower: produce a `reqwest::Client` whose root store contains every parseable certificate -//! block from the configured PEM bundle, or fail early with a precise error before the caller -//! starts a login flow. +//! narrower: produce a transport configuration whose root store contains every parseable +//! certificate block from the configured PEM bundle, or fail early with a precise error before +//! the caller starts network traffic. //! -//! The tests in this module therefore split on that boundary: +//! In this module's test setup, a hermetic test is one whose result depends only on the CA file +//! and environment variables that the test chose for itself. That matters here because the normal +//! reqwest client-construction path is not hermetic enough for environment-sensitive tests: //! -//! - unit tests cover pure env-selection logic without constructing a real client -//! - subprocess tests in `tests/ca_env.rs` cover real client construction, because that path is -//! not hermetic in macOS sandboxed runs and must also scrub inherited CA environment variables -//! - the spawned `login_ca_probe` binary reaches the probe-only builder through the hidden -//! `probe_support` module so that workaround does not become part of the normal crate API +//! - on macOS seatbelt runs, `reqwest::Client::builder().build()` can panic inside +//! `system-configuration` while probing platform proxy settings, which means the process can die +//! before the custom-CA code reports success or a structured error. That matters in practice +//! because Codex itself commonly runs spawned test processes under seatbelt, so this is not just +//! a hypothetical CI edge case. +//! - child processes inherit CA-related environment variables by default, which lets developer +//! shell state or CI configuration affect a test unless the test scrubs those variables first +//! +//! The tests in this crate therefore stay split across two layers: +//! +//! - unit tests in this module cover env-selection logic without constructing a real client +//! - subprocess integration tests under `tests/` cover real client construction through +//! [`build_reqwest_client_for_subprocess_tests`], which disables reqwest proxy autodetection so +//! the tests can observe custom-CA success and failure directly +//! - those subprocess tests also scrub inherited CA environment variables before launch so their +//! result depends only on the test fixtures and env vars set by the test itself use std::env; use std::fs; use std::io; use std::path::Path; use std::path::PathBuf; +use std::sync::Arc; +use codex_utils_rustls_provider::ensure_rustls_crypto_provider; +use rustls::ClientConfig; +use rustls::RootCertStore; use rustls_pki_types::CertificateDer; use rustls_pki_types::pem::PemObject; use rustls_pki_types::pem::SectionKind; @@ -39,20 +58,20 @@ use thiserror::Error; use tracing::info; use tracing::warn; -const CODEX_CA_CERT_ENV: &str = "CODEX_CA_CERTIFICATE"; -const SSL_CERT_FILE_ENV: &str = "SSL_CERT_FILE"; +pub const CODEX_CA_CERT_ENV: &str = "CODEX_CA_CERTIFICATE"; +pub const SSL_CERT_FILE_ENV: &str = "SSL_CERT_FILE"; const CA_CERT_HINT: &str = "If you set CODEX_CA_CERTIFICATE or SSL_CERT_FILE, ensure it points to a PEM file containing one or more CERTIFICATE blocks, or unset it to use system roots."; type PemSection = (SectionKind, Vec); -/// Describes why the login HTTP client could not be constructed. +/// Describes why a transport using shared custom CA support could not be constructed. /// -/// This boundary is more specific than `io::Error`: login can fail because the configured CA file -/// could not be read, could not be parsed as certificates, contained certs that `reqwest` refused -/// to register, or because the final client builder failed. The rest of the login crate still -/// speaks `io::Error`, so callers that do not care about the distinction can rely on the -/// `From for io::Error` conversion. +/// These failure modes apply to both reqwest client construction and websocket TLS +/// configuration. A build can fail because the configured CA file could not be read, could not be +/// parsed as certificates, contained certs that the target TLS stack refused to register, or +/// because the final reqwest client builder failed. Callers that do not care about the +/// distinction can rely on the `From for io::Error` conversion. #[derive(Debug, Error)] -pub enum BuildLoginHttpClientError { +pub enum BuildCustomCaTransportError { /// Reading the selected CA file from disk failed before any PEM parsing could happen. #[error( "Failed to read CA certificate file {} selected by {}: {source}. {hint}", @@ -95,7 +114,7 @@ pub enum BuildLoginHttpClientError { /// Reqwest rejected the final client configuration after a custom CA bundle was loaded. #[error( - "Failed to build login HTTP client while using CA bundle from {} ({}): {source}", + "Failed to build HTTP client while using CA bundle from {} ({}): {source}", source_env, path.display() )] @@ -107,76 +126,151 @@ pub enum BuildLoginHttpClientError { }, /// Reqwest rejected the final client configuration while using only system roots. - #[error("Failed to build login HTTP client while using system root certificates: {0}")] + #[error("Failed to build HTTP client while using system root certificates: {0}")] BuildClientWithSystemRoots(#[source] reqwest::Error), + + /// One parsed certificate block could not be registered with the websocket TLS root store. + #[error( + "Failed to register certificate #{certificate_index} from {} selected by {} in rustls root store: {source}. {hint}", + path.display(), + source_env, + hint = CA_CERT_HINT + )] + RegisterRustlsCertificate { + source_env: &'static str, + path: PathBuf, + certificate_index: usize, + source: rustls::Error, + }, } -impl From for io::Error { - fn from(error: BuildLoginHttpClientError) -> Self { +impl From for io::Error { + fn from(error: BuildCustomCaTransportError) -> Self { match error { - BuildLoginHttpClientError::ReadCaFile { ref source, .. } => { + BuildCustomCaTransportError::ReadCaFile { ref source, .. } => { io::Error::new(source.kind(), error) } - BuildLoginHttpClientError::InvalidCaFile { .. } - | BuildLoginHttpClientError::RegisterCertificate { .. } => { + BuildCustomCaTransportError::InvalidCaFile { .. } + | BuildCustomCaTransportError::RegisterCertificate { .. } + | BuildCustomCaTransportError::RegisterRustlsCertificate { .. } => { io::Error::new(io::ErrorKind::InvalidData, error) } - BuildLoginHttpClientError::BuildClientWithCustomCa { .. } - | BuildLoginHttpClientError::BuildClientWithSystemRoots(_) => io::Error::other(error), + BuildCustomCaTransportError::BuildClientWithCustomCa { .. } + | BuildCustomCaTransportError::BuildClientWithSystemRoots(_) => io::Error::other(error), } } } -/// Builds the HTTP client used by login and device-code flows. +/// Builds a reqwest client that honors Codex custom CA environment variables. /// -/// Callers should use this instead of constructing a raw `reqwest::Client` so every login entry -/// point honors the same CA override behavior. A caller that bypasses this helper can silently -/// regress enterprise login setups that rely on `CODEX_CA_CERTIFICATE` or `SSL_CERT_FILE`. -/// `CODEX_CA_CERTIFICATE` takes precedence over `SSL_CERT_FILE`, and empty values for either are -/// treated as unset so callers do not accidentally turn `VAR=""` into a bogus path lookup. +/// Callers supply the baseline builder configuration they need, and this helper layers in custom +/// CA handling before finally constructing the client. `CODEX_CA_CERTIFICATE` takes precedence +/// over `SSL_CERT_FILE`, and empty values for either are treated as unset so callers do not +/// accidentally turn `VAR=""` into a bogus path lookup. +/// +/// Callers that build a raw `reqwest::Client` directly bypass this policy entirely. That is an +/// easy mistake to make when adding a new outbound Codex HTTP path, and the resulting bug only +/// shows up in environments where a proxy or gateway requires a custom root CA. /// /// # Errors /// -/// Returns a [`BuildLoginHttpClientError`] when the configured CA file is unreadable, malformed, -/// or contains a certificate block that `reqwest` cannot register as a root. Calling raw -/// `reqwest::Client::builder()` instead would skip those user-facing errors and can make login -/// failures in enterprise environments much harder to diagnose. -pub fn build_login_http_client() -> Result { - build_login_http_client_with_env(&ProcessEnv, reqwest::Client::builder()) +/// Returns a [`BuildCustomCaTransportError`] when the configured CA file is unreadable, +/// malformed, or contains a certificate block that `reqwest` cannot register as a root. +pub fn build_reqwest_client_with_custom_ca( + builder: reqwest::ClientBuilder, +) -> Result { + build_reqwest_client_with_env(&ProcessEnv, builder) } -/// Builds the login HTTP client used behind the spawned CA probe binary. +/// Builds a rustls client config when a Codex custom CA bundle is configured. /// -/// This stays crate-private because normal callers should continue to go through -/// [`build_login_http_client`]. The hidden `probe_support` module exposes this behavior only to -/// `login_ca_probe`, which disables proxy autodetection so the subprocess tests can reach the -/// custom-CA code path in sandboxed macOS test runs without crashing first in reqwest's platform -/// proxy setup. Using this path for normal login would make the tests and production behavior -/// diverge on proxy handling, which is exactly what the hidden module arrangement is trying to -/// avoid. -pub(crate) fn build_login_http_client_for_subprocess_tests() --> Result { - build_login_http_client_with_env( - &ProcessEnv, - // The probe disables proxy autodetection so the subprocess tests can reach the custom-CA - // code path even in macOS seatbelt runs, where platform proxy discovery can panic first. - reqwest::Client::builder().no_proxy(), - ) +/// This is the websocket-facing sibling of [`build_reqwest_client_with_custom_ca`]. When +/// `CODEX_CA_CERTIFICATE` or `SSL_CERT_FILE` selects a CA bundle, the returned config starts from +/// the platform native roots and then adds the configured custom CA certificates. When no custom +/// CA env var is set, this returns `Ok(None)` so websocket callers can keep using their ordinary +/// default connector path. +/// +/// Callers that let tungstenite build its default TLS connector directly bypass this policy +/// entirely. That bug only shows up in environments where secure websocket traffic needs the same +/// enterprise root CA bundle as HTTPS traffic. +pub fn maybe_build_rustls_client_config_with_custom_ca() +-> Result>, BuildCustomCaTransportError> { + maybe_build_rustls_client_config_with_env(&ProcessEnv) } -/// Builds a login HTTP client using an injected environment source and reqwest builder. +/// Builds a reqwest client for spawned subprocess tests that exercise CA behavior. /// -/// This exists so unit tests can exercise precedence and PEM-handling behavior deterministically. -/// Production code should call [`build_login_http_client`] instead of supplying its own -/// environment adapter, otherwise the tests and the real process environment can drift apart. -/// This function is also the place where module responsibilities come together: it selects the CA -/// bundle, delegates file parsing to [`ConfiguredCaBundle::load_certificates`], preserves the -/// caller's chosen `reqwest` builder configuration, and finally registers each parsed certificate -/// with that builder. -fn build_login_http_client_with_env( +/// This is the test-only client-construction path used by the subprocess coverage in `tests/`. +/// The module-level docs explain the hermeticity problem in full; this helper only addresses the +/// reqwest proxy-discovery panic side of that problem by disabling proxy autodetection. The tests +/// still scrub inherited CA environment variables themselves. Normal production callers should use +/// [`build_reqwest_client_with_custom_ca`] so test-only proxy behavior does not leak into +/// ordinary client construction. +pub fn build_reqwest_client_for_subprocess_tests( + builder: reqwest::ClientBuilder, +) -> Result { + build_reqwest_client_with_env(&ProcessEnv, builder.no_proxy()) +} + +fn maybe_build_rustls_client_config_with_env( + env_source: &dyn EnvSource, +) -> Result>, BuildCustomCaTransportError> { + let Some(bundle) = env_source.configured_ca_bundle() else { + return Ok(None); + }; + + ensure_rustls_crypto_provider(); + + // Start from the platform roots so websocket callers keep the same baseline trust behavior + // they would get from tungstenite's default rustls connector, then layer in the Codex custom + // CA bundle on top when configured. + let mut root_store = RootCertStore::empty(); + let rustls_native_certs::CertificateResult { certs, errors, .. } = + rustls_native_certs::load_native_certs(); + if !errors.is_empty() { + warn!( + native_root_error_count = errors.len(), + "encountered errors while loading native root certificates" + ); + } + let _ = root_store.add_parsable_certificates(certs); + + let certificates = bundle.load_certificates()?; + for (idx, cert) in certificates.into_iter().enumerate() { + if let Err(source) = root_store.add(cert) { + warn!( + source_env = bundle.source_env, + ca_path = %bundle.path.display(), + certificate_index = idx + 1, + error = %source, + "failed to register CA certificate in rustls root store" + ); + return Err(BuildCustomCaTransportError::RegisterRustlsCertificate { + source_env: bundle.source_env, + path: bundle.path.clone(), + certificate_index: idx + 1, + source, + }); + } + } + + Ok(Some(Arc::new( + ClientConfig::builder() + .with_root_certificates(root_store) + .with_no_client_auth(), + ))) +} + +/// Builds a reqwest client using an injected environment source and reqwest builder. +/// +/// This exists so tests can exercise precedence behavior deterministically without mutating the +/// real process environment. It selects the CA bundle, delegates file parsing to +/// [`ConfiguredCaBundle::load_certificates`], preserves the caller's chosen `reqwest` builder +/// configuration, and finally registers each parsed certificate with that builder. +fn build_reqwest_client_with_env( env_source: &dyn EnvSource, mut builder: reqwest::ClientBuilder, -) -> Result { +) -> Result { if let Some(bundle) = env_source.configured_ca_bundle() { let certificates = bundle.load_certificates()?; @@ -189,9 +283,9 @@ fn build_login_http_client_with_env( ca_path = %bundle.path.display(), certificate_index = idx + 1, error = %source, - "failed to register login CA certificate" + "failed to register CA certificate" ); - return Err(BuildLoginHttpClientError::RegisterCertificate { + return Err(BuildCustomCaTransportError::RegisterCertificate { source_env: bundle.source_env, path: bundle.path.clone(), certificate_index: idx + 1, @@ -210,7 +304,7 @@ fn build_login_http_client_with_env( error = %source, "failed to build client after loading custom CA bundle" ); - Err(BuildLoginHttpClientError::BuildClientWithCustomCa { + Err(BuildCustomCaTransportError::BuildClientWithCustomCa { source_env: bundle.source_env, path: bundle.path.clone(), source, @@ -232,7 +326,7 @@ fn build_login_http_client_with_env( error = %source, "failed to build client while using system root certificates" ); - Err(BuildLoginHttpClientError::BuildClientWithSystemRoots( + Err(BuildCustomCaTransportError::BuildClientWithSystemRoots( source, )) } @@ -245,17 +339,17 @@ trait EnvSource { /// Returns the environment variable value for `key`, if this source considers it set. /// /// Implementations should return `None` for absent values and may also collapse unreadable - /// process-environment states into `None`, because the login CA logic treats both cases as + /// process-environment states into `None`, because the custom CA logic treats both cases as /// "no override configured". Callers build precedence and empty-string handling on top of this /// method, so implementations should not trim or normalize the returned string. fn var(&self, key: &str) -> Option; /// Returns a non-empty environment variable value interpreted as a filesystem path. /// - /// Empty strings are treated as unset because login uses presence here as a boolean "custom CA + /// Empty strings are treated as unset because presence here acts as a boolean "custom CA /// override requested" signal. This keeps the precedence logic from treating `VAR=""` as an - /// attempt to open the current working directory or some other platform-specific oddity once it - /// is converted into a path. + /// attempt to open the current working directory or some other platform-specific oddity once + /// it is converted into a path. fn non_empty_path(&self, key: &str) -> Option { self.var(key) .filter(|value| !value.is_empty()) @@ -264,7 +358,7 @@ trait EnvSource { /// Returns the configured CA bundle and which environment variable selected it. /// - /// `CODEX_CA_CERTIFICATE` wins over `SSL_CERT_FILE` because it is the login-specific override. + /// `CODEX_CA_CERTIFICATE` wins over `SSL_CERT_FILE` because it is the Codex-specific override. /// Keeping the winning variable name with the path lets later logging explain not only which /// file was used but also why that file was chosen. fn configured_ca_bundle(&self) -> Option { @@ -283,12 +377,11 @@ trait EnvSource { } } -/// Reads login CA configuration from the real process environment. +/// Reads CA configuration from the real process environment. /// /// This is the production `EnvSource` implementation used by -/// [`build_login_http_client`]. Tests substitute in-memory env maps so they can -/// exercise precedence and empty-value behavior without mutating process-global -/// variables. +/// [`build_reqwest_client_with_custom_ca`]. Tests substitute in-memory env maps so they can +/// exercise precedence and empty-value behavior without mutating process-global variables. struct ProcessEnv; impl EnvSource for ProcessEnv { @@ -297,7 +390,7 @@ impl EnvSource for ProcessEnv { } } -/// Identifies the CA bundle selected for login and the policy decision that selected it. +/// Identifies the CA bundle selected for a client and the policy decision that selected it. /// /// This is the concrete output of the environment-precedence logic. Callers use `source_env` for /// logging and diagnostics, while `path` is the bundle that will actually be loaded. @@ -315,7 +408,9 @@ impl ConfiguredCaBundle { /// the natural point where the file-loading phase begins. The method owns the high-level /// success/failure logs for that phase and keeps the source env and path together for lower- /// level parsing and error shaping. - fn load_certificates(&self) -> Result>, BuildLoginHttpClientError> { + fn load_certificates( + &self, + ) -> Result>, BuildCustomCaTransportError> { match self.parse_certificates() { Ok(certificates) => { info!( @@ -338,26 +433,20 @@ impl ConfiguredCaBundle { } } - /// Loads every certificate block from a PEM file intended for login CA overrides. + /// Loads every certificate block from a PEM file intended for Codex CA overrides. /// - /// This accepts a few common real-world variants so login behaves like other CA-aware tooling: + /// This accepts a few common real-world variants so Codex behaves like other CA-aware tooling: /// leading comments are preserved, `TRUSTED CERTIFICATE` labels are normalized to standard - /// certificate labels, and embedded CRLs are ignored. - /// - /// # Errors - /// - /// Returns `InvalidData` when the file cannot be interpreted as one or more certificates, and - /// preserves the filesystem error kind when the file itself cannot be read. + /// certificate labels, and embedded CRLs are ignored when they are well-formed enough for the + /// section iterator to classify them. fn parse_certificates( &self, - ) -> Result>, BuildLoginHttpClientError> { + ) -> Result>, BuildCustomCaTransportError> { let pem_data = self.read_pem_data()?; let normalized_pem = NormalizedPem::from_pem_data(self.source_env, &self.path, &pem_data); let mut certificates = Vec::new(); let mut logged_crl_presence = false; - // Use the mixed-section parser from `rustls-pki-types` so CRLs can be identified and - // skipped explicitly instead of being removed with ad hoc text rewriting. for section_result in normalized_pem.sections() { // Known limitation: if `rustls-pki-types` fails while parsing a malformed CRL section, // that error is reported here before we can classify the block as ignorable. A bundle @@ -369,6 +458,9 @@ impl ConfiguredCaBundle { }; match section_kind { SectionKind::Certificate => { + // Standard CERTIFICATE blocks already decode to the exact DER bytes reqwest + // wants. Only OpenSSL TRUSTED CERTIFICATE blocks need trimming to drop any + // trailing X509_AUX trust metadata before registration. let cert_der = normalized_pem.certificate_der(&der).ok_or_else(|| { self.invalid_ca_file( "failed to extract certificate data from TRUSTED CERTIFICATE: invalid DER length", @@ -396,13 +488,14 @@ impl ConfiguredCaBundle { Ok(certificates) } + /// Reads the CA bundle bytes while preserving the original filesystem error kind. /// /// The caller wants a user-facing error that includes the bundle path and remediation hint, but - /// the higher-level login surfaces still benefit from distinguishing "not found" from other I/O + /// higher-level surfaces still benefit from distinguishing "not found" from other I/O /// failures. This helper keeps both pieces together. - fn read_pem_data(&self) -> Result, BuildLoginHttpClientError> { - fs::read(&self.path).map_err(|source| BuildLoginHttpClientError::ReadCaFile { + fn read_pem_data(&self) -> Result, BuildCustomCaTransportError> { + fs::read(&self.path).map_err(|source| BuildCustomCaTransportError::ReadCaFile { source_env: self.source_env, path: self.path.clone(), source, @@ -414,7 +507,7 @@ impl ConfiguredCaBundle { /// The underlying parser knows whether the file was empty, malformed, or contained unsupported /// PEM content, but callers need a message that also points them back to the relevant /// environment variables and the expected remediation. - fn pem_parse_error(&self, error: &pem::Error) -> BuildLoginHttpClientError { + fn pem_parse_error(&self, error: &pem::Error) -> BuildCustomCaTransportError { let detail = match error { pem::Error::NoItemsFound => "no certificates found in PEM file".to_string(), _ => format!("failed to parse PEM file: {error}"), @@ -428,8 +521,8 @@ impl ConfiguredCaBundle { /// Most parse-time failures in this module eventually collapse to "the configured CA bundle is /// not usable", but the detailed reason still matters for operator debugging. Centralizing that /// formatting keeps the path and hint text consistent across the different parser branches. - fn invalid_ca_file(&self, detail: impl std::fmt::Display) -> BuildLoginHttpClientError { - BuildLoginHttpClientError::InvalidCaFile { + fn invalid_ca_file(&self, detail: impl std::fmt::Display) -> BuildCustomCaTransportError { + BuildCustomCaTransportError::InvalidCaFile { source_env: self.source_env, path: self.path.clone(), detail: detail.to_string(), @@ -452,7 +545,7 @@ enum NormalizedPem { impl NormalizedPem { /// Normalizes PEM text from a CA bundle into the label shape this module expects. /// - /// Login only needs certificate DER bytes to seed `reqwest`'s root store, but operators may + /// Codex only needs certificate DER bytes to seed `reqwest`'s root store, but operators may /// point it at CA files that came from OpenSSL tooling rather than from a minimal certificate /// bundle. OpenSSL's `TRUSTED CERTIFICATE` form is one such variant: it is still certificate /// material, but it uses a different PEM label and may carry auxiliary trust metadata that @@ -589,18 +682,20 @@ fn der_item_length(der: &[u8]) -> Option { #[cfg(test)] mod tests { use std::collections::HashMap; + use std::fs; use std::path::PathBuf; use pretty_assertions::assert_eq; + use tempfile::TempDir; + use super::BuildCustomCaTransportError; use super::CODEX_CA_CERT_ENV; use super::EnvSource; use super::SSL_CERT_FILE_ENV; + use super::maybe_build_rustls_client_config_with_env; + + const TEST_CERT: &str = include_str!("../tests/fixtures/test-ca.pem"); - // Keep this module limited to pure precedence logic. Building a real reqwest client here is - // not hermetic on macOS sandboxed test runs because client construction can consult platform - // networking configuration and panic before the test asserts anything. The real client-building - // cases live in `tests/ca_env.rs`, which exercises them in a subprocess with explicit env. struct MapEnv { values: HashMap, } @@ -620,6 +715,14 @@ mod tests { } } + fn write_cert_file(temp_dir: &TempDir, name: &str, contents: &str) -> PathBuf { + let path = temp_dir.path().join(name); + fs::write(&path, contents).unwrap_or_else(|error| { + panic!("write cert fixture failed for {}: {error}", path.display()) + }); + path + } + #[test] fn ca_path_prefers_codex_env() { let env = map_env(&[ @@ -655,4 +758,31 @@ mod tests { Some(PathBuf::from("/tmp/fallback.pem")) ); } + + #[test] + fn rustls_config_uses_custom_ca_bundle_when_configured() { + let temp_dir = TempDir::new().expect("tempdir"); + let cert_path = write_cert_file(&temp_dir, "ca.pem", TEST_CERT); + let env = map_env(&[(CODEX_CA_CERT_ENV, cert_path.to_string_lossy().as_ref())]); + + let config = maybe_build_rustls_client_config_with_env(&env) + .expect("rustls config") + .expect("custom CA config should be present"); + + assert!(config.enable_sni); + } + + #[test] + fn rustls_config_reports_invalid_ca_file() { + let temp_dir = TempDir::new().expect("tempdir"); + let cert_path = write_cert_file(&temp_dir, "empty.pem", ""); + let env = map_env(&[(CODEX_CA_CERT_ENV, cert_path.to_string_lossy().as_ref())]); + + let error = maybe_build_rustls_client_config_with_env(&env).expect_err("invalid CA"); + + assert!(matches!( + error, + BuildCustomCaTransportError::InvalidCaFile { .. } + )); + } } diff --git a/codex-rs/codex-client/src/lib.rs b/codex-rs/codex-client/src/lib.rs index 089d777c3..93dd81506 100644 --- a/codex-rs/codex-client/src/lib.rs +++ b/codex-rs/codex-client/src/lib.rs @@ -1,3 +1,4 @@ +mod custom_ca; mod default_client; mod error; mod request; @@ -6,6 +7,16 @@ mod sse; mod telemetry; mod transport; +pub use crate::custom_ca::BuildCustomCaTransportError; +/// Test-only subprocess hook for custom CA coverage. +/// +/// This stays public only so the `custom_ca_probe` binary target can reuse the shared helper. It +/// is hidden from normal docs because ordinary callers should use +/// [`build_reqwest_client_with_custom_ca`] instead. +#[doc(hidden)] +pub use crate::custom_ca::build_reqwest_client_for_subprocess_tests; +pub use crate::custom_ca::build_reqwest_client_with_custom_ca; +pub use crate::custom_ca::maybe_build_rustls_client_config_with_custom_ca; pub use crate::default_client::CodexHttpClient; pub use crate::default_client::CodexRequestBuilder; pub use crate::error::StreamError; diff --git a/codex-rs/login/tests/ca_env.rs b/codex-rs/codex-client/tests/ca_env.rs similarity index 82% rename from codex-rs/login/tests/ca_env.rs rename to codex-rs/codex-client/tests/ca_env.rs index d4fd1fa27..6992ea732 100644 --- a/codex-rs/login/tests/ca_env.rs +++ b/codex-rs/codex-client/tests/ca_env.rs @@ -1,13 +1,12 @@ //! Subprocess coverage for custom CA behavior that must build a real reqwest client. //! -//! These tests intentionally run through `login_ca_probe` instead of calling the helper in-process: -//! reqwest client construction is not hermetic on macOS sandboxed runs, and these cases also need -//! exact control over inherited CA environment variables. The probe disables reqwest proxy -//! autodetection because `reqwest::Client::builder().build()` can panic inside -//! `system-configuration` while probing macOS proxy settings under seatbelt. The probe-level -//! workaround keeps these tests focused on custom-CA success and failure instead of failing first -//! on unrelated platform proxy discovery. These tests still stop at client construction: they -//! verify CA file selection, PEM parsing, and user-facing errors, not a full TLS handshake. +//! These tests intentionally run through `custom_ca_probe` and +//! `build_reqwest_client_for_subprocess_tests` instead of calling the helper in-process. The +//! detailed explanation of what "hermetic" means here lives in `codex_client::custom_ca`; these +//! tests add the process-level half of that contract by scrubbing inherited CA environment +//! variables before each subprocess launch. They still stop at client construction: the +//! assertions here cover CA file selection, PEM parsing, and user-facing errors, not a full TLS +//! handshake. use codex_utils_cargo_bin::cargo_bin; use std::fs; @@ -32,8 +31,8 @@ fn write_cert_file(temp_dir: &TempDir, name: &str, contents: &str) -> std::path: fn run_probe(envs: &[(&str, &Path)]) -> std::process::Output { let mut cmd = Command::new( - cargo_bin("login_ca_probe") - .unwrap_or_else(|error| panic!("failed to locate login_ca_probe: {error}")), + cargo_bin("custom_ca_probe") + .unwrap_or_else(|error| panic!("failed to locate custom_ca_probe: {error}")), ); // `Command` inherits the parent environment by default, so scrub CA-related variables first or // these tests can accidentally pass/fail based on the developer shell or CI runner. @@ -43,7 +42,7 @@ fn run_probe(envs: &[(&str, &Path)]) -> std::process::Output { cmd.env(key, value); } cmd.output() - .unwrap_or_else(|error| panic!("failed to run login_ca_probe: {error}")) + .unwrap_or_else(|error| panic!("failed to run custom_ca_probe: {error}")) } #[test] diff --git a/codex-rs/login/tests/fixtures/test-ca-trusted.pem b/codex-rs/codex-client/tests/fixtures/test-ca-trusted.pem similarity index 91% rename from codex-rs/login/tests/fixtures/test-ca-trusted.pem rename to codex-rs/codex-client/tests/fixtures/test-ca-trusted.pem index 76716033c..0b394ce84 100644 --- a/codex-rs/login/tests/fixtures/test-ca-trusted.pem +++ b/codex-rs/codex-client/tests/fixtures/test-ca-trusted.pem @@ -2,6 +2,8 @@ # `openssl x509 -addtrust serverAuth -trustout`. # The extra trailing bytes model the OpenSSL X509_AUX data that follows the # certificate DER in real TRUSTED CERTIFICATE bundles. +# This fixture exists to validate the X509_AUX trimming path against a real +# OpenSSL-generated artifact, not just label normalization. -----BEGIN TRUSTED CERTIFICATE----- MIIDBTCCAe2gAwIBAgIUZYhGvBUG7SucNzYh9VIeZ7b9zHowDQYJKoZIhvcNAQEL BQAwEjEQMA4GA1UEAwwHdGVzdC1jYTAeFw0yNTEyMTEyMzEyNTFaFw0zNTEyMDky diff --git a/codex-rs/login/tests/fixtures/test-ca.pem b/codex-rs/codex-client/tests/fixtures/test-ca.pem similarity index 100% rename from codex-rs/login/tests/fixtures/test-ca.pem rename to codex-rs/codex-client/tests/fixtures/test-ca.pem diff --git a/codex-rs/login/tests/fixtures/test-intermediate.pem b/codex-rs/codex-client/tests/fixtures/test-intermediate.pem similarity index 100% rename from codex-rs/login/tests/fixtures/test-intermediate.pem rename to codex-rs/codex-client/tests/fixtures/test-intermediate.pem diff --git a/codex-rs/core/src/default_client.rs b/codex-rs/core/src/default_client.rs index 6d0b5496c..809c73eed 100644 --- a/codex-rs/core/src/default_client.rs +++ b/codex-rs/core/src/default_client.rs @@ -1,7 +1,9 @@ use crate::config_loader::ResidencyRequirement; use crate::spawn::CODEX_SANDBOX_ENV_VAR; +use codex_client::BuildCustomCaTransportError; use codex_client::CodexHttpClient; pub use codex_client::CodexRequestBuilder; +use codex_client::build_reqwest_client_with_custom_ca; use reqwest::header::HeaderMap; use reqwest::header::HeaderValue; use std::sync::LazyLock; @@ -182,7 +184,24 @@ pub fn create_client() -> CodexHttpClient { CodexHttpClient::new(inner) } +/// 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()`. 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"); + 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 { let ua = get_codex_user_agent(); let mut builder = reqwest::Client::builder() @@ -193,7 +212,7 @@ pub fn build_reqwest_client() -> reqwest::Client { builder = builder.no_proxy(); } - builder.build().unwrap_or_else(|_| reqwest::Client::new()) + build_reqwest_client_with_custom_ca(builder) } pub fn default_headers() -> HeaderMap { diff --git a/codex-rs/login/Cargo.toml b/codex-rs/login/Cargo.toml index 794b2be28..5524fec7c 100644 --- a/codex-rs/login/Cargo.toml +++ b/codex-rs/login/Cargo.toml @@ -10,16 +10,15 @@ workspace = true [dependencies] base64 = { workspace = true } chrono = { workspace = true, features = ["serde"] } +codex-client = { workspace = true } codex-core = { workspace = true } codex-app-server-protocol = { workspace = true } rand = { workspace = true } reqwest = { workspace = true, features = ["json", "blocking"] } -rustls-pki-types = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } sha2 = { workspace = true } tiny_http = { workspace = true } -thiserror = { workspace = true } tokio = { workspace = true, features = [ "io-std", "macros", @@ -34,7 +33,6 @@ webbrowser = { workspace = true } [dev-dependencies] anyhow = { workspace = true } -codex-utils-cargo-bin = { workspace = true } core_test_support = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/login/src/bin/login_ca_probe.rs b/codex-rs/login/src/bin/login_ca_probe.rs deleted file mode 100644 index 1e9073601..000000000 --- a/codex-rs/login/src/bin/login_ca_probe.rs +++ /dev/null @@ -1,31 +0,0 @@ -//! Helper binary for exercising custom CA environment handling in tests. -//! -//! The login flows honor `CODEX_CA_CERTIFICATE` and `SSL_CERT_FILE`, but those environment -//! variables are process-global and unsafe to mutate in parallel test execution. This probe keeps -//! the behavior under test while letting integration tests (`tests/ca_env.rs`) set env vars -//! per-process, proving: -//! -//! - env precedence is respected, -//! - multi-cert PEM bundles load, -//! - error messages guide users when CA files are invalid. -//! -//! The probe intentionally disables reqwest proxy autodetection while building the client. That -//! keeps the subprocess tests hermetic in macOS seatbelt runs, where -//! `reqwest::Client::builder().build()` can panic inside the `system-configuration` crate while -//! probing macOS proxy settings. Without that workaround, the subprocess exits before the custom -//! CA code reports either success or a structured `BuildLoginHttpClientError`, so tests that are -//! supposed to validate CA parsing instead fail on unrelated platform proxy discovery. - -use std::process; - -fn main() { - match codex_login::probe_support::build_login_http_client() { - Ok(_) => { - println!("ok"); - } - Err(error) => { - eprintln!("{error}"); - process::exit(1); - } - } -} diff --git a/codex-rs/login/src/device_code_auth.rs b/codex-rs/login/src/device_code_auth.rs index c283ad117..678781ac1 100644 --- a/codex-rs/login/src/device_code_auth.rs +++ b/codex-rs/login/src/device_code_auth.rs @@ -6,9 +6,9 @@ use serde::de::{self}; use std::time::Duration; use std::time::Instant; -use crate::build_login_http_client; use crate::pkce::PkceCodes; use crate::server::ServerOptions; +use codex_client::build_reqwest_client_with_custom_ca; use std::io; const ANSI_BLUE: &str = "\x1b[94m"; @@ -157,7 +157,7 @@ fn print_device_code_prompt(verification_url: &str, code: &str) { } pub async fn request_device_code(opts: &ServerOptions) -> std::io::Result { - let client = build_login_http_client()?; + let client = build_reqwest_client_with_custom_ca(reqwest::Client::builder())?; let base_url = opts.issuer.trim_end_matches('/'); let api_base_url = format!("{base_url}/api/accounts"); let uc = request_user_code(&client, &api_base_url, &opts.client_id).await?; @@ -174,7 +174,7 @@ pub async fn complete_device_code_login( opts: ServerOptions, device_code: DeviceCode, ) -> std::io::Result<()> { - let client = build_login_http_client()?; + let client = build_reqwest_client_with_custom_ca(reqwest::Client::builder())?; let base_url = opts.issuer.trim_end_matches('/'); let api_base_url = format!("{base_url}/api/accounts"); diff --git a/codex-rs/login/src/lib.rs b/codex-rs/login/src/lib.rs index ea2e059fb..60b0c57f2 100644 --- a/codex-rs/login/src/lib.rs +++ b/codex-rs/login/src/lib.rs @@ -1,16 +1,8 @@ -mod custom_ca; mod device_code_auth; mod pkce; -// Hidden because this exists only to let the spawned `login_ca_probe` binary call the -// probe-specific client builder without exposing that workaround as part of the normal API. -// `login_ca_probe` is a separate binary target, not a `#[cfg(test)]` module inside this crate, so -// it cannot call crate-private helpers and would not see test-only modules. -#[doc(hidden)] -pub mod probe_support; mod server; -pub use custom_ca::BuildLoginHttpClientError; -pub use custom_ca::build_login_http_client; +pub use codex_client::BuildCustomCaTransportError as BuildLoginHttpClientError; pub use device_code_auth::DeviceCode; pub use device_code_auth::complete_device_code_login; pub use device_code_auth::request_device_code; diff --git a/codex-rs/login/src/probe_support.rs b/codex-rs/login/src/probe_support.rs deleted file mode 100644 index bf050a347..000000000 --- a/codex-rs/login/src/probe_support.rs +++ /dev/null @@ -1,22 +0,0 @@ -//! Test-only support for spawned login probe binaries. -//! -//! This module exists because `login_ca_probe` is compiled as a separate binary target, so it -//! cannot call crate-private helpers directly. Keeping the probe entry point under a hidden module -//! avoids surfacing it as part of the normal `codex-login` public API while still letting the -//! subprocess tests share the real custom-CA client-construction code. It is intentionally not a -//! general-purpose login API: the functions here exist only so the subprocess tests can exercise -//! CA loading in a separate process without duplicating logic in the probe binary. - -use crate::BuildLoginHttpClientError; - -/// Builds the login HTTP client for the subprocess CA probe tests. -/// -/// The probe disables reqwest proxy autodetection so it can exercise custom-CA success and -/// failure in macOS seatbelt runs without tripping the known `system-configuration` panic during -/// platform proxy discovery. This is intentionally not the main public login entry point: normal -/// login callers should continue to use [`crate::build_login_http_client`]. A non-test caller that -/// reached for this helper would mask real proxy behavior and risk debugging a code path that does -/// not match production login. -pub fn build_login_http_client() -> Result { - crate::custom_ca::build_login_http_client_for_subprocess_tests() -} diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 08f99bb97..2cb88c9ca 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -23,12 +23,12 @@ use std::sync::Arc; use std::thread; use std::time::Duration; -use crate::custom_ca::build_login_http_client; use crate::pkce::PkceCodes; use crate::pkce::generate_pkce; use base64::Engine; use chrono::Utc; use codex_app_server_protocol::AuthMode; +use codex_client::build_reqwest_client_with_custom_ca; use codex_core::auth::AuthCredentialsStoreMode; use codex_core::auth::AuthDotJson; use codex_core::auth::save_auth; @@ -692,7 +692,7 @@ pub(crate) async fn exchange_code_for_tokens( refresh_token: String, } - let client = build_login_http_client()?; + let client = build_reqwest_client_with_custom_ca(reqwest::Client::builder())?; info!( issuer = %sanitize_url_for_logging(issuer), redirect_uri = %redirect_uri, @@ -1061,7 +1061,7 @@ pub(crate) async fn obtain_api_key( struct ExchangeResp { access_token: String, } - let client = build_login_http_client()?; + let client = build_reqwest_client_with_custom_ca(reqwest::Client::builder())?; let resp = client .post(format!("{issuer}/oauth/token")) .header("Content-Type", "application/x-www-form-urlencoded") diff --git a/codex-rs/rmcp-client/Cargo.toml b/codex-rs/rmcp-client/Cargo.toml index 7393368b3..4b20e9d6e 100644 --- a/codex-rs/rmcp-client/Cargo.toml +++ b/codex-rs/rmcp-client/Cargo.toml @@ -13,6 +13,7 @@ axum = { workspace = true, default-features = false, features = [ "http1", "tokio", ] } +codex-client = { workspace = true } codex-keyring-store = { workspace = true } codex-protocol = { workspace = true } codex-utils-pty = { workspace = true } diff --git a/codex-rs/rmcp-client/src/rmcp_client.rs b/codex-rs/rmcp-client/src/rmcp_client.rs index f679c077b..97514ba20 100644 --- a/codex-rs/rmcp-client/src/rmcp_client.rs +++ b/codex-rs/rmcp-client/src/rmcp_client.rs @@ -9,6 +9,7 @@ use std::time::Duration; use anyhow::Result; use anyhow::anyhow; +use codex_client::build_reqwest_client_with_custom_ca; use futures::FutureExt; use futures::StreamExt; use futures::future::BoxFuture; @@ -99,6 +100,11 @@ impl StreamableHttpResponseClient { } } +fn build_http_client(default_headers: &HeaderMap) -> Result { + let builder = apply_default_headers(reqwest::Client::builder(), default_headers); + Ok(build_reqwest_client_with_custom_ca(builder)?) +} + #[derive(Debug, thiserror::Error)] enum StreamableHttpResponseClientError { #[error("streamable HTTP session expired with 404 Not Found")] @@ -922,9 +928,7 @@ impl RmcpClient { let http_config = StreamableHttpClientTransportConfig::with_uri(url.clone()) .auth_header(access_token); - let http_client = - apply_default_headers(reqwest::Client::builder(), &default_headers) - .build()?; + let http_client = build_http_client(&default_headers)?; let transport = StreamableHttpClientTransport::with_client( StreamableHttpResponseClient::new(http_client), http_config, @@ -940,9 +944,7 @@ impl RmcpClient { http_config = http_config.auth_header(bearer_token); } - let http_client = - apply_default_headers(reqwest::Client::builder(), &default_headers) - .build()?; + let http_client = build_http_client(&default_headers)?; let transport = StreamableHttpClientTransport::with_client( StreamableHttpResponseClient::new(http_client), @@ -1130,8 +1132,7 @@ async fn create_oauth_transport_and_runtime( StreamableHttpClientTransport>, OAuthPersistor, )> { - let http_client = - apply_default_headers(reqwest::Client::builder(), &default_headers).build()?; + let http_client = build_http_client(&default_headers)?; let mut oauth_state = OAuthState::new(url.to_string(), Some(http_client.clone())).await?; oauth_state diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 594acb33d..230dd6548 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -33,6 +33,7 @@ codex-app-server-protocol = { workspace = true } codex-arg0 = { workspace = true } codex-backend-client = { workspace = true } codex-chatgpt = { workspace = true } +codex-client = { workspace = true } codex-cloud-requirements = { workspace = true } codex-core = { workspace = true } codex-feedback = { workspace = true } diff --git a/codex-rs/tui/src/voice.rs b/codex-rs/tui/src/voice.rs index 227d27c88..7e4d8a85e 100644 --- a/codex-rs/tui/src/voice.rs +++ b/codex-rs/tui/src/voice.rs @@ -1,6 +1,7 @@ use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use base64::Engine; +use codex_client::build_reqwest_client_with_custom_ca; use codex_core::auth::AuthCredentialsStoreMode; use codex_core::config::Config; use codex_core::config::find_codex_home; @@ -791,7 +792,8 @@ async fn transcribe_bytes( duration_seconds: f32, ) -> Result { let auth = resolve_auth().await?; - let client = reqwest::Client::new(); + let client = build_reqwest_client_with_custom_ca(reqwest::Client::builder()) + .map_err(|error| format!("failed to build transcription HTTP client: {error}"))?; let audio_bytes = wav_bytes.len(); let prompt_for_log = context.as_deref().unwrap_or("").to_string(); let (endpoint, request) = diff --git a/docs/config.md b/docs/config.md index 8adc3c660..d03fb9843 100644 --- a/docs/config.md +++ b/docs/config.md @@ -36,23 +36,27 @@ Codex stores the SQLite-backed state DB under `sqlite_home` (config key) or the `CODEX_SQLITE_HOME` environment variable. When unset, WorkspaceWrite sandbox sessions default to a temp directory; other modes default to `CODEX_HOME`. -## Login Custom CA Certificates +## Custom CA Certificates -Browser login and device-code login can trust a custom root CA bundle when -enterprise proxies or gateways intercept TLS. +Codex can trust a custom root CA bundle for outbound HTTPS and secure websocket +connections when enterprise proxies or gateways intercept TLS. This applies to +login flows and to Codex's other external connections, including Codex +components that build reqwest clients or secure websocket clients through the +shared `codex-client` CA-loading path and remote MCP connections that use it. Set `CODEX_CA_CERTIFICATE` to the path of a PEM file containing one or more -certificate blocks to use a login-specific CA bundle. If `CODEX_CA_CERTIFICATE` -is unset, login falls back to `SSL_CERT_FILE`. If neither variable is set, -login uses the system root certificates. +certificate blocks to use a Codex-specific CA bundle. If +`CODEX_CA_CERTIFICATE` is unset, Codex falls back to `SSL_CERT_FILE`. If +neither variable is set, Codex uses the system root certificates. `CODEX_CA_CERTIFICATE` takes precedence over `SSL_CERT_FILE`. Empty values are treated as unset. The PEM file may contain multiple certificates. Codex also tolerates OpenSSL `TRUSTED CERTIFICATE` labels and ignores well-formed `X509 CRL` sections in the -same bundle. If the file is empty, unreadable, or malformed, login fails with a -user-facing error that points back to these environment variables. +same bundle. If the file is empty, unreadable, or malformed, the affected Codex +HTTP or secure websocket connection reports a user-facing error that points +back to these environment variables. ## Notices