diff --git a/config/packaged-runtime-node-modules.cjs b/config/packaged-runtime-node-modules.cjs index 3d4a968ea23..c88d70350eb 100644 --- a/config/packaged-runtime-node-modules.cjs +++ b/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 { PE_MACHINE, readPeMachine } = require('./scripts/windows-pe-machine.cjs') const projectDir = resolve(__dirname, '..') const requireFromProject = createRequire(join(projectDir, 'package.json')) @@ -367,6 +368,16 @@ function ensurePackagedNodePtyConptyRuntime(nodePtyDir, electronArch) { } } +/** Whether node-pty's source build holds a conpty.node the `electronArch` slice could load. */ +function conptyTargetsArch(nodePtyDir, electronArch) { + const releaseAddon = join(nodePtyDir, 'build', 'Release', 'conpty.node') + if (!existsSync(releaseAddon)) { + return false + } + // Null (not a PE) counts as unloadable, so a truncated or quarantined build keeps the fallback. + return readPeMachine(releaseAddon) === PE_MACHINE[normalizeNodePtyWindowsArch(electronArch)] +} + function prunePackagedNodePty(resourcesDir, electronPlatformName, electronArch) { const nodePtyDir = join(resourcesDir, 'node_modules', 'node-pty') if (!existsSync(nodePtyDir)) { @@ -388,14 +399,14 @@ 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')) - ) { + // 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 the target can load it -- 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 emit a correct arm64 addon. That slice kept the + // unpatched prebuild as a reachable fallback for any later load failure of build/Release. + // Read the PE header instead of guessing. + if (electronPlatformName === 'win32' && conptyTargetsArch(nodePtyDir, electronArch)) { const prebuildDir = join(nodePtyDir, 'prebuilds', `win32-${electronArch}`) for (const staleFallback of ['conpty.node', 'conpty.pdb']) { rmSync(join(prebuildDir, staleFallback), { force: true }) diff --git a/config/scripts/packaged-node-pty-prebuild-prune.test.mjs b/config/scripts/packaged-node-pty-prebuild-prune.test.mjs index c0b24b8a8a2..196e6aceb23 100644 --- a/config/scripts/packaged-node-pty-prebuild-prune.test.mjs +++ b/config/scripts/packaged-node-pty-prebuild-prune.test.mjs @@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' const require = createRequire(import.meta.url) const { prunePackagedNodePty } = require('../packaged-runtime-node-modules.cjs') +const { PE_MACHINE } = require('./windows-pe-machine.cjs') /** * node-pty's loader tries build/Release, then build/Debug, then @@ -22,18 +23,29 @@ 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 peAddon = (arch) => { + const buffer = Buffer.alloc(0x50) + buffer.write('MZ') + buffer.writeUInt32LE(0x40, 0x3c) + buffer.write('PE\0\0', 0x40) + buffer.writeUInt16LE(PE_MACHINE[arch], 0x44) + return buffer } const prebuilt = (name, arch = HOST_ARCH) => join(nodePty(), 'prebuilds', `win32-${arch}`, name) /** What a real packaged win32 prebuilds/ 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 reachable 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) diff --git a/docs/reference/windows-msys-job-breakaway.md b/docs/reference/windows-msys-job-breakaway.md index f4221078afb..d50e3413b28 100644 --- a/docs/reference/windows-msys-job-breakaway.md +++ b/docs/reference/windows-msys-job-breakaway.md @@ -119,18 +119,29 @@ never carries the patch: | -------------------- | ----------------------------- | ---------------- | ------------------ | | same host, same arch | patched | yes | `build/Release` | | cross host | absent, cannot be cross-built | no | the prebuild | -| cross arch, built | patched, target arch | no | `build/Release` | +| cross arch, built | patched, target arch | yes | `build/Release` | | cross arch, failed | the host's arch | no | the prebuild | `beforeBuild` runs `rebuild-native-deps.mjs --platform=win32 --arch=`, so a cross-arch slice normally does get a patched `build/Release` for the target — -row three is a correct package whose leftover prebuild is never reached. -`prunePackagedNodePty` keeps that prebuild anyway, because its guard is -`electronArch === process.arch` rather than the arch of the binary. +row three is a correct package. `prunePackagedNodePty` asks the same question the +loader does, reading the PE machine of `build/Release` rather than comparing +`electronArch` to `process.arch`, so row three's leftover prebuild goes. Keying +off the host arch kept it: unreached in the normal case, but still the binary the +loader takes if `build/Release` ever fails to load for an unrelated reason — an +AV quarantine, a missing dependency — which is the silent fall-through this whole +gate exists to close. Rows two and four keep the prebuild because it is the only +thing there the target could load. Measured on Windows 11 x64 with the VS 2022 +ARM64 cross toolset: `node-gyp rebuild --arch=arm64` does emit a `conpty.node` +with machine `0xaa64`, so row three is a real package shape — but as of this +writing no release produces it, because `electron-builder --win` is run without +an arch and packages x64 only. -So presence alone cannot separate row three from row four, and failing on any -unmarked file present would reject a correct package with advice its builder -could not act on. `verifyPackagedConptyBreakawayMarker` instead resolves the +The verifier still does not key on presence: the prune is the step it is +checking, and `build/Debug` is never pruned, so an unmarked file beside a +correct `build/Release` cannot by itself separate row three from row four, and +failing on one would reject a correct package with advice its builder could not +act on. `verifyPackagedConptyBreakawayMarker` instead resolves the addon the way the loader does — first candidate whose PE `IMAGE_FILE_HEADER` machine matches the target — and checks the marker on that one. A package with no candidate at all, or none of the target's architecture, is refused: it has no