From 06126a9ee786440caa68590da79c748fbb7f7168 Mon Sep 17 00:00:00 2001 From: Test User Date: Fri, 10 Apr 2026 11:10:57 +0800 Subject: [PATCH] Fix multiple bugs: XSS, operator precedence, event listener loss, race condition - Fix XSS in browserViewManager.ts: HTML-escape URLs and error messages before interpolating into innerHTML via executeJavaScript (3 locations) - Fix operator precedence bug in videoActionHandler.ts: `+` has higher precedence than `??`, making `failure_reason ?? ""` dead code. Added parentheses to correct the logic - Fix unhandled promise rejection in setContent.ts: added .catch() handler for CSS loading in iframe so content is still shown on fetch failure - Fix event listener loss in setContent.ts: replaced `innerHTML +=` with `insertAdjacentHTML("beforeend")` to avoid destroying existing DOM nodes and their event listeners - Fix race condition in webSocketAPI.ts: added `reconnecting` flag to prevent multiple concurrent reconnection attempts on rapid socket closes Co-Authored-By: Claude Opus 4.6 --- .../agents/video/src/videoActionHandler.ts | 2 +- .../shell/src/main/browserViewManager.ts | 45 +++++++++++++++++-- .../shell/src/renderer/src/setContent.ts | 6 ++- .../shell/src/renderer/src/webSocketAPI.ts | 11 ++++- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/ts/packages/agents/video/src/videoActionHandler.ts b/ts/packages/agents/video/src/videoActionHandler.ts index 162216b21b..b05562d772 100644 --- a/ts/packages/agents/video/src/videoActionHandler.ts +++ b/ts/packages/agents/video/src/videoActionHandler.ts @@ -198,7 +198,7 @@ function createVideoPlaceHolder( container.innerText = "❌ Failed to retrieve video content."; } } else { - container.innerHTML = "
❌ Video generation failed: " + statusData.failure_reason ?? "" + "
"; + container.innerHTML = "
❌ Video generation failed: " + (statusData.failure_reason ?? "") + "
"; console.log(JSON.stringify(statusData, null, 2)); } } diff --git a/ts/packages/shell/src/main/browserViewManager.ts b/ts/packages/shell/src/main/browserViewManager.ts index c62a4ff918..e4ca66cb77 100644 --- a/ts/packages/shell/src/main/browserViewManager.ts +++ b/ts/packages/shell/src/main/browserViewManager.ts @@ -196,8 +196,20 @@ export class BrowserViewManager { // only show the error if it's for the page the user was asking // it's possible some other resource failed to load (image, script, etc.) if (validatedURL === options.url) { + const safeUrl = options.url + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); + const safeDesc = errorDesc + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); webContentsView.webContents.executeJavaScript( - `document.body.innerHTML = "There was an error loading '${options.url}'.
Error Details:
${errorCode} - ${errorDesc}"`, + `document.body.innerHTML = "There was an error loading '${safeUrl}'.
Error Details:
${errorCode} - ${safeDesc}"`, ); } }, @@ -261,9 +273,21 @@ export class BrowserViewManager { `Error loading URL ${webContentsView.webContents.getURL()} in tab ${tabId}:`, err, ); - + const safeUrl = webContentsView.webContents + .getURL() + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); + const safeErr = String(err) + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); webContentsView.webContents.executeJavaScript( - `document.body.innerHTML = "There was an error loading '${webContentsView.webContents.getURL()}'.
: ${err}"`, + `document.body.innerHTML = "There was an error loading '${safeUrl}'.
: ${safeErr}"`, ); }); } else { @@ -274,8 +298,21 @@ export class BrowserViewManager { `Error loading URL ${webContentsView.webContents.getURL()} in tab ${tabId}:`, err, ); + const safeUrl = webContentsView.webContents + .getURL() + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); + const safeErr = String(err) + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); webContentsView.webContents.executeJavaScript( - `document.body.innerHTML = "There was an error loading '${webContentsView.webContents.getURL()}'.
: ${err}"`, + `document.body.innerHTML = "There was an error loading '${safeUrl}'.
: ${safeErr}"`, ); }); } diff --git a/ts/packages/shell/src/renderer/src/setContent.ts b/ts/packages/shell/src/renderer/src/setContent.ts index baa95b2516..ec5c079057 100644 --- a/ts/packages/shell/src/renderer/src/setContent.ts +++ b/ts/packages/shell/src/renderer/src/setContent.ts @@ -294,12 +294,16 @@ export function setContent( ${css} ${message}`; + }).catch((err) => { + console.error(`Failed to load CSS for iframe: ${err}`); + iframe.srcdoc = ` + ${message}`; }); contentElm.appendChild(iframe); } else { // vanilla, sanitized HTML only - contentElm.innerHTML += contentHtml; + contentElm.insertAdjacentHTML("beforeend", contentHtml); // Add click handlers for all links to open in browser tabs const allLinks = contentElm.querySelectorAll("a[href]"); diff --git a/ts/packages/shell/src/renderer/src/webSocketAPI.ts b/ts/packages/shell/src/renderer/src/webSocketAPI.ts index f73b8ee8b4..5ccb738fd3 100644 --- a/ts/packages/shell/src/renderer/src/webSocketAPI.ts +++ b/ts/packages/shell/src/renderer/src/webSocketAPI.ts @@ -99,6 +99,7 @@ export async function createWebSocket(autoReconnect: boolean = true) { const endpoint = `${protocol}://${url.hostname}${port}`; return new Promise((resolve) => { + let reconnecting = false; console.log(`opening web socket to ${endpoint} `); const webSocket = new WebSocket(endpoint); @@ -162,8 +163,14 @@ export async function createWebSocket(autoReconnect: boolean = true) { resolve(undefined); // reconnect? - if (autoReconnect) { - createWebSocket().then((ws) => (globalThis.ws = ws)); + if (autoReconnect && !reconnecting) { + reconnecting = true; + createWebSocket().then((ws) => { + if (ws) { + globalThis.ws = ws; + } + reconnecting = false; + }); } else { clientIOChannel.notifyDisconnected(); dispatcherChannel.notifyDisconnected();