Skip to content

fix(installer): download verified archives with the identity encoding - #2064

Merged
Alan-TheGentleman merged 1 commit into
mainfrom
fix/installer-download-identity
Oct 10, 2026
Merged

Alan-TheGentleman merged 1 commit into
mainfrom
fix/installer-download-identity

Conversation

@Alan-TheGentleman

@Alan-TheGentleman Alan-TheGentleman commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #2063

Root cause

Node's fetch sends Accept-Encoding: gzip, deflate, br by default. dl.google.com then serves the Go archive with Content-Encoding: gzip and Content-Length: 65267505 (encoded), while fetch decodes to 67591780 bytes, so download()'s length check threw Download truncated and verifiedDownload turned it into a bare Download failed. Verified locally with Node's fetch (default vs accept-encoding: identity) and on windows-latest (diag run 38092917737). The pinned Go was therefore never downloadable in production; tests used local adapters and CI used setup-go.

What

  • download() requests accept-encoding: identity; an encoded response is refused with Download was encoded (<enc>) although identity was requested; a non-OK status names the status.
  • verifiedDownload keeps the transport error as cause.

Tests

  • RED then GREEN: a local server that gzips on request (like dl.google.com) now delivers the archive whole and sees identity; a server that always encodes is refused with its cause; a failed transport keeps its cause.
  • Real acquireGo against production dl.google.com on macOS published Go 1.25.14.
  • New native Windows test downloads, verifies and runs the real pinned Go; the native gate counts 36.
  • Installer suites 506 tests, 470 pass, 0 fail, 36 skipped (Windows native only off Windows); typecheck no regressions.

Summary by CodeRabbit

  • Bug Fixes
    • Installer downloads now reject encoded responses that could interfere with archive verification, and download failures retain more diagnostic detail.
  • Tests
    • Expanded Windows installer checks to verify that the pinned Go toolchain downloads, installs, and reports the expected version.

Node's fetch accepts gzip by default and dl.google.com then gzips the Go
archive: Content-Length is the encoded size while fetch yields the decoded
bytes, so every pinned Go download failed as truncated. download() now asks
for the identity encoding, refuses an encoded response with a clear cause,
and verifiedDownload keeps the transport error as its cause. A native Windows
test downloads, verifies and runs the real pinned Go; the native gate counts 36.

Closes #2063
@Alan-TheGentleman Alan-TheGentleman added the type:bug Bug fix label Oct 10, 2026
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 6abe9103-6a01-4c8a-9ae8-5d26a56e4c03

📥 Commits

Reviewing files that changed from the base of the PR and between 563fede and 59f41a5.


📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • scripts/installer-downloads.mjs
  • tests/installer-posix-bootstrap.test.ts
  • tests/installer-windows-bootstrap.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.



📝 Walkthrough

Walkthrough

The download path requests identity encoding, rejects encoded responses, and preserves transport errors. Tests cover download encoding behavior and native Windows acquisition of the pinned Go toolchain. The Windows CI gate now expects 36 passing tests.

Changes

Installer download verification

Layer / File(s) Summary
Download encoding and error handling
scripts/installer-downloads.mjs, tests/installer-posix-bootstrap.test.ts
Downloads request identity encoding and reject encoded responses. Errors include HTTP status or preserve the underlying transport cause. Tests cover identity requests, encoded responses, and transport failures.
Native Windows Go acquisition
tests/installer-windows-bootstrap.test.ts, .github/workflows/ci.yml, tests/installer-posix-bootstrap.test.ts
A native Windows test acquires and runs the pinned Go toolchain, checks its reported version, and removes its temporary root. The Windows CI gate and its test now expect 36 passes and zero skips.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: High

Suggested reviewers: decode2


Merge Risk: ⚪ Minimal · up to 59f41

The installer now requests unencoded archives, so the pinned Go download no longer fails on a length mismatch. Tests cover the new behavior, and I found no merge-blocking risk.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main installer change: downloading verified archives with identity encoding.
Linked Issues check Passed The PR meets the coding requirements in issue #2063. download() sends accept-encoding: identity, rejects non-identity content-encoding, and includes the HTTP status for non-OK responses. `verifi…
Out of Scope Changes check Passed The changes stay within issue #2063. The download tests validate the requested encoding and error behavior. The Windows test implements the requested native pinned-Go coverage. The CI count update sup…

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR


🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Alan-TheGentleman
Alan-TheGentleman merged commit de9127c into main Oct 10, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Installer: the pinned Go download always fails because fetch accepts gzip

1 participant