refactor: desktop cleanup, shared loopback helpers, api-validation reconcile

Desktop reuse / dead-code (whole-app audit):
- driver.ts: use shared `sleep` (@sim/utils/helpers) and `getErrorMessage`
  (@sim/utils/errors) instead of local reimplementations; drop 7 banner
  separators.
- Remove dead code: LocalFilesystemService.clear(), launcher isVisible(),
  the unused `url` arg on onDownloadBlocked, and decideStartRoute's always-
  'unknown' sessionState param + its dead branch (no launch-time probe by
  design — see README).
- Extract the duplicated launcher load-failure tail into failLaunchLoad().
- handoff.ts: promote a loose comment to TSDoc.

Shared loopback helpers (@sim/security/ssrf):
- Add isLoopbackIp (full 127/8 + ::1 range) and isLoopbackHostname /
  LOOPBACK_HOSTNAMES (exact-set), consolidating three duplicated copies:
  mcp/domain-check.ts (now uses isLoopbackIp + isIpLiteral + unwrapIpv6Brackets,
  drops its ipaddr import), connectors/s3.ts, and desktop config.ts /
  navigation.ts (drop the local LOCAL_HOSTNAMES set). urls.ts keeps its own
  client-side set to avoid pulling ipaddr.js into client bundles.

api-validation:strict reconcile (pre-existing dev drift, not from these changes):
- Allowlist speech/tts (raw-read streaming route that validates inline) in
  INDIRECT_ZOD_ROUTES; annotate the launcher's validated-envelope double-cast.
  Bump the route-count baseline 964→966 for dev's two already-merged speech
  routes. Non-Zod and double-cast ratchets stay at baseline.

Author: waleedlatif1

apps/desktop/src/main/browser-agent/driver.ts

@@ -22,6 +22,8 @@ import type {
   BrowserToolName,
 } from '@sim/browser-protocol'
 import { createLogger } from '@sim/logger'
