Verify release checksum before replacing lstk binary#410
Open
skyrpex wants to merge 3 commits into
Open
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
This reverts commit acc2407.
skyrpex
marked this pull request as ready for review
July 24, 2026 14:34
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.
What
The self-updater's direct-binary path (
lstk updatewhen not installed via Homebrew/npm) downloaded the release archive from GitHub and replaced the running executable without any integrity check. It now verifies the archive's SHA-256 against the release's publishedchecksums.txtbefore extraction:parseChecksums(internal/update/checksum.go) parses the goreleaser manifest (tolerates CRLF, blank lines,*binary-mode marker; rejects malformed lines with line numbers).updateBinaryis now abinaryUpdatermethod with injectable download base URL and executable resolver, enabling unit tests againsthttptestservers. It fetcheschecksums.txtfirst (fail fast, 1 MiB read cap), requires an entry for the platform asset, stream-hashes the archive during download (io.MultiWriter), and aborts beforeextractAndReplaceon mismatch.checksums.txt(404), missing asset entry, malformed manifest, or hash mismatch — warn-and-continue would make the control bypassable by deleting one release asset.Why
Flagged on Slack (see DEVX-999): the downloaded binary was trusted without verification. goreleaser already publishes a SHA-256
checksums.txton every release, so this needs no release-pipeline changes. Homebrew and npm update paths are unchanged — those package managers verify integrity themselves.Notes for reviewers
ErrInternalper the error-code policy ininternal/output/error_code.go; the message text carries the security signal ("may be corrupted or tampered with; update aborted"). A dedicated integrity code would be an additive follow-up.signs:block,id-token: writein the release job, sigstore verification in the updater) are deliberately out of scope — worth a separate ticket.binaryUpdatermatrix (happy path, mismatch leaves the binary untouched with no temp leftovers, 404, missing entry, malformed manifest). The existing integration testTestUpdateBinaryInPlacedoes a real GitHub download and now exercises verification end-to-end (its spinner-text assertion was updated).Review: security-relevant change to the self-update path — human review advisable.
Closes DEVX-999
Co-Authored-By: Claude noreply@anthropic.com