fix(desktop): DNS-check agent subresources that are readable or execute

The agent partition's onBeforeRequest ran the resolving guard for mainFrame and
subFrame only; everything else fell to isBlockedRequestUrl, which sees literal
IPs and returns false for any hostname by design. A public name with a static
private A record therefore reached internal services from a page the agent was
steered to — no rebinding required. The vectors that matter are the ones whose
response comes back or runs: a WebSocket to an internal server reads data frames
cross-origin because such servers commonly ignore Origin, and a script or xhr
response executes in the page or is readable.

Subresources now take isBlockedSubresourceUrl, with the verdict cached per host
(30s TTL, bounded at 256 entries, oldest evicted) so this is not a lookup per
asset. Images and fonts keep the synchronous path: high volume, not readable
cross-origin, leaving the load/error timing oracle as the accepted residual.

The exemption is expressed as what skips the check, not what gets it, so a
resource type Chromium labels unexpectedly fails safe into the checked path —
fetch is `xhr` on some versions and `other` on others, and an allowlist that
missed the label in use would silently reopen the hole.

Resolver errors fail closed, matching checkAgentUrl, and that verdict is not
cached so a transient failure does not stick. The deliberate loopback carve-out
is unchanged — isBlockedAddress already exempts it on both paths.

Author: waleedlatif1

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

@@ -25,7 +25,12 @@ import {
   panelWindow,
 } from '@/main/browser-agent/panel'
 import { registerAgentWebContents } from '@/main/browser-agent/registry'
-import { checkAgentUrl, isBlockedRequestUrl } from '@/main/browser-agent/url-guard'
+import {
+  checkAgentUrl,
+  isBlockedRequestUrl,
+  isBlockedSubresourceUrl,
+  subresourceNeedsResolution,
+} from '@/main/browser-agent/url-guard'
 
 const logger = createLogger('BrowserAgentSession')
 
