fix(windows): stop shipping an unpatched conpty prebuild a cross-arch package can load

config/electron-builder.config.cjs

@@ -337,6 +337,7 @@ module.exports = {
     // Node cannot load cross-arch. `Arch` enum: ia32=0, x64=1, armv7l=2,
     // arm64=3, universal=4 (universal contains the host slice, so run it).
     const archEnumByNodeArch = { ia32: 0, x64: 1, armv7l: 2, arm64: 3 }
+    const nodeArchByArchEnum = { 0: 'ia32', 1: 'x64', 2: 'armv7l', 3: 'arm64' }
     const hostArchEnum = archEnumByNodeArch[process.arch]
     const canExecuteTargetArch = context.arch === hostArchEnum || context.arch === 4
     if (context.electronPlatformName === 'win32') {
@@ -347,7 +348,10 @@ module.exports = {
         // MSYS breakaway marker is a file read, and skipping it is how a
         // cross-host Windows release could ship the orphan bug.
         console.log('[verify-packaged-node-pty] skipped cross-platform or cross-arch package')
-        verifyPackagedConptyBreakawayMarker(resourcesDir)
+        // The arch names the prebuild directory the loader would fall through to.
+        verifyPackagedConptyBreakawayMarker(resourcesDir, {
+          arch: nodeArchByArchEnum[context.arch]
+        })
       }
     }
     verifySkillsCliRuntime(join(resourcesDir, 'app.asar.unpacked', 'out'), resourcesDir, {

config/packaged-runtime-node-modules.cjs

@@ -9,6 +9,7 @@ const {
 } = require('node:fs')
 const { dirname, join, resolve } = require('node:path')
 const { builtinModules, createRequire } = require('node:module')
+const { conptyTargetsArch } = require('./scripts/node-pty-job-ownership.cjs')
 
 const projectDir = resolve(__dirname, '..')
 const requireFromProject = createRequire(join(projectDir, 'package.json'))
@@ -388,17 +389,19 @@ function prunePackagedNodePty(resourcesDir, electronPlatformName, electronArch)
   // require, and its caller resolves null with silent: true), and removes the
   // winpty backend that node-pty still selects below Windows build 18309.
   //
-  // Why the arch check: a cross-arch package copies the host's build/Release,
-  // so its mere presence does not mean it matches electronArch -- deleting the
-  // target-arch prebuild would then remove the only loadable binary.
-  if (
-    electronPlatformName === 'win32' &&
-    electronArch === process.arch &&
-    existsSync(join(nodePtyDir, 'build', 'Release', 'conpty.node'))
-  ) {
-    const prebuildDir = join(nodePtyDir, 'prebuilds', `win32-${electronArch}`)
-    for (const staleFallback of ['conpty.node', 'conpty.pdb']) {
-      rmSync(join(prebuildDir, staleFallback), { force: true })
+  // Why the arch check: a cross-HOST package can copy a build/Release that is not a Windows
+  // binary at all, so its mere presence does not mean it matches electronArch -- deleting the
+  // target-arch prebuild would then remove the only loadable binary. This used to approximate
+  // that with `electronArch === process.arch`, which also skipped the arm64 slice cross-built on
+  // an x64 Windows host -- a rebuild that DOES produce a correct arm64 addon. That slice then
+  // shipped the unpatched prebuild as a live fallback. Read the header instead of guessing.
+  if (electronPlatformName === 'win32') {
+    const releaseAddon = join(nodePtyDir, 'build', 'Release', 'conpty.node')
+    if (conptyTargetsArch(releaseAddon, electronArch)) {
+      const prebuildDir = join(nodePtyDir, 'prebuilds', `win32-${electronArch}`)
+      for (const staleFallback of ['conpty.node', 'conpty.pdb']) {
+        rmSync(join(prebuildDir, staleFallback), { force: true })
+      }
     }
   }
 

config/scripts/node-pty-job-ownership.cjs

@@ -24,6 +24,40 @@ const NODE_PTY_JOB_EXPORTS = ['listJobProcessIds', 'terminateJob', 'assignCurren
  */
 const CYGWIN_BREAKAWAY_MARKER = Buffer.from('msys-2.0.dll', 'utf16le')
 
+/** PE `Machine`, for the Windows slices we package. Names match electron-builder's Arch enum. */
+const PE_MACHINE_BY_ARCH = Object.freeze({ ia32: 0x14c, x64: 0x8664, arm64: 0xaa64 })
+
+/**
+ * Whether a `.node` is a PE built for `arch`, or null when the file cannot be read as one.
+ *
+ * Why the packaged prune needs this: it used to approximate "build/Release holds the target's
+ * addon" with `electronArch === process.arch`, which is false for an arm64 slice cross-built on
+ * x64 even though that rebuild does produce a correct arm64 addon. Reading the header answers the
+ * question the proxy was standing in for. Shape mirrors readElfMachine() in
+ * verify-linux-glibc-floor.cjs.
+ */
+function conptyTargetsArch(addonPath, arch) {
+  const expected = PE_MACHINE_BY_ARCH[arch]
+  if (expected === undefined) {
+    return null
+  }
+  let binary
+  try {
+    binary = readFileSync(addonPath)
+  } catch {
+    return null
+  }
+  // PE header offset lives at 0x3c; `Machine` is the first field after the 4-byte signature.
+  if (binary.length < 0x40) {
+    return null
+  }
+  const peHeaderOffset = binary.readUInt32LE(0x3c)
+  if (peHeaderOffset + 6 > binary.length) {
+    return null
+  }
+  return binary.readUInt16LE(peHeaderOffset + 4) === expected
+}
+
 /**
  * Absolute path of the addon `loadNativeModule` just resolved.
  *
@@ -95,5 +129,6 @@ function assertCygwinBreakawayDenied(addonPath, native) {
 module.exports = {
   assertNodePtyJobOwnership,
   assertCygwinBreakawayDenied,
+  conptyTargetsArch,
   nodePtyAddonPath
 }

config/scripts/packaged-node-pty-prebuild-prune.test.mjs

@@ -22,18 +22,30 @@ describe('prunePackagedNodePty: the Windows conpty fallback', () => {
   const HOST_ARCH = process.arch
 
   const nodePty = () => join(resources, 'node_modules', 'node-pty')
-  const write = (relative) => {
+  const write = (relative, contents = 'x') => {
     const target = join(nodePty(), relative)
     mkdirSync(join(target, '..'), { recursive: true })
-    writeFileSync(target, 'x')
+    writeFileSync(target, contents)
+  }
+
+  /** Minimal PE far enough to carry a `Machine`, which is what the prune now reads. */
+  const PE_MACHINE_BY_ARCH = { x64: 0x8664, arm64: 0xaa64 }
+  const peAddon = (arch) => {
+    const buffer = Buffer.alloc(0x50)
+    buffer.write('MZ')
+    buffer.writeUInt32LE(0x40, 0x3c)
+    buffer.write('PE\0\0', 0x40)
+    buffer.writeUInt16LE(PE_MACHINE_BY_ARCH[arch], 0x44)
+    return buffer
   }
   const prebuilt = (name, arch = HOST_ARCH) => join(nodePty(), 'prebuilds', `win32-${arch}`, name)
 
   /** What a real packaged win32 prebuilds/<arch> directory holds. */
   const SIBLINGS = ['conpty_console_list.node', 'pty.node', 'winpty.dll', 'winpty-agent.exe']
 
-  const seedWindowsTree = (arch = HOST_ARCH) => {
-    write(join('build', 'Release', 'conpty.node'))
+  /** `buildArch` is what build/Release actually holds, which is not always the slice's arch. */
+  const seedWindowsTree = (arch = HOST_ARCH, buildArch = arch) => {
+    write(join('build', 'Release', 'conpty.node'), peAddon(buildArch))
     write(join('third_party', 'conpty', 'v1', `win10-${arch}`, 'conpty.dll'))
     write(join('third_party', 'conpty', 'v1', `win10-${arch}`, 'OpenConsole.exe'))
     write(join('prebuilds', `win32-${arch}`, 'conpty.node'))
@@ -79,23 +91,44 @@ describe('prunePackagedNodePty: the Windows conpty fallback', () => {
     expect(existsSync(prebuilt('conpty.node'))).toBe(true)
   })
 
-  it('keeps the fallback on a cross-arch package, where build/Release is the host arch', () => {
-    // electron-builder --win --arm64 on an x64 host copies an x64
-    // build/Release; deleting the arm64 prebuild would remove the only binary
-    // the shipped app could load.
+  it('keeps the fallback when build/Release holds a different arch than the slice', () => {
+    // A cross-HOST package can copy a build/Release that the target cannot load;
+    // deleting the prebuild would remove the only binary the shipped app has.
     const target = HOST_ARCH === 'arm64' ? 'x64' : 'arm64'
-    seedWindowsTree(target)
+    seedWindowsTree(target, HOST_ARCH)
 
     prunePackagedNodePty(resources, 'win32', target)
 
     expect(existsSync(prebuilt('conpty.node', target))).toBe(true)
   })
 
+  it('removes the fallback on a cross-arch package whose build/Release IS the slice arch', () => {
+    // Measured on Windows: `electron-builder --win --arm64` on an x64 host does
+    // cross-rebuild a correct arm64 addon. Keying off the host arch skipped this
+    // case, so the slice shipped the unpatched prebuild as a live fallback.
+    const target = HOST_ARCH === 'arm64' ? 'x64' : 'arm64'
+    seedWindowsTree(target, target)
+
+    prunePackagedNodePty(resources, 'win32', target)
+
+    expect(existsSync(prebuilt('conpty.node', target))).toBe(false)
+    expect(existsSync(prebuilt('pty.node', target))).toBe(true)
+  })
+
+  it('keeps the fallback when build/Release is not a readable PE at all', () => {
+    seedWindowsTree()
+    write(join('build', 'Release', 'conpty.node'), 'not-a-pe')
+
+    prunePackagedNodePty(resources, 'win32', HOST_ARCH)
+
+    expect(existsSync(prebuilt('conpty.node'))).toBe(true)
+  })
+
   it.each([
     ['darwin', 'arm64'],
     ['linux', 'x64']
   ])('leaves %s prebuilds alone', (platform, arch) => {
-    write(join('build', 'Release', 'conpty.node'))
+    write(join('build', 'Release', 'conpty.node'), peAddon('x64'))
     write(join('prebuilds', `${platform}-${arch}`, 'pty.node'))
 
     prunePackagedNodePty(resources, platform, arch)

config/scripts/verify-packaged-node-pty-job-ownership.cjs

@@ -12,6 +12,13 @@ function packagedConptyPath(resourcesDir) {
   return join(resourcesDir, 'node_modules', 'node-pty', 'build', 'Release', 'conpty.node')
 }
 
+/** The last entry in node-pty's loader order, and the one the published tarball fills. */
+function prebuiltConptyPath(resourcesDir, arch) {
+  return arch
+    ? join(resourcesDir, 'node_modules', 'node-pty', 'prebuilds', `win32-${arch}`, 'conpty.node')
+    : null
+}
+
 function loadPackagedConpty(resourcesDir) {
   const packagedRequire = createRequire(join(resourcesDir, 'package.json'))
   const utilsPath = packagedRequire.resolve('./node_modules/node-pty/lib/utils')
@@ -42,16 +49,35 @@ function verifyPackagedNodePtyJobOwnership(resourcesDir, options = {}) {
  * release built elsewhere could ship a node-pty that leaks every MSYS pane
  * child out of its job. Reading the binary needs neither.
  *
- * Absence is logged rather than thrown: an unrecognised layout must not fail a
- * release that was packaging fine, and the export check still covers the
- * same-host case. A binary that IS there and lacks the marker is fatal.
+ * Absence is logged rather than thrown ONLY when nothing else would load: an
+ * unrecognised layout must not fail a release that was packaging fine, and the
+ * export check still covers the same-host case. A binary that IS there and
+ * lacks the marker is fatal.
+ *
+ * Why the prebuild is checked too: node-pty's loader falls through
+ * build/Release -> build/Debug -> prebuilds/<platform>-<arch>, so a package
+ * with no build/Release addon loads the prebuild -- and the published prebuild
+ * has never carried the breakaway denial. Warning there would pass exactly the
+ * package that ships the bug.
  */
 function verifyPackagedConptyBreakawayMarker(resourcesDir, options = {}) {
   // Deliberately no host-platform gate: the caller has already established that
   // the *target* is Windows, and gating on the host is the very skip this
   // closes.
+  const exists = options.exists ?? existsSync
   const addonPath = (options.packagedConptyPath ?? packagedConptyPath)(resourcesDir)
-  if (!(options.exists ?? existsSync)(addonPath)) {
+  if (!exists(addonPath)) {
+    const fallbackPath = (options.prebuiltConptyPath ?? prebuiltConptyPath)(
+      resourcesDir,
+      options.arch
+    )
+    if (fallbackPath && exists(fallbackPath)) {
+      assertCygwinBreakawayDenied(fallbackPath, { dir: fallbackPath })
+      console.log(
+        '[verify-packaged-node-pty] OK — packaged ConPTY denies MSYS job breakaway (prebuild fallback)'
+      )
+      return
+    }
     console.warn(
       `[verify-packaged-node-pty] no addon at ${addonPath}; could not check the MSYS ` +
         'job-breakaway denial for this cross-host package.'

config/scripts/verify-packaged-node-pty-job-ownership.test.mjs

@@ -109,6 +109,62 @@ describe('verifyPackagedConptyBreakawayMarker', () => {
     warn.mockRestore()
   })
 
+  // node-pty falls through build/Release -> build/Debug -> prebuilds/<platform>-<arch>, so a
+  // package with no build/Release addon loads the prebuild. Warning there passes exactly the
+  // package that ships the bug.
+  it('fails when the only loadable addon is the unpatched prebuild', () => {
+    expect(() =>
+      verifyPackagedConptyBreakawayMarker('resources', {
+        arch: 'arm64',
+        packagedConptyPath: () => join(fixtureDir, 'absent.node'),
+        prebuiltConptyPath: () => PRE_MSYS_ADDON
+      })
+    ).toThrow(/predates the Cygwin\/MSYS job-breakaway denial/)
+  })
+
+  it('accepts a package whose only addon is a patched prebuild', () => {
+    expect(() =>
+      verifyPackagedConptyBreakawayMarker('resources', {
+        arch: 'arm64',
+        packagedConptyPath: () => join(fixtureDir, 'absent.node'),
+        prebuiltConptyPath: () => CURRENT_ADDON
+      })
+    ).not.toThrow()
+  })
+
+  it('still warns when neither the build output nor a prebuild is there', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    expect(() =>
+      verifyPackagedConptyBreakawayMarker('resources', {
+        arch: 'arm64',
+        packagedConptyPath: () => join(fixtureDir, 'absent.node'),
+        prebuiltConptyPath: () => join(fixtureDir, 'also-absent.node')
+      })
+    ).not.toThrow()
+    expect(warn).toHaveBeenCalledWith(expect.stringContaining('could not check the MSYS'))
+    warn.mockRestore()
+  })
+
+  it('looks where the loader would find the prebuild fallback', () => {
+    const exists = vi.fn().mockReturnValue(false)
+    verifyPackagedConptyBreakawayMarker(join('out', 'win-arm64-unpacked', 'resources'), {
+      arch: 'arm64',
+      exists
+    })
+    expect(exists).toHaveBeenCalledWith(
+      join(
+        'out',
+        'win-arm64-unpacked',
+        'resources',
+        'node_modules',
+        'node-pty',
+        'prebuilds',
+        'win32-arm64',
+        'conpty.node'
+      )
+    )
+  })
+
   it('looks where electron-builder actually lands the addon', () => {
     const exists = vi.fn().mockReturnValue(false)
     verifyPackagedConptyBreakawayMarker(join('out', 'win-unpacked', 'resources'), { exists })