From b03e52b7ba2dbe743163beb97366c505d3a50d18 Mon Sep 17 00:00:00 2001 From: ework-agent Date: Thu, 3 Sep 2026 10:05:05 +0800 Subject: [PATCH] fix(launcher): child-reported bind via handshake file kills port TOCTOU; stop forcing --debug Parent-side findFreePort probed the preferred port, released it, then let the child re-bind (TOCTOU, #401): another bili grabbing it made probeHealth pass and silently attached client+MCP to a foreign instance; any other grabber burned the full 20s health timeout. No retry either way. - launcher: drop findFreePort; each launch passes a per-launch --handshake-file (BILI_HANDSHAKE_FILE). The child publishes {origin, pid} atomically after binding, so the origin is ours by construction; parent cross-checks pid before attaching and fails fast (<5s) with the child log path if the child exits before reporting/bind/health. - server: in handshake mode, EADDRINUSE on the preferred port falls back to an OS-assigned port (atomic bind, zero probe window) with a clear warn. Plain bili start conflict behavior (hint + exit 1) is unchanged. - launcher: no longer force debug:true on spawned proxies; the child inherits its log level exactly like a plain bili start (--debug flag / ACP_DEBUG env / config file). Per-launch /tmp/bili-proxy-.log kept. --- src/cli.ts | 4 +- src/config.ts | 5 + src/launcher.ts | 123 ++++++++++++----- src/server.ts | 51 +++++-- tests/handshake-fallback.test.ts | 79 +++++++++++ tests/launcher.test.ts | 226 ++++++++++++++++++++++--------- 6 files changed, 377 insertions(+), 111 deletions(-) create mode 100644 tests/handshake-fallback.test.ts diff --git a/src/cli.ts b/src/cli.ts index a0369f02d..3d9894b78 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -206,7 +206,8 @@ export function parseArgs(argv: string[]): Parsed { case "--config": case "--origin": case "--agent": - case "--bin": { + case "--bin": + case "--handshake-file": { const val = argv[++i]; if (val === undefined || val.length === 0) { console.error(`bili: ${a} requires a non-empty value`); @@ -218,6 +219,7 @@ export function parseArgs(argv: string[]): Parsed { else if (a === "--origin") overrides.BILI_MCP_PROXY = val; else if (a === "--bin") process.env.BILI_CLIENT_BIN = val; else if (a === "-F") overrides.BILI_UPSTREAM_PROXY = val; + else if (a === "--handshake-file") overrides.BILI_HANDSHAKE_FILE = val; else overrides.BILI_PLUGIN_AGENT = val; break; } diff --git a/src/config.ts b/src/config.ts index 92ccd084c..25f37db2a 100644 --- a/src/config.ts +++ b/src/config.ts @@ -233,6 +233,10 @@ export type ProxyOptions = { passthrough: boolean; autoUpdate: boolean; logFile?: string; + /** Launcher handshake (#401): EADDRINUSE on the preferred port falls back + * to an OS-assigned port, and the actual origin + pid are written here + * after bind so the spawning parent attaches to exactly this child. */ + handshakeFile?: string; /** MITM transparent-proxy mode. When enabled, an HTTP CONNECT handler is * attached so clients that only know how to set HTTP_PROXY (ZCode with a * locked-in endpoint) can route through the proxy. Whitelisted model @@ -359,6 +363,7 @@ export function loadOptions(env: NodeJS.ProcessEnv = process.env): ProxyOptions passthrough: (env.ACP_PASSTHROUGH ?? (fileConfig.passthrough ? "1" : "0")) === "1", autoUpdate: (env.ACP_AUTO_UPDATE ?? (fileConfig.autoUpdate === false ? "0" : "1")) !== "0", logFile: env.ACP_LOG_FILE !== undefined ? (env.ACP_LOG_FILE || undefined) : fileConfig.logFile, + handshakeFile: env.BILI_HANDSHAKE_FILE || undefined, mitm: { enabled: (env.BILI_MITM ?? (fileConfig.mitm?.enabled === false ? "0" : "1")) !== "0", domains: dedupeDomains([ diff --git a/src/launcher.ts b/src/launcher.ts index d34e4c6d4..100c5f520 100644 --- a/src/launcher.ts +++ b/src/launcher.ts @@ -27,7 +27,6 @@ */ import { randomUUID } from "node:crypto"; import fs from "node:fs"; -import net from "node:net"; import os from "node:os"; import path from "node:path"; import { pathToFileURL } from "node:url"; @@ -90,6 +89,12 @@ const HEALTH_PATH = "/__bili/health"; const HEALTH_POLL_INTERVAL_MS = 200; const SPAWN_WAIT_MS = 20000; const PROBE_TIMEOUT_MS = 1500; +// #401 handshake protocol: how long to wait for OUR child to report its actual +// bind (origin + pid) before giving up. Polls are fast because a dead child is +// detected as soon as its "exit" event fires, so a crashed proxy fails in ms +// instead of the old 20s health-timeout. +const HANDSHAKE_WAIT_MS = 10000; +const HANDSHAKE_POLL_INTERVAL_MS = 50; const DEFAULT_MITM_DOMAIN_SET = new Set(DEFAULT_MITM_DOMAINS.map((d) => d.toLowerCase())); @@ -1452,30 +1457,6 @@ async function probeHealth( } } -export function findFreePort(preferred: number, host = LAUNCHER_DEFAULT_HOST): Promise { - const tryBind = (port: number): Promise => - new Promise((resolve) => { - const srv = net.createServer(); - srv.once("error", () => resolve(false)); - srv.once("listening", () => srv.close(() => resolve(true))); - srv.listen(port, host); - }); - return tryBind(preferred).then((free) => { - if (free) return preferred; - return new Promise((resolve, reject) => { - const srv = net.createServer(); - srv.once("error", reject); - srv.listen(0, host, () => { - const addr = srv.address(); - srv.close(() => { - if (addr && typeof addr === "object") resolve(addr.port); - else reject(new Error("could not allocate a free port")); - }); - }); - }); - }); -} - const INHERITED_PROXY_VARS = ["http_proxy", "https_proxy", "all_proxy", "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY"]; export function stripInheritedProxy(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { @@ -1484,13 +1465,47 @@ export function stripInheritedProxy(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { return cleaned; } -function proxyStartArgs(opts: LaunchOptions): string[] { - const args = ["start", "--host", opts.host, "--port", String(opts.port)]; +function proxyStartArgs(opts: LaunchOptions, handshakeFile: string): string[] { + // The preferred port is only a HINT: the child binds it itself and, if it + // is taken by then, falls back to an OS-assigned port and reports the + // actual origin through the handshake file (#401 — no parent-side probe). + const args = ["start", "--host", opts.host, "--port", String(opts.port), "--handshake-file", handshakeFile]; if (opts.passthrough) args.push("--passthrough"); if (opts.debug) args.push("--debug"); return args; } +interface HandshakeInfo { + origin: string; + pid?: number; +} + +function readHandshake(file: string): HandshakeInfo | undefined { + let raw: string; + try { + raw = fs.readFileSync(file, "utf8"); + } catch { + return undefined; + } + try { + const parsed = JSON.parse(raw) as { origin?: unknown; pid?: unknown }; + if (typeof parsed.origin === "string" && parsed.origin.startsWith("http")) { + return { origin: parsed.origin, pid: typeof parsed.pid === "number" ? parsed.pid : undefined }; + } + return undefined; + } catch { + return undefined; + } +} + +function originPort(origin: string): number { + try { + return Number(new URL(origin).port) || 0; + } catch { + return 0; + } +} + export async function ensureProxyRunning( opts: LaunchOptions, deps: LauncherDeps = {}, @@ -1500,18 +1515,21 @@ export async function ensureProxyRunning( const now = deps.now ?? Date.now; const sleepImpl = deps.sleep ?? ((ms: number) => new Promise((r) => setTimeout(r, ms))); - const port = await findFreePort(opts.port, opts.host); - const spawnedOrigin = proxyOrigin(opts.host, port); + // Per-launch handshake file: only OUR child writes it, so whatever origin + // it reports is ours by construction — attaching to a foreign instance is + // impossible even when another bili grabs the preferred port (#401). + const launchId = randomUUID(); + const handshakeFile = path.join(os.tmpdir(), `bili-handshake-${launchId}.json`); const script = process.argv[1]; if (!script) throw new Error("bili: cannot resolve launcher script path"); - const logPath = path.join(os.tmpdir(), `bili-proxy-${port}.log`); + const logPath = path.join(os.tmpdir(), `bili-proxy-${launchId}.log`); const logFd = fs.openSync(logPath, "a"); let child: SpawnChild; try { child = spawnImpl( process.execPath, - [script, ...proxyStartArgs({ ...opts, port, debug: true })], + [script, ...proxyStartArgs(opts, handshakeFile)], { detached: true, stdio: ["ignore", logFd, logFd], @@ -1535,14 +1553,51 @@ export async function ensureProxyRunning( child.unref?.(); } catch {} + let childExited = false; + child.on?.("exit", () => { + childExited = true; + }); + + const cleanup = (): void => { + try { + fs.rmSync(handshakeFile, { force: true }); + } catch {} + }; + function fail(reason: string): never { + cleanup(); + stopProxy({ origin: "", port: 0, child }); + throw new Error(`bili: ${reason} (proxy log: ${logPath})`); + } + + const handshakeDeadline = now() + HANDSHAKE_WAIT_MS; + let handshake: HandshakeInfo | undefined; + while (!handshake && now() < handshakeDeadline) { + if (childExited) break; + await sleepImpl(HANDSHAKE_POLL_INTERVAL_MS); + handshake = readHandshake(handshakeFile); + } + if (!handshake) { + fail(childExited + ? "proxy child exited before reporting its bind" + : `proxy did not report its bind within ${HANDSHAKE_WAIT_MS}ms`); + } + if (child.pid !== undefined && handshake.pid !== undefined && handshake.pid !== child.pid) { + fail(`handshake ownership check failed (expected pid ${child.pid}, got ${handshake.pid}) — refusing to attach`); + } + + const origin = handshake.origin; const deadline = now() + SPAWN_WAIT_MS; while (now() < deadline) { + if (childExited) break; await sleepImpl(HEALTH_POLL_INTERVAL_MS); - if (await probeHealth(spawnedOrigin, fetchImpl)) { - return { origin: spawnedOrigin, port, child, logPath }; + if (await probeHealth(origin, fetchImpl)) { + cleanup(); + return { origin, port: originPort(origin), child, logPath }; } } - throw new Error(`bili: proxy did not become healthy at ${spawnedOrigin} within ${SPAWN_WAIT_MS}ms`); + fail(childExited + ? `proxy child exited before becoming healthy at ${origin}` + : `proxy did not become healthy at ${origin} within ${SPAWN_WAIT_MS}ms`); } export function stopProxy(handle: ProxyHandle): void { diff --git a/src/server.ts b/src/server.ts index 351ea5f51..78b68a746 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,5 +1,6 @@ import http from "node:http"; import fs from "node:fs"; +import path from "node:path"; import { randomUUID } from "node:crypto"; import { createCore, type CompressionCore, type CompressionState, type Config, type CoreMessage, type NudgeDecision, type Prompts, defaultPrompts, defaultCountTokens, estimateTokensFast, renderNudgeText, deactivateBlock, viableRanges } from "acp-kernel"; import { resolveCompress, resolveCompressPrompts, resolveRequestConfig } from "./compress-settings.js"; @@ -347,48 +348,74 @@ export async function startServer(opts: ProxyOptions): Promise { const allowRemoteConnect = opts.host === "0.0.0.0" || opts.host === "::" || !isLoopbackAddress(opts.host); setupMitm(server, opts.mitm.domains, (msg) => log("info", msg), (host) => resolveProxy(opts.routes, opts.proxy, `https://${host}`, opts.proxyFallback), allowRemoteConnect); } - server.listen(opts.port, opts.host, () => { + // Discovery origin local MCP shells dial: collapse wildcard binds to + // loopback (localhost may resolve to ::1, where an IPv4-only listener is + // absent) and bracket bare IPv6 literals so the file always holds a valid URL. + const originHost = opts.host === "0.0.0.0" || opts.host === "::" || opts.host === "localhost" ? "127.0.0.1" : opts.host.includes(":") && !opts.host.startsWith("[") ? `[${opts.host}]` : opts.host; + const onListening = () => { const nonLoopbackBind = opts.host === "0.0.0.0" || opts.host === "::" || !isLoopbackAddress(opts.host); // Honest bind display: a wildcard bind shows as 0.0.0.0 (the user // chose to expose the proxy — hiding it behind "localhost" made // remote setups look broken in the log, see #240). const displayHost = nonLoopbackBind ? opts.host : opts.host === "0.0.0.0" ? "localhost" : opts.host; + const actualPort = server.address() === null ? opts.port : (server.address() as { port: number }).port; try { fs.mkdirSync(stateDir(), { recursive: true }); - // Discovery origin local MCP shells dial: collapse wildcard - // binds to loopback (localhost may resolve to ::1, where an - // IPv4-only listener is absent) and bracket bare IPv6 literals - // so the file always holds a valid URL. - const originHost = opts.host === "0.0.0.0" || opts.host === "::" || opts.host === "localhost" ? "127.0.0.1" : opts.host.includes(":") && !opts.host.startsWith("[") ? `[${opts.host}]` : opts.host; - fs.writeFileSync(proxyOriginFile(), `http://${originHost}:${server.address() === null ? opts.port : (server.address() as { port: number }).port}\n`); + fs.writeFileSync(proxyOriginFile(), `http://${originHost}:${actualPort}\n`); } catch { // best-effort discovery hint for host-spawned MCP shells } + if (opts.handshakeFile) { + // Atomic publish (tmp+rename): the spawning launcher polls this + // file and must never observe a half-written handshake (#401). + try { + fs.mkdirSync(path.dirname(opts.handshakeFile), { recursive: true }); + const hsTmp = `${opts.handshakeFile}.tmp`; + fs.writeFileSync(hsTmp, JSON.stringify({ origin: `http://${originHost}:${actualPort}`, pid: process.pid }) + "\n"); + fs.renameSync(hsTmp, opts.handshakeFile); + } catch { + // best-effort: the launcher falls back to its health-poll deadline + } + } const nOverrides = Object.keys(opts.routes).length; log( "info", - `acp-proxy listening on http://${displayHost}:${opts.port}` + - ` — web UI: http://${displayHost}:${opts.port}/__bili/` + - ` — zero-config: prefix any baseURL with http://${displayHost}:${opts.port}/bili/` + + `acp-proxy listening on http://${displayHost}:${actualPort}` + + ` — web UI: http://${displayHost}:${actualPort}/__bili/` + + ` — zero-config: prefix any baseURL with http://${displayHost}:${actualPort}/bili/` + (nOverrides ? ` — context overrides for ${nOverrides} upstream URL(s)` : "") + (opts.mitm.enabled ? ` — MITM proxy on (whitelist)${opts.mitm.domains.length ? ` +${opts.mitm.domains.join(",")}` : ""}` : ""), ); if (nonLoopbackBind) { log( "warn", - `[security] bound to ${opts.host} — proxy endpoints (/bili/, CONNECT for whitelisted model hosts) are reachable from the network with NO authentication; /__bili/ management endpoints stay loopback-only. Restrict access with a firewall on untrusted networks. Remote agents: point baseURL at http://:${opts.port}/bili/`, + `[security] bound to ${opts.host} — proxy endpoints (/bili/, CONNECT for whitelisted model hosts) are reachable from the network with NO authentication; /__bili/ management endpoints stay loopback-only. Restrict access with a firewall on untrusted networks. Remote agents: point baseURL at http://:${actualPort}/bili/`, ); } if (opts.debug) { log("info", `[debug] build features: raw-HTTP-capture(on) | remote_compaction_v2-strip(on) | cert-MITM-launcher(on) | strip-acp-summary(on) — seeing this line confirms the launcher build (not registry 0.1.34)`); } - }); + }; + const listenOn = (port: number) => { + server.listen(port, opts.host, onListening); + }; + listenOn(opts.port); // Listen errors (EADDRINUSE port taken, EACCES privileged port, EAFNOSUPPORT // bad host) surface as an 'error' event on the server. Without a listener // Node treats it as an unhandled 'error' and throws, aborting before the // graceful-shutdown flush can run. Catch, log a human-readable message, // flush sessions, and exit cleanly (exit code 1 so callers/scripts notice). + let handshakeFallbackUsed = false; server.on("error", (err: NodeJS.ErrnoException) => { + // #401: in launcher handshake mode the preferred port may be taken at + // bind time — fall back to an OS-assigned port (atomic, no probe + // window) and report the actual origin via the handshake file. + if (err.code === "EADDRINUSE" && opts.handshakeFile && !handshakeFallbackUsed && !server.listening) { + handshakeFallbackUsed = true; + log("warn", `listen: port ${opts.port} is already in use — falling back to an OS-assigned free port (launcher handshake mode)`); + listenOn(0); + return; + } const hint = err.code === "EADDRINUSE" ? ` — port ${opts.port} is already in use. Stop the other process or use --port .` diff --git a/tests/handshake-fallback.test.ts b/tests/handshake-fallback.test.ts new file mode 100644 index 000000000..392b3be56 --- /dev/null +++ b/tests/handshake-fallback.test.ts @@ -0,0 +1,79 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import http from "node:http"; +import net from "node:net"; +import os from "node:os"; +import path from "node:path"; +import { defaultConfig } from "acp-kernel"; +import { startServer } from "../src/server.ts"; +import { SessionStore, _setStoreForTest } from "../src/persist.ts"; +import type { ProxyOptions } from "../src/config.ts"; +import { _setForTest as setRegistryForTest } from "../src/registry.ts"; + +test("server: preferred port taken + handshake file → OS-port fallback, origin+pid reported (#401)", async () => { + _setStoreForTest(new SessionStore({ enabled: false })); + setRegistryForTest({}); + + const blocker = net.createServer(); + await new Promise((resolve, reject) => { + blocker.once("error", reject); + blocker.listen(0, "127.0.0.1", () => resolve()); + }); + const blockerAddr = blocker.address(); + const stolenPort = typeof blockerAddr === "object" && blockerAddr ? blockerAddr.port : 0; + + const handshakeFile = path.join(os.tmpdir(), `bili-handshake-test-${process.pid}-${Date.now()}.json`); + try { + const upstream = http.createServer((_req, res) => { + res.writeHead(200); + res.end("ok"); + }); + await new Promise((resolve, reject) => { + upstream.once("error", reject); + upstream.listen(0, "127.0.0.1", () => resolve()); + }); + const upstreamPort = (upstream.address() as { port: number }).port; + + const opts: ProxyOptions = { + port: stolenPort, + host: "127.0.0.1", + upstream: "http://127.0.0.1", + routes: { + [`http://127.0.0.1:${upstreamPort}`]: { models: { "gpt-test": { context: 400_000 } } }, + }, + modelContextLimit: 400_000, + kernelConfig: defaultConfig(400_000), + compress: { injectTool: true, injectNudge: true }, + promptCache: { routing: "auto" }, + sessionHeader: "x-acp-session", + log: false, + debug: false, + passthrough: false, + autoUpdate: false, + mitm: { enabled: false, domains: [] }, + handshakeFile, + }; + const proxy = await startServer(opts); + const t0 = Date.now(); + while (!fs.existsSync(handshakeFile)) { + assert.ok(Date.now() - t0 < 5000, "handshake file published after fallback bind"); + await new Promise((r) => setTimeout(r, 10)); + } + const actualPort = (proxy.address() as { port: number }).port; + assert.notEqual(actualPort, stolenPort, "fell back to an OS-assigned port"); + + const info = JSON.parse(fs.readFileSync(handshakeFile, "utf8")) as { origin: string; pid?: number }; + assert.equal(info.origin, `http://127.0.0.1:${actualPort}`); + assert.equal(info.pid, process.pid); + + const health = await fetch(`http://127.0.0.1:${actualPort}/__bili/health`); + assert.equal(health.status, 200); + + await new Promise((resolve, reject) => proxy.close((e) => (e ? reject(e) : resolve()))); + upstream.close(); + } finally { + fs.rmSync(handshakeFile, { force: true }); + blocker.close(); + } +}); diff --git a/tests/launcher.test.ts b/tests/launcher.test.ts index 7df4c7840..fa390161d 100644 --- a/tests/launcher.test.ts +++ b/tests/launcher.test.ts @@ -46,7 +46,6 @@ import { dshArgsWithPatch, readOpencodeConfig, resolveOpencodeConfigFile, - findFreePort, ensureProxyRunning, stopProxy, resolveLauncherWindow, @@ -247,31 +246,6 @@ tools.web_search = false assert.equal(cfg.providers["bili-openai"].baseUrl, "http://127.0.0.1:8787/bili/https://api.openai.com/v1"); }); -test("findFreePort: returns preferred when it is free", async () => { - const port = 50000 + Math.floor(Math.random() * 1000); - const got = await findFreePort(port, LAUNCHER_DEFAULT_HOST); - assert.equal(got, port); -}); - -test("findFreePort: returns another port when preferred is occupied", async () => { - const blocker = net.createServer(); - const occupied = await new Promise((resolve, reject) => { - blocker.once("error", reject); - blocker.listen(0, LAUNCHER_DEFAULT_HOST, () => { - const addr = blocker.address(); - if (addr && typeof addr === "object") resolve(addr.port); - else reject(new Error("no port")); - }); - }); - try { - const got = await findFreePort(occupied, LAUNCHER_DEFAULT_HOST); - assert.notEqual(got, occupied); - assert.ok(got > 0); - } finally { - blocker.close(); - } -}); - import { selfPackageRoot, ompPluginLoadedFrom } from "../src/plugin-install.js"; test("runLaunch pi: native -e plugin injected only when not installed", async () => { @@ -317,7 +291,7 @@ test("runLaunch pi: native -e plugin injected only when not installed", async () }; return child; } - return makeFakeChild(42422); + return fakeProxyChild(args, 42422); }; const fetchImpl = async () => ({ ok: true }); @@ -423,7 +397,7 @@ test("runLaunch omp: native -e plugin injected only when no loadable config entr }; return child; } - return makeFakeChild(42422); + return fakeProxyChild(args, 42422); }; const fetchImpl = async () => ({ ok: true }); @@ -500,12 +474,19 @@ test("ompPluginLoadedFrom: only entries whose file exists count as loaded", () = } }); -function makeFakeChild(pid: number): SpawnChild { +interface FakeChild extends SpawnChild { + fire(event: string, ...args: unknown[]): void; + killed: boolean; +} + +function makeFakeChild(pid: number): FakeChild { const handlers = new Map void)[]>(); - return { + const child: FakeChild = { pid, + killed: false, unref() {}, kill() { + child.killed = true; return true; }, on(event, listener) { @@ -513,64 +494,181 @@ function makeFakeChild(pid: number): SpawnChild { list.push(listener); handlers.set(event, list); }, + fire(event, ...args) { + for (const h of handlers.get(event) ?? []) h(...args); + }, }; + return child; +} + +function writeHandshakeFromArgs(args: readonly string[], info: { origin: string; pid?: number }): void { + fs.writeFileSync(args[args.indexOf("--handshake-file") + 1], JSON.stringify(info)); +} + +function fakeProxyChild(args: readonly string[], pid: number): FakeChild { + const host = args[args.indexOf("--host") + 1]; + const port = args[args.indexOf("--port") + 1]; + writeHandshakeFromArgs(args, { origin: `http://${host}:${port}`, pid }); + return makeFakeChild(pid); } -test("ensureProxyRunning: always spawns a fresh proxy, never reuses a listener", async () => { +test("ensureProxyRunning: spawns once and attaches to the origin the child reports", async () => { let spawnCalls = 0; - const spawnImpl: SpawnFn = () => { + let seenHandshakeFile = ""; + const spawnImpl: SpawnFn = (_cmd, args) => { spawnCalls++; - return makeFakeChild(0); + seenHandshakeFile = args[args.indexOf("--handshake-file") + 1]; + writeHandshakeFromArgs(args, { origin: "http://127.0.0.1:8787", pid: 42421 }); + return makeFakeChild(42421); }; - const fetchImpl = async () => ({ ok: true }); const handle = await ensureProxyRunning( { host: "127.0.0.1", port: 8787, passthrough: false, debug: false }, - { fetchImpl, spawnImpl }, + { fetchImpl: async () => ({ ok: true }), spawnImpl, sleep: () => Promise.resolve() }, ); assert.equal(spawnCalls, 1); assert.ok(handle.child); - assert.equal(handle.origin, `http://127.0.0.1:${handle.port}`); - assert.notEqual(handle.child, null); + assert.equal(handle.origin, "http://127.0.0.1:8787"); + assert.equal(handle.port, 8787); + assert.match(seenHandshakeFile, /bili-handshake-.*\.json$/); + assert.equal(fs.existsSync(seenHandshakeFile), false, "handshake file removed after attach"); }); -test("ensureProxyRunning: spawns when not healthy, polls until healthy", async () => { +test("ensureProxyRunning: preferred port stolen → attaches to own child's fallback origin, never the stealer (#401)", async () => { + const blocker = net.createServer(); + const stolenPort = await new Promise((resolve, reject) => { + blocker.once("error", reject); + blocker.listen(0, LAUNCHER_DEFAULT_HOST, () => { + const addr = blocker.address(); + if (addr && typeof addr === "object") resolve(addr.port); + else reject(new Error("no port")); + }); + }); + try { + const fallbackPort = stolenPort === 59999 ? 59998 : 59999; + const probedUrls: string[] = []; + const spawnImpl: SpawnFn = (_cmd, args) => { + writeHandshakeFromArgs(args, { origin: `http://127.0.0.1:${fallbackPort}`, pid: 42424 }); + return makeFakeChild(42424); + }; + const handle = await ensureProxyRunning( + { host: LAUNCHER_DEFAULT_HOST, port: stolenPort, passthrough: false, debug: false }, + { + fetchImpl: async (url) => { + probedUrls.push(url); + return { ok: true }; + }, + spawnImpl, + sleep: () => Promise.resolve(), + }, + ); + assert.notEqual(fallbackPort, stolenPort); + assert.equal(handle.origin, `http://127.0.0.1:${fallbackPort}`); + assert.equal(handle.port, fallbackPort); + assert.ok(probedUrls.length >= 1, "health probe happened"); + for (const url of probedUrls) { + assert.ok(url.includes(`:${fallbackPort}/`), `probed own child only, got ${url}`); + assert.ok(!url.includes(`:${stolenPort}/`), `must never probe the stealer, got ${url}`); + } + } finally { + blocker.close(); + } +}); + +test("ensureProxyRunning: child dies before reporting bind → fails fast (<5s) with log path (#401)", async () => { let probes = 0; - const fetchImpl = async () => { - probes++; - return { ok: probes >= 2 }; + const child = makeFakeChild(42423); + const spawnImpl: SpawnFn = () => { + setImmediate(() => child.fire("exit", 1, null)); + return child; }; - let spawnedArgs: string[] | null = null; + const t0 = Date.now(); + await assert.rejects( + ensureProxyRunning( + { host: "127.0.0.1", port: 8787, passthrough: false, debug: false }, + { + fetchImpl: async () => { + probes++; + return { ok: true }; + }, + spawnImpl, + }, + ), + (err: Error) => { + assert.match(err.message, /exited before reporting its bind/); + assert.match(err.message, /proxy log:/); + assert.equal(probes, 0, "never probed when the child died pre-handshake"); + return true; + }, + ); + assert.ok(Date.now() - t0 < 5000, "failure must be fast, not a 20s hang"); + assert.equal(child.killed, true, "failed launch kills the child"); +}); + +test("ensureProxyRunning: handshake pid mismatch refuses to attach (#401)", async () => { const spawnImpl: SpawnFn = (_cmd, args) => { - spawnedArgs = [...args]; + writeHandshakeFromArgs(args, { origin: "http://127.0.0.1:8787", pid: 11111 }); + return makeFakeChild(22222); + }; + await assert.rejects( + ensureProxyRunning( + { host: "127.0.0.1", port: 8787, passthrough: false, debug: false }, + { fetchImpl: async () => ({ ok: true }), spawnImpl, sleep: () => Promise.resolve() }, + ), + /ownership check failed/, + ); +}); + +test("ensureProxyRunning: --debug is never forced — inherited from parent opts only (#401)", async () => { + const run = async (debug: boolean): Promise => { + let seenArgs: string[] | null = null; + const spawnImpl: SpawnFn = (_cmd, args) => { + seenArgs = [...args]; + writeHandshakeFromArgs(args, { origin: "http://127.0.0.1:8787", pid: 42425 }); + return makeFakeChild(42425); + }; + await ensureProxyRunning( + { host: "127.0.0.1", port: 8787, passthrough: false, debug }, + { fetchImpl: async () => ({ ok: true }), spawnImpl, sleep: () => Promise.resolve() }, + ); + return seenArgs!; + }; + assert.ok(!(await run(false)).includes("--debug"), "debug off → no --debug flag"); + assert.ok((await run(true)).includes("--debug"), "debug on → --debug passed through"); +}); + +test("ensureProxyRunning: polls until healthy; throws when never healthy within deadline", async () => { + let probes = 0; + const spawnImpl: SpawnFn = (_cmd, args) => { + writeHandshakeFromArgs(args, { origin: "http://127.0.0.1:8787", pid: 42421 }); return makeFakeChild(42421); }; const handle = await ensureProxyRunning( { host: "127.0.0.1", port: 8787, passthrough: false, debug: false }, - { fetchImpl, spawnImpl, sleep: () => Promise.resolve() }, + { + fetchImpl: async () => { + probes++; + return { ok: probes >= 2 }; + }, + spawnImpl, + sleep: () => Promise.resolve(), + }, ); assert.equal(handle.child?.pid, 42421); - assert.ok(spawnedArgs !== null); - assert.ok(spawnedArgs.includes("start")); - assert.ok(spawnedArgs.includes("--host")); - const portIdx = spawnedArgs.indexOf("--port"); - assert.ok(portIdx >= 0, "spawn args include --port"); - assert.equal(spawnedArgs[portIdx + 1], String(handle.port)); assert.ok(probes >= 2); -}); -test("ensureProxyRunning: throws when never healthy within deadline", async () => { - const fetchImpl = async () => ({ ok: false }); - const spawnImpl: SpawnFn = () => makeFakeChild(42422); let ticks = 0; - const now = () => ticks * 1000; - const sleep = () => { - ticks += 10; - return Promise.resolve(); - }; await assert.rejects( ensureProxyRunning( { host: "127.0.0.1", port: 8787, passthrough: false, debug: false }, - { fetchImpl, spawnImpl, now, sleep }, + { + fetchImpl: async () => ({ ok: false }), + spawnImpl, + now: () => ticks * 1000, + sleep: () => { + ticks += 10; + return Promise.resolve(); + }, + }, ), /did not become healthy/, ); @@ -1785,7 +1883,7 @@ test("runLaunch dsh: DSH_HOME overlay + DEEPSEEK_BASE_URL env, real home untouch }; return child; } - return makeFakeChild(42423); + return fakeProxyChild(args, 42423); }; const prevExit = process.exit; @@ -1876,7 +1974,7 @@ test("runLaunch omp: launcher hands per-model windows to the spawned proxy", asy return child; } proxyEnvs.push((opts as { env?: NodeJS.ProcessEnv } | undefined)?.env); - return makeFakeChild(42422); + return fakeProxyChild(args, 42422); }; const fetchImpl = async () => ({ ok: true }); @@ -2102,7 +2200,7 @@ test("runLaunch codex: budget args injected for MITM mode (built-in table window }; return child; } - return makeFakeChild(42422); + return fakeProxyChild(args, 42422); }; const fetchImpl = async () => ({ ok: true }); const prevExit = process.exit; @@ -2187,7 +2285,7 @@ test("runLaunch claude: CLAUDE_CODE_AUTO_COMPACT_WINDOW injected (built-in table }; return child; } - return makeFakeChild(42422); + return fakeProxyChild(args, 42422); }; const fetchImpl = async () => ({ ok: true }); const prevExit = process.exit;