+import { getErrorMessage } from '@sim/utils/errors'
+import { sleep } from '@sim/utils/helpers'
 import type { BrowserWindow, WebContents } from 'electron'
 import * as cdp from '@/main/browser-agent/cdp'
 import {
@@ -119,7 +121,7 @@ function instrumentTab(contents: WebContents): void {
     })
     .catch((error) => {
       logger.warn('CDP instrumentation failed', {
-        error: error instanceof Error ? error.message : String(error),
+        error: getErrorMessage(error),
       })
     })
   for (const event of [
@@ -161,10 +163,6 @@ export function setPanelBounds(bounds: BrowserPanelBounds | null): void {
   session.setPanelBounds(bounds)
 }
 
-function sleep(ms: number): Promise<void> {
-  return new Promise((resolve) => setTimeout(resolve, ms))
-}
-
 function str(params: Record<string, unknown>, key: string): string | undefined {
   const value = params[key]
   return typeof value === 'string' && value.length > 0 ? value : undefined
@@ -192,10 +190,6 @@ function requireNum(params: Record<string, unknown>, key: string): number {
   return value
 }
 
-// ---------------------------------------------------------------------------
-// Page-function execution
-// ---------------------------------------------------------------------------
-
 /**
  * Serializes a self-contained page function and executes it in the page's
  * main world with JSON-encoded arguments (Electron's executeJavaScript has no
@@ -210,7 +204,7 @@ async function execInPage<Args extends unknown[], Result>(
   try {
     return (await contents.executeJavaScript(expression, true)) as Result
   } catch (error) {
-    const message = error instanceof Error ? error.message : String(error)
+    const message = getErrorMessage(error)
     throw new ToolError(
       `Cannot act on this page (${message}). Browser-internal pages cannot be automated — ` +
         'navigate to a regular website first.'
@@ -250,10 +244,6 @@ function unwrapPageResult(result: unknown): unknown {
   return result
 }
 
-// ---------------------------------------------------------------------------
-// Navigation
-// ---------------------------------------------------------------------------
-
 function waitForLoadComplete(contents: WebContents, timeoutMs: number): Promise<void> {
   return new Promise((resolve) => {
     let settled = false
@@ -279,10 +269,6 @@ async function navigationResult(contents: WebContents): Promise<Record<string, u
   return { url: contents.getURL(), title: contents.getTitle() }
 }
 
-// ---------------------------------------------------------------------------
-// Key parsing for browser_press_key
-// ---------------------------------------------------------------------------
-
 interface KeyDescriptor {
   key: string
   code: string
@@ -350,10 +336,6 @@ export function parseKeyCombo(combo: string): ParsedCombo {
   throw new ToolError(`Unrecognized key: "${keyPart}"`)
 }
 
-// ---------------------------------------------------------------------------
-// Trusted key dispatch (CDP Input.dispatchKeyEvent)
-// ---------------------------------------------------------------------------
-
 /** CDP `Input` modifier bitmask: Alt=1, Ctrl=2, Meta=4, Shift=8. */
 function cdpModifiers(combo: ParsedCombo): number {
   return (combo.alt ? 1 : 0) | (combo.ctrl ? 2 : 0) | (combo.meta ? 4 : 0) | (combo.shift ? 8 : 0)
@@ -445,10 +427,6 @@ async function activeElementState(contents: WebContents): Promise<Record<string,
   return typeof state === 'object' && state !== null ? (state as Record<string, unknown>) : {}
 }
 
-// ---------------------------------------------------------------------------
-// Takeover
-// ---------------------------------------------------------------------------
-
 /**
  * Hands control to the user: the page is already natively interactive in the
  * panel, and the chat's takeover tool row shows the reason with a Done chip.
@@ -482,10 +460,6 @@ async function runTakeover(): Promise<unknown> {
   }
 }
 
-// ---------------------------------------------------------------------------
-// Tool implementations
-// ---------------------------------------------------------------------------
-
 async function executeToolInner(
   tool: BrowserToolName,
   params: Record<string, unknown>
@@ -728,10 +702,6 @@ function withNotices(result: unknown): unknown {
   return { value: result, notices }
 }
 
-// ---------------------------------------------------------------------------
-// Public entry points
-// ---------------------------------------------------------------------------
-
 /** One real browser can only do one thing at a time — serialize tool calls. */
 let toolQueue: Promise<unknown> = Promise.resolve()
 
@@ -761,7 +731,7 @@ export async function executeTool(
   try {
     return { ok: true, result: await settled }
   } catch (error) {
-    const message = error instanceof Error ? error.message : String(error)
+    const message = getErrorMessage(error)
     logger.warn('Browser tool failed', { tool, error: message })
     return { ok: false, error: message }
   }

apps/desktop/src/main/browser-agent/session.ts

@@ -25,7 +25,7 @@ export interface AgentSessionEvents {
   /** The active tab changed (new tab, switch, close). */
   onActiveTabChanged: (contents: WebContents) => void
   /** A download was blocked on the agent partition. */
-  onDownloadBlocked: (filename: string, url: string) => void
+  onDownloadBlocked: (filename: string) => void
 }
 
 /**
@@ -98,10 +98,9 @@ function configureAgentPartition(ses: Session): void {
   })
   ses.on('will-download', (_event, item) => {
     const filename = item.getFilename()
-    const url = item.getURL()
     logger.info('Blocked download in agent browser', { filename })
     item.cancel()
-    events?.onDownloadBlocked(filename, url)
+    events?.onDownloadBlocked(filename)
   })
 }
 

apps/desktop/src/main/config.ts

@@ -1,18 +1,12 @@
 import { mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs'
 import { dirname } from 'node:path'
 import { createLogger } from '@sim/logger'
+import { isLoopbackHostname } from '@sim/security/ssrf'
 
 const logger = createLogger('DesktopConfig')
 
 export const DEFAULT_ORIGIN = 'https://sim.ai'
 
-/**
- * Loopback hostnames that may use plain HTTP (dev + self-host testing). Matched
- * against a parsed `URL.hostname`, which brackets IPv6 authorities — so `[::1]`
- * is the form that appears, never a bare `::1`.
- */
-export const LOCAL_HOSTNAMES = new Set(['localhost', '127.0.0.1', '[::1]'])
-
 export interface WindowBounds {
   x?: number
   y?: number
@@ -56,7 +50,7 @@ export function validateOriginInput(raw: string): OriginValidation {
   if (url.protocol === 'https:') {
     return { ok: true, origin: url.origin }
   }
-  if (url.protocol === 'http:' && LOCAL_HOSTNAMES.has(url.hostname)) {
+  if (url.protocol === 'http:' && isLoopbackHostname(url.hostname)) {
     return { ok: true, origin: url.origin }
   }
   return { ok: false, error: 'Server URL must use HTTPS (HTTP is allowed for localhost only)' }

apps/desktop/src/main/handoff.test.ts

@@ -106,16 +106,17 @@ describe('createHandoffManager', () => {
     expect(received).toHaveLength(0)
     expect(manager.consume(wrongState)).toBe(false)
 
-    // The genuine callback still lands while the server is up.
+    // The genuine callback lands and immediately tears the listener down, so a
+    // refresh/retry of the callback URL can't fire a second handleCallback.
     const ok = await fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`)
     expect(ok.status).toBe(200)
-    expect(received).toContainEqual({ token: VALID_TOKEN, state })
-
-    // Consuming the real state validates and closes the loopback.
-    expect(manager.consume(state)).toBe(true)
+    expect(received).toEqual([{ token: VALID_TOKEN, state }])
     await expect(
       fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`)
     ).rejects.toThrow()
+
+    // consume() still validates the pending state for the redeem path.
+    expect(manager.consume(state)).toBe(true)
   })
 
   it('cleans up the pending handoff when the browser cannot be opened', async () => {

apps/desktop/src/main/handoff.ts

@@ -14,10 +14,12 @@ const STATE_PATTERN = /^[A-Za-z0-9_-]{16,256}$/
 const STATE_LENGTH = 32
 const REDEEM_PATH = '/api/auth/one-time-token/verify'
 const CALLBACK_PATH = '/auth/callback'
-// Measured from begin() (when the browser opens) so it comfortably covers a
-// full interactive login — email/OTP round-trips or OAuth consent — not just
-// the redirect back. Bounds how long the loopback listener and the CSRF state
-// stay valid.
+/**
+ * Measured from begin() (when the browser opens) so it comfortably covers a
+ * full interactive login — email/OTP round-trips or OAuth consent — not just
+ * the redirect back. Bounds how long the loopback listener and the CSRF state
+ * stay valid.
+ */
 const HANDOFF_TTL_MS = 30 * 60 * 1000
 
 const CALLBACK_RESPONSE_HTML = `<!doctype html>
@@ -96,11 +98,14 @@ export function createHandoffManager(
         .writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' })
         .end(CALLBACK_RESPONSE_HTML)
       // Only a state match advances the flow. A format-valid but wrong/expired
-      // state gets the generic 200 page and is otherwise ignored: the one-shot
-      // listener survives for the genuine callback (TTL is the backstop) and no
-      // spurious "sign-in failed" UI fires. `consume()` in the callback path
-      // re-validates and tears the server down once the real state lands.
+      // state gets the generic 200 page and is otherwise ignored, so the
+      // one-shot listener survives for the genuine callback (TTL is the
+      // backstop) and no spurious "sign-in failed" UI fires. A genuine match
+      // tears the listener down immediately — before the async redeem — so a
+      // refresh/retry of the callback URL can't fire a second handleCallback
+      // whose consume() would fail and surface a false "sign-in failed".
       if (matchesPendingState(state)) {
+        stopLoopback()
         onCallback({ token, state })
       }
     })

apps/desktop/src/main/index.ts

@@ -140,7 +140,7 @@ function main(): void {
     })
     loadHealth = attachLoadHealth(win, {
       offlinePagePath: OFFLINE_PAGE,
-      getStartUrl: () => `${appOrigin()}${decideStartRoute('unknown', config.get('lastRoute'))}`,
+      getStartUrl: () => `${appOrigin()}${decideStartRoute(config.get('lastRoute'))}`,
       isOnline: () => net.isOnline(),
       events,
     })
@@ -155,7 +155,7 @@ function main(): void {
       onReauthRequested: () => void authFlow.beginLoginHandoff(),
     })
     loadHealth.startWatchdog()
-    const route = decideStartRoute('unknown', config.get('lastRoute'))
+    const route = decideStartRoute(config.get('lastRoute'))
     // Fire-and-forget: the window and all its handlers are wired synchronously
     // above, so callers get a usable window immediately and the app menu and
     // updater never wait on the remote page's load (load-health surfaces any

apps/desktop/src/main/launcher-window.ts

@@ -62,7 +62,6 @@ export interface LauncherWindowHandle {
   prewarm(): void
   /** Renderer-driven growth as a response streams in; clamped. */
   resize(height: number): void
-  isVisible(): boolean
   /** Tears the window down (origin change, quit). Next toggle recreates it. */
   destroy(): void
 }
@@ -116,6 +115,16 @@ export function createLauncherWindow(deps: LauncherWindowDeps): LauncherWindowHa
       }
     })
 
+    // A failed launcher load hides the panel and falls back to the main window.
+    const failLaunchLoad = (code: number, reason: string) => {
+      deps.events.record('launcher_load_failed', { code, reason })
+      loadFailed = true
+      if (panel.isVisible()) {
+        panel.hide()
+        deps.openMainWindow()
+      }
+    }
+
     // did-fail-load covers network-level failures (offline); an HTTP error
     // status (older self-hosted server without the route) still "loads", so
     // it is caught from the navigation's response code instead.
@@ -124,26 +133,17 @@ export function createLauncherWindow(deps: LauncherWindowDeps): LauncherWindowHa
         return
       }
       logger.warn('Launcher route failed to load', { code, description, url: scrubUrl(url) })
-      deps.events.record('launcher_load_failed', { code, reason: description })
-      loadFailed = true
-      if (panel.isVisible()) {
-        panel.hide()
-        deps.openMainWindow()
-      }
+      failLaunchLoad(code, description)
     })
     panel.webContents.on('did-navigate', (_event, url, httpResponseCode) => {
-      if (httpResponseCode >= 400) {
-        logger.warn('Launcher route returned an error status', {
-          status: httpResponseCode,
-          url: scrubUrl(url),
-        })
-        deps.events.record('launcher_load_failed', { code: httpResponseCode, reason: 'http' })
-        loadFailed = true
-        if (panel.isVisible()) {
-          panel.hide()
-          deps.openMainWindow()
-        }
+      if (httpResponseCode < 400) {
+        return
       }
+      logger.warn('Launcher route returned an error status', {
+        status: httpResponseCode,
+        url: scrubUrl(url),
+      })
+      failLaunchLoad(httpResponseCode, 'http')
     })
     panel.webContents.on('render-process-gone', (_event, details) => {
       if (details.reason !== 'clean-exit') {
@@ -212,9 +212,6 @@ export function createLauncherWindow(deps: LauncherWindowDeps): LauncherWindowHa
       const bounds = win.getBounds()
       win.setBounds({ ...bounds, height: clampLauncherHeight(height) })
     },
-    isVisible() {
-      return Boolean(win && !win.isDestroyed() && win.isVisible())
-    },
     destroy() {
       if (win && !win.isDestroyed()) {
         win.destroy()

apps/desktop/src/main/local-filesystem.test.ts

@@ -137,9 +137,9 @@ describe('LocalFilesystemService', () => {
     expect(traversal).toMatchObject({ ok: false, code: 'ACCESS_DENIED' })
   })
 
-  it('clears all grants without touching files on disk', async () => {
+  it('releases active mounts without touching files on disk', async () => {
     const granted = await mount(service)
-    service.clear()
+    service.close()
 
     const response = await service.handle({ operation: 'stat', uri: granted.uri })
     expect(response).toMatchObject({ ok: false, code: 'MOUNT_NOT_FOUND' })

apps/desktop/src/main/local-filesystem.ts

@@ -206,10 +206,6 @@ export class LocalFilesystemService {
     return (this.initializePromise ??= this.restoreRememberedMounts())
   }
 
-  clear(): void {
-    this.close()
-  }
-
   /** Release active OS handles while keeping encrypted grants for next launch. */
   close(): void {
     for (const mount of this.mounts.values()) {

apps/desktop/src/main/navigation.ts

@@ -1,6 +1,6 @@
 import { createLogger } from '@sim/logger'
+import { isLoopbackHostname } from '@sim/security/ssrf'
 import { shell } from 'electron'
-import { LOCAL_HOSTNAMES } from '@/main/config'
 
 const logger = createLogger('DesktopNavigation')
 
@@ -207,7 +207,7 @@ export function isSafeExternalUrl(raw: string, allowHttpLocalhost = false): bool
     return true
   }
   if (url.protocol === 'http:') {
-    return allowHttpLocalhost && LOCAL_HOSTNAMES.has(url.hostname)
+    return allowHttpLocalhost && isLoopbackHostname(url.hostname)
   }
   return false
 }