diff --git a/src/lib/services/portProbe.ts b/src/lib/services/portProbe.ts index 3f0deaaf07..81c3d8a846 100644 --- a/src/lib/services/portProbe.ts +++ b/src/lib/services/portProbe.ts @@ -103,19 +103,86 @@ export async function probeBeforeSpawn(healthUrl: string, port: number): Promise return { healthy, portInUse }; } +/** `lsof -ti :PORT` prints one pid per line and nothing else. */ +export function parseLsofPid(stdout: string): number | null { + const firstLine = stdout + .split("\n") + .map((line) => line.trim()) + .find((line) => line.length > 0); + const parsed = firstLine ? Number.parseInt(firstLine, 10) : Number.NaN; + return Number.isFinite(parsed) ? parsed : null; +} + /** - * Resolve the pid of whatever process is listening on `port`, if any. + * `ss -tlnp 'sport = :PORT'` carries the pid inside the process column: * - * Used when adopting an already-healthy instance (see `decidePreSpawn`'s - * "adopt" outcome): the supervisor didn't spawn that process itself, so it - * has no pid from a `ChildProcess` handle, but tracking a real pid is still - * needed for downstream liveness checks to trust an adopted service the same - * way they trust a freshly-spawned one. Returns null if nothing is found or - * the lookup fails/times out (best-effort; never blocks adoption on this). + * LISTEN 0 511 127.0.0.1:20128 0.0.0.0:* users:(("node",pid=596922,fd=18)) + * + * The filter is applied by `ss` itself, so any `pid=` on any line belongs to + * the requested port. */ -export async function resolvePortPid(port: number): Promise { +export function parseSsPid(stdout: string): number | null { + const match = /\bpid=(\d+)/.exec(stdout); + const parsed = match ? Number.parseInt(match[1], 10) : Number.NaN; + return Number.isFinite(parsed) ? parsed : null; +} + +/** + * `netstat -tlnp` cannot filter by port, so the port is matched here: + * + * tcp 0 0 127.0.0.1:20128 0.0.0.0:* LISTEN 596922/node + * + * Matching on the local-address column keeps a foreign address that happens to + * end in the same number from being read as a listener. + */ +export function parseNetstatPid(stdout: string, port: number): number | null { + for (const line of stdout.split("\n")) { + const columns = line.trim().split(/\s+/); + // proto recv-q send-q local-address foreign-address state pid/program + if (columns.length < 7 || columns[5] !== "LISTEN") continue; + if (!columns[3].endsWith(`:${port}`)) continue; + const parsed = Number.parseInt(columns[6], 10); + if (Number.isFinite(parsed)) return parsed; + } + return null; +} + +/** + * Ways to ask the OS which process holds a port, in preference order. + * + * `lsof` stays first because it is the most direct, but it is absent from slim + * container images, and a missing binary is indistinguishable from a free port + * once `spawn` has turned ENOENT into a null. `ss` ships with iproute2 and + * `netstat` with net-tools, so between the three there is normally something + * to ask on any host the supervisor runs on. + */ +const PID_PROBES: ReadonlyArray<{ + command: string; + args: (port: number) => string[]; + parse: (stdout: string, port: number) => number | null; +}> = [ + { command: "lsof", args: (port) => ["-ti", `:${port}`], parse: (stdout) => parseLsofPid(stdout) }, + { + command: "ss", + args: (port) => ["-tlnp", `sport = :${port}`], + parse: (stdout) => parseSsPid(stdout), + }, + { command: "netstat", args: () => ["-tlnp"], parse: parseNetstatPid }, +]; + +/** Run one probe, resolving null on a missing binary, a non-match or a timeout. */ +function runPidProbe( + probe: (typeof PID_PROBES)[number], + port: number, + timeoutMs: number +): Promise { return new Promise((resolve) => { - const proc = spawn("lsof", ["-ti", `:${port}`]); + if (timeoutMs <= 0) { + resolve(null); + return; + } + + const proc = spawn(probe.command, probe.args(port)); let output = ""; let settled = false; @@ -129,19 +196,39 @@ export async function resolvePortPid(port: number): Promise { const timeout = setTimeout(() => { proc.kill(); finish(null); - }, PID_RESOLVE_TIMEOUT_MS); + }, timeoutMs); proc.stdout?.on("data", (chunk: Buffer) => { output += chunk.toString("utf8"); }); + // ENOENT when the binary is not installed — fall through to the next probe. proc.on("error", () => finish(null)); - proc.on("close", () => { - const firstLine = output - .split("\n") - .map((line) => line.trim()) - .find((line) => line.length > 0); - const parsed = firstLine ? Number.parseInt(firstLine, 10) : Number.NaN; - finish(Number.isFinite(parsed) ? parsed : null); - }); + proc.on("close", () => finish(probe.parse(output, port))); }); } + +/** + * Resolve the pid of whatever process is listening on `port`, if any. + * + * Used when adopting an already-healthy instance (see `decidePreSpawn`'s + * "adopt" outcome): the supervisor didn't spawn that process itself, so it + * has no pid from a `ChildProcess` handle, but tracking a real pid is still + * needed for downstream liveness checks to trust an adopted service the same + * way they trust a freshly-spawned one. Returns null if nothing is found or + * the lookup fails/times out (best-effort; never blocks adoption on this). + * + * Tries `lsof`, then `ss`, then `netstat`, so a host missing any one of them + * still reports a real pid instead of a silent null (#10431). The probes share + * one deadline, so the whole lookup still costs at most + * `PID_RESOLVE_TIMEOUT_MS`. + */ +export async function resolvePortPid(port: number): Promise { + const deadline = Date.now() + PID_RESOLVE_TIMEOUT_MS; + + for (const probe of PID_PROBES) { + const pid = await runPidProbe(probe, port, deadline - Date.now()); + if (pid !== null) return pid; + } + + return null; +} diff --git a/tests/unit/services/portProbePid.test.ts b/tests/unit/services/portProbePid.test.ts new file mode 100644 index 0000000000..b88383fd80 --- /dev/null +++ b/tests/unit/services/portProbePid.test.ts @@ -0,0 +1,129 @@ +/** + * `resolvePortPid` output parsing and probe fallback (#10431). + * + * The regression these guard is that `resolvePortPid` used to shell out to + * `lsof` alone. On a host without it, `spawn` raises ENOENT, the handler turned + * that into `null`, and an adopted service silently kept `pid: null` forever. + */ + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { createServer } from "node:net"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, rmSync, symlinkSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { + parseLsofPid, + parseNetstatPid, + parseSsPid, + resolvePortPid, +} from "@/lib/services/portProbe"; + +/** Absolute path of `command`, or null when it is not on PATH. */ +function which(command: string): string | null { + try { + return execFileSync("/bin/sh", ["-c", `command -v ${command}`], { encoding: "utf8" }).trim(); + } catch { + return null; + } +} + +test("parseLsofPid reads the first pid line", () => { + assert.equal(parseLsofPid("596922\n"), 596922); + assert.equal(parseLsofPid("\n 596922 \n123\n"), 596922); +}); + +test("parseLsofPid returns null for empty or non-numeric output", () => { + assert.equal(parseLsofPid(""), null); + assert.equal(parseLsofPid("\n \n"), null); + assert.equal(parseLsofPid("lsof: command not found\n"), null); +}); + +test("parseSsPid reads the pid out of the users:(...) column", () => { + const line = + 'LISTEN 0 511 127.0.0.1:20128 0.0.0.0:* users:(("node",pid=596922,fd=18))\n'; + assert.equal(parseSsPid(line), 596922); +}); + +test("parseSsPid returns null when ss reports no process column", () => { + // Without ownership of the socket (or CAP_NET_ADMIN) ss prints the row but + // no users:(...) column, which must not be read as a match. + assert.equal(parseSsPid("LISTEN 0 511 127.0.0.1:20128 0.0.0.0:*\n"), null); + assert.equal(parseSsPid(""), null); +}); + +test("parseNetstatPid matches on the local address, not the foreign one", () => { + const stdout = [ + "Active Internet connections (only servers)", + "Proto Recv-Q Send-Q Local Address Foreign Address State PID/Program name", + "tcp 0 0 127.0.0.1:9999 0.0.0.0:20128 LISTEN 111/other", + "tcp 0 0 127.0.0.1:20128 0.0.0.0:* LISTEN 596922/node", + "", + ].join("\n"); + assert.equal(parseNetstatPid(stdout, 20128), 596922); +}); + +test("parseNetstatPid ignores non-listening rows and unknown ports", () => { + const stdout = + "tcp 0 0 127.0.0.1:20128 1.2.3.4:5555 ESTABLISHED 596922/node\n"; + assert.equal(parseNetstatPid(stdout, 20128), null); + assert.equal(parseNetstatPid("", 20128), null); +}); + +test("resolvePortPid finds the pid holding a port", async () => { + const server = createServer(); + await new Promise((resolve) => server.listen(29994, "127.0.0.1", resolve)); + try { + assert.equal(await resolvePortPid(29994), process.pid); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } +}); + +test("resolvePortPid returns null for a port nobody holds", async () => { + assert.equal(await resolvePortPid(29993), null); +}); + +test("resolvePortPid still resolves a pid on a host without lsof", async (t) => { + // The reported environment: ss and/or netstat present, lsof absent. Emulated + // by pointing PATH at a directory holding only the fallbacks, so `spawn` + // raises the same ENOENT for lsof that a slim image would. + const fallbacks = ["ss", "netstat"] + .map((command) => ({ command, real: which(command) })) + .filter((entry): entry is { command: string; real: string } => entry.real !== null); + + if (fallbacks.length === 0) { + t.skip("neither ss nor netstat is installed"); + return; + } + + const shim = mkdtempSync(path.join(tmpdir(), "portprobe-")); + const originalPath = process.env.PATH; + const server = createServer(); + + try { + for (const { command, real } of fallbacks) { + symlinkSync(real, path.join(shim, command)); + } + + // Guard the guard: if lsof were still reachable the assertion below would + // pass for the wrong reason. + assert.throws( + () => + execFileSync("/bin/sh", ["-c", "command -v lsof"], { + stdio: "ignore", + env: { PATH: shim }, + }), + "lsof must not resolve on the shimmed PATH" + ); + + process.env.PATH = shim; + await new Promise((resolve) => server.listen(29992, "127.0.0.1", resolve)); + assert.equal(await resolvePortPid(29992), process.pid); + } finally { + process.env.PATH = originalPath; + await new Promise((resolve) => server.close(() => resolve())); + rmSync(shim, { recursive: true, force: true }); + } +});