Coalesce overlapping native TLS root certificate loads (#54056)

## Why

Concurrent TLS client builds can independently load the same platform trust store. Share overlapping loads while preserving fresh trust reads for later builds.

## What changed

- Share one in-flight native root load without reusing completed loads.
- Bypass sharing when `SSL_CERT_FILE` or `SSL_CERT_DIR` is set, and continue reading custom CA bundles independently for every build.

## Testing

Add tests for overlapping loads, fresh trust after completion, and recovery from a panicking loader. Extend TLS handshake coverage to verify that replacing a custom CA file affects new configurations while existing configurations retain their trust, and that empty or removed files produce errors.

GitOrigin-RevId: 1465ef3faf82f53651d9dbfe314341b17346580b

codex-rs/http-client/src/custom_ca.rs

@@ -6,6 +6,8 @@
 //! `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.
+//! Concurrent native-root reads may share one in-flight load; later builds reload trust, and
+//! custom CA files are read independently for every build.
 //!
 //! The module intentionally has a narrow responsibility:
 //!
@@ -49,7 +51,6 @@ 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;
@@ -247,16 +248,7 @@ fn build_rustls_client_config(
     // 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 mut root_store = (*crate::native_roots::load()).clone();
 
     if let Some(bundle) = bundle {
         let certificates = bundle.load_certificates()?;

codex-rs/http-client/src/custom_ca_tls_tests.rs

@@ -1,4 +1,4 @@
-//! Handshake coverage for the custom trust path retained by Windows realtime connections.
+//! Handshake and fresh-file-read coverage for shared custom trust configuration.
 
 use super::CODEX_CA_CERT_ENV;
 use super::ConfiguredCaBundle;
@@ -26,11 +26,11 @@ fn custom_intermediate_trust_preserves_hostname_validation() {
     let temp = tempfile::TempDir::new().unwrap();
     let path = temp.path().join("intermediate.pem");
     std::fs::write(&path, intermediate.pem()).unwrap();
-    let config = build_rustls_client_config(Some(&ConfiguredCaBundle {
+    let bundle = ConfiguredCaBundle {
         source_env: CODEX_CA_CERT_ENV,
         path,
-    }))
-    .unwrap();
+    };
+    let config = build_rustls_client_config(Some(&bundle)).unwrap();
     let server = Arc::new(
         rustls::ServerConfig::builder()
             .with_no_client_auth()
@@ -42,7 +42,7 @@ fn custom_intermediate_trust_preserves_hostname_validation() {
         handshake(config.clone(), server.clone(), "localhost"),
         Ok(())
     );
-    let error = handshake(config, server, "wrong.example").unwrap_err();
+    let error = handshake(config.clone(), server.clone(), "wrong.example").unwrap_err();
     assert!(
         matches!(
             error,
@@ -53,6 +53,27 @@ fn custom_intermediate_trust_preserves_hostname_validation() {
         ),
         "{error:?}"
     );
+
+    std::fs::write(&bundle.path, include_str!("../tests/fixtures/test-ca.pem")).unwrap();
+    let replaced = build_rustls_client_config(Some(&bundle)).unwrap();
+    assert!(matches!(
+        handshake(replaced, server.clone(), "localhost"),
+        Err(rustls::Error::InvalidCertificate(
+            rustls::CertificateError::UnknownIssuer
+        ))
+    ));
+    assert_eq!(handshake(config, server, "localhost"), Ok(()));
+
+    std::fs::write(&bundle.path, "").unwrap();
+    assert!(matches!(
+        build_rustls_client_config(Some(&bundle)),
+        Err(super::BuildCustomCaTransportError::InvalidCaFile { .. })
+    ));
+    std::fs::remove_file(&bundle.path).unwrap();
+    assert!(matches!(
+        build_rustls_client_config(Some(&bundle)),
+        Err(super::BuildCustomCaTransportError::ReadCaFile { .. })
+    ));
 }
 
 fn handshake(

codex-rs/http-client/src/lib.rs

@@ -5,6 +5,7 @@ mod client_builder;
 mod client_tls;
 mod custom_ca;
 mod error;
+mod native_roots;
 mod network_policy;
 mod outbound_proxy;
 mod request;

codex-rs/http-client/src/native_roots.rs

@@ -0,0 +1,68 @@
+//! Coalesces overlapping platform-root reads without retaining completed trust snapshots.
+//! Later callers always reload. File/directory overrides bypass coalescing so their contents
+//! are reread independently; custom Codex CA bundles are handled by the caller.
+
+use std::sync::Arc;
+use std::sync::Mutex;
+use std::sync::OnceLock;
+use std::sync::Weak;
+
+use rustls::RootCertStore;
+
+#[derive(Default)]
+struct NativeRootsLoader {
+    in_flight: Mutex<Weak<OnceLock<Arc<RootCertStore>>>>,
+}
+
+impl NativeRootsLoader {
+    fn load(&self, load: impl FnOnce() -> RootCertStore) -> Arc<RootCertStore> {
+        let pending = {
+            let mut in_flight = self
+                .in_flight
+                .lock()
+                .unwrap_or_else(std::sync::PoisonError::into_inner);
+            match in_flight
+                .upgrade()
+                .filter(|pending| pending.get().is_none())
+            {
+                Some(pending) => pending,
+                None => {
+                    let pending = Arc::new(OnceLock::new());
+                    *in_flight = Arc::downgrade(&pending);
+                    pending
+                }
+            }
+        };
+        Arc::clone(pending.get_or_init(|| Arc::new(load())))
+    }
+}
+
+pub(crate) fn load() -> Arc<RootCertStore> {
+    // rustls-native-certs treats these as overrides of the platform store.
+    // Preserve fresh file reads even when several callers construct clients together.
+    if std::env::var_os("SSL_CERT_FILE").is_some() || std::env::var_os("SSL_CERT_DIR").is_some() {
+        return Arc::new(load_roots());
+    }
+    static LOADER: NativeRootsLoader = NativeRootsLoader {
+        in_flight: Mutex::new(Weak::new()),
+    };
+    LOADER.load(load_roots)
+}
+
+fn load_roots() -> RootCertStore {
+    let rustls_native_certs::CertificateResult { certs, errors, .. } =
+        rustls_native_certs::load_native_certs();
+    if !errors.is_empty() {
+        tracing::warn!(
+            native_root_error_count = errors.len(),
+            "encountered errors while loading native root certificates"
+        );
+    }
+    let mut roots = RootCertStore::empty();
+    let _ = roots.add_parsable_certificates(certs);
+    roots
+}
+
+#[cfg(test)]
+#[path = "native_roots_tests.rs"]
+mod tests;

codex-rs/http-client/src/native_roots_tests.rs

@@ -0,0 +1,55 @@
+//! Verifies overlapping root loads, fresh trust generations, and panic recovery.
+
+use super::*;
+use pretty_assertions::assert_eq;
+use rustls_pki_types::CertificateDer;
+use rustls_pki_types::pem::PemObject;
+use std::sync::mpsc;
+use std::thread;
+
+#[test]
+fn concurrent_loads_share_roots_but_completed_loads_are_never_reused() {
+    let loader = NativeRootsLoader::default();
+    let (entered, started) = mpsc::channel();
+    let (release, released) = mpsc::channel();
+    thread::scope(|scope| {
+        let first_loader = &loader;
+        let first = scope.spawn(move || {
+            first_loader.load(|| {
+                entered.send(()).unwrap();
+                released.recv().unwrap();
+                RootCertStore::empty()
+            })
+        });
+        started.recv().unwrap();
+        let pending = loader.in_flight.lock().unwrap().upgrade().unwrap();
+        let second = scope.spawn(|| loader.load(|| panic!("overlapping load must be shared")));
+        // Wait for the second caller to own this generation before allowing it to finish.
+        while Arc::strong_count(&pending) < 3 {
+            thread::yield_now();
+        }
+        drop(loader.in_flight.lock().unwrap());
+        release.send(()).unwrap();
+        let first = first.join().unwrap();
+        let second = second.join().unwrap();
+        assert!(Arc::ptr_eq(&first, &second));
+
+        // Keep the old generation alive: completion, not refcount expiry, must force a reload.
+        let updated = loader.load(|| {
+            let mut roots = RootCertStore::empty();
+            roots.add_parsable_certificates(
+                CertificateDer::pem_slice_iter(include_bytes!("../tests/fixtures/test-ca.pem"))
+                    .map(Result::unwrap),
+            );
+            roots
+        });
+        assert_eq!((first.len(), second.len(), updated.len()), (0, 0, 1));
+    });
+}
+
+#[test]
+fn panicking_load_does_not_poison_later_trust_reads() {
+    let loader = NativeRootsLoader::default();
+    assert!(std::panic::catch_unwind(|| loader.load(|| panic!("interrupted load"))).is_err());
+    assert!(loader.load(RootCertStore::empty).is_empty());
+}