From 403904ecd107ba6a5e01c196f3c1e052fd3c03a1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 21 Jan 2026 18:52:26 +0000 Subject: [PATCH] fix: harden port listener detection --- src/cli/daemon-cli/shared.ts | 32 ++++++++++++++++----------- src/cli/daemon-cli/status.print.ts | 27 +++++++++++++++++++++-- src/cli/ports.ts | 4 +++- src/infra/ports-inspect.test.ts | 35 ++++++++++++++++++++++++++++++ src/infra/ports-inspect.ts | 27 ++++++++++++++++++----- src/infra/ports-lsof.ts | 35 ++++++++++++++++++++++++++++++ 6 files changed, 139 insertions(+), 21 deletions(-) create mode 100644 src/infra/ports-inspect.test.ts create mode 100644 src/infra/ports-lsof.ts diff --git a/src/cli/daemon-cli/shared.ts b/src/cli/daemon-cli/shared.ts index 7be597ac5..1e0d82dce 100644 --- a/src/cli/daemon-cli/shared.ts +++ b/src/cli/daemon-cli/shared.ts @@ -50,22 +50,28 @@ export function pickProbeHostForBind( return "127.0.0.1"; } -export function safeDaemonEnv(env: Record | undefined): string[] { - if (!env) return []; - const allow = [ - "CLAWDBOT_PROFILE", - "CLAWDBOT_STATE_DIR", - "CLAWDBOT_CONFIG_PATH", - "CLAWDBOT_GATEWAY_PORT", - "CLAWDBOT_NIX_MODE", - ]; - const lines: string[] = []; - for (const key of allow) { +const SAFE_DAEMON_ENV_KEYS = [ + "CLAWDBOT_PROFILE", + "CLAWDBOT_STATE_DIR", + "CLAWDBOT_CONFIG_PATH", + "CLAWDBOT_GATEWAY_PORT", + "CLAWDBOT_NIX_MODE", +]; + +export function filterDaemonEnv(env: Record | undefined): Record { + if (!env) return {}; + const filtered: Record = {}; + for (const key of SAFE_DAEMON_ENV_KEYS) { const value = env[key]; if (!value?.trim()) continue; - lines.push(`${key}=${value.trim()}`); + filtered[key] = value.trim(); } - return lines; + return filtered; +} + +export function safeDaemonEnv(env: Record | undefined): string[] { + const filtered = filterDaemonEnv(env); + return Object.entries(filtered).map(([key, value]) => `${key}=${value}`); } export function normalizeListenerAddress(raw: string): string { diff --git a/src/cli/daemon-cli/status.print.ts b/src/cli/daemon-cli/status.print.ts index eb7f9f500..931dc3f10 100644 --- a/src/cli/daemon-cli/status.print.ts +++ b/src/cli/daemon-cli/status.print.ts @@ -14,16 +14,39 @@ import { getResolvedLoggerSettings } from "../../logging.js"; import { defaultRuntime } from "../../runtime.js"; import { colorize, isRich, theme } from "../../terminal/theme.js"; import { formatCliCommand } from "../command-format.js"; -import { formatRuntimeStatus, renderRuntimeHints, safeDaemonEnv } from "./shared.js"; +import { + filterDaemonEnv, + formatRuntimeStatus, + renderRuntimeHints, + safeDaemonEnv, +} from "./shared.js"; import { type DaemonStatus, renderPortDiagnosticsForCli, resolvePortListeningAddresses, } from "./status.gather.js"; +function sanitizeDaemonStatusForJson(status: DaemonStatus): DaemonStatus { + const command = status.service.command; + if (!command?.environment) return status; + const safeEnv = filterDaemonEnv(command.environment); + const nextCommand = { + ...command, + environment: Object.keys(safeEnv).length > 0 ? safeEnv : undefined, + }; + return { + ...status, + service: { + ...status.service, + command: nextCommand, + }, + }; +} + export function printDaemonStatus(status: DaemonStatus, opts: { json: boolean }) { if (opts.json) { - defaultRuntime.log(JSON.stringify(status, null, 2)); + const sanitized = sanitizeDaemonStatusForJson(status); + defaultRuntime.log(JSON.stringify(sanitized, null, 2)); return; } diff --git a/src/cli/ports.ts b/src/cli/ports.ts index afa4210af..5384c056b 100644 --- a/src/cli/ports.ts +++ b/src/cli/ports.ts @@ -1,4 +1,5 @@ import { execFileSync } from "node:child_process"; +import { resolveLsofCommandSync } from "../infra/ports-lsof.js"; export type PortProcess = { pid: number; command?: string }; @@ -30,7 +31,8 @@ export function parseLsofOutput(output: string): PortProcess[] { export function listPortListeners(port: number): PortProcess[] { try { - const out = execFileSync("lsof", ["-nP", `-iTCP:${port}`, "-sTCP:LISTEN", "-FpFc"], { + const lsof = resolveLsofCommandSync(); + const out = execFileSync(lsof, ["-nP", `-iTCP:${port}`, "-sTCP:LISTEN", "-FpFc"], { encoding: "utf-8", }); return parseLsofOutput(out); diff --git a/src/infra/ports-inspect.test.ts b/src/infra/ports-inspect.test.ts new file mode 100644 index 000000000..8aaeff424 --- /dev/null +++ b/src/infra/ports-inspect.test.ts @@ -0,0 +1,35 @@ +import net from "node:net"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const runCommandWithTimeoutMock = vi.fn(); + +vi.mock("../process/exec.js", () => ({ + runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args), +})); + +const describeUnix = process.platform === "win32" ? describe.skip : describe; + +describeUnix("inspectPortUsage", () => { + beforeEach(() => { + runCommandWithTimeoutMock.mockReset(); + }); + + it("reports busy when lsof is missing but loopback listener exists", async () => { + const server = net.createServer(); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const port = (server.address() as net.AddressInfo).port; + + runCommandWithTimeoutMock.mockRejectedValueOnce( + Object.assign(new Error("spawn lsof ENOENT"), { code: "ENOENT" }), + ); + + try { + const { inspectPortUsage } = await import("./ports-inspect.js"); + const result = await inspectPortUsage(port); + expect(result.status).toBe("busy"); + expect(result.errors?.some((err) => err.includes("ENOENT"))).toBe(true); + } finally { + server.close(); + } + }); +}); diff --git a/src/infra/ports-inspect.ts b/src/infra/ports-inspect.ts index 124c1255b..767480ced 100644 --- a/src/infra/ports-inspect.ts +++ b/src/infra/ports-inspect.ts @@ -1,5 +1,6 @@ import net from "node:net"; import { runCommandWithTimeout } from "../process/exec.js"; +import { resolveLsofCommand } from "./ports-lsof.js"; import { buildPortHints } from "./ports-format.js"; import type { PortListener, PortUsage, PortUsageStatus } from "./ports-types.js"; @@ -71,7 +72,8 @@ async function readUnixListeners( port: number, ): Promise<{ listeners: PortListener[]; detail?: string; errors: string[] }> { const errors: string[] = []; - const res = await runCommandSafe(["lsof", "-nP", `-iTCP:${port}`, "-sTCP:LISTEN", "-FpFcn"]); + const lsof = await resolveLsofCommand(); + const res = await runCommandSafe([lsof, "-nP", `-iTCP:${port}`, "-sTCP:LISTEN", "-FpFcn"]); if (res.code === 0) { const listeners = parseLsofFieldOutput(res.stdout); await Promise.all( @@ -87,11 +89,12 @@ async function readUnixListeners( ); return { listeners, detail: res.stdout.trim() || undefined, errors }; } - if (res.code === 1) { + const stderr = res.stderr.trim(); + if (res.code === 1 && !res.error && !stderr) { return { listeners: [], detail: undefined, errors }; } if (res.error) errors.push(res.error); - const detail = [res.stderr.trim(), res.stdout.trim()].filter(Boolean).join("\n"); + const detail = [stderr, res.stdout.trim()].filter(Boolean).join("\n"); if (detail) errors.push(detail); return { listeners: [], detail: undefined, errors }; } @@ -175,7 +178,7 @@ async function readWindowsListeners( return { listeners, detail: res.stdout.trim() || undefined, errors }; } -async function checkPortInUse(port: number): Promise { +async function tryListenOnHost(port: number, host: string): Promise { try { await new Promise((resolve, reject) => { const tester = net @@ -184,15 +187,29 @@ async function checkPortInUse(port: number): Promise { .once("listening", () => { tester.close(() => resolve()); }) - .listen(port); + .listen({ port, host, exclusive: true }); }); return "free"; } catch (err) { if (isErrno(err) && err.code === "EADDRINUSE") return "busy"; + if (isErrno(err) && (err.code === "EADDRNOTAVAIL" || err.code === "EAFNOSUPPORT")) { + return "skip"; + } return "unknown"; } } +async function checkPortInUse(port: number): Promise { + const hosts = ["127.0.0.1", "0.0.0.0", "::1", "::"]; + let sawUnknown = false; + for (const host of hosts) { + const result = await tryListenOnHost(port, host); + if (result === "busy") return "busy"; + if (result === "unknown") sawUnknown = true; + } + return sawUnknown ? "unknown" : "free"; +} + export async function inspectPortUsage(port: number): Promise { const errors: string[] = []; const result = diff --git a/src/infra/ports-lsof.ts b/src/infra/ports-lsof.ts new file mode 100644 index 000000000..4b5f01a6a --- /dev/null +++ b/src/infra/ports-lsof.ts @@ -0,0 +1,35 @@ +import fs from "node:fs"; +import fsPromises from "node:fs/promises"; + +const LSOF_CANDIDATES = + process.platform === "darwin" + ? ["/usr/sbin/lsof", "/usr/bin/lsof"] + : ["/usr/bin/lsof", "/usr/sbin/lsof"]; + +async function canExecute(path: string): Promise { + try { + await fsPromises.access(path, fs.constants.X_OK); + return true; + } catch { + return false; + } +} + +export async function resolveLsofCommand(): Promise { + for (const candidate of LSOF_CANDIDATES) { + if (await canExecute(candidate)) return candidate; + } + return "lsof"; +} + +export function resolveLsofCommandSync(): string { + for (const candidate of LSOF_CANDIDATES) { + try { + fs.accessSync(candidate, fs.constants.X_OK); + return candidate; + } catch { + // keep trying + } + } + return "lsof"; +}