From 9457651fdd99b85be4269c687ab8f734c3978e01 Mon Sep 17 00:00:00 2001 From: Simos Mikelatos Date: Wed, 24 Jun 2026 15:36:25 +0000 Subject: [PATCH] fix(browser-use): harden browser settings state --- server/browser-use-mcp.ts | 2 +- server/index.js | 1 + .../browser-use/browser-use.service.ts | 58 ++++++++++++++++++- .../browser-use/browser-use.settings.ts | 48 ++++++++++++--- .../modules/browser-use/browser-use.viewer.ts | 9 ++- .../tests/browser-use.settings.test.ts | 57 ++++++++++++++++++ .../services/websocket-server.service.ts | 4 +- .../browser-use/view/BrowserUsePanel.tsx | 1 - .../BrowserUseSettingsTab.tsx | 27 ++++++--- 9 files changed, 180 insertions(+), 27 deletions(-) create mode 100644 server/modules/browser-use/tests/browser-use.settings.test.ts diff --git a/server/browser-use-mcp.ts b/server/browser-use-mcp.ts index d1ece36a..b83b6c05 100644 --- a/server/browser-use-mcp.ts +++ b/server/browser-use-mcp.ts @@ -69,7 +69,7 @@ const sessionIdSchema = { const tools: ToolDefinition[] = [ { name: 'browser_create_session', - description: 'Create a Browser session that the agent can control. Uses the configured persistent profile by default; optionally provide profileName to override it.', + description: 'Create a Browser session that the agent can control. Provide profileName to use a specific persistent profile; when omitted, the configured persistent profile is used only if session persistence is enabled, otherwise a temporary session is created.', inputSchema: { type: 'object', properties: { diff --git a/server/index.js b/server/index.js index df1a4224..80cc0f18 100755 --- a/server/index.js +++ b/server/index.js @@ -238,6 +238,7 @@ function authenticateBrowserUse(req, res, next) { if (browserUseService.validateViewerToken(sessionId, token)) { return next(); } + return res.status(401).json({ error: 'Browser viewer access requires a valid session token.' }); } return authenticateToken(req, res, next); } diff --git a/server/modules/browser-use/browser-use.service.ts b/server/modules/browser-use/browser-use.service.ts index efcf8da3..ec440da0 100644 --- a/server/modules/browser-use/browser-use.service.ts +++ b/server/modules/browser-use/browser-use.service.ts @@ -219,6 +219,54 @@ function delay(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } +function isRuntimeProcessAlive(child: ReturnType): boolean { + return child.exitCode === null && child.signalCode === null && !child.killed; +} + +function assertRuntimeProcessesAlive(processes: Array>, label: string) { + const exited = processes.find((child) => !isRuntimeProcessAlive(child)); + if (exited) { + throw new Error(`${label} exited before the Browser viewer runtime was ready.`); + } +} + +async function isPortListening(port: number): Promise { + return new Promise((resolve) => { + const socket = net.createConnection({ host: '127.0.0.1', port }); + let settled = false; + const finish = (listening: boolean) => { + if (settled) { + return; + } + settled = true; + socket.destroy(); + resolve(listening); + }; + socket.setTimeout(250); + socket.once('connect', () => finish(true)); + socket.once('timeout', () => finish(false)); + socket.once('error', () => finish(false)); + }); +} + +async function waitForRuntimePort( + port: number, + label: string, + processes: Array>, + timeoutMs = 5_000, +) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + assertRuntimeProcessesAlive(processes, label); + if (await isPortListening(port)) { + return; + } + await delay(100); + } + assertRuntimeProcessesAlive(processes, label); + throw new Error(`${label} did not start listening on 127.0.0.1:${port}.`); +} + function killRuntimeProcesses(processes?: Array>) { processes?.forEach((child) => child.kill('SIGTERM')); } @@ -272,6 +320,7 @@ async function startVisibleRuntime(): Promise entry.isDirectory() && normalizeProfileName(entry.name) === normalizedProfileName); + return match?.name || null; + } catch { + return null; + } } export function useVisibleCamoufoxBackend(settings: BrowserUseSettings): boolean { diff --git a/server/modules/browser-use/browser-use.viewer.ts b/server/modules/browser-use/browser-use.viewer.ts index 5ddb7ec2..7714af18 100644 --- a/server/modules/browser-use/browser-use.viewer.ts +++ b/server/modules/browser-use/browser-use.viewer.ts @@ -5,10 +5,15 @@ import type { RuntimeHandle } from './browser-use.types.js'; type BrowserUseViewer = NonNullable; export const VIEWER_COOKIE_NAME = 'browser_use_viewer_token'; -export const VIEWER_TOKEN_TTL_MS = Number.parseInt( - process.env.CLOUDCLI_BROWSER_USE_VIEWER_TOKEN_TTL_MS || String(30 * 60 * 1000), +const DEFAULT_VIEWER_TOKEN_TTL_MS = 30 * 60 * 1000; +const parsedViewerTokenTtlMs = Number.parseInt( + process.env.CLOUDCLI_BROWSER_USE_VIEWER_TOKEN_TTL_MS || String(DEFAULT_VIEWER_TOKEN_TTL_MS), 10, ); +export const VIEWER_TOKEN_TTL_MS = + Number.isFinite(parsedViewerTokenTtlMs) && parsedViewerTokenTtlMs > 0 + ? parsedViewerTokenTtlMs + : DEFAULT_VIEWER_TOKEN_TTL_MS; export function getViewerUrl(sessionId: string, viewerToken?: string): string { const basePath = `/api/browser-use/sessions/${encodeURIComponent(sessionId)}/viewer`; diff --git a/server/modules/browser-use/tests/browser-use.settings.test.ts b/server/modules/browser-use/tests/browser-use.settings.test.ts new file mode 100644 index 00000000..a814e1d3 --- /dev/null +++ b/server/modules/browser-use/tests/browser-use.settings.test.ts @@ -0,0 +1,57 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import test from 'node:test'; + +import { + getProfilePath, + normalizeDefaultProfileName, + normalizeProfileName, + PROFILE_ROOT, + resolveSessionProfileName, +} from '@/modules/browser-use/browser-use.settings.js'; + +test('browser profile names are canonicalized before storage and path resolution', () => { + assert.equal(normalizeProfileName(' Work Profile!! '), 'work-profile'); + assert.equal(normalizeDefaultProfileName(' Work Profile!! '), 'work-profile'); + assert.equal( + getProfilePath(' Work Profile!! '), + `${PROFILE_ROOT}/work-profile`, + ); + assert.equal( + resolveSessionProfileName({ + enabled: true, + persistSessions: true, + defaultProfileName: ' Work Profile!! ', + browserBackend: 'playwright', + }), + 'work-profile', + ); +}); + +test('browser profile aliases are rejected when the normalized profile already exists', () => { + const profileName = `alias-test-${Date.now()}`; + fs.mkdirSync(getProfilePath(profileName), { recursive: true }); + + try { + assert.throws( + () => resolveSessionProfileName({ + enabled: true, + persistSessions: false, + defaultProfileName: 'default', + browserBackend: 'playwright', + }, profileName.toUpperCase()), + /resolves to existing profile/, + ); + assert.equal( + resolveSessionProfileName({ + enabled: true, + persistSessions: false, + defaultProfileName: 'default', + browserBackend: 'playwright', + }, profileName), + profileName, + ); + } finally { + fs.rmSync(getProfilePath(profileName), { recursive: true, force: true }); + } +}); diff --git a/server/modules/websocket/services/websocket-server.service.ts b/server/modules/websocket/services/websocket-server.service.ts index b6dbbff1..246bd49e 100644 --- a/server/modules/websocket/services/websocket-server.service.ts +++ b/server/modules/websocket/services/websocket-server.service.ts @@ -48,9 +48,7 @@ export function createWebSocketServer( && requestUrl.pathname.endsWith('/viewer/websockify') ) { const token = getBrowserUseViewerToken(requestUrl, info.req.headers as Record); - if (dependencies.authenticateBrowserUseViewer?.(requestUrl.pathname, token)) { - return true; - } + return Boolean(dependencies.authenticateBrowserUseViewer?.(requestUrl.pathname, token)); } return verifyWebSocketClient(info, dependencies.verifyClient); }), diff --git a/src/components/browser-use/view/BrowserUsePanel.tsx b/src/components/browser-use/view/BrowserUsePanel.tsx index e887f804..10ba9ce4 100644 --- a/src/components/browser-use/view/BrowserUsePanel.tsx +++ b/src/components/browser-use/view/BrowserUsePanel.tsx @@ -277,7 +277,6 @@ export default function BrowserUsePanel({ isVisible, projectId, onShowSettings } browserUsePanelCache.set(cacheKey, { ...cachedEntry, selectedSessionId, - updatedAt: Date.now(), }); }, [cacheKey, selectedSessionId]); diff --git a/src/components/settings/view/tabs/browser-use-settings/BrowserUseSettingsTab.tsx b/src/components/settings/view/tabs/browser-use-settings/BrowserUseSettingsTab.tsx index 90c93581..42589910 100644 --- a/src/components/settings/view/tabs/browser-use-settings/BrowserUseSettingsTab.tsx +++ b/src/components/settings/view/tabs/browser-use-settings/BrowserUseSettingsTab.tsx @@ -42,6 +42,7 @@ async function readJson(response: Response): Promise { export default function BrowserUseSettingsTab() { const [settings, setSettings] = useState(null); const [status, setStatus] = useState(null); + const [hasLoadedSettings, setHasLoadedSettings] = useState(false); const [isSettingsLoading, setIsSettingsLoading] = useState(true); const [isStatusLoading, setIsStatusLoading] = useState(true); const [isSaving, setIsSaving] = useState(false); @@ -53,6 +54,7 @@ export default function BrowserUseSettingsTab() { const settingsResponse = await authenticatedFetch('/api/browser-use/settings'); const settingsData = await readJson<{ data: { settings: BrowserUseSettings } }>(settingsResponse); setSettings(settingsData.data.settings); + setHasLoadedSettings(true); setProfileNameDraft(settingsData.data.settings.defaultProfileName || 'default'); }, []); @@ -64,6 +66,7 @@ export default function BrowserUseSettingsTab() { useEffect(() => { setError(null); + setHasLoadedSettings(false); setIsSettingsLoading(true); setIsStatusLoading(true); @@ -86,6 +89,7 @@ export default function BrowserUseSettingsTab() { }); const data = await readJson<{ data: { settings: BrowserUseSettings } }>(response); setSettings(data.data.settings); + setHasLoadedSettings(true); window.dispatchEvent(new Event('browserUseSettingsChanged')); setIsStatusLoading(true); await loadStatus(); @@ -123,6 +127,7 @@ export default function BrowserUseSettingsTab() { }; const browserEnabled = settings?.enabled === true; + const browserDisabled = hasLoadedSettings && settings?.enabled === false; const persistSessions = settings?.persistSessions === true; const selectedBackend = settings?.browserBackend || 'playwright'; const effectiveBackend = status?.backend || 'playwright'; @@ -145,19 +150,21 @@ export default function BrowserUseSettingsTab() { label="Give Agents Browser Access" description="Let agents use a browser during coding tasks while you can watch live sessions, open them in a tab, and stop them at any time." > - {isSettingsLoading && !settings ? ( + {isSettingsLoading && !hasLoadedSettings ? ( - ) : ( + ) : hasLoadedSettings ? ( void updateSettings({ enabled: value })} ariaLabel="Give Agents Browser Access" disabled={isSaving} /> + ) : ( + Unavailable )} - {!browserEnabled && ( + {browserDisabled && (