PR #688 - preloopdev/preloop

Stacks on #685; retarget to main after it merges. (Rebased onto 793573744; the conflicts this file previously showed are resolved.)

Closes audit rows #8, #11, #12, #13, #17 from the blocking-call sweep of 44d51b523 — the remaining "one hung external dependency" edges around platform TLS and the OS credential store.

What was unbounded (audit → file:line)

Row	Location	Was
#12	`crates/preloop-observability/src/lib.rs` (exporter client ~652-660, fallbacks ~853-922)	Blocking reqwest client with default TLS: on macOS every handshake goes through `rustls-platform-verifier` → `SecTrustEvaluateWithError` → `trustd`/`securityd`, with no deadline on the exporter thread
#13	`crates/preloop-runner/src/client/http.rs:68-72`	Same platform verifier; the 100 s/30 s timeouts cannot preempt a synchronous trust-daemon block inside the handshake
#17	`crates/preloop-runner/src/main.rs:127-129`	`lint` remote-workflow client: platform verifier and no timeout at all
#11	`crates/preloop-runner-server/src/credential_store.rs:395-404`	`security delete-generic-password … .status()` inside the guard thread: the caller's 10 s guard abandoned the thread, but the thread and the `security` child lived on forever when `securityd` wedged
#8	`crates/preloop-runner-server/src/secrets_api.rs:224-256, 327-393`; `execution_protection_api.rs:112-145`	Sync `load_config_from`/`write_config_to` (which reach the keychain for the sealing key, `config.rs:1614-1624, 2213-2350`) ran on tokio workers while `secret_mutation`/`policy_mutation` were held — a 10 s guard wait parked a worker

What changed

#12 ObservabilityConfig::with_otel_client_builder(fn(reqwest::blocking::ClientBuilder) -> …) lets the host install the workspace roots on the exporters' blocking client (shared_http::with_tls_roots_blocking in the engine/server binaries, the runner's own installer in preloop-runner). The customizer runs on the dedicated thread that builds the client, outside any runtime. Without it the old platform-verifier default stands (unchanged for embedders who pass nothing).
#13/#17 new preloop_runner::client::tls: one process-wide root set — bundled Mozilla (webpki-roots), the OS store read once on a detached thread with a 5 s deadline, then the PRELOOP_GITHUB_CA_FILE/SSL_CERT_FILE bundles — installed with use_preconfigured_tls on every runner client (async engine/GitHub client, control client, lint). rustls + rustls-native-certs + webpki-roots were already declared dependencies of that crate; no manifest changes anywhere.
#17 REMOTE_WORKFLOW_REQUEST_TIMEOUT 30 s / REMOTE_WORKFLOW_CONNECT_TIMEOUT 10 s, matching the engine's GitHub client; a reusable-workflow fetch is a small metadata call, so nothing legitimate needs longer.
#13 HTTP_REQUEST_TIMEOUT 100 s / HTTP_CONNECT_TIMEOUT 30 s are now named consts documenting why 100 s (the message-poll window in client/broker.rs, MESSAGE_POLL_TIMEOUT).
#11 the probe add and its cleanup delete both run through preloop_observability::deadline::run_command_blocking (this PR is stacked on #685): a wedged security is SIGKILLed and reaped at the deadline. PROBE_TIMEOUT stays 5 s, the cleanup gets PROBE_CLEANUP_TIMEOUT = 2 s so PROBE_TIMEOUT + PROBE_CLEANUP_TIMEOUT < OS_STORE_TIMEOUT, and the timeout error names the probe and its deadline; the old "killed the hung security process" text keeps the operator hint about the locked keychain.
#8 new config_blocking module. Main's config_mutation (a parking_lot lock shared with engine-hosted App setup) must cover a whole load → mutate → persist → publish sequence and cannot span an await, so the mutating handlers (set_secret, delete_secret, mutate_policy) now run that entire locked section — guard included — through deadline::run_blocking (CONFIG_IO_DEADLINE 60 s, deliberately far above the 10 s OS-store guard because blocking work cannot be cancelled and a tight bound could report a failed mutation that still lands). The async secret_mutation/policy_mutation guards still span the section, ApiError 404/409/400 behaviour is unchanged, and the read path (load_policy) is async + off-worker. Net effect: a 10 s keychain wait costs a blocking-pool thread instead of an async worker, with main's stale-write protection intact.

