From 4c2d06be2bf8860e5e67c1a14d99a185de960729 Mon Sep 17 00:00:00 2001 From: wahaha1223 <0668001153@xydigit.com> Date: Tue, 28 Jul 2026 18:01:02 +0800 Subject: [PATCH] fix(browser): reject startup when control ports are occupied (#109994) Co-authored-by: Peter Steinberger --- .../src/browser/bridge-server.auth.test.ts | 28 +++++++++++++++++++ .../browser/src/browser/bridge-server.ts | 6 ++-- extensions/browser/src/browser/http-listen.ts | 24 ++++++++++++++++ .../browser/server.auth-fail-closed.test.ts | 24 ++++++++++++++++ extensions/browser/src/server.ts | 7 ++--- 5 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 extensions/browser/src/browser/http-listen.ts diff --git a/extensions/browser/src/browser/bridge-server.auth.test.ts b/extensions/browser/src/browser/bridge-server.auth.test.ts index 56d0fed4354a..a51a9a21b597 100644 --- a/extensions/browser/src/browser/bridge-server.auth.test.ts +++ b/extensions/browser/src/browser/bridge-server.auth.test.ts @@ -1,4 +1,5 @@ // Browser tests cover bridge server.auth plugin behavior. +import { createServer } from "node:http"; import { afterEach, describe, expect, it, vi } from "vitest"; import { getBridgeAuthForPort } from "./bridge-auth-registry.js"; import { startBrowserBridgeServer, stopBrowserBridgeServer } from "./bridge-server.js"; @@ -96,6 +97,33 @@ describe("startBrowserBridgeServer auth", () => { ).rejects.toThrow(/requires auth/i); }); + it("rejects startup when the bridge port is already in use", async () => { + const blocker = createServer(); + await new Promise((resolve) => { + blocker.listen(0, "127.0.0.1", resolve); + }); + const address = blocker.address(); + if (!address || typeof address === "string") { + throw new Error("expected blocker TCP address"); + } + + try { + await expect( + startBrowserBridgeServer({ + resolved: buildResolvedConfig(), + authToken: "secret-token", + host: "127.0.0.1", + port: address.port, + skipRouteRegistrationForTest: true, + }), + ).rejects.toMatchObject({ code: "EADDRINUSE" }); + } finally { + await new Promise((resolve) => { + blocker.close(() => resolve()); + }); + } + }); + it("closes ingress but retains exact bridge cleanup state for retry", async () => { const bridge = await startBrowserBridgeServer({ resolved: buildResolvedConfig(), diff --git a/extensions/browser/src/browser/bridge-server.ts b/extensions/browser/src/browser/bridge-server.ts index c9c412983cfc..b30113ee4c4a 100644 --- a/extensions/browser/src/browser/bridge-server.ts +++ b/extensions/browser/src/browser/bridge-server.ts @@ -11,6 +11,7 @@ import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runti import { isLoopbackHost } from "../gateway/net.js"; import { deleteBridgeAuthForPort, setBridgeAuthForPort } from "./bridge-auth-registry.js"; import type { ResolvedBrowserConfig } from "./config.js"; +import { listenBrowserHttpServer } from "./http-listen.js"; import type { BrowserRouteRegistrar } from "./routes/types.js"; import { stopBrowserBridgeRuntime } from "./runtime-lifecycle.js"; import type { BrowserServerState, ProfileContext } from "./server-context.js"; @@ -154,10 +155,7 @@ export async function startBrowserBridgeServer(params: { registerBrowserRoutes(app as unknown as BrowserRouteRegistrar, ctx); } - const server = await new Promise((resolve, reject) => { - const s = app.listen(port, host, () => resolve(s)); - s.once("error", reject); - }); + const server = await listenBrowserHttpServer(app, port, host); const address = server.address() as AddressInfo | null; const resolvedPort = address?.port ?? port; diff --git a/extensions/browser/src/browser/http-listen.ts b/extensions/browser/src/browser/http-listen.ts new file mode 100644 index 000000000000..551861b51a8d --- /dev/null +++ b/extensions/browser/src/browser/http-listen.ts @@ -0,0 +1,24 @@ +import { createServer, type RequestListener, type Server } from "node:http"; + +export function listenBrowserHttpServer( + app: RequestListener, + port: number, + host: string, +): Promise { + return new Promise((resolve, reject) => { + const server = createServer(app); + const onError = (error: Error) => { + server.off("listening", onListening); + reject(error); + }; + const onListening = () => { + server.off("error", onError); + resolve(server); + }; + // Install both terminal listeners before listen so Express cannot settle + // its callback with a bind error before this startup Promise rejects. + server.once("error", onError); + server.once("listening", onListening); + server.listen(port, host); + }); +} diff --git a/extensions/browser/src/browser/server.auth-fail-closed.test.ts b/extensions/browser/src/browser/server.auth-fail-closed.test.ts index 7f70f6926044..700f93621c6c 100644 --- a/extensions/browser/src/browser/server.auth-fail-closed.test.ts +++ b/extensions/browser/src/browser/server.auth-fail-closed.test.ts @@ -1,4 +1,5 @@ // Browser tests cover server.auth fail closed plugin behavior. +import { createServer } from "node:http"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { startBrowserControlServerFromConfig, stopBrowserControlServer } from "../server.js"; import { getFreePort } from "./test-port.js"; @@ -127,4 +128,27 @@ describe("browser control auth bootstrap failures", () => { expect(started).toBeNull(); }); + + it("returns null when the browser control port is already in use", async () => { + const blocker = createServer(); + await new Promise((resolve) => { + blocker.listen(0, "127.0.0.1", resolve); + }); + const address = blocker.address(); + if (!address || typeof address === "string") { + throw new Error("expected blocker TCP address"); + } + mocks.controlPort = address.port; + mocks.ensureBrowserControlAuth.mockResolvedValueOnce({ auth: { token: "test-token" } }); + mocks.resolveBrowserControlAuth.mockReturnValueOnce({ token: "test-token" }); + mocks.shouldAutoGenerateBrowserAuth.mockReturnValueOnce(false); + + try { + await expect(startBrowserControlServerFromConfig()).resolves.toBeNull(); + } finally { + await new Promise((resolve) => { + blocker.close(() => resolve()); + }); + } + }); }); diff --git a/extensions/browser/src/server.ts b/extensions/browser/src/server.ts index 9cd50a1400f5..5baa7c4ef188 100644 --- a/extensions/browser/src/server.ts +++ b/extensions/browser/src/server.ts @@ -1,7 +1,6 @@ /** * Browser control HTTP server startup and shutdown entrypoints. */ -import type { Server } from "node:http"; import express from "express"; import { createBrowserControlContext, @@ -18,6 +17,7 @@ import { resolveBrowserControlAuth, shouldAutoGenerateBrowserAuth, } from "./browser/control-auth.js"; +import { listenBrowserHttpServer } from "./browser/http-listen.js"; import { registerBrowserRoutes } from "./browser/routes/index.js"; import type { BrowserRouteRegistrar } from "./browser/routes/types.js"; import type { BrowserServerState } from "./browser/server-context.js"; @@ -85,10 +85,7 @@ async function startBrowserControlServerUnlocked(): Promise((resolve, reject) => { - const s = app.listen(port, "127.0.0.1", () => resolve(s)); - s.once("error", reject); - }).catch((err: unknown) => { + const server = await listenBrowserHttpServer(app, port, "127.0.0.1").catch((err: unknown) => { logServer.error(`openclaw browser server failed to bind 127.0.0.1:${port}: ${String(err)}`); return null; });