@@ -297,8 +302,18 @@ function configureAgentPartition(ses: Session): void {
   // iframes) get the full DNS-resolving check — the one seam every navigation
   // passes through, including page-initiated ones the driver never sees (server
   // redirects, link clicks, location.href, meta-refresh) — so an internal host
-  // can't slip in that way. Subresources take the cheap synchronous literal-IP
-  // backstop instead of a DNS lookup per asset.
+  // can't slip in that way.
+  //
+  // Subresources that come back readable or that execute get the resolving
+  // check too, cached per host: a literal-IP backstop alone let a public
+  // hostname with a private A record reach internal services, and a WebSocket
+  // to one is a cross-origin read primitive because internal servers commonly
+  // ignore Origin. Only images and fonts keep the cheap synchronous path —
+  // they are the high-volume types and are not readable cross-origin, leaving a
+  // load/error timing oracle as the accepted residual. The list is expressed as
+  // what is exempt rather than what is checked, so a resource type Chromium
+  // labels differently than expected fails safe into the checked path; see
+  // subresourceNeedsResolution.
   ses.webRequest.onBeforeRequest((details, callback) => {
     if (details.resourceType === 'mainFrame' || details.resourceType === 'subFrame') {
       void checkAgentUrl(details.url)
@@ -316,7 +331,16 @@ function configureAgentPartition(ses: Session): void {
         })
       return
     }
-    callback({ cancel: isBlockedRequestUrl(details.url) })
+    if (!subresourceNeedsResolution(details.resourceType)) {
+      callback({ cancel: isBlockedRequestUrl(details.url) })
+      return
+    }
+    void isBlockedSubresourceUrl(details.url)
+      .then((blocked) => callback({ cancel: blocked }))
+      .catch((error) => {
+        logger.error('Agent subresource SSRF check failed; cancelling request', { error })
+        callback({ cancel: true })
+      })
   })
   ses.on('will-download', (_event, item) => {
     const filename = item.getFilename()

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

@@ -6,7 +6,13 @@ vi.mock('node:dns/promises', () => ({
   default: { lookup: mockLookup },
 }))
 
-import { checkAgentUrl, isBlockedRequestUrl } from '@/main/browser-agent/url-guard'
+import {
+  checkAgentUrl,
+  clearHostVerdictCache,
+  isBlockedRequestUrl,
+  isBlockedSubresourceUrl,
+  subresourceNeedsResolution,
+} from '@/main/browser-agent/url-guard'
 
 describe('checkAgentUrl', () => {
   beforeEach(() => {
@@ -120,3 +126,104 @@ describe('isBlockedRequestUrl', () => {
     expect(isBlockedRequestUrl('::::')).toBe(false)
   })
 })
+
+describe('isBlockedSubresourceUrl', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    clearHostVerdictCache()
+    mockLookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }])
+  })
+
+  it('blocks a public hostname whose A record points at a private address', async () => {
+    mockLookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }])
+
+    await expect(isBlockedSubresourceUrl('https://10-0-0-5.evil.example/probe.json')).resolves.toBe(
+      true
+    )
+  })
+
+  it('catches what the synchronous literal-IP backstop cannot', async () => {
+    mockLookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }])
+    const url = 'https://10-0-0-5.evil.example/probe.json'
+
+    // The gap this guard exists to close: the sync check only sees literals, so
+    // a hostname with a private A record sailed through it.
+    expect(isBlockedRequestUrl(url)).toBe(false)
+    await expect(isBlockedSubresourceUrl(url)).resolves.toBe(true)
+  })
+
+  it('blocks a websocket to a privately-resolving host', async () => {
+    mockLookup.mockResolvedValue([{ address: '172.16.4.4', family: 4 }])
+
+    await expect(isBlockedSubresourceUrl('ws://internal.evil.example/socket')).resolves.toBe(true)
+  })
+
+  it('allows a hostname that resolves publicly', async () => {
+    await expect(isBlockedSubresourceUrl('https://example.com/app.js')).resolves.toBe(false)
+  })
+
+  it('blocks IPv6-mapped and link-local literals without resolving', async () => {
+    await expect(isBlockedSubresourceUrl('http://169.254.169.254/latest/meta-data')).resolves.toBe(
+      true
+    )
+    await expect(isBlockedSubresourceUrl('http://[::ffff:10.0.0.5]/x')).resolves.toBe(true)
+    expect(mockLookup).not.toHaveBeenCalled()
+  })
+
+  it('keeps the deliberate loopback carve-out', async () => {
+    await expect(isBlockedSubresourceUrl('http://127.0.0.1:3000/x')).resolves.toBe(false)
+    expect(mockLookup).not.toHaveBeenCalled()
+  })
+
+  it('resolves a host once and reuses the verdict', async () => {
+    await isBlockedSubresourceUrl('https://example.com/a.js')
+    await isBlockedSubresourceUrl('https://example.com/b.js')
+    await isBlockedSubresourceUrl('https://example.com/c.js')
+
+    expect(mockLookup).toHaveBeenCalledTimes(1)
+  })
+
+  it('fails closed when the host does not resolve, without caching that', async () => {
+    mockLookup.mockRejectedValueOnce(new Error('ENOTFOUND'))
+    await expect(isBlockedSubresourceUrl('https://flaky.example/a.js')).resolves.toBe(true)
+
+    mockLookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }])
+    await expect(isBlockedSubresourceUrl('https://flaky.example/a.js')).resolves.toBe(false)
+  })
+
+  it('ignores a malformed url rather than blocking every request', async () => {
+    await expect(isBlockedSubresourceUrl('not a url')).resolves.toBe(false)
+  })
+
+  it('bounds the verdict cache', async () => {
+    for (let i = 0; i < 300; i++) {
+      await isBlockedSubresourceUrl(`https://host-${i}.example/a.js`)
+    }
+    mockLookup.mockClear()
+
+    // The earliest hosts were evicted, so they resolve again; the newest do not.
+    await isBlockedSubresourceUrl('https://host-0.example/a.js')
+    await isBlockedSubresourceUrl('https://host-299.example/a.js')
+    expect(mockLookup).toHaveBeenCalledTimes(1)
+  })
+})
+
+describe('subresourceNeedsResolution', () => {
+  it('exempts only the high-volume, non-readable types', () => {
+    expect(subresourceNeedsResolution('image')).toBe(false)
+    expect(subresourceNeedsResolution('font')).toBe(false)
+  })
+
+  it('checks every type that is readable or executes', () => {
+    for (const type of ['xhr', 'webSocket', 'media', 'script', 'stylesheet', 'object', 'ping']) {
+      expect(subresourceNeedsResolution(type)).toBe(true)
+    }
+  })
+
+  it('checks an unrecognised label rather than exempting it', () => {
+    // fetch is `xhr` on some Chromium versions and `other` on others; a label
+    // this code has never heard of must not be the one that skips the check.
+    expect(subresourceNeedsResolution('other')).toBe(true)
+    expect(subresourceNeedsResolution('someFutureType')).toBe(true)
+  })
+})

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

