diff --git a/config/scripts/rebuild-native-deps-node-pty.test.mjs b/config/scripts/rebuild-native-deps-node-pty.test.mjs index c3a8f9bbd83..6826a871370 100644 --- a/config/scripts/rebuild-native-deps-node-pty.test.mjs +++ b/config/scripts/rebuild-native-deps-node-pty.test.mjs @@ -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) + } + }) }) diff --git a/config/scripts/rebuild-native-deps-test-fixtures.mjs b/config/scripts/rebuild-native-deps-test-fixtures.mjs index db5af45a454..719f53d910f 100644 --- a/config/scripts/rebuild-native-deps-test-fixtures.mjs +++ b/config/scripts/rebuild-native-deps-test-fixtures.mjs @@ -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'), '') diff --git a/config/scripts/rebuild-native-deps.mjs b/config/scripts/rebuild-native-deps.mjs index 6aaa6526f7e..60f5d987200 100644 --- a/config/scripts/rebuild-native-deps.mjs +++ b/config/scripts/rebuild-native-deps.mjs @@ -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