feat(desktop): SSRF hardening for the browser agent + review fixes

Shared SSRF primitive:
- Extract IP classification into @sim/security/ssrf (isPrivateIp,
  isPrivateIpHost, isIpLiteral, unwrapIpv6Brackets). Collapses the two
  divergent isPrivateOrReservedIP copies in apps/sim and the ad-hoc regex
  copies in sap_concur/zoominfo/sap contracts into one audited, ipaddr.js-based
  implementation consumed by both apps. All callers migrated; no logic left
  behind.

Browser-agent SSRF guard (apps/desktop):
- The agent partition's onBeforeRequest is now the authoritative choke point:
  document navigations (top-level + iframes) get a full DNS-resolving check —
  covering page-initiated navigation the driver never sees (redirects, link
  clicks, location.href, meta-refresh) — and subresources get a cheap
  synchronous literal-IP backstop.
- browser_navigate / browser_open_tab validate up front so the model gets a
  clean error instead of a silent load failure.
- Blocks loopback, RFC1918, link-local/cloud-metadata (169.254.169.254), and
  obfuscated/IPv4-mapped forms; hostnames are resolved (DNS-rebinding aware).

Other hardening:
- crashReporter: local-only native minidumps (no backend, uploadToServer:false),
  crash-dump dir recorded in the event log.
- CSP fallback: a minimal, non-drifting policy injected only when an app-origin
  document response ships no CSP of its own.
- window.open (MCP OAuth) gated to https so a renderer-controlled frame name
  can't ride an http URL into an in-app window.
- updater: shared configureAutoUpdater re-asserts channel/allowDowngrade on both
  the launch and manual-check paths.
- handoff: the loopback listener now tears down only after the CSRF state
  validates, not on any format-valid request (self-DoS guard).
- cdp: per-WebContents callbacks so a background tab's events reach its own
  driver.
- config: drop the unreachable bare '::1' loopback entry.
- README: correct the App Sandbox / security-scoped-bookmark note for the
  Developer ID build.

Tests: @sim/security/ssrf (67), desktop suite (200), affected apps/sim suites
green; type-check + biome clean across desktop, security, and touched sim files.

Author: waleedlatif1

apps/desktop/README.md

@@ -137,7 +137,7 @@ Good fits for the bridge: OS notifications + dock badge on workflow completion,
 The main Copilot agent can inspect a user-selected local directory in the desktop app through request-local client tools (`local_mount_directory`, `local_list_mounts`, `local_list`, `local_glob`, `local_read`, `local_grep`, `local_stat`, and `local_forget_mount`). This capability is:
 
 - **Explicit and read-only:** the native folder picker creates the grant; there are no write/delete/execute operations.
-- **Remembered securely:** grants are encrypted in Electron's private app data with OS-backed `safeStorage` and restored with the same opaque URI after a normal app restart. Sandboxed macOS builds also retain the security-scoped bookmark. There is no plaintext fallback: when secure storage is unavailable, the returned mount has `remembered: false` and lasts only for that app session.
+- **Remembered securely:** grants are encrypted in Electron's private app data with OS-backed `safeStorage` and restored with the same opaque URI after a normal app restart. (A security-scoped bookmark is stored alongside each grant, but it is a no-op in the current Developer ID build — only the macOS App Sandbox consumes it — and is kept purely for forward-compatibility should a sandboxed/MAS build ever ship.) There is no plaintext fallback: when secure storage is unavailable, the returned mount has `remembered: false` and lasts only for that app session.
 - **Revocable:** `local_forget_mount` removes one grant. All grants are removed on explicit sign-out or server-origin change so another Sim account or server cannot inherit them. Normal app quit only releases active OS handles and keeps the encrypted grants.
 - **Opaque:** renderer and model see only `localfs://<mount-id>/...` URIs. Electron resolves every URI, checks lexical and realpath containment, and refuses symlink escapes.
 - **Desktop-only:** the web app advertises these request-local tools only when `window.simDesktop.localFilesystem` is present. They are available directly to the main agent and are not added to subagent allowlists.

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