Runner/CLI hosts also preload the root set on a blocking thread at startup so the (bounded) OS-store read never lands on an async worker.

Why the root set is re-expressed per crate (and not hoisted to one crate)

The audit's preferred fix was to move shared_http::tls_roots down into a crate every TLS client depends on (preloop-observability). That needs rustls/rustls-native-certs/webpki-root-certs added to that crate's manifest, and this repo's supply-chain isolation rule (.github/workflows/supply-chain.yml, AGENTS.md) fails any PR that carries Cargo.lock/manifests together with source. So this PR adds no dependencies and expresses the same policy where each crate already has the needed crates: shared_http remains the canonical engine implementation (now also exposing with_tls_roots_blocking for the exporter and a root_certs_from(env) seam so the behavioural test needs no process-env mutation), and preloop_runner::client::tls mirrors it with the identical three sources and 5 s OS-store deadline. Follow-up option if wanted: once a dependency-only PR lands, the runner copy and the observability customizer can collapse into preloop-observability::tls_roots.

Tests

shared_http::tests::blocking_client_trusts_the_shared_roots_including_the_env_ca_bundle — behavioural: generates a CA + localhost leaf in-process (rcgen, already a dependency of that crate), serves HTTPS on loopback, and asserts the client built from the shared roots completes the handshake when the CA arrives via PRELOOP_GITHUB_CA_FILE and is rejected without it. Fail-before: neutering the root installation makes it fail with InvalidCertificate(certificate is not trusted) — verified locally, then restored.
client::tls::tests::client_config_trusts_the_ca_bundle_over_tls — same for the runner's use_preconfigured_tls path, proving the explicit --ca-bundle/SSL_CERT_FILE CA is still honoured after leaving the platform verifier.
client::tls::tests::env_ca_bundle_is_loaded_and_missing_files_are_ignored, …::os_root_store_that_never_answers_is_abandoned_at_the_deadline, shared_http::tests::os_root_store_that_never_answers_is_abandoned_at_the_deadline — the bundled/OS-store/bundle composition and the 5 s OS-store bound.
#11's kill-at-deadline mechanism is now deadline::run_command_blocking and is covered by #685's blocking_command_is_killed_at_the_deadline; this PR therefore carries no duplicate copy of that test (the two local wait_bounded tests were removed on rebase).
No test for #17's timeout wiring (it is reqwest's own client timeout on a bin-local builder); #8's off-worker offload is structural (run_blocking), not timing-testable without flakiness.

Gates

cargo fmt --all --check — clean.
cargo clippy --locked -p preloop-observability -p preloop-runner -p preloop-runner-server --all-targets -- -D warnings — clean.
Focused on the rebased head: shared_http:: 5/5, credential_store/secrets_api/execution_protection* 82/82, client::tls 3/3 — all pass.
Earlier head (ca2d29776, pre-rebase): full cargo test -p preloop-observability -p preloop-runner -p preloop-runner-server — observability and runner suites green; runner-server 1232 passed / 1 failed, dispatch_tests::select_app_for_repo_picks_the_app_installed_on_the_owner, which passes in isolation (order/interference flake in a file this PR does not touch).
Rebased onto fix/bound-shared @ 793573744 (group-kill in deadline::run_command*): conflicts in secrets_api.rs and execution_protection_api.rs only, resolved as described above and re-gated (fmt clean, clippy clean, focused suites above green). Head 33b35585a.

Not in this PR

The preloop serve (CLI) path constructs its ObservabilityConfig in crates/preloop-cli/src/main.rs, so its one-line .with_otel_client_builder(preloop_runner_server::shared_http::with_tls_roots_blocking) is carried by the CLI slice stacked on this branch: #689 (head 375901ce2), so row #12 is closed for the preloop serve path there rather than pending a follow-up. This PR carries no Cargo.toml/Cargo.lock changes.