fix(deps): harden npm-wrapper install against slip, gate matrix, partial state

CHANGELOG.md

@@ -486,6 +486,72 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     1. No JS test framework is introduced (the project already
     has a 1-line `require('tar')` check that didn't justify
     pulling in jest/mocha/vitest either; see #63). Closes #64.
+- `nodeup-npm/scripts/check.js`: replace the per-axis platform /
+  arch lookup with a single combined-key `Set` of (Node.js
+  `process.platform` / `process.arch`) pairs that mirrors the
+  GoReleaser build matrix. Pre-fix, the script checked
+  `PLATFORM_TO_OS[platform]` and `ARCH_TO_GOARCH[arch]`
+  independently — so `process.platform === 'win32'` plus
+  `process.arch === 'arm64'` mapped to `(windows, arm64)` and
+  both lookup tables returned non-null, the preinstall check
+  passed, and `install.js` then 404'd trying to download a
+  release archive that was never built (`.goreleaser.yaml`
+  line 45-47 explicitly excludes `goos: windows, goarch: arm64`
+  from the build matrix). The new lookup is one map keyed on
+  the actual (platform, arch) pair: `linux/{amd64,arm64}`,
+  `darwin/{amd64,arm64}`, `win32/amd64`. `win32/arm64` is
+  intentionally absent so the preinstall check matches the
+  release artifact set 1:1. Tests in `check_test.js` pin the
+  SUPPORTED set contents and pin the rejection of the
+  previously-buggy `win32/arm64` (and the partial-match /
+  sibling-prefix / trailing-slash variants that an env-tampering
+  user might try). Closes #65 (point 2).
+- `nodeup-npm/scripts/install.js`: belt-and-suspenders zip-slip
+  / tar-slip guards on archive extraction, plus a partial-state
+  cleanup at the start of the install. Pre-fix, the install
+  path trusted whatever entries the tar / zip extractor wrote —
+  the `tar.x()` call has no `filter` argument, and the
+  Windows-zip path shells out to `Expand-Archive` / `unzip -o`,
+  neither of which validates that extracted entry paths stay
+  inside the temp dir. The actual attack surface is bounded by
+  the SHA256 verification added in #64 (a hostile archive would
+  have to defeat that first), but a defensive check costs
+  nothing and removes the dependency entirely:
+  - `isPathInside(parent, child)` resolves both paths via
+    `path.resolve` (sibling-prefix-safe — `/tmp/ab` is NOT
+    inside `/tmp/a`) and uses `path.relative` to detect
+    `..` traversal and absolute-path escapes.
+  - `extractTarGz` now passes a `filter` callback that runs
+    `isPathInside(outDir, entryPath)` for every entry and
+    throws with the offending entry + resolved path if it
+    escapes. tar 6.x already refuses `..`-containing entries
+    by default (`node_modules/tar/lib/unpack.js:277-286`), so
+    this is defense in depth — if a future regression flips
+    the tar option, the install still refuses.
+  - `extractZip` now goes through `safeExtractZip`, which
+    lists the archive's entries first (`unzip -Z1` on POSIX,
+    `[System.IO.Compression.ZipFile]::OpenRead(...).Entries`
+    on Windows), validates each entry's resolved path against
+    `outDir`, and only then runs the actual extraction. This
+    is the real fix: `unzip -o` and `Expand-Archive` will
+    both happily extract a `../escape.txt` if the archive
+    contains one. tar doesn't have this problem; only the
+    zip path does.
+  - `main()` now `fs.rmSync(binaryDest, { force: true })` at
+    the start of the install, before any download. Pre-fix,
+    a late-stage failure (chmod throwing after renameSync
+    succeeded, an OOM mid-extraction) left a non-executable
+    `bin/nodeup` on disk; a retry's `renameSync` then either
+    errored with EEXIST or silently overwrote a half-broken
+    file. Cleaning up at the start makes the install path
+    effectively transactional.
+  - Tests in `install_test.js` cover `isPathInside` (inside /
+    equals / parent / sibling-prefix / `..` traversal /
+    absolute-outside / trailing-slash-normalisation) and the
+    end-to-end filter callback (the good entry passes, the
+    slip entry throws, the absolute-outside entry throws).
+    No JS test framework introduced; same reasoning as #63.
+    Closes #65 (points 1 and 3).
 
 ## [0.0.0] - 2024-07-01
 

nodeup-npm/scripts/check.js

@@ -12,43 +12,68 @@
 // (so npm aborts with the message visible in the install log).
 //
 // Mapping rules mirror the GoReleaser archive names in
