* fix(windows): read command lines from the kernel, not each process's PEB MDE incident D scored Orca for suspicious memory activity: the vendored `@vscode/windows-process-tree` recovered every process's command line by opening it with `PROCESS_QUERY_INFORMATION | PROCESS_VM_READ` and chaining three `ReadProcessMemory` calls through the PEB and `RTL_USER_PROCESS_PARAMETERS`. On a 750ms/2s cadence over the whole table that is the credential-dumping primitive, whatever the intent. Windows 8.1 added `NtQueryInformationProcess`'s `ProcessCommandLineInformation` class (60), which returns the same string as a kernel-built `UNICODE_STRING` under `PROCESS_QUERY_LIMITED_INFORMATION` alone. Electron's floor is Windows 10, so every supported OS has it. The PEB reader stays behind a process-wide latch that only `STATUS_INVALID_INFO_CLASS`/`NOT_SUPPORTED`/`NOT_IMPLEMENTED` can set; a pid that merely denied a handle does not re-arm it, because `PROCESS_QUERY_INFORMATION` implicitly grants the limited right and so cannot be obtained where the weaker open already failed. The same hunk drops `PROCESS_VM_READ` from `GetProcessMemoryUsage` and `GetCpuUsage`, which acquired it and never read an address space. Measured on Windows 11 (514 processes), counted in-process by swapping the addon's import table entries for counting stubs, per CommandLine scan: `ReadProcessMemory` 1128 -> 0, desired access 0x0410 -> 0x1000, p50 12.7ms -> 9.3ms. Command lines were byte-identical on every process both readers recovered (376/376, 379/379 across runs), including a 24,068-character argv with quotes, non-ASCII and trailing whitespace, and a WOW64 target. Three processes that refused the old rights granted the new one; none went the other way. * chore(deps): refresh the windows-process-tree patch hash in the lockfile * fix(windows): drop the PEB fallback and detect the unpatched prebuilt Review of #17886 found three ways the reader could still perform, or silently resume, the primitive it exists to remove. The class-missing latch was a permanent, process-wide, one-way downgrade back to the PEB read, and any single target returning STATUS_INVALID_INFO_CLASS / NOT_SUPPORTED / NOT_IMPLEMENTED could trip it. On an EDR-hooked ntdll -- the entire premise of this change -- a hook that does not recognise class 60 would have restored PROCESS_VM_READ plus three ReadProcessMemory per pid per scan for the life of the process, unobservably, on precisely the machines this was written for. The fallback is deleted rather than guarded: GetProcessCommandLine now returns false and leaves the command line empty, which callers already handle, so the addon imports no ReadProcessMemory at all. That absence is what makes the property checkable on the artifact. The published 0.8.0 tarball ships a loadable prebuilt built from unpatched source; it is node-addon-api, so a bare require() accepts it, allowBuilds is false and CI installs with --ignore-scripts, and a rebuild that soft-exits on a Windows file lock leaves it in place. Source-text guards could never see it. windowsProcessTreeAddonReadsProcessMemory() checks the compiled binary instead, and is wired into the install check, the rebuild, and the relay build. The repair itself never worked: `git apply` run inside a work tree prefixes patch paths with the cwd-relative prefix, skips what does not match, and exits 0, so the branch always fell through to its own post-check throw. The package dir is always under the project root, while the fixture that covered it was in %TEMP%, outside any repo. Blinding git with GIT_DIR fixes it, and the test now runs inside a real work tree. Also from review: bounds-check the returned UNICODE_STRING against the allocation (not the size the second query clobbers) and cap the probe so a bogus length cannot bad_alloc a whole scan; test NT_SUCCESS explicitly; value- initialize ProcessInfo, which left `memory` as stack garbage -- measured, 82 processes reported the same bogus working set; and correct a comment in windows-process-table.ts that still described the command line as a PEB read. Re-measured on Windows 11 (543 processes): ReadProcessMemory 1128 -> 0, with the symbol absent from the import table so the IAT hook finds no slot to count; desired access 0x0410 -> 0x1000 on all 543 opens; p50 13.5 -> 12.3ms; 405/405 command lines byte-identical including a 24,087-character quoted non-ASCII argv and a WOW64 target; 3 processes recovered only by the new path, 0 only by the old. * chore(deps): refresh the windows-process-tree patch hash in the lockfile * test(scripts): stage a script's local imports into the native-runtime fixture ensure-native-runtime.mjs gained an import of windows-process-tree-gyp-rebuild.mjs, but the fixture copied only the script itself, so every case in the suite died with ERR_MODULE_NOT_FOUND before reaching its own assertions. copyScriptWithLocalModules already walks a script's co-located imports for exactly this reason -- its own doc comment names this failure -- so use it rather than listing files by hand. The two Windows cases still fail here, on a missing node-pty ConPTY runtime that also fails on main; this only stops a resolution error from standing in front of whatever they were meant to catch. * fix(windows): route a locked stale addon to the Windows file-lock message `pnpm install` with Orca running aborted with a raw EPERM stack. The stale-binary guard -- which deletes an addon that still imports ReadProcessMemory so a skipped rebuild cannot use it -- ran outside the try whose catch classifies Windows file locks, and whose message is literally "Close running Orca/Electron/dev processes for this worktree": exactly this situation. Measured rather than assumed: rmSync against a loaded (memory-mapped) addon throws EPERM, and `force: true` does not help, since it only swallows ENOENT. Cold copies of the same file delete fine. So the delete threw a page before the handler that knows what it means. Moving the guard inside the try is the whole fix; the classifier already matches the EPERM text. The new case runs the real script against a temp project whose stale addon is held open by a live child process, and fails against the old placement with the raw `syscall: 'rm'` stack the report described. * feat(windows): warn once when command-line recovery is refused host-wide Removing the PEB fallback removed a total-defeat vector, but it left a cliff: if NtQueryInformationProcess(ProcessCommandLineInformation) is refused -- a hooked ntdll that does not know class 60 -- every command line comes back empty and agent identity matching silently degrades to image names. The addon still loads and still enumerates, so every health check the app has stays green. A cliff nobody can see is the failure mode this area keeps producing. The querying process is the unambiguous probe. A process can always open itself with PROCESS_QUERY_LIMITED_INFORMATION, so its own command line coming back empty means the query is refused for every process -- not that some target denied a handle, which is normal for roughly a quarter of the table. Keying on our own row rather than a fraction means no threshold to tune and no false positive on a hardened box where most processes deny. One warning per session, gated on the CommandLine flag actually being requested so a future identity-only reader cannot trip it. The suite's own SELF fixture gains a command line for the same reason: a self row without one is the alarm, not a detail. * fix(windows): check the relay's staged addon at load, and answer tri-state Two gaps in the ReadProcessMemory check, both about what it does not see. It only ever looked at node_modules/@vscode/windows-process-tree. A relay host has no node_modules of ours: it loads ./windows-process-tree.node staged beside the bundle. The relay build asserts the symbol on the artifact it produces, but a bundle and the addon beside it redeploy independently, so a host that has not taken a new bundle keeps whatever binary is already there -- and the published prebuilt is node-addon-api, so it binds cleanly and then walks every process's address space. loadWindowsProcessTree now checks that file too and refuses it, falling back to the CIM scan: slower, but not the thing an EDR quarantines a host for. The predicate is duplicated rather than imported, because the config-script copy is install-time tooling that drags in node-gyp and child_process, and this module is bundled into the app and the relay. And it returned false for a binary that is not there. All three callers happened to be safe, but the name read as a safety predicate, so a future caller would take a missing binary as verified. inspectWindowsProcessTreeAddon() now answers clean/unpatched/missing over an explicit binary path -- which is also what lets the relay's staged addon be checked at all -- and each caller states which state it acts on. Both are covered by cases that fail against the old code: without the load-time check the unpatched staged addon is bound and the CIM fallback never runs, and with 'missing' folded back into 'clean' the absence case fails outright. * test(windows): load the addon in beforeAll, not at collection time loadAddon() ran while the file was being collected, so on a Windows checkout with no built addon the require threw before any case existed and took the seven patch-text cases down with it -- cases that read only the patch file and need no binary at all. Verified both ways against a deliberately unresolvable addon path: at collection time vitest reports "no tests" for the file; from beforeAll the seven text cases pass and only the three addon cases go. * fix(deps): normalize the windows-process-tree patch to LF and let pnpm own its hash `pnpm install --frozen-lockfile` failed on this branch on every platform with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH, which breaks CI and the release build. Two coupled defects. The patch file was committed with CRLF -- 174 CR bytes, against zero on main -- and `.gitattributes` pins `/config/patches/*.patch -text` precisely so checkout cannot convert it, so those bytes reached every runner. And pnpm hashes a patch **LF-normalized**, so the raw sha256 of a CRLF file is a value pnpm never computes: raw sha256 322965470c05f63d8527f7d8e892ee26ee444136b66b57fd64c362a9f2ff05d1 LF-normalized f8ea245391c94da5770045aeea01fa6de466c2199c6ef46b5b769b398aa9823e The lockfile carried the raw one, at all three sites. It is the only one of the seven patches where the two digests differ, which is why the other six passed. Normalized the patch to LF and took pnpm's own value from `pnpm install --no-frozen-lockfile`; nothing here is hand-computed. With the file LF-only the two interpretations coincide, so the lockfile, the contract test's no-CR assertion and its hash assertion all agree at one number -- and `config/scripts/windows-process-tree-patch-contract.test.mjs`, which was red on this branch for the same reason, is green again. The lockfile diff is exactly the three hash lines. The regression check is the installer, not a digest. Two separate reviews "verified" the shipped hash by recomputing sha256(patchBytes) and matching the lockfile; both were wrong, because both repeated the same wrong assumption about which bytes pnpm hashes. A check that reproduces the original mistake is not independent. So the new case runs `pnpm install --frozen-lockfile --lockfile-only --ignore-scripts` against a copy of the manifest, lockfile and patches, and asserts exit 0 -- verified by deletion: restoring the shipped hash fails it with the exact ERR_PNPM_LOCKFILE_CONFIG_MISMATCH from the branch's package (windows) job. Also corrected the `.gitattributes` comment claiming pnpm hashes patches byte-for-byte. The `-text` setting is right -- `git apply` needs the exact bytes -- but that sentence is the claim that produced the wrong hash twice. * ci(windows): run the process-tree patch suites in CI Both suites only self-skip off Windows, so the binary-level check that the addon carries no ReadProcessMemory passed vacuously in every lane. * fix(windows): force core.autocrlf=input for the patch repair My LF normalization of the windows-process-tree patch broke the `git apply` repair path introduced in this PR. The two are coupled and I checked only one. Those 174 CR bytes were not editor noise. They sat on exactly the pre-image lines and nowhere else -- 107/107 in src/process.cc, 67/67 in src/process_commandline.cc, 0 on every added or context line -- because @vscode/windows-process-tree@0.8.0 ships those two sources as CRLF. Normalizing the patch made its pre-image stop matching the file it is applied against. Measured, reconstructing the true CRLF pre-image from the pre-normalization blob and applying the current LF patch: core.autocrlf plain -c core.autocrlf=input true exit 0 exit 0 input exit 0 exit 0 false exit 1 exit 0 `false` is Git's own built-in default and what "checkout as-is" selects in the Git for Windows installer -- on this box the `true` that hides it comes from the installer's system gitconfig, not from anything in the repo. There the repair throws, ensureWindowsProcessTreeCommandLinePatch reports "still reads the PEB, and repairing it ... failed", isWindowsNativeLockError does not match that text, and `pnpm install` dies with no path forward. Forcing the mode rather than `--ignore-whitespace`: both fix every cell and both leave the applied file fully LF, but `input` relaxes line endings only, so a hunk whose real content drifted is still rejected. The repair rewrites a security-relevant source file; it should stay strict about everything except the thing that is legitimately ambiguous. Not reverting the patch to CRLF: windows-process-tree-patch-contract.test.mjs (pre-existing on main) forbids CR bytes in it, and pnpm computes the same hash either way. LF plus the forced mode is the end state. The suite could not have caught this. The fixture built its pre-image from the patch itself and joined with '\n', so fixture and patch agreed by construction on any encoding -- once again a test that passes without its fix. It now emits the CRLF the real package ships, and the case runs under both autocrlf modes pinned through a temp HOME gitconfig, because the repair blinds git to the repo and so reads global config. Verified by deletion in both directions: with the flag removed the autocrlf=false case fails with the exact "still reads the PEB" dead end while autocrlf=true still passes, and with the fixture back on LF all eight cases pass with no fix present at all. Also corrected the .gitattributes comment I added last commit. It said `git apply` needs the bytes the patch was written against, which is now false -- the pinned bytes are LF and the bytes it was written against are CRLF. That is the same class of confident-and-wrong claim that produced the bad hash twice. * fix(windows): assert the rebuilt addon, and install the patch for real in tests Three follow-ups from review. **The packaged binary had no check.** The relay build asserts its own artifact and ensure-native-runtime asserts what it loads, but nothing looked at the addon copied into the packaged app -- so a rebuild that silently produced the upstream reader shipped. `rebuild-native-deps.mjs` now asserts `clean` on it after `rebuild()`. This is also the caller D4's tri-state was missing: every existing site branches on `=== 'unpatched'`, so `missing` still behaved exactly like `clean` everywhere, which was the thing making it a state rather than a boolean. Here both non-clean states fail, and they fail differently: after a rebuild that reported success, an absent binary is a broken build, not an absence to shrug at. The fake `rebuild()` had to start producing a binary for that to mean anything, so it now emits stand-in bytes and takes `addon: 'clean' | 'unpatched' | 'none'`. Verified by deletion: with the assertion removed both new cases pass. **The frozen-install case could not see a patch at all.** `--lockfile-only` resolves and never applies one, so its coverage stops at hash consistency. Added a case that installs `@vscode/windows-process-tree@0.8.0` for real with the patch and asserts the materialized `src/process_commandline.cc` carries the marker and no longer carries `ReadProcessMemory` -- about 1.5s for the pair. Correcting the brief on that one: it does **not** catch the `git apply` breakage from the previous commit. Measured -- with `-c core.autocrlf=input` removed it passes cleanly, because `pnpm install` uses pnpm's own patch applier and never runs our repair script. What it does catch is a patch pnpm can no longer apply: corrupting one pre-image line fails both cases. The repair path stays covered by the CRLF fixture in rebuild-native-deps-node-pty.test.mjs. Worth recording, since it decides whether the LF normalization was safe at all: pnpm applies the LF patch to the CRLF tarball sources without complaint, and materializes them as LF with the marker present and `ReadProcessMemory` absent. The primary install path was never affected -- only the `git apply` fallback was. **Dead timeout.** The frozen-install case passed `timeoutMs: 300_000` to the spawn while vitest capped the case itself at 30s, so on a cold runner vitest would have killed it first. Both cases now declare the budget they use. * test(windows): route the frozen-install check through the pnpm invocation owner The new patched-dependencies check hand-rolled a PATH walk naming 'pnpm.cmd', which the windows batch shim spawn boundary ratchet rejects: pnpm-cli-invocation already owns that decision for every other script, and its allowlist only shrinks. Reuse resolvePnpmCliInvocation for the command and prefixArgs, and the shared resolveCliCommand for the presence check, so no shim name is spelled here. Its `shell` flag is dropped because runProcessSync refuses it and already drives a shim through the interpreter itself. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
25 KiB
Reading the Windows process table
Orca needs three things from the Windows process table: who a PID's parent is (descendant walks and teardown identity), what a process is running (agent recognition), and how much memory/CPU it uses (Resource Manager).
Node cannot answer the first one without native code. That is why seven
independent readers existed, each forking powershell.exe to run
Get-CimInstance Win32_Process, with a wmic fallback that Windows 11 24H2 has
since removed.
Use the native snapshot
src/main/windows/windows-process-table.ts is the only module that may read the
table. It wraps a Toolhelp32 snapshot from @vscode/windows-process-tree.
import {
readWindowsProcessTable,
readWindowsProcessTableFresh
} from '../windows/windows-process-table'
readWindowsProcessTable()— shared TTL cache. Use for anything periodic.readWindowsProcessTableFresh()— a snapshot that starts after the call. Use for teardown identity, where a cached row can predate the exit it is being asked about.
Both reject when the table cannot be read. Do not convert that into an empty array. An empty table is a claim that nothing is running, and callers act on that claim by declaring a tree dead or a shell childless. "Unavailable" has to stay distinguishable from "empty" — collapsing the two is how a PTY tree survived its own teardown (#9045).
Measured on Windows 11 with 1050 processes (p50 / p95):
| p50 | p95 | |
|---|---|---|
| pid + ppid + name | 15.9 ms | 17.5 ms |
| + memory + command line | 30.6 ms | 33.7 ms |
Get-CimInstance via PowerShell |
706 ms | 723 ms |
Those are the module's published figures. The flag set this module actually
requests is CommandLine | CreationTime — not Memory, which cost a second
OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ) plus
GetProcessMemoryInfo per process (src/process.cc:47-63) for a value nothing
read. Dropping it halves the handles a snapshot opens. The remaining set sits
between the two rows above and has not been measured separately; on a real
Windows host, Get-Counter '\Process(Orca)\Handle Count' sampled across a
snapshot cadence is the check.
Those CIM numbers are from a 1050-process host. The scan scales with process count: on a 1486-process Windows SSH host it measured 1.36 s and produced 4.8 MiB of JSON, against the fallback's 3 s and 8 MiB limits. Both limits match the pre-#15749 reader, so relay hosts are at parity rather than newly at risk — but the headroom is roughly 2x on time and 1.7x on bytes, not the ~4x the 706 ms figure implies. On overflow the output is truncated, the JSON fails to parse, and the read rejects, so a busy host loses the table rather than receiving a wrong one.
When a read wedges
The vendored reader pushes every callback onto a module-global queue and drains
that queue only when the request holding its requestInProgress latch
completes. If a Toolhelp32 snapshot never comes back — an EDR hook, a restricted
token, a worker that dies — the latch is stuck for the life of the process and
every later call parks another closure in that queue.
Two guards, and they work together:
- a 3 s deadline on each read, so a caller gets a rejection instead of a promise that never settles;
- a sticky wedge: once a read misses its deadline and has not called back, the module refuses every further read until that read's callback fires.
The wedge used to be a 30 s cooldown that let one probe through per window. That
bounded the rate of new callbacks but not the total: a permanently wedged
reader retained one more closure every 30 s for as long as the app ran, and each
probe also blocked its caller for the full 3 s deadline first. Gating on the
outstanding read instead bounds retention at exactly one callback, and gives up
nothing on recovery — a probe queued behind the latch could never have observed
recovery anyway, whereas the stuck callback firing is the drain itself. On the
relay's bare addon, which has no queue of its own, it is also what keeps Orca
from re-entering CreateToolhelp32Snapshot while a call is still running.
That last part is not just tidiness. The addon runs each read as a
Napi::AsyncWorker, so a wedged read holds a libuv threadpool slot for good. On
the relay the JS queue is not there to absorb the retries, so one probe per
window would have pinned all four default threads inside ~2 minutes — hanging
every async fs and DNS call in that process, not only the process table.
A wedge does not engage the PowerShell fallback; see the next section for why only absence does.
The relay has no binding, and falls back
Relay deployment installs only node-pty and @parcel/watcher on the remote
host (RELAY_NATIVE_DEPS in src/main/ssh/ssh-relay-deploy.ts), so a Windows
machine used as an SSH host has no @vscode/windows-process-tree at all. It is
not added there on purpose. Both ways of installing it fail, and both were
checked on a real Windows SSH host with 1486 processes:
Installing it normally rebuilds from source, and that build fails. The
tarball carries a binding.gyp, so npm runs node-gyp rebuild regardless of
what is already compiled inside it. On a host that already had MSVC Build
Tools 2022 installed, that build still failed:
error MSB8040: Spectre-mitigated libraries are required for this project.
That is the requirement the binding.gyp hunk of our patch deletes, and the
patch cannot reach a remote host — pnpm patches do not cross SSH. Relay deploy
would then break outright rather than degrade: installNativeDeps throws on
failure, and the toolchain-skip retry is gated to Linux.
Skipping the build and using the shipped binary returns a truncated table.
Contrary to what this file used to claim, the published 0.8.0 tarball does
contain build/Release/windows_process_tree.node — an MSVC build directory that
looks accidentally published (.obj and .tlog files ship with it). It is
N-API, so it loads on any modern Node. But it predates our patch and still has
the process_count < 1024 cap, so on that 1486-process host:
LOADED OK
rows=1024
selfPid=21964 present=false
Exactly 1024 rows, with the querying process itself among the missing. The self-presence guard rejects that, so the fallback engages anyway — but only on hosts busy enough to cross the cap. That is worse than no binding at all: it works on a quiet machine and fails silently under load, which is precisely the shape of bug that survives testing.
So the constraint is not that no binary exists to ship. It is that the only binary available to ship is the broken one, and building the good one needs a toolchain the remote does not have.
Instead, windows-process-table.ts falls back to
readWindowsProcessRowsWithCim (windows-process-table-cim-scan.ts), the
Get-CimInstance scan this module replaced. The gate is deliberately narrow:
- it engages only when the module cannot be required, never when a loaded module fails, wedges, or returns an unreadable table — a present-but-failing reader must not silently start forking a shell at the caller's poll rate;
- a fallback that also fails still rejects, so "unavailable" never degrades into "nothing is running";
- the scan applies the same self-presence guard as the native path.
src/main/ssh/relay-native-dependency-coverage.test.ts asserts that every
native addon reachable from the relay entry is either installed on relay hosts
or listed there with the reason its absence is safe. That test exists because
#15749 shipped this gap: the relay tests injected a fake module through
__setWindowsProcessTreeLoaderForTests, so nothing exercised the real require.
Shipping the native reader to a relay anyway
The scan is the floor, not the destination: it costs ~1.4 s and a powershell.exe
where the addon costs ~57 ms. Release builds therefore compile the addon and ship
it as an optional relay artifact.
config/scripts/build-windows-process-tree-relay-addon.mjs builds it from the
source pnpm has already patched, on a Windows runner, and refuses to run if
any patch hunk is missing — the Spectre hunk fails loudly, the 1024-process
hunk fails silently, and the relative gyp path dies at configure on Windows.
The source is checked rather than the install trusted. It also reads the PE
machine field of the output, because a cross-build that quietly emitted host
arch would ship a binary the target cannot load.
Windows arm64 cross-compiles from the x64 runner — verified on real hardware,
producing IMAGE_FILE_MACHINE_ARM64 (0xaa64) against x64's 0x8664. It needs the
optional MSVC v143 ARM64 build tools component; without it node-gyp fails with
MSB8020, which is why the addon build runs before the long packaging step.
ORCA_REQUIRE_RELAY_NATIVE_ADDONS is a per-arch list so a future arch can be
added best-effort before it is promoted to required.
windows-process-table.ts binds the bare addon directly rather than the package
wrapper. That wrapper adds only a queue over getProcessList, and that queue is
the wedge described above — it latches a module-global requestInProgress with
no try/catch. This module already holds a single-flight and a deadline, so going
straight to the addon drops the duplicate.
The artifact is optional in RELAY_ARTIFACTS: hashed when present, so a relay
carrying it never shares an immutable directory with one that does not, and
never probed, because requiring a file only a Windows build machine can produce
would make a correct relay read as MISSING and redeploy forever. A relay built
on any other OS keeps using the scan.
Why the package is patched
config/patches/@vscode__windows-process-tree@0.8.0.patch carries four hunks.
- Spectre mitigation. The upstream
binding.gyprequires Spectre-mitigated libraries, which Orca's Windows build agents do not install.node-ptyis patched the same way for the same reason. - The 1024-process cap.
GetRawProcessListstopped after 1024 entries. Measured on a real host with 1051 processes, the module returned exactly 1024 and the querying process was itself among the 27 missing. A truncated snapshot silently hides the descendants a teardown is trying to reap — the exact failure the native path exists to remove. - Absolute
node-addon-apigyp path.require('node-addon-api').targetsis cwd-relative. node-gyp on Windows evaluates it from the pnpm store realpath, then loads the relative path from thenode_modulessymlink, sonode_addon_api.gypresolves outside the repo and hourly Windows builds die at configure.node-ptyis patched the same way for the same reason. - No PEB reads, no
PROCESS_VM_READ. See below.
The typings claim commandLine is truncated at 512 characters. Measured, it is
not: the longest observed on a real host was 26,059.
The command line comes from the kernel, not the target's memory
Upstream, GetProcessCommandLine opens every process with
PROCESS_QUERY_INFORMATION | PROCESS_VM_READ and issues three chained
ReadProcessMemory calls — PEB, RTL_USER_PROCESS_PARAMETERS, then the string
— to recover the command line. Walking another process's address space for
credentials-adjacent data on a repeating timer is what a credential dumper does,
so Defender for Endpoint scores it as such regardless of intent. Nothing about
the flag sets above changes that; only removing the read does.
Windows 8.1 added NtQueryInformationProcess's ProcessCommandLineInformation
class (60), which returns the same string as a UNICODE_STRING the kernel
builds, needing only PROCESS_QUERY_LIMITED_INFORMATION. Electron's floor is
Windows 10, so every OS Orca supports has it. The entry point is resolved with
GetProcAddress on ntdll.dll — it has no import library — and the size is
probed with a null-buffer call that answers STATUS_INFO_LENGTH_MISMATCH.
The same hunk drops PROCESS_VM_READ from GetProcessMemoryUsage and
GetCpuUsage, which acquired it and never read an address space:
GetProcessMemoryInfo and GetProcessTimes are satisfied by
PROCESS_QUERY_LIMITED_INFORMATION. Measured, both return identical values
under the weaker right on every process that opens at all.
Measured on Windows 11, ~540 processes, counted in-process by replacing the addon's import table entries with counting stubs:
per CommandLine scan |
before | after |
|---|---|---|
OpenProcess calls |
543 | 543 |
| desired access | 0x0410 (VM_READ | QUERY_INFORMATION) |
0x1000 (QUERY_LIMITED_INFORMATION) |
ReadProcessMemory |
1128 | 0 |
| p50 / p95 | 13.5 / 14.5 ms | 12.3 / 13.5 ms |
Command lines were byte-identical on every process both readers recovered
(405/405, and 399/399 and 376/376 on other runs), including a 24,087-character
argv with embedded quotes, non-ASCII characters and trailing whitespace, and a
WOW64 target. The weaker right is also a strict superset in reach: three
processes that refused PROCESS_QUERY_INFORMATION | PROCESS_VM_READ granted
PROCESS_QUERY_LIMITED_INFORMATION, and none went the other way.
There is no PEB fallback, deliberately
An earlier revision kept the PEB reader for a kernel without class 60, behind a
latch. That was wrong, and the reason is worth recording: ClassifyQueryFailure
mapped STATUS_INVALID_INFO_CLASS / NOT_SUPPORTED / NOT_IMPLEMENTED from
any single target onto a process-wide, one-way switch back to
PROCESS_VM_READ plus three ReadProcessMemory per pid per scan, for the life
of the process, with nothing observable from JS.
The environment this reader exists for is one where an EDR hooks ntdll. A hook
that returns STATUS_INVALID_INFO_CLASS for a class it does not recognise would
have silently reinstated the exact primitive the patch removes, on precisely the
machines it was written for — and one stray status from one process was enough.
The same applies under Wine or any instrumented ntdll.
So the fallback is gone rather than guarded. GetProcessCommandLine returns
false and leaves the command line empty, which is already a normal outcome
(WindowsProcessRow.command is documented as empty when a process denies a
query handle, and callers fall back to the image name). Degrading to no command
line is recoverable; silently resuming address-space reads is not.
This also makes the property checkable on the artifact rather than the source:
the patched reader never calls ReadProcessMemory, so the symbol is absent from
the compiled addon's import table. inspectWindowsProcessTreeAddon() in
config/scripts/windows-process-tree-gyp-rebuild.mjs is that check, and it is
the only way to tell the two binaries apart — see below. It answers
clean / unpatched / missing rather than a boolean, because a binary that is
not there has not been cleared, and a caller reading false as “verified” would
pass exactly the thing the check exists to catch.
Because the returned UNICODE_STRING comes from that same hookable boundary,
its Buffer and Length are bounds-checked against the allocation before the
characters are encoded, and the probed size is capped at the header plus 64 KiB
(Length is a USHORT) so a bogus size cannot turn into a bad_alloc that
fails an entire scan instead of one process.
The published tarball ships a loadable unpatched prebuilt
@vscode/windows-process-tree@0.8.0 publishes
build/Release/windows_process_tree.node in the tarball. It is node-addon-api,
so it is ABI-stable and loads cleanly under both Node and Electron — and it was
built from unpatched source, so it performs 1179 ReadProcessMemory calls and
opens every process at 0x0410 per scan.
That matters because allowBuilds is false for this package and CI installs
with --ignore-scripts, so nothing compiles it at install time. A require()
health check cannot tell the two binaries apart, and a rebuild that is skipped —
rebuild-native-deps.mjs soft-exits 0 on a Windows file lock during postinstall
— leaves the upstream prebuilt in place and cached.
Four checks close that, all keyed on the absent ReadProcessMemory import:
ensureWindowsProcessTreeCommandLinePatch()deletes a binary that still has it, so a skipped rebuild fails loudly instead of using the prebuilt;ensure-native-runtime.mjstreats such a binary as a load failure, which is what triggers the rebuild;- the relay build asserts it on the artifact it just produced;
loadWindowsProcessTree()asserts it again on the addon staged beside a relay bundle and refuses to bind one that still imports the symbol, falling back to the CIM scan. The build-time assertion is not enough on its own: a bundle and the addon beside it redeploy independently, so a host that has not taken a new bundle keeps whatever.nodeis already there.
What none of this does is narrow which processes are asked. A detailed scan
still queries every pid, including lsass.exe; it now asks with the same right
Task Manager uses instead of PROCESS_VM_READ. Restricting the command-line
pass to Orca's own subtree is the complementary change, and it belongs with the
identity/detailed reader split rather than here — a ppid-derived allowlist would
miss exactly the detached, reparented descendants the trackers exist to find
(#9045, #10475), so it needs the job-object membership as its source of truth.
Packaging
The addon is Windows-only, so it follows the same contract as
windows-native-registry (asserted by
config/scripts/package-electron-runtime-contract.test.mjs):
- an
optionalDependency, so a macOS/Linux install tolerates its absence; - not enabled in
allowBuildsinpnpm-workspace.yaml— pnpm installs optional dependencies on every host, and macOS/Linux must never runnode-gypfor it; - listed in the win32 branch of
rebuild-native-deps.mjsandensure-native-runtime.mjs; - copied into the packaged
node_modulesfor win32 only.
The relay's copy is a separate artifact staged beside the bundle, so a relay host only picks up a rebuilt addon on redeploy. Until then it keeps whatever binary it already has, which is why the addon is checked again at load.
What the snapshot does not provide
CreationDate (process start time) has no equivalent. Anything using a start
time to prove a PID has not been recycled — daemon identity, managed-hook
ownership, and CPU accounting in the memory collector — still reads it through
its own query. Those callers are not migrated.
Committed private bytes have no equivalent either, and the one memory value the
snapshot can carry is unusable for the sizes Orca now sees: process.cc stores
pmc.WorkingSetSize into a DWORD, so anything above 4 GB wraps. That is the
second reason windows-process-resource-collector.ts still runs its own
Get-CimInstance sweep — it needs PageFileUsage (commit) and the CPU-time
counters in the same pass. Migrating it to the native table would cost both, and
it is why this module no longer sets the Memory flag at all: the field had no
reader, and asking for it opened a handle per process on every snapshot.
Start time is a proxy for identity, not identity. The durable answer for the process trees Orca itself spawns is an inherited handle: a job object names the tree Orca created, so no start-time comparison is needed. Those readers should be resolved that way rather than by adding a start time to this module.
Do not adopt getProcessCpuUsage() from the package. It takes both CPU samples
inside one call with a blocking Sleep(1000) in the middle, which would hold a
libuv threadpool slot for a full second out of the Resource Manager's two-second
poll.
Owning a PTY's process tree
src/main/windows/windows-pty-job.ts is the counterpart to reading the table:
it answers "is this tree mine, and how do I kill it?" with a handle instead of
an inference.
node-pty is patched (config/patches/node-pty@1.1.0.patch) to create a job
object per ConPTY and assign the shell to it under CREATE_SUSPENDED, before
the shell can spawn anything. Assigning after the fact leaves a window in which
a fast child escapes the job.
terminatePtyJob(proc)— oneTerminateJobObjectcall for the whole tree.listPtyJobProcessIds(proc)— the live pids under a tree that is still tracked, including children that detached from the console.
Measured on Windows 11 against a shell whose grandchild was spawned detached:
job membership was [shell, grandchild] and one call killed both. Neither a
parent-pid walk nor GetConsoleProcessList sees that grandchild — it leaves
the console and reparents, which is what left claude.exe/node.exe/cmd.exe
holding worktree directories open (#9045, #10475, #10897).
The per-PTY job deliberately does not set
JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE. Measured on Windows 11: with that flag,
releasing the handle when the shell exits also kills whatever the user left
running, so typing exit in a pane reaped a start /b server that used to
survive. The job exists to make an explicit teardown exact, not to redefine
what a clean exit means.
Reaping a dead daemon's shells (#9195, #10415) is therefore a second, nested
job, not this one. The terminal daemon assigns itself to a kill-on-close job
at startup (assignHostProcessToKillOnCloseJob); children inherit membership,
so every pty is covered and the per-PTY jobs nest inside it. Its handle is
released only when the daemon process dies, so a crashed daemon reaps its tree
without changing what a clean shell exit means.
The split is the point. One job answers "kill exactly this pane's tree, now";
the other answers "do not strand anything if the host dies". Trying to get both
from one job is what reaped users' backgrounded work on a clean exit.
It belongs to the daemon and never to the app: an app-main crash must still
leave sessions alive, which .github/workflows/win-crash-survival-e2e.yml
asserts. The app spawns the daemon detached and is itself in no job, so
nothing is inherited across that boundary.
The consequence is that a PTY hosted by the app rather than the daemon gets a per-PTY job but no crash reaping. That is deliberate — the alternative is a kill-on-close job on the app, which is exactly what the crash-survival guarantee forbids.
Once the shell exits, node-pty drops its handle record and closes the job, so a
terminated tree reports null rather than []. Null means unverifiable in
the sense of ssh-execution-boundary.md — no job
support, not a ConPTY, or no longer tracked. It is never evidence that
processes died.
Both functions report unavailable / null rather than a false success when a
pty has no job — an outer job without JOB_OBJECT_LIMIT_BREAKAWAY_OK (some EDR
and container hosts) can refuse the assignment, and a pty started before this
build has none. Callers must fall back, not conclude the tree is gone. That
conflation is the original bug.
Known limitation: the baton table is not synchronised
node-pty keeps its per-terminal handles in a plain std::vector and erases from
it on a detached exit thread, while get_pty_baton is called from the main JS
thread. That race predates this change — PtyResize, PtyClear and PtyKill
all read the table the same way — but terminatePtyJob adds an instance of it:
the exit thread can close hJob between the lookup and TerminateJobObject.
Losing that race normally just returns FALSE, which surfaces as unavailable
and falls back. The case that would not be benign is a recycled HANDLE value,
where the call could reach a different job in the same process. Fixing it
properly means synchronising node-pty's handle table rather than adding a lock
around one accessor, so it is deliberately left alone here.
The patch must actually be compiled
node-pty prefers its upstream prebuild and only builds from source when
npm_config_build_from_source is set or no prebuild exists for the platform.
The Windows prebuild does not contain this patch, so a plain pnpm install
on Windows yields a node-pty without the job-object exports — and
terminatePtyJob then reports unavailable on every call, which is
indistinguishable from a correctly degraded build.
Packaging is unaffected: rebuild-native-deps.mjs rebuilds node-pty from source
for Electron and restores the ConPTY runtime files that a bare node-gyp rebuild skips. The gap is the node-runtime test environment, which is why
the Windows CI job rebuilds from source before running the win32 suites.
isPtyJobOwnershipAvailable() exists for exactly this: the win32 suite asserts
it is true before asserting anything else, so an unpatched binary fails loudly
instead of passing every case vacuously. That guard is what caught this.
requiresPatchedNodePtySourceBuild() in ensure-native-runtime.mjs now covers
win32 as well, and pnpm rebuild node-pty sets npm_config_build_from_source
so the patched source build actually replaces the upstream prebuild.