refactor(observability): one native TLS root loader for the engine and the runner

Merged
BN
4d ago
+638-705

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), because preloop-runner does not depend on preloop-runner-server.

Both are gone. The single implementation is crates/preloop-observability/src/tls.rs (preloop_observability::tls), which both binaries already depend on:

wasis 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 the PRELOOP_GITHUB_CA_FILE / SSL_CERT_FILE bundles. The one deliberate data change is the point of the PR: the runner now trusts the webpki-root-certs snapshot the engine already ships instead of its own webpki-roots 0.26 snapshot, 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 through preloop_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_only semantics: with_roots/with_roots_blocking still build the reqwest Certificate list from the DER roots (skipping anything reqwest rejects) and install it with tls_certs_only, so reqwest's own rustls config, not the platform verifier, terminates the handshake.
  • Same preconfigured rustls config: client_config still uses ring, still advertises http/1.1 ALPN, still merges the caller's --ca-bundle PEM, 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

gateresult
cargo fmt --all --checkclean
cargo clippy --locked -p preloop-observability -p preloop-runner -p preloop-runner-server -p preloop-cli --all-targets -- -D warningsclean
cargo test -p preloop-observability --lib52 passed, incl. all 7 tls loader/handshake tests
cargo test -p preloop-runner-server --lib1229 passed, 3 ignored, incl. the shared_http delegation test
cargo test -p preloop-runner --lib668 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-runner no longer uses rustls, rustls-native-certs, or webpki-roots, and preloop-runner-server no longer uses rustls-native-certs or webpki-root-certs. Dropping them is a policy-only PR after this one lands: today main and #688 still use them (the guard's split is what keeps those out of here).
  • CHANGELOG.md is untouched: this is a behaviour-preserving move, and the stack's refactor commits (#685's refactor(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.


Devin Review


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.