fix(windows): check the MSYS breakaway denial on the rebuild path too

The Electron probe carried the marker check, but it lives inside
probeElectronNativeModules, which returns early whenever the Electron package
binary is unusable. Covered by another path is not this path checks -- and the
defect this whole change closes was a gate that looked like it checked.

Reading the binary needs neither a loadable Electron nor an executable target
arch, so assert it after the rebuild, beside the windows-process-tree
assertion that exists for the same reason: this is the addon copied into the
packaged app. Absent warns (a cross-platform rebuild need not leave a win32
addon on this disk); present and unmarked is fatal.

The fixtures now write a real addon file, because the gate reads the binary it
was told about rather than trusting the exports. Verified against the two real
binaries measured on the Windows host: the pre-#19068 build fails this path,
the build from current patched source passes.
This commit is contained in:
Neil
2026-09-10 22:16:38 -07:00
parent bf9176fda6
commit f9850f9bc5
3 changed files with 157 additions and 6 deletions
@@ -399,4 +399,75 @@ describe('rebuild-native-deps patched node-pty rebuild', () => {
}
})
}
// The Electron probe carries this check too, but it is skipped whenever the
// Electron package binary is unusable. Every job export predates the MSYS
// breakaway denial, so without reading the binary this step would hand the
// packaged app one that leaks every Git Bash child out of its pane's job.
it('fails a Windows rebuild that leaves an addon predating the MSYS breakaway denial', () => {
const projectDir = mkTempProject()
try {
writeFakeUsableElectronPackage(projectDir, { platform: 'win32' })
writeFakeElectronRebuild(projectDir)
writeFakeNodePtyConptyPayload(projectDir, 'x64', { cygwinBreakawayDenied: false })
writeFakeWindowsProcessTreeWithNodeAddonApi(projectDir)
const result = runRebuildScript(
projectDir,
{ npm_config_platform: 'win32', npm_config_arch: 'x64' },
['--platform=win32', '--arch=x64', '--force']
)
expect(result.status).not.toBe(0)
expect(result.stderr).toContain('predates the Cygwin/MSYS job-breakaway denial')
} finally {
removeTreeSync(projectDir)
}
})
it('accepts a Windows rebuild whose addon carries the denial', () => {
const projectDir = mkTempProject()
try {
writeFakeUsableElectronPackage(projectDir, { platform: 'win32' })
writeFakeElectronRebuild(projectDir)
writeFakeNodePtyConptyPayload(projectDir, 'x64')
writeFakeWindowsProcessTreeWithNodeAddonApi(projectDir)
const result = runRebuildScript(
projectDir,
{ npm_config_platform: 'win32', npm_config_arch: 'x64' },
['--platform=win32', '--arch=x64', '--force']
)
expect(result.status, result.stderr).toBe(0)
expect(result.stderr).not.toContain('job-breakaway denial')
} finally {
removeTreeSync(projectDir)
}
})
// A cross-platform rebuild does not necessarily leave a win32 addon on this
// disk. That must warn, not fail an install that was working.
it('warns rather than fails a Windows rebuild that produced no addon here', () => {
const projectDir = mkTempProject()
try {
writeFakeUsableElectronPackage(projectDir, { platform: 'win32' })
writeFakeElectronRebuild(projectDir)
writeFakeWindowsProcessTreeWithNodeAddonApi(projectDir)
const result = runRebuildScript(
projectDir,
{ npm_config_platform: 'win32', npm_config_arch: 'x64' },
['--platform=win32', '--arch=x64', '--force']
)
expect(result.status, result.stderr).toBe(0)
expect(result.stderr + result.stdout).toContain('could not check the MSYS job-breakaway')
} finally {
removeTreeSync(projectDir)
}
})
})
@@ -8,7 +8,7 @@ import {
writeFileSync
} from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { join, resolve } from 'node:path'
import { fileURLToPath } from 'node:url'
import { copyScriptWithLocalModules } from './script-module-dependencies.mjs'
@@ -310,10 +310,20 @@ process.exit(result.status ?? 0)
}
}
export function writeFakeNodePtyConptyPayload(projectDir, arch) {
export function writeFakeNodePtyConptyPayload(
projectDir,
arch,
{ cygwinBreakawayDenied = true } = {}
) {
const releaseDir = join(projectDir, 'node_modules', 'node-pty', 'build', 'Release')
mkdirSync(releaseDir, { recursive: true })
writeFileSync(join(releaseDir, 'conpty.node'), 'native addon')
writeFileSync(
join(releaseDir, 'conpty.node'),
Buffer.concat([
Buffer.from('native addon'),
cygwinBreakawayDenied ? CYGWIN_BREAKAWAY_MARKER : Buffer.alloc(0)
])
)
const sourceDir = join(
projectDir,
'node_modules',
@@ -328,12 +338,37 @@ export function writeFakeNodePtyConptyPayload(projectDir, arch) {
writeFileSync(join(sourceDir, 'OpenConsole.exe'), `OpenConsole.exe ${arch}`)
}
/**
* The wide literal `usesCygwinRuntime` holds, as it sits in a real addon. A
* fixture addon without it is a build that predates the MSYS breakaway denial,
* which is what these tests need to be able to represent.
*/
const CYGWIN_BREAKAWAY_MARKER = Buffer.from('msys-2.0.dll', 'utf16le')
function writeFakeNodePtyAddon(nodePtyDir, nativeDir, { cygwinBreakawayDenied }) {
const addonDir = resolve(join(nodePtyDir, 'lib'), nativeDir)
mkdirSync(addonDir, { recursive: true })
for (const nativeName of ['conpty', 'pty']) {
writeFileSync(
join(addonDir, `${nativeName}.node`),
Buffer.concat([
Buffer.from('MZ fake addon '),
cygwinBreakawayDenied ? CYGWIN_BREAKAWAY_MARKER : Buffer.alloc(0)
])
)
}
}
export function writeFakeLoadableNodePty(
projectDir,
{ nativeDir = 'prebuilds/pty', ownsPtyJob = true } = {}
{ nativeDir = 'prebuilds/pty', ownsPtyJob = true, cygwinBreakawayDenied = true } = {}
) {
const nodePtyDir = join(projectDir, 'node_modules', 'node-pty')
mkdirSync(join(nodePtyDir, 'lib'), { recursive: true })
// Why a real file: the job-ownership gate reads the addon it was told about,
// because every job export predates the MSYS breakaway denial and so cannot
// distinguish a current build from one that leaks Git Bash children.
writeFakeNodePtyAddon(nodePtyDir, nativeDir, { cygwinBreakawayDenied })
writeFileSync(join(nodePtyDir, 'index.js'), 'module.exports = {}\n')
writeFileSync(
join(nodePtyDir, 'lib', 'utils.js'),
@@ -434,11 +469,17 @@ export function writeNodePtyPatchFile(projectDir) {
writeFileSync(join(projectDir, 'config', 'patches', 'node-pty@1.1.0.patch'), 'patch marker\n')
}
export function writePatchedNodePtyBuildArtifacts(projectDir) {
export function writePatchedNodePtyBuildArtifacts(
projectDir,
{ cygwinBreakawayDenied = true } = {}
) {
const buildDir = join(projectDir, 'node_modules', 'node-pty', 'build', 'Release')
mkdirSync(buildDir, { recursive: true })
if (process.platform === 'win32') {
writeFileSync(join(buildDir, 'conpty.node'), '')
writeFileSync(
join(buildDir, 'conpty.node'),
cygwinBreakawayDenied ? CYGWIN_BREAKAWAY_MARKER : Buffer.alloc(0)
)
mkdirSync(join(buildDir, 'conpty'), { recursive: true })
writeFileSync(join(buildDir, 'conpty', 'conpty.dll'), '')
writeFileSync(join(buildDir, 'conpty', 'OpenConsole.exe'), '')
+39
View File
@@ -35,9 +35,12 @@ import {
readFileSync,
writeFileSync
} from 'node:fs'
import { createRequire } from 'node:module'
import { platform as osPlatform } from 'node:os'
import { join, resolve } from 'node:path'
const requireLocal = createRequire(import.meta.url)
const projectDir = process.cwd()
let cliOptions
try {
@@ -177,6 +180,7 @@ try {
})
restoreNodePtyWindowsConptyRuntime()
assertWindowsProcessTreeAddonIsPatched()
assertNodePtyConptyDeniesMsysBreakaway()
} catch (/** @type {any} */ err) {
console.error('[rebuild] Native module rebuild failed:', err?.message ?? err)
if (isWindowsNativeLockError(err)) {
@@ -230,6 +234,41 @@ function assertWindowsProcessTreeAddonIsPatched() {
)
}
/**
* The other half of the same problem, for the addon this rebuild just produced.
*
* The Electron probe below carries the marker check too, but it is skipped
* whenever the Electron package binary is unusable -- and "covered by another
* path" is not "this path checks". Reading the binary needs neither a loadable
* Electron nor an executable target arch, so it runs here regardless.
*
* Absent rather than unmarked warns: a cross-platform rebuild does not
* necessarily leave a win32 addon on this disk, and that must not fail an
* install that was working. A binary that IS there and predates the denial is
* fatal -- it is the one that ships.
*/
function assertNodePtyConptyDeniesMsysBreakaway() {
if (rebuildPlatform !== 'win32' || !modulesToRebuild.includes('node-pty')) {
return
}
const { assertCygwinBreakawayDenied } = requireLocal('./node-pty-job-ownership.cjs')
const addonPath = resolve(
projectDir,
'node_modules',
'node-pty',
'build',
'Release',
'conpty.node'
)
if (!existsSync(addonPath)) {
console.warn(
`[rebuild] no addon at ${addonPath}; could not check the MSYS job-breakaway denial.`
)
return
}
assertCygwinBreakawayDenied(addonPath, { dir: addonPath })
}
function restoreNodePtyWindowsConptyRuntime() {
if (rebuildPlatform !== 'win32' || !onlyModules.includes('node-pty')) {
return