diff --git a/src/main/windows/windows-command-line-recovery-health.test.ts b/src/main/windows/windows-command-line-recovery-health.test.ts new file mode 100644 index 00000000000..3791acf6c93 --- /dev/null +++ b/src/main/windows/windows-command-line-recovery-health.test.ts @@ -0,0 +1,59 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + reportWindowsCommandLineRecoveryHealth, + resetWindowsCommandLineRecoveryHealthForTests +} from './windows-command-line-recovery-health' + +describe('windows command line recovery health', () => { + let warn: ReturnType + + beforeEach(() => { + resetWindowsCommandLineRecoveryHealthForTests() + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + }) + afterEach(() => { + warn.mockRestore() + }) + + const selfRow = (commandLine: string): { pid: number; commandLine: string } => ({ + pid: process.pid, + commandLine + }) + + it('warns when the querying process has no command line of its own', () => { + // We can always open ourselves with PROCESS_QUERY_LIMITED_INFORMATION, so + // an empty self command line means the query is refused host-wide. + reportWindowsCommandLineRecoveryHealth([selfRow(''), { pid: 4, commandLine: '' }]) + + expect(warn).toHaveBeenCalledTimes(1) + expect(warn.mock.calls[0][0]).toContain('ProcessCommandLineInformation') + expect(warn.mock.calls[0][1]).toEqual({ processes: 2, withCommandLine: 0 }) + }) + + it('warns once per session, not once per scan', () => { + for (let i = 0; i < 5; i++) { + reportWindowsCommandLineRecoveryHealth([selfRow('')]) + } + expect(warn).toHaveBeenCalledTimes(1) + }) + + it('stays quiet when only other processes denied a handle', () => { + // Roughly a quarter of a real table denies access; that is not a fault. + const denied = Array.from({ length: 40 }, (_, index) => ({ + pid: index + 1, + commandLine: '' + })) + reportWindowsCommandLineRecoveryHealth([selfRow('node.exe --run'), ...denied]) + expect(warn).not.toHaveBeenCalled() + }) + + it('stays quiet when our own row is absent, which the caller rejects separately', () => { + reportWindowsCommandLineRecoveryHealth([{ pid: process.pid + 1, commandLine: '' }]) + expect(warn).not.toHaveBeenCalled() + }) + + it('treats a missing commandLine field the same as an empty one', () => { + reportWindowsCommandLineRecoveryHealth([{ pid: process.pid }]) + expect(warn).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/windows/windows-command-line-recovery-health.ts b/src/main/windows/windows-command-line-recovery-health.ts new file mode 100644 index 00000000000..173e8f5a969 --- /dev/null +++ b/src/main/windows/windows-command-line-recovery-health.ts @@ -0,0 +1,47 @@ +/** + * One warning, once per session, when command-line recovery has stopped working. + * + * The reader has no PEB fallback by design: falling back was a total-defeat + * vector, because any single anomalous NTSTATUS reinstated address-space reads + * for the life of the process. The cost of removing it is a cliff -- if + * `NtQueryInformationProcess(ProcessCommandLineInformation)` is refused, every + * command line comes back empty and agent identity matching silently degrades + * to image names, while the addon still loads and still enumerates, so every + * health check stays green. A cliff nobody can see is the failure mode this + * area keeps producing, so it gets a signal. + * + * 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 host-wide -- not that some + * target denied a handle, which is normal for roughly a quarter of the table. + * That is why this keys on our own row rather than a fraction: no threshold to + * tune, and no false positive on a hardened box where most processes deny. + */ +type CommandLineRow = { pid: number; commandLine?: string } + +let warned = false + +export function reportWindowsCommandLineRecoveryHealth(rows: CommandLineRow[]): void { + if (warned) { + return + } + const self = rows.find((row) => row.pid === process.pid) + // No self row is a different failure, and the caller's own guard rejects it. + if (!self || (self.commandLine ?? '') !== '') { + return + } + warned = true + const recovered = rows.filter((row) => (row.commandLine ?? '') !== '').length + console.warn( + '[windows-process-table] command-line recovery is refused on this host: the querying ' + + 'process has no command line of its own, so NtQueryInformationProcess' + + '(ProcessCommandLineInformation) is failing for every process. Agent identity matching ' + + 'falls back to image names. A hooked ntdll that does not know class 60 is the usual cause.', + { processes: rows.length, withCommandLine: recovered } + ) +} + +/** Test-only: the warning is once per session, so cases must not inherit it. */ +export function resetWindowsCommandLineRecoveryHealthForTests(): void { + warned = false +} diff --git a/src/main/windows/windows-process-table.test.ts b/src/main/windows/windows-process-table.test.ts index 609de009820..c4ef7a2e829 100644 --- a/src/main/windows/windows-process-table.test.ts +++ b/src/main/windows/windows-process-table.test.ts @@ -9,13 +9,16 @@ import { readWindowsProcessTableFresh, resetWindowsProcessTableForTests } from './windows-process-table' +import { resetWindowsCommandLineRecoveryHealthForTests } from './windows-command-line-recovery-health' const getAllProcesses = vi.fn() // A real snapshot always contains the querying process; the reader rejects a // table without it, because that is what a blocked CreateToolhelp32Snapshot -// returns -- an empty list rather than an error. -const SELF = { pid: process.pid, ppid: 0, name: 'vitest.exe' } +// returns -- an empty list rather than an error. It also always carries our own +// command line, since a process can always open itself -- an empty one there is +// the host-wide-refusal signal, not a fixture detail. +const SELF = { pid: process.pid, ppid: 0, name: 'vitest.exe', commandLine: 'vitest.exe --run' } const NATIVE = [ SELF, { @@ -52,7 +55,13 @@ describe('windows process table', () => { it('maps native rows, defaulting an unreadable command line to empty', async () => { const rows = await readWindowsProcessTableFresh() expect(rows).toEqual([ - { pid: process.pid, ppid: 0, name: 'vitest.exe', command: '', memoryBytes: undefined }, + { + pid: process.pid, + ppid: 0, + name: 'vitest.exe', + command: 'vitest.exe --run', + memoryBytes: undefined + }, { pid: 100, ppid: 4, @@ -400,7 +409,13 @@ describe('resolving the native reader', () => { }) const rows = await readWindowsProcessTableFresh() expect(rows).toEqual([ - { pid: process.pid, ppid: 0, name: 'vitest.exe', command: '', memoryBytes: undefined }, + { + pid: process.pid, + ppid: 0, + name: 'vitest.exe', + command: 'vitest.exe --run', + memoryBytes: undefined + }, { pid: 100, ppid: 4, @@ -469,3 +484,56 @@ describe('resolving the native reader', () => { expect(resolve).not.toHaveBeenCalled() }) }) + +// The cliff the removed PEB fallback leaves behind: a hooked ntdll that refuses +// class 60 empties every command line, and the addon still loads and still +// enumerates, so every health check the app has stays green. +describe('warning when command-line recovery is refused host-wide', () => { + let platform: PropertyDescriptor | undefined + let warn: ReturnType + + beforeEach(() => { + platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + resetWindowsCommandLineRecoveryHealthForTests() + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + }) + + afterEach(() => { + __setWindowsProcessTreeLoaderForTests() + warn.mockRestore() + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + }) + + type NativeRow = { pid: number; ppid: number; name: string; commandLine?: string } + + function loaderReturning(rows: NativeRow[], commandLineFlag: number): void { + __setWindowsProcessTreeLoaderForTests(() => ({ + ProcessDataFlag: { None: 0, Memory: 1, CommandLine: commandLineFlag, CreationTime: 4 }, + getAllProcesses: (cb: (r: NativeRow[] | undefined) => void) => cb(rows) + })) + } + + it('warns once when our own row comes back with no command line', async () => { + loaderReturning([{ pid: process.pid, ppid: 0, name: 'vitest.exe' }], 2) + await readWindowsProcessTableFresh() + await readWindowsProcessTableFresh() + expect(warn).toHaveBeenCalledTimes(1) + expect(warn.mock.calls[0][0]).toContain('ProcessCommandLineInformation') + }) + + it('stays quiet when our own command line came back', async () => { + loaderReturning(NATIVE, 2) + await readWindowsProcessTableFresh() + expect(warn).not.toHaveBeenCalled() + }) + + it('stays quiet when the read never asked for a command line', async () => { + // A reader that requests identity fields only must not read as a refusal. + loaderReturning([{ pid: process.pid, ppid: 0, name: 'vitest.exe' }], 0) + await readWindowsProcessTableFresh() + expect(warn).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/windows/windows-process-table.ts b/src/main/windows/windows-process-table.ts index 79deda82ab5..d2d838f5cb4 100644 --- a/src/main/windows/windows-process-table.ts +++ b/src/main/windows/windows-process-table.ts @@ -1,5 +1,6 @@ import { createRequire } from 'node:module' import { createProcessTableSnapshotReader } from '../../shared/process-table-snapshot' +import { reportWindowsCommandLineRecoveryHealth } from './windows-command-line-recovery-health' import { readWindowsProcessRowsWithCim } from './windows-process-table-cim-scan' /** @@ -147,7 +148,7 @@ function loadWindowsProcessTree(): WindowsProcessTreeModule | null { * `requestInProgress` and clears it only after draining its callback queue, * with no try/catch. One throw or one worker that never calls back leaves it * latched, every later call enqueues a callback that never fires, and the - * single-flight cache above then holds a promise that never settles — the + * single-flight cache above then holds a promise that never settles — the * process table is dead for the life of the app. The PowerShell reader this * replaced self-healed in 3s because execFile owned a timeout; keep that. */ @@ -174,7 +175,7 @@ function readNativeRows(): Promise { // Why only when the module is absent: a binding that loads is the fast // path even when a read fails or wedges, so a failing native reader must // never silently start forking shells at the caller's poll rate. Absence - // is the one condition that can never resolve itself — see + // is the one condition that can never resolve itself — see // docs/reference/windows-process-enumeration.md. return readCimRows() } @@ -235,6 +236,10 @@ function readNativeRows(): Promise { reject(new Error('windows process table is unreadable')) return } + // Only meaningful when a command line was actually asked for. + if ((flags & native.ProcessDataFlag.CommandLine) !== 0) { + reportWindowsCommandLineRecoveryHealth(processes) + } resolve( processes.map((row) => ({ pid: row.pid, @@ -286,7 +291,7 @@ export function readWindowsProcessTable(): Promise { /** * A snapshot taken after this call returns. * - * Identity checks during teardown must not reuse a cached row — it can predate + * Identity checks during teardown must not reuse a cached row — it can predate * the very process exit it is being asked about. */ export function readWindowsProcessTableFresh(): Promise {