@@ -27,7 +27,9 @@ export interface CdpCallbacks {
   onFileChooser: () => void
 }
 
-let callbacks: CdpCallbacks | null = null
+/** Per-tab callbacks, so a background tab's events reach ITS driver, not the
+ * most-recently-instrumented tab's. */
+const callbacksByContents = new WeakMap<WebContents, CdpCallbacks>()
 /** Contents already instrumented (attach survives for the tab's lifetime). */
 const instrumented = new WeakSet<WebContents>()
 
@@ -41,7 +43,7 @@ async function send<T = unknown>(
 
 /** Idempotently instruments a tab's WebContents. */
 export async function ensureInstrumented(contents: WebContents, cb: CdpCallbacks): Promise<void> {
-  callbacks = cb
+  callbacksByContents.set(contents, cb)
   if (instrumented.has(contents) && contents.debugger.isAttached()) return
 
   if (!contents.debugger.isAttached()) {
@@ -65,6 +67,7 @@ function handleDebuggerEvent(
   method: string,
   params: Record<string, unknown>
 ): void {
+  const callbacks = callbacksByContents.get(contents)
   if (method === 'Page.javascriptDialogOpening') {
     const type = String(params.type ?? 'dialog')
     const message = String(params.message ?? '').slice(0, 500)

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

@@ -39,6 +39,7 @@ import {
   typeIntoElement,
 } from '@/main/browser-agent/page-functions'
 import * as session from '@/main/browser-agent/session'
+import { checkAgentUrl } from '@/main/browser-agent/url-guard'
 
 const logger = createLogger('BrowserAgentDriver')
 
@@ -492,8 +493,9 @@ async function executeToolInner(
   switch (tool) {
     case 'browser_navigate': {
       const url = requireStr(params, 'url')
-      if (!/^https?:\/\//i.test(url)) {
-        throw new ToolError('URL must be absolute and start with http:// or https://')
+      const guard = await checkAgentUrl(url)
+      if (!guard.ok) {
+        throw new ToolError(guard.error ?? 'That address was blocked.')
       }
       const tab = session.ensureTab()
       const contents = tab.view.webContents
@@ -521,12 +523,15 @@ async function executeToolInner(
 
     case 'browser_open_tab': {
       const url = str(params, 'url')
+      if (url) {
+        const guard = await checkAgentUrl(url)
+        if (!guard.ok) {
+          throw new ToolError(guard.error ?? 'That address was blocked.')
+        }
+      }
       const tab = session.addTab()
       const contents = tab.view.webContents
       if (url) {
-        if (!/^https?:\/\//i.test(url)) {
-          throw new ToolError('URL must be absolute and start with http:// or https://')
-        }
         void contents.loadURL(url).catch(() => {})
         const result = await navigationResult(contents)
         return { tabId: tab.id, ...result }

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

@@ -2,6 +2,13 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 vi.mock('electron', () => import('@/test/electron-mock'))
 
+vi.mock('@/main/browser-agent/url-guard', () => ({
+  checkAgentUrl: vi.fn(async (url: string) => ({
+    ok: /^https?:\/\//i.test(url) && !url.includes('169.254.169.254'),
+  })),
+  isBlockedRequestUrl: vi.fn((url: string) => url.includes('169.254.169.254')),
+}))
+
 import { BrowserWindow } from 'electron'
 
 type SessionModule = typeof import('@/main/browser-agent/session')
@@ -11,6 +18,7 @@ interface MockView {
     session: {
       setPermissionRequestHandler: ReturnType<typeof vi.fn>
       setPermissionCheckHandler: ReturnType<typeof vi.fn>
+      webRequest: { onBeforeRequest: ReturnType<typeof vi.fn> }
     }
     setWindowOpenHandler: ReturnType<typeof vi.fn>
     loadURL: ReturnType<typeof vi.fn>
@@ -133,6 +141,8 @@ describe('browser-agent session', () => {
     const openHandler = contents.setWindowOpenHandler.mock.calls[0][0] as (details: {
       url: string
     }) => { action: string }
+    // window.open collapses into the same view; the SSRF check runs on the
+    // resulting document request in onBeforeRequest, not here.
     expect(openHandler({ url: 'https://example.com/popup' })).toEqual({ action: 'deny' })
     expect(contents.loadURL).toHaveBeenCalledWith('https://example.com/popup')
     // Non-http(s) popups are denied without navigating anywhere.
@@ -141,6 +151,38 @@ describe('browser-agent session', () => {
     expect(contents.loadURL).not.toHaveBeenCalled()
   })
 
+  it('gates agent-partition requests: DNS-checks documents, literal-checks subresources', async () => {
+    const tab = session.ensureTab()
+    const contents = (tab.view as unknown as MockView).webContents
+    const onBeforeRequest = contents.session.webRequest.onBeforeRequest.mock.calls[0][0] as (
+      details: { url: string; resourceType: string },
+      callback: (response: { cancel?: boolean }) => void
+    ) => void
+
+    // A document navigation to a private host is cancelled by the DNS check.
+    const blockedDoc = vi.fn()
+    onBeforeRequest(
+      { url: 'http://169.254.169.254/latest/meta-data', resourceType: 'mainFrame' },
+      blockedDoc
+    )
+    await vi.waitFor(() => expect(blockedDoc).toHaveBeenCalledWith({ cancel: true }))
+
+    // A document navigation to a public host is allowed.
+    const allowedDoc = vi.fn()
+    onBeforeRequest({ url: 'https://example.com/', resourceType: 'subFrame' }, allowedDoc)
+    await vi.waitFor(() => expect(allowedDoc).toHaveBeenCalledWith({ cancel: false }))
+
+    // A subresource to a literal private IP is cancelled synchronously.
+    const blockedSub = vi.fn()
+    onBeforeRequest({ url: 'http://169.254.169.254/x.js', resourceType: 'image' }, blockedSub)
+    expect(blockedSub).toHaveBeenCalledWith({ cancel: true })
+
+    // A public subresource passes.
+    const allowedSub = vi.fn()
+    onBeforeRequest({ url: 'https://example.com/x.js', resourceType: 'script' }, allowedSub)
+    expect(allowedSub).toHaveBeenCalledWith({ cancel: false })
+  })
+
   it('permission handlers deny every request on the agent partition', () => {
     const tab = session.ensureTab()
     const ses = (tab.view as unknown as MockView).webContents.session

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

@@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger'
 import type { BrowserWindow, Session, WebContents } from 'electron'
 import { WebContentsView } from 'electron'
 import { registerAgentWebContents } from '@/main/browser-agent/registry'
+import { checkAgentUrl, isBlockedRequestUrl } from '@/main/browser-agent/url-guard'
 
 const logger = createLogger('BrowserAgentSession')
 
@@ -68,6 +69,26 @@ function configureAgentPartition(ses: Session): void {
   partitionConfigured = true
   ses.setPermissionRequestHandler((_wc, _permission, callback) => callback(false))
   ses.setPermissionCheckHandler(() => false)
+  // The SSRF choke point for the agent partition. Document navigations
+  // (top-level + iframes) get the full DNS-resolving check — this is the ONE
+  // seam every navigation passes through, including the page-initiated ones the
+  // driver never sees (server redirects, link clicks, location.href,
+  // meta-refresh), so an internal-resolving hostname can't slip in that way.
+  // Subresources take the cheap synchronous literal-IP backstop instead of a
+  // DNS lookup per asset (a hostname subresource is already gated by its
+  // document's check).
+  ses.webRequest.onBeforeRequest((details, callback) => {
+    if (details.resourceType === 'mainFrame' || details.resourceType === 'subFrame') {
+      void checkAgentUrl(details.url).then((guard) => {
+        if (!guard.ok) {
+          logger.warn('Blocked agent document navigation to a private host')
+        }
+        callback({ cancel: !guard.ok })
+      })
+      return
+    }
+    callback({ cancel: isBlockedRequestUrl(details.url) })
+  })
   ses.on('will-download', (_event, item) => {
     const filename = item.getFilename()
     const url = item.getURL()
@@ -97,7 +118,9 @@ function createTabView(): WebContentsView {
   configureAgentPartition(contents.session)
 
   // Popup-neutralize: window.open and target=_blank navigate the SAME view
-  // instead of spawning windows — the agent browses one page per tab.
+  // instead of spawning windows — the agent browses one page per tab. The
+  // resulting navigation is a document request, so the partition's
+  // onBeforeRequest SSRF check gates it; no per-call guard is needed here.
   contents.setWindowOpenHandler((details) => {
     if (/^https?:\/\//i.test(details.url)) {
       void contents.loadURL(details.url).catch(() => {})

apps/desktop/src/main/browser-agent/url-guard.test.ts

@@ -0,0 +1,80 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockLookup } = vi.hoisted(() => ({ mockLookup: vi.fn() }))
+
+vi.mock('node:dns/promises', () => ({
+  default: { lookup: mockLookup },
+}))
+
+import { checkAgentUrl, isBlockedRequestUrl } from '@/main/browser-agent/url-guard'
+
+describe('checkAgentUrl', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockLookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }])
+  })
+
+  it('rejects non-http(s) schemes without resolving', async () => {
+    const result = await checkAgentUrl('file:///etc/passwd')
+    expect(result.ok).toBe(false)
+    expect(mockLookup).not.toHaveBeenCalled()
+  })
+
+  it('rejects malformed URLs', async () => {
+    expect((await checkAgentUrl('not a url')).ok).toBe(false)
+  })
+
+  it('blocks private IP literals without resolving', async () => {
+    expect((await checkAgentUrl('http://127.0.0.1/')).ok).toBe(false)
+    expect((await checkAgentUrl('http://169.254.169.254/latest/meta-data')).ok).toBe(false)
+    expect((await checkAgentUrl('http://10.0.0.5/')).ok).toBe(false)
+    expect((await checkAgentUrl('http://[::1]/')).ok).toBe(false)
+    expect(mockLookup).not.toHaveBeenCalled()
+  })
+
+  it('allows public IP literals without resolving', async () => {
+    expect((await checkAgentUrl('https://8.8.8.8/')).ok).toBe(true)
+    expect(mockLookup).not.toHaveBeenCalled()
+  })
+
+  it('allows hostnames that resolve to public addresses', async () => {
+    const result = await checkAgentUrl('https://example.com/page')
+    expect(result.ok).toBe(true)
+    expect(mockLookup).toHaveBeenCalledWith('example.com', { all: true, verbatim: true })
+  })
+
+  it('blocks hostnames that resolve to a private address (DNS rebinding)', async () => {
+    mockLookup.mockResolvedValue([{ address: '10.1.2.3', family: 4 }])
+    expect((await checkAgentUrl('https://rebind.evil.test/')).ok).toBe(false)
+  })
+
+  it('blocks when any resolved address is private', async () => {
+    mockLookup.mockResolvedValue([
+      { address: '93.184.216.34', family: 4 },
+      { address: '192.168.0.9', family: 4 },
+    ])
+    expect((await checkAgentUrl('https://mixed.test/')).ok).toBe(false)
+  })
+
+  it('allows the load to proceed when DNS resolution fails', async () => {
+    mockLookup.mockRejectedValue(new Error('ENOTFOUND'))
+    expect((await checkAgentUrl('https://nope.invalid/')).ok).toBe(true)
+  })
+})
+
+describe('isBlockedRequestUrl', () => {
+  it('blocks literal private/reserved hosts', () => {
+    expect(isBlockedRequestUrl('http://169.254.169.254/latest/meta-data')).toBe(true)
+    expect(isBlockedRequestUrl('http://127.0.0.1:8080/x')).toBe(true)
+    expect(isBlockedRequestUrl('https://[fd00::1]/')).toBe(true)
+  })
+
+  it('allows public literals and hostnames (classified at nav time)', () => {
+    expect(isBlockedRequestUrl('https://8.8.8.8/')).toBe(false)
+    expect(isBlockedRequestUrl('https://example.com/x')).toBe(false)
+  })
+
+  it('does not throw on malformed input', () => {
+    expect(isBlockedRequestUrl('::::')).toBe(false)
+  })
+})

apps/desktop/src/main/browser-agent/url-guard.ts

@@ -0,0 +1,83 @@
+import dns from 'node:dns/promises'
+import { createLogger } from '@sim/logger'
+import { isIpLiteral, isPrivateIp, isPrivateIpHost, unwrapIpv6Brackets } from '@sim/security/ssrf'
+import { getErrorMessage } from '@sim/utils/errors'
+import { parseHttpUrl } from '@/main/navigation'
+
+const logger = createLogger('BrowserAgentUrlGuard')
+
+export interface UrlGuardResult {
+  ok: boolean
+  error?: string
+}
+
+const OK: UrlGuardResult = { ok: true }
+const BLOCKED: UrlGuardResult = {
+  ok: false,
+  error: 'That address points to a private or internal network and was blocked.',
+}
+
+/**
+ * SSRF guard for agent-browser navigation. The embedded browser is a
+ * general-purpose surface driven by model/tool input, so a navigation to a
+ * loopback/RFC1918/link-local host (e.g. the `169.254.169.254` cloud-metadata
+ * endpoint) would let a page's contents be read back through the read/snapshot
+ * tools. This resolves the host the same way `apps/sim` does for outbound
+ * fetches and blocks any that land on a private/reserved address.
+ *
+ * IP literals are classified directly; hostnames are DNS-resolved and every
+ * returned address is checked. A resolution failure is not treated as a block —
+ * an unresolvable host reaches nothing, and Chromium fails the load naturally.
+ * The residual DNS-rebinding TOCTOU window (our lookup vs Chromium's) is only
+ * fully closable with egress firewalling; {@link isBlockedRequestUrl} adds a
+ * synchronous per-request literal-IP backstop for redirects and subresources.
+ */
+export async function checkAgentUrl(rawUrl: string): Promise<UrlGuardResult> {
+  const url = parseHttpUrl(rawUrl)
+  if (!url) {
+    return { ok: false, error: 'URL must be absolute and start with http:// or https://' }
+  }
+
+  const host = unwrapIpv6Brackets(url.hostname)
+
+  // IP literal: classify directly, no DNS lookup needed.
+  if (isIpLiteral(host)) {
+    if (isPrivateIp(host)) {
+      logger.warn('Blocked agent navigation to private IP literal', { host })
+      return BLOCKED
+    }
+    return OK
+  }
+
+  try {
+    const resolved = await dns.lookup(host, { all: true, verbatim: true })
+    if (resolved.some(({ address }) => isPrivateIp(address))) {
+      logger.warn('Blocked agent navigation resolving to private IP', { host })
+      return BLOCKED
+    }
+  } catch (error) {
+    logger.info('Agent navigation host did not resolve; letting the load fail naturally', {
+      host,
+      error: getErrorMessage(error),
+    })
+  }
+
+  return OK
+}
+
+/**
+ * Synchronous backstop for the agent partition's `onBeforeRequest`: blocks any
+ * request whose host is a **literal** private/reserved IP. This is cheap enough
+ * to run per-request and catches redirects and subresources that target the
+ * metadata endpoint or an internal IP directly, without the cost of a DNS
+ * lookup on every subresource. Hostnames pass here (they are classified at
+ * navigation time by {@link checkAgentUrl}).
+ */
+export function isBlockedRequestUrl(rawUrl: string): boolean {
+  try {
+    // isPrivateIpHost strips IPv6 brackets itself.
+    return isPrivateIpHost(new URL(rawUrl).hostname)
+  } catch {
+    return false
+  }
+}

apps/desktop/src/main/config.ts

@@ -6,8 +6,12 @@ const logger = createLogger('DesktopConfig')
 
 export const DEFAULT_ORIGIN = 'https://sim.ai'
 
-/** Loopback hostnames that may use plain HTTP (dev + self-host testing). */
-export const LOCAL_HOSTNAMES = new Set(['localhost', '127.0.0.1', '[::1]', '::1'])
+/**
+ * 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