mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 08:02:32 +00:00
fix(windows): reject a node-pty addon that predates the MSYS breakaway denial (#20047)
* docs(windows): record the measured MSYS job-breakaway mechanism The per-PTY job already denies JOB_OBJECT_LIMIT_BREAKAWAY_OK for Cygwin/MSYS shells (#19068), but nothing records why, and a conpty.node built before that commit fails windows-msys-job.win32.test.ts in a way that reads as a source defect. Measured on a real Windows 11 host: both the plain and the exec- replacement Git Bash shapes leak, the escape is the MSYS runtime's own spawn/exec (fork keeps membership), and a single-variable A/B on usesCygwinRuntime flips the result 0/2 -> 4/4. Also names the gap the failure hid behind: node-pty-job-ownership.cjs asserts symbol presence, which cannot distinguish patch revisions. * fix(windows): reject a node-pty addon that predates the MSYS breakaway denial The native-runtime gate asserted only that terminateJob, listJobProcessIds and assignCurrentProcessToJob were exported. All three predate the Cygwin/MSYS breakaway denial, so an addon built before it passes every gate, isPtyJobOwnershipAvailable() returns true, and windows-pty-job.win32.test.ts passes 6/6 -- while every Git Bash child is created outside its pane's job and survives terminatePtyJob. Read the resolved .node and require the wide msys-2.0.dll literal that usesCygwinRuntime holds, the way stagedRelayAddonIsUnpatched() already tells a patched windows-process-tree addon from a published one. An addon the caller cannot name is refused rather than skipped: a gate that cannot see its subject is not a gate. Verified against real binaries on a Windows 11 host: the shared checkout's pre-#19068 build errors, a build from current patched source passes, a missing path errors. Also closes the cross-host packaging skip. The export half has to load the addon so it cannot run when the packaging host is not the target, which is how a Windows release built elsewhere could ship this. The marker is a file read and needs neither; an unrecognised layout warns rather than fails a release that was packaging fine. * 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. * fix(windows): check the marker on every ConPTY path the packaged app can load The packaged marker check read one hard-coded path, `build/Release/conpty.node`, and warned when it was absent. `loadNativeModule` tries `build/Release`, then `build/Debug`, then `prebuilds/win32-<arch>`, swallowing each failure, and `prunePackagedNodePty` drops the published prebuild only when a same-arch `build/Release` exists to replace it. So the two packages the check was added for were the two it could not see: - cross-host: no host but Windows can build conpty.node, so there is no `build/Release` and the prebuild is what ships. The check warned and returned. - cross-arch: `build/Release` is the packaging host's own arch, patched and marked, so the check printed OK -- while the target app cannot load it and falls through to the unmarked prebuild underneath. Measured, not assumed: both published Windows prebuilds in the node-pty tarball contain neither `msys-2.0.dll` nor `cygwin1.dll` in any encoding. They are the binary that leaks every MSYS pane child out of its job. It now sweeps every candidate present for the *target* arch and refuses a package with no candidate at all, which is a package with no ConPTY backend rather than a layout to shrug at. It runs for every Windows slice instead of only the branch the export check skips, so deleting the export check cannot silently take it too. A stale source build keeps the rebuild advice; the prebuild gets the advice that actually works, which is to package the slice on a Windows host of that arch. Also: the marker constant was re-typed in four places and was tied to the C++ literal that produces it by nothing at all, so editing the patch would have left a gate that fails every correctly rebuilt addon and tells the developer to do the one thing that cannot help. The fixtures now take the constant from the gate, and a test asserts the patch still adds `L"msys-2.0.dll"` to conpty.cc. And the rebuild path treated a missing addon as a warning even on the host that will run the install, where node-pty would fall through to that same prebuild. The verdict is now a value, so it is tested without a platform gate. * fix(windows): resolve the packaged ConPTY the way its loader does Sweeping every candidate and demanding the marker on all of them was wrong in the one case it was meant to make safe. `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; `prunePackagedNodePty` keeps the prebuild anyway because its guard is `electronArch === process.arch` rather than the arch of the binary. That package is correct and its leftover prebuild is never reached, and the sweep failed it -- telling whoever ran it to package on a Windows arm64 host, which is both the wrong remedy and one no runner here can offer. Presence cannot separate that package from the one whose cross-arch rebuild quietly emitted the host's architecture, because the only difference is the arch of `build/Release`. So the gate now resolves the addon the way `loadNativeModule` does -- first candidate whose PE `IMAGE_FILE_HEADER.Machine` matches the target, walking root-then-lib for each layout in node-pty's own order -- and checks the marker on the one that will actually run. A package with no candidate, or none of the target's architecture, is refused: it has no ConPTY backend either way, and the second is exactly what a silently host-arch cross-build looks like. The PE machine reader already existed, privately, in the relay addon builder that needed the same "a cross-build cannot silently emit host arch" guarantee. It is now shared rather than copied. Two seams were unreachable from anything but Windows, so nothing tested them: - the afterPack hook's win32 block was an inline if/else that only a source-text assertion could inspect, and that assertion could not tell the difference between the check running and the check being wrapped in `try {} catch {}`. It is now `verifyPackagedWindowsNodePty`, and "the marker check runs even where the export check cannot" is four spied assertions instead of a string match. - the rebuild path's verdict read `process` directly, so the branch that fires only on the host being rebuilt for was dead on every other host. It now takes the host as arguments, and the fs checks, the warning and the failure are all exercised from macOS. Fixtures write a real PE header rather than `MZ fake addon`, since the gate now reads one. The machine table is pinned to the documented IMAGE_FILE_MACHINE values, because every fixture builds its header from that table and a table wrong in both entries would otherwise agree with itself. * fix(windows): say why the packaged ConPTY fell back, not just that it did The previous commit resolved the addon by architecture but still had one message for every way the resolution could land on the published prebuild. Those ways want opposite remedies, and the one it printed was the remedy the commit before it had just called wrong: - no source build in the package at all — the slice has to be built somewhere that can build node-pty for the target arch. - a source build that is there but is the packaging host's architecture, because the cross-arch rebuild did not honour `--arch` — re-running that rebuild is the fix, and "package on a Windows arm64 host" is neither necessary nor possible. The second is the common one, since node-pty publishes a prebuild for both Windows arches and prune keeps the target's on every cross-arch package. So the old text fired mostly on the case it described least. It now reports which source builds were skipped and the machine field each carried, and names the rebuild command. "Nothing the target can load" had the same problem in reverse: a zero-length or truncated `conpty.node` got a cross-architecture diagnosis. Every candidate is now named with what was actually read, including "not a PE image". The rebuild path asserts the architecture too. A rebuild that ignored `--arch` was otherwise only visible at packaging, two steps from the command that fixes it. Arches with no known machine value are left unjudged rather than guessed at. Two things the extraction broke or nearly broke, both found by mutation: - the shared PE reader answers `null` where the relay builder's private copy returned a number, which would have turned its "node-gyp ignored --arch" error into a `TypeError`. Both callers now go through `describePeMachine`. - the rebuild fixtures stage a script's co-located modules by walking its imports, and the walker only understood `from '...'` — so the gate's new `require('./windows-pe-machine.cjs')` was left behind and every subprocess test failed with a resolution error, which is the exact failure its own comment warns about. It now follows `require` and bare side-effect `import` as well, and has tests; the fixture stages the gate by walking it rather than by naming one file. Fixtures write real PE headers through one shared builder instead of three hand-rolled ones. * fix(windows): run the node-pty addon gates on the Windows job that can `rebuild-native-deps-node-pty.test.mjs` carries four `skipIf(platform !== 'win32')` tests. The full suite runs on ubuntu, and the Windows PR job runs an explicit file list that never named this file -- so those tests were skipped on Linux and never reached anywhere else. Three of them predate this branch. The Windows job is added the four node-pty addon suites plus the module-walker one; the comment above that list already says why it is the right place, which is that the addon assertions only hold once natives have been rebuilt. Running the path-joining suites there also covers the separator this gate's candidate list is built from. The rest is round-three review: - the rebuild-time arch assertion told a reader "node-gyp did not honour --arch" about a file that was not a PE image at all, which is a truncated or quarantined artifact and a different command to run. The two now read differently, and neither claims the other's cause. Same fix the packaged gate had one commit ago, in the place that had not had it yet. - the missing-addon error said node-pty "would load" a prebuild without checking it is there. It says "fall through to" now, which is true either way. - `isLoadableByArch` had no caller left once the packaged gate started needing the raw machine field for its message. Removed rather than kept warm. - each candidate's header is read once instead of up to three times. - the module walker's comment claimed every shape that reaches a co-located module; it does not follow `projectRequire`/`requireLocal`, and it must not -- those specifiers resolve against the project root, so following one stages the wrong path and the copy fails. Proven by trying: widening the pattern to require-shaped names broke nine tests on `projectRequire('./config/scripts/...')`. The comment now says what it follows and why it stops there. - a new test resolved a file URL with `.pathname`, which keeps the drive-letter slash on Windows -- the very job this commit adds it to. * docs(windows): put the superseded export-only gate in the past tense It describes what used to pass a broken addon, so present tense reads as a description of the gate the same document then explains replacing it. * fix(windows): repair what running the node-pty suites on Windows exposed Putting these files on the Windows job turned four assertions red on the first run. Three of them were in tests that carried `skipIf(platform !== 'win32')` and had therefore never executed anywhere, on any branch. - `writeFakeElectronRebuild` emitted the `windows-process-tree` addon a real rebuild leaves but never node-pty's, so every Windows test of the rebuild path ran against a tree no real rebuild can produce: node-pty "rebuilt" with nothing in `build/Release`. The new same-host check reads that state correctly and said so. The fake rebuild now writes `build/Release/conpty.node` when it was asked to rebuild node-pty for win32, with the marker and the target machine. - `mkTempProject` never staged `windows-process-tree-creation-time.cjs`. The rebuild script reaches it through `projectRequire`, which resolves against the project root, so the module walker cannot follow it and must not try. Staged by name, with a comment saying which of the two it is. Without it the windows-process-tree probe failed to load its own checker and the module joined `modulesToRebuild`, which is the second and third red assertion. - the two `nodePtyAddonPath` cases compared against a literal POSIX string. `resolve` returns a drive letter and backslashes on Windows, so they could only ever pass off it. Built from segments now, which still pins the `..` traversal that is the point of the test. Verified on macOS: ensure-native-runtime-job-ownership, verify-packaged-node-pty-job-ownership, windows-pe-machine, script-module-dependencies, rebuild-native-deps-node-pty, rebuild-native-deps, rebuild-native-deps-windows-process-tree, ensure-native-runtime -- 109 passed, 6 skipped. The 6 are the Windows-gated rebuild tests, which is the job this change is aimed at; Windows CI is the arbiter. * fix(windows): give the packaged fallback a third verdict, for a file that is no image The packaged gate had two remedies for landing on the published prebuild and picked between them on `!prebuilt`, which puts a truncated, empty or quarantined `build/Release/conpty.node` in the cross-arch bucket: "the source build beside it is the wrong architecture ... re-run with --arch". It is not the wrong architecture, it is not an architecture, and `--arch` is not the command. The rebuild-path gate was split for exactly this a commit ago; this is the same split in the place that had not had it. Also from review of the settled state: - the stale-source-build branch ended in a call that happened to throw, so a reader could not see it was terminal and the file was read twice to get there. The verdict is now an Error the caller throws, built once from the read it already did, and shared with `assertCygwinBreakawayDenied` rather than copied. - four injection seams had no consumer in production or in tests (`deniesBreakaway`, `peMachine`, and `exists`/`peMachine` on the rebuild verdict). An unused seam is a way for the tested path and the real one to drift apart; the tests drive both with real files. Removed. - the loader table existed in a docblock and in the reference doc, already disagreeing about row four. The docblock cites the doc now. - `peImage` stamped machine `0x0000` for an arch it had no value for, because `writeUInt16LE(undefined)` coerces to zero. A fixture that quietly invents the field the gates read is the same species of silent lie the gates exist to catch; it throws, and a test holds it to that. - a test named for refusing an unreadable candidate asserted only that something threw. Renamed to what it proves. * fix(windows): make the rebuild fixtures represent a tree that can exist Second round of what running these suites on Windows exposed. The module the walker could not stage is now staged, so the probe reached its own checker and the real reasons surfaced: - `writeFakeWindowsProcessTree` exported `{}`. The creation-time gate reads `supportedProcessDataFlags` off the addon and calls its absence "the tarball prebuilt, not a build of the patched source" — correctly. The fixture predates that gate and, being Windows-only, never met it. The healthy fake now reports the flag, taken from the gate's own constant. Two tests were failing on this, the second only because the module then joined `modulesToRebuild`. - `rebuilds a loadable ConPTY native that lacks Orca job ownership` asked for a node-pty rebuild in a tree where node-pty had none of the payload its package ships. It gets `writeFakeNodePtyConptyPayload` like its two siblings. I also tried making the fake rebuild emit `build/Release/conpty.node` the way a real one does, and backed it out: `restoreNodePtyWindowsConptyRuntime` keys off that file and then reads `third_party/conpty`, so emitting it in a tree without the package payload turns one honest gap into an ENOENT two steps away. The payload fixture is where "node-pty has its addon" belongs. macOS: ensure-native-runtime-job-ownership, verify-packaged-node-pty-job-ownership, windows-pe-machine, script-module-dependencies, rebuild-native-deps-node-pty, rebuild-native-deps, rebuild-native-deps-windows-process-tree, ensure-native-runtime — 112 passed, 6 skipped. The 6 are the Windows-gated rebuild tests; Windows CI is the arbiter and is why they are on that job now. * fix(windows): register the node-pty addon suites in the scope list too Putting the five suites in the Windows lane's vitest argv gets them run once the job starts; `WINDOWS_PACKAGE_TESTS` in `pr-code-change-scope.mjs` is what decides whether the job starts at all. Only the argv was updated, so a PR touching just `rebuild-native-deps-node-pty.test.mjs` would not have started the Windows job, and its four Windows-only cases — including the same-host-absent one added here — would have run on no machine for that PR. Exactly the shape of gap this branch is about. Both lists now name all five, and `windows-pe-machine`, `windows-pe-image-fixture` and `script-module-dependencies` join `NATIVE_RUNTIME_PREFIXES` so a change to the modules themselves starts it too. `win32-test-lane-registration.test.mjs` exists to catch precisely this and did not, because its matcher only recognises suite-level gates (`describe.runIf` / `describe.skipIf`) and a `.win32.` filename. These tests gate per `it`. Widening it is not this branch's change to make: about thirty files across the repo carry per-`it` Windows gates and are unregistered, so the ratchet would move far beyond node-pty. Flagged rather than done. Message repairs from the same review: - the non-PE arm of the rebuild-time arch error read "... is not a PE image, so nothing can load it, so node-pty would fall back ...". The shared consequence clause already opens with ", so". - the no-source-build packaging error ended "Package this Windows slice on such a host", which is wrong advice for the case where the host IS such a host and the rebuild simply left nothing — reachable when the artifact is removed before prune runs. It now names both readings and points at the beforeBuild output. - the relay-addon builder blamed `--arch` for a build output that is not a PE at all, the same guess the node-pty gate was taught to stop making. - the patch-drift assertion was a bare `toBe(true)`, so a real drift read as "expected false to be true". It now names the two things that can have drifted and what happens until they agree.
This commit is contained in:
@@ -871,6 +871,11 @@ jobs:
|
||||
pnpm exec vitest run --config config/vitest.config.ts
|
||||
config/scripts/rebuild-native-deps.test.mjs
|
||||
config/scripts/rebuild-native-deps-windows-process-tree.test.mjs
|
||||
config/scripts/rebuild-native-deps-node-pty.test.mjs
|
||||
config/scripts/ensure-native-runtime-job-ownership.test.mjs
|
||||
config/scripts/verify-packaged-node-pty-job-ownership.test.mjs
|
||||
config/scripts/windows-pe-machine.test.mjs
|
||||
config/scripts/script-module-dependencies.test.mjs
|
||||
src/main/windows-registry-addon.test.ts
|
||||
config/scripts/windows-process-tree-gyp-path.test.mjs
|
||||
config/scripts/windows-process-tree-gyp-rebuild.test.mjs
|
||||
|
||||
@@ -122,6 +122,7 @@ docs/**
|
||||
!docs/reference/windows-cmd-shim-resolution.md
|
||||
!docs/reference/windows-daemon-host-relocation.md
|
||||
!docs/reference/windows-edr-posture.md
|
||||
!docs/reference/windows-msys-job-breakaway.md
|
||||
!docs/reference/windows-process-enumeration.md
|
||||
!docs/reference/wsl-runner-verification.md
|
||||
!docs/reference/remote-wire-compatibility.md
|
||||
|
||||
@@ -76,6 +76,7 @@ Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior beh
|
||||
- **Windows setup scripts**: the setup/issue-command runner is a `.cmd` batch file unless the script starts with a `#!` line — never derive that from the user's terminal-shell preference, and never launch a `.cmd` runner with a bare `cmd.exe /c` from a Git Bash pane (MSYS rewrites the `/c`). See [`docs/reference/windows-setup-shell.md`](./docs/reference/windows-setup-shell.md).
|
||||
- **Windows child processes**: start them through `runProcess`/`spawnProcess` in `src/shared/child-process/` — never `child_process` directly. It pins `windowsHide`, refuses `shell: true`, and encodes `.cmd`/`.bat` arguments so neither `CommandLineToArgvW` nor `cmd.exe` mangles them. A ratchet test fails on any new direct import. Recognised npm/pnpm `.cmd` shims are resolved to their real target so the spawn skips `cmd.exe` entirely; see [`docs/reference/windows-cmd-shim-resolution.md`](./docs/reference/windows-cmd-shim-resolution.md) before adding a shim shape or debugging one.
|
||||
- **Windows process enumeration**: read the table through `src/main/windows/windows-process-table.ts`, never by forking `powershell.exe`. See [`docs/reference/windows-process-enumeration.md`](./docs/reference/windows-process-enumeration.md).
|
||||
- **Windows MSYS/Git Bash panes**: their children break away from the per-PTY job unless it is created without `JOB_OBJECT_LIMIT_BREAKAWAY_OK`, and a `conpty.node` built before that fix passes every existing gate. Before changing the per-PTY job or debugging `windows-msys-job.win32.test.ts`, read [`docs/reference/windows-msys-job-breakaway.md`](./docs/reference/windows-msys-job-breakaway.md).
|
||||
- **Windows daemon-host relocation**: the terminal daemon runs from a copy of the app runtime under `%LOCALAPPDATA%`, which is what survives an auto-update. Before touching that copy, its exe name, or the NSIS uninstall macro, read [`docs/reference/windows-daemon-host-relocation.md`](./docs/reference/windows-daemon-host-relocation.md).
|
||||
- **Windows EDR signal**: don't add `-ExecutionPolicy Bypass`, `-EncodedCommand`, `cmd.exe /c` with escaped free text, per-operation interpreter spawning, or runtime `Add-Type` compilation without reading [`docs/reference/windows-edr-posture.md`](./docs/reference/windows-edr-posture.md) first — behavioural EDR scores each of those, and being signed does not clear them.
|
||||
- **WSL commands**: build argv with `buildWslExecArgs` (always `--exec` — under `--`, `wsl.exe` expands `$name` in every argument and silently rewrites the script), and fence anything whose stdout you parse with `buildWslCapturedLoginShellCommand`, because the interactive login shell prints the distro banner to stdout. See [`docs/reference/wsl-command-execution.md`](./docs/reference/wsl-command-execution.md).
|
||||
|
||||
@@ -16,7 +16,7 @@ const { verifyLinuxGlibcFloor } = require('./scripts/verify-linux-glibc-floor.cj
|
||||
const { writeMacBuildCompatibility } = require('./scripts/mac-build-compatibility.cjs')
|
||||
const { verifyPackagedPluginResources } = require('./scripts/verify-packaged-plugin-resources.cjs')
|
||||
const {
|
||||
verifyPackagedNodePtyJobOwnership
|
||||
verifyPackagedWindowsNodePty
|
||||
} = require('./scripts/verify-packaged-node-pty-job-ownership.cjs')
|
||||
const { verifySkillsCliRuntime } = require('./scripts/verify-skills-cli-runtime.cjs')
|
||||
const { verifyStaticAppImagePackage } = require('./scripts/static-appimage-package-contract.cjs')
|
||||
@@ -353,11 +353,7 @@ module.exports = {
|
||||
const hostArchEnum = archEnumByNodeArch[process.arch]
|
||||
const canExecuteTargetArch = context.arch === hostArchEnum || context.arch === 4
|
||||
if (context.electronPlatformName === 'win32') {
|
||||
if (process.platform === 'win32' && canExecuteTargetArch) {
|
||||
verifyPackagedNodePtyJobOwnership(resourcesDir)
|
||||
} else {
|
||||
console.log('[verify-packaged-node-pty] skipped cross-platform or cross-arch package')
|
||||
}
|
||||
verifyPackagedWindowsNodePty(resourcesDir, context.arch, { canExecuteTargetArch })
|
||||
}
|
||||
verifySkillsCliRuntime(join(resourcesDir, 'app.asar.unpacked', 'out'), resourcesDir, {
|
||||
executeCommands: canExecuteTargetArch
|
||||
|
||||
@@ -607,6 +607,7 @@ module.exports = {
|
||||
createPackagedRuntimeNodeModuleResources,
|
||||
findAsarEntry,
|
||||
isPackagedExternalSpecifier,
|
||||
normalizeNodePtyWindowsArch,
|
||||
packageNameFromSpecifier,
|
||||
prunePackagedNodePty,
|
||||
prunePackagedParcelWatcher,
|
||||
|
||||
@@ -19,16 +19,8 @@
|
||||
* node config/scripts/build-windows-process-tree-relay-addon.mjs --arch=arm64
|
||||
*/
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import {
|
||||
closeSync,
|
||||
copyFileSync,
|
||||
existsSync,
|
||||
mkdirSync,
|
||||
openSync,
|
||||
readFileSync,
|
||||
readSync,
|
||||
writeFileSync
|
||||
} from 'node:fs'
|
||||
import { copyFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'
|
||||
import { createRequire } from 'node:module'
|
||||
import { join, resolve } from 'node:path'
|
||||
import { RELAY_WINDOWS_PROCESS_TREE_FILENAME } from '../../src/shared/relay-artifacts.ts'
|
||||
import {
|
||||
@@ -39,12 +31,13 @@ import {
|
||||
WINDOWS_PROCESS_TREE_PACKAGE_DIR as PACKAGE_DIR
|
||||
} from './windows-process-tree-gyp-rebuild.mjs'
|
||||
|
||||
const { PE_MACHINE, describePeMachine, readPeMachine } = createRequire(import.meta.url)(
|
||||
'./windows-pe-machine.cjs'
|
||||
)
|
||||
|
||||
const ROOT = resolve(import.meta.dirname, '..', '..')
|
||||
const SUPPORTED_ARCHES = ['x64', 'arm64']
|
||||
|
||||
/** PE `IMAGE_FILE_HEADER.Machine` values, so a cross-build cannot silently emit host arch. */
|
||||
const PE_MACHINE = { x64: 0x8664, arm64: 0xaa64 }
|
||||
|
||||
function parseArgs(argv) {
|
||||
const arch = argv.find((a) => a.startsWith('--arch='))?.slice('--arch='.length) ?? process.arch
|
||||
const outDir = argv.find((a) => a.startsWith('--out='))?.slice('--out='.length)
|
||||
@@ -363,21 +356,6 @@ function applyWindowsProcessTreeBuildFixes() {
|
||||
}
|
||||
}
|
||||
|
||||
/** Read the PE machine field, so an arm64 request cannot ship an x64 binary. */
|
||||
function readPeMachine(binaryPath) {
|
||||
const fd = openSync(binaryPath, 'r')
|
||||
try {
|
||||
const header = Buffer.alloc(4)
|
||||
readSync(fd, header, 0, 4, 0x3c)
|
||||
const peOffset = header.readUInt32LE(0)
|
||||
const machine = Buffer.alloc(2)
|
||||
readSync(fd, machine, 0, 2, peOffset + 4)
|
||||
return machine.readUInt16LE(0)
|
||||
} finally {
|
||||
closeSync(fd)
|
||||
}
|
||||
}
|
||||
|
||||
function main() {
|
||||
const { arch, outDir } = parseArgs(process.argv.slice(2))
|
||||
if (process.platform !== 'win32') {
|
||||
@@ -410,9 +388,12 @@ function main() {
|
||||
}
|
||||
const machine = readPeMachine(built)
|
||||
if (machine !== PE_MACHINE[arch]) {
|
||||
const cause =
|
||||
machine === null
|
||||
? 'A truncated or quarantined build output looks like this; a relay would get a binary no host can load.'
|
||||
: 'node-gyp ignored --arch; a relay would get a binary its host cannot load.'
|
||||
throw new Error(
|
||||
`Built binary is machine 0x${machine.toString(16)}, expected 0x${PE_MACHINE[arch].toString(16)} for ${arch}. ` +
|
||||
'node-gyp ignored --arch; a relay would get a binary its host cannot load.'
|
||||
`Built binary is ${describePeMachine(machine)}, expected 0x${PE_MACHINE[arch].toString(16)} for ${arch}. ${cause}`
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -1,22 +1,48 @@
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'
|
||||
import { createRequire } from 'node:module'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { dirname, join, resolve } from 'node:path'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { peImage } from './windows-pe-image-fixture.mjs'
|
||||
|
||||
const require = createRequire(import.meta.url)
|
||||
const { assertNodePtyJobOwnership } = require('./node-pty-job-ownership.cjs')
|
||||
const {
|
||||
CYGWIN_BREAKAWAY_MARKER,
|
||||
CYGWIN_BREAKAWAY_MARKER_TEXT,
|
||||
assertNodePtyJobOwnership,
|
||||
assertRebuiltConptyDeniesMsysBreakaway,
|
||||
nodePtyAddonPath
|
||||
} = require('./node-pty-job-ownership.cjs')
|
||||
const NODE_PTY_PATCH = readFileSync(
|
||||
new URL('../patches/node-pty@1.1.0.patch', import.meta.url),
|
||||
'utf8'
|
||||
)
|
||||
|
||||
const PATCHED = {
|
||||
dir: 'build/Release/',
|
||||
module: {
|
||||
listJobProcessIds: () => [],
|
||||
terminateJob: () => true,
|
||||
assignCurrentProcessToJob: () => true
|
||||
}
|
||||
const JOB_EXPORTS = {
|
||||
listJobProcessIds: () => [],
|
||||
terminateJob: () => true,
|
||||
assignCurrentProcessToJob: () => true
|
||||
}
|
||||
|
||||
const fixtureDir = mkdtempSync(join(tmpdir(), 'node-pty-job-ownership-'))
|
||||
|
||||
/** A stand-in addon; only the wide literal the gate reads has to be real. */
|
||||
function writeAddon(name, { cygwinBreakawayDenied }) {
|
||||
const path = join(fixtureDir, name)
|
||||
writeFileSync(
|
||||
path,
|
||||
Buffer.concat([
|
||||
Buffer.from('MZ fake addon '),
|
||||
cygwinBreakawayDenied ? CYGWIN_BREAKAWAY_MARKER : Buffer.alloc(0)
|
||||
])
|
||||
)
|
||||
return path
|
||||
}
|
||||
|
||||
const CURRENT_ADDON = writeAddon('current.node', { cygwinBreakawayDenied: true })
|
||||
const PRE_MSYS_ADDON = writeAddon('pre-msys.node', { cygwinBreakawayDenied: false })
|
||||
|
||||
const PATCHED = { dir: 'build/Release/', module: JOB_EXPORTS }
|
||||
const PREBUILD = {
|
||||
dir: 'prebuilds/win32-x64/',
|
||||
module: {
|
||||
@@ -28,6 +54,13 @@ const PREBUILD = {
|
||||
}
|
||||
}
|
||||
|
||||
const onWindows = (native, addonPath) => ({
|
||||
platform: 'win32',
|
||||
nativeName: 'conpty',
|
||||
native,
|
||||
addonPath
|
||||
})
|
||||
|
||||
describe('assertNodePtyJobOwnership', () => {
|
||||
it('keeps node-addon-api project paths absolute during Windows source builds', () => {
|
||||
expect(NODE_PTY_PATCH).toContain(
|
||||
@@ -40,28 +73,227 @@ describe('assertNodePtyJobOwnership', () => {
|
||||
expect(NODE_PTY_PATCH).toContain("- 'target_name': 'pty'")
|
||||
})
|
||||
|
||||
// The gate's whole case rests on this literal, and nothing else ties the
|
||||
// constant to the C++ that compiles it in. Drift either way has to fail HERE:
|
||||
// otherwise it fails every correctly rebuilt addon, and no rebuild can fix it.
|
||||
it('sniffs for a literal the patch really adds to conpty.cc', () => {
|
||||
const conptyHunk = NODE_PTY_PATCH.split(/^diff --git /m).find((section) =>
|
||||
section.startsWith('a/src/win/conpty.cc ')
|
||||
)
|
||||
expect(conptyHunk, 'the patch no longer touches src/win/conpty.cc').toBeDefined()
|
||||
const addedCode = conptyHunk
|
||||
.split('\n')
|
||||
.filter((line) => line.startsWith('+') && !/^\+\s*(\/\/|\*)/.test(line))
|
||||
expect(
|
||||
addedCode.some((line) => line.includes(`L"${CYGWIN_BREAKAWAY_MARKER_TEXT}"`)),
|
||||
`No added conpty.cc line carries L"${CYGWIN_BREAKAWAY_MARKER_TEXT}". Either the patch ` +
|
||||
'stopped adding it or CYGWIN_BREAKAWAY_MARKER_TEXT drifted; until they agree the gate ' +
|
||||
'rejects every correctly rebuilt addon.'
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
// MSVC compiles L"" to UTF-16LE; reading the addon as anything else finds nothing.
|
||||
it('looks for that literal in the encoding the compiler stores it in', () => {
|
||||
expect(CYGWIN_BREAKAWAY_MARKER.toString('utf16le')).toBe(CYGWIN_BREAKAWAY_MARKER_TEXT)
|
||||
expect(CYGWIN_BREAKAWAY_MARKER.length).toBe(CYGWIN_BREAKAWAY_MARKER_TEXT.length * 2)
|
||||
})
|
||||
|
||||
it('rejects the prebuild that shipped without the job exports', () => {
|
||||
expect(() =>
|
||||
assertNodePtyJobOwnership({ platform: 'win32', nativeName: 'conpty', native: PREBUILD })
|
||||
).toThrow(/listJobProcessIds, terminateJob, assignCurrentProcessToJob/)
|
||||
expect(() => assertNodePtyJobOwnership(onWindows(PREBUILD, CURRENT_ADDON))).toThrow(
|
||||
/listJobProcessIds, terminateJob, assignCurrentProcessToJob/
|
||||
)
|
||||
})
|
||||
|
||||
it('names where the bad native came from, so the fix is obvious', () => {
|
||||
expect(() =>
|
||||
assertNodePtyJobOwnership({ platform: 'win32', nativeName: 'conpty', native: PREBUILD })
|
||||
).toThrow(/prebuilds\/win32-x64/)
|
||||
expect(() => assertNodePtyJobOwnership(onWindows(PREBUILD, CURRENT_ADDON))).toThrow(
|
||||
/prebuilds\/win32-x64/
|
||||
)
|
||||
})
|
||||
|
||||
it('accepts a source build carrying the patch', () => {
|
||||
expect(() =>
|
||||
assertNodePtyJobOwnership({ platform: 'win32', nativeName: 'conpty', native: PATCHED })
|
||||
).not.toThrow()
|
||||
expect(() => assertNodePtyJobOwnership(onWindows(PATCHED, CURRENT_ADDON))).not.toThrow()
|
||||
})
|
||||
|
||||
// The reason this gate reads the binary at all: every export above predates
|
||||
// the Cygwin/MSYS breakaway denial, so a build that leaks every Git Bash
|
||||
// child out of its pane's job satisfies all of them.
|
||||
it('rejects a source build that predates the Cygwin/MSYS breakaway denial', () => {
|
||||
expect(() => assertNodePtyJobOwnership(onWindows(PATCHED, PRE_MSYS_ADDON))).toThrow(
|
||||
/predates the Cygwin\/MSYS job-breakaway denial/
|
||||
)
|
||||
})
|
||||
|
||||
it('tells that build apart by path, and says to rebuild', () => {
|
||||
expect(() => assertNodePtyJobOwnership(onWindows(PATCHED, PRE_MSYS_ADDON))).toThrow(
|
||||
/pre-msys\.node[\s\S]*Rebuild node-pty from source/
|
||||
)
|
||||
})
|
||||
|
||||
it.each([
|
||||
['non-Windows hosts', { platform: 'darwin', nativeName: 'pty' }],
|
||||
['no path at all', undefined],
|
||||
['a path that is not there', join(fixtureDir, 'absent.node')]
|
||||
])('refuses rather than skip when the addon cannot be read: %s', (_case, addonPath) => {
|
||||
expect(() => assertNodePtyJobOwnership(onWindows(PATCHED, addonPath))).toThrow(
|
||||
/Cannot read node-pty's conpty native/
|
||||
)
|
||||
})
|
||||
|
||||
// Passing the conpty name and no readable addon: on win32 every remaining
|
||||
// branch throws, so only the platform gate can keep these quiet. The MSYS
|
||||
// breakaway denial is a Windows concern and must cost other hosts nothing.
|
||||
it.each([
|
||||
['non-Windows hosts', { platform: 'darwin', nativeName: 'conpty' }],
|
||||
['non-Windows hosts building for one', { platform: 'linux', nativeName: 'conpty' }],
|
||||
['the pre-ConPTY winpty backend', { platform: 'win32', nativeName: 'pty' }]
|
||||
])('stays out of the way on %s', (_case, spec) => {
|
||||
expect(() => assertNodePtyJobOwnership({ ...spec, native: PREBUILD })).not.toThrow()
|
||||
})
|
||||
|
||||
it('would have thrown on Windows for the very same input', () => {
|
||||
expect(() =>
|
||||
assertNodePtyJobOwnership({ platform: 'win32', nativeName: 'conpty', native: PREBUILD })
|
||||
).toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
describe('nodePtyAddonPath', () => {
|
||||
// Built from segments rather than a POSIX string: on Windows `resolve` returns
|
||||
// a drive letter and backslashes, so a literal only ever passed off Windows.
|
||||
it('resolves the addon against node-pty lib, which is the only base callers share', () => {
|
||||
expect(
|
||||
nodePtyAddonPath(
|
||||
resolve('/app/node_modules/node-pty/lib/utils.js'),
|
||||
{ dir: '../build/Release/' },
|
||||
'conpty'
|
||||
)
|
||||
).toBe(join(resolve('/app/node_modules/node-pty'), 'build', 'Release', 'conpty.node'))
|
||||
})
|
||||
|
||||
it('handles the bundled layout, where the addon sits beside lib', () => {
|
||||
expect(
|
||||
nodePtyAddonPath(
|
||||
resolve('/app/resources/node-pty/lib/utils.js'),
|
||||
{ dir: './build/Release/' },
|
||||
'conpty'
|
||||
)
|
||||
).toBe(join(resolve('/app/resources/node-pty/lib'), 'build', 'Release', 'conpty.node'))
|
||||
})
|
||||
})
|
||||
|
||||
describe('assertRebuiltConptyDeniesMsysBreakaway', () => {
|
||||
const rebuiltInto = (files) => {
|
||||
const nodePtyDir = join(mkdtempSync(join(fixtureDir, 'rebuild-')), 'node-pty')
|
||||
for (const [relativePath, options] of Object.entries(files)) {
|
||||
const { arch = 'x64', cygwinBreakawayDenied = true } = options
|
||||
const addonPath = join(nodePtyDir, ...relativePath.split('/'))
|
||||
mkdirSync(dirname(addonPath), { recursive: true })
|
||||
writeFileSync(
|
||||
addonPath,
|
||||
Buffer.concat([
|
||||
peImage({ arch }),
|
||||
cygwinBreakawayDenied ? CYGWIN_BREAKAWAY_MARKER : Buffer.alloc(0)
|
||||
])
|
||||
)
|
||||
}
|
||||
return nodePtyDir
|
||||
}
|
||||
|
||||
it('accepts the addon a good same-host rebuild leaves behind', () => {
|
||||
const nodePtyDir = rebuiltInto({ 'build/Release/conpty.node': {} })
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({ nodePtyDir, rebuildArch: 'x64', crossHost: false })
|
||||
).not.toThrow()
|
||||
})
|
||||
|
||||
it('rejects one that predates the denial, wherever the rebuild ran', () => {
|
||||
const nodePtyDir = rebuiltInto({
|
||||
'build/Release/conpty.node': { cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({ nodePtyDir, rebuildArch: 'x64', crossHost: true })
|
||||
).toThrow(/predates the Cygwin\/MSYS job-breakaway denial/)
|
||||
})
|
||||
|
||||
// On the host that will run this install, no addon means loadNativeModule
|
||||
// falls through to the published prebuild -- the binary that leaks every MSYS
|
||||
// pane child. That is a broken build, not an absence to shrug at.
|
||||
it('refuses a same-host rebuild that reported success and produced nothing', () => {
|
||||
const nodePtyDir = rebuiltInto({ 'package.json': {} })
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({ nodePtyDir, rebuildArch: 'x64', crossHost: false })
|
||||
).toThrow(/the rebuild reported success/)
|
||||
})
|
||||
|
||||
it('names both the addon it wanted and the prebuild that would load instead', () => {
|
||||
const nodePtyDir = rebuiltInto({ 'package.json': {} })
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({ nodePtyDir, rebuildArch: 'arm64', crossHost: false })
|
||||
).toThrow(/build[\\/]Release[\s\S]*prebuilds[\\/]win32-arm64/)
|
||||
})
|
||||
|
||||
// A rebuild that ignored --arch leaves a binary the target cannot load, so
|
||||
// node-pty falls back to the prebuild. Saying so here is two steps closer to
|
||||
// the command that fixes it than saying so at packaging time.
|
||||
it('rejects an addon of an architecture this rebuild did not target', () => {
|
||||
const nodePtyDir = rebuiltInto({ 'build/Release/conpty.node': { arch: 'x64' } })
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({ nodePtyDir, rebuildArch: 'arm64', crossHost: true })
|
||||
).toThrow(/machine 0x8664, but this rebuild targets win32-arm64/)
|
||||
})
|
||||
|
||||
// "node-gyp ignored --arch" is a guess when the file is not a PE at all: that
|
||||
// is a truncated or quarantined artifact, and saying otherwise sends the
|
||||
// reader to the wrong command.
|
||||
it('does not blame --arch for a file that is not a PE image', () => {
|
||||
const nodePtyDir = join(mkdtempSync(join(fixtureDir, 'rebuild-')), 'node-pty')
|
||||
mkdirSync(join(nodePtyDir, 'build', 'Release'), { recursive: true })
|
||||
writeFileSync(join(nodePtyDir, 'build', 'Release', 'conpty.node'), Buffer.alloc(0x200))
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({ nodePtyDir, rebuildArch: 'x64', crossHost: false })
|
||||
).toThrow(/is not a PE image[\s\S]*truncated or quarantined/)
|
||||
})
|
||||
|
||||
it('still names the consequence for that file, which is the prebuild', () => {
|
||||
const nodePtyDir = join(mkdtempSync(join(fixtureDir, 'rebuild-')), 'node-pty')
|
||||
mkdirSync(join(nodePtyDir, 'build', 'Release'), { recursive: true })
|
||||
writeFileSync(join(nodePtyDir, 'build', 'Release', 'conpty.node'), Buffer.alloc(0x200))
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({ nodePtyDir, rebuildArch: 'x64', crossHost: false })
|
||||
).toThrow(/fall back to the published prebuild/)
|
||||
})
|
||||
|
||||
it('accepts one a cross-arch rebuild really did emit for the target', () => {
|
||||
const nodePtyDir = rebuiltInto({ 'build/Release/conpty.node': { arch: 'arm64' } })
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({ nodePtyDir, rebuildArch: 'arm64', crossHost: true })
|
||||
).not.toThrow()
|
||||
})
|
||||
|
||||
// PE_MACHINE covers what Orca ships; an arch it does not know is not one this
|
||||
// can judge, and guessing would fail a rebuild that was fine.
|
||||
it('does not judge an architecture it has no machine value for', () => {
|
||||
const nodePtyDir = rebuiltInto({ 'build/Release/conpty.node': { arch: 'x64' } })
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({ nodePtyDir, rebuildArch: 'ia32', crossHost: true })
|
||||
).not.toThrow()
|
||||
})
|
||||
|
||||
it.each([
|
||||
['a cross-host rebuild need not leave a win32 addon here', { crossHost: true }, true],
|
||||
['no node-pty on this disk is not a bad build', { crossHost: false }, false]
|
||||
])('warns instead: %s', (_case, verdict, nodePtyInstalled) => {
|
||||
const nodePtyDir = nodePtyInstalled
|
||||
? rebuiltInto({ 'package.json': {} })
|
||||
: join(fixtureDir, 'no-node-pty-here')
|
||||
const warn = vi.fn()
|
||||
expect(() =>
|
||||
assertRebuiltConptyDeniesMsysBreakaway({
|
||||
nodePtyDir,
|
||||
rebuildArch: 'x64',
|
||||
...verdict,
|
||||
warn
|
||||
})
|
||||
).not.toThrow()
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('could not check the MSYS'))
|
||||
})
|
||||
})
|
||||
|
||||
@@ -13,7 +13,7 @@ import {
|
||||
} from './windows-process-tree-gyp-rebuild.mjs'
|
||||
|
||||
const require = createRequire(import.meta.url)
|
||||
const { assertNodePtyJobOwnership } = require('./node-pty-job-ownership.cjs')
|
||||
const { assertNodePtyJobOwnership, nodePtyAddonPath } = require('./node-pty-job-ownership.cjs')
|
||||
const { assertWindowsProcessTreeCreationTime } = require('./windows-process-tree-creation-time.cjs')
|
||||
const scriptPath = import.meta.filename
|
||||
const projectDir = resolve(import.meta.dirname, '../..')
|
||||
@@ -298,7 +298,11 @@ function loadNodePtyNativeModule() {
|
||||
// terminal is created, so require('node-pty') alone can miss ABI mismatches.
|
||||
const native = loadNativeModule(nativeName)
|
||||
assertNodePtyWindowsConptyRuntime(native?.dir)
|
||||
assertNodePtyJobOwnership({ nativeName, native })
|
||||
assertNodePtyJobOwnership({
|
||||
nativeName,
|
||||
native,
|
||||
addonPath: nodePtyAddonPath(require.resolve('node-pty/lib/utils'), native, nativeName)
|
||||
})
|
||||
if (requiresPatchedNodePtySourceBuild() && !isNodePtyReleaseBuildDir(native?.dir)) {
|
||||
throw new Error(
|
||||
`node-pty resolved to ${native.dir}; expected build/Release so Orca's node-pty patch is active`
|
||||
|
||||
@@ -1,25 +1,188 @@
|
||||
'use strict'
|
||||
|
||||
const { existsSync, readFileSync } = require('node:fs')
|
||||
const { dirname, join, resolve } = require('node:path')
|
||||
const { PE_MACHINE, describePeMachine, readPeMachine } = require('./windows-pe-machine.cjs')
|
||||
|
||||
const NODE_PTY_JOB_EXPORTS = ['listJobProcessIds', 'terminateJob', 'assignCurrentProcessToJob']
|
||||
|
||||
function assertNodePtyJobOwnership({ nativeName, native, platform = process.platform }) {
|
||||
/**
|
||||
* The wide literal `usesCygwinRuntime` probes for in conpty.cc, as it sits in
|
||||
* the compiled addon.
|
||||
*
|
||||
* Why sniff the binary rather than trust the exports: all three job exports
|
||||
* predate the Cygwin/MSYS breakaway denial, so symbol presence cannot tell a
|
||||
* current build from one whose per-PTY job still carries
|
||||
* JOB_OBJECT_LIMIT_BREAKAWAY_OK. Measured on Windows 11: such a build passes
|
||||
* every export check, reports isPtyJobOwnershipAvailable() true, and passes
|
||||
* windows-pty-job.win32.test.ts 6/6, while every child of a Git Bash pane is
|
||||
* created outside the pane's job and survives terminatePtyJob. See
|
||||
* docs/reference/windows-msys-job-breakaway.md.
|
||||
*
|
||||
* Same shape as stagedRelayAddonIsUnpatched() in
|
||||
* src/main/windows/windows-process-table.ts, which already tells a patched
|
||||
* addon from a published one by a binary import name.
|
||||
*/
|
||||
const CYGWIN_BREAKAWAY_MARKER_TEXT = 'msys-2.0.dll'
|
||||
const CYGWIN_BREAKAWAY_MARKER = Buffer.from(CYGWIN_BREAKAWAY_MARKER_TEXT, 'utf16le')
|
||||
|
||||
/** True when the addon carries the denial. Read errors propagate: callers that cannot read it must not pass. */
|
||||
function conptyDeniesCygwinBreakaway(addonPath) {
|
||||
return readFileSync(addonPath).includes(CYGWIN_BREAKAWAY_MARKER)
|
||||
}
|
||||
|
||||
/**
|
||||
* Why here and not only at packaging: a rebuild that did not honour `--arch`
|
||||
* leaves a binary the target cannot load, the app falls back to the published
|
||||
* prebuild, and the packaged gate then reports it two steps from the command
|
||||
* that could fix it. `PE_MACHINE` covers the Windows arches Orca ships; anything
|
||||
* else this cannot judge, so it does not pretend to.
|
||||
*/
|
||||
function assertRebuiltConptyMatchesArch(addonPath, rebuildArch) {
|
||||
const expected = PE_MACHINE[rebuildArch]
|
||||
if (expected === undefined) {
|
||||
return
|
||||
}
|
||||
const machine = readPeMachine(addonPath)
|
||||
if (machine === expected) {
|
||||
return
|
||||
}
|
||||
const consequence = [
|
||||
', so node-pty would fall back to the published prebuild, which predates the',
|
||||
'Cygwin/MSYS job-breakaway denial and leaks every MSYS pane child out of its job.'
|
||||
].join(' ')
|
||||
throw new Error(
|
||||
machine === null
|
||||
? `${addonPath} is not a PE image${consequence} Check the ` +
|
||||
`node-pty build output above; a truncated or quarantined artifact looks like this.`
|
||||
: `${addonPath} is ${describePeMachine(machine)}, but this rebuild targets ` +
|
||||
`win32-${rebuildArch} (0x${expected.toString(16)}): node-gyp did not honour ` +
|
||||
`--arch${consequence}`
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The verdict on the addon a Windows rebuild just claimed to produce.
|
||||
*
|
||||
* Takes the host as arguments rather than reading `process`, because the branch
|
||||
* that matters -- a rebuild for the very host running it -- is otherwise
|
||||
* reachable only from Windows, and a gate nobody can run is a gate nobody
|
||||
* checks.
|
||||
*
|
||||
* Absent is fatal on that host: `loadNativeModule` falls through to
|
||||
* prebuilds/win32-<arch>, and the published prebuild predates the denial, so
|
||||
* the app would load it with nothing said. A cross-host rebuild need not leave
|
||||
* a win32 addon on this disk, and node-pty may not be installed at all --
|
||||
* neither is evidence of a bad build.
|
||||
*/
|
||||
function assertRebuiltConptyDeniesMsysBreakaway({
|
||||
nodePtyDir,
|
||||
rebuildArch,
|
||||
crossHost,
|
||||
warn = console.warn
|
||||
}) {
|
||||
const addonPath = join(nodePtyDir, 'build', 'Release', 'conpty.node')
|
||||
if (existsSync(addonPath)) {
|
||||
assertRebuiltConptyMatchesArch(addonPath, rebuildArch)
|
||||
assertCygwinBreakawayDenied(addonPath, { dir: addonPath })
|
||||
return
|
||||
}
|
||||
if (crossHost || !existsSync(nodePtyDir)) {
|
||||
warn(`[rebuild] no addon at ${addonPath}; could not check the MSYS job-breakaway denial.`)
|
||||
return
|
||||
}
|
||||
const prebuildPath = join(nodePtyDir, 'prebuilds', `win32-${rebuildArch}`, 'conpty.node')
|
||||
throw new Error(
|
||||
`the rebuild reported success but ${addonPath} is not there, so node-pty would fall through ` +
|
||||
`to ${prebuildPath}. That published prebuild predates the Cygwin/MSYS ` +
|
||||
'job-breakaway denial: every Git Bash pane child would be created outside its job and ' +
|
||||
'survive terminatePtyJob. Check the node-pty build output above; a same-host source ' +
|
||||
'build must leave conpty.node in build/Release.'
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Absolute path of the addon `loadNativeModule` just resolved.
|
||||
*
|
||||
* `native.dir` is relative to node-pty's own `lib/`, which is the only base
|
||||
* every caller shares -- the project install, a staged rebuild and the packaged
|
||||
* resources tree all reach the addon through a different root.
|
||||
*/
|
||||
function nodePtyAddonPath(nodePtyUtilsPath, native, nativeName) {
|
||||
return resolve(dirname(nodePtyUtilsPath), native.dir, `${nativeName}.node`)
|
||||
}
|
||||
|
||||
function assertNodePtyJobOwnership({ nativeName, native, addonPath, platform = process.platform }) {
|
||||
if (platform !== 'win32' || nativeName !== 'conpty') {
|
||||
return
|
||||
}
|
||||
const exported = native?.module ?? native
|
||||
const missing = NODE_PTY_JOB_EXPORTS.filter((name) => typeof exported?.[name] !== 'function')
|
||||
if (missing.length === 0) {
|
||||
if (missing.length > 0) {
|
||||
throw new Error(
|
||||
[
|
||||
`node-pty's conpty native is missing ${missing.join(', ')}.`,
|
||||
`Resolved from: ${native?.dir ?? 'unknown'}`,
|
||||
'That build cannot own a PTY tree, so terminatePtyJob degrades to "unavailable"',
|
||||
'and pane teardown falls back to guessing by PID ancestry.',
|
||||
'Rebuild node-pty from source so config/patches/node-pty@1.1.0.patch applies.'
|
||||
].join(' ')
|
||||
)
|
||||
}
|
||||
assertCygwinBreakawayDenied(addonPath, native)
|
||||
}
|
||||
|
||||
/**
|
||||
* Why this refuses instead of skipping when the addon cannot be read: an
|
||||
* unreadable binary is exactly the state that used to pass. `loadNativeModule`
|
||||
* has already required this file, so "cannot read it" means the caller did not
|
||||
* say which file it loaded, and a gate that cannot see its subject is not a
|
||||
* gate.
|
||||
*/
|
||||
function assertCygwinBreakawayDenied(addonPath, native) {
|
||||
let binary
|
||||
try {
|
||||
binary = readFileSync(addonPath)
|
||||
} catch (error) {
|
||||
throw new Error(
|
||||
[
|
||||
`Cannot read node-pty's conpty native at ${addonPath ?? '<no path given>'}`,
|
||||
`(resolved from ${native?.dir ?? 'unknown'}): ${error.message}.`,
|
||||
'Without the binary this cannot tell a current build from one that leaks',
|
||||
'every MSYS pane child out of its job, so it refuses rather than assume.'
|
||||
].join(' ')
|
||||
)
|
||||
}
|
||||
if (binary.includes(CYGWIN_BREAKAWAY_MARKER)) {
|
||||
return
|
||||
}
|
||||
throw new Error(
|
||||
throw staleConptySourceBuildError(addonPath)
|
||||
}
|
||||
|
||||
/** The verdict on a source build that is simply out of date: rebuild it here. */
|
||||
function staleConptySourceBuildError(addonPath) {
|
||||
return new Error(
|
||||
[
|
||||
`node-pty's conpty native is missing ${missing.join(', ')}.`,
|
||||
`Resolved from: ${native?.dir ?? 'unknown'}`,
|
||||
'That build cannot own a PTY tree, so terminatePtyJob degrades to "unavailable"',
|
||||
'and pane teardown falls back to guessing by PID ancestry.',
|
||||
'Rebuild node-pty from source so config/patches/node-pty@1.1.0.patch applies.'
|
||||
`node-pty's conpty native at ${addonPath} predates the Cygwin/MSYS job-breakaway denial.`,
|
||||
'It exports the job functions, so it looks patched, but its per-PTY job still carries',
|
||||
'JOB_OBJECT_LIMIT_BREAKAWAY_OK and every Git Bash child is created outside the job:',
|
||||
'terminatePtyJob reports "terminated" and leaves the tree running.',
|
||||
'Rebuild node-pty from source so the current config/patches/node-pty@1.1.0.patch applies',
|
||||
'(a worktree sharing node_modules with its main checkout shares that stale addon).',
|
||||
`If that patch no longer adds L"${CYGWIN_BREAKAWAY_MARKER_TEXT}" to conpty.cc then this marker is`,
|
||||
'stale, not the addon, and no rebuild can satisfy it.',
|
||||
'See docs/reference/windows-msys-job-breakaway.md.'
|
||||
].join(' ')
|
||||
)
|
||||
}
|
||||
|
||||
module.exports = { assertNodePtyJobOwnership }
|
||||
module.exports = {
|
||||
CYGWIN_BREAKAWAY_MARKER,
|
||||
CYGWIN_BREAKAWAY_MARKER_TEXT,
|
||||
assertNodePtyJobOwnership,
|
||||
assertCygwinBreakawayDenied,
|
||||
assertRebuiltConptyDeniesMsysBreakaway,
|
||||
conptyDeniesCygwinBreakaway,
|
||||
nodePtyAddonPath,
|
||||
staleConptySourceBuildError
|
||||
}
|
||||
|
||||
@@ -144,6 +144,9 @@ const NATIVE_RUNTIME_PREFIXES = [
|
||||
'config/scripts/ensure-native-runtime',
|
||||
'config/scripts/rebuild-native-deps',
|
||||
'config/scripts/node-pty-job-ownership',
|
||||
'config/scripts/windows-pe-machine',
|
||||
'config/scripts/windows-pe-image-fixture',
|
||||
'config/scripts/script-module-dependencies',
|
||||
'config/scripts/windows-process-tree-creation-time',
|
||||
'config/scripts/windows-process-tree-gyp-rebuild',
|
||||
'config/scripts/electron-builder-native-rebuild',
|
||||
@@ -220,6 +223,11 @@ const WINDOWS_PACKAGE_TESTS = [
|
||||
...LINUX_PACKAGE_TESTS,
|
||||
'config/scripts/rebuild-native-deps.test.mjs',
|
||||
'config/scripts/rebuild-native-deps-windows-process-tree.test.mjs',
|
||||
'config/scripts/rebuild-native-deps-node-pty.test.mjs',
|
||||
'config/scripts/ensure-native-runtime-job-ownership.test.mjs',
|
||||
'config/scripts/verify-packaged-node-pty-job-ownership.test.mjs',
|
||||
'config/scripts/windows-pe-machine.test.mjs',
|
||||
'config/scripts/script-module-dependencies.test.mjs',
|
||||
'src/main/windows-registry-addon.test.ts',
|
||||
'src/main/providers/windows-conpty-wide-char-duplication.node-pty.test.ts',
|
||||
'src/main/providers/pty-repaint-wide-char-buffer.node-pty.test.ts',
|
||||
|
||||
@@ -260,6 +260,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => {
|
||||
writeFakeLoadableNodePty(projectDir, { ownsPtyJob: false })
|
||||
writeFakeWindowsRegistry(projectDir)
|
||||
writeFakeWindowsProcessTree(projectDir)
|
||||
writeFakeNodePtyConptyPayload(projectDir, process.arch)
|
||||
|
||||
const result = runRebuildScript(projectDir, {
|
||||
ORCA_REBUILD_TEST_LOG: rebuildLogPath,
|
||||
@@ -399,4 +400,106 @@ 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-host rebuild does not necessarily leave a win32 addon on this disk,
|
||||
// and neither does a tree with no node-pty in it. That must warn, not fail an
|
||||
// install that was working.
|
||||
it('warns rather than fails when no addon is expected on this disk', () => {
|
||||
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)
|
||||
}
|
||||
})
|
||||
|
||||
// The other half: on the host that will run this install, a missing addon is
|
||||
// not an absence to shrug at. loadNativeModule falls through to the published
|
||||
// prebuild, which is the binary that leaks every MSYS pane child.
|
||||
// Runs only on Windows -- nothing else can make a win32 rebuild same-host.
|
||||
it.skipIf(process.platform !== 'win32')(
|
||||
'fails a same-host Windows rebuild that left no addon, naming the prebuild that would load',
|
||||
() => {
|
||||
const projectDir = mkTempProject()
|
||||
|
||||
try {
|
||||
writeFakeUsableElectronPackage(projectDir, { platform: 'win32' })
|
||||
writeFakeElectronRebuild(projectDir)
|
||||
writeFakeWindowsProcessTreeWithNodeAddonApi(projectDir)
|
||||
writeFakeLoadableNodePty(projectDir, { nativeDir: `prebuilds/win32-${process.arch}` })
|
||||
|
||||
const result = runRebuildScript(
|
||||
projectDir,
|
||||
{ npm_config_platform: 'win32', npm_config_arch: process.arch },
|
||||
['--platform=win32', `--arch=${process.arch}`, '--force']
|
||||
)
|
||||
|
||||
expect(result.status).not.toBe(0)
|
||||
expect(result.stderr).toContain(join('build', 'Release', 'conpty.node'))
|
||||
expect(result.stderr).toContain(join('prebuilds', `win32-${process.arch}`, 'conpty.node'))
|
||||
} finally {
|
||||
removeTreeSync(projectDir)
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
@@ -7,10 +7,25 @@ import {
|
||||
readFileSync,
|
||||
writeFileSync
|
||||
} from 'node:fs'
|
||||
import { createRequire } from 'node:module'
|
||||
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'
|
||||
import { peImage } from './windows-pe-image-fixture.mjs'
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*
|
||||
* Taken from the gate itself: a re-typed copy agrees with a stale gate by
|
||||
* construction, which is the one thing these fixtures must not do.
|
||||
*/
|
||||
const { CYGWIN_BREAKAWAY_MARKER } = createRequire(import.meta.url)('./node-pty-job-ownership.cjs')
|
||||
const { CREATION_TIME_FLAG } = createRequire(import.meta.url)(
|
||||
'./windows-process-tree-creation-time.cjs'
|
||||
)
|
||||
|
||||
const sourceScriptPath = fileURLToPath(new URL('./rebuild-native-deps.mjs', import.meta.url))
|
||||
const sourceInstallScriptPath = fileURLToPath(
|
||||
@@ -22,6 +37,11 @@ const sourceNodePtyJobOwnershipPath = fileURLToPath(
|
||||
const sourceWindowsProcessTreeGypRebuildPath = fileURLToPath(
|
||||
new URL('./windows-process-tree-gyp-rebuild.mjs', import.meta.url)
|
||||
)
|
||||
// Reached through projectRequire, so the module walker cannot see it: that
|
||||
// specifier resolves against the project root, not against the script.
|
||||
const sourceWindowsProcessTreeCreationTimePath = fileURLToPath(
|
||||
new URL('./windows-process-tree-creation-time.cjs', import.meta.url)
|
||||
)
|
||||
const sourceWindowsProcessTreePatchPath = fileURLToPath(
|
||||
new URL('../patches/@vscode__windows-process-tree@0.8.0.patch', import.meta.url)
|
||||
)
|
||||
@@ -90,9 +110,10 @@ export function mkTempProject() {
|
||||
mkdirSync(join(projectDir, 'config', 'scripts'), { recursive: true })
|
||||
copyFileSync(sourceScriptPath, join(projectDir, 'config', 'scripts', 'rebuild-native-deps.mjs'))
|
||||
copyScriptWithLocalModules(sourceInstallScriptPath, join(projectDir, 'config', 'scripts'))
|
||||
copyScriptWithLocalModules(sourceNodePtyJobOwnershipPath, join(projectDir, 'config', 'scripts'))
|
||||
copyFileSync(
|
||||
sourceNodePtyJobOwnershipPath,
|
||||
join(projectDir, 'config', 'scripts', 'node-pty-job-ownership.cjs')
|
||||
sourceWindowsProcessTreeCreationTimePath,
|
||||
join(projectDir, 'config', 'scripts', 'windows-process-tree-creation-time.cjs')
|
||||
)
|
||||
copyFileSync(
|
||||
sourceWindowsProcessTreeGypRebuildPath,
|
||||
@@ -310,10 +331,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([
|
||||
peImage({ arch }),
|
||||
cygwinBreakawayDenied ? CYGWIN_BREAKAWAY_MARKER : Buffer.alloc(0)
|
||||
])
|
||||
)
|
||||
const sourceDir = join(
|
||||
projectDir,
|
||||
'node_modules',
|
||||
@@ -328,12 +359,30 @@ export function writeFakeNodePtyConptyPayload(projectDir, arch) {
|
||||
writeFileSync(join(sourceDir, 'OpenConsole.exe'), `OpenConsole.exe ${arch}`)
|
||||
}
|
||||
|
||||
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([
|
||||
peImage({ arch: process.arch === 'arm64' ? 'arm64' : 'x64' }),
|
||||
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'),
|
||||
@@ -366,10 +415,18 @@ export function writeFakeWindowsRegistry(projectDir) {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A healthy one: the addon reports CreationTime, which is what a build of the
|
||||
* patched source does and what the probe has required since the creation-time
|
||||
* gate landed. Exporting nothing means "the tarball prebuilt" to that gate.
|
||||
*/
|
||||
export function writeFakeWindowsProcessTree(projectDir) {
|
||||
const processTreeDir = join(projectDir, 'node_modules', '@vscode', 'windows-process-tree')
|
||||
mkdirSync(processTreeDir, { recursive: true })
|
||||
writeFileSync(join(processTreeDir, 'index.js'), 'module.exports = {}\n')
|
||||
writeFileSync(
|
||||
join(processTreeDir, 'index.js'),
|
||||
`module.exports = { supportedProcessDataFlags: ${CREATION_TIME_FLAG} }\n`
|
||||
)
|
||||
}
|
||||
|
||||
export function writeFakeWindowsProcessTreeWithNodeAddonApi(
|
||||
@@ -434,11 +491,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'), '')
|
||||
|
||||
@@ -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 {
|
||||
@@ -79,11 +82,10 @@ const NATIVE_MODULES = [
|
||||
...(rebuildPlatform === 'win32' ? ['@orca/windows-registry', '@vscode/windows-process-tree'] : [])
|
||||
]
|
||||
const onlyModules = NATIVE_MODULES.filter((m) => !ignoreModules.includes(m))
|
||||
/** Whether this rebuild targets something other than the machine running it. */
|
||||
const isCrossHostRebuild = rebuildPlatform !== osPlatform() || rebuildArch !== process.arch
|
||||
const forceRebuild =
|
||||
process.env.ORCA_FORCE_NATIVE_REBUILD === '1' ||
|
||||
cliOptions.force ||
|
||||
rebuildPlatform !== osPlatform() ||
|
||||
rebuildArch !== process.arch
|
||||
process.env.ORCA_FORCE_NATIVE_REBUILD === '1' || cliOptions.force || isCrossHostRebuild
|
||||
let modulesToRebuild = onlyModules
|
||||
|
||||
ensureElectronPackageInstalled()
|
||||
@@ -175,6 +177,7 @@ try {
|
||||
})
|
||||
restoreNodePtyWindowsConptyRuntime()
|
||||
assertWindowsProcessTreeAddonIsPatched()
|
||||
assertNodePtyConptyDeniesMsysBreakaway()
|
||||
} catch (/** @type {any} */ err) {
|
||||
console.error('[rebuild] Native module rebuild failed:', err?.message ?? err)
|
||||
if (isWindowsNativeLockError(err)) {
|
||||
@@ -228,6 +231,32 @@ 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 is fatal on the host that will run this install: loadNativeModule
|
||||
* falls through to prebuilds/win32-<arch>, and the published prebuild predates
|
||||
* the denial, so the app would load it with nothing said. A cross-host rebuild
|
||||
* does not necessarily leave a win32 addon on this disk, and that must not fail
|
||||
* an install that was working.
|
||||
*/
|
||||
function assertNodePtyConptyDeniesMsysBreakaway() {
|
||||
if (rebuildPlatform !== 'win32' || !modulesToRebuild.includes('node-pty')) {
|
||||
return
|
||||
}
|
||||
const { assertRebuiltConptyDeniesMsysBreakaway } = requireLocal('./node-pty-job-ownership.cjs')
|
||||
assertRebuiltConptyDeniesMsysBreakaway({
|
||||
nodePtyDir: resolve(projectDir, 'node_modules', 'node-pty'),
|
||||
rebuildArch,
|
||||
crossHost: isCrossHostRebuild
|
||||
})
|
||||
}
|
||||
|
||||
function restoreNodePtyWindowsConptyRuntime() {
|
||||
if (rebuildPlatform !== 'win32' || !onlyModules.includes('node-pty')) {
|
||||
return
|
||||
@@ -548,14 +577,22 @@ function loadNativeModule(moduleName) {
|
||||
}
|
||||
if (moduleName === 'node-pty') {
|
||||
projectRequire('node-pty')
|
||||
const { assertNodePtyJobOwnership } = projectRequire(
|
||||
const { assertNodePtyJobOwnership, nodePtyAddonPath } = projectRequire(
|
||||
'./config/scripts/node-pty-job-ownership.cjs'
|
||||
)
|
||||
const { loadNativeModule } = projectRequire('node-pty/lib/utils')
|
||||
const nativeName = getNodePtyNativeModuleName()
|
||||
const native = loadNativeModule(nativeName)
|
||||
assertNodePtyWindowsConptyRuntime(native.dir)
|
||||
assertNodePtyJobOwnership({ nativeName, native })
|
||||
assertNodePtyJobOwnership({
|
||||
nativeName,
|
||||
native,
|
||||
addonPath: nodePtyAddonPath(
|
||||
projectRequire.resolve('node-pty/lib/utils'),
|
||||
native,
|
||||
nativeName
|
||||
)
|
||||
})
|
||||
if (requirePatchedNodePtySourceBuild && !isNodePtyReleaseBuildDir(native.dir)) {
|
||||
throw new Error(
|
||||
'node-pty resolved to ' +
|
||||
|
||||
@@ -19,7 +19,17 @@ function collectScriptModules(scriptPath, seen = new Set()) {
|
||||
return seen
|
||||
}
|
||||
seen.add(scriptPath)
|
||||
for (const [, specifier] of readFileSync(scriptPath, 'utf8').matchAll(/from '(\.\/[^']+)'/g)) {
|
||||
// `from`, bare and dynamic `import`, and plain `require` -- the Windows gates
|
||||
// are .cjs, and a module reached only by require or by a side-effect import is
|
||||
// the one nobody notices is missing until a subprocess fails with a
|
||||
// resolution error instead. Deliberately not `projectRequire`/`requireLocal`
|
||||
// wrappers: those specifiers are resolved against the project root at runtime,
|
||||
// not against this file, so following them would stage the wrong path.
|
||||
const source = readFileSync(scriptPath, 'utf8')
|
||||
const specifiers = source.matchAll(
|
||||
/(?:\bfrom|\brequire\s*\(|\bimport\s*\(|\bimport)\s*'(\.\/[^']+)'/g
|
||||
)
|
||||
for (const [, specifier] of specifiers) {
|
||||
collectScriptModules(join(dirname(scriptPath), specifier), seen)
|
||||
}
|
||||
return seen
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
import { existsSync, mkdtempSync, readdirSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { copyScriptWithLocalModules } from './script-module-dependencies.mjs'
|
||||
|
||||
const fixtureDir = mkdtempSync(join(tmpdir(), 'script-module-dependencies-'))
|
||||
|
||||
function sourceTree(files) {
|
||||
const sourceDir = mkdtempSync(join(fixtureDir, 'source-'))
|
||||
for (const [name, contents] of Object.entries(files)) {
|
||||
writeFileSync(join(sourceDir, name), contents)
|
||||
}
|
||||
return sourceDir
|
||||
}
|
||||
|
||||
function copiedNames(files, entryName) {
|
||||
const sourceDir = sourceTree(files)
|
||||
const destinationDir = join(mkdtempSync(join(fixtureDir, 'dest-')), 'scripts')
|
||||
copyScriptWithLocalModules(join(sourceDir, entryName), destinationDir)
|
||||
return readdirSync(destinationDir).sort()
|
||||
}
|
||||
|
||||
describe('copyScriptWithLocalModules', () => {
|
||||
it('takes the entry script itself', () => {
|
||||
expect(copiedNames({ 'entry.mjs': 'export const a = 1\n' }, 'entry.mjs')).toEqual(['entry.mjs'])
|
||||
})
|
||||
|
||||
it('follows a co-located import', () => {
|
||||
expect(
|
||||
copiedNames(
|
||||
{ 'entry.mjs': "import { a } from './dep.mjs'\n", 'dep.mjs': 'export const a = 1\n' },
|
||||
'entry.mjs'
|
||||
)
|
||||
).toEqual(['dep.mjs', 'entry.mjs'])
|
||||
})
|
||||
|
||||
// The Windows addon gates are .cjs and reach each other by require. A module
|
||||
// pulled in only that way used to be left behind, and the subprocess then
|
||||
// failed with a resolution error that looks nothing like the defect it hides.
|
||||
it('follows a co-located require, not only an import', () => {
|
||||
expect(
|
||||
copiedNames(
|
||||
{
|
||||
'entry.cjs': "const { a } = require('./dep.cjs')\nmodule.exports = { a }\n",
|
||||
'dep.cjs': 'module.exports = { a: 1 }\n'
|
||||
},
|
||||
'entry.cjs'
|
||||
)
|
||||
).toEqual(['dep.cjs', 'entry.cjs'])
|
||||
})
|
||||
|
||||
it('follows a require reached only through an imported module', () => {
|
||||
const names = copiedNames(
|
||||
{
|
||||
'entry.mjs': "import './middle.cjs'\n",
|
||||
'middle.cjs': "require('./leaf.cjs')\n",
|
||||
'leaf.cjs': 'module.exports = {}\n'
|
||||
},
|
||||
'entry.mjs'
|
||||
)
|
||||
expect(names).toContain('leaf.cjs')
|
||||
})
|
||||
|
||||
it('leaves package and builtin specifiers alone', () => {
|
||||
const sourceDir = sourceTree({
|
||||
'entry.mjs': "import { join } from 'node:path'\nimport x from 'some-package'\n"
|
||||
})
|
||||
const destinationDir = join(mkdtempSync(join(fixtureDir, 'dest-')), 'scripts')
|
||||
copyScriptWithLocalModules(join(sourceDir, 'entry.mjs'), destinationDir)
|
||||
expect(readdirSync(destinationDir)).toEqual(['entry.mjs'])
|
||||
})
|
||||
|
||||
it('terminates on a cycle rather than recursing forever', () => {
|
||||
expect(
|
||||
copiedNames({ 'a.mjs': "import './b.mjs'\n", 'b.mjs': "import './a.mjs'\n" }, 'a.mjs')
|
||||
).toEqual(['a.mjs', 'b.mjs'])
|
||||
})
|
||||
|
||||
it('creates the destination directory it was handed', () => {
|
||||
const sourceDir = sourceTree({ 'entry.mjs': 'export const a = 1\n' })
|
||||
const destinationDir = join(mkdtempSync(join(fixtureDir, 'dest-')), 'nested', 'scripts')
|
||||
copyScriptWithLocalModules(join(sourceDir, 'entry.mjs'), destinationDir)
|
||||
expect(existsSync(join(destinationDir, 'entry.mjs'))).toBe(true)
|
||||
})
|
||||
|
||||
// The real tree this stages: the packaged-addon gate reaches its PE reader by
|
||||
// require, so a walker that missed it would break every rebuild fixture.
|
||||
it('stages the node-pty job-ownership gate with everything it requires', () => {
|
||||
const destinationDir = join(mkdtempSync(join(fixtureDir, 'dest-')), 'scripts')
|
||||
copyScriptWithLocalModules(
|
||||
fileURLToPath(new URL('./node-pty-job-ownership.cjs', import.meta.url)),
|
||||
destinationDir
|
||||
)
|
||||
expect(readdirSync(destinationDir).sort()).toEqual([
|
||||
'node-pty-job-ownership.cjs',
|
||||
'windows-pe-machine.cjs'
|
||||
])
|
||||
})
|
||||
})
|
||||
@@ -1,11 +1,52 @@
|
||||
const { existsSync } = require('node:fs')
|
||||
const { createRequire } = require('node:module')
|
||||
const { join } = require('node:path')
|
||||
const { assertNodePtyJobOwnership } = require('./node-pty-job-ownership.cjs')
|
||||
const {
|
||||
assertNodePtyJobOwnership,
|
||||
conptyDeniesCygwinBreakaway,
|
||||
nodePtyAddonPath,
|
||||
staleConptySourceBuildError
|
||||
} = require('./node-pty-job-ownership.cjs')
|
||||
const { normalizeNodePtyWindowsArch } = require('../packaged-runtime-node-modules.cjs')
|
||||
const { PE_MACHINE, describePeMachine, readPeMachine } = require('./windows-pe-machine.cjs')
|
||||
|
||||
/**
|
||||
* Every conpty.node the packaged tree can hand `loadNativeModule`, in its order.
|
||||
*
|
||||
* Why the order matters: the loader swallows each require failure and falls
|
||||
* through, so a wrong-arch or otherwise unloadable build hands the pane to the
|
||||
* next candidate. First loadable wins, and the published prebuild is always the
|
||||
* last one standing.
|
||||
*/
|
||||
function packagedConptyCandidates(resourcesDir, targetArch) {
|
||||
const nodePtyDir = join(resourcesDir, 'node_modules', 'node-pty')
|
||||
const layouts = [
|
||||
{ segments: ['build', 'Release'], prebuilt: false },
|
||||
{ segments: ['build', 'Debug'], prebuilt: false },
|
||||
{ segments: ['prebuilds', `win32-${targetArch}`], prebuilt: true }
|
||||
]
|
||||
// Each layout is tried relative to node-pty's root, then to lib/, before the
|
||||
// next layout -- the unbundled then bundled pair node-pty's loader walks.
|
||||
return layouts.flatMap(({ segments, prebuilt }) =>
|
||||
[nodePtyDir, join(nodePtyDir, 'lib')].map((root) => ({
|
||||
path: join(root, ...segments, 'conpty.node'),
|
||||
prebuilt
|
||||
}))
|
||||
)
|
||||
}
|
||||
|
||||
function describeCandidates(candidates) {
|
||||
return candidates
|
||||
.map((candidate) => `${candidate.path} (${describePeMachine(candidate.machine)})`)
|
||||
.join(', ')
|
||||
}
|
||||
|
||||
function loadPackagedConpty(resourcesDir) {
|
||||
const packagedRequire = createRequire(join(resourcesDir, 'package.json'))
|
||||
const { loadNativeModule } = packagedRequire('./node_modules/node-pty/lib/utils')
|
||||
return loadNativeModule('conpty')
|
||||
const utilsPath = packagedRequire.resolve('./node_modules/node-pty/lib/utils')
|
||||
const { loadNativeModule } = packagedRequire(utilsPath)
|
||||
const native = loadNativeModule('conpty')
|
||||
return { native, addonPath: nodePtyAddonPath(utilsPath, native, 'conpty') }
|
||||
}
|
||||
|
||||
function verifyPackagedNodePtyJobOwnership(resourcesDir, options = {}) {
|
||||
@@ -14,12 +55,145 @@ function verifyPackagedNodePtyJobOwnership(resourcesDir, options = {}) {
|
||||
return
|
||||
}
|
||||
|
||||
const native = (options.loadNative ?? loadPackagedConpty)(resourcesDir)
|
||||
assertNodePtyJobOwnership({ platform, nativeName: 'conpty', native })
|
||||
const { native, addonPath } = (options.loadNative ?? loadPackagedConpty)(resourcesDir)
|
||||
assertNodePtyJobOwnership({ platform, nativeName: 'conpty', native, addonPath })
|
||||
if (!native.dir.replace(/\\/g, '/').includes('build/Release/')) {
|
||||
throw new Error(`Packaged node-pty resolved to ${native.dir}; expected patched build/Release`)
|
||||
}
|
||||
console.log('[verify-packaged-node-pty] OK — packaged ConPTY owns process trees')
|
||||
}
|
||||
|
||||
module.exports = { verifyPackagedNodePtyJobOwnership }
|
||||
/**
|
||||
* The half of the packaged check that survives a cross-host build.
|
||||
*
|
||||
* The export check has to load the addon, so it cannot run when the packaging
|
||||
* host is not the target platform/arch -- and that skip is how a Windows
|
||||
* release built elsewhere could ship a node-pty that leaks every MSYS pane
|
||||
* child out of its job. Reading the binary needs neither.
|
||||
*
|
||||
* It resolves the addon the way the loader does rather than reading one path:
|
||||
* only the PE machine field separates a cross-arch package that built correctly
|
||||
* from one whose rebuild silently emitted the host's arch, and the first is a
|
||||
* correct package whose leftover prebuild is never reached. See the table in
|
||||
* docs/reference/windows-msys-job-breakaway.md.
|
||||
*
|
||||
* Nothing loadable is fatal, not skipped: that package has no ConPTY backend,
|
||||
* which a gate must not shrug at.
|
||||
*/
|
||||
function verifyPackagedConptyBreakawayMarker(resourcesDir, targetArch, 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 architecture = normalizeNodePtyWindowsArch(targetArch)
|
||||
const candidates = packagedConptyCandidates(resourcesDir, architecture)
|
||||
const exists = options.exists ?? existsSync
|
||||
const present = candidates.filter((candidate) => exists(candidate.path))
|
||||
if (present.length === 0) {
|
||||
throw new Error(
|
||||
[
|
||||
`Packaged node-pty for win32-${architecture} has no conpty.node on any path its loader`,
|
||||
`tries (${candidates.map((c) => c.path).join(', ')}), so the packaged app has no`,
|
||||
'ConPTY backend at all.',
|
||||
'Nothing here can be checked for the Cygwin/MSYS job-breakaway denial, and a gate that',
|
||||
'cannot see its subject refuses rather than assume.'
|
||||
].join(' ')
|
||||
)
|
||||
}
|
||||
// Read once: the same header answers "which one loads" and "what did we find".
|
||||
const inspected = present.map((candidate) => ({
|
||||
...candidate,
|
||||
machine: readPeMachine(candidate.path)
|
||||
}))
|
||||
const loaded = inspected.find((candidate) => candidate.machine === PE_MACHINE[architecture])
|
||||
if (!loaded) {
|
||||
throw new Error(
|
||||
[
|
||||
`Packaged node-pty for win32-${architecture} has conpty.node at`,
|
||||
`${describeCandidates(inspected)},`,
|
||||
'and the app can load none of them: a Windows process only loads a PE of its own',
|
||||
`machine, which for win32-${architecture} is`,
|
||||
`0x${PE_MACHINE[architecture].toString(16)}.`,
|
||||
'Rebuild node-pty for the target architecture and repackage.'
|
||||
].join(' ')
|
||||
)
|
||||
}
|
||||
const addonPath = loaded.path
|
||||
if (conptyDeniesCygwinBreakaway(addonPath)) {
|
||||
console.log(
|
||||
`[verify-packaged-node-pty] OK — win32-${architecture} loads ${addonPath}, which denies ` +
|
||||
'MSYS job breakaway'
|
||||
)
|
||||
return
|
||||
}
|
||||
if (!loaded.prebuilt) {
|
||||
throw staleConptySourceBuildError(addonPath)
|
||||
}
|
||||
// Past here the app falls back to the published prebuild, which never carries
|
||||
// the patch. Why it fell back decides the remedy, and the three are different
|
||||
// enough that naming the wrong one wastes the reader's build.
|
||||
const unusableSourceBuilds = inspected.filter((candidate) => !candidate.prebuilt)
|
||||
if (unusableSourceBuilds.some((candidate) => candidate.machine === null)) {
|
||||
throw new Error(
|
||||
[
|
||||
`Packaged node-pty for win32-${architecture} falls back to ${addonPath}, which predates`,
|
||||
'the Cygwin/MSYS job-breakaway denial, because the source build beside it is not a PE',
|
||||
`image at all: ${describeCandidates(unusableSourceBuilds)}.`,
|
||||
'A truncated, empty or quarantined build artifact looks like this. Rebuild node-pty and',
|
||||
'repackage. See docs/reference/windows-msys-job-breakaway.md.'
|
||||
].join(' ')
|
||||
)
|
||||
}
|
||||
if (unusableSourceBuilds.length > 0) {
|
||||
throw new Error(
|
||||
[
|
||||
`Packaged node-pty for win32-${architecture} falls back to ${addonPath}, which predates`,
|
||||
'the Cygwin/MSYS job-breakaway denial, because the source build beside it is the wrong',
|
||||
`architecture: ${describeCandidates(unusableSourceBuilds)}.`,
|
||||
'A cross-arch rebuild that did not honour --arch looks exactly like this. Re-run',
|
||||
`config/scripts/rebuild-native-deps.mjs --platform=win32 --arch=${architecture},`,
|
||||
'confirm it emitted a conpty.node of that machine, and repackage.',
|
||||
'See docs/reference/windows-msys-job-breakaway.md.'
|
||||
].join(' ')
|
||||
)
|
||||
}
|
||||
throw new Error(
|
||||
[
|
||||
`Packaged node-pty for win32-${architecture} loads ${addonPath}, the published prebuilt`,
|
||||
'fallback, which predates the Cygwin/MSYS job-breakaway denial: its per-PTY job still',
|
||||
'carries JOB_OBJECT_LIMIT_BREAKAWAY_OK, so every Git Bash pane child is created outside',
|
||||
'the job and survives terminatePtyJob.',
|
||||
'It is here because this package holds no node-pty source build at all for',
|
||||
'prunePackagedNodePty to have replaced it with, and only a host that can build node-pty',
|
||||
`for win32-${architecture} produces one.`,
|
||||
`If this IS a Windows ${architecture} host, the rebuild did not leave one -- check the`,
|
||||
'beforeBuild output above. Otherwise package this Windows slice on a host that can.',
|
||||
'See docs/reference/windows-msys-job-breakaway.md.'
|
||||
].join(' ')
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The whole Windows verdict for one packaged slice.
|
||||
*
|
||||
* Both halves live here rather than in the afterPack hook so that "the marker
|
||||
* sweep runs even when the export check cannot" is a tested claim instead of
|
||||
* the shape of an if/else somebody could re-nest.
|
||||
*/
|
||||
function verifyPackagedWindowsNodePty(resourcesDir, targetArch, options = {}) {
|
||||
;(options.verifyMarker ?? verifyPackagedConptyBreakawayMarker)(resourcesDir, targetArch)
|
||||
const hostPlatform = options.hostPlatform ?? process.platform
|
||||
if (hostPlatform !== 'win32' || !options.canExecuteTargetArch) {
|
||||
console.log(
|
||||
'[verify-packaged-node-pty] skipped the export check on a cross-platform or cross-arch package'
|
||||
)
|
||||
return
|
||||
}
|
||||
;(options.verifyExports ?? verifyPackagedNodePtyJobOwnership)(resourcesDir)
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
packagedConptyCandidates,
|
||||
verifyPackagedConptyBreakawayMarker,
|
||||
verifyPackagedNodePtyJobOwnership,
|
||||
verifyPackagedWindowsNodePty
|
||||
}
|
||||
|
||||
@@ -1,10 +1,53 @@
|
||||
import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'
|
||||
import { createRequire } from 'node:module'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { dirname, join } from 'node:path'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { peImage } from './windows-pe-image-fixture.mjs'
|
||||
|
||||
const require = createRequire(import.meta.url)
|
||||
const {
|
||||
verifyPackagedNodePtyJobOwnership
|
||||
packagedConptyCandidates,
|
||||
verifyPackagedConptyBreakawayMarker,
|
||||
verifyPackagedNodePtyJobOwnership,
|
||||
verifyPackagedWindowsNodePty
|
||||
} = require('./verify-packaged-node-pty-job-ownership.cjs')
|
||||
const { CYGWIN_BREAKAWAY_MARKER } = require('./node-pty-job-ownership.cjs')
|
||||
|
||||
const fixtureDir = mkdtempSync(join(tmpdir(), 'packaged-node-pty-job-'))
|
||||
const ELECTRON_BUILDER_CONFIG = readFileSync(
|
||||
new URL('../electron-builder.config.cjs', import.meta.url),
|
||||
'utf8'
|
||||
)
|
||||
|
||||
/** A real enough addon: a machine field the arch check reads, and the marker. */
|
||||
function conptyImage({ arch = 'x64', cygwinBreakawayDenied = true } = {}) {
|
||||
return Buffer.concat([
|
||||
peImage({ arch }),
|
||||
cygwinBreakawayDenied ? CYGWIN_BREAKAWAY_MARKER : Buffer.alloc(0)
|
||||
])
|
||||
}
|
||||
|
||||
function writeAddon(name, options) {
|
||||
const path = join(fixtureDir, name)
|
||||
writeFileSync(path, conptyImage(options))
|
||||
return path
|
||||
}
|
||||
|
||||
/** A packaged resources tree carrying exactly the conpty.node files named. */
|
||||
function packagedResources(addons) {
|
||||
const resourcesDir = mkdtempSync(join(fixtureDir, 'resources-'))
|
||||
const nodePtyDir = join(resourcesDir, 'node_modules', 'node-pty')
|
||||
for (const [relativePath, options] of Object.entries(addons)) {
|
||||
const addonPath = join(nodePtyDir, ...relativePath.split('/'))
|
||||
mkdirSync(dirname(addonPath), { recursive: true })
|
||||
writeFileSync(addonPath, conptyImage(options))
|
||||
}
|
||||
return resourcesDir
|
||||
}
|
||||
|
||||
const CURRENT_ADDON = writeAddon('current.node', { cygwinBreakawayDenied: true })
|
||||
const PRE_MSYS_ADDON = writeAddon('pre-msys.node', { cygwinBreakawayDenied: false })
|
||||
|
||||
const PATCHED = {
|
||||
dir: '../build/Release/',
|
||||
@@ -15,31 +58,39 @@ const PATCHED = {
|
||||
}
|
||||
}
|
||||
|
||||
const packaged = (native, addonPath = CURRENT_ADDON) => ({
|
||||
platform: 'win32',
|
||||
loadNative: () => ({ native, addonPath })
|
||||
})
|
||||
|
||||
describe('verifyPackagedNodePtyJobOwnership', () => {
|
||||
it('accepts the packaged patched ConPTY binding', () => {
|
||||
expect(() =>
|
||||
verifyPackagedNodePtyJobOwnership('resources', {
|
||||
platform: 'win32',
|
||||
loadNative: () => PATCHED
|
||||
})
|
||||
).not.toThrow()
|
||||
expect(() => verifyPackagedNodePtyJobOwnership('resources', packaged(PATCHED))).not.toThrow()
|
||||
})
|
||||
|
||||
it('rejects a packaged upstream prebuild', () => {
|
||||
expect(() =>
|
||||
verifyPackagedNodePtyJobOwnership('resources', {
|
||||
platform: 'win32',
|
||||
loadNative: () => ({ dir: '../prebuilds/win32-x64/', module: {} })
|
||||
})
|
||||
verifyPackagedNodePtyJobOwnership(
|
||||
'resources',
|
||||
packaged({ dir: '../prebuilds/win32-x64/', module: {} })
|
||||
)
|
||||
).toThrow(/missing listJobProcessIds, terminateJob, assignCurrentProcessToJob/)
|
||||
})
|
||||
|
||||
// A release built against a stale native cache ships the MSYS orphan bug
|
||||
// while exporting every job function, so packaging has to read the binary.
|
||||
it('rejects a packaged build that predates the Cygwin/MSYS breakaway denial', () => {
|
||||
expect(() =>
|
||||
verifyPackagedNodePtyJobOwnership('resources', packaged(PATCHED, PRE_MSYS_ADDON))
|
||||
).toThrow(/predates the Cygwin\/MSYS job-breakaway denial/)
|
||||
})
|
||||
|
||||
it('requires the patched source-build directory', () => {
|
||||
expect(() =>
|
||||
verifyPackagedNodePtyJobOwnership('resources', {
|
||||
platform: 'win32',
|
||||
loadNative: () => ({ ...PATCHED, dir: '../prebuilds/win32-x64/' })
|
||||
})
|
||||
verifyPackagedNodePtyJobOwnership(
|
||||
'resources',
|
||||
packaged({ ...PATCHED, dir: '../prebuilds/win32-x64/' })
|
||||
)
|
||||
).toThrow(/expected patched build\/Release/)
|
||||
})
|
||||
|
||||
@@ -49,3 +100,331 @@ describe('verifyPackagedNodePtyJobOwnership', () => {
|
||||
expect(loadNative).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('packagedConptyCandidates', () => {
|
||||
// Pinned because the gate resolves the addon by walking this list in order:
|
||||
// a wrong order blesses a binary the app would never reach.
|
||||
it('walks the paths node-pty tries, in node-pty order', () => {
|
||||
expect(packagedConptyCandidates('RES', 'arm64').map((candidate) => candidate.path)).toEqual([
|
||||
join('RES', 'node_modules', 'node-pty', 'build', 'Release', 'conpty.node'),
|
||||
join('RES', 'node_modules', 'node-pty', 'lib', 'build', 'Release', 'conpty.node'),
|
||||
join('RES', 'node_modules', 'node-pty', 'build', 'Debug', 'conpty.node'),
|
||||
join('RES', 'node_modules', 'node-pty', 'lib', 'build', 'Debug', 'conpty.node'),
|
||||
join('RES', 'node_modules', 'node-pty', 'prebuilds', 'win32-arm64', 'conpty.node'),
|
||||
join('RES', 'node_modules', 'node-pty', 'lib', 'prebuilds', 'win32-arm64', 'conpty.node')
|
||||
])
|
||||
})
|
||||
|
||||
// Only the published prebuild gets the "no rebuild here can fix this" advice.
|
||||
it('knows which of them node-pty publishes prebuilt', () => {
|
||||
expect(packagedConptyCandidates('RES', 'x64').map((candidate) => candidate.prebuilt)).toEqual([
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
true,
|
||||
true
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('verifyPackagedConptyBreakawayMarker', () => {
|
||||
// The release as built today: prunePackagedNodePty already dropped the
|
||||
// same-arch prebuild because a patched source build replaced it.
|
||||
it('passes a package whose only ConPTY load path carries the denial', () => {
|
||||
const resourcesDir = packagedResources({ 'build/Release/conpty.node': {} })
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).not.toThrow()
|
||||
})
|
||||
|
||||
// The cross-host package. No host but Windows can build conpty.node, so there
|
||||
// is no build/Release for prune to have replaced the prebuild with -- and the
|
||||
// published prebuild is exactly the binary that leaks every MSYS pane child.
|
||||
it('fails a cross-host package left holding the published prebuild', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'prebuilds/win32-x64/conpty.node': { cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).toThrow(
|
||||
/predates the Cygwin\/MSYS job-breakaway denial/
|
||||
)
|
||||
})
|
||||
|
||||
it('tells that package how to fix it, which is not a rebuild it can run', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'prebuilds/win32-x64/conpty.node': { cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).toThrow(
|
||||
/this package holds no node-pty source build at all[\s\S]*Otherwise package this Windows slice on a host that can/
|
||||
)
|
||||
})
|
||||
|
||||
// The same state reaches this from a capable host too, when the rebuild left
|
||||
// nothing: telling that packager to change hosts would send them nowhere.
|
||||
it('does not assume the packaging host is the wrong one', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'prebuilds/win32-x64/conpty.node': { cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).toThrow(
|
||||
/If this IS a Windows x64 host, the rebuild did not leave one/
|
||||
)
|
||||
})
|
||||
|
||||
// The cross-arch package that worked: beforeBuild rebuilds node-pty for the
|
||||
// TARGET arch, so build/Release is patched and loadable and the prebuild
|
||||
// prune left behind is never reached. Failing this would be a false positive
|
||||
// whose advice -- change hosts -- is both wrong and impossible.
|
||||
it('passes a cross-arch package whose build/Release really is the target arch', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'build/Release/conpty.node': { arch: 'arm64' },
|
||||
'prebuilds/win32-arm64/conpty.node': { arch: 'arm64', cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'arm64')).not.toThrow()
|
||||
})
|
||||
|
||||
// The cross-arch package that silently did not: build/Release is the
|
||||
// packaging host's own arch, the target cannot load it, and the loader falls
|
||||
// through to the unpatched prebuild underneath.
|
||||
it('fails a cross-arch package whose build/Release is the packaging host arch', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'build/Release/conpty.node': { arch: 'x64' },
|
||||
'prebuilds/win32-arm64/conpty.node': { arch: 'arm64', cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'arm64')).toThrow(
|
||||
/prebuilds[\\/]win32-arm64/
|
||||
)
|
||||
})
|
||||
|
||||
it('refuses a package whose every conpty.node is the wrong architecture', () => {
|
||||
const resourcesDir = packagedResources({ 'build/Release/conpty.node': { arch: 'x64' } })
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'arm64')).toThrow(
|
||||
/the app can load none of them/
|
||||
)
|
||||
})
|
||||
|
||||
// Naming the machine it found is what separates a cross-arch build from a
|
||||
// truncated download, which are the same "cannot load this" to the loader.
|
||||
it('names what it found rather than guessing why', () => {
|
||||
const resourcesDir = packagedResources({ 'build/Release/conpty.node': { arch: 'x64' } })
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'arm64')).toThrow(
|
||||
/machine 0x8664[\s\S]*0xaa64/
|
||||
)
|
||||
})
|
||||
|
||||
it('calls a candidate that is not a PE image what it is', () => {
|
||||
const resourcesDir = packagedResources({})
|
||||
const addonPath = join(
|
||||
resourcesDir,
|
||||
'node_modules',
|
||||
'node-pty',
|
||||
'build',
|
||||
'Release',
|
||||
'conpty.node'
|
||||
)
|
||||
mkdirSync(dirname(addonPath), { recursive: true })
|
||||
writeFileSync(addonPath, Buffer.alloc(0x200))
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).toThrow(/not a PE image/)
|
||||
})
|
||||
|
||||
// A truncated or quarantined artifact reaches the gate looking exactly like a
|
||||
// cross-arch build, and "re-run with --arch" is not the command that fixes it.
|
||||
it('does not blame --arch for a source build that is not a PE image', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'prebuilds/win32-x64/conpty.node': { cygwinBreakawayDenied: false }
|
||||
})
|
||||
const addonPath = join(
|
||||
resourcesDir,
|
||||
'node_modules',
|
||||
'node-pty',
|
||||
'build',
|
||||
'Release',
|
||||
'conpty.node'
|
||||
)
|
||||
mkdirSync(dirname(addonPath), { recursive: true })
|
||||
writeFileSync(addonPath, Buffer.alloc(0x200))
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).toThrow(
|
||||
/is not a PE image at all[\s\S]*truncated, empty or quarantined/
|
||||
)
|
||||
})
|
||||
|
||||
// The remedy for this one is a rebuild, not a different host, and the
|
||||
// difference is a build somebody has to run twice to find out.
|
||||
it('blames the wrong-arch source build rather than the host, when there is one', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'build/Release/conpty.node': { arch: 'x64' },
|
||||
'prebuilds/win32-arm64/conpty.node': { arch: 'arm64', cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'arm64')).toThrow(
|
||||
/the source build beside it is the wrong architecture[\s\S]*machine 0x8664/
|
||||
)
|
||||
})
|
||||
|
||||
it('tells that build the command that would fix it', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'build/Release/conpty.node': { arch: 'x64' },
|
||||
'prebuilds/win32-arm64/conpty.node': { arch: 'arm64', cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'arm64')).toThrow(
|
||||
/rebuild-native-deps\.mjs --platform=win32 --arch=arm64/
|
||||
)
|
||||
})
|
||||
|
||||
it('does not tell it to change hosts, which would not help', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'build/Release/conpty.node': { arch: 'x64' },
|
||||
'prebuilds/win32-arm64/conpty.node': { arch: 'arm64', cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'arm64')).toThrow(
|
||||
/^(?![\s\S]*Package this Windows slice on such a host)[\s\S]*$/
|
||||
)
|
||||
})
|
||||
|
||||
it('ignores a prebuild for an arch this slice will never load', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'build/Release/conpty.node': {},
|
||||
'prebuilds/win32-arm64/conpty.node': { arch: 'arm64', cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).not.toThrow()
|
||||
})
|
||||
|
||||
// build/Debug sits between Release and the prebuilds in the load order, and
|
||||
// nothing prunes it, so it wins whenever Release cannot be loaded.
|
||||
it('resolves past a Release build the target cannot load', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'build/Release/conpty.node': { arch: 'x64' },
|
||||
'build/Debug/conpty.node': { arch: 'arm64', cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'arm64')).toThrow(
|
||||
/build[\\/]Debug/
|
||||
)
|
||||
})
|
||||
|
||||
// A stale source build is the packaging host's own to rebuild, so it gets the
|
||||
// advice that actually works rather than the cross-host one.
|
||||
it('tells a stale source build to rebuild, not to change hosts', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'build/Release/conpty.node': { cygwinBreakawayDenied: false }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).toThrow(
|
||||
/Rebuild node-pty from source/
|
||||
)
|
||||
})
|
||||
|
||||
// Nothing to load is not "a layout we do not recognise", it is a package with
|
||||
// no ConPTY backend, and a gate that cannot see its subject is not a gate.
|
||||
it('refuses rather than skip a package with no conpty.node at all', () => {
|
||||
const resourcesDir = packagedResources({})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).toThrow(
|
||||
/no conpty\.node on any path its loader tries/
|
||||
)
|
||||
})
|
||||
|
||||
it('names every path it looked at when it finds none', () => {
|
||||
const resourcesDir = packagedResources({})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).toThrow(
|
||||
/build[\\/]Release[\s\S]*build[\\/]Debug[\s\S]*prebuilds[\\/]win32-x64/
|
||||
)
|
||||
})
|
||||
|
||||
// Present but unreadable is the state that used to pass, so it must not warn.
|
||||
// The read error itself is the message; the point is that it does not return.
|
||||
it('fails rather than pass a candidate it cannot read', () => {
|
||||
const resourcesDir = packagedResources({})
|
||||
mkdirSync(join(resourcesDir, 'node_modules', 'node-pty', 'build', 'Release', 'conpty.node'), {
|
||||
recursive: true
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'x64')).toThrow()
|
||||
})
|
||||
|
||||
it('accepts the electron-builder Arch enum the afterPack hook passes', () => {
|
||||
const resourcesDir = packagedResources({
|
||||
'prebuilds/win32-arm64/conpty.node': { arch: 'arm64' }
|
||||
})
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 3)).not.toThrow()
|
||||
})
|
||||
|
||||
it('refuses a target arch no Windows slice ships', () => {
|
||||
const resourcesDir = packagedResources({ 'build/Release/conpty.node': {} })
|
||||
expect(() => verifyPackagedConptyBreakawayMarker(resourcesDir, 'ia32')).toThrow(
|
||||
/Unsupported packaged node-pty Windows architecture/
|
||||
)
|
||||
})
|
||||
|
||||
it('looks where electron-builder actually lands the addon', () => {
|
||||
const exists = vi.fn().mockReturnValue(false)
|
||||
expect(() =>
|
||||
verifyPackagedConptyBreakawayMarker(join('out', 'win-unpacked', 'resources'), 'x64', {
|
||||
exists
|
||||
})
|
||||
).toThrow()
|
||||
expect(exists).toHaveBeenCalledWith(
|
||||
join(
|
||||
'out',
|
||||
'win-unpacked',
|
||||
'resources',
|
||||
'node_modules',
|
||||
'node-pty',
|
||||
'build',
|
||||
'Release',
|
||||
'conpty.node'
|
||||
)
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('verifyPackagedWindowsNodePty', () => {
|
||||
const spies = () => ({ verifyMarker: vi.fn(), verifyExports: vi.fn() })
|
||||
|
||||
// The bug this replaced: the marker check sat in the else of the host gate, so
|
||||
// the cross-host package it exists for was the one package it never checked.
|
||||
it.each([
|
||||
['a cross-platform host', { hostPlatform: 'darwin', canExecuteTargetArch: true }],
|
||||
['a cross-arch slice', { hostPlatform: 'win32', canExecuteTargetArch: false }],
|
||||
['both', { hostPlatform: 'linux', canExecuteTargetArch: false }],
|
||||
['neither', { hostPlatform: 'win32', canExecuteTargetArch: true }]
|
||||
])('checks the marker on %s', (_case, host) => {
|
||||
const { verifyMarker, verifyExports } = spies()
|
||||
verifyPackagedWindowsNodePty('resources', 'x64', { ...host, verifyMarker, verifyExports })
|
||||
expect(verifyMarker).toHaveBeenCalledWith('resources', 'x64')
|
||||
})
|
||||
|
||||
it('loads the addon for the export check only where that can work', () => {
|
||||
const { verifyMarker, verifyExports } = spies()
|
||||
verifyPackagedWindowsNodePty('resources', 'x64', {
|
||||
hostPlatform: 'win32',
|
||||
canExecuteTargetArch: true,
|
||||
verifyMarker,
|
||||
verifyExports
|
||||
})
|
||||
expect(verifyExports).toHaveBeenCalledWith('resources')
|
||||
})
|
||||
|
||||
it.each([
|
||||
['a cross-platform host', { hostPlatform: 'darwin', canExecuteTargetArch: true }],
|
||||
['a cross-arch slice', { hostPlatform: 'win32', canExecuteTargetArch: false }]
|
||||
])('skips the export check on %s', (_case, host) => {
|
||||
const { verifyMarker, verifyExports } = spies()
|
||||
verifyPackagedWindowsNodePty('resources', 'x64', { ...host, verifyMarker, verifyExports })
|
||||
expect(verifyExports).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Swallowing the marker verdict would leave a gate that runs and decides
|
||||
// nothing, which is the failure mode this whole change is about.
|
||||
it('lets the marker verdict fail the package', () => {
|
||||
const verifyMarker = vi.fn(() => {
|
||||
throw new Error('predates the Cygwin/MSYS job-breakaway denial')
|
||||
})
|
||||
expect(() =>
|
||||
verifyPackagedWindowsNodePty('resources', 'x64', {
|
||||
hostPlatform: 'win32',
|
||||
canExecuteTargetArch: true,
|
||||
verifyMarker,
|
||||
verifyExports: vi.fn()
|
||||
})
|
||||
).toThrow(/predates the Cygwin\/MSYS job-breakaway denial/)
|
||||
})
|
||||
|
||||
it('is what the afterPack hook calls for a Windows slice', () => {
|
||||
expect(ELECTRON_BUILDER_CONFIG).toContain(
|
||||
'verifyPackagedWindowsNodePty(resourcesDir, context.arch, { canExecuteTargetArch })'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
import { createRequire } from 'node:module'
|
||||
|
||||
const { PE_MACHINE } = createRequire(import.meta.url)('./windows-pe-machine.cjs')
|
||||
|
||||
/**
|
||||
* A PE image with nothing in it but a readable `IMAGE_FILE_HEADER.Machine`.
|
||||
*
|
||||
* Fixtures need this because the Windows addon gates read the binary: one that
|
||||
* is not a PE cannot stand in for an addon whose architecture decides whether
|
||||
* the app loads it at all.
|
||||
*/
|
||||
export function peImage({ arch = 'x64', machine, peOffset = 0x80, signature = 'PE\0\0' } = {}) {
|
||||
if (machine === undefined && PE_MACHINE[arch] === undefined) {
|
||||
throw new Error(`No PE machine value for ${arch}; a fixture must not invent one.`)
|
||||
}
|
||||
const image = Buffer.alloc(peOffset + 8)
|
||||
image.write('MZ', 0, 'latin1')
|
||||
image.writeUInt32LE(peOffset, 0x3c)
|
||||
image.write(signature, peOffset, 'latin1')
|
||||
image.writeUInt16LE(machine ?? PE_MACHINE[arch], peOffset + 4)
|
||||
return image
|
||||
}
|
||||
@@ -0,0 +1,40 @@
|
||||
'use strict'
|
||||
|
||||
const { closeSync, openSync, readSync } = require('node:fs')
|
||||
|
||||
/** PE `IMAGE_FILE_HEADER.Machine` values, so a cross-build cannot silently emit host arch. */
|
||||
const PE_MACHINE = { x64: 0x8664, arm64: 0xaa64 }
|
||||
|
||||
/**
|
||||
* `IMAGE_FILE_HEADER.Machine`, or null when the file is not a PE image.
|
||||
*
|
||||
* Null rather than a throw because callers ask this of files they did not
|
||||
* produce: a truncated or non-PE binary is a thing to decide about, not a crash.
|
||||
*/
|
||||
function readPeMachine(binaryPath) {
|
||||
const fd = openSync(binaryPath, 'r')
|
||||
try {
|
||||
const dosHeader = Buffer.alloc(0x40)
|
||||
if (readSync(fd, dosHeader, 0, 0x40, 0) < 0x40 || dosHeader.toString('latin1', 0, 2) !== 'MZ') {
|
||||
return null
|
||||
}
|
||||
const peOffset = dosHeader.readUInt32LE(0x3c)
|
||||
const peHeader = Buffer.alloc(6)
|
||||
if (
|
||||
readSync(fd, peHeader, 0, 6, peOffset) < 6 ||
|
||||
peHeader.toString('latin1', 0, 4) !== 'PE\0\0'
|
||||
) {
|
||||
return null
|
||||
}
|
||||
return peHeader.readUInt16LE(4)
|
||||
} finally {
|
||||
closeSync(fd)
|
||||
}
|
||||
}
|
||||
|
||||
/** How to name a machine field in an error, including the file that has none. */
|
||||
function describePeMachine(machine) {
|
||||
return machine === null ? 'not a PE image' : `machine 0x${machine.toString(16)}`
|
||||
}
|
||||
|
||||
module.exports = { PE_MACHINE, describePeMachine, readPeMachine }
|
||||
@@ -0,0 +1,85 @@
|
||||
import { mkdtempSync, writeFileSync } from 'node:fs'
|
||||
import { createRequire } from 'node:module'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { peImage } from './windows-pe-image-fixture.mjs'
|
||||
|
||||
const require = createRequire(import.meta.url)
|
||||
const { PE_MACHINE, describePeMachine, readPeMachine } = require('./windows-pe-machine.cjs')
|
||||
|
||||
const fixtureDir = mkdtempSync(join(tmpdir(), 'windows-pe-machine-'))
|
||||
|
||||
function writeImage(name, build) {
|
||||
const path = join(fixtureDir, name)
|
||||
writeFileSync(path, build())
|
||||
return path
|
||||
}
|
||||
|
||||
const X64 = writeImage('x64.node', () => peImage({ machine: PE_MACHINE.x64 }))
|
||||
const ARM64 = writeImage('arm64.node', () => peImage({ machine: PE_MACHINE.arm64 }))
|
||||
|
||||
describe('PE_MACHINE', () => {
|
||||
// Spelled out rather than taken from the module: the fixtures below build
|
||||
// their headers from these, so a table that is wrong in both entries would
|
||||
// otherwise agree with itself.
|
||||
it('holds the IMAGE_FILE_MACHINE values Windows actually stamps', () => {
|
||||
expect(PE_MACHINE).toEqual({ x64: 0x8664, arm64: 0xaa64 })
|
||||
})
|
||||
})
|
||||
|
||||
describe('readPeMachine', () => {
|
||||
it.each([
|
||||
['x64', X64, PE_MACHINE.x64],
|
||||
['arm64', ARM64, PE_MACHINE.arm64]
|
||||
])('reads the machine field of a %s image', (_case, path, expected) => {
|
||||
expect(readPeMachine(path)).toBe(expected)
|
||||
})
|
||||
|
||||
// Callers ask this of files they did not produce, so anything that is not a
|
||||
// PE has to be an answer rather than a crash.
|
||||
it.each([
|
||||
['a Mach-O or ELF binary', () => Buffer.alloc(0x200)],
|
||||
['a file too short to hold a DOS header', () => Buffer.from('MZ')],
|
||||
[
|
||||
'a DOS stub whose PE offset points nowhere',
|
||||
() => peImage({ machine: 0x8664, peOffset: 0x8000 }).subarray(0, 0x88)
|
||||
],
|
||||
['a file with no PE signature', () => peImage({ machine: 0x8664, signature: 'XX\0\0' })]
|
||||
])('returns null for %s', (_case, build) => {
|
||||
expect(readPeMachine(writeImage(`not-pe-${Math.random()}.bin`, build))).toBeNull()
|
||||
})
|
||||
|
||||
it('respects the DOS header pointer rather than a fixed offset', () => {
|
||||
const path = writeImage('shifted.node', () =>
|
||||
peImage({ machine: PE_MACHINE.arm64, peOffset: 0x120 })
|
||||
)
|
||||
expect(readPeMachine(path)).toBe(PE_MACHINE.arm64)
|
||||
})
|
||||
})
|
||||
|
||||
describe('peImage fixture', () => {
|
||||
// A fixture that quietly stamps machine 0x0000 for an arch it does not know
|
||||
// is the same species of silent lie these gates exist to catch.
|
||||
it('refuses to invent a machine value for an arch it has none for', () => {
|
||||
expect(() => peImage({ arch: 'ia32' })).toThrow(/must not invent one/)
|
||||
})
|
||||
|
||||
it('still takes an explicit machine, which is how the non-PE cases are built', () => {
|
||||
expect(readPeMachine(writeImage('explicit.node', () => peImage({ machine: 0x1234 })))).toBe(
|
||||
0x1234
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('describePeMachine', () => {
|
||||
// The callers put this straight into an error, and "not a PE image" is a
|
||||
// different problem from a cross-arch build.
|
||||
it('names a machine field it read', () => {
|
||||
expect(describePeMachine(PE_MACHINE.arm64)).toBe('machine 0xaa64')
|
||||
})
|
||||
|
||||
it('says so when there was none, rather than throwing on null', () => {
|
||||
expect(describePeMachine(null)).toBe('not a PE image')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,137 @@
|
||||
# Why an MSYS pane's children escape the per-PTY job
|
||||
|
||||
Every child started from a Git Bash / MSYS2 / Cygwin pane leaves the pane's job
|
||||
object unless the job is created **without** `JOB_OBJECT_LIMIT_BREAKAWAY_OK`.
|
||||
`terminatePtyJob` then reports `terminated` and leaves the child running — the
|
||||
orphan that holds a worktree directory open.
|
||||
|
||||
The denial is already in `config/patches/node-pty@1.1.0.patch`
|
||||
(`usesCygwinRuntime`, added in #19068). This page records the measurement
|
||||
behind it, because the failure mode it prevents is indistinguishable from a
|
||||
stale native addon and the gates of the day could not tell the two apart.
|
||||
|
||||
## The mechanism
|
||||
|
||||
The MSYS/Cygwin runtime asks for `CREATE_BREAKAWAY_FROM_JOB` on the
|
||||
`CreateProcessW` inside its `spawn`/`exec` path. A job that carries
|
||||
`JOB_OBJECT_LIMIT_BREAKAWAY_OK` grants it, so the child is created outside the
|
||||
job; a job without that limit denies it with `ERROR_ACCESS_DENIED`, and the
|
||||
runtime retries without the flag rather than failing the spawn. `fork` is not
|
||||
affected — forked Cygwin processes stay in the job either way.
|
||||
|
||||
Measured on Windows 11 `10.0.26200.9168`, Git `2.55.0.windows.3`,
|
||||
bash `5.3.15(1)-release`, node `v24.18.0`, `useConptyDll: true`, for
|
||||
`node-pty.spawn('C:\Program Files\Git\bin\bash.exe', ['--noprofile','--norc','-i'])`
|
||||
— `+J` / `-J` is membership of the per-PTY job, read with
|
||||
`QueryInformationJobObject(JobObjectBasicProcessIdList)`:
|
||||
|
||||
```
|
||||
bin\bash.exe +J ConPTY shell (assigned by node-pty)
|
||||
└ ..\usr\bin\bash.exe +J launcher hand-off, plain CreateProcess
|
||||
└ usr\bin\bash.exe +J Cygwin fork for the typed command
|
||||
└ node.exe -J Cygwin exec -- ESCAPES HERE
|
||||
```
|
||||
|
||||
`bin\bash.exe` is a 47 KB launcher, not an MSYS binary: `C:\Program Files\Git\bin`
|
||||
holds only `bash.exe`, `git.exe` and `sh.exe`, with no `msys-2.0.dll`. Its
|
||||
hand-off to `bin\..\usr\bin\bash.exe` is an ordinary `CreateProcess` and keeps
|
||||
job membership. Only the MSYS runtime's own spawn breaks away.
|
||||
|
||||
The shell-replacement shape (`bash -c 'exec "$BASH" --noprofile --norc -i'`)
|
||||
loses membership one step earlier, at the `exec`, and everything below inherits
|
||||
the loss:
|
||||
|
||||
```
|
||||
bin\bash.exe +J
|
||||
└ ..\usr\bin\bash.exe +J
|
||||
└ usr\bin\bash.exe -J Cygwin exec -- ESCAPES HERE
|
||||
└ usr\bin\bash -J
|
||||
└ node.exe -J
|
||||
```
|
||||
|
||||
Both shapes leak. The `exec` is not the cause; it only moves the escape earlier.
|
||||
|
||||
## The A/B that pins it
|
||||
|
||||
One source tree, one toolchain, one variable — `usesCygwinRuntime` forced to
|
||||
`false` so the per-PTY job keeps `JOB_OBJECT_LIMIT_BREAKAWAY_OK`:
|
||||
|
||||
| per-PTY job limit | `listPtyJobProcessIds` | child reaped by `terminatePtyJob` | runs |
|
||||
| ---------------------- | ---------------------- | --------------------------------- | ---- |
|
||||
| `BREAKAWAY_OK` set | 2 pids, child absent | no | 0/2 |
|
||||
| `BREAKAWAY_OK` cleared | 5 pids, child present | yes | 4/4 |
|
||||
|
||||
The job **is** the right boundary. With breakaway denied it holds the whole MSYS
|
||||
tree, including the child that detached from the console, and one
|
||||
`terminateJob` reaps all of it. No alternative tracking mechanism is needed.
|
||||
|
||||
Denying breakaway did not break ordinary launches from the pane: `git`,
|
||||
`cmd //c`, an absolute-path `node`, a `&`-backgrounded job with `disown`, and
|
||||
`where.exe` all returned 0 with no `Access is denied`, identically to the
|
||||
breakaway-allowed control. Untested: a **non-Cygwin** program that itself passes
|
||||
`CREATE_BREAKAWAY_FROM_JOB` (installers, updaters) and therefore has no runtime
|
||||
to retry for it. That needs a helper that calls `CreateProcess` with the flag;
|
||||
`start /b` does not exercise it (it uses `CREATE_NEW_CONSOLE`).
|
||||
|
||||
## A stale addon looks exactly like the bug
|
||||
|
||||
`config/scripts/node-pty-job-ownership.cjs` used to assert only that
|
||||
`terminateJob`, `listJobProcessIds` and `assignCurrentProcessToJob` are
|
||||
exported. All three predate #19068, so a `conpty.node` built before it passed
|
||||
every gate: `isPtyJobOwnershipAvailable()` returned true and
|
||||
`windows-pty-job.win32.test.ts` passed 6/6, while
|
||||
`windows-msys-job.win32.test.ts` failed with a two-pid job list that read as a
|
||||
source defect rather than a build-freshness one.
|
||||
|
||||
When that test fails, check the binary before the code:
|
||||
|
||||
```js
|
||||
// UTF-16LE, because usesCygwinRuntime holds the literals
|
||||
readFileSync(conptyNodePath).includes(Buffer.from('msys-2.0.dll', 'utf16le'))
|
||||
```
|
||||
|
||||
False means the addon predates the fix; rebuild node-pty from patched source.
|
||||
Note that a git worktree sharing `node_modules` with its main checkout shares
|
||||
that checkout's `build/Release/conpty.node`, so pinning the _source_ to a commit
|
||||
does not pin the _addon_.
|
||||
|
||||
The gate asserts that marker, the way `stagedRelayAddonIsUnpatched()` in
|
||||
`src/main/windows/windows-process-table.ts` already sniffs a patched addon by a
|
||||
binary import name. Symbol presence cannot distinguish patch revisions; a marker
|
||||
can.
|
||||
|
||||
Because the marker is a literal in `conpty.cc` and the gate's copy of it is a
|
||||
separate constant, `ensure-native-runtime-job-ownership.test.mjs` asserts the
|
||||
patch still adds `L"msys-2.0.dll"` to that file. Without that, editing the patch
|
||||
would turn the gate into a permanent false positive that fails every correctly
|
||||
rebuilt addon and tells the developer to do the one thing that cannot help.
|
||||
|
||||
## Every path the loader can fall through to
|
||||
|
||||
`loadNativeModule` tries `build/Release`, then `build/Debug`, then
|
||||
`prebuilds/win32-<arch>`, each relative to node-pty's root and then to `lib/`,
|
||||
swallowing every failure in between. A require of a wrong-architecture `.node`
|
||||
is one of those failures, so the candidate that runs is the first one the target
|
||||
arch can actually load. The published prebuild is always the last candidate and
|
||||
never carries the patch:
|
||||
|
||||
| package | `build/Release` | prebuild pruned? | what the app loads |
|
||||
| -------------------- | ----------------------------- | ---------------- | ------------------ |
|
||||
| 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, 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.
|
||||
|
||||
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
|
||||
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
|
||||
ConPTY backend to load.
|
||||
Reference in New Issue
Block a user