feat(install): verify release archive checksums in both installers#942
feat(install): verify release archive checksums in both installers#942mdesmet wants to merge 1 commit into
Conversation
Raises the integrity bar for the standalone installers (follow-up to #930). - release.yml: generate a checksums.txt (sha256sum format) over the release archives and publish it as a release asset. - install (bash) + install.ps1: fetch checksums.txt and verify the downloaded archive's SHA256 before extracting. Hard-fail on mismatch; soft-skip with a notice when checksums.txt is absent (older pinned releases) or unreachable, so existing version-pinned installs keep working. - Cross-platform sha in bash (sha256sum or shasum -a 256); Get-FileHash on Windows. Verification runs before extraction in both. - Tests: checksum-verification.test.ts asserts release.yml publishes the file and both installers fetch + compare + hard-fail on mismatch. Verified: bash -n clean; install.ps1 parses clean and the Pester suite (6/6) still passes on PowerShell 7.6.2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="install.ps1">
<violation number="1" location="install.ps1:204">
P2: Using the mutable `releases/latest/download` URL for checksum verification creates a race: archive and checksums can come from different releases. This can cause transient hard-fail installs even when artifacts are valid.</violation>
</file>
<file name="install">
<violation number="1" location="install:398">
P2: Checksum mismatch path references undefined `tmp_dir` under `set -u`. This triggers an unbound-variable error and breaks intended error-handling cleanup logic.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| if ($useLatest) { | ||
| $url = "https://github.com/AltimateAI/altimate-code/releases/latest/download/$filename" | ||
| $base = "https://github.com/AltimateAI/altimate-code/releases/latest/download" |
There was a problem hiding this comment.
P2: Using the mutable releases/latest/download URL for checksum verification creates a race: archive and checksums can come from different releases. This can cause transient hard-fail installs even when artifacts are valid.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install.ps1, line 204:
<comment>Using the mutable `releases/latest/download` URL for checksum verification creates a race: archive and checksums can come from different releases. This can cause transient hard-fail installs even when artifacts are valid.</comment>
<file context>
@@ -171,10 +201,12 @@ function Install-Target {
if ($useLatest) {
- $url = "https://github.com/AltimateAI/altimate-code/releases/latest/download/$filename"
+ $base = "https://github.com/AltimateAI/altimate-code/releases/latest/download"
} else {
- $url = "https://github.com/AltimateAI/altimate-code/releases/download/v$specificVersion/$filename"
</file context>
| $base = "https://github.com/AltimateAI/altimate-code/releases/latest/download" | |
| $base = "https://github.com/AltimateAI/altimate-code/releases/download/v$specificVersion" |
| print_message error "Checksum mismatch for $name" | ||
| print_message error " expected: $expected" | ||
| print_message error " actual: $actual" | ||
| rm -rf "$tmp_dir" |
There was a problem hiding this comment.
P2: Checksum mismatch path references undefined tmp_dir under set -u. This triggers an unbound-variable error and breaks intended error-handling cleanup logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install, line 398:
<comment>Checksum mismatch path references undefined `tmp_dir` under `set -u`. This triggers an unbound-variable error and breaks intended error-handling cleanup logic.</comment>
<file context>
@@ -356,6 +356,51 @@ download_with_progress() {
+ print_message error "Checksum mismatch for $name"
+ print_message error " expected: $expected"
+ print_message error " actual: $actual"
+ rm -rf "$tmp_dir"
+ exit 1
+ fi
</file context>
| rm -rf "$tmp_dir" | |
| rm -rf "$(dirname "$file")" |
❌ Tests — Failures DetectedTypeScript — 15 failure(s)
Next StepPlease address the failing cases above and re-run verification. cc @mdesmet |
There was a problem hiding this comment.
Local review of #942. One real issue inline (the PS 5.1 byte[] case on install.ps1:89 — verification is effectively dead on the default Windows shell). Notes from triage:
False positives I verified before discarding:
tmp_dirallegedly unbound underset -uinsideverify_checksum(cubic + OCR): bashlocalis dynamically scoped —tmp_dirset indownload_and_installis visible to the function it calls. Verified with a 4-line test.\r\nline endings allegedly corrupting the extracted hash (OCR): the regex's\s*$consumes the trailing\r, and-split '\s+'puts the\rin the separator, not in[0]. The hash comes out clean.
Out of scope: the catch-all-errors behavior in Test-Checksum's try/catch (also flagged by OCR) is a deliberate design choice per the PR description (soft-skip on any fetch failure for backwards-compat with pre-checksums releases). Worth a note as a follow-up if this ever moves to GPG-signed releases, but not a defect against this PR's stated contract.
|
|
||
| $sums = $null | ||
| try { | ||
| $sums = (Invoke-WebRequest -Uri $ChecksumsUrl -UseBasicParsing).Content |
There was a problem hiding this comment.
P1 — verification is effectively dead on Windows PowerShell 5.1.
GitHub release-assets are served with Content-Type: application/octet-stream (verified with curl -I against an existing release-asset checksums file). On Windows PowerShell 5.1, Invoke-WebRequest -UseBasicParsing returns .Content as System.Byte[] (not String) whenever the content-type isn't text-recognized. PS 5.1 is the default shell on Windows 10 and is preinstalled alongside PS 7 on Windows 11.
Downstream effect:
$sums = (... ).Content→ byte array, not text$sums -split "n"→ PowerShell coerces the byte[] to a"49 50 51 …"decimal string before splitting → no\n` boundaries → one elementWhere-Object { $_ -match … }→ no match- Falls into the "no checksum entry for X" soft-skip branch every time
So on the dominant Windows shell, every install soft-skips verification and the user sees a misleading "no checksum entry" notice instead of either a real check or a clear "unsupported" error.
Fix — decode bytes explicitly:
$resp = Invoke-WebRequest -Uri $ChecksumsUrl -UseBasicParsing
$sums = if ($resp.Content -is [byte[]]) {
[System.Text.Encoding]::UTF8.GetString($resp.Content)
} else {
$resp.Content
}Worth a Pester test that feeds a known-good fixture through Test-Checksum so this regression can't come back.
3 participants
What
Follow-up to #930 (raised in review by @coderabbitai and the consensus panel): publish a checksums file with releases and verify downloaded archives in both the curl/bash and PowerShell installers.
Stacked on
feat/windows-powershell-installer(base of this PR) becauseinstall.ps1only exists on that branch. Merge #930 first, then this — or rebase ontomainafter #930 lands.Changes
.github/workflows/release.yml— generatechecksums.txt(sha256sum *.tar.gz *.zip) and publish it as a release asset alongside the archives.install(bash) —verify_checksum()fetcheschecksums.txt, looks up the archive's expected hash, and compares (sha256sumorshasum -a 256) before extracting.install.ps1—Test-Checksumfetcheschecksums.txtand comparesGet-FileHash -Algorithm SHA256beforeExpand-Archive. Replaces the deferral note from feat: Windows PowerShell installer (install.ps1) #930.checksums.txtis absent (releases predating this change) or unreachable, so existing version-pinned installs keep working.checksum-verification.test.tsasserts the release publishes the file and both installers fetch + compare + hard-fail on mismatch.Verification
bash -n installclean;install.ps1parses clean and the Pester suite (6/6) still passes on PowerShell 7.6.2.checksum-verification.test.tsgreen (57 pass across the install/branding set).checksums.txt.🤖 Generated with Claude Code
Summary by cubic
Adds SHA256 checksum verification to both installers and publishes a
checksums.txtwith each release. Blocks extraction on mismatch and gracefully skips verification for older releases or unreachable checksums.checksums.txtfor all.tar.gzand.zipassets.install: fetcheschecksums.txt, verifies SHA256 (sha256sumorshasum -a 256) before extract.install.ps1: fetcheschecksums.txt, verifies viaGet-FileHashbeforeExpand-Archive.checksums.txtis missing/unreachable, there’s no entry, or no SHA tool is available.checksum-verification.test.tsasserts release publishes the file and both installers verify before extraction and fail on mismatch.Written for commit a0a44a1. Summary will update on new commits.