diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 536b5c0f273..38daf8a71de 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -869,6 +869,7 @@ jobs: src/shared/secure-file-fsync-flags.test.ts src/main/ipc/pty-codex-account-attribution.test.ts src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts + src/relay/windows-port-scan.win32.test.ts # Why the :parallel variant: identical to build:release except the three # electron-vite targets overlap instead of running back to back. The Linux package diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index c1d5c731cd4..f83c1508c26 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -238,7 +238,8 @@ const WINDOWS_PACKAGE_TESTS = [ 'src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts', 'src/shared/secure-file-fsync-flags.test.ts', 'src/main/ipc/pty-codex-account-attribution.test.ts', - 'src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts' + 'src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts', + 'src/relay/windows-port-scan.win32.test.ts' ] const DESKTOP_IRRELEVANT_PREFIXES = [ diff --git a/src/main/windows/windows-process-table.ts b/src/main/windows/windows-process-table.ts index 8d00f408132..6683770435d 100644 --- a/src/main/windows/windows-process-table.ts +++ b/src/main/windows/windows-process-table.ts @@ -29,6 +29,10 @@ import { readWindowsProcessRowsWithCim } from './windows-process-table-cim-scan' * Those are the module's published figures for both extra fields together; the * only flag set this module asks for is `CommandLine` (+ `CreationTime`, free), * which sits between the two rows and has not been separately measured. + * + * Both Toolhelp32 rows assume the optional `windows-process-tree.node` addon. + * The desktop bundles it; no released relay carries it, so on an SSH host the + * CIM row is the operative number and the child process is not avoided at all. */ export type WindowsProcessRow = { diff --git a/src/relay/windows-port-scan.test.ts b/src/relay/windows-port-scan.test.ts index 139d3ede6be..9fb074180b9 100644 --- a/src/relay/windows-port-scan.test.ts +++ b/src/relay/windows-port-scan.test.ts @@ -1,22 +1,30 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { execFileAsyncMock, execFileMock, promisifyCustom } = vi.hoisted(() => ({ - execFileAsyncMock: vi.fn(), - execFileMock: vi.fn(), - promisifyCustom: Symbol.for('nodejs.util.promisify.custom') -})) - -vi.mock('child_process', () => ({ - execFile: Object.assign(execFileMock, { - [promisifyCustom]: execFileAsyncMock - }) +const runProcessMock = vi.fn() +vi.mock('../shared/child-process/run-process', () => ({ + runProcess: (spec: unknown) => runProcessMock(spec) })) vi.mock('./relay-command-env', () => ({ buildRelayCommandEnv: () => ({ PATH: 'C:\\Windows\\System32' }) })) -const { scanWindowsListeningPorts } = await import('./windows-port-scan') +import { + __setWindowsProcessTableCimScanForTests, + __setWindowsProcessTreeLoaderForTests, + resetWindowsProcessTableForTests +} from '../main/windows/windows-process-table' +import { + resetWindowsPortScanDiagnosticsForTests, + scanWindowsListeningPorts +} from './windows-port-scan' + +type Spec = { + program: string + args?: readonly string[] + timeoutMs?: number | null + signal?: AbortSignal +} // The scanner drops any row whose pid is the relay process or its parent, so a fixture pid // that happens to match the vitest worker's own pid silently empties the result and the @@ -32,78 +40,365 @@ function pidUnlikeSelf(seed: number): number { return pid } -const POWERSHELL_PID = pidUnlikeSelf(1234) const NETSTAT_PID = pidUnlikeSelf(2468) +const SSHD_PID = pidUnlikeSelf(4321) +const POWERSHELL_PID = pidUnlikeSelf(1234) + +const NETSTAT_STDOUT = [ + ' Proto Local Address Foreign Address State PID', + ` TCP 0.0.0.0:3000 0.0.0.0:0 LISTENING ${NETSTAT_PID}`, + ` TCP [::]:3000 [::]:0 LISTENING ${NETSTAT_PID}`, + ` TCP 0.0.0.0:4000 93.184.216.34:443 ESTABLISHED ${NETSTAT_PID}`, + ` UDP 0.0.0.0:5353 *:* ${NETSTAT_PID}`, + ` TCP 0.0.0.0:2222 0.0.0.0:0 LISTENING ${SSHD_PID}` +].join('\r\n') + +function ok(stdout: string): { + code: number + signal: null + stdout: string + stderr: string + timedOut: boolean +} { + return { code: 0, signal: null, stdout, stderr: '', timedOut: false } +} + +type NativeRow = { + pid: number + ppid: number + name: string + memory?: number + commandLine?: string + creationTimeMs?: number +} + +/** A native snapshot must contain the reader's own pid or the table rejects. */ +function nativeTable(rows: NativeRow[]) { + return () => ({ + ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 }, + getAllProcesses: (callback: (processes: NativeRow[] | undefined) => void) => + callback([{ pid: process.pid, ppid: 0, name: 'vitest.exe' }, ...rows]) + }) +} + +function specs(): Spec[] { + return runProcessMock.mock.calls.map((call) => call[0] as Spec) +} describe('scanWindowsListeningPorts', () => { beforeEach(() => { - execFileAsyncMock.mockReset() + runProcessMock.mockReset() + resetWindowsPortScanDiagnosticsForTests() + resetWindowsProcessTableForTests() + __setWindowsProcessTreeLoaderForTests( + nativeTable([ + { pid: NETSTAT_PID, ppid: 4, name: 'node.exe' }, + { pid: SSHD_PID, ppid: 4, name: 'sshd.exe' } + ]) + ) }) - it('bounds the PowerShell scan with the caller abort signal and timeout', async () => { + afterEach(() => { + __setWindowsProcessTreeLoaderForTests() + __setWindowsProcessTableCimScanForTests() + resetWindowsProcessTableForTests() + }) + + it('reads netstat first and never starts PowerShell', async () => { const controller = new AbortController() - execFileAsyncMock.mockResolvedValueOnce({ - stdout: JSON.stringify({ + runProcessMock.mockResolvedValueOnce(ok(NETSTAT_STDOUT)) + + await expect(scanWindowsListeningPorts(controller.signal)).resolves.toEqual([ + { host: '::', port: 3000, pid: NETSTAT_PID, processName: 'node' }, + { host: '0.0.0.0', port: 3000, pid: NETSTAT_PID, processName: 'node' } + ]) + + expect(specs()).toHaveLength(1) + expect(specs()[0].program).toMatch(/netstat\.exe$/) + // `-p tcp` is absent on purpose: on Windows it means IPv4-only and would + // hide every `[::]` listener. + expect(specs()[0].args).toEqual(['-ano']) + expect(specs()[0].signal).toBe(controller.signal) + expect(specs()[0].timeoutMs).toBe(5000) + }) + + it('keeps the sshd and self-pid filters working off native process names', async () => { + runProcessMock.mockResolvedValueOnce( + ok( + [ + NETSTAT_STDOUT, + ` TCP 0.0.0.0:9999 0.0.0.0:0 LISTENING ${process.pid}` + ].join('\r\n') + ) + ) + + const ports = await scanWindowsListeningPorts() + + // sshd.exe is matched despite the table's `.exe` spelling, and the relay's + // own listener never reaches a client. + expect(ports.map((port) => `${port.host}:${port.port}`)).toEqual([':::3000', '0.0.0.0:3000']) + }) + + it('still reports host/port/pid when no process table is readable', async () => { + runProcessMock.mockResolvedValueOnce(ok(NETSTAT_STDOUT)) + __setWindowsProcessTreeLoaderForTests(() => null) + __setWindowsProcessTableCimScanForTests(() => + Promise.reject(new Error('windows process table unavailable')) + ) + resetWindowsProcessTableForTests() + + await expect(scanWindowsListeningPorts()).resolves.toEqual([ + { host: '0.0.0.0', port: 2222, pid: SSHD_PID }, + { host: '::', port: 3000, pid: NETSTAT_PID }, + { host: '0.0.0.0', port: 3000, pid: NETSTAT_PID } + ]) + // Names were unavailable, so nothing else was spawned to go get them. + expect(specs()).toHaveLength(1) + }) + + it('falls back to PowerShell without an execution-policy override', async () => { + const controller = new AbortController() + runProcessMock + .mockResolvedValueOnce({ + code: 1, + signal: null, + stdout: '', + stderr: 'blocked', + timedOut: false + }) + .mockResolvedValueOnce( + ok( + JSON.stringify({ + host: '127.0.0.1', + port: 5173, + pid: POWERSHELL_PID, + processName: 'node' + }) + ) + ) + + await expect(scanWindowsListeningPorts(controller.signal)).resolves.toEqual([ + { host: '127.0.0.1', port: 5173, pid: POWERSHELL_PID, processName: 'node' - }), - stderr: '' - }) - - await expect(scanWindowsListeningPorts(controller.signal)).resolves.toEqual([ - { host: '127.0.0.1', port: 5173, pid: POWERSHELL_PID, processName: 'node' } + } ]) - expect(execFileAsyncMock).toHaveBeenCalledWith( - 'powershell.exe', - expect.arrayContaining(['-EncodedCommand', expect.any(String)]), - expect.objectContaining({ - signal: controller.signal, - timeout: 5000, - windowsHide: true - }) - ) + const powershell = specs()[1] + expect(powershell.program).toMatch(/powershell\.exe$/i) + expect(powershell.args?.slice(0, 3)).toEqual(['-NoProfile', '-NonInteractive', '-Command']) + expect(powershell.args).not.toContain('-ExecutionPolicy') + expect(powershell.args).not.toContain('-EncodedCommand') + expect(powershell.args).toHaveLength(4) + expect(powershell.args?.[3]).toContain('Get-NetTCPConnection') + expect(powershell.signal).toBe(controller.signal) + expect(powershell.timeoutMs).toBe(5000) }) - it('bounds the netstat fallback with the same abort signal and timeout', async () => { - const controller = new AbortController() - execFileAsyncMock + it('tries pwsh when Windows PowerShell cannot answer, then gives up empty', async () => { + runProcessMock + .mockResolvedValueOnce(ok('')) .mockRejectedValueOnce(new Error('powershell unavailable')) .mockRejectedValueOnce(new Error('pwsh unavailable')) - .mockResolvedValueOnce({ - stdout: [ - ' Proto Local Address Foreign Address State PID', - ` TCP 0.0.0.0:3000 0.0.0.0:0 LISTENING ${NETSTAT_PID}` - ].join('\r\n'), - stderr: '' - }) - await expect(scanWindowsListeningPorts(controller.signal)).resolves.toEqual([ - { host: '0.0.0.0', port: 3000, pid: NETSTAT_PID } + await expect(scanWindowsListeningPorts()).resolves.toEqual([]) + + expect(specs().map((spec) => spec.program)).toEqual([ + expect.stringMatching(/netstat\.exe$/), + expect.stringMatching(/powershell\.exe$/i), + 'pwsh.exe' ]) - - expect(execFileAsyncMock).toHaveBeenLastCalledWith( - 'netstat.exe', - ['-ano', '-p', 'tcp'], - expect.objectContaining({ - signal: controller.signal, - timeout: 5000, - windowsHide: true - }) - ) }) - it('does not start the netstat fallback after the scan is cancelled', async () => { + it('gives up rather than falling back once the scan is cancelled', async () => { const controller = new AbortController() controller.abort() - execFileAsyncMock.mockRejectedValueOnce( - Object.assign(new Error('cancelled'), { name: 'AbortError' }) - ) + runProcessMock.mockResolvedValueOnce({ + code: null, + signal: null, + stdout: '', + stderr: '', + timedOut: false + }) await expect(scanWindowsListeningPorts(controller.signal)).resolves.toEqual([]) - expect(execFileAsyncMock).toHaveBeenCalledTimes(1) + expect(runProcessMock).toHaveBeenCalledTimes(1) + }) + + // `LISTENING` ships in netstat.exe.mui, picked by UI language, so no env can + // pin it. Without the shape-based re-read a German host parses zero rows, + // reads that as a blocked reader, and runs the flagged payload every 12-30s. + it('reads a localized host by socket shape rather than the state word', async () => { + runProcessMock.mockResolvedValueOnce( + ok( + [ + 'Aktive Verbindungen', + '', + ' Proto Lokale Adresse Remoteadresse Status PID', + ' TCP 0.0.0.0:135 0.0.0.0:0 ABHÖREN 1116', + ` TCP 0.0.0.0:3000 0.0.0.0:0 ABHÖREN ${NETSTAT_PID}`, + ` TCP [::]:3000 [::]:0 ABHÖREN ${NETSTAT_PID}`, + ` TCP 192.168.0.5:52000 93.184.216.34:443 HERGESTELLT ${NETSTAT_PID}` + ].join('\r\n') + ) + ) + + await expect(scanWindowsListeningPorts()).resolves.toEqual([ + { host: '0.0.0.0', port: 135, pid: 1116 }, + { host: '::', port: 3000, pid: NETSTAT_PID, processName: 'node' }, + { host: '0.0.0.0', port: 3000, pid: NETSTAT_PID, processName: 'node' } + ]) + expect(specs()).toHaveLength(1) + }) + + // The gap the state-word cases cannot cover: on a localized host BOUND is as + // unreadable as ABHÖREN, so shape alone would publish 8080 as a listener. + // Listeners dominate, and that is what separates them. + it('drops a BOUND socket that shape alone would promote on a localized host', async () => { + runProcessMock.mockResolvedValueOnce( + ok( + [ + ` TCP 0.0.0.0:3000 0.0.0.0:0 ABHÖREN ${NETSTAT_PID}`, + ` TCP [::]:3000 [::]:0 ABHÖREN ${NETSTAT_PID}`, + ` TCP 0.0.0.0:8080 0.0.0.0:0 GEBUNDEN ${NETSTAT_PID}`, + ` TCP 192.168.0.5:52000 93.184.216.34:443 HERGESTELLT ${NETSTAT_PID}` + ].join('\r\n') + ) + ) + + await expect(scanWindowsListeningPorts()).resolves.toEqual([ + { host: '::', port: 3000, pid: NETSTAT_PID, processName: 'node' }, + { host: '0.0.0.0', port: 3000, pid: NETSTAT_PID, processName: 'node' } + ]) + }) + + // Only on an exact tie does shape have nothing left to go on, and then it + // keeps both rather than guessing — no worse than reading shape alone. + it('keeps every tied zero-peer state when none dominates', async () => { + runProcessMock.mockResolvedValueOnce( + ok( + [ + ` TCP 0.0.0.0:3000 0.0.0.0:0 ABHÖREN ${NETSTAT_PID}`, + ` TCP 0.0.0.0:8080 0.0.0.0:0 GEBUNDEN ${NETSTAT_PID}` + ].join('\r\n') + ) + ) + + await expect(scanWindowsListeningPorts()).resolves.toEqual([ + { host: '0.0.0.0', port: 3000, pid: NETSTAT_PID, processName: 'node' }, + { host: '0.0.0.0', port: 8080, pid: NETSTAT_PID, processName: 'node' } + ]) + }) + + // Windows prints BOUND with a zero peer too, so the shape test must stay the + // fallback: a host with at least one readable LISTENING row never reaches it. + it('does not promote a BOUND socket on a host whose state word parsed', async () => { + runProcessMock.mockResolvedValueOnce( + ok( + [ + ` TCP 0.0.0.0:3000 0.0.0.0:0 LISTENING ${NETSTAT_PID}`, + ` TCP 0.0.0.0:8080 0.0.0.0:0 BOUND ${NETSTAT_PID}` + ].join('\r\n') + ) + ) + + await expect(scanWindowsListeningPorts()).resolves.toEqual([ + { host: '0.0.0.0', port: 3000, pid: NETSTAT_PID, processName: 'node' } + ]) + }) + + // A capped read exits 0 and its head parses, and netstat orders IPv4 TCP + // before IPv6 TCP, so publishing the head would drop every `[::]` listener. + it('refuses a netstat table that hit the capture cap', async () => { + const filler = Array.from( + { length: 60_000 }, + (_, index) => + ` TCP 10.0.0.1:${1000 + (index % 5000)} 10.0.0.2:443 TIME_WAIT 4` + ).join('\r\n') + runProcessMock + .mockResolvedValueOnce(ok(`${NETSTAT_STDOUT}\r\n${filler}`.slice(0, 4 * 1024 * 1024))) + .mockResolvedValueOnce(ok('[]')) + + await expect(scanWindowsListeningPorts()).resolves.toEqual([]) + + // Fell through instead of publishing the IPv4 head it could still parse. + expect(specs()).toHaveLength(2) + expect(specs()[1].args).toContain('-Command') + }) + + it('does not wait on the shared process table once the scan is cancelled', async () => { + const controller = new AbortController() + const getAllProcesses = vi.fn() + __setWindowsProcessTreeLoaderForTests(() => ({ + ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 }, + getAllProcesses + })) + resetWindowsProcessTableForTests() + // netstat answered, then the request was abandoned before names were needed. + runProcessMock.mockImplementationOnce(() => { + controller.abort() + return Promise.resolve(ok(NETSTAT_STDOUT)) + }) + + await expect(scanWindowsListeningPorts(controller.signal)).resolves.toEqual([ + // Unnamed, so the sshd row survives its own filter — the cost of not + // waiting, and strictly better than blocking an abandoned request. + { host: '0.0.0.0', port: 2222, pid: SSHD_PID }, + { host: '::', port: 3000, pid: NETSTAT_PID }, + { host: '0.0.0.0', port: 3000, pid: NETSTAT_PID } + ]) + expect(getAllProcesses).not.toHaveBeenCalled() + }) + + // The relay daemon's stderr is what installRelayLogRotation routes into + // relay.log, so a fall-through logged anywhere else is a fall-through nobody + // can diagnose. Pin the stream, not just the fact that something was called. + it('reports leaving the native path on the relay diagnostic stream, once', async () => { + const lines: string[] = [] + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation((chunk: string | Uint8Array) => { + lines.push(String(chunk)) + return true + }) + try { + runProcessMock.mockResolvedValue(ok('')) + await scanWindowsListeningPorts() + await scanWindowsListeningPorts() + // A second, different fault on the same host must still be heard: one + // flag for the whole module would have swallowed it. + runProcessMock.mockResolvedValue(ok('x'.repeat(4 * 1024 * 1024))) + await scanWindowsListeningPorts() + await scanWindowsListeningPorts() + } finally { + stderr.mockRestore() + } + + const reported = lines.filter((line) => line.includes('[ports] netstat unusable')) + expect(reported).toHaveLength(2) + expect(reported[0]).toContain('no listening row parsed') + expect(reported[1]).toContain('truncated') + // relayLogLine's ISO stamp: an unplaceable line cannot be read against the + // reconnect flaps around it. + expect(reported[0]).toMatch(/^\d{4}-\d{2}-\d{2}T[\d:.]+Z /) + }) + + it('treats a netstat timeout as unanswered and falls through', async () => { + runProcessMock + .mockResolvedValueOnce({ + code: null, + signal: 'SIGKILL', + stdout: '', + stderr: '', + timedOut: true + }) + .mockResolvedValueOnce(ok('[]')) + + await expect(scanWindowsListeningPorts()).resolves.toEqual([]) + + expect(specs()).toHaveLength(2) }) }) diff --git a/src/relay/windows-port-scan.ts b/src/relay/windows-port-scan.ts index f26a2bb172a..f99fce4a087 100644 --- a/src/relay/windows-port-scan.ts +++ b/src/relay/windows-port-scan.ts @@ -1,84 +1,231 @@ -import { execFile } from 'node:child_process' -import { promisify } from 'node:util' +import { readWindowsProcessTable } from '../main/windows/windows-process-table' +import { runProcess } from '../shared/child-process/run-process' +import { + windowsPowerShellPath, + windowsSystem32Binary +} from '../shared/child-process/windows-system-binary' import { getProcessOutputFields } from '../shared/process-output-field-scanner' -import { encodePowerShellCommand } from '../shared/powershell-command-encoding' import type { DetectedPort } from './port-scan-handler' import { buildRelayCommandEnv } from './relay-command-env' +import { relayLogLine } from './relay-diagnostic-log' const SYSTEM_PORTS_TO_EXCLUDE = new Set([22]) const MAX_DETECTED_PORTS = 50 const WINDOWS_PORT_SCAN_TIMEOUT_MS = 5_000 -const execFileAsync = promisify(execFile) +// Wide enough for `netstat -ano` on a busy host: it prints every connection, not +// just the listeners, and a truncated table silently drops the tail. +const WINDOWS_PORT_SCAN_MAX_OUTPUT_BYTES = 4 * 1024 * 1024 +/** + * Listening TCP ports, attributed to their owning process. + * + * `netstat.exe -ano` answers all of it except the process name, which comes + * from the shared process table -- so this scan starts no PowerShell of its + * own. One still runs on a released relay: without the optional + * `windows-process-tree.node` addon (built only by dev-channel-win-build.yml, + * so no release carries it) that table falls back to a CIM scan that forks one + * `powershell.exe`. That scan is TTL-shared with pane naming, so a relay with a + * live pane pays nothing extra for it. + * + * The EDR win is therefore the shape, not the absence of PowerShell. The + * retired payload ran `-ExecutionPolicy Bypass -EncodedCommand ` + * wrapping `Get-NetTCPConnection` joined to `Get-Process`: base64 beside a + * policy override is the highest-weighted token pair Defender for Endpoint + * scores on a PowerShell command line, and listing listeners with their owners + * reads as network discovery (T1049) on top of it. The shared CIM scan carries + * neither token. That payload survives only as the last resort below, without + * the override. + */ export async function scanWindowsListeningPorts(signal?: AbortSignal): Promise { + const netstatPorts = await readWindowsNetstatPorts(signal) + if (netstatPorts) { + return normalizeWindowsDetectedPorts(await attachWindowsProcessNames(netstatPorts, signal)) + } + if (signal?.aborted) { + return [] + } try { const json = await runWindowsPortScanPowerShell(signal) return normalizeWindowsDetectedPorts(parseWindowsPowerShellPortRows(json)) } catch { - if (signal?.aborted) { - return [] - } - try { - const { stdout } = await execFileAsync('netstat.exe', ['-ano', '-p', 'tcp'], { - env: buildRelayCommandEnv(), - encoding: 'utf-8', - signal, - timeout: WINDOWS_PORT_SCAN_TIMEOUT_MS, - windowsHide: true - }) - return normalizeWindowsDetectedPorts(parseWindowsNetstatOutput(stdout)) - } catch { - return [] - } + return [] } } -async function runWindowsPortScanPowerShell(signal?: AbortSignal): Promise { - const script = [ - "$ErrorActionPreference = 'Stop'", - '$connections = Get-NetTCPConnection -State Listen -ErrorAction Stop', - '$items = foreach ($connection in $connections) {', - ' $name = $null', - ' try {', - ' $process = Get-Process -Id $connection.OwningProcess -ErrorAction Stop', - ' $name = $process.ProcessName', - ' } catch {}', - ' [pscustomobject]@{', - ' host = [string]$connection.LocalAddress', - ' port = [int]$connection.LocalPort', - ' pid = [int]$connection.OwningProcess', - ' processName = $name', - ' }', - '}', - '$items | ConvertTo-Json -Compress -Depth 3' - ].join('\n') - const encoded = encodePowerShellCommand(script) - const lastError: unknown[] = [] +/** Rows, or null when netstat could not answer and the fallback should run. */ +async function readWindowsNetstatPorts(signal?: AbortSignal): Promise { + let stdout: string + try { + const result = await runProcess({ + program: windowsSystem32Binary('netstat.exe'), + // No `-p tcp`: on Windows that protocol name means TCP over IPv4 only, so + // it hides every `[::]` listener the retired PowerShell payload reported. + args: ['-ano'], + env: buildRelayCommandEnv(), + timeoutMs: WINDOWS_PORT_SCAN_TIMEOUT_MS, + maxOutputBytes: WINDOWS_PORT_SCAN_MAX_OUTPUT_BYTES, + signal + }) + if (result.timedOut || result.code !== 0) { + return null + } + // A capped read still exits 0 and its head still parses, so nothing + // downstream can tell a partial table from a whole one. netstat prints IPv4 + // TCP, then IPv6 TCP, then UDP, so the rows lost first are exactly the + // `[::]` listeners that dropping `-p tcp` above exists to keep. Refuse the + // whole read rather than publish its head. + if (Buffer.byteLength(result.stdout) >= WINDOWS_PORT_SCAN_MAX_OUTPUT_BYTES) { + reportWindowsNetstatUnusable('output hit the capture cap and was truncated') + return null + } + stdout = result.stdout + } catch { + return null + } + const ports = parseWindowsNetstatOutput(stdout) + // Windows always has a listener (RPC endpoint mapper, SMB), so an exit-0 scan + // that parses to nothing is a reader that was blocked, not an idle host. + if (ports.length === 0) { + reportWindowsNetstatUnusable('exited 0 but no listening row parsed') + return null + } + return ports +} - for (const binary of ['powershell.exe', 'pwsh.exe']) { - try { - const { stdout } = await execFileAsync( - binary, - ['-NoProfile', '-NonInteractive', '-ExecutionPolicy', 'Bypass', '-EncodedCommand', encoded], - { - env: buildRelayCommandEnv(), - encoding: 'utf-8', - maxBuffer: 1024 * 1024, - signal, - timeout: WINDOWS_PORT_SCAN_TIMEOUT_MS, - windowsHide: true - } +/** Reasons already reported. A fixed two-value vocabulary, so it cannot grow. */ +const reportedNetstatFailures = new Set() + +/** + * Say once why the scan left the native path. + * + * Both fall-throughs are permanent when they are wrong — the host stays on the + * PowerShell payload, or on nothing, for the life of the relay — and the scan + * repeats every 12-30s, so this logs one line rather than a stream. + * + * Through relayLogLine, not console.warn: this only ever runs in the detached + * daemon, whose stderr installRelayLogRotation routes into the relay.log that + * the remote-diagnostics tail reads. An untimestamped line in that file cannot + * be placed against the reconnect flaps around it (#7773), and "since when" is + * most of what this line is for. + */ +function reportWindowsNetstatUnusable(reason: string): void { + // Per reason, not per module: a host that parses nothing today and truncates + // tomorrow has two different faults, and one flag would hide the second. + if (reportedNetstatFailures.has(reason)) { + return + } + reportedNetstatFailures.add(reason) + relayLogLine(`[ports] netstat unusable on this host (${reason}); falling back to PowerShell`) +} + +/** Test-only: re-arm the one-shot so each case can observe its own line. */ +export function resetWindowsPortScanDiagnosticsForTests(): void { + reportedNetstatFailures.clear() +} + +/** + * Fill in owning-process names from the shared process-table snapshot. + * + * Names are optional data — the panel renders host/port/pid without them — so a + * host that cannot read the table keeps its rows. This shares whatever scan the + * table already runs rather than avoiding one, and on a relay that scan is a + * `powershell.exe` CIM query -- no released relay carries the native addon, so + * that is the path every SSH host takes, not a fallback. + * See docs/reference/windows-process-enumeration.md. + * + * Only `name` is read here, so this wants `readWindowsProcessIdentityTable` + * once #17866 lands -- on the detailed reader it would pay per-process handles + * for a field it discards. + * + * Best-effort by design: the snapshot is shared and TTL-cached, so it can + * predate netstat and hand a recycled PID its previous owner's name. Only + * labels read this field, and a fresh read would cost every caller a scan. + */ +async function attachWindowsProcessNames( + ports: DetectedPort[], + signal?: AbortSignal +): Promise { + const pids = new Set(ports.flatMap((port) => (port.pid == null ? [] : [port.pid]))) + // The shared snapshot takes no signal and must not be cancelled on one + // caller's behalf, so an abandoned scan declines to wait for it instead. + if (pids.size === 0 || signal?.aborted) { + return ports + } + let names: Map + try { + const rows = await readWindowsProcessTable() + names = new Map( + rows.flatMap((row) => + pids.has(row.pid) && row.name ? [[row.pid, stripExecutableSuffix(row.name)] as const] : [] ) - return stdout + ) + } catch { + return ports + } + return ports.map((port) => { + const processName = port.pid == null ? undefined : names.get(port.pid) + return processName ? { ...port, processName } : port + }) +} + +// The process table reports `sshd.exe`; the retired `Get-Process` payload +// reported `sshd`. The sshd filter below and every client that already renders +// these rows read the bare name, so keep publishing that spelling. +function stripExecutableSuffix(name: string): string { + return name.replace(/\.exe$/i, '') +} + +/** + * Single line so it survives as one argv element regardless of how the + * shell-less spawn hands it to PowerShell's `-Command` parser. Exported so + * windows-port-scan.win32.test.ts can run it: a missing `;` between statements + * is a parse error the mocked tests cannot see. + */ +export const WINDOWS_PORT_SCAN_SCRIPT = [ + "$ErrorActionPreference = 'Stop';", + 'Get-NetTCPConnection -State Listen | ForEach-Object {', + '$connection = $_; $name = $null;', + 'try { $name = (Get-Process -Id $connection.OwningProcess -ErrorAction Stop).ProcessName } catch { };', + '[pscustomobject]@{ host = [string]$connection.LocalAddress; port = [int]$connection.LocalPort;', + 'pid = [int]$connection.OwningProcess; processName = $name }', + '} | ConvertTo-Json -Compress -Depth 3' +].join(' ') + +async function runWindowsPortScanPowerShell(signal?: AbortSignal): Promise { + let lastError: unknown + + for (const program of [windowsPowerShellPath(), 'pwsh.exe']) { + try { + const result = await runProcess({ + program, + // No `-ExecutionPolicy` override: the policy gates script *files*, never + // `-Command`. Verified on Windows 11 — `-ExecutionPolicy Restricted + // -Command` still runs, while `-File` against an unsigned .ps1 does not. + args: ['-NoProfile', '-NonInteractive', '-Command', WINDOWS_PORT_SCAN_SCRIPT], + env: buildRelayCommandEnv(), + timeoutMs: WINDOWS_PORT_SCAN_TIMEOUT_MS, + maxOutputBytes: WINDOWS_PORT_SCAN_MAX_OUTPUT_BYTES, + signal + }) + if (signal?.aborted) { + throw new Error('windows port scan aborted') + } + if (result.timedOut || result.code !== 0) { + lastError ??= new Error( + `windows port scan PowerShell failed (code=${result.code} timedOut=${result.timedOut})` + ) + continue + } + return result.stdout } catch (error) { if (signal?.aborted) { throw error } - lastError.push(error) + lastError ??= error } } - throw lastError[0] ?? new Error('PowerShell unavailable') + throw lastError ?? new Error('PowerShell unavailable') } export function parseWindowsPowerShellPortRows(json: string): DetectedPort[] { @@ -98,26 +245,85 @@ export function parseWindowsPowerShellPortRows(json: string): DetectedPort[] { return rows.flatMap((row) => parseWindowsPortRow(row)) } +/** + * Listening rows, on a host in any UI language. + * + * `LISTENING` is not in `netstat.exe` — it lives in + * `System32\\netstat.exe.mui` beside `ESTABLISHED` and `Proto`, and MUI + * selection follows the UI language, so the pinned-locale env in + * relay-command-env.ts cannot reach it. A German host prints `ABHÖREN` and the + * word test finds nothing at all. + * + * The shape is language-independent: a listening socket has no peer, so its + * foreign address is `0.0.0.0:0` / `[::]:0`, and on every state Windows prints + * with a real peer that port is non-zero. Shape stays the fallback because the + * converse does not hold — `BOUND` and `CLOSED` print a zero peer too, and on a + * localized host their words are just as unreadable as the listening one. + */ export function parseWindowsNetstatOutput(output: string): DetectedPort[] { - const rows: DetectedPort[] = [] + const { rows, tcpRows } = scanWindowsNetstatTcpRows(output) + const byStateWord = rows.filter((row) => row.state === 'LISTENING') + if (byStateWord.length > 0 || tcpRows === 0) { + return byStateWord.map((row) => row.port) + } + return readDominantZeroPeerState(rows) +} + +/** + * Of the zero-peer states, keep only the one that dominates. + * + * Shape alone would publish a phantom listener: one `BOUND` socket among real + * listeners looks identical to them once the state word is unreadable. But it + * cannot dominate — listeners outnumber those transients by roughly 50:1 on a + * real host (51 against 0 here), so the largest zero-peer group is the + * listening one. An exact tie keeps every tied group rather than guessing, + * which is no worse than reading shape alone. + * + * A majority rule inverts if the majority is wrong: enough transient zero-peer + * sockets and the phantoms win, publishing those and dropping the real + * listeners. The hatch is to return [] here and defer to the PowerShell reader, + * which reads the state word instead of inferring it. + */ +function readDominantZeroPeerState(rows: NetstatTcpRow[]): DetectedPort[] { + const countByState = new Map() + for (const row of rows) { + if (row.zeroPeer) { + countByState.set(row.state, (countByState.get(row.state) ?? 0) + 1) + } + } + const largest = Math.max(0, ...countByState.values()) + const dominant = new Set( + [...countByState].filter(([, count]) => count === largest).map(([state]) => state) + ) + return rows.flatMap((row) => (row.zeroPeer && dominant.has(row.state) ? [row.port] : [])) +} + +type NetstatTcpRow = { state: string; zeroPeer: boolean; port: DetectedPort } + +/** `tcpRows` separates a localized host from one with genuinely no TCP output. */ +function scanWindowsNetstatTcpRows(output: string): { rows: NetstatTcpRow[]; tcpRows: number } { + const rows: NetstatTcpRow[] = [] + let tcpRows = 0 for (const line of output.split(/\r?\n/)) { const fields = getProcessOutputFields(line, 5) if (fields.length < 5 || fields[0].toUpperCase() !== 'TCP') { continue } - if (fields[3].toUpperCase() !== 'LISTENING') { - continue - } + tcpRows += 1 const hostPort = parseWindowsNetstatAddress(fields[1]) const pid = Number.parseInt(fields[4], 10) if (!hostPort || !Number.isSafeInteger(pid) || pid <= 0) { continue } - rows.push({ ...hostPort, pid }) + rows.push({ + state: fields[3].toUpperCase(), + zeroPeer: readWindowsNetstatPort(fields[2]) === 0, + port: { ...hostPort, pid } + }) } - return rows + return { rows, tcpRows } } function parseWindowsPortRow(row: unknown): DetectedPort[] { @@ -165,13 +371,20 @@ function readInteger(value: unknown): number | undefined { return Number.isSafeInteger(parsed) ? parsed : undefined } -function parseWindowsNetstatAddress(value: string): { host: string; port: number } | null { - const ipv6Match = /^\[(.*)\]:(\d+)$/.exec(value) - const portText = ipv6Match?.[2] ?? value.slice(value.lastIndexOf(':') + 1) +/** Port alone, keeping 0 — the foreign-address test above turns on that value. */ +function readWindowsNetstatPort(value: string): number | null { + const ipv6Match = /^\[.*\]:(\d+)$/.exec(value) + const portText = ipv6Match?.[1] ?? value.slice(value.lastIndexOf(':') + 1) const port = Number.parseInt(portText, 10) - if (!Number.isSafeInteger(port) || port <= 0) { + return Number.isSafeInteger(port) ? port : null +} + +function parseWindowsNetstatAddress(value: string): { host: string; port: number } | null { + const port = readWindowsNetstatPort(value) + if (port == null || port <= 0) { return null } + const ipv6Match = /^\[(.*)\]:\d+$/.exec(value) if (ipv6Match) { return { host: ipv6Match[1], port } } diff --git a/src/relay/windows-port-scan.win32.test.ts b/src/relay/windows-port-scan.win32.test.ts new file mode 100644 index 00000000000..976f477cd9c --- /dev/null +++ b/src/relay/windows-port-scan.win32.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest' +import { runProcess } from '../shared/child-process/run-process' +import { windowsPowerShellPath } from '../shared/child-process/windows-system-binary' +import { + WINDOWS_PORT_SCAN_SCRIPT, + parseWindowsPowerShellPortRows, + scanWindowsListeningPorts +} from './windows-port-scan' + +/** + * The mocked suite pins the argv; this pins that the argv works. + * + * Both halves are things a mock cannot see: netstat's real column layout (a + * `-p tcp` here silently drops every `[::]` listener), and whether the joined + * one-line PowerShell script even parses — a missing `;` between statements is + * a ParserError, and the fallback would then be dead on the day it is needed. + * + * Runs only on win32; skipped elsewhere. + */ +const describeOnWindows = process.platform === 'win32' ? describe : describe.skip + +describeOnWindows('windows port scan against the real host', () => { + it('finds listeners over both address families through netstat', async () => { + const ports = await scanWindowsListeningPorts() + + expect(ports.length).toBeGreaterThan(0) + for (const port of ports) { + expect(port.port).toBeGreaterThan(0) + expect(port.host.length).toBeGreaterThan(0) + } + // Windows binds RPC/SMB dual-stack, so both families must be represented. + expect(ports.some((port) => port.host.includes(':'))).toBe(true) + expect(ports.some((port) => !port.host.includes(':'))).toBe(true) + // Names come from the shared process table, never from a shell of our own. + expect(ports.some((port) => port.processName)).toBe(true) + // The table spells them `svchost.exe`; clients have always seen `svchost`. + expect(ports.every((port) => !port.processName?.endsWith('.exe'))).toBe(true) + }, 30_000) + + it('runs the de-escalated PowerShell fallback command line', async () => { + const result = await runProcess({ + program: windowsPowerShellPath(), + args: ['-NoProfile', '-NonInteractive', '-Command', WINDOWS_PORT_SCAN_SCRIPT], + timeoutMs: 20_000 + }) + + // No stderr assertion: an autoload or first-run banner writes there without + // the scan having failed. + expect(result.code).toBe(0) + expect(parseWindowsPowerShellPortRows(result.stdout).length).toBeGreaterThan(0) + }, 30_000) +}) diff --git a/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt b/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt index 929cc4f10ad..b2b82fdff69 100644 --- a/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt +++ b/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt @@ -176,7 +176,6 @@ src/relay/git-stdout-stream.ts src/relay/preflight-handler.ts src/relay/pty-shell-utils.ts src/relay/subprocess-tree-termination.ts -src/relay/windows-port-scan.ts src/relay/workspace-space-scan.ts src/shared/ephemeral-vm-recipe-process.ts src/shared/ephemeral-vm-recipe-runner.ts diff --git a/src/shared/child-process/__fixtures__/windows-console-visibility-allowlist.txt b/src/shared/child-process/__fixtures__/windows-console-visibility-allowlist.txt index a657002a8ff..068f9a8a96f 100644 --- a/src/shared/child-process/__fixtures__/windows-console-visibility-allowlist.txt +++ b/src/shared/child-process/__fixtures__/windows-console-visibility-allowlist.txt @@ -60,7 +60,6 @@ relay/fs-list-files-fallback-chain.ts relay/git-handler.ts relay/pty-shell-utils.ts relay/subprocess-tree-termination.ts -relay/windows-port-scan.ts relay/workspace-space-scan.ts shared/fish-binary-requirement.ts shared/process-table-snapshot-reader.ts diff --git a/src/shared/child-process/child-process-import-boundary.test.ts b/src/shared/child-process/child-process-import-boundary.test.ts index 32c6c9d890e..678bc53c4d0 100644 --- a/src/shared/child-process/child-process-import-boundary.test.ts +++ b/src/shared/child-process/child-process-import-boundary.test.ts @@ -29,7 +29,7 @@ const CHILD_PROCESS_IMPORT_ALLOWLIST: readonly string[] = readFileSync( * May only ever be DECREASED, and only by migrating a file off * `node:child_process`. Raising it is never the fix. */ -const DIRECT_IMPORTER_PIN = 158 +const DIRECT_IMPORTER_PIN = 157 const IMPORT_PATTERN = /(?:from\s+['"]node:child_process['"]|from\s+['"]child_process['"]|require\(\s*['"]node:child_process['"]|require\(\s*['"]child_process['"])/ diff --git a/src/shared/child-process/windows-console-visibility.test.ts b/src/shared/child-process/windows-console-visibility.test.ts index 596728fa245..f135fc10555 100644 --- a/src/shared/child-process/windows-console-visibility.test.ts +++ b/src/shared/child-process/windows-console-visibility.test.ts @@ -34,7 +34,7 @@ const ALLOWLIST: readonly string[] = readAllowlist( * the allowlist does not bound this: a swap (one file fixed and delisted, one * new file added with its entry) satisfies both membership assertions. */ -const UNHIDDEN_SPAWNER_PIN = 66 +const UNHIDDEN_SPAWNER_PIN = 65 const CHILD_PROCESS_IMPORT = /from\s+['"](?:node:)?child_process['"]|require\(\s*['"](?:node:)?child_process['"]/