@@ -129,6 +129,104 @@ export async function checkAgentUrl(rawUrl: string): Promise<UrlGuardResult> {
  * lookup on every subresource. Hostnames pass here (they are classified at
  * navigation time by {@link checkAgentUrl}).
  */
+/**
+ * Subresource types that keep the cheap synchronous literal-IP check.
+ *
+ * Images and fonts are the high-volume types and are not readable
+ * cross-origin, so the residual for them is a load/error timing oracle — a
+ * documented, accepted trade against a DNS lookup per asset.
+ */
+const LITERAL_ONLY_RESOURCE_TYPES: ReadonlySet<string> = new Set(['image', 'font'])
+
+/**
+ * Whether a subresource needs the DNS-resolving check rather than the literal-IP
+ * backstop.
+ *
+ * Expressed as what is exempt rather than what is checked, so a resource type
+ * Chromium labels differently than expected fails safe into the checked path —
+ * `fetch` surfaces as `xhr` or `other` depending on version, and an allowlist
+ * that missed the label in use would silently reopen the hole.
+ */
+export function subresourceNeedsResolution(resourceType: string): boolean {
+  return !LITERAL_ONLY_RESOURCE_TYPES.has(resourceType)
+}
+
+/**
+ * How long a host's resolved classification is reused. Deliberately short: a
+ * DNS rebind should not stay authorized past roughly the life of a page view.
+ */
+const HOST_VERDICT_TTL_MS = 30_000
+
+/**
+ * Ceiling on the cache. A hostile page can name unlimited hostnames, so this is
+ * bounded rather than left to grow; the oldest entry is evicted first.
+ */
+const MAX_HOST_VERDICTS = 256
+
+const hostVerdicts = new Map<string, { blocked: boolean; expiry: number }>()
+
+function rememberHostVerdict(host: string, blocked: boolean): void {
+  if (hostVerdicts.size >= MAX_HOST_VERDICTS) {
+    const oldest = hostVerdicts.keys().next()
+    if (!oldest.done) hostVerdicts.delete(oldest.value)
+  }
+  hostVerdicts.set(host, { blocked, expiry: Date.now() + HOST_VERDICT_TTL_MS })
+}
+
+/** Drops every cached host classification. */
+export function clearHostVerdictCache(): void {
+  hostVerdicts.clear()
+}
+
+/**
+ * DNS-resolving guard for the agent partition's readable and executable
+ * subresources.
+ *
+ * {@link isBlockedRequestUrl} only sees literal IPs, so a public hostname whose
+ * A record points at an RFC1918 or link-local address reached internal services
+ * from a page the agent was steered to — no rebinding needed, a static record
+ * was enough. The vectors that matter are the ones where the response comes
+ * back or runs: `new WebSocket('ws://internal/…')` reads data frames
+ * cross-origin because internal servers commonly ignore `Origin`, and a script
+ * or xhr response either executes in the page or is readable.
+ *
+ * Fails closed on a resolver error, for the same reason {@link checkAgentUrl}
+ * does: an unresolved host cannot be confirmed public, and Chromium resolves
+ * independently. That verdict is not cached, so a transient failure does not
+ * stick.
+ */
+export async function isBlockedSubresourceUrl(rawUrl: string): Promise<boolean> {
+  let hostname: string
+  try {
+    hostname = new URL(rawUrl).hostname
+  } catch {
+    return false
+  }
+  const host = unwrapIpv6Brackets(hostname)
+  if (!host) return false
+  if (isIpLiteral(host)) return isBlockedAddress(host)
+
+  const cached = hostVerdicts.get(host)
+  if (cached && Date.now() < cached.expiry) return cached.blocked
+
+  let blocked: boolean
+  try {
+    const resolved = await resolveHost(host)
+    blocked = resolved.some(({ address }) => isBlockedAddress(address))
+  } catch (error) {
+    logger.warn('Agent subresource host did not resolve; blocking', {
+      host,
+      error: getErrorMessage(error),
+    })
+    return true
+  }
+  rememberHostVerdict(host, blocked)
+  if (blocked) {
+    logger.warn('Blocked agent subresource resolving to private IP', { host })
+  }
+  return blocked
+}
+
 export function isBlockedRequestUrl(rawUrl: string): boolean {
   try {
     // isPrivateIpHost strips IPv6 brackets itself; unwrap again for the