fix(desktop): act on adversarial review of the security fixes

Six review agents went through the branch line by line. Several of the fixes
were wrong, incomplete, or worse than the finding they closed.

terminal:write was gated on the whole channel, which is a functional break, not
a fix. That channel carries xterm.js's entire upstream stream, and much of it is
not typing: the PTY solicits replies the terminal must answer unprompted — DSR
cursor position (p10k/starship emit it every prompt), device attributes, focus
reports set by tmux and vim. Now only a payload that can submit (one containing
a newline) needs input behind it. Also stated plainly in the code: this is a
mitigation. Text without a newline still lands in the line buffer where the
user's own Enter submits it, and closing that needs the interactive path off the
renderer surface, not a better gate.

The panel occlusion gate is reverted outright. Occlusion is driven by any
element marked data-native-surface-overlay, which includes tooltips (hover, and
hover is deliberately not "deliberate input") and toasts (no input at all), so
the common case regressed. Worse, the renderer only ever sets panelSnapshot and
never clears it, so withholding a frame shows the PREVIOUS overlay's frame — the
exact defect panel.test.ts was written to prevent — while the already-delivered
frame stays readable. Net negative on both axes.

The credential grant ordering is inverted to exact match. reveal was treated as
dominating copy, but copy publishes plaintext to the macOS pasteboard: readable
by every process, persisted by clipboard managers past the 30s clear, and synced
to other devices by Universal Clipboard. The operations are incomparable.

Update downloads are constrained to the release asset prefix, not just to https.
The feed rewrites every entry to github.com/simstudioai/sim/releases/download/,
so nothing legitimate is excluded — while scheme-only validation still admitted
an attacker-hosted DMG that the download dialog walks the user through
installing, which is worse than the protocol-handler launch originally fixed.
The loopback exemption is dropped with it: no legitimate asset is ever http.
State goes to 'error', not 'idle', so a blocked shell is not told it is current.

Subresource DNS verdicts cache the promise, not the boolean. Caching only the
result left every request arriving before the first lookup settled to start its
own, and dns.lookup is getaddrinfo on the four-slot libuv threadpool shared with
every fs call in main — a page naming hundreds of hosts could stall the settings
write and the credential vault.

Also: an eighth copy of the credential-token vocabulary in the browser preload
was missed by the original commit while its comment claimed parity, so fill went
blind to `current-password webauthn`; the OTP/payment readback zeroed valueLength
and told the agent a successful fill was still empty, inviting a doubled code;
tagName comparisons are upper-cased for XHTML; DIALOG/VIDEO/AUDIO/EMBED/OBJECT
are focusable and no longer report opaque; drags count as input; a backwards
clock step no longer satisfies a recency gate; and clearing cache or profile now
clears resolved-host verdicts too.

The closed-shadow residual is documented rather than closed: a host carrying
tabindex or contenteditable still reports safe, and refusing those would block
Enter and Space on ordinary <div tabindex="0"> buttons, since a closed root is
indistinguishable from no root at all.

Author: waleedlatif1

apps/desktop/src/main/browser-agent/page-functions.test.ts

