Repository navigation
prek, tealdeer: trust the system CA store by default - #1131
Conversation
prek: patch the unset PREK_NATIVE_TLS case to native TLS when /etc/ssl/certs/ca-certificates.crt exists (an explicit 0/1 still wins; a grep fails the build if upstream rewrites the code). tealdeer: build only the rustls-with-native-roots backend, which makes it the config default. Either way a CA removed from ca-certificates (CVE-2026-111943) stops being trusted without a rebuild. Re-applied from the 10-20 sysca/build branch (144d3d0) onto current main: keeps main's Cargo.lock security-floor checks and --locked builds, and tealdeer 1.9.1. Targets verified unchanged at prek v0.5.7 (http.rs .unwrap_or(false)) and tealdeer v1.9.1 (rustls-with-native-roots feature, 'Unsupported TLS backend' error). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reached
This review includes 4 billable files and costs up to $1.00.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 17 seconds for your next included review. View limit details
|
There was a problem hiding this comment.
The tealdeer change is good. The rustls-with-native-roots-only build matches the v1.9.1 feature table, and needs.internet puts ca-certificates into its closure. One gap in prek:
packages/prek/build.ncl:27-30: addca-certificatestoruntime_deps. prek declares noneeds.internetand none of its runtime deps bring in the store, so a session or task with prek and no curl/git has no/etc/ssl/certs/ca-certificates.crt. The patched default (Path::new(...).is_file()) then quietly falls back to the compiled-in webpki roots, which is exactly what this PR is meant to stop. AGENTS.md "Trust store → Declare the store when you read it" asks for the edge, and #1130/#1133/#1134/#1135 all add it. Thesystem_ca_defaulttest would not catch this, because it putsca-certificates(and git, which brings curl) intest_deps. Right nowca-certificatesis imported only for that test.
Everything else checks out. The sed target .unwrap_or(false); is the only such line in crates/prek/src/http.rs at v0.5.7, and the grep guard fails the build if upstream rewrites it.
|
@bryan-minimal one item is still open here, from my review on 10-28: add |
…t (review) The patched PREK_NATIVE_TLS default reads /etc/ssl/certs/ca-certificates.crt, so the store has to be in prek's closure, as #1130/#1133/#1134/#1135 do. Without it a session with prek and no store silently uses the compiled-in roots. ca-certificates leaves system_ca_default's test_deps (base and git do not bring it), so the test's `test -s` line now proves the runtime edge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@mitodrummer thanks, good catch. Done in dbdc66b:
I also merged main (e80e0e8) so the build picks up the tzdata test fix from #1241. |
|
Thanks, the If |
git brings curl, which declares needs.internet, so the [base, git] closure of system_ca_default gets ca-certificates injected (AGENTS.md "Trust store", item 1) and its `test -s` passed with or without prek's edge. Move the check to store_from_runtime_deps with test_deps = [base]: base plus prek's runtime closure has no needs.internet package, so the store can only come from prek's own runtime_deps. system_ca_default keeps git (prek shells out to it) and its comment now says what that closure gets, host network included. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@mitodrummer you're right, and thanks for checking. git → curl →
|
There was a problem hiding this comment.
The code is right now. store_from_runtime_deps (packages/prek/build.ncl:57-72) runs with test_deps = [base], and I walked the runtime closure of base plus prek: nothing in it declares needs.internet, so the store can only come from prek's own edge (:30-32). The system_ca_default comment (:73-81) now says correctly that git brings the store and the network in regardless.
The one open item is CI: the build-bot hasn't posted a minimal build check on 9223355 yet, so the new test has never run. I'll approve once it passes on both arches.
There was a problem hiding this comment.
The last open item was the build: minimal build is green on 9223355, and the other checks pass. The store_from_runtime_deps test with test_deps = [base] is in the diff, as checked last round. Approving.
Part of gominimal/pkgmgr-rs#1026 (prefer the system trust store), following the CVE-2026-111943 (eMudhra) response.
Why: both tools trust a root list compiled into the binary, so a CA removed from
ca-certificatesstays trusted until they're rebuilt. The Cargo.lock floors on main keep the bundled list current only as of the last rebuild.prek 0.5.7
PREK_NATIVE_TLS=1. This patches the unset case to "on when/etc/ssl/certs/ca-certificates.crtexists". Without that file, native TLS would trust nothing.PREK_NATIVE_TLS=0or1still wins.grepfails the build if upstream rewrites the code, rather than silently dropping the default.crates/prek/src/http.rsstill ends that expression with.unwrap_or(false);at v0.5.7.tealdeer 1.9.1
rustls-with-native-rootsbackend (--no-default-features --features …). Upstream's default compiles both rustls backends and picks webpki first.rustls-with-webpki-rootsnow fails with "Unsupported TLS backend", instead of quietly trusting the compiled-in list.tls_backendnames and that error string at v1.9.1.Tests: each package gets a standalone
system_ca_defaulttest with a negative control:PREK_NATIVE_TLS=0must still select the compiled-in roots;This is the 10-20
sysca/buildbranch re-applied onto current main. Main's Cargo.lock security-floor checks and--lockedbuilds are kept. (deno and claude-code were done in #956; uv is #1130.)🤖 Generated with Claude Code