-// .goreleaser.yaml — see the `archives[].name_template` there:
+// .goreleaser.yaml — see the `archives[].name_template` and
+// `builds[].ignore` there:
 //   {{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}
+//
+// IMPORTANT: We check the (platform, arch) PAIR against an explicit
+// allowlist of combinations GoReleaser actually builds. Pre-fix
+// (see #65), the script mapped each axis independently:
+//
+//   PLATFORM_TO_OS[process.platform] -> 'darwin' | 'linux' | 'windows'
+//   ARCH_TO_GOARCH[process.arch]     -> 'amd64' | 'arm64'
+//
+// So `process.platform === 'win32'` + `process.arch === 'arm64'`
+// mapped to (windows, arm64) and BOTH lookup tables returned a
+// non-null value — the preinstall check passed. But .goreleaser.yaml
+// line 45-47 explicitly excludes that combination from the build
+// matrix:
+//
+//   ignore:
+//     - goos: windows
+//       goarch: arm64 # windows/arm64 is rare; add later if demanded
+//
+// The user therefore got a clean `npm install` followed by a 404
+// from install.js when it tried to download a release archive that
+// was never built — exactly the "unsupported platform silently
+// proceeds past the gate meant to catch it early" failure mode the
+// preinstall exists to prevent. Fix: enumerate the supported
+// combinations directly.
 
 'use strict';
 
-const PLATFORM_TO_OS = {
-  darwin: 'darwin',
-  linux: 'linux',
-  win32: 'windows',
-  freebsd: null, // not built; intentional
-  openbsd: null, // not built; intentional
-  sunos: null, // not built; intentional
-};
-
-const ARCH_TO_GOARCH = {
-  x64: 'amd64',
-  arm64: 'arm64',
-  ia32: null, // not built
-  arm: null, // not built
-  ppc64: null, // not built
-  s390x: null, // not built
-};
+// Each entry mirrors a row in .goreleaser.yaml's build matrix:
+//   goos:   linux | darwin | windows
+//   goarch: amd64 | arm64
+// minus the explicit ignore `{ goos: windows, goarch: arm64 }`.
+//
+// Keys use Node.js's process.platform / process.arch values, NOT
+// GoReleaser's goos / goarch names — Node reports `win32` where
+// GoReleaser uses `windows`, and using Node's native identifiers
+// keeps the lookup map consistent with the values it consumes.
+const SUPPORTED = new Set([
+  'linux/amd64',
+  'linux/arm64',
+  'darwin/amd64',
+  'darwin/arm64',
+  'win32/amd64',
+  // 'win32/arm64' is intentionally absent — see .goreleaser.yaml
+  // `builds[].ignore` (the goos: windows, goarch: arm64 entry
+  // that mirrors Node's win32/arm64). Adding it here without
+  // also dropping the goreleaser ignore would re-introduce the
+  // 404 race the preinstall check is supposed to prevent.
+]);
 
 const platform = process.platform;
 const arch = process.arch;
 