@@ -353,14 +353,16 @@ describe('readActiveElementState', () => {
     ['a one-time code', 'one-time-code', '123456'],
     ['a card number', 'cc-number', '4111111111111111'],
     ['a card security code', 'cc-csc', '737'],
-  ])('withholds %s on readback but keeps the real tag', (_label, token, value) => {
+  ])('withholds %s on readback but still confirms the fill', (_label, token, value) => {
     document.body.innerHTML = `<input type="text" autocomplete="${token}" value="${value}" />`
     setActiveElement(document, document.querySelector('input'))
 
+    // valueLength is kept: without it a successful type reads as "still empty"
+    // and the agent types the code a second time.
     expect(readActiveElementState()).toEqual({
       activeElement: 'input',
       selectedChars: 0,
-      valueLength: 0,
+      valueLength: value.length,
       valuePreview: '',
       redacted: true,
     })

apps/desktop/src/main/browser-agent/page-functions.ts

@@ -488,13 +488,18 @@ export function readActiveElementState(): unknown {
       redacted: true,
     }
   }
-  // Reported as the real tag rather than 'password-field': the agent may
-  // still type here, it just never learns what is already in the field.
+  // Only the preview is withheld, and the real tag is kept: the agent is allowed
+  // to fill these, so it still needs the readback this function exists for.
+  // Zeroing valueLength told it the field was empty after a successful type, and
+  // the natural next move is to type again — a doubled OTP or card number.
+  // A length is not a disclosure here; 6 and 16 are properties of the format.
   if (isSensitiveValueField(active)) {
+    const field = active as HTMLInputElement
+    const current = String(field.value ?? '')
     return {
       activeElement: active.tagName.toLowerCase(),
-      selectedChars: 0,
-      valueLength: 0,
+      selectedChars: Math.abs((field.selectionEnd ?? 0) - (field.selectionStart ?? 0)),
+      valueLength: current.length,
       valuePreview: '',
       redacted: true,
     }
@@ -569,19 +574,38 @@ export function activeElementSecrecy(): string {
     // `activeElement` unless focus was retargeted out of a shadow tree, which
     // makes "not focusable yet focused" the reliable signal. Frames stay out of
     // it so the branch below still classifies them.
+    // Tag compared upper-cased: tagName preserves case outside the HTML
+    // namespace and is lower-case for HTML elements in an XHTML document, where
+    // every comparison below would otherwise miss.
+    const tag = String(active.tagName || '').toUpperCase()
+    // RESIDUAL, stated rather than papered over: `tabindex` and
+    // `contenteditable` exempt an element even on a shadow-capable tag, so a
+    // host carrying either — `<div tabindex="0">` with a closed root — still
+    // reports 'safe'. A closed root is indistinguishable from no root at all
+    // (that is what `mode: 'closed'` buys the page), and `<div tabindex="0">`
+    // buttons and menu items are everywhere, so refusing them would block Enter
+    // and Space on ordinary pages to close a targeted case. The only reliable
+    // detector is `attachShadow` throwing, which is destructive. Narrowing this
+    // needs the driver to stop trusting a page-derived signal, not a better
+    // guess here.
     const focusableItself =
-      active === document.body ||
+      active === active.ownerDocument.body ||
       active.isContentEditable ||
       active.hasAttribute('tabindex') ||
-      active.tagName === 'INPUT' ||
-      active.tagName === 'TEXTAREA' ||
-      active.tagName === 'SELECT' ||
-      active.tagName === 'BUTTON' ||
-      active.tagName === 'A' ||
-      active.tagName === 'AREA' ||
-      active.tagName === 'SUMMARY' ||
-      active.tagName === 'IFRAME' ||
-      active.tagName === 'FRAME'
+      tag === 'INPUT' ||
+      tag === 'TEXTAREA' ||
+      tag === 'SELECT' ||
+      tag === 'BUTTON' ||
+      tag === 'A' ||
+      tag === 'AREA' ||
+      tag === 'SUMMARY' ||
+      tag === 'DIALOG' ||
+      tag === 'VIDEO' ||
+      tag === 'AUDIO' ||
+      tag === 'EMBED' ||
+      tag === 'OBJECT' ||
+      tag === 'IFRAME' ||
+      tag === 'FRAME'
     if (!shadow && !focusableItself) return 'opaque'
     if (active.tagName === 'IFRAME' || active.tagName === 'FRAME') {
       let inner: Document | null = null

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

@@ -2,10 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 vi.mock('electron', () => import('@/test/electron-mock'))
 
-import type { WebContents } from 'electron'
 import { BrowserWindow, WebContentsView } from 'electron'
 import * as panelModule from '@/main/browser-agent/panel'
-import { trackInputActivity } from '@/main/input-activity'
 
 type PanelModule = typeof import('@/main/browser-agent/panel')
 
@@ -29,29 +27,6 @@ function freshPanel(): PanelModule {
 
 const PANEL_RECT = { x: 400, y: 64, width: 600, height: 800 }
 
-type InputListener = (event: unknown, input: { type: string }) => void
-
-/**
- * Registers a window's renderer with the main-process input tracker and returns
- * a way to give it a real click. The occlusion snapshot is only handed over to a
- * renderer the user has recently driven, so a fixture has to say so explicitly.
- */
-function trackWindowInput(win: BrowserWindow) {
-  const listeners: InputListener[] = []
-  const contents = win.webContents as unknown as {
-    on: (channel: string, listener: InputListener) => void
-    isDestroyed: () => boolean
-  }
-  contents.on = (channel, listener) => {
-    if (channel === 'input-event') listeners.push(listener)
-  }
-  contents.isDestroyed = () => false
-  trackInputActivity(win.webContents as WebContents)
-  return () => {
-    for (const listener of listeners) listener({}, { type: 'mouseDown' })
-  }
-}
-
 /** A panel showing one tab, which is the state occlusion applies to. */
 function showPanel(panel: PanelModule) {
   const win = new BrowserWindow()
@@ -63,15 +38,13 @@ function showPanel(panel: PanelModule) {
     ensureInitialTab: () => {},
     onViewDetached: () => {},
   })
-  const press = trackWindowInput(win)
-  press()
   panel.setPanelBounds(PANEL_RECT, win)
   /** Swaps in another tab's view, as switching tabs does. */
   const switchTab = (next: WebContentsView) => {
     active = { id: 'tab-2', view: next, pinned: false }
     panel.layout()
   }
-  return { win, view, switchTab, press }
+  return { win, view, switchTab }
 }
 
 /** When the view was hidden, in the global mock invocation order. */
@@ -118,38 +91,6 @@ describe('panel occlusion', () => {
     expect(sent).toBeLessThan(hiddenAt(view) as number)
   })
 
-  it('withholds the frame from a renderer the user has not driven', async () => {
-    const panel = freshPanel()
-    const { win, view } = showPanel(panel)
-    // A script-only caller: no real OS input has reached this renderer inside
-    // the recency window, so the pixels of the agent page are not handed over.
-    vi.useFakeTimers()
-    try {
-      vi.advanceTimersByTime(5_000)
-      panel.setPanelOccluded(true, win)
-      await vi.waitFor(() => expect(hiddenAt(view)).toBeDefined())
-    } finally {
-      vi.useRealTimers()
-    }
-
-    expect(snapshotSentAt(win)).toBeUndefined()
-  })
-
-  it('still occludes when the frame is withheld, so nothing is left half-applied', async () => {
-    const panel = freshPanel()
-    const { win, view } = showPanel(panel)
-    vi.useFakeTimers()
-    try {
-      vi.advanceTimersByTime(5_000)
-      panel.setPanelOccluded(true, win)
-      // Hiding must not depend on the snapshot: an empty capture already
-      // reaches the same path, so this is an existing outcome, not a new one.
-      await vi.waitFor(() => expect(hiddenAt(view)).toBeDefined())
-    } finally {
-      vi.useRealTimers()
-    }
-  })
-
   it('stays visible when the overlay closes while the frame is being taken', async () => {
     const { win, view } = showPanel(panel)
 

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

@@ -20,7 +20,6 @@ import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
 import type { BrowserWindow, WebContentsView } from 'electron'
 import type { AgentTab } from '@/main/browser-agent/session'
-import { hasRecentDeliberateInput } from '@/main/input-activity'
 
 const logger = createLogger('BrowserAgentPanel')
 
@@ -334,28 +333,6 @@ function capturePanelSnapshot(onSettled?: () => void): void {
       // picture of the page, so it goes to the window still showing the
       // browser or nowhere at all.
       if (panelWindow() !== win || win.isDestroyed()) return
-      // A frame of the agent browser is content the renderer's own JS cannot
-      // otherwise read — the view is a separate process composited over the
-      // window. It is handed over so an overlay can show a placeholder instead
-      // of a blank gap, and an overlay opens because the user did something. A
-      // compromised renderer can drive set-panel-bounds, panel-action navigate
-      // and set-panel-occluded on its own, so without this the pixels of an
-      // authenticated page could be collected by script alone, bypassing the
-      // tool-call binding that guards browser_screenshot for exactly this.
-      //
-      // Skipping the send is an already-supported outcome: an empty capture
-      // returns here too, and `finally` still runs the occlusion, so no new
-      // state is introduced — at worst a placeholder is missing for one frame.
-      if (!hasRecentDeliberateInput(win.webContents)) {
-        logger.warn('Withheld browser panel snapshot with no recent user input')
-        return
-      }
-      // Downscale and JPEG-encode before crossing IPC. capturePage returns a
-      // device-pixel PNG — on a retina half-window that is millions of pixels,
-      // and toDataURL's PNG encode is synchronous on the main process, so a
-      // full-size encode stalls every window's input for the frame. This is a
-      // placeholder shown under a transient overlay, so a downscaled JPEG is
-      // indistinguishable and an order of magnitude cheaper to encode and send.
       const snapshot: BrowserPanelSnapshot = { dataUrl: encodeSnapshot(image), tabId }
       win.webContents.send('browser-agent:panel-snapshot', snapshot)
     })

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

@@ -4,11 +4,9 @@ vi.mock('electron', () => import('@/test/electron-mock'))
 
 import { MAX_BROWSER_TABS } from '@sim/browser-protocol'
 import { sleep } from '@sim/utils/helpers'
-import type { WebContents } from 'electron'
 import { BrowserWindow, session as electronSession } from 'electron'
 import * as panel from '@/main/browser-agent/panel'
 import * as sessionModule from '@/main/browser-agent/session'
-import { trackInputActivity } from '@/main/input-activity'
 
 type SessionModule = typeof import('@/main/browser-agent/session')
 
@@ -36,8 +34,6 @@ interface MockView {
   setVisible: ReturnType<typeof vi.fn>
 }
 
-type InputListener = (event: unknown, input: { type: string }) => void
-
 function mainWindowMock() {
   const win = new BrowserWindow() as unknown as {
     contentView: {
@@ -47,20 +43,6 @@ function mainWindowMock() {
     webContents: { getZoomFactor?: ReturnType<typeof vi.fn> }
   }
   win.webContents.getZoomFactor = vi.fn(() => 1)
-  // The occlusion snapshot is only handed to a renderer the user has recently
-  // driven, so these fixtures — which stand in for a user with the panel open —
-  // register with the input tracker and report one real click.
-  const listeners: InputListener[] = []
-  const contents = win.webContents as unknown as {
-    on: (channel: string, listener: InputListener) => void
-    isDestroyed: () => boolean
-  }
-  contents.on = (channel, listener) => {
-    if (channel === 'input-event') listeners.push(listener)
-  }
-  contents.isDestroyed = () => false
-  trackInputActivity(win.webContents as unknown as WebContents)
-  for (const listener of listeners) listener({}, { type: 'mouseDown' })
   return win as unknown as BrowserWindow
 }
 

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

@@ -27,6 +27,7 @@ import {
 import { registerAgentWebContents } from '@/main/browser-agent/registry'
 import {
   checkAgentUrl,
+  clearHostVerdictCache,
   isBlockedRequestUrl,
   isBlockedSubresourceUrl,
   subresourceNeedsResolution,
@@ -315,31 +316,45 @@ function configureAgentPartition(ses: Session): void {
   // labels differently than expected fails safe into the checked path; see
   // subresourceNeedsResolution.
   ses.webRequest.onBeforeRequest((details, callback) => {
+    // Answered exactly once, and never throwing. A throw inside the `then`
+    // below would otherwise land in the `catch` and answer a second time, and
+    // by the time an async check settles the request's loader may be gone —
+    // now the case for most subresources, not just the odd navigation.
+    let settled = false
+    const settle = (cancel: boolean) => {
+      if (settled) return
+      settled = true
+      try {
+        callback({ cancel })
+      } catch (error) {
+        logger.warn('Could not answer an agent request', { error: getErrorMessage(error) })
+      }
+    }
     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 })
+          settle(!guard.ok)
         })
         .catch((error) => {
           // Fail closed: an unexpected rejection must cancel, never leave the
           // request suspended with no callback.
           logger.error('Agent SSRF check failed; cancelling request', { error })
-          callback({ cancel: true })
+          settle(true)
         })
       return
     }
     if (!subresourceNeedsResolution(details.resourceType)) {
-      callback({ cancel: isBlockedRequestUrl(details.url) })
+      settle(isBlockedRequestUrl(details.url))
       return
     }
     void isBlockedSubresourceUrl(details.url)
-      .then((blocked) => callback({ cancel: blocked }))
+      .then((blocked) => settle(blocked))
       .catch((error) => {
         logger.error('Agent subresource SSRF check failed; cancelling request', { error })
-        callback({ cancel: true })
+        settle(true)
       })
   })
   ses.on('will-download', (_event, item) => {
@@ -1068,6 +1083,9 @@ export function closeSession(): void {
  * pinned tabs, or browsing trail.
  */
 export async function clearProfileStorage(): Promise<void> {
+  // Cached DNS verdicts are part of the browsing trail: without this a wipe
+  // leaves up to the TTL of resolved-host classifications behind.
+  clearHostVerdictCache()
   closeLiveTabs()
   // Stays true so a later restore cannot re-read the list being erased here.
   pinnedTabsRestored = true
@@ -1114,7 +1132,12 @@ export async function clearAgentData(kinds: readonly BrowserDataKind[]): Promise
   if (storages.length > 0) {
     await ses.clearStorageData({ storages } as Parameters<Session['clearStorageData']>[0])
   }
-  if (kinds.includes('cache')) await ses.clearCache()
+  if (kinds.includes('cache')) {
+    await ses.clearCache()
+    // Resolved-host verdicts are a cache too, and a user clearing the cache
+    // means all of it.
+    clearHostVerdictCache()
+  }
 }
 
 export function listTabs(): BrowserTabState[] {