Merge remote-tracking branch 'origin/release/v3.8.50' into fix/qdrant-health-badge

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<number | null> {
+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<number | null> {
   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<number | null> {
     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<number | null> {
+  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;
+}

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<void>((resolve) => server.listen(29994, "127.0.0.1", resolve));
+  try {
+    assert.equal(await resolvePortPid(29994), process.pid);
+  } finally {
+    await new Promise<void>((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<void>((resolve) => server.listen(29992, "127.0.0.1", resolve));
+    assert.equal(await resolvePortPid(29992), process.pid);
+  } finally {
+    process.env.PATH = originalPath;
+    await new Promise<void>((resolve) => server.close(() => resolve()));
+    rmSync(shim, { recursive: true, force: true });
+  }
+});