fix(dispatcher): Verify dispatcher archive attestation before installer runs#1673
Merged
Conversation
…manifest Dispatcher self-update flows now resolve the target release tag before running the installer, fetch the attestation bundle for that tag, and pass a Sigstore-verified digest manifest to install.sh / install.ps1 via ULOOP_ARCHIVE_MANIFEST. The installer scripts enforce the manifest entry for the archive they downloaded, so a compromised same-origin .sha256 file alone can no longer bless a swapped archive. The manifest step layers on top of the existing installer-script attestation check without changing the README curl|iex bootstrap path: when the env is unset, install.sh / install.ps1 fall back to the .sha256 comparison only. Add VerifySubjects to the attestation package to extract verified subject digests from a bundle without pinning to a caller-supplied artifact digest. Add resolveDispatcherLatestReleaseTag with GitHub /releases pagination and channel filtering that mirrors install.sh's rules so dispatcher-side and script-side resolution converge on the same tag.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe dispatcher now resolves release targets, verifies Sigstore attestation subjects, passes a generated archive manifest to the installer, and validates downloaded archives against both checksum files and optional attestation manifests. ChangesAttested Dispatcher Updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant UpdateRequest
participant Dispatcher
participant GitHub
participant Sigstore
participant Installer
UpdateRequest->>Dispatcher: request update
Dispatcher->>GitHub: resolve release tag
GitHub-->>Dispatcher: release metadata and bundle
Dispatcher->>Sigstore: verify attestation subjects
Sigstore-->>Dispatcher: verified archive manifest
Dispatcher->>Installer: launch with manifest environment
Installer->>Installer: verify checksum and manifest
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…n filter test install.sh switched from `sha256sum -c` (verify from a manifest file) to `sha256sum <path>` so the same digest string can be reused by both the same-origin .sha256 check and the new attestation-manifest cross-check. The mock in test-install-release-filter.sh only recognized the -c form and was failing every run with "unexpected sha256sum arguments". Emit "fakehash <path>" for the single-file form so the mock digest still matches the fixtures' .sha256 content, and keep the unrecognized-shape branch loud so a future accidental change to install.sh still fails the test.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cli/dispatcher/internal/dispatcher/attestation_release_test.go (1)
110-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover second-page release selection.
The suite only serves page 1, leaving the new pagination contract untested. Add 100 nonmatching entries on page 1 and a matching dispatcher release on page 2.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/dispatcher/internal/dispatcher/attestation_release_test.go` around lines 110 - 126, Extend installReleaseListServer and its tests to exercise pagination: provide 100 nonmatching githubReleaseListEntry values for page 1 and return a matching dispatcher release when the request query parameter page is "2", while preserving empty responses for later pages. Add or update the relevant release-selection test to assert that the matching page-2 release is selected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/dispatcher/internal/dispatcher/update.go`:
- Around line 42-50: Validate the platform with update.CommandForOS before
calling resolveUpdateTargetVersionFunc, so unsupported runtime.GOOS values
return the deterministic classified error without network access. Preserve the
resolved options for supported platforms, and add a regression test covering an
offline unsupported-platform invocation.
---
Nitpick comments:
In `@cli/dispatcher/internal/dispatcher/attestation_release_test.go`:
- Around line 110-126: Extend installReleaseListServer and its tests to exercise
pagination: provide 100 nonmatching githubReleaseListEntry values for page 1 and
return a matching dispatcher release when the request query parameter page is
"2", while preserving empty responses for later pages. Add or update the
relevant release-selection test to assert that the matching page-2 release is
selected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fd18e95c-743e-42d8-88f9-35f1e71c0765
⛔ Files ignored due to path filters (1)
scripts/install.ps1is excluded by none and included by none
📒 Files selected for processing (11)
cli/dispatcher/internal/attestation/verifier.gocli/dispatcher/internal/attestation/verifier_test.gocli/dispatcher/internal/dispatcher/attestation_release.gocli/dispatcher/internal/dispatcher/attestation_release_test.gocli/dispatcher/internal/dispatcher/dispatcher_bootstrap_routing_test.gocli/dispatcher/internal/dispatcher/run_dispatcher.gocli/dispatcher/internal/dispatcher/update.gocli/dispatcher/internal/dispatcher/update_test.gocli/dispatcher/shared-inputs-stamp.jsonscripts/install.shscripts/test-install-archive-manifest.sh
Comment on lines
+42
to
+50
| resolvedOptions, err := resolveUpdateTargetVersionFunc(ctx, update.Options{ | ||
| CurrentVersion: dispatcherVersion, | ||
| TargetVersion: options.targetVersion, | ||
| }) | ||
| if err != nil { | ||
| clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{Command: clicore.UpdateCommandName}) | ||
| return true, 1 | ||
| } | ||
| updateCommand, err := update.CommandForOS(runtime.GOOS, resolvedOptions) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject unsupported platforms before resolving releases.
Line 42 performs GitHub resolution before CommandForOS validates the platform. An offline Linux invocation now reports a network error instead of the deterministic unsupported-platform error. Validate runtime.GOOS first and add a regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/dispatcher/internal/dispatcher/update.go` around lines 42 - 50, Validate
the platform with update.CommandForOS before calling
resolveUpdateTargetVersionFunc, so unsupported runtime.GOOS values return the
deterministic classified error without network access. Preserve the resolved
options for supported platforms, and add a regression test covering an offline
unsupported-platform invocation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.sha256.User Impact
uloop updateand the auto-update path trusted the archive when its bytes matched the sibling.sha256. An attacker who could tamper with a release asset could rewrite the archive and its.sha256together and pass verification.install.sh/install.ps1. The installer scripts refuse to extract an archive whose sha256 does not match the manifest entry for its filename.curl | shbootstrap path is unchanged: whenULOOP_ARCHIVE_MANIFESTis unset, the installer scripts continue to rely on the.sha256file (bootstrap trust remains rooted in TLS + repo ownership, as documented in the security TODO).Changes
cli/dispatcher/internal/attestation/verifier.go: addVerifySubjectswhich runs full Sigstore verification (signature, transparency log, integrated timestamps, SCT, cert identity, source-repo digest binding) and returns the verified subject-digest map, without a caller-supplied artifact digest.cli/dispatcher/internal/dispatcher/attestation_release.go: paginated/releasesenumeration withdispatcher-v+ channel filtering that mirrorsinstall.sh's selector so dispatcher-side and script-side resolution converge on the same tag;fetchAttestationSubjectManifestbuilds asha256sum-style manifest from the verified statement.cli/dispatcher/internal/dispatcher/update.go&run_dispatcher.go: pre-resolve latest → concrete target version beforeCommandForOS; appendULOOP_ARCHIVE_MANIFESTenv to the installer command after the installer script's own attestation passes.scripts/install.sh&scripts/install.ps1: consumeULOOP_ARCHIVE_MANIFEST; fail closed when the manifest is set but has no entry for the asset or the digest mismatches; skip when the env is unset so first-time installs still work.scripts/test-install-archive-manifest.sh).Verification
scripts/check-go-cli.sh— pass (fmt, vet, lint, tests, dist rebuild)sh scripts/test-install-archive-manifest.sh— pass (matched digest, mismatched digest, missing entry, unset env)