fix(install): add SHA-256 verification for cli.js download#3389
Open
fix(install): add SHA-256 verification for cli.js download#3389
Conversation
The install script downloads cli.js from GitHub Releases but does not verify its integrity, unlike the bun installer which checks a pinned SHA-256 hash. This adds checksum verification using a companion cli.js.sha256 release artifact (same pattern as the bun hash check). When the checksum file is not yet published, the installer warns and continues — once CI publishes cli.js.sha256, verification activates automatically with no further install.sh changes needed. Fixes #3327 Agent: security-auditor Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
la14-1
commented
May 4, 2026
Member
Author
la14-1
left a comment
There was a problem hiding this comment.
Security Review: PR #3389 — SHA-256 verification for cli.js download
Does the fix address the vulnerability in #3327?
Partially. This PR adds the client-side verification logic to install.sh, which is the necessary first half. However, as #3327 explicitly notes, both halves are needed:
- install.sh side (this PR): Download
cli.js.sha256, verify the hash ofcli.jsbefore installing. Done correctly. - CI side (not in this PR): Publish
cli.js.sha256alongsidecli.jsin thecli-latestrelease. Without this, the verification is a no-op — the script gracefully degrades with "No cli.js.sha256 checksum published yet — skipping verification".
Findings
Positive:
- The
sha256_filehelper already exists on main (portable macOS/Linux implementation). The PR reuses it correctly. - The
tr -d '[:space:]'on the expected hash is a good defensive measure against trailing newlines in the checksum file. - Graceful degradation is implemented at two levels: (a) no checksum file published yet, (b) no sha256sum/shasum binary available. Both log warnings instead of failing.
- The error messaging on mismatch is excellent — clear "possible supply chain attack" language with the expected vs actual hashes and a pointer to the issues page.
- The
--proto '=https'flag on the checksum download enforces HTTPS, preventing downgrade attacks. - The
curlfor the checksum file uses2>/dev/nullto suppress errors when the file doesn't exist yet, which is appropriate.
Concerns:
- TOCTOU is not a risk here — the downloaded
cli.jsis verified in the sametmpdirbefore being copied to the install location. No race window. - The checksum file format is simple — just the hex digest, no filename. This matches the implementation (
tr -d '[:space:]'strips any whitespace). Consistent with common practice. - Until CI publishes the
.sha256file, this provides zero protection. The graceful skip means an attacker who compromises only thecli.jsartifact (but not.sha256because it doesn't exist) gets through silently. This is acceptable as a phased rollout — the warning makes it visible that verification is not happening.
Recommendation: This PR is correct and safe to merge. The CI-side companion change (publishing cli.js.sha256 in the release workflow) should be tracked as a follow-up — without it, the verification is dormant. Is there an issue tracking the CI side?
-- refactor/security-auditor
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
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.
Why: Prevents MITM/tampered binary downloads by verifying SHA-256 of cli.js before execution — closes the last unverified download in the install path (bun installer already has hash verification).
Changes
cli.js.sha256companion file from the same GitHub Releasesha256_file()helper (works on both macOSshasumand Linuxsha256sum)Remaining work (separate PR, requires human review)
.github/workflows/release.ymlmust be updated to publishcli.js.sha256alongsidecli.jsin thecli-latestrelease. Workflow changes are off-limits for automated PRs per team rules. Once CI publishes the checksum file, this verification activates automatically.Fixes #3327