Skip to content

chore(install): verify downloaded tarball integrity + enforce HTTPS#1221

Merged
John-David Dalton (jdalton) merged 3 commits intomainfrom
chore/harden-installer
Apr 17, 2026
Merged

chore(install): verify downloaded tarball integrity + enforce HTTPS#1221
John-David Dalton (jdalton) merged 3 commits intomainfrom
chore/harden-installer

Conversation

@jdalton
Copy link
Copy Markdown
Contributor

@jdalton John-David Dalton (jdalton) commented Apr 17, 2026

Summary

Small polish pass on install.sh so the download path is a little more defensive.

What changed

  • Integrity check: after downloading, we compute the SSRI-style hash of the tarball (e.g. sha512-<base64>) and compare it to the value the npm registry publishes for the same version. Mismatch aborts before extraction.
  • HTTPS only: consolidated curl/wget into two helpers that pass --proto =https --tlsv1.2 (curl) or --https-only (wget), so the installer won't follow an http redirect.
  • Temp file via mktemp: downloads land in a mktemp file outside the final install directory, with an EXIT trap for cleanup. A failed verify can't leave behind a partial blob that a later run might trust.

What didn't change

  • The user-visible flow, output, and final binary location are identical on a successful install.
  • Still supports curl or wget, falls back across openssl / shasum for the hash.

Test plan

  • Fresh install in a throwaway $HOME succeeds and prints "✓ Integrity verified (sha512)"
  • Artificially tampering with the downloaded tarball causes the script to abort with a clear mismatch message (verified locally with a one-line patch appending junk to the download)
  • bash -n install.sh clean
  • CI green

Note

Medium Risk
Changes the installer’s download and verification path (new HTTPS enforcement, temp-file handling, and mandatory integrity checking), which could cause installs to fail in environments missing openssl/mktemp or with unexpected registry responses.

Overview
Hardens install.sh by routing all network reads/downloads through new fetch_url/fetch_url_to_file helpers that enforce HTTPS/TLS and prevent redirect-based downgrade.

Adds an npm-registry integrity verification step: the installer now fetches the published integrity for the exact package version, computes the downloaded tarball’s SSRI hash via openssl, and aborts before extraction on mismatch or missing metadata.

Improves safety of the download flow by writing the tarball to a mktemp-style temp file outside the install directory and cleaning it up via an EXIT trap (plus explicit removal on success).

Reviewed by Cursor Bugbot for commit cec6b1c. Configure here.

Tightens install.sh so the downloaded tarball is checked against the
integrity value the npm registry publishes for that specific version.

Changes:
  * Consolidated curl/wget into `fetch_url` and `fetch_url_to_file`
    helpers that pass `--proto =https --tlsv1.2` (curl) or `--https-only`
    (wget) so we don't silently follow an http redirect.
  * After download, compute the SSRI-style hash (e.g. `sha512-<base64>`)
    of the tarball and compare to the value returned by
    `GET /<pkg>/<version>`. Mismatch aborts before extraction.
  * Download the tarball into a `mktemp` file outside the install dir
    so a failed verification can't leave a partial blob where a later
    run might trust it; an `EXIT` trap cleans up.

Behavior for users on a successful install is unchanged.
Comment thread install.sh Outdated
Comment thread install.sh Outdated
Two follow-up fixes flagged by Cursor bugbot on #1221:

1. `get_published_integrity` pipeline tripped `set -e` under `pipefail`
   when npm metadata was truncated/malformed. Extract the body first,
   then parse via a `parse_json_string` helper that tolerates a
   missing field (returns empty). The caller's "refusing to install
   without a published checksum" message now fires with context
   instead of the script exiting silently.

2. `compute_integrity` relied on `xxd -r -p` in the `shasum` fallback
   branch to convert hex to binary. `xxd` isn't POSIX — it ships with
   vim-common and is often absent on minimal/Alpine containers. With
   `set -e` the missing binary killed the script before the friendly
   else-branch error could print.

   Replaced the fallback stack with a single openssl-only path.
   openssl is ubiquitous (macOS, mainstream Linux, default Alpine
   image, WSL, Git Bash); requiring it removes the portability bug
   and simplifies the function. If openssl is genuinely missing the
   script now prints a platform-specific install hint.

Verified locally:
  * happy path (real install) still passes
  * simulated missing-integrity response prints the helpful message
  * tampered-download still fails with expected vs got mismatch
@jdalton
Copy link
Copy Markdown
Contributor Author

Both Cursor bugbot findings fixed in 4e3a9e1:

  1. [Low] grep-pipeline exit-on-no-match: fetch body first, parse via new parse_json_string helper that tolerates a missing field under set -euo pipefail. The caller's "refusing to install without a published checksum" message now fires with context instead of a silent exit.

  2. [Medium] xxd dependency in the shasum fallback: dropped the fallback entirely and made openssl the single required tool. It's ubiquitous on every real target (macOS, mainstream Linux, default Alpine image, WSL, Git Bash); dropping the fallback removes the portability bug and simplifies the function. If openssl is genuinely missing the script prints a platform-specific install hint.

Verified locally:

  • happy path still succeeds
  • simulated missing-integrity response prints the helpful message
  • tampered-download still rejected with expected vs got mismatch

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Comment thread install.sh Outdated
Cursor bugbot flagged that wget's `--https-only` only applies to
recursive downloads — for the single-file fetches in this script it's
effectively a no-op, so a MITM could theoretically redirect us to an
http:// mirror. curl's `--proto '=https'` closed this correctly; wget
had a gap.

Swap `--https-only` for `--max-redirect=0` on both wget calls. npm's
registry serves metadata and tarballs directly with no redirect (verified
with `curl -sI`), so this is safe for the happy path and hard-blocks
any attempt to downgrade to http via redirect.

Side benefit: the two metadata fetches (version + integrity) also got
this protection. Previously the tarball was covered by the integrity
check that follows, but a MITM on the metadata call could have fed us
a fake version + matching fake integrity value. Now that vector is
closed too.
@jdalton
Copy link
Copy Markdown
Contributor Author

Good catch from Cursor (@cursor) — wget's --https-only only applies to recursive downloads, so for single-file fetches it was effectively a no-op. Swapped both wget calls to --max-redirect=0 in cec6b1c, which hard-blocks any HTTPS→HTTP redirect attempt. Verified npm's registry serves both metadata and tarball endpoints with direct 200s (no redirect), so the flag doesn't break the happy path.

Bonus win: this closes a gap in the metadata fetches (version + integrity) that the tarball's own integrity check couldn't cover — a MITM could previously have fed us a fake version + matching fake integrity. Now that vector is blocked at the network layer too.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit cec6b1c. Configure here.

@jdalton John-David Dalton (jdalton) merged commit 1e443ae into main Apr 17, 2026
6 checks passed
@jdalton John-David Dalton (jdalton) deleted the chore/harden-installer branch April 17, 2026 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants