mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
fix(windows): prune the unpatched conpty prebuild by header, not host arch (#20048)
* fix(windows): prune the unpatched conpty prebuild by header, not host arch `prunePackagedNodePty` deleted the published `prebuilds/win32-<arch>/conpty.node` only when `electronArch === process.arch`. That proxy stood in for "build/Release holds an addon this slice can load", and it is false for the arm64 slice cross-built on an x64 Windows host — a rebuild that DOES emit a correct arm64 addon. That slice shipped the unpatched prebuild. Nothing loads it today: `verifyPackagedConptyBreakawayMarker` resolves the addon the way node-pty's loader does, so the patched `build/Release` wins and the release passes correctly. But the loader swallows every require failure and falls through, so an AV quarantine or a missing dependency on `build/Release` hands the pane to that unpatched prebuild — the silent downgrade the gate exists to close, with the binary still sitting in the package. Read the PE `Machine` field instead of guessing, reusing `readPeMachine` from the verifier's `windows-pe-machine.cjs` so prune and verifier ask one question. A missing, truncated or non-PE `build/Release` reads as unloadable and keeps the prebuild, which is what the true cross-host case needs: packaging Windows from macOS leaves no Windows binary in `build/Release`, and removing the prebuild there would leave the package with no ConPTY at all. Mutation-proven: restoring the `electronArch === process.arch` guard fails exactly the two new rows in packaged-node-pty-prebuild-prune. * docs(windows): note the cross-arch conpty slice is real but not yet built
This commit is contained in:
@@ -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 })
|
||||
|
||||
@@ -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/<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 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)
|
||||
|
||||
@@ -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=<target>`, 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
|
||||
|
||||
Reference in New Issue
Block a user