Repository navigation
trust store: declare ca-certificates on ruby, typst, atuin, gws, skopeo; document where the store comes from - #957
Conversation
…ere the store comes from ruby, typst, atuin, gws and skopeo verify TLS against /etc/ssl/certs but had no ca-certificates in their runtime closure; they only saw a store when something else in the environment (curl, git, a needs.internet package) brought it in. Add the edge, plus a ruby standalone test that every live root verifies through OpenSSL's default paths. AGENTS.md 8000 gains a "Trust store" section: how the store reaches an environment (needed_for_internet injection vs a declared edge), that the planner's top-level HACK only builds it, why python/perl/openssl do not take the edge, and the no-embedded-store / one-denylist rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reached
This review includes 6 billable files and costs up to $1.50.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 25 minutes for your next included review. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title accurately identifies the main change: adding the ca-certificates runtime dependency to five packages and documenting the trust store source. It is specific and related to the changeset, alt… |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Title check
-
Explanation
The title accurately identifies the main change: adding the ca-certificates runtime dependency to five packages and documenting the trust store source. It is specific and related to the changeset, although it is somewhat long.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (6 skipped: 6 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
Approving with nits. Verified: gws 0.22.5 resolves reqwest rustls-tls-native-roots (rustls-native-certs + openssl-probe, no webpki-roots in its lockfile); typst 0.15.1's system-downloader links the system openssl and we don't enable vendor-openssl; atuin already carried openssl as a runtime dep; ruby links our openssl, built with --openssldir=/etc/ssl. The ruby trust_store test reads correctly: the 100-root floor prevents a vacuous pass, the m%" string leaves Ruby's #{} alone, and a malformed cert raises rather than passes.
Should fix (doc only)
- AGENTS.md "One denylist": main currently has two lists,
packages/ca-certificates/distrust.txtandcve-2026-111943-blocked-sha256.txt(the JDK patch reads the second through its own import). Following the rule as written at the next incident would append to one file and leave the JDKs unpatched. Describe both files as they are and say they should converge, or drop the sentence until they do.
Nits
- PR body is stale: #948 and #949 merged this morning, so "merge after #948" is satisfied and the test no longer fails against main.
- The
roots.size < 100guard counts all roots; a wrong sandbox clock could emptyliveand the test would verify nothing. Guardlive.sizeinstead. - AGENTS.md "so OpenSSL, curl, git, Python's ssl, Ruby, Go and rustls-native-certs all look for roots there": Go's crypto/x509 and openssl-probe check /etc/ssl/certs on their own, not because of openssldir.
- In each
runtime_deps,ca-certificatesis placed ahead of glibc, which every touched file lists first; the import sits in the build-tool import block rather than the runtime-dep block.
Review follow-ups (mitodrummer): - AGENTS.md: replace "One denylist" with the two lists as they are on main: distrust.txt (ca-certificates build gate, the Chromium policy checker, pkgmgr truststores) and cve-2026-111943-blocked-sha256.txt via cve-2026-111943.ncl (the JDK patch, which needs all 48 certificate fingerprints). They should converge (gominimal/pkgmgr-rs#1020); until then an incident lands in both. - AGENTS.md: Go's crypto/x509 and openssl-probe probe /etc/ssl/certs on their own; only OpenSSL and what links it go through openssldir. - ruby trust_store test: the 100-root floor counts live roots, so a wrong sandbox clock cannot make the test verify nothing. - atuin, gws, ruby, skopeo, typst: ca-certificates after glibc in runtime_deps, and its import in the runtime-dep import block. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @mitodrummer. Addressed in 6acc40e, plus a merge of main (c98d29f). Should fix (doc)
Nits
Spec-hash impact of the reorder (
🤖 Generated with Claude Code |
…um 155.0.8059.84) One conflicted file: - packages/ruby/build.ncl: both sides added a runtime dep after glibc and a standalone test after `eval`. Kept both. runtime_deps is glibc, ca-certificates (main's #957 placement, "after glibc", with its comment), then this branch's glibc-locale-c-utf8. Tests keep this branch's utf8_default_external and main's trust_store, in that order; the import block already had both imports from the auto-merge. Everything else merged cleanly. chromedriver-bin and the ruby-3.x siblings are brought in line with main in the commits that follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in's trust store and build cleanup - chromedriver-bin: 155.0.8059.39 -> 155.0.8059.84, matching main's chromium-headless-shell-bin (#894). The version is no longer a literal: it is chromium-headless-shell-bin.attrs.upstream_version, the same shape as glibc-locale-c-utf8 and rust-arm-embedded. A browser bump now changes the driver's URLs, so the driver's build fails on its sha256s until they move in the same change, instead of shipping a driver that refuses the browser. matches_browser therefore checks the driver binary, the browser binary and the browser package's declared version against each other. CfT zips: linux64 4fe24330..., linux-arm64 738853f0...; both downloads' MD5s match the GCS object x-goog-hash, and both binaries carry the 155.0.8059.84 version string and the BSD-3-Clause LICENSE.chromedriver. - ruby-3.2/3.3/3.4: the drop-in siblings of `ruby` take what main gave it. From #957: ca-certificates in runtime_deps (after glibc) plus the trust_store test, per AGENTS.md "Declare the store when you read it". From #875: delete mkmf.log / gem_make.out after install, since they record gcc's random temp names and break reproducibility. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up to CVE-2026-111943 (eMudhra). This PR is about which environments can see
/etc/ssl/certsat all. The bundle fixes themselves are in #948 and the other SECURITY PRs.When a runtime has no trust store
The semantics, read from minimal
fbf8505f6:Injection.
ca-certificatescarriesattrs.needed_for_internet.Transitives::new_with_seenset(crates/graph/src/transitives.rs:144-154) adds every such package to the closure of any spec withneeds.internet. The check runs at every level of the recursion, so the store also reaches anything that has aneeds.internetpackage (curl, wget, go, rust, uv, bun, …) in its runtime closure, and any build whose build deps include one.Not hashed. The spec hash covers only declared build and runtime edges plus the names of
needs(crates/graph/src/spec_hasher.rs:154-186, 332-340). A new bundle therefore rebuilds nothing that gets the store by injection; today a bump rehashes 10 packages.The planner HACK only builds.
ExecPlan::with_toplevels(crates/graph/src/planner.rs:321-329) addsneeded_for_internetpackages to the build plan's top levels. Nothing mounts the plan's top levels. What gets mounted is the closure:crates/minimald/src/env.rs:489, 547-551;crates/op/src/oci_image.rs:45;crates/op/src/standalone_test.rs:38-45;crates/op/src/specs.rs:108.None of these consult the HACK.
So a runtime has no
/etc/ssl/certswhenever nothing in its environment's closure declaresneeds.internet. Examples:packages = ["python"], or["base", "ruby"].basedoes not bring the store;Sessions that include curl or git, like the repo's default tasks, already have the store. Session network access comes from the session plan (
with_plan(args.network)), not fromneeds, so a session can be online with no store.Today 124 of 621 packages have
ca-certificatesin their runtime closure. With this PR, 130 do.Blast radius, measured
Measured with
MINIMAL_SCIENCE_MODE=1 mip --no-fetch dump --packages --arch amd64, diffing spec hashes against main. "Per future bundle bump" is the rehash count whenca-certificatesitself changes after the option lands.openssl→ca-certificatesca-certificatesbuild-depends onopensslpythonalone /perlaloneThe 11 rehashed by this PR are atuin, bun, dev-language-servers, gws, hunk, opencode, plannotator, ruby, ruby-lsp, skopeo and typst. bun and plannotator come in through ruby. They are also rebuilt by #951; this PR does not touch their files.
What changes
ruby,typst,atuin,gws,skopeo: addca-certificatestoruntime_deps. Each one reads the system store:OPENSSLDIR=/etc/ssl;rustls-tls-native-roots, checked in the 0.22.5Cargo.toml;crypto/x509.atuin also loses a duplicated comment in the same hunk.
ruby: new standalone testtrust_store. Every live per-root file under/etc/ssl/certsmust verify throughOpenSSL::X509::Store#set_default_pathsalone, and a store with no paths must verify none of them (the control). It runs offline.AGENTS.md: new "Trust store" section. It covers where the store comes from and how it reaches an environment (injected vs declared, and that the HACK only builds it). It also gives the rules: why python, perl and openssl don't take the edge, don't useneeds.internetto get the store, don't embed a store, the two denylists as they are today (and that they should converge), and the steps when a CA is distrusted.Not changed here, on purpose
needs.internetpackage. It is better fixed in minimal than with hashed edges. One way: when a session's network plan allows egress, mount theneeded_for_internetpackages too. Another: an unhashed "runtime needs the trust store" edge. minimal is not edited from here; this is a proposal for Bryan.cacerts. An edge alone changes nothing for them.--use-system-ca, node-lts needs the edge (27 rehash). Do that after SECURITY: node, node-lts: backport CVE-2026-111943 eMudhra fixes (nodejs/node#66871, #66877) #946 merges.custom_ca_certs_fileat the bundle./etc/ssl/certsby default, so an edge would do nothing. deno could be switched withDENO_TLS_CA_STORE=system; that would be a separate change.Merge order and caveats
a19e4dc4). The ruby test failure described in the first version of this body came from main's pre-SECURITY: ca-certificates: rebuild from curl 2026-10-20 bundle, drop compromised eMudhra roots (CVE-2026-111943) #948ca-certificates(the stale bundle hash-linked asa94d09e5.0); SECURITY: ca-certificates: rebuild from curl 2026-10-20 bundle, drop compromised eMudhra roots (CVE-2026-111943) #948 removed that link, so the test is expected to pass against main. It has not been run here against main's built tree (see the last bullet).packages/ca-certificates/distrust.txt(cert/SPKI, 53 entries; read by theca-certificatesbuild gate, the Chromium policy checker from SECURITY: chromium-bin: distrust the compromised eMudhra keys via CADistrustedCertificates policy (CVE-2026-111943) #961 andpkgmgr truststores) andcve-2026-111943-blocked-sha256.txtviacve-2026-111943.ncl(48 certificate fingerprints, read by the JDK patch). AGENTS.md now describes both and says they should converge (gominimal/pkgmgr-rs#1020). It no longer claims one list.ca-certificatesnow comes afterglibcin eachruntime_deps, with its import in the runtime-dep import block. The ruby test's 100-root floor counts live roots, so a wrong sandbox clock fails it instead of making it vacuous. AGENTS.md now says Go'scrypto/x509and openssl-probe check/etc/ssl/certsthemselves, not through openssldir.runtime_depsorder is part of the spec hash: the reorder changed the 11 packages' hashes compared with the previous head. Against main (mip dump, amd64 and arm64), the rehash set is still exactly the 11 listed above, so the reorder adds no rebuilds.min checkin a session.🤖 Generated with Claude Code
Summary by CodeRabbit
Enhancements
/etc/ssl/certs.Documentation