diff --git a/src/main/ssh/ssh-port-scanner.test.ts b/src/main/ssh/ssh-port-scanner.test.ts index 09be7c1f686..f903d71acf0 100644 --- a/src/main/ssh/ssh-port-scanner.test.ts +++ b/src/main/ssh/ssh-port-scanner.test.ts @@ -235,6 +235,40 @@ describe('PortScanner', () => { expect(scanner.getDetectedPorts('t1')).toEqual([]) }) + it('aborts in-flight requests before stopping or replacing a target scan', async () => { + const harness = createVisibilityHarness(true) + const signals: AbortSignal[] = [] + const request = vi.fn( + (_method: string, _params?: Record, options?: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + const signal = options?.signal + if (!signal) { + reject(new Error('missing request signal')) + return + } + signals.push(signal) + signal.addEventListener('abort', () => reject(signal.reason), { once: true }) + }) + ) + const mux = { request } as unknown as SshChannelMultiplexer + const scanner = new PortScanner(harness.visibility) + + scanner.startScanning('t1', mux, vi.fn()) + expect(signals).toHaveLength(1) + expect(signals[0].aborted).toBe(false) + + scanner.startScanning('t1', mux, vi.fn()) + expect(signals).toHaveLength(2) + expect(signals[0].aborted).toBe(true) + expect(signals[1].aborted).toBe(false) + + scanner.stopScanning('t1') + expect(signals[1].aborted).toBe(true) + await vi.advanceTimersByTimeAsync(10 * BASE) + expect(request).toHaveBeenCalledTimes(2) + expect(harness.listenerCount()).toBe(0) + }) + it('keeps targets independent: stopping one host leaves the other scanning', async () => { const harness = createVisibilityHarness(true) let nextA = 3000 diff --git a/src/main/ssh/ssh-port-scanner.ts b/src/main/ssh/ssh-port-scanner.ts index 6404d035f22..2dbabff597a 100644 --- a/src/main/ssh/ssh-port-scanner.ts +++ b/src/main/ssh/ssh-port-scanner.ts @@ -19,6 +19,7 @@ export type PortScannerWindowVisibility = { type ScanHandle = { timer: ReturnType | null + requestAbortController: AbortController | null intervalMs: number // Why: while the window is hidden the scan chain is parked outright — no // timer wakeups, no remote requests — and the visibility listener resumes @@ -48,6 +49,7 @@ export class PortScanner { const handle: ScanHandle = { timer: null, + requestAbortController: null, intervalMs: SSH_PORT_SCAN_BASE_INTERVAL_MS, parkedWhileHidden: false, unsubscribeVisibility: () => {}, @@ -65,8 +67,12 @@ export class PortScanner { return } polling = true + const requestAbortController = new AbortController() + handle.requestAbortController = requestAbortController try { - const result = (await mux.request('ports.detect')) as { + const result = (await mux.request('ports.detect', undefined, { + signal: requestAbortController.signal + })) as { ports: DetectedPort[] platform: string } @@ -94,6 +100,9 @@ export class PortScanner { } catch { // Relay disconnected or request timed out — retry on next interval } finally { + if (handle.requestAbortController === requestAbortController) { + handle.requestAbortController = null + } polling = false } } @@ -142,6 +151,8 @@ export class PortScanner { if (handle.timer) { clearTimeout(handle.timer) } + handle.requestAbortController?.abort() + handle.requestAbortController = null handle.unsubscribeVisibility() this.handles.delete(targetId) } diff --git a/src/relay/port-scan-handler.test.ts b/src/relay/port-scan-handler.test.ts index 9df947b1aad..4ed3bce6ecd 100644 --- a/src/relay/port-scan-handler.test.ts +++ b/src/relay/port-scan-handler.test.ts @@ -1,7 +1,155 @@ -import { describe, expect, it, vi } from 'vitest' -import { parseHexAddress } from './port-scan-handler' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { MethodHandler, RequestContext } from './dispatcher' + +const { readFileMock, readdirMock, readlinkMock } = vi.hoisted(() => ({ + readFileMock: vi.fn(), + readdirMock: vi.fn(), + readlinkMock: vi.fn() +})) + +vi.mock('node:fs/promises', () => ({ + readFile: readFileMock, + readdir: readdirMock, + readlink: readlinkMock +})) + +import { parseHexAddress, PortScanHandler } from './port-scan-handler' import { parseWindowsNetstatOutput, parseWindowsPowerShellPortRows } from './windows-port-scan' +const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform') + +beforeEach(() => { + vi.clearAllMocks() + Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) +}) + +afterEach(() => { + if (originalPlatformDescriptor) { + Object.defineProperty(process, 'platform', originalPlatformDescriptor) + } +}) + +function capturePortDetectHandler(): MethodHandler { + let handler: MethodHandler | undefined + new PortScanHandler({ + onRequest: (method, nextHandler) => { + expect(method).toBe('ports.detect') + handler = nextHandler + } + }) + if (!handler) { + throw new Error('ports.detect handler was not registered') + } + return handler +} + +function requestContext(signal?: AbortSignal): RequestContext { + return { clientId: 1, isStale: () => signal?.aborted ?? false, signal } +} + +function createDeferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve = (_value: T): void => { + throw new Error('deferred promise was not initialized') + } + const promise = new Promise((nextResolve) => { + resolve = nextResolve + }) + return { promise, resolve } +} + +function mockLinuxProcScan({ + pidCount, + fdCount, + firstReadlink +}: { + pidCount: number + fdCount: number + firstReadlink?: Promise +}): void { + const tcpHeader = + 'sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode' + const tcpRow = + '0: 0100007F:0BB8 00000000:0000 0A 00000000:00000000 00:00000000 00000000 1000 0 11111' + readFileMock.mockImplementation(async (path: string) => { + if (path === '/proc/net/tcp') { + return `${tcpHeader}\n${tcpRow}\n` + } + if (path === '/proc/net/tcp6') { + return `${tcpHeader}\n` + } + if (path.endsWith('/cmdline')) { + return '/usr/bin/node\0server.js' + } + throw new Error(`unexpected readFile: ${path}`) + }) + + const pids = Array.from({ length: pidCount }, (_, index) => String(1_000 + index)) + const fds = Array.from({ length: fdCount }, (_, index) => String(index)) + readdirMock.mockImplementation(async (path: string) => { + if (path === '/proc') { + return pids + } + if (path.endsWith('/fd')) { + return fds + } + throw new Error(`unexpected readdir: ${path}`) + }) + + let first = true + readlinkMock.mockImplementation(() => { + if (first && firstReadlink) { + first = false + return firstReadlink + } + first = false + return Promise.resolve('socket:[11111]') + }) +} + +describe('PortScanHandler Linux cancellation', () => { + it('does not touch procfs for an already-cancelled request', async () => { + const controller = new AbortController() + controller.abort() + + await expect( + capturePortDetectHandler()({}, requestContext(controller.signal)) + ).rejects.toMatchObject({ name: 'AbortError' }) + + expect(readFileMock).not.toHaveBeenCalled() + expect(readdirMock).not.toHaveBeenCalled() + expect(readlinkMock).not.toHaveBeenCalled() + }) + + it('stops a large pid and fd walk at the filesystem operation already in flight', async () => { + const firstReadlink = createDeferred() + mockLinuxProcScan({ pidCount: 1_000, fdCount: 100, firstReadlink: firstReadlink.promise }) + const controller = new AbortController() + const scan = capturePortDetectHandler()({}, requestContext(controller.signal)) + + await vi.waitFor(() => expect(readlinkMock).toHaveBeenCalledTimes(1)) + controller.abort() + firstReadlink.resolve('socket:[11111]') + + await expect(scan).rejects.toMatchObject({ name: 'AbortError' }) + expect(readdirMock).toHaveBeenCalledTimes(2) + expect(readdirMock).toHaveBeenNthCalledWith(1, '/proc') + expect(readdirMock).toHaveBeenNthCalledWith(2, '/proc/1000/fd') + expect(readlinkMock).toHaveBeenCalledTimes(1) + expect(readlinkMock).toHaveBeenCalledWith('/proc/1000/fd/0') + }) + + it('preserves detected port results when the request stays live', async () => { + mockLinuxProcScan({ pidCount: 1, fdCount: 1 }) + + await expect( + capturePortDetectHandler()({}, requestContext(new AbortController().signal)) + ).resolves.toEqual({ + ports: [{ host: '127.0.0.1', port: 3000, pid: 1_000, processName: 'node' }], + platform: 'linux' + }) + }) +}) + describe('parseHexAddress', () => { it('parses IPv4 localhost (127.0.0.1)', () => { // 127.0.0.1 in little-endian hex: 0100007F diff --git a/src/relay/port-scan-handler.ts b/src/relay/port-scan-handler.ts index a576d8921fa..40150133a04 100644 --- a/src/relay/port-scan-handler.ts +++ b/src/relay/port-scan-handler.ts @@ -16,11 +16,11 @@ const SYSTEM_PORTS_TO_EXCLUDE = new Set([22]) const MAX_DETECTED_PORTS = 50 export class PortScanHandler { - constructor(dispatcher: RelayDispatcher) { + constructor(dispatcher: Pick) { dispatcher.onRequest('ports.detect', async (_params, context: RequestContext) => { if (process.platform === 'linux') { return { - ports: await this.scanLinuxListeningPorts(), + ports: await this.scanLinuxListeningPorts(context.signal), platform: process.platform } } @@ -37,11 +37,13 @@ export class PortScanHandler { }) } - private async scanLinuxListeningPorts(): Promise { + private async scanLinuxListeningPorts(signal?: AbortSignal): Promise { + signal?.throwIfAborted() const [tcp4, tcp6] = await Promise.all([ - this.readProcNet('/proc/net/tcp'), - this.readProcNet('/proc/net/tcp6') + this.readProcNet('/proc/net/tcp', signal), + this.readProcNet('/proc/net/tcp6', signal) ]) + signal?.throwIfAborted() const listeningSockets = [...tcp4, ...tcp6] if (listeningSockets.length === 0) { @@ -49,7 +51,7 @@ export class PortScanHandler { } const inodeSet = new Set(listeningSockets.map((s) => s.inode)) - const inodeToPid = await this.mapInodesToPids(inodeSet) + const inodeToPid = await this.mapInodesToPids(inodeSet, signal) const seen = new Set() const results: DetectedPort[] = [] @@ -57,6 +59,7 @@ export class PortScanHandler { const relayParentPid = process.ppid for (const socket of listeningSockets) { + signal?.throwIfAborted() const key = `${socket.host}:${socket.port}` if (seen.has(key)) { continue @@ -72,7 +75,7 @@ export class PortScanHandler { continue } - const processName = pid != null ? await this.getProcessName(pid) : undefined + const processName = pid != null ? await this.getProcessName(pid, signal) : undefined if (processName === 'sshd') { continue @@ -93,14 +96,18 @@ export class PortScanHandler { } private async readProcNet( - path: string + path: string, + signal?: AbortSignal ): Promise<{ port: number; host: string; inode: number }[]> { + signal?.throwIfAborted() let content: string try { content = await readFile(path, 'utf-8') } catch { + signal?.throwIfAborted() return [] } + signal?.throwIfAborted() const lines = content.split('\n') const results: { port: number; host: string; inode: number }[] = [] @@ -133,7 +140,11 @@ export class PortScanHandler { return results } - private async mapInodesToPids(inodes: Set): Promise> { + private async mapInodesToPids( + inodes: Set, + signal?: AbortSignal + ): Promise> { + signal?.throwIfAborted() const result = new Map() if (inodes.size === 0) { return result @@ -143,27 +154,35 @@ export class PortScanHandler { try { pids = (await readdir('/proc')).filter((name) => /^\d+$/.test(name)) } catch { + signal?.throwIfAborted() return result } + signal?.throwIfAborted() for (const pidStr of pids) { + signal?.throwIfAborted() const fdDir = `/proc/${pidStr}/fd` let fds: string[] try { fds = await readdir(fdDir) } catch { + signal?.throwIfAborted() continue } + signal?.throwIfAborted() const pid = Number.parseInt(pidStr, 10) for (const fd of fds) { + signal?.throwIfAborted() let link: string try { link = await readlink(`${fdDir}/${fd}`) } catch { + signal?.throwIfAborted() continue } + signal?.throwIfAborted() const match = link.match(/^socket:\[(\d+)\]$/) if (!match) { @@ -180,9 +199,11 @@ export class PortScanHandler { return result } - private async getProcessName(pid: number): Promise { + private async getProcessName(pid: number, signal?: AbortSignal): Promise { + signal?.throwIfAborted() try { const cmdline = await readFile(`/proc/${pid}/cmdline`, 'utf-8') + signal?.throwIfAborted() if (!cmdline) { return undefined } @@ -195,6 +216,7 @@ export class PortScanHandler { const parts = exe.split('/') return parts.at(-1) } catch { + signal?.throwIfAborted() return undefined } }