fix(sdk): verify runtime bundle digest on download by default - #1300
fix(sdk): verify runtime bundle digest on download by default#1300ya-luotao wants to merge 4 commits into
Conversation
The shared installer's default path (setup::install() and the Setup builder without expected_bundle_sha256) downloaded and extracted the msb + libkrunfw release bundle without any integrity check; only CLI self-downgrade wired a digest through. The installer now fetches the digest from the release's checksums.sha256 asset — served by the same releases/download/ endpoint as the bundle, so no GitHub API rate-limit exposure — and verifies it before extraction, fail-closed. An explicit expected_bundle_sha256 still wins and skips the checksums fetch; the CI-local-bundle path is unchanged. Python/Node/Ruby SDK wrappers and the CLI's Unix `msb self update` inherit the verification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WZKrTmBG9RQzn85yQP2bTK
EnsureInstalled downloaded and extracted the msb + libkrunfw release bundle without any integrity check. downloadMsbAndKrunfw now fetches the release's checksums.sha256 asset, hashes the downloaded bundle, and errors before extraction on any mismatch, fail-closed. The checksums asset is served by the same releases/download/ endpoint as the bundle, so this adds no GitHub API calls. WithSkipDownload semantics are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WZKrTmBG9RQzn85yQP2bTK
cd42fd3 to
56da2de
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56da2de2fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let expected_digest = match self.expected_bundle_sha256.clone() { | ||
| Some(digest) => digest, | ||
| None => fetch_bundle_digest(version, &url).await?, | ||
| }; |
There was a problem hiding this comment.
When the default prebuilt feature is enabled—as it is for ordinary Rust consumers and the Python, Node, and Ruby manifests—a fresh source build still downloads and extracts the runtime without a digest in sdk/rust/build.rs:78-84. Because that installs the matching msb and libkrunfw first, Setup::install_bundle subsequently returns through its existing-version check at lines 85-95, so this new expected_digest path never runs and the default download remains unverified. Apply the same checksum validation in the build script or route it through a single verified installer.
AGENTS.md reference: AGENTS.md:L96-L96
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 8a2345b, fail-closed same as the runtime path.
build.rs now fetches the release's checksums.sha256 before downloading and fails the build (before extract_bundle) on a missing checksums asset, a missing bundle entry, or a digest mismatch. The sha256sum entry lookup is shared with the runtime installer via microsandbox_utils::bundle_digest_from_checksums (unit-tested in utils), and the CI-local-bundle branch stays exempt as before.
I chose to verify in the build script rather than route it through the shared Setup installer: build scripts can't depend on the SDK crate itself (cycle), and the build script's ureq/blocking download stack is intentionally lighter than the installer's reqwest/tokio one — duplicating only the ~10-line verify glue keeps that separation while both paths share the same URL builder and checksum parsing from microsandbox-utils.
Verified end-to-end with a fresh MSB_HOME: cargo check -p microsandbox fetched checksums + bundle from the real v0.6.8 release, verified, and extracted a working msb 0.6.8.
The build-time installer in sdk/rust/build.rs (default `prebuilt` feature) downloaded and extracted the release bundle without any integrity check — and because it installs msb + libkrunfw into ~/.microsandbox first, the runtime installer's existing-version early-return meant the new digest verification never ran for source-built consumers. The build script now fetches the release's checksums.sha256 asset and fails the build before extraction on a missing asset, missing entry, or digest mismatch, matching the runtime installer's fail-closed semantics. The CI-local-bundle path stays exempt as before. The sha256sum entry lookup is shared with the runtime installer via microsandbox_utils::bundle_digest_from_checksums. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WZKrTmBG9RQzn85yQP2bTK
The prebuilt build script for the embedded guest agent downloaded agentd-<arch> from the release without any integrity check — the same class of hole superradcompany#1300 closes for the msb bundle, which its two cherry-picked commits (the base of this branch) do not cover. The download now verifies the binary against the release's checksums.sha256 asset before it is embedded, fail-closed on a missing asset, missing entry, or digest mismatch, sharing the sha256sum entry lookup via microsandbox_utils::bundle_digest_from_checksums. The local build/agentd branch stays trusted as a developer build artifact, but an externally supplied binary can now be pinned fail-closed with MSB_AGENTD_SHA256; without the pin a cargo warning notes the exemption. Note: this extends beyond upstream superradcompany#1300's scope (msb bundle only) and exists only on this v0.6.8 backport branch until an equivalent lands upstream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0191N1iCiTVVFsUpmHGKZBH1
1 participant
Follow-up to #1283 (comment): the SDKs download the msb + libkrunfw bundle from GitHub releases without checking it. That covers the Rust installer's default path (
setup::install(), which the Python/Node/Ruby wrappers call), theprebuiltbuild script insdk/rust/build.rs, Go'sEnsureInstalled, and Unixmsb self update. Only the CLI self-downgrade passed a digest through.All of these now verify the bundle's SHA-256 before extraction, fail-closed. The digest comes from the release's
checksums.sha256asset. Everyv*release back to v0.3.0 publishes it, and its values match the APIdigestfield (spot-checked on v0.6.8).Why not the API
digestfield the self-downgrade uses: the default install path makes noapi.github.comcalls today, and unauthenticated API calls are capped at 60/h per IP. Verifying through the API would mean either flaky installs in shared-IP CI, or a check that gets skipped when the API fails.checksums.sha256is served from the same endpoint as the bundle itself, so verification can fail closed without adding a new failure mode. The tradeoff: the checksums file is generated by the release workflow rather than computed by GitHub, and it lives in the same asset store as the bundle. Neither option is a signature. If you'd rather use the API digest anyway, happy to change it.Details:
expected_bundle_sha256still wins and skips the checksums fetch, so self-downgrade behaves exactly as before.msb self updateon Unix now fails if a release has no checksums file. The only tags without one are the oldmicrosandbox-v0.2.x/monocore-*ones, which thev{version}URL scheme never matched anyway.Tests:
cargo fmt/clippy/cargo test -p microsandbox --lib(530 passed) /cargo test -p microsandbox-utils;go test ./...with an httptest-backed test covering verified install, digest mismatch, missing checksums, and missing entry. I also ran the build script against the real v0.6.8 release with a freshMSB_HOME; it fetched, verified, and extracted a workingmsb.🤖 Generated with Claude Code
https://claude.ai/code/session_01WZKrTmBG9RQzn85yQP2bTK