fix(updater): reject zero-byte staged binary + error on oversized archive#2
Merged
Merged
Conversation
…hive replaceBinary previously called os.Rename on src without checking its size, so a 0-byte staged binary would atomic-rename over the live daemon binary and brick the daemon on next start. Refuse explicitly when the source is empty so the existing binary keeps running. downloadFile capped the body at maxDownloadBytes via io.LimitReader, which silently truncated oversize archives. The SHA256 check then failed with a confusing "checksum mismatch" instead of telling the operator the archive was too large. Read one byte past the cap and surface a clear "archive exceeds max download size" error. Adds TestReplaceBinary_RejectsEmptySource and TestDownloadRejectsOversizedArchive as regressions for both paths.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
2 participants
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
Two real bugs in
updater.gothat would surface as silent corruption / confusing operator errors.Bug 1 (HIGH, operational):
replaceBinarywould brick the daemon on a zero-byte staged binaryreplaceBinarydid not validate thatsrcwas non-empty beforeos.Rename. A zero-byte staged binary atomic-renamed over the live daemon binary would brick the daemon on the next start. Now statssrcfirst and refuses with"refusing to replace binary with empty source: <path>"— the existing binary keeps running, the operator sees the update failed loudly.Bug 2 (MED, UX):
downloadFilesilently truncated oversize archivesio.LimitReader(resp.Body, maxDownloadBytes)quietly capped the body at 256 MiB. The downloaded file became a corrupt prefix and the subsequent SHA256 check failed with"checksum mismatch"— the operator had no clue the real problem was archive size. Now reads one byte past the cap and returns"archive exceeds max download size %d bytes"explicitly.Test plan
TestReplaceBinary_RejectsEmptySource— zero-byte src + non-empty dst; expect error, dst untouched, no.newscratch left behind.TestDownloadRejectsOversizedArchive—httptest.NewServerstreamsmaxDownloadBytes + 1 KiB; expect explicit "exceeds max download size" error, and assert the error does NOT mention "checksum".go test -race -count=1 -timeout 120s ./...passes.