Merge pull request #82171 from NousResearch/bb/allowscripts-sync

fix(build): unblock Windows desktop builds — allowScripts drift + get-windows self-heal

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]

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 {

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
   }
 }

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<string, unknown> {
+  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<string, unknown>): Map<string, Set<string>> {
+  const versions = new Map<string, Set<string>>()
+
+  for (const [lockPath, entry] of Object.entries(
+    (lock.packages ?? {}) as Record<string, LockPackage>
+  )) {
+    if (!lockPath || !entry.version) {
+      continue
+    }
+
+    const name = packageNameFor(lockPath, entry)
+    const seen = versions.get(name) ?? new Set<string>()
+
+    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<string, boolean>
+    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(', ') : '<nothing>'}`)
+      }
+    }
+
+    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<string, boolean>
+    const lock = readJson(lockPath)
+    const uncovered: string[] = []
+
+    for (const [entryPath, entry] of Object.entries(
+      (lock.packages ?? {}) as Record<string, LockPackage>
+    )) {
+      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 "<name>@<version>": true to allow, or "<name>": false ' +
+        'to deny permanently.'
+    )
+  })
+})

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
   }
 }