Skip to content

fix(launcher): eliminate port TOCTOU via child-reported handshake bind; stop forcing --debug (#401) #480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ranxianglei wants to merge 1 commit into from
Closed

fix(launcher): eliminate port TOCTOU via child-reported handshake bind; stop forcing --debug (#401) #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand All @@ -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;
}
Expand Down
5 changes: 5 additions & 0 deletions src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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([
Expand Down
123 changes: 89 additions & 34 deletions src/launcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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()));

Expand Down Expand Up @@ -1452,30 +1457,6 @@ async function probeHealth(
}
}

export function findFreePort(preferred: number, host = LAUNCHER_DEFAULT_HOST): Promise<number> {
const tryBind = (port: number): Promise<boolean> =>
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<number>((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 {
Expand All @@ -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 = {},
Expand All @@ -1500,18 +1515,21 @@ export async function ensureProxyRunning(
const now = deps.now ?? Date.now;
const sleepImpl = deps.sleep ?? ((ms: number) => new Promise<void>((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],
Expand All @@ -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 {
Expand Down
51 changes: 39 additions & 12 deletions src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -347,48 +348,74 @@ export async function startServer(opts: ProxyOptions): Promise<http.Server> {
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://<this-host>:${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://<this-host>:${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 <N>.`
Expand Down
79 changes: 79 additions & 0 deletions tests/handshake-fallback.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<void>((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<void>((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<void>((resolve, reject) => proxy.close((e) => (e ? reject(e) : resolve())));
upstream.close();
} finally {
fs.rmSync(handshakeFile, { force: true });
blocker.close();
}
});
Loading