mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-11 07:04:32 +00:00
fix(browser): harden extension relay reconnect race
Co-authored-by: Ho Lim <166576253+HOYALIM@users.noreply.github.com>
This commit is contained in:
@@ -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.
|
- 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)
|
- 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.
|
- 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.
|
- 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.
|
- 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.
|
- 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.
|
||||||
|
|||||||
@@ -207,6 +207,51 @@ describe("chrome extension relay server", () => {
|
|||||||
expect(err.message).toContain("401");
|
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<void>((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 () => {
|
it("accepts extension websocket access with relay token query param", async () => {
|
||||||
const port = await getFreePort();
|
const port = await getFreePort();
|
||||||
cdpUrl = `http://127.0.0.1:${port}`;
|
cdpUrl = `http://127.0.0.1:${port}`;
|
||||||
|
|||||||
@@ -223,6 +223,7 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
|||||||
let extensionWs: WebSocket | null = null;
|
let extensionWs: WebSocket | null = null;
|
||||||
const cdpClients = new Set<WebSocket>();
|
const cdpClients = new Set<WebSocket>();
|
||||||
const connectedTargets = new Map<string, ConnectedTarget>();
|
const connectedTargets = new Map<string, ConnectedTarget>();
|
||||||
|
const extensionConnected = () => extensionWs?.readyState === WebSocket.OPEN;
|
||||||
|
|
||||||
const pendingExtension = new Map<
|
const pendingExtension = new Map<
|
||||||
number,
|
number,
|
||||||
@@ -386,7 +387,7 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
|||||||
|
|
||||||
if (path === "/extension/status") {
|
if (path === "/extension/status") {
|
||||||
res.writeHead(200, { "Content-Type": "application/json" });
|
res.writeHead(200, { "Content-Type": "application/json" });
|
||||||
res.end(JSON.stringify({ connected: Boolean(extensionWs) }));
|
res.end(JSON.stringify({ connected: extensionConnected() }));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -403,7 +404,7 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
|||||||
"Protocol-Version": "1.3",
|
"Protocol-Version": "1.3",
|
||||||
};
|
};
|
||||||
// Only advertise the WS URL if a real extension is connected.
|
// Only advertise the WS URL if a real extension is connected.
|
||||||
if (extensionWs) {
|
if (extensionConnected()) {
|
||||||
payload.webSocketDebuggerUrl = cdpWsUrl;
|
payload.webSocketDebuggerUrl = cdpWsUrl;
|
||||||
}
|
}
|
||||||
res.writeHead(200, { "Content-Type": "application/json" });
|
res.writeHead(200, { "Content-Type": "application/json" });
|
||||||
@@ -504,10 +505,19 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
|||||||
rejectUpgrade(socket, 401, "Unauthorized");
|
rejectUpgrade(socket, 401, "Unauthorized");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (extensionWs) {
|
if (extensionConnected()) {
|
||||||
rejectUpgrade(socket, 409, "Extension already connected");
|
rejectUpgrade(socket, 409, "Extension already connected");
|
||||||
return;
|
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.handleUpgrade(req, socket, head, (ws) => {
|
||||||
wssExtension.emit("connection", ws, req);
|
wssExtension.emit("connection", ws, req);
|
||||||
});
|
});
|
||||||
@@ -520,7 +530,7 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
|||||||
rejectUpgrade(socket, 401, "Unauthorized");
|
rejectUpgrade(socket, 401, "Unauthorized");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (!extensionWs) {
|
if (!extensionConnected()) {
|
||||||
rejectUpgrade(socket, 503, "Extension not connected");
|
rejectUpgrade(socket, 503, "Extension not connected");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -544,6 +554,9 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
|||||||
}, 5000);
|
}, 5000);
|
||||||
|
|
||||||
ws.on("message", (data) => {
|
ws.on("message", (data) => {
|
||||||
|
if (extensionWs !== ws) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
let parsed: ExtensionMessage | null = null;
|
let parsed: ExtensionMessage | null = null;
|
||||||
try {
|
try {
|
||||||
parsed = JSON.parse(rawDataToString(data)) as ExtensionMessage;
|
parsed = JSON.parse(rawDataToString(data)) as ExtensionMessage;
|
||||||
@@ -645,6 +658,9 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
|||||||
|
|
||||||
ws.on("close", () => {
|
ws.on("close", () => {
|
||||||
clearInterval(ping);
|
clearInterval(ping);
|
||||||
|
if (extensionWs !== ws) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
extensionWs = null;
|
extensionWs = null;
|
||||||
for (const [, pending] of pendingExtension) {
|
for (const [, pending] of pendingExtension) {
|
||||||
clearTimeout(pending.timer);
|
clearTimeout(pending.timer);
|
||||||
@@ -681,7 +697,7 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!extensionWs) {
|
if (!extensionConnected()) {
|
||||||
sendResponseToCdp(ws, {
|
sendResponseToCdp(ws, {
|
||||||
id: cmd.id,
|
id: cmd.id,
|
||||||
sessionId: cmd.sessionId,
|
sessionId: cmd.sessionId,
|
||||||
@@ -779,7 +795,7 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
|||||||
port,
|
port,
|
||||||
baseUrl,
|
baseUrl,
|
||||||
cdpWsUrl: `ws://${host}:${port}/cdp`,
|
cdpWsUrl: `ws://${host}:${port}/cdp`,
|
||||||
extensionConnected: () => Boolean(extensionWs),
|
extensionConnected,
|
||||||
stop: async () => {
|
stop: async () => {
|
||||||
relayRuntimeByPort.delete(port);
|
relayRuntimeByPort.delete(port);
|
||||||
try {
|
try {
|
||||||
|
|||||||
Reference in New Issue
Block a user