mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 08:02:43 +00:00
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.
This commit is contained in:
@@ -114,6 +114,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
|
||||
|
||||
@@ -55,6 +55,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).
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
# 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 existing gates cannot 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` asserts only that `terminateJob`,
|
||||
`listJobProcessIds` and `assignCurrentProcessToJob` are exported. All three
|
||||
predate #19068, so a `conpty.node` built before it passes every gate,
|
||||
`isPtyJobOwnershipAvailable()` returns true, `windows-pty-job.win32.test.ts`
|
||||
passes 6/6 — and `windows-msys-job.win32.test.ts` fails with a two-pid job list
|
||||
that reads 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 should assert the same 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 or an exported revision number can.
|
||||
Reference in New Issue
Block a user