Repository navigation
fix(install): tie install.sh tag to the archive, pin archive owner, verify first - #524
Merged
Merged
Conversation
Unpacked mode wrote the tag argument to VERSION without looking at the archive, so a wrong tag corrupted the downgrade guard. When checksums.txt sits next to the script, the archive of that tag must be there too and match its checksum line; without checksums.txt the tag stays unchecked and the script says so. Tests cover matching and wrong tag, a tampered archive, the missing checksums.txt, a writable prefix (warned, not refused, per #493) and the piped script in an unpacked directory. Refs: #495 Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Without it the archive records the uid and group of the build runner, which a root extraction (sudo tar -xzf) would restore on the installed tree. Pinning root:wheel for every file keeps that identity out of the archive; harmless today, as the installer sets its own modes. Refs: #495 Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
The unpacked mode never checks the attestation, so an upgrade, where gh is installed, ran as root on the checksum and TLS alone. The manual now says to run gh attestation verify on the archive before sudo ./install.sh and describes how the tag is bound to the archive next to the script. Refs: #495 Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
The tag was bound to the archive name and checksum, but the installed files still came from bin/ and guest/ beside the script, which could differ from the archive. With checksums.txt present the script now unpacks the checked archive and installs from that, so tag, archive and installed files are one thing. The wrong-tag message also covers a removed archive, and stale header and log text is corrected. Refs: #495 Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
The attestation check now comes before the tar and sudo install.sh block, so pasting top to bottom verifies before installing; a first install without gh skips it. The script now installs the files of the archive beside checksums.txt, and the piped form is narrowed to the tested cat install.sh | bash -s. Refs: #495 Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
The unpacked branch kept a root variable from the environment when no checksums.txt sat beside the script, so an exported root decided where the files were installed from. The no-checksums.txt branch now sets root itself. A test exports root and checks both branches. Refs: #495 Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Owner
Author
|
Opus review CLEAR on the whole branch (1d5ed0fd, rebased onto main 3877085 as a59e2f3; git range-diff shows all six commits equal). Reviewer runs on 1d5ed0fd: full go test ./... and make check-ci exit 0 (default bash); mutation checks show TestUnpackedWrongTagIsRefused, TestUnpackedTamperedArchiveIsRefused, TestUnpackedInstallsTheArchiveNotTheStrayTree and TestUnpackedIgnoresAnExportedRoot can fail. The rebased head is verified by this PR's CI. Known pre-existing, unrelated failure under macOS /bin/bash 3.2 only: TestHugoRefusesAssetWithWrongChecksum (scripts/hugo.sh, empty array with set -u), also at the base commit. Lows (batched on #399): /bin/bash -s piped form falls into download mode (documented). Unverified: real release archive install, root:wheel on the real release runner, sudo ./install.sh on a clean Mac. |
wstein
marked this pull request as ready for review
October 9, 2026 11:20
This was referenced Oct 9, 2026
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.
Smaller scope of #495 (items 1, 3, 4, 6; item 2 skipped, 5 resolved by alpha.5, 7 goes to the design lane via #399):
install.sh: whenchecksums.txtsits beside the script, the tag must match the archive next to it, the archive is checked against its checksum, and the files are installed from that unpacked archive (an exportedrootis ignored). Download mode shares the same checks.builds_infoandfiles[].info).gh attestation verifybeforesudo ./install.sh; narrowed claim for the piped form.root.Closes #495
Review: Opus CLEAR on the whole branch (head 1d5ed0fd before a rebase onto current main; range-diff equal). Open Low (batched on #399): the
/bin/bash -spiped form falls into download mode (documented, not changed). Unverified: a real release archive install, root:wheel ownership on the real release runner,sudo ./install.shon a clean Mac. Known pre-existing failure under macOS bash 3.2: TestHugoRefusesAssetWithWrongChecksum (unrelated).🤖 Generated with Claude Code