diff --git a/apps/desktop/scripts/stage-native-deps.mjs b/apps/desktop/scripts/stage-native-deps.mjs index 987d37946cd65..95fea7d13b4e4 100644 --- a/apps/desktop/scripts/stage-native-deps.mjs +++ b/apps/desktop/scripts/stage-native-deps.mjs @@ -426,7 +426,11 @@ function resolveGetWindowsRoot() { */ const GET_WINDOWS_VERSION = '9.3.0' -export function stageGetWindowsInto(srcRoot, destRoot, { platform = process.platform } = {}) { +export function stageGetWindowsInto( + srcRoot, + destRoot, + { platform = process.platform, rebuild } = {} +) { // The STAGED_WINDOWS_JS rewrite mirrors this exact version's export surface. // A version bump must fail the build here until the rewrite is re-verified — // otherwise it ships stale and fails soft as a generic "unavailable". @@ -473,17 +477,32 @@ export function stageGetWindowsInto(srcRoot, destRoot, { platform = process.plat // the target platform; the classify gate below still catches a dir that // claims win32 but holds a foreign binary. const bindingRoot = join(srcRoot, 'lib', 'binding') - const bindingDirs = existsSync(bindingRoot) - ? readdirSync(bindingRoot).filter( - (dir) => - dir.includes(`-${platform}-`) && - existsSync(join(bindingRoot, dir, 'node-get-windows.node')) - ) - : [] + const scanBindingDirs = () => + existsSync(bindingRoot) + ? readdirSync(bindingRoot).filter( + (dir) => + dir.includes(`-${platform}-`) && + existsSync(join(bindingRoot, dir, 'node-get-windows.node')) + ) + : [] + let bindingDirs = scanBindingDirs() + if (bindingDirs.length === 0 && typeof rebuild === 'function') { + // A plain `npm install` won't re-run an install script for a package + // that is already on disk, so every checkout that installed while + // get-windows was missing from allowScripts stays bricked even after + // the allowlist is fixed. `npm rebuild` re-runs it. + console.log( + '[stage-native-deps] get-windows has no win32 binding; running `npm rebuild get-windows`...' + ) + rebuild() + bindingDirs = scanBindingDirs() + } if (bindingDirs.length === 0) { throw new Error( - '[stage-native-deps] get-windows has no win32 prebuilt binding under lib/binding; ' + - 'reinstall dependencies on the Windows build host.' + '[stage-native-deps] get-windows has no win32 prebuilt binding under lib/binding. ' + + 'Recover from the checkout root with:\n' + + ' npm install-scripts approve get-windows\n' + + ' npm rebuild get-windows' ) } for (const dir of bindingDirs) { @@ -506,10 +525,26 @@ export function stageGetWindowsInto(srcRoot, destRoot, { platform = process.plat return destRoot } +function rebuildGetWindowsViaNpm() { + const result = spawnSync('npm', ['rebuild', 'get-windows'], { + cwd: resolve(projectRoot, '..', '..'), + stdio: 'inherit', + // npm resolves to npm.cmd on Windows, which needs a shell. + shell: process.platform === 'win32' + }) + if (result.status !== 0) { + console.warn(`[stage-native-deps] npm rebuild get-windows exited with ${result.status}`) + } +} + export function stageGetWindows({ platform = process.platform } = {}) { const srcRoot = resolveGetWindowsRoot() const destRoot = resolve(projectRoot, 'dist/node_modules/get-windows') - return stageGetWindowsInto(srcRoot, destRoot, { platform }) + // Only a win32 host can produce the win32 binding, so a cross-platform pack + // has nothing to gain from the rebuild. + const rebuild = + platform === 'win32' && process.platform === 'win32' ? rebuildGetWindowsViaNpm : undefined + return stageGetWindowsInto(srcRoot, destRoot, { platform, rebuild }) } // Allow direct CLI invocation: node scripts/stage-native-deps.mjs [platform] [arch] diff --git a/apps/desktop/scripts/stage-native-deps.test.mjs b/apps/desktop/scripts/stage-native-deps.test.mjs index dfd06bb25c5be..48a4101175d17 100644 --- a/apps/desktop/scripts/stage-native-deps.test.mjs +++ b/apps/desktop/scripts/stage-native-deps.test.mjs @@ -437,6 +437,53 @@ test('win32 staging fails when only foreign bindings exist', () => { } }) +test('win32 staging self-heals through the rebuild hook when the binding is missing', () => { + const tmp = fs.mkdtempSync(join(os.tmpdir(), 'hermes-stage-')) + try { + const srcRoot = join(tmp, 'get-windows') + const destRoot = join(tmp, 'dest') + + // The bricked state a blocked install script leaves behind: the package is + // present, lib/binding was never populated by node-pre-gyp. + makeFakeGetWindows(srcRoot, { bindings: [] }) + + let calls = 0 + const rebuild = () => { + calls += 1 + makeFakeNode( + join(srcRoot, 'lib', 'binding', 'napi-9-win32-unknown-x64', 'node-get-windows.node'), + 'win32' + ) + } + + stageGetWindowsInto(srcRoot, destRoot, { platform: 'win32', rebuild }) + + assert.equal(calls, 1) + assert.ok( + existsSync(join(destRoot, 'lib', 'binding', 'napi-9-win32-unknown-x64', 'node-get-windows.node')) + ) + } finally { + fs.rmSync(tmp, { recursive: true, force: true }) + } +}) + +test('win32 staging reports the recovery steps when the rebuild hook produces nothing', () => { + const tmp = fs.mkdtempSync(join(os.tmpdir(), 'hermes-stage-')) + try { + const srcRoot = join(tmp, 'get-windows') + const destRoot = join(tmp, 'dest') + + makeFakeGetWindows(srcRoot, { bindings: [] }) + + assert.throws( + () => stageGetWindowsInto(srcRoot, destRoot, { platform: 'win32', rebuild: () => {} }), + /npm rebuild get-windows/ + ) + } finally { + fs.rmSync(tmp, { recursive: true, force: true }) + } +}) + test('staging refuses a get-windows version the lib/windows.js rewrite was not verified against', () => { const tmp = fs.mkdtempSync(join(os.tmpdir(), 'hermes-stage-')) try { diff --git a/package.json b/package.json index 7f9b40649c3a2..b89c014e4bd03 100644 --- a/package.json +++ b/package.json @@ -71,8 +71,9 @@ "node-pty@1.1.0": true, "electron-winstaller@5.4.0": true, "agent-browser@0.26.0": true, - "electron@40.10.2": true, + "electron@40.10.6": true, "fsevents@2.3.2": true, - "fsevents@2.3.3": true + "fsevents@2.3.3": true, + "get-windows@9.3.0": true } } diff --git a/tests-js/allow-scripts-sync.test.ts b/tests-js/allow-scripts-sync.test.ts new file mode 100644 index 0000000000000..0aa8ea6bbb5af --- /dev/null +++ b/tests-js/allow-scripts-sync.test.ts @@ -0,0 +1,159 @@ +/** + * Invariants tying ``allowScripts`` to the lockfile it gates. + * + * npm's ``allowScripts`` allowlist is keyed by exact ``name@version``, so an + * entry silently stops matching the moment that dependency is bumped. Nothing + * else in the build notices: npm downgrades the blocked script to a warning + * buried in install output, and the failure only surfaces much later as a + * missing native artifact. + * + * That has now bitten twice on Windows. ``get-windows`` was added to + * ``apps/desktop`` without an allow entry, so its node-pre-gyp install script + * never downloaded the win32 binding and ``hermes desktop`` died in + * ``stage-native-deps``. In the same window, a CVE sweep moved Electron to + * 40.10.6 and left the ``electron@40.10.2`` pin behind, blocking Electron's + * own postinstall on any clean install. + * + * Two contracts keep the allowlist honest: + * + * - Every versioned pin names a version the lockfile actually resolves, so a + * dependency bump that orphans its pin fails here instead of in a user's + * build. + * - Every package the lockfile marks as having an install script is covered + * by a decision — allowed at its exact version, or denied by name. + * + * A bare-name key (no ``@version``) is a deliberate standing decision that + * survives version bumps, which is how ``unicode-animations: false`` stays a + * permanent denial. + */ + +import assert from 'node:assert/strict' +import fs from 'node:fs' +import path from 'node:path' + +import { describe, test } from 'vitest' + +const REPO_ROOT = path.resolve(__dirname, '..') + +const MANIFESTS = [ + { name: 'root', dir: '.' }, + { name: 'website', dir: 'website' } +] + +function manifestLabel(dir: string): string { + return path.join(dir === '.' ? '' : dir, 'package.json') +} + +interface LockPackage { + name?: string + version?: string + hasInstallScript?: boolean +} + +function readJson(filePath: string): Record { + return JSON.parse(fs.readFileSync(filePath, 'utf-8')) +} + +function packageNameFor(lockPath: string, entry: LockPackage): string { + return entry.name ?? lockPath.split('node_modules/').pop() ?? lockPath +} + +/** Every version of every package the lockfile installs, keyed by name. */ +function installedVersions(lock: Record): Map> { + const versions = new Map>() + + for (const [lockPath, entry] of Object.entries( + (lock.packages ?? {}) as Record + )) { + if (!lockPath || !entry.version) { + continue + } + + const name = packageNameFor(lockPath, entry) + const seen = versions.get(name) ?? new Set() + + seen.add(entry.version) + versions.set(name, seen) + } + + return versions +} + +function splitPin(key: string): { name: string; version: string } | null { + // Scoped packages carry a leading @, so match the LAST @ as the separator. + const match = key.match(/^(.+)@([^@]+)$/) + + return match ? { name: match[1], version: match[2] } : null +} + +describe.each(MANIFESTS)('$name allowScripts', ({ dir }) => { + const manifestPath = path.join(REPO_ROOT, dir, 'package.json') + const lockPath = path.join(REPO_ROOT, dir, 'package-lock.json') + const label = manifestLabel(dir) + + test('every versioned pin matches a version in the lockfile', () => { + if (!fs.existsSync(lockPath)) { + return + } + + const allow = (readJson(manifestPath).allowScripts ?? {}) as Record + const versions = installedVersions(readJson(lockPath)) + const stale: string[] = [] + + for (const key of Object.keys(allow)) { + const pin = splitPin(key) + + if (!pin) { + continue + } + + const installed = versions.get(pin.name) + + if (!installed?.has(pin.version)) { + stale.push(` "${key}" — lockfile resolves ${pin.name} to ${installed ? [...installed].join(', ') : ''}`) + } + } + + assert.deepEqual( + stale, + [], + `Stale allowScripts entries in ${label}:\n${stale.join('\n')}\n` + + "npm matches these by exact version, so each package's install script is " + + 'silently blocked. Update the pin to the installed version, or drop the ' + + 'entry if the dependency is gone.' + ) + }) + + test('every package with an install script has an allowScripts decision', () => { + if (!fs.existsSync(lockPath)) { + return + } + + const allow = (readJson(manifestPath).allowScripts ?? {}) as Record + const lock = readJson(lockPath) + const uncovered: string[] = [] + + for (const [entryPath, entry] of Object.entries( + (lock.packages ?? {}) as Record + )) { + if (!entryPath || !entry.hasInstallScript) { + continue + } + + const name = packageNameFor(entryPath, entry) + + if (!(`${name}@${entry.version}` in allow) && !(name in allow)) { + uncovered.push(` ${name}@${entry.version}`) + } + } + + assert.deepEqual( + uncovered, + [], + `Packages with install scripts and no allowScripts decision in ${label}:\n` + + `${uncovered.join('\n')}\n` + + 'npm blocks these. Add "@": true to allow, or "": false ' + + 'to deny permanently.' + ) + }) +}) diff --git a/website/package.json b/website/package.json index 635dbb5fc6bde..d040d879ef66e 100644 --- a/website/package.json +++ b/website/package.json @@ -59,6 +59,6 @@ }, "allowScripts": { "core-js@3.49.0": true, - "core-js-pure@3.49.0": true + "fsevents@2.3.3": true } }