From 40494d67f2289215fc7fddd5db7e3b25858dd576 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 18:15:32 +0100 Subject: [PATCH] fix(browser): harden extension relay reconnect race Co-authored-by: Ho Lim <166576253+HOYALIM@users.noreply.github.com> --- CHANGELOG.md | 1 + src/browser/extension-relay.test.ts | 45 +++++++++++++++++++++++++++++ src/browser/extension-relay.ts | 28 ++++++++++++++---- 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2575beb7d0..4d44c50b9f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ Docs: https://docs.openclaw.ai - Memory/Batch: route OpenAI/Voyage/Gemini batch upload/create/status/download requests through the same guarded HTTP path for consistent SSRF policy enforcement. - Install/Discord Voice: make `@discordjs/opus` an optional dependency so `openclaw` install/update no longer hard-fails when native Opus builds fail, while keeping `opusscript` as the runtime fallback decoder for Discord voice flows. (#23737, #23733, #23703) - Slack/Upload: resolve bare user IDs (U-prefix) to DM channel IDs via `conversations.open` before calling `files.uploadV2`, which rejects non-channel IDs. `chat.postMessage` tolerates user IDs directly, but `files.uploadV2` → `completeUploadExternal` validates `channel_id` against `^[CGDZ][A-Z0-9]{8,}$`, causing `invalid_arguments` when agents reply with media to DM conversations. +- Browser/Relay: treat extension websocket as connected only when `OPEN`, allow reconnect when a stale `CLOSING/CLOSED` extension socket lingers, and guard stale socket message/close handlers so late events cannot clear active relay state; includes regression coverage for live-duplicate `409` rejection and immediate reconnect-after-close races. (#15099, #18698, #20688) - Signal/RPC: guard malformed Signal RPC JSON responses with a clear status-scoped error and add regression coverage for invalid JSON responses. (#22995) Thanks @adhitShet. - Gateway/Subagents: guard gateway and subagent session-key/message trim paths against undefined inputs to prevent early `Cannot read properties of undefined (reading 'trim')` crashes during subagent spawn and wait flows. - Agents/Workspace: guard `resolveUserPath` against undefined/null input to prevent `Cannot read properties of undefined (reading 'trim')` crashes when workspace paths are missing in embedded runner flows. diff --git a/src/browser/extension-relay.test.ts b/src/browser/extension-relay.test.ts index 3464e82f34c..16da3ea8bc6 100644 --- a/src/browser/extension-relay.test.ts +++ b/src/browser/extension-relay.test.ts @@ -207,6 +207,51 @@ describe("chrome extension relay server", () => { expect(err.message).toContain("401"); }); + it("rejects a second live extension connection with 409", async () => { + const port = await getFreePort(); + cdpUrl = `http://127.0.0.1:${port}`; + await ensureChromeExtensionRelayServer({ cdpUrl }); + + const ext1 = new WebSocket(`ws://127.0.0.1:${port}/extension`, { + headers: relayAuthHeaders(`ws://127.0.0.1:${port}/extension`), + }); + await waitForOpen(ext1); + + const ext2 = new WebSocket(`ws://127.0.0.1:${port}/extension`, { + headers: relayAuthHeaders(`ws://127.0.0.1:${port}/extension`), + }); + const err = await waitForError(ext2); + expect(err.message).toContain("409"); + + ext1.close(); + }); + + it("allows immediate reconnect when prior extension socket is closing", async () => { + const port = await getFreePort(); + cdpUrl = `http://127.0.0.1:${port}`; + await ensureChromeExtensionRelayServer({ cdpUrl }); + + const ext1 = new WebSocket(`ws://127.0.0.1:${port}/extension`, { + headers: relayAuthHeaders(`ws://127.0.0.1:${port}/extension`), + }); + await waitForOpen(ext1); + const ext1Closed = new Promise((resolve) => ext1.once("close", () => resolve())); + + ext1.close(); + const ext2 = new WebSocket(`ws://127.0.0.1:${port}/extension`, { + headers: relayAuthHeaders(`ws://127.0.0.1:${port}/extension`), + }); + await waitForOpen(ext2); + await ext1Closed; + + const status = (await fetch(`${cdpUrl}/extension/status`).then((r) => r.json())) as { + connected?: boolean; + }; + expect(status.connected).toBe(true); + + ext2.close(); + }); + it("accepts extension websocket access with relay token query param", async () => { const port = await getFreePort(); cdpUrl = `http://127.0.0.1:${port}`; diff --git a/src/browser/extension-relay.ts b/src/browser/extension-relay.ts index 7d519d48b42..c9825248c8e 100644 --- a/src/browser/extension-relay.ts +++ b/src/browser/extension-relay.ts @@ -223,6 +223,7 @@ export async function ensureChromeExtensionRelayServer(opts: { let extensionWs: WebSocket | null = null; const cdpClients = new Set(); const connectedTargets = new Map(); + const extensionConnected = () => extensionWs?.readyState === WebSocket.OPEN; const pendingExtension = new Map< number, @@ -386,7 +387,7 @@ export async function ensureChromeExtensionRelayServer(opts: { if (path === "/extension/status") { res.writeHead(200, { "Content-Type": "application/json" }); - res.end(JSON.stringify({ connected: Boolean(extensionWs) })); + res.end(JSON.stringify({ connected: extensionConnected() })); return; } @@ -403,7 +404,7 @@ export async function ensureChromeExtensionRelayServer(opts: { "Protocol-Version": "1.3", }; // Only advertise the WS URL if a real extension is connected. - if (extensionWs) { + if (extensionConnected()) { payload.webSocketDebuggerUrl = cdpWsUrl; } res.writeHead(200, { "Content-Type": "application/json" }); @@ -504,10 +505,19 @@ export async function ensureChromeExtensionRelayServer(opts: { rejectUpgrade(socket, 401, "Unauthorized"); return; } - if (extensionWs) { + if (extensionConnected()) { rejectUpgrade(socket, 409, "Extension already connected"); return; } + // MV3 worker reconnect races can leave a stale non-OPEN socket reference. + if (extensionWs && extensionWs.readyState !== WebSocket.OPEN) { + try { + extensionWs.terminate(); + } catch { + // ignore + } + extensionWs = null; + } wssExtension.handleUpgrade(req, socket, head, (ws) => { wssExtension.emit("connection", ws, req); }); @@ -520,7 +530,7 @@ export async function ensureChromeExtensionRelayServer(opts: { rejectUpgrade(socket, 401, "Unauthorized"); return; } - if (!extensionWs) { + if (!extensionConnected()) { rejectUpgrade(socket, 503, "Extension not connected"); return; } @@ -544,6 +554,9 @@ export async function ensureChromeExtensionRelayServer(opts: { }, 5000); ws.on("message", (data) => { + if (extensionWs !== ws) { + return; + } let parsed: ExtensionMessage | null = null; try { parsed = JSON.parse(rawDataToString(data)) as ExtensionMessage; @@ -645,6 +658,9 @@ export async function ensureChromeExtensionRelayServer(opts: { ws.on("close", () => { clearInterval(ping); + if (extensionWs !== ws) { + return; + } extensionWs = null; for (const [, pending] of pendingExtension) { clearTimeout(pending.timer); @@ -681,7 +697,7 @@ export async function ensureChromeExtensionRelayServer(opts: { return; } - if (!extensionWs) { + if (!extensionConnected()) { sendResponseToCdp(ws, { id: cmd.id, sessionId: cmd.sessionId, @@ -779,7 +795,7 @@ export async function ensureChromeExtensionRelayServer(opts: { port, baseUrl, cdpWsUrl: `ws://${host}:${port}/cdp`, - extensionConnected: () => Boolean(extensionWs), + extensionConnected, stop: async () => { relayRuntimeByPort.delete(port); try {