fix(desktop): address release review — count app downloads and unchecked terminals before an update restart, send the page referrer on Save Image, and tighten sign-in and people edge cases

apps/desktop/e2e/browser-page-parity.spec.ts

@@ -101,6 +101,8 @@ test.describe('browser page parity with Chrome', () => {
   const held = new Set<ServerResponse>()
   /** The Referer each load of the link target arrived with. */
   const referrers: (string | undefined)[] = []
+  /** The Referer each request for the image arrived with. */
+  const imageReferrers: (string | undefined)[] = []
   const calls = new Map<string, { chatId: string; toolName: BrowserToolName; args: unknown }>()
   const server = createServer(async (request, response) => {
     const path = new URL(request.url ?? '/', 'http://localhost').pathname
@@ -113,7 +115,14 @@ test.describe('browser page parity with Chrome', () => {
       return
     }
     if (path === '/pixel.png') {
-      response.writeHead(200, { 'Content-Type': 'image/png' })
+      imageReferrers.push(request.headers.referer)
+      // Hotlink-protected, like many image hosts: a request without the page as referrer fails.
+      if (!request.headers.referer) {
+        response.writeHead(403)
+        response.end()
+        return
+      }
+      response.writeHead(200, { 'Content-Type': 'image/png', 'Cache-Control': 'no-store' })
       response.end(PNG)
       return
     }
@@ -358,20 +367,25 @@ test.describe('browser page parity with Chrome', () => {
     await expect.poll(pageUrl).toBe(`${site}/next`)
   })
 
-  checkedTest("the page menu's Save Image lands through the browser's downloads", async () => {
-    for (const file of readdirSync(downloads)) rmSync(join(downloads, file), { force: true })
-    await navigate('/image')
-    await expect
-      .poll(() => inPage({ kind: 'script', script: 'document.getElementById("pic").complete' }))
-      .toBe(true)
-    const labels = await openPageMenu('#pic')
-    expect(labels).toEqual(
-      expect.arrayContaining(['Open Image in New Tab', 'Copy Image', 'Copy Image Address'])
-    )
-    await choosePageMenuItem('Save Image')
-    await expect.poll(() => readdirSync(downloads)).toEqual(['pixel.png'])
-    expect(readFileSync(join(downloads, 'pixel.png')).equals(PNG)).toBe(true)
-  })
+  checkedTest(
+    "the page menu's Save Image lands through the browser's downloads, with the page as referrer",
+    async () => {
+      for (const file of readdirSync(downloads)) rmSync(join(downloads, file), { force: true })
+      await navigate('/image')
+      await expect
+        .poll(() => inPage({ kind: 'script', script: 'document.getElementById("pic").complete' }))
+        .toBe(true)
+      const labels = await openPageMenu('#pic')
+      expect(labels).toEqual(
+        expect.arrayContaining(['Open Image in New Tab', 'Copy Image', 'Copy Image Address'])
+      )
+      imageReferrers.length = 0
+      await choosePageMenuItem('Save Image')
+      await expect.poll(() => readdirSync(downloads)).toEqual(['pixel.png'])
+      expect(readFileSync(join(downloads, 'pixel.png')).equals(PNG)).toBe(true)
+      expect(imageReferrers).toEqual([`${site}/image`])
+    }
+  )
 
   checkedTest("the page menu's Open Link in New Tab sends the page as referrer", async () => {
     referrers.length = 0

apps/desktop/e2e/fixtures/updater.ts

@@ -102,6 +102,7 @@ void app.whenReady().then(async () => {
     describeInFlightWork: async () =>
       describeInFlightWork({
         terminalCommands,
+        uncheckedTerminalCount: 0,
         computerUseApp: null,
         browserToolRunning,
         agentWorkRunning: false,

apps/desktop/src/main/browser-agent/context-menu.ts

@@ -22,6 +22,7 @@ import {
   consumeAgentContextMenu,
   isDispatchingAgentInput,
 } from '@/main/browser-agent/cdp'
+import { menuReferrer, referrerForRequest } from '@/main/browser-agent/referrer'
 import { spellingItems } from '@/main/context-menu'
 import { parseHttpUrl } from '@/main/navigation'
 
@@ -139,33 +140,6 @@ export interface AgentContextMenuHost {
 
 const isWebUrl = (url: string) => parseHttpUrl(url) !== null
 
-const REFERRER_POLICIES: ReadonlySet<unknown> = new Set<Referrer['policy']>([
-  'default',
-  'unsafe-url',
-  'no-referrer-when-downgrade',
-  'no-referrer',
-  'origin',
-  'strict-origin-when-cross-origin',
-  'same-origin',
-  'strict-origin',
-])
-
-function isReferrerPolicy(value: unknown): value is Referrer['policy'] {
-  return REFERRER_POLICIES.has(value)
-}
-
-/** Electron types `referrerPolicy` as a Referrer, but it arrives as the bare policy string. */
-function menuReferrer(
-  params: Pick<ContextMenuParams, 'referrerPolicy' | 'frameURL' | 'pageURL'>
-): Referrer {
-  const policy: unknown = params.referrerPolicy
-  const url = params.frameURL || params.pageURL
-  return {
-    url: isWebUrl(url) ? url : '',
-    policy: isReferrerPolicy(policy) ? policy : 'default',
-  }
-}
-
 /** Ends the template with one separator, never two in a row or one at the top. */
 function endGroup(template: MenuItemConstructorOptions[]): void {
   if (template.length > 0 && template[template.length - 1].type !== 'separator') {
@@ -314,8 +288,12 @@ export function attachAgentContextMenu(contents: WebContents, host: AgentContext
         openTab: (url) => host.openTab(url, menuReferrer(params)),
         copyLink: (url) => clipboard.writeText(url),
         copyImage: () => contents.copyImageAt(params.x, params.y),
-        // A download, so the browser's download limits and request guards apply.
-        saveImage: (url) => contents.downloadURL(url),
+        // A download, so the browser's download limits and request guards apply. Electron maps a
+        // Referer header into the download's referrer, so referrer-protected images still save.
+        saveImage: (url) => {
+          const referrer = referrerForRequest(menuReferrer(params), url)
+          contents.downloadURL(url, referrer ? { headers: { Referer: referrer } } : undefined)
+        },
         setZoomFactor: (factor) => contents.setZoomFactor(factor),
       }
     )

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

@@ -0,0 +1,57 @@
+import { describe, expect, it, vi } from 'vitest'
+
+vi.mock('electron', () => import('@/test/electron-mock'))
+
+import { referrerForRequest } from '@/main/browser-agent/referrer'
+
+describe('referrerForRequest', () => {
+  const PAGE = 'https://user:secret@news.example.com/story?id=7#comments'
+  const FULL = 'https://news.example.com/story?id=7'
+  const ORIGIN = 'https://news.example.com/'
+
+  it('sends the full URL same-origin and only the origin cross-origin by default', () => {
+    const referrer = { url: PAGE, policy: 'default' } as const
+    expect(referrerForRequest(referrer, 'https://news.example.com/img.png')).toBe(FULL)
+    expect(referrerForRequest(referrer, 'https://cdn.example.net/img.png')).toBe(ORIGIN)
+  })
+
+  it('never sends credentials or the fragment, even under unsafe-url', () => {
+    expect(
+      referrerForRequest({ url: PAGE, policy: 'unsafe-url' }, 'http://cdn.example.net/img.png')
+    ).toBe(FULL)
+  })
+
+  it('sends nothing from an https page to an http image unless the policy allows downgrades', () => {
+    const image = 'http://cdn.example.net/img.png'
+    for (const policy of [
+      'default',
+      'strict-origin-when-cross-origin',
+      'strict-origin',
+      'no-referrer-when-downgrade',
+    ] as const) {
+      expect(referrerForRequest({ url: PAGE, policy }, image)).toBeUndefined()
+    }
+    expect(referrerForRequest({ url: PAGE, policy: 'origin' }, image)).toBe(ORIGIN)
+    expect(referrerForRequest({ url: PAGE, policy: 'no-referrer-when-downgrade' }, FULL)).toBe(FULL)
+  })
+
+  it('honors no-referrer and same-origin', () => {
+    const crossOrigin = 'https://cdn.example.net/img.png'
+    expect(referrerForRequest({ url: PAGE, policy: 'no-referrer' }, FULL)).toBeUndefined()
+    expect(referrerForRequest({ url: PAGE, policy: 'same-origin' }, crossOrigin)).toBeUndefined()
+    expect(referrerForRequest({ url: PAGE, policy: 'same-origin' }, FULL)).toBe(FULL)
+  })
+
+  it('cuts a referrer longer than 4096 characters to its origin', () => {
+    const longPage = `https://news.example.com/story?q=${'x'.repeat(4096)}`
+    expect(
+      referrerForRequest({ url: longPage, policy: 'unsafe-url' }, 'https://cdn.example.net/img.png')
+    ).toBe(ORIGIN)
+  })
+
+  it('sends nothing for a page that is not on the web', () => {
+    expect(
+      referrerForRequest({ url: '', policy: 'unsafe-url' }, 'https://cdn.example.net/img.png')
+    ).toBeUndefined()
+  })
+})

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

@@ -0,0 +1,81 @@
+/**
+ * The referrer a page-menu action carries: the clicked frame's URL and Referrer-Policy, and the
+ * `Referer` that policy allows for a given request.
+ */
+import type { ContextMenuParams, Referrer } from 'electron'
+import { parseHttpUrl } from '@/main/navigation'
+
+const isWebUrl = (url: string) => parseHttpUrl(url) !== null
+
+const REFERRER_POLICIES: ReadonlySet<unknown> = new Set<Referrer['policy']>([
+  'default',
+  'unsafe-url',
+  'no-referrer-when-downgrade',
+  'no-referrer',
+  'origin',
+  'strict-origin-when-cross-origin',
+  'same-origin',
+  'strict-origin',
+])
+
+function isReferrerPolicy(value: unknown): value is Referrer['policy'] {
+  return REFERRER_POLICIES.has(value)
+}
+
+/** Electron types `referrerPolicy` as a Referrer, but it arrives as the bare policy string. */
+export function menuReferrer(
+  params: Pick<ContextMenuParams, 'referrerPolicy' | 'frameURL' | 'pageURL'>
+): Referrer {
+  const policy: unknown = params.referrerPolicy
+  const url = params.frameURL || params.pageURL
+  return {
+    url: isWebUrl(url) ? url : '',
+    policy: isReferrerPolicy(policy) ? policy : 'default',
+  }
+}
+
+/** Longest referrer sent in full; a longer one is cut to its origin, as browsers do. */
+const MAX_REFERRER_LENGTH = 4096
+const LOOPBACK_HOSTS: ReadonlySet<string> = new Set(['localhost', '127.0.0.1', '[::1]'])
+
+function isPotentiallyTrustworthy(url: URL): boolean {
+  if (url.protocol === 'https:') return true
+  return LOOPBACK_HOSTS.has(url.hostname) || url.hostname.endsWith('.localhost')
+}
+
+/**
+ * The `Referer` a request from `referrer`'s document to `targetUrl` carries under its
+ * Referrer-Policy (W3C Referrer Policy, "determine request's referrer"), or undefined when it
+ * sends none. Chromium's `default` is `strict-origin-when-cross-origin`; Electron's `Referrer` has
+ * no `origin-when-cross-origin`, which {@link menuReferrer} already reads as `default`.
+ */
+export function referrerForRequest(referrer: Referrer, targetUrl: string): string | undefined {
+  const source = parseHttpUrl(referrer.url)
+  const target = parseHttpUrl(targetUrl)
+  if (!source || !target) return undefined
+  source.username = ''
+  source.password = ''
+  source.hash = ''
+  const origin = `${source.origin}/`
+  const full = source.href.length > MAX_REFERRER_LENGTH ? origin : source.href
+  const sameOrigin = source.origin === target.origin
+  const downgrade = isPotentiallyTrustworthy(source) && !isPotentiallyTrustworthy(target)
+
+  switch (referrer.policy) {
+    case 'no-referrer':
+      return undefined
+    case 'origin':
+      return origin
+    case 'unsafe-url':
+      return full
+    case 'same-origin':
+      return sameOrigin ? full : undefined
+    case 'no-referrer-when-downgrade':
+      return downgrade ? undefined : full
+    case 'strict-origin':
+      return downgrade ? undefined : origin
+    default:
+      if (downgrade) return undefined
+      return sameOrigin ? full : origin
+  }
+}

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

@@ -1,11 +1,40 @@
+import { EventEmitter } from 'node:events'
 import { mkdtempSync, symlinkSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { describe, expect, it, vi } from 'vitest'
 
 vi.mock('electron', () => import('@/test/electron-mock'))
 
-import { sanitizeFilename, uniqueDownloadPath } from '@/main/downloads'
+import type { DownloadItem, Session } from 'electron'
+import {
+  attachDownloadHandling,
+  countProgressingAppDownloads,
+  sanitizeFilename,
+  uniqueDownloadPath,
+} from '@/main/downloads'
+import type { EventRecorder } from '@/main/observability'
+
+type DownloadState = ReturnType<DownloadItem['getState']>
+
+function startAppDownload(session: EventEmitter) {
+  const item = new EventEmitter()
+  let state: DownloadState = 'progressing'
+  Object.assign(item, {
+    getFilename: () => 'export.csv',
+    getMimeType: () => 'text/csv',
+    getSavePath: () => '/Downloads/export.csv',
+    getState: () => state,
+    setSaveDialogOptions: vi.fn(),
+  })
+  session.emit('will-download', {}, item, undefined)
+  return {
+    finish(finalState: DownloadState) {
+      state = finalState
+      item.emit('done', {}, finalState)
+    },
+  }
+}
 
 describe('sanitizeFilename', () => {
   it('neutralizes path separators and leading dots', () => {
@@ -49,3 +78,21 @@ describe('uniqueDownloadPath', () => {
     )
   })
 })
+
+describe('countProgressingAppDownloads', () => {
+  it('counts an app-window download until it completes or is cancelled', () => {
+    const session = new EventEmitter()
+    const events = { record: vi.fn() } as unknown as EventRecorder
+    attachDownloadHandling(session as unknown as Session, events)
+    const before = countProgressingAppDownloads()
+
+    const completed = startAppDownload(session)
+    const cancelled = startAppDownload(session)
+    expect(countProgressingAppDownloads()).toBe(before + 2)
+
+    completed.finish('completed')
+    expect(countProgressingAppDownloads()).toBe(before + 1)
+    cancelled.finish('cancelled')
+    expect(countProgressingAppDownloads()).toBe(before)
+  })
+})

apps/desktop/src/main/downloads.ts

@@ -2,7 +2,7 @@ import { lstat } from 'node:fs/promises'
 import { basename, extname, join } from 'node:path'
 import { createLogger } from '@sim/logger'
 import { generateShortId } from '@sim/utils/id'
-import type { Session } from 'electron'
+import type { DownloadItem, Session } from 'electron'
 import { app } from 'electron'
 import { isAgentWebContents } from '@/main/browser-agent/registry'
 import type { EventRecorder } from '@/main/observability'
@@ -131,6 +131,17 @@ export async function uniqueDownloadPath(
   return null
 }
 
+const activeAppDownloads = new Set<DownloadItem>()
+
+/** App-window downloads (Files exports and saves) still receiving bytes, which a restart would cut off. */
+export function countProgressingAppDownloads(): number {
+  let count = 0
+  for (const item of activeAppDownloads) {
+    if (item.getState() === 'progressing') count += 1
+  }
+  return count
+}
+
 /**
  * Wires will-download so exports, blob URLs, and presigned-URL downloads all
  * get a native save dialog with a sensible default name, and completed
@@ -139,11 +150,13 @@ export async function uniqueDownloadPath(
 export function attachDownloadHandling(session: Session, events: EventRecorder): void {
   session.on('will-download', (_event, item, contents) => {
     if (contents && isAgentWebContents(contents)) return
+    activeAppDownloads.add(item)
     const filename = suggestedFilename(item.getFilename(), item.getMimeType())
     item.setSaveDialogOptions({
       defaultPath: join(app.getPath('downloads'), filename),
     })
     item.once('done', (_doneEvent, state) => {
+      activeAppDownloads.delete(item)
       if (state === 'completed') {
         logger.info('Download completed', { filename })
         if (process.platform === 'darwin') {