diff --git a/bin/cli/commands/serve.mjs b/bin/cli/commands/serve.mjs index 8be09c1a808f..e6704cf66ab7 100644 --- a/bin/cli/commands/serve.mjs +++ b/bin/cli/commands/serve.mjs @@ -246,7 +246,8 @@ export async function runServe(opts = {}) { // BEFORE any pid file is written or any child is spawned. Otherwise the // doomed child's EADDRINUSE arrives only after this process has rewritten // the pid files of the healthy instance that actually owns the port. - const busyPids = await resolveServeBusyPids(dashboardPort); + const serverHost = resolveServerHost(); + const busyPids = await resolveServeBusyPids(dashboardPort, { host: serverHost }); if (busyPids.length > 0) { reportPortInUse(dashboardPort, busyPids); process.exit(1); @@ -276,7 +277,7 @@ export async function runServe(opts = {}) { // #10492: HOSTNAME is standard shell state on Unix-like systems, not an // OmniRoute bind setting. The resolver only keeps its legacy meaning on // Windows; OMNIROUTE_SERVER_HOST is the cross-platform explicit setting. - HOSTNAME: resolveServerHost(), + HOSTNAME: serverHost, NODE_ENV: "production", // #5238: preserve a user-set NODE_OPTIONS (incl. their own // `--max-old-space-size=…`) instead of clobbering it with the calibrated @@ -347,15 +348,15 @@ export async function resolveServeBusyPids(port, deps = {}) { // findListeningPids() returning null means the discovery tool itself is // missing or unusable (Termux, slim containers, #14518) — fall back to a // bind probe so the guard still answers before spawning the doomed child. - let busyPids = await discover(port); + let busyPids = await discover(port, { host: deps.host }); if (busyPids === null) { // Discovery tool missing/unusable (#14518): the bind probe is the guard. - if (!(await probe(port))) busyPids = [null]; + if (!(await probe(port, { host: deps.host }))) busyPids = [null]; else busyPids = []; } else if (busyPids.length === 0) { // Discovery ran and saw nothing, but that window can race a starting // instance; a bind probe costs nothing and doubles as confirmation. - if (!(await probe(port))) busyPids = [null]; + if (!(await probe(port, { host: deps.host }))) busyPids = [null]; } return busyPids; } diff --git a/bin/cli/utils/pid.mjs b/bin/cli/utils/pid.mjs index 36dc8019ff61..3c6792460b21 100644 --- a/bin/cli/utils/pid.mjs +++ b/bin/cli/utils/pid.mjs @@ -86,20 +86,12 @@ export async function findListeningPids(port, deps = {}) { const { stdout } = await exec("netstat", ["-ano"]); return parseNetstatListeningPids(stdout, port); } - const { stdout } = await exec("lsof", ["-nP", "-t", `-iTCP:${port}`, "-sTCP:LISTEN"]); - return stdout - .trim() - .split("\n") - .map((entry) => parseInt(entry, 10)) - .filter((entry) => Number.isFinite(entry) && entry > 0); + const { stdout } = await exec("lsof", ["-nP", `-iTCP:${port}`, "-sTCP:LISTEN"]); + return parseLsofListeningPids(stdout, port, deps.host); } catch (err) { // POSIX lsof exits 1 with empty output when there are simply no matches. // That is the normal "port is free" result, not a discovery failure. - if ( - platform !== "win32" && - err?.code === 1 && - !String(err?.stdout ?? "").trim() - ) { + if (platform !== "win32" && err?.code === 1 && !String(err?.stdout ?? "").trim()) { return []; } // Tool missing (ENOENT) or genuinely unusable: "no listener" cannot be @@ -132,12 +124,62 @@ export async function probePortFree(port, deps = {}) { // port, so a server on 0.0.0.0 (the default), 127.0.0.1 or ::1 (localhost) is // only visible to a probe on that same address. A host without one of these // addresses gets EADDRNOTAVAIL, which reads as free. - for (const host of [undefined, "0.0.0.0", "127.0.0.1", "::1"]) { + const bindHost = deps.host; + const hosts = + bindHost && bindHost !== "0.0.0.0" && bindHost !== "::" && bindHost !== "*" + ? [bindHost] + : [undefined, "0.0.0.0", "127.0.0.1", "::1"]; + for (const host of hosts) { if (!(await bindable(host))) return false; } return true; } +function parseLsofListeningPids(stdout, port, host) { + const pids = []; + for (const line of String(stdout || "").split(/\r?\n/)) { + const trimmed = line.trim(); + if (!trimmed || trimmed.startsWith("COMMAND")) continue; + const numeric = parseInt(trimmed, 10); + if (String(numeric) === trimmed && numeric > 0) { + pids.push(numeric); + continue; + } + const cols = trimmed.split(/\s+/); + const pid = parseInt(cols[1], 10); + if (!Number.isFinite(pid) || pid <= 0) continue; + const name = + cols.find((col) => { + const at = col.lastIndexOf(":"); + return at > 0 && col.slice(at + 1) === String(port); + }) || ""; + if (!name) continue; + const at = name.lastIndexOf(":"); + const listenHost = name.slice(0, at); + if (!listenerBlocksBind(listenHost, host)) continue; + pids.push(pid); + } + return pids; +} + +function listenerBlocksBind(listenHost, bindHost) { + if (!bindHost || bindHost === "0.0.0.0" || bindHost === "::" || bindHost === "*") return true; + const listen = normalizeListenHost(listenHost); + const bind = normalizeListenHost(bindHost); + if (!listen) return true; + if (listen === "*" || listen === "0.0.0.0" || listen === "::") return true; + if (bind === "127.0.0.1" || bind === "::1" || bind === "localhost") { + return listen === "127.0.0.1" || listen === "::1" || listen === "localhost"; + } + return listen === bind; +} + +function normalizeListenHost(host) { + const value = String(host || "").trim(); + if (value.startsWith("[") && value.endsWith("]")) return value.slice(1, -1); + return value; +} + function parseNetstatListeningPids(stdout, port) { const portCol = `:${port}`; const pids = []; diff --git a/changelog.d/fixes/15472-serve-port-guard-bind-address.md b/changelog.d/fixes/15472-serve-port-guard-bind-address.md new file mode 100644 index 000000000000..39f9d5a5a810 --- /dev/null +++ b/changelog.d/fixes/15472-serve-port-guard-bind-address.md @@ -0,0 +1 @@ +- **fix(cli):** the serve port guard ignores a listener on another address when the server binds a specific host ([#15472](https://github.com/diegosouzapw/OmniRoute/pull/15472) — thanks @HouMinXi) diff --git a/tests/unit/cli-serve-port-in-use-preflight.test.mjs b/tests/unit/cli-serve-port-in-use-preflight.test.mjs index c46ba9cbc8d3..72d4db2238ff 100644 --- a/tests/unit/cli-serve-port-in-use-preflight.test.mjs +++ b/tests/unit/cli-serve-port-in-use-preflight.test.mjs @@ -66,6 +66,52 @@ test("findListeningPids ignores TIME_WAIT and other ports", async () => { assert.deepEqual(pids, [4]); }); +test("findListeningPids asks lsof for LISTEN sockets with the listen address", async () => { + const calls = []; + await findListeningPids(20128, { + platform: "linux", + execFileAsync: async (_cmd, args) => { + calls.push(args); + return { stdout: "" }; + }, + }); + assert.deepEqual( + calls[0], + ["-nP", "-iTCP:20128", "-sTCP:LISTEN"], + "must keep the listen address so a listener on another address is not a conflict" + ); +}); + +test("findListeningPids ignores a listener on another address when binding loopback (#15424)", async () => { + const lsof = "socat 9001 houminxi 6u IPv4 0x1 0t0 TCP 192.168.1.50:20128 (LISTEN)\n"; + const pids = await findListeningPids(20128, { + platform: "linux", + host: "127.0.0.1", + execFileAsync: async () => ({ stdout: lsof }), + }); + assert.deepEqual(pids, [], "a LAN-only forwarder does not block a loopback bind"); +}); + +test("findListeningPids still reports a wildcard listener when binding loopback (#15424)", async () => { + const lsof = "node 19348 houminxi 23u IPv4 0x1 0t0 TCP *:20128 (LISTEN)\n"; + const pids = await findListeningPids(20128, { + platform: "linux", + host: "127.0.0.1", + execFileAsync: async () => ({ stdout: lsof }), + }); + assert.deepEqual(pids, [19348], "0.0.0.0 / * holds every address, including loopback"); +}); + +test("findListeningPids does not treat a longer port as the target port", async () => { + const lsof = "node 1443 houminxi 23u IPv4 0x1 0t0 TCP 127.0.0.1:1443 (LISTEN)\n"; + const pids = await findListeningPids(443, { + platform: "linux", + host: "127.0.0.1", + execFileAsync: async () => ({ stdout: lsof }), + }); + assert.deepEqual(pids, [], "1443 must not match a search for 443"); +}); + test("findListeningPids reports the PID holding the port (posix lsof)", async () => { const pids = await findListeningPids(20128, { platform: "linux", @@ -81,24 +127,21 @@ test("findListeningPids reports the PID holding the port (posix lsof)", async () // "Port 20128 is already in use by PID " while `ss -ltn` showed the // port free — a gateway that could not restart because something else had once // connected to it. Same defect class already fixed in stop.mjs::killByPortPosix. -test( - "findListeningPids asks lsof for LISTEN sockets only, not every client on the port", - async () => { - const calls = []; - await findListeningPids(20128, { - platform: "linux", - execFileAsync: async (_cmd, args) => { - calls.push(args); - return { stdout: "" }; - }, - }); - assert.deepEqual( - calls[0], - ["-nP", "-t", "-iTCP:20128", "-sTCP:LISTEN"], - "must scope discovery to TCP listeners so client sockets cannot fake a conflict" - ); - } -); +test("findListeningPids asks lsof for LISTEN sockets only, not every client on the port", async () => { + const calls = []; + await findListeningPids(20128, { + platform: "linux", + execFileAsync: async (_cmd, args) => { + calls.push(args); + return { stdout: "" }; + }, + }); + assert.deepEqual( + calls[0], + ["-nP", "-iTCP:20128", "-sTCP:LISTEN"], + "must scope discovery to TCP listeners and keep the address" + ); +}); test("findListeningPids treats an empty lsof result as a free port", async () => { const noMatch = Object.assign(new Error("lsof exited with no matches"), {