refactor(observability): one native TLS root loader for the engine and the runner
Stacked PR. Base: fix/bound-shared, the current bound-* stack head (includes #685's deadline module, #688's TLS clients, and current main). The real diff is 8 files. It touches no Cargo.toml/Cargo.lock, as the dependency/policy guard requires. Rebase instructions at the bottom.
What this does
The native TLS root policy existed twice:
crates/preloop-runner-server/src/shared_http.rs— for the engine, CLI, orchestrator, and runner-client clients (tls_certs_only);crates/preloop-runner/src/client/tls.rs— the same policy re-expressed for the runner's rustls config (use_preconfigured_tls), becausepreloop-runnerdoes not depend onpreloop-runner-server.
Both are gone. The single implementation is crates/preloop-observability/src/tls.rs (preloop_observability::tls), which both binaries already depend on:
| was | is now |
|---|---|
shared_http::{tls_roots, with_tls_roots, with_tls_roots_blocking} (+ the loader) | preloop_observability::tls::{root_certs, with_roots, with_roots_blocking}, called through the engine-facing names in shared_http so no call site changes |
shared_http::{github_client_builder, CLIENT} | unchanged in shared_http (engine-specific timeouts and user-agent) |
preloop_runner::client::tls::{client_config, preload_roots, with_shared_roots_blocking} | preloop_observability::tls::{client_config, preload_roots, with_client_config_blocking}; the runner module is deleted |
shared_http.rs keeps its public API (tls_roots, with_tls_roots, with_tls_roots_blocking, github_client_builder, CLIENT) and delegates; preloop-runner's client::tls module no longer exists (client/mod.rs and the two main.rs call sites and client/http.rs point at the shared module).
Behaviour is identical
- Same three sources, in the same order: bundled Mozilla anchors (
webpki-root-certs), the OS store, then thePRELOOP_GITHUB_CA_FILE/SSL_CERT_FILEbundles. The one deliberate data change is the point of the PR: the runner now trusts thewebpki-root-certssnapshot the engine already ships instead of its ownwebpki-roots 0.26snapshot, so there is one Mozilla snapshot to update rather than two that can drift (#695 records why this crate won). Everything added on top — OS store and planted CAs — is the same set the runner already had. - Same 5 s OS-store deadline, now the shared one. The loader no longer hand-rolls a third
mpsc+ detached-thread wait: it runs the store read throughpreloop_observability::deadline::DeadlineGuard(the workspace's single bounded-blocking implementation from #685), which detaches the worker on expiry and returns the caller-supplied "unresponsive" text. Timeout, spawn failure, and a panicking loader all warn and continue with the other roots, exactly as before. - Same
tls_certs_onlysemantics:with_roots/with_roots_blockingstill build the reqwestCertificatelist from the DER roots (skipping anything reqwest rejects) and install it withtls_certs_only, so reqwest's own rustls config, not the platform verifier, terminates the handshake. - Same preconfigured rustls config:
client_configstill usesring, still advertiseshttp/1.1ALPN, still merges the caller's--ca-bundlePEM, and still resolves the roots (and the bounded OS-store read) at first use. The runner's OTLP installer is still fail-open: a config that cannot be built logs and leaves the platform verifier in place rather than dropping the observability handle. - Same logs: per-source (
loaded OS root certificates, per-bundle counts, "reading the OS certificate store") plus the summary line naming the bundled and total counts.
Tests moved, none dropped
The loader's tests moved to preloop-observability/src/tls.rs and now cover both sides' cases in one place: PEM bundle parsing, missing file, invalid PEM, the PRELOOP_GITHUB_CA_FILE lookup path, the hung-store deadline, the loopback handshake through client_config, and the reqwest blocking client trusting a CA that arrives through the environment override. The in-process CA/leaf fixtures (rcgen) are the ones #688 added; no test needs the openssl CLI. shared_http.rs keeps a delegation test asserting its tls_roots() is the shared list.
Gates
| gate | result |
|---|---|
cargo fmt --all --check | clean |
cargo clippy --locked -p preloop-observability -p preloop-runner -p preloop-runner-server -p preloop-cli --all-targets -- -D warnings | clean |
cargo test -p preloop-observability --lib | 52 passed, incl. all 7 tls loader/handshake tests |
cargo test -p preloop-runner-server --lib | 1229 passed, 3 ignored, incl. the shared_http delegation test |
cargo test -p preloop-runner --lib | 668 passed, 1 ignored |
supply-chain/guard-policy-files.sh (the main version, run against deps/observability-tls-stacked..HEAD) | Policy file guard: OK (no policy or manifest files modified) |
The base branch predates #666, which moved the guard out of the workflow and into supply-chain/guard-policy-files.sh; the rule it applies to this diff is the same, and the PR touches no Cargo.* file, so both the old inline check and the current script accept it.
Follow-ups (not in this PR)
preloop-runnerno longer usesrustls,rustls-native-certs, orwebpki-roots, andpreloop-runner-serverno longer usesrustls-native-certsorwebpki-root-certs. Dropping them is a policy-only PR after this one lands: todaymainand #688 still use them (the guard's split is what keeps those out of here).CHANGELOG.mdis untouched: this is a behaviour-preserving move, and the stack's refactor commits (#685'srefactor(observability)commit) set that precedent.
Rebase after the bound stack lands on main
git fetch public
git checkout refactor/shared-tls-roots
git rebase --onto public/main public/fix/bound-shared refactor/shared-tls-roots
git push --no-verify --force-with-lease public refactor/shared-tls-roots
gh pr edit 697 --base main
This branch is already rebased onto fix/bound-shared; the rebase replays only the two TLS commits. deps/observability-tls-stacked can then be deleted.
Summary by cubic
Consolidates the native TLS root policy into a single implementation in preloop_observability::tls, so the engine and the runner no longer keep two copies of the same security-relevant loader. The engine's shared_http keeps its public API and delegates; the runner's client::tls is deleted and its call sites point at the shared module.
Behavior is unchanged: same three sources in the same order (bundled Mozilla anchors, OS store with a 5 s deadline, environment CA bundles), same rustls config and tls_certs_only semantics, same fail-open OTLP installer. One deliberate data change: the runner now trusts the engine's webpki-root-certs snapshot instead of its own webpki-roots 0.26 snapshot, so there is one Mozilla snapshot to update instead of two that can drift. The OS-store wait uses the workspace's DeadlineGuard instead of a hand-rolled detached-thread wait. The loader's tests move with it and now use in-process rcgen fixtures, so no test needs the openssl CLI.
Two follow-up fixes from the review round landed on this branch: the parsed root set is now cached in a LazyLock — the first pass re-ran the anchor parse on every client build — and environment-bundle certificates are filtered through the same usable_anchors check as the OS store. A malformed cert in PRELOOP_GITHUB_CA_FILE/SSL_CERT_FILE is now dropped with a log instead of breaking consumers that add roots strictly, like the Postgres connector.
No Cargo.toml/Cargo.lock changes; dropping the now-unused TLS dependencies is a follow-up policy-only PR.