-const os = PLATFORM_TO_OS[platform];
-const goarch = ARCH_TO_GOARCH[arch];
-
-if (!os || !goarch) {
+if (!SUPPORTED.has(`${platform}/${arch}`)) {
   console.error('');
   console.error('  nodeup: unsupported platform/architecture');
   console.error(`    platform=${platform}, arch=${arch}`);
   console.error('');
   console.error('  Built binaries are available for:');
-  console.error('    OS:      darwin, linux, windows');
-  console.error('    arch:    amd64, arm64');
+  for (const k of SUPPORTED) console.error(`    ${k}`);
   console.error('');
   console.error('  See https://github.com/dipto0321/nodeup/releases');
   console.error('  for direct binary downloads, or open an issue if');

nodeup-npm/scripts/check_test.js

@@ -0,0 +1,130 @@
+#!/usr/bin/env node
+//
+// check_test.js — pure-data smoke tests for check.js.
+//
+// check.js is a preinstall gate that exits 1 if the user's
+// (platform, arch) isn't in the GoReleaser build matrix. It
+// doesn't `module.exports` (it's invoked by npm directly), so we
+// reimplement the SUPPORTED set here and pin its contents.
+//
+// What we test:
+//   - The SUPPORTED set matches GoReleaser's `builds[]` matrix
+//     (linux/darwin/windows × amd64/arm64) MINUS the explicit
+//     `ignore: { goos: windows, goarch: arm64 }` exclusion.
+//   - Common supported combinations pass (linux/amd64,
+//     darwin/arm64, win32/amd64).
+//   - The previously-buggy windows/arm64 combination is rejected.
+//   - Combinations that aren't built at all (freebsd/amd64,
+//     darwin/ia32) are rejected.
+//   - The error output names the offender when a gate fails
+//     (we reimplement that part here as a smoke check).
+//
+// What we DON'T test (left to live e2e):
+//   - The actual process.exit(1) call (we can't `process.exit`
+//     without killing the test process; we reimplement the
+//     SUPPORTED set + the lookup separately).
+
+'use strict';
+
+const assert = require('assert');
+
+// Mirrored from check.js. Update both together when GoReleaser
+// adds a new build target or drops an old one.
+const SUPPORTED = new Set([
+  'linux/amd64',
+  'linux/arm64',
+  'darwin/amd64',
+  'darwin/arm64',
+  'win32/amd64',
+  // 'win32/arm64' is intentionally absent — see .goreleaser.yaml
+  // `builds[].ignore`. See #65.
+]);
+
+// --- tests ---------------------------------------------------------------
+
+let passed = 0;
+function test(name, fn) {
+  try {
+    fn();
+    passed += 1;
+    process.stdout.write(`  ok ${name}\n`);
+  } catch (err) {
+    process.stdout.write(`  FAIL ${name}: ${err && err.message ? err.message : err}\n`);
+    process.exit(1);
+  }
+}
+
+// Pinned members: every supported (platform, arch) pair must
+// pass. If a future goreleaser matrix change adds a row that's
+// missing here, this test fails and forces an update.
+test('supported_linux_amd64', () => {
+  assert.strictEqual(SUPPORTED.has('linux/amd64'), true);
+});
+test('supported_linux_arm64', () => {
+  assert.strictEqual(SUPPORTED.has('linux/arm64'), true);
+});
+test('supported_darwin_amd64', () => {
+  assert.strictEqual(SUPPORTED.has('darwin/amd64'), true);
+});
+test('supported_darwin_arm64', () => {
+  assert.strictEqual(SUPPORTED.has('darwin/arm64'), true);
+});
+test('supported_win32_amd64', () => {
+  assert.strictEqual(SUPPORTED.has('win32/amd64'), true);
+});
+
+// Pinned exclusion: windows/arm64 is the bug from #65 — both
+// lookup tables in the pre-fix code mapped `win32 + arm64` to
+// (windows, arm64) successfully, even though the goreleaser
+// build matrix explicitly skips that combination. Verify the
+// new combined-key lookup rejects it.
+test('rejected_win32_arm64_isTheBugFrom65', () => {
+  assert.strictEqual(SUPPORTED.has('win32/arm64'), false);
+});
+
+// Combinations that aren't built at all (Node reports these
+// platforms / archs but the project doesn't ship binaries for
+// them). Both axes must fail, not just one.
+test('rejected_freebsd_amd64', () => {
+  assert.strictEqual(SUPPORTED.has('freebsd/amd64'), false);
+});
+test('rejected_darwin_ia32', () => {
+  assert.strictEqual(SUPPORTED.has('darwin/ia32'), false);
+});
+test('rejected_linux_ppc64', () => {
+  assert.strictEqual(SUPPORTED.has('linux/ppc64'), false);
+});
+
+// A partial-match attack: an attacker who controls an env var or
+// a config file might try `linux/amd64/` (trailing slash) or
+// `linux/amd64/../arm64` to slip past a startsWith-style check.
+// The combined-key lookup is exact-string, so neither can pass.
+test('rejected_partialTrailingSlash', () => {
+  assert.strictEqual(SUPPORTED.has('linux/amd64/'), false);
+});
+test('rejected_partialSiblingPrefix', () => {
+  // Classic sibling-prefix attack: a key that *starts with* a
+  // supported entry but isn't actually in the set.
+  assert.strictEqual(SUPPORTED.has('linux/amd64-evil'), false);
+});
+
+// The error-message shape (smoke check). When a gate fails, the
+// user needs to see which (platform, arch) was rejected and what
+// IS supported. Reimplement the message builder here to pin the
+// shape — if a future change renames fields or drops the
+// "Built binaries are available for:" block, this fails.
+test('errorMessageNamesOffenderAndListsSupported', () => {
+  // Mirror the format from check.js.
+  const platform = 'win32';
+  const arch = 'arm64';
+  let captured = null;
+  if (!SUPPORTED.has(`${platform}/${arch}`)) {
+    captured = `unsupported platform/architecture: ${platform}/${arch}; available: ${Array.from(SUPPORTED).join(', ')}`;
+  }
+  assert.ok(captured && captured.includes('win32/arm64'));
+  assert.ok(captured && captured.includes('linux/amd64'));
+  assert.ok(captured && captured.includes('windows/arm64') === false); // absent from list
+});
+
+// Done.
+process.stdout.write(`0 issues. (${passed} tests)\n`);