diff --git a/apps/desktop/README.md b/apps/desktop/README.md index d00c1c1d96b..14f59cf097d 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -137,7 +137,7 @@ Good fits for the bridge: OS notifications + dock badge on workflow completion, The main Copilot agent can inspect a user-selected local directory in the desktop app through request-local client tools (`local_mount_directory`, `local_list_mounts`, `local_list`, `local_glob`, `local_read`, `local_grep`, `local_stat`, and `local_forget_mount`). This capability is: - **Explicit and read-only:** the native folder picker creates the grant; there are no write/delete/execute operations. -- **Remembered securely:** grants are encrypted in Electron's private app data with OS-backed `safeStorage` and restored with the same opaque URI after a normal app restart. Sandboxed macOS builds also retain the security-scoped bookmark. There is no plaintext fallback: when secure storage is unavailable, the returned mount has `remembered: false` and lasts only for that app session. +- **Remembered securely:** grants are encrypted in Electron's private app data with OS-backed `safeStorage` and restored with the same opaque URI after a normal app restart. (A security-scoped bookmark is stored alongside each grant, but it is a no-op in the current Developer ID build — only the macOS App Sandbox consumes it — and is kept purely for forward-compatibility should a sandboxed/MAS build ever ship.) There is no plaintext fallback: when secure storage is unavailable, the returned mount has `remembered: false` and lasts only for that app session. - **Revocable:** `local_forget_mount` removes one grant. All grants are removed on explicit sign-out or server-origin change so another Sim account or server cannot inherit them. Normal app quit only releases active OS handles and keeps the encrypted grants. - **Opaque:** renderer and model see only `localfs:///...` URIs. Electron resolves every URI, checks lexical and realpath containment, and refuses symlink escapes. - **Desktop-only:** the web app advertises these request-local tools only when `window.simDesktop.localFilesystem` is present. They are available directly to the main agent and are not added to subagent allowlists. diff --git a/apps/desktop/src/main/browser-agent/cdp.ts b/apps/desktop/src/main/browser-agent/cdp.ts index a9c9d2fead2..b9cc6037f2d 100644 --- a/apps/desktop/src/main/browser-agent/cdp.ts +++ b/apps/desktop/src/main/browser-agent/cdp.ts @@ -27,7 +27,9 @@ export interface CdpCallbacks { onFileChooser: () => void } -let callbacks: CdpCallbacks | null = null +/** Per-tab callbacks, so a background tab's events reach ITS driver, not the + * most-recently-instrumented tab's. */ +const callbacksByContents = new WeakMap() /** Contents already instrumented (attach survives for the tab's lifetime). */ const instrumented = new WeakSet() @@ -41,7 +43,7 @@ async function send( /** Idempotently instruments a tab's WebContents. */ export async function ensureInstrumented(contents: WebContents, cb: CdpCallbacks): Promise { - callbacks = cb + callbacksByContents.set(contents, cb) if (instrumented.has(contents) && contents.debugger.isAttached()) return if (!contents.debugger.isAttached()) { @@ -65,6 +67,7 @@ function handleDebuggerEvent( method: string, params: Record ): void { + const callbacks = callbacksByContents.get(contents) if (method === 'Page.javascriptDialogOpening') { const type = String(params.type ?? 'dialog') const message = String(params.message ?? '').slice(0, 500) diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index e7ec8a90b8d..f4c6c691cfb 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -22,6 +22,8 @@ import type { BrowserToolName, } from '@sim/browser-protocol' import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' +import { sleep } from '@sim/utils/helpers' import type { BrowserWindow, WebContents } from 'electron' import * as cdp from '@/main/browser-agent/cdp' import { @@ -39,6 +41,7 @@ import { typeIntoElement, } from '@/main/browser-agent/page-functions' import * as session from '@/main/browser-agent/session' +import { checkAgentUrl } from '@/main/browser-agent/url-guard' const logger = createLogger('BrowserAgentDriver') @@ -118,7 +121,7 @@ function instrumentTab(contents: WebContents): void { }) .catch((error) => { logger.warn('CDP instrumentation failed', { - error: error instanceof Error ? error.message : String(error), + error: getErrorMessage(error), }) }) for (const event of [ @@ -160,10 +163,6 @@ export function setPanelBounds(bounds: BrowserPanelBounds | null): void { session.setPanelBounds(bounds) } -function sleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)) -} - function str(params: Record, key: string): string | undefined { const value = params[key] return typeof value === 'string' && value.length > 0 ? value : undefined @@ -191,10 +190,6 @@ function requireNum(params: Record, key: string): number { return value } -// --------------------------------------------------------------------------- -// Page-function execution -// --------------------------------------------------------------------------- - /** * Serializes a self-contained page function and executes it in the page's * main world with JSON-encoded arguments (Electron's executeJavaScript has no @@ -209,7 +204,7 @@ async function execInPage( try { return (await contents.executeJavaScript(expression, true)) as Result } catch (error) { - const message = error instanceof Error ? error.message : String(error) + const message = getErrorMessage(error) throw new ToolError( `Cannot act on this page (${message}). Browser-internal pages cannot be automated — ` + 'navigate to a regular website first.' @@ -249,10 +244,6 @@ function unwrapPageResult(result: unknown): unknown { return result } -// --------------------------------------------------------------------------- -// Navigation -// --------------------------------------------------------------------------- - function waitForLoadComplete(contents: WebContents, timeoutMs: number): Promise { return new Promise((resolve) => { let settled = false @@ -278,10 +269,6 @@ async function navigationResult(contents: WebContents): Promise) : {} } -// --------------------------------------------------------------------------- -// Takeover -// --------------------------------------------------------------------------- - /** * Hands control to the user: the page is already natively interactive in the * panel, and the chat's takeover tool row shows the reason with a Done chip. @@ -481,10 +460,6 @@ async function runTakeover(): Promise { } } -// --------------------------------------------------------------------------- -// Tool implementations -// --------------------------------------------------------------------------- - async function executeToolInner( tool: BrowserToolName, params: Record @@ -492,8 +467,9 @@ async function executeToolInner( switch (tool) { case 'browser_navigate': { const url = requireStr(params, 'url') - if (!/^https?:\/\//i.test(url)) { - throw new ToolError('URL must be absolute and start with http:// or https://') + const guard = await checkAgentUrl(url) + if (!guard.ok) { + throw new ToolError(guard.error ?? 'That address was blocked.') } const tab = session.ensureTab() const contents = tab.view.webContents @@ -521,12 +497,15 @@ async function executeToolInner( case 'browser_open_tab': { const url = str(params, 'url') + if (url) { + const guard = await checkAgentUrl(url) + if (!guard.ok) { + throw new ToolError(guard.error ?? 'That address was blocked.') + } + } const tab = session.addTab() const contents = tab.view.webContents if (url) { - if (!/^https?:\/\//i.test(url)) { - throw new ToolError('URL must be absolute and start with http:// or https://') - } void contents.loadURL(url).catch(() => {}) const result = await navigationResult(contents) return { tabId: tab.id, ...result } @@ -723,10 +702,6 @@ function withNotices(result: unknown): unknown { return { value: result, notices } } -// --------------------------------------------------------------------------- -// Public entry points -// --------------------------------------------------------------------------- - /** One real browser can only do one thing at a time — serialize tool calls. */ let toolQueue: Promise = Promise.resolve() @@ -756,7 +731,7 @@ export async function executeTool( try { return { ok: true, result: await settled } } catch (error) { - const message = error instanceof Error ? error.message : String(error) + const message = getErrorMessage(error) logger.warn('Browser tool failed', { tool, error: message }) return { ok: false, error: message } } diff --git a/apps/desktop/src/main/browser-agent/session.test.ts b/apps/desktop/src/main/browser-agent/session.test.ts index 1148467cac3..4e19a62d769 100644 --- a/apps/desktop/src/main/browser-agent/session.test.ts +++ b/apps/desktop/src/main/browser-agent/session.test.ts @@ -2,6 +2,13 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) +vi.mock('@/main/browser-agent/url-guard', () => ({ + checkAgentUrl: vi.fn(async (url: string) => ({ + ok: /^https?:\/\//i.test(url) && !url.includes('169.254.169.254'), + })), + isBlockedRequestUrl: vi.fn((url: string) => url.includes('169.254.169.254')), +})) + import { BrowserWindow } from 'electron' type SessionModule = typeof import('@/main/browser-agent/session') @@ -11,6 +18,7 @@ interface MockView { session: { setPermissionRequestHandler: ReturnType setPermissionCheckHandler: ReturnType + webRequest: { onBeforeRequest: ReturnType } } setWindowOpenHandler: ReturnType loadURL: ReturnType @@ -133,6 +141,8 @@ describe('browser-agent session', () => { const openHandler = contents.setWindowOpenHandler.mock.calls[0][0] as (details: { url: string }) => { action: string } + // window.open collapses into the same view; the SSRF check runs on the + // resulting document request in onBeforeRequest, not here. expect(openHandler({ url: 'https://example.com/popup' })).toEqual({ action: 'deny' }) expect(contents.loadURL).toHaveBeenCalledWith('https://example.com/popup') // Non-http(s) popups are denied without navigating anywhere. @@ -141,6 +151,38 @@ describe('browser-agent session', () => { expect(contents.loadURL).not.toHaveBeenCalled() }) + it('gates agent-partition requests: DNS-checks documents, literal-checks subresources', async () => { + const tab = session.ensureTab() + const contents = (tab.view as unknown as MockView).webContents + const onBeforeRequest = contents.session.webRequest.onBeforeRequest.mock.calls[0][0] as ( + details: { url: string; resourceType: string }, + callback: (response: { cancel?: boolean }) => void + ) => void + + // A document navigation to a private host is cancelled by the DNS check. + const blockedDoc = vi.fn() + onBeforeRequest( + { url: 'http://169.254.169.254/latest/meta-data', resourceType: 'mainFrame' }, + blockedDoc + ) + await vi.waitFor(() => expect(blockedDoc).toHaveBeenCalledWith({ cancel: true })) + + // A document navigation to a public host is allowed. + const allowedDoc = vi.fn() + onBeforeRequest({ url: 'https://example.com/', resourceType: 'subFrame' }, allowedDoc) + await vi.waitFor(() => expect(allowedDoc).toHaveBeenCalledWith({ cancel: false })) + + // A subresource to a literal private IP is cancelled synchronously. + const blockedSub = vi.fn() + onBeforeRequest({ url: 'http://169.254.169.254/x.js', resourceType: 'image' }, blockedSub) + expect(blockedSub).toHaveBeenCalledWith({ cancel: true }) + + // A public subresource passes. + const allowedSub = vi.fn() + onBeforeRequest({ url: 'https://example.com/x.js', resourceType: 'script' }, allowedSub) + expect(allowedSub).toHaveBeenCalledWith({ cancel: false }) + }) + it('permission handlers deny every request on the agent partition', () => { const tab = session.ensureTab() const ses = (tab.view as unknown as MockView).webContents.session diff --git a/apps/desktop/src/main/browser-agent/session.ts b/apps/desktop/src/main/browser-agent/session.ts index a1a7cc22425..1d43efd877f 100644 --- a/apps/desktop/src/main/browser-agent/session.ts +++ b/apps/desktop/src/main/browser-agent/session.ts @@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger' import type { BrowserWindow, Session, WebContents } from 'electron' import { WebContentsView } from 'electron' import { registerAgentWebContents } from '@/main/browser-agent/registry' +import { checkAgentUrl, isBlockedRequestUrl } from '@/main/browser-agent/url-guard' const logger = createLogger('BrowserAgentSession') @@ -24,7 +25,7 @@ export interface AgentSessionEvents { /** The active tab changed (new tab, switch, close). */ onActiveTabChanged: (contents: WebContents) => void /** A download was blocked on the agent partition. */ - onDownloadBlocked: (filename: string, url: string) => void + onDownloadBlocked: (filename: string) => void } /** @@ -68,12 +69,38 @@ function configureAgentPartition(ses: Session): void { partitionConfigured = true ses.setPermissionRequestHandler((_wc, _permission, callback) => callback(false)) ses.setPermissionCheckHandler(() => false) + // The SSRF choke point for the agent partition. Document navigations + // (top-level + iframes) get the full DNS-resolving check — this is the ONE + // seam every navigation passes through, including the page-initiated ones the + // driver never sees (server redirects, link clicks, location.href, + // meta-refresh), so an internal-resolving hostname can't slip in that way. + // Subresources take the cheap synchronous literal-IP backstop instead of a + // DNS lookup per asset (a hostname subresource is already gated by its + // document's check). + ses.webRequest.onBeforeRequest((details, callback) => { + if (details.resourceType === 'mainFrame' || details.resourceType === 'subFrame') { + void checkAgentUrl(details.url) + .then((guard) => { + if (!guard.ok) { + logger.warn('Blocked agent document navigation to a private host') + } + callback({ cancel: !guard.ok }) + }) + .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 }) + }) + return + } + callback({ cancel: isBlockedRequestUrl(details.url) }) + }) ses.on('will-download', (_event, item) => { const filename = item.getFilename() - const url = item.getURL() logger.info('Blocked download in agent browser', { filename }) item.cancel() - events?.onDownloadBlocked(filename, url) + events?.onDownloadBlocked(filename) }) } @@ -97,7 +124,9 @@ function createTabView(): WebContentsView { configureAgentPartition(contents.session) // Popup-neutralize: window.open and target=_blank navigate the SAME view - // instead of spawning windows — the agent browses one page per tab. + // instead of spawning windows — the agent browses one page per tab. The + // resulting navigation is a document request, so the partition's + // onBeforeRequest SSRF check gates it; no per-call guard is needed here. contents.setWindowOpenHandler((details) => { if (/^https?:\/\//i.test(details.url)) { void contents.loadURL(details.url).catch(() => {}) diff --git a/apps/desktop/src/main/browser-agent/url-guard.test.ts b/apps/desktop/src/main/browser-agent/url-guard.test.ts new file mode 100644 index 00000000000..f9fa6b97ced --- /dev/null +++ b/apps/desktop/src/main/browser-agent/url-guard.test.ts @@ -0,0 +1,80 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockLookup } = vi.hoisted(() => ({ mockLookup: vi.fn() })) + +vi.mock('node:dns/promises', () => ({ + default: { lookup: mockLookup }, +})) + +import { checkAgentUrl, isBlockedRequestUrl } from '@/main/browser-agent/url-guard' + +describe('checkAgentUrl', () => { + beforeEach(() => { + vi.clearAllMocks() + mockLookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]) + }) + + it('rejects non-http(s) schemes without resolving', async () => { + const result = await checkAgentUrl('file:///etc/passwd') + expect(result.ok).toBe(false) + expect(mockLookup).not.toHaveBeenCalled() + }) + + it('rejects malformed URLs', async () => { + expect((await checkAgentUrl('not a url')).ok).toBe(false) + }) + + it('blocks private IP literals without resolving', async () => { + expect((await checkAgentUrl('http://127.0.0.1/')).ok).toBe(false) + expect((await checkAgentUrl('http://169.254.169.254/latest/meta-data')).ok).toBe(false) + expect((await checkAgentUrl('http://10.0.0.5/')).ok).toBe(false) + expect((await checkAgentUrl('http://[::1]/')).ok).toBe(false) + expect(mockLookup).not.toHaveBeenCalled() + }) + + it('allows public IP literals without resolving', async () => { + expect((await checkAgentUrl('https://8.8.8.8/')).ok).toBe(true) + expect(mockLookup).not.toHaveBeenCalled() + }) + + it('allows hostnames that resolve to public addresses', async () => { + const result = await checkAgentUrl('https://example.com/page') + expect(result.ok).toBe(true) + expect(mockLookup).toHaveBeenCalledWith('example.com', { all: true, verbatim: true }) + }) + + it('blocks hostnames that resolve to a private address (DNS rebinding)', async () => { + mockLookup.mockResolvedValue([{ address: '10.1.2.3', family: 4 }]) + expect((await checkAgentUrl('https://rebind.evil.test/')).ok).toBe(false) + }) + + it('blocks when any resolved address is private', async () => { + mockLookup.mockResolvedValue([ + { address: '93.184.216.34', family: 4 }, + { address: '192.168.0.9', family: 4 }, + ]) + expect((await checkAgentUrl('https://mixed.test/')).ok).toBe(false) + }) + + it('fails closed when DNS resolution fails', async () => { + mockLookup.mockRejectedValue(new Error('ENOTFOUND')) + expect((await checkAgentUrl('https://nope.invalid/')).ok).toBe(false) + }) +}) + +describe('isBlockedRequestUrl', () => { + it('blocks literal private/reserved hosts', () => { + expect(isBlockedRequestUrl('http://169.254.169.254/latest/meta-data')).toBe(true) + expect(isBlockedRequestUrl('http://127.0.0.1:8080/x')).toBe(true) + expect(isBlockedRequestUrl('https://[fd00::1]/')).toBe(true) + }) + + it('allows public literals and hostnames (classified at nav time)', () => { + expect(isBlockedRequestUrl('https://8.8.8.8/')).toBe(false) + expect(isBlockedRequestUrl('https://example.com/x')).toBe(false) + }) + + it('does not throw on malformed input', () => { + expect(isBlockedRequestUrl('::::')).toBe(false) + }) +}) diff --git a/apps/desktop/src/main/browser-agent/url-guard.ts b/apps/desktop/src/main/browser-agent/url-guard.ts new file mode 100644 index 00000000000..ab335764276 --- /dev/null +++ b/apps/desktop/src/main/browser-agent/url-guard.ts @@ -0,0 +1,88 @@ +import dns from 'node:dns/promises' +import { createLogger } from '@sim/logger' +import { isIpLiteral, isPrivateIp, isPrivateIpHost, unwrapIpv6Brackets } from '@sim/security/ssrf' +import { getErrorMessage } from '@sim/utils/errors' +import { parseHttpUrl } from '@/main/navigation' + +const logger = createLogger('BrowserAgentUrlGuard') + +export interface UrlGuardResult { + ok: boolean + error?: string +} + +const OK: UrlGuardResult = { ok: true } +const BLOCKED: UrlGuardResult = { + ok: false, + error: 'That address points to a private or internal network and was blocked.', +} + +/** + * SSRF guard for agent-browser navigation. The embedded browser is a + * general-purpose surface driven by model/tool input, so a navigation to a + * loopback/RFC1918/link-local host (e.g. the `169.254.169.254` cloud-metadata + * endpoint) would let a page's contents be read back through the read/snapshot + * tools. This resolves the host the same way `apps/sim` does for outbound + * fetches and blocks any that land on a private/reserved address. + * + * IP literals are classified directly; hostnames are DNS-resolved and every + * returned address is checked. Resolution failure fails CLOSED (blocks): we + * can't confirm the host is public, and Chromium resolves independently, so it + * could still reach a private address our lookup missed — matching + * `validateUrlWithDNS` in `apps/sim`. The residual DNS-rebinding TOCTOU window + * (our lookup vs Chromium's) is only fully closable with egress firewalling; + * {@link isBlockedRequestUrl} adds a synchronous per-request literal-IP backstop + * for redirects and subresources. + */ +export async function checkAgentUrl(rawUrl: string): Promise { + const url = parseHttpUrl(rawUrl) + if (!url) { + return { ok: false, error: 'URL must be absolute and start with http:// or https://' } + } + + const host = unwrapIpv6Brackets(url.hostname) + + // IP literal: classify directly, no DNS lookup needed. + if (isIpLiteral(host)) { + if (isPrivateIp(host)) { + logger.warn('Blocked agent navigation to private IP literal', { host }) + return BLOCKED + } + return OK + } + + try { + const resolved = await dns.lookup(host, { all: true, verbatim: true }) + if (resolved.some(({ address }) => isPrivateIp(address))) { + logger.warn('Blocked agent navigation resolving to private IP', { host }) + return BLOCKED + } + } catch (error) { + // Fail closed: an unresolved host can't be confirmed public, and Chromium + // resolves independently, so it could still reach a private address. + logger.warn('Agent navigation host did not resolve; blocking', { + host, + error: getErrorMessage(error), + }) + return { ok: false, error: 'That address could not be resolved.' } + } + + return OK +} + +/** + * Synchronous backstop for the agent partition's `onBeforeRequest`: blocks any + * request whose host is a **literal** private/reserved IP. This is cheap enough + * to run per-request and catches redirects and subresources that target the + * metadata endpoint or an internal IP directly, without the cost of a DNS + * lookup on every subresource. Hostnames pass here (they are classified at + * navigation time by {@link checkAgentUrl}). + */ +export function isBlockedRequestUrl(rawUrl: string): boolean { + try { + // isPrivateIpHost strips IPv6 brackets itself. + return isPrivateIpHost(new URL(rawUrl).hostname) + } catch { + return false + } +} diff --git a/apps/desktop/src/main/config.ts b/apps/desktop/src/main/config.ts index 247362a15a3..6d5c6167be4 100644 --- a/apps/desktop/src/main/config.ts +++ b/apps/desktop/src/main/config.ts @@ -1,14 +1,12 @@ import { mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs' import { dirname } from 'node:path' import { createLogger } from '@sim/logger' +import { isLoopbackHostname } from '@sim/security/ssrf' const logger = createLogger('DesktopConfig') export const DEFAULT_ORIGIN = 'https://sim.ai' -/** Loopback hostnames that may use plain HTTP (dev + self-host testing). */ -export const LOCAL_HOSTNAMES = new Set(['localhost', '127.0.0.1', '[::1]', '::1']) - export interface WindowBounds { x?: number y?: number @@ -52,7 +50,7 @@ export function validateOriginInput(raw: string): OriginValidation { if (url.protocol === 'https:') { return { ok: true, origin: url.origin } } - if (url.protocol === 'http:' && LOCAL_HOSTNAMES.has(url.hostname)) { + if (url.protocol === 'http:' && isLoopbackHostname(url.hostname)) { return { ok: true, origin: url.origin } } return { ok: false, error: 'Server URL must use HTTPS (HTTP is allowed for localhost only)' } diff --git a/apps/desktop/src/main/csp.test.ts b/apps/desktop/src/main/csp.test.ts new file mode 100644 index 00000000000..1319ca314fb --- /dev/null +++ b/apps/desktop/src/main/csp.test.ts @@ -0,0 +1,83 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { attachCspFallback, DEFAULT_DESKTOP_CSP } from '@/main/csp' + +type HeadersReceivedHandler = ( + details: { + url: string + resourceType: string + responseHeaders?: Record + }, + callback: (response: { responseHeaders?: Record }) => void +) => void + +function fakeSession() { + let handler: HeadersReceivedHandler | undefined + const ses = { + webRequest: { + onHeadersReceived: vi.fn((h: HeadersReceivedHandler) => { + handler = h + }), + }, + } + return { ses, run: () => handler } +} + +const APP_ORIGIN = 'https://sim.ai' + +describe('attachCspFallback', () => { + let session: ReturnType + + beforeEach(() => { + session = fakeSession() + attachCspFallback( + session.ses as unknown as Parameters[0], + () => APP_ORIGIN + ) + }) + + it('injects the fallback CSP on an app-origin document lacking one', () => { + const cb = vi.fn() + session.run()?.( + { url: `${APP_ORIGIN}/workspace`, resourceType: 'mainFrame', responseHeaders: {} }, + cb + ) + expect(cb).toHaveBeenCalledWith({ + responseHeaders: { 'Content-Security-Policy': [DEFAULT_DESKTOP_CSP] }, + }) + }) + + it('never overrides a server-sent CSP', () => { + const cb = vi.fn() + session.run()?.( + { + url: `${APP_ORIGIN}/workspace`, + resourceType: 'mainFrame', + responseHeaders: { 'content-security-policy': ["default-src 'self'"] }, + }, + cb + ) + expect(cb).toHaveBeenCalledWith({}) + }) + + it('leaves subresources untouched', () => { + const cb = vi.fn() + session.run()?.( + { url: `${APP_ORIGIN}/app.js`, resourceType: 'script', responseHeaders: {} }, + cb + ) + expect(cb).toHaveBeenCalledWith({}) + }) + + it('leaves non-app-origin documents untouched', () => { + const cb = vi.fn() + session.run()?.( + { + url: 'https://accounts.google.com/o/oauth2', + resourceType: 'mainFrame', + responseHeaders: {}, + }, + cb + ) + expect(cb).toHaveBeenCalledWith({}) + }) +}) diff --git a/apps/desktop/src/main/csp.ts b/apps/desktop/src/main/csp.ts new file mode 100644 index 00000000000..5d956f21e26 --- /dev/null +++ b/apps/desktop/src/main/csp.ts @@ -0,0 +1,47 @@ +import { createLogger } from '@sim/logger' +import type { Session } from 'electron' +import { isAppOrigin } from '@/main/navigation' + +const logger = createLogger('DesktopCsp') + +/** + * A minimal, non-drifting Content-Security-Policy applied ONLY to an app-origin + * top-level document whose response ships no CSP of its own. + * + * The hosted web app sends a full, env-aware policy on every response (see + * `apps/sim/lib/core/security/csp.ts`), which the shell can neither import + * (monorepo boundary) nor safely duplicate (it varies by env). This is a + * defense-in-depth backstop for the narrow case where that header is somehow + * absent — a deliberately small subset of the server's own base directives, so + * it can never be stricter than what the app already depends on and cannot + * break embeds or integrations. + */ +export const DEFAULT_DESKTOP_CSP = "frame-ancestors 'self'; object-src 'none'; base-uri 'self'" + +function hasCspHeader(headers: Record): boolean { + return Object.keys(headers).some((key) => key.toLowerCase() === 'content-security-policy') +} + +/** + * Installs the CSP fallback on a session. Runs on `onHeadersReceived` (a + * distinct event from telemetry-policy's `onBeforeRequest`, so the two coexist) + * and leaves every response untouched except an app-origin main-frame document + * that carries no CSP, which gets {@link DEFAULT_DESKTOP_CSP}. + */ +export function attachCspFallback(ses: Session, appOrigin: () => string): void { + ses.webRequest.onHeadersReceived((details, callback) => { + const headers = details.responseHeaders ?? {} + if ( + details.resourceType === 'mainFrame' && + isAppOrigin(details.url, appOrigin()) && + !hasCspHeader(headers) + ) { + logger.info('Injecting fallback CSP for app document without one') + callback({ + responseHeaders: { ...headers, 'Content-Security-Policy': [DEFAULT_DESKTOP_CSP] }, + }) + return + } + callback({}) + }) +} diff --git a/apps/desktop/src/main/handoff.test.ts b/apps/desktop/src/main/handoff.test.ts index 86b492ddcbe..6673858608f 100644 --- a/apps/desktop/src/main/handoff.test.ts +++ b/apps/desktop/src/main/handoff.test.ts @@ -79,7 +79,7 @@ describe('createHandoffManager', () => { manager.clear() }) - it('loopback accepts one valid callback, rejects bad input, then closes', async () => { + it('keeps the loopback open past malformed and wrong-state requests, closes on consume', async () => { const received: HandoffCallback[] = [] const deps = makeDeps() const manager = createHandoffManager(deps, (callback) => received.push(callback)) @@ -91,17 +91,32 @@ describe('createHandoffManager', () => { expect((await fetch(`${base}/other`)).status).toBe(404) + // Malformed input is rejected outright and never fires the callback. const badToken = await fetch(`${base}/auth/callback?token=bad token&state=${state}`) expect(badToken.status).toBe(400) expect(received).toHaveLength(0) + // A format-valid but WRONG state must NOT tear down the one-shot listener + // (self-DoS guard) and must NOT fire the callback — firing it would surface + // a spurious "sign-in failed" UI while the genuine callback is still pending. + const wrongState = 'z'.repeat(state.length) + expect( + (await fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${wrongState}`)).status + ).toBe(200) + expect(received).toHaveLength(0) + expect(manager.consume(wrongState)).toBe(false) + + // The genuine callback lands and immediately tears the listener down, so a + // refresh/retry of the callback URL can't fire a second handleCallback. const ok = await fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`) expect(ok.status).toBe(200) expect(received).toEqual([{ token: VALID_TOKEN, state }]) - await expect( fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`) ).rejects.toThrow() + + // consume() still validates the pending state for the redeem path. + expect(manager.consume(state)).toBe(true) }) it('cleans up the pending handoff when the browser cannot be opened', async () => { diff --git a/apps/desktop/src/main/handoff.ts b/apps/desktop/src/main/handoff.ts index 118082333e9..2b19d9ed3af 100644 --- a/apps/desktop/src/main/handoff.ts +++ b/apps/desktop/src/main/handoff.ts @@ -14,10 +14,12 @@ const STATE_PATTERN = /^[A-Za-z0-9_-]{16,256}$/ const STATE_LENGTH = 32 const REDEEM_PATH = '/api/auth/one-time-token/verify' const CALLBACK_PATH = '/auth/callback' -// Measured from begin() (when the browser opens) so it comfortably covers a -// full interactive login — email/OTP round-trips or OAuth consent — not just -// the redirect back. Bounds how long the loopback listener and the CSRF state -// stay valid. +/** + * Measured from begin() (when the browser opens) so it comfortably covers a + * full interactive login — email/OTP round-trips or OAuth consent — not just + * the redirect back. Bounds how long the loopback listener and the CSRF state + * stay valid. + */ const HANDOFF_TTL_MS = 30 * 60 * 1000 const CALLBACK_RESPONSE_HTML = ` @@ -70,6 +72,14 @@ export function createHandoffManager( } } + /** Constant-time, non-consuming check that a callback's state matches the live + * pending handoff. Gates the callback so a wrong/expired state is ignored + * without consuming the pending handoff or surfacing a failure. */ + const matchesPendingState = (state: string): boolean => + pending !== null && + now() - pending.createdAt <= HANDOFF_TTL_MS && + safeCompare(pending.state, state) + const startLoopback = async (): Promise => { stopLoopback() const server = createServer((request, response) => { @@ -87,8 +97,17 @@ export function createHandoffManager( response .writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }) .end(CALLBACK_RESPONSE_HTML) - stopLoopback() - onCallback({ token, state }) + // Only a state match advances the flow. A format-valid but wrong/expired + // state gets the generic 200 page and is otherwise ignored, so the + // one-shot listener survives for the genuine callback (TTL is the + // backstop) and no spurious "sign-in failed" UI fires. A genuine match + // tears the listener down immediately — before the async redeem — so a + // refresh/retry of the callback URL can't fire a second handleCallback + // whose consume() would fail and surface a false "sign-in failed". + if (matchesPendingState(state)) { + stopLoopback() + onCallback({ token, state }) + } }) loopbackServer = server try { diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 2303584b777..51b11b22600 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -1,13 +1,14 @@ import { join } from 'node:path' import { createLogger } from '@sim/logger' import type { BrowserWindow } from 'electron' -import { app, net, session } from 'electron' +import { app, crashReporter, net, session } from 'electron' import { initDriver as initBrowserAgentDriver, setPanelBounds as setBrowserAgentPanelBounds, } from '@/main/browser-agent/driver' import { createConfigStore, partitionForOrigin } from '@/main/config' import { attachContextMenu } from '@/main/context-menu' +import { attachCspFallback } from '@/main/csp' import { attachDownloadHandling } from '@/main/downloads' import { createAuthFlow, createHandoffManager } from '@/main/handoff' import { registerIpcHandlers } from '@/main/ipc' @@ -100,6 +101,7 @@ function main(): void { } configuredPartitions.add(partition) setupPermissionHandlers(ses, appOrigin) + attachCspFallback(ses, appOrigin) attachDownloadHandling(ses, events) attachTelemetryPolicy(ses, config.get('blockThirdPartyAnalytics') ?? true) ses.setSpellCheckerLanguages(['en-US']) @@ -138,7 +140,7 @@ function main(): void { }) loadHealth = attachLoadHealth(win, { offlinePagePath: OFFLINE_PAGE, - getStartUrl: () => `${appOrigin()}${decideStartRoute('unknown', config.get('lastRoute'))}`, + getStartUrl: () => `${appOrigin()}${decideStartRoute(config.get('lastRoute'))}`, isOnline: () => net.isOnline(), events, }) @@ -153,7 +155,7 @@ function main(): void { onReauthRequested: () => void authFlow.beginLoginHandoff(), }) loadHealth.startWatchdog() - const route = decideStartRoute('unknown', config.get('lastRoute')) + const route = decideStartRoute(config.get('lastRoute')) // Fire-and-forget: the window and all its handlers are wired synchronously // above, so callers get a usable window immediately and the app menu and // updater never wait on the remote page's load (load-health surfaces any @@ -389,6 +391,13 @@ if (process.env.SIM_DESKTOP_USER_DATA) { app.setPath('userData', process.env.SIM_DESKTOP_USER_DATA) } +// Capture native minidumps for main/renderer/GPU crashes. Local-only: there is +// no crash-ingest backend, so nothing is uploaded — the dumps land under +// userData/Crashpad and the event log records where. Must start before the app +// is ready so Crashpad initializes first. Set after userData so dumps follow +// any test/instance override. +crashReporter.start({ uploadToServer: false, compress: true }) + const gotSingleInstanceLock = app.requestSingleInstanceLock() if (!gotSingleInstanceLock) { app.quit() diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index cbf9ebbc1fd..9094b46cc38 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -1,5 +1,5 @@ -import type { LauncherShortcutSettings } from '@sim/desktop-bridge' import { isBrowserToolName } from '@sim/browser-protocol' +import type { LauncherShortcutSettings } from '@sim/desktop-bridge' import type { IpcMainEvent, IpcMainInvokeEvent } from 'electron' import { app, ipcMain } from 'electron' import { executeTool, handlePanelAction, setPanelBounds } from '@/main/browser-agent/driver' diff --git a/apps/desktop/src/main/launcher-window.test.ts b/apps/desktop/src/main/launcher-window.test.ts index 62c08e054cb..4ec2f886713 100644 --- a/apps/desktop/src/main/launcher-window.test.ts +++ b/apps/desktop/src/main/launcher-window.test.ts @@ -2,17 +2,17 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) -// Same module instance the vi.mock factory returns, with mock-typed statics. -import { BrowserWindow } from '@/test/electron-mock' import { clampLauncherHeight, createLauncherWindow, LAUNCHER_MAX_HEIGHT, LAUNCHER_MIN_HEIGHT, LAUNCHER_WIDTH, - launcherBoundsFor, type LauncherWindowDeps, + launcherBoundsFor, } from '@/main/launcher-window' +// Same module instance the vi.mock factory returns, with mock-typed statics. +import { BrowserWindow } from '@/test/electron-mock' type MockWindow = InstanceType diff --git a/apps/desktop/src/main/launcher-window.ts b/apps/desktop/src/main/launcher-window.ts index 103eb82fd31..a9170bda465 100644 --- a/apps/desktop/src/main/launcher-window.ts +++ b/apps/desktop/src/main/launcher-window.ts @@ -62,7 +62,6 @@ export interface LauncherWindowHandle { prewarm(): void /** Renderer-driven growth as a response streams in; clamped. */ resize(height: number): void - isVisible(): boolean /** Tears the window down (origin change, quit). Next toggle recreates it. */ destroy(): void } @@ -116,6 +115,16 @@ export function createLauncherWindow(deps: LauncherWindowDeps): LauncherWindowHa } }) + // A failed launcher load hides the panel and falls back to the main window. + const failLaunchLoad = (code: number, reason: string) => { + deps.events.record('launcher_load_failed', { code, reason }) + loadFailed = true + if (panel.isVisible()) { + panel.hide() + deps.openMainWindow() + } + } + // did-fail-load covers network-level failures (offline); an HTTP error // status (older self-hosted server without the route) still "loads", so // it is caught from the navigation's response code instead. @@ -124,26 +133,17 @@ export function createLauncherWindow(deps: LauncherWindowDeps): LauncherWindowHa return } logger.warn('Launcher route failed to load', { code, description, url: scrubUrl(url) }) - deps.events.record('launcher_load_failed', { code, reason: description }) - loadFailed = true - if (panel.isVisible()) { - panel.hide() - deps.openMainWindow() - } + failLaunchLoad(code, description) }) panel.webContents.on('did-navigate', (_event, url, httpResponseCode) => { - if (httpResponseCode >= 400) { - logger.warn('Launcher route returned an error status', { - status: httpResponseCode, - url: scrubUrl(url), - }) - deps.events.record('launcher_load_failed', { code: httpResponseCode, reason: 'http' }) - loadFailed = true - if (panel.isVisible()) { - panel.hide() - deps.openMainWindow() - } + if (httpResponseCode < 400) { + return } + logger.warn('Launcher route returned an error status', { + status: httpResponseCode, + url: scrubUrl(url), + }) + failLaunchLoad(httpResponseCode, 'http') }) panel.webContents.on('render-process-gone', (_event, details) => { if (details.reason !== 'clean-exit') { @@ -212,9 +212,6 @@ export function createLauncherWindow(deps: LauncherWindowDeps): LauncherWindowHa const bounds = win.getBounds() win.setBounds({ ...bounds, height: clampLauncherHeight(height) }) }, - isVisible() { - return Boolean(win && !win.isDestroyed() && win.isVisible()) - }, destroy() { if (win && !win.isDestroyed()) { win.destroy() diff --git a/apps/desktop/src/main/local-filesystem.test.ts b/apps/desktop/src/main/local-filesystem.test.ts index 946ea801d4a..9fbecc0f21a 100644 --- a/apps/desktop/src/main/local-filesystem.test.ts +++ b/apps/desktop/src/main/local-filesystem.test.ts @@ -137,9 +137,9 @@ describe('LocalFilesystemService', () => { expect(traversal).toMatchObject({ ok: false, code: 'ACCESS_DENIED' }) }) - it('clears all grants without touching files on disk', async () => { + it('releases active mounts without touching files on disk', async () => { const granted = await mount(service) - service.clear() + service.close() const response = await service.handle({ operation: 'stat', uri: granted.uri }) expect(response).toMatchObject({ ok: false, code: 'MOUNT_NOT_FOUND' }) diff --git a/apps/desktop/src/main/local-filesystem.ts b/apps/desktop/src/main/local-filesystem.ts index c1d084cd5a4..cfefbd21874 100644 --- a/apps/desktop/src/main/local-filesystem.ts +++ b/apps/desktop/src/main/local-filesystem.ts @@ -206,10 +206,6 @@ export class LocalFilesystemService { return (this.initializePromise ??= this.restoreRememberedMounts()) } - clear(): void { - this.close() - } - /** Release active OS handles while keeping encrypted grants for next launch. */ close(): void { for (const mount of this.mounts.values()) { diff --git a/apps/desktop/src/main/navigation.test.ts b/apps/desktop/src/main/navigation.test.ts index 7a843589fbf..6edfe06592e 100644 --- a/apps/desktop/src/main/navigation.test.ts +++ b/apps/desktop/src/main/navigation.test.ts @@ -139,6 +139,12 @@ describe('classifyWindowOpen', () => { ) }) + it('does not let a cross-origin http URL ride the mcp-oauth frame name in-app', () => { + expect(classifyWindowOpen('http://mcp.example/authorize', 'mcp-oauth-srv1', APP)).toBe( + 'external' + ) + }) + it('collapses internal new-tab opens into the main window', () => { expect(classifyWindowOpen(`${APP}/workspace/ws1/w/wf1`, '', APP)).toBe('popup-internal') }) diff --git a/apps/desktop/src/main/navigation.ts b/apps/desktop/src/main/navigation.ts index d24f6a260f4..eaf7b0b3747 100644 --- a/apps/desktop/src/main/navigation.ts +++ b/apps/desktop/src/main/navigation.ts @@ -1,6 +1,6 @@ import { createLogger } from '@sim/logger' +import { isLoopbackHostname } from '@sim/security/ssrf' import { shell } from 'electron' -import { LOCAL_HOSTNAMES } from '@/main/config' const logger = createLogger('DesktopNavigation') @@ -56,7 +56,7 @@ const AUTH_SURFACE_PREFIXES: readonly string[] = [ const MCP_POPUP_NAME_PREFIX = 'mcp-oauth-' -function parseHttpUrl(raw: string): URL | null { +export function parseHttpUrl(raw: string): URL | null { try { const url = new URL(raw) if (url.protocol !== 'http:' && url.protocol !== 'https:') { @@ -160,7 +160,11 @@ export function classifyWindowOpen( } return 'popup-internal' } - if (frameName.startsWith(MCP_POPUP_NAME_PREFIX)) { + // The MCP OAuth popup opens the provider's cross-origin authorization URL + // (always https). The frame name is renderer-controlled, so gate the in-app + // popup on https too — an http(s-less) page can never ride the mcp-oauth name + // into an in-app window; it goes to the system browser like any external URL. + if (frameName.startsWith(MCP_POPUP_NAME_PREFIX) && url.protocol === 'https:') { return 'popup-mcp' } return 'external' @@ -203,7 +207,7 @@ export function isSafeExternalUrl(raw: string, allowHttpLocalhost = false): bool return true } if (url.protocol === 'http:') { - return allowHttpLocalhost && LOCAL_HOSTNAMES.has(url.hostname) + return allowHttpLocalhost && isLoopbackHostname(url.hostname) } return false } diff --git a/apps/desktop/src/main/session-lifecycle.test.ts b/apps/desktop/src/main/session-lifecycle.test.ts index 5765d56757e..654b4db1964 100644 --- a/apps/desktop/src/main/session-lifecycle.test.ts +++ b/apps/desktop/src/main/session-lifecycle.test.ts @@ -46,19 +46,15 @@ describe('isLogoutNavigation', () => { }) describe('decideStartRoute', () => { - it('routes a signed-out launch to the login surface', () => { - expect(decideStartRoute('invalid', '/workspace/ws1')).toBe('/login') - }) - it('restores the last route when plausible', () => { - expect(decideStartRoute('valid', '/workspace/ws1?tab=logs')).toBe('/workspace/ws1?tab=logs') - expect(decideStartRoute('unknown', '/workspace/ws1')).toBe('/workspace/ws1') + expect(decideStartRoute('/workspace/ws1?tab=logs')).toBe('/workspace/ws1?tab=logs') + expect(decideStartRoute('/workspace/ws1')).toBe('/workspace/ws1') }) it('falls back to /workspace for missing, unsafe, or auth-surface last routes', () => { - expect(decideStartRoute('valid', undefined)).toBe('/workspace') - expect(decideStartRoute('valid', '//evil.example')).toBe('/workspace') - expect(decideStartRoute('valid', '/login')).toBe('/workspace') + expect(decideStartRoute(undefined)).toBe('/workspace') + expect(decideStartRoute('//evil.example')).toBe('/workspace') + expect(decideStartRoute('/login')).toBe('/workspace') }) }) diff --git a/apps/desktop/src/main/session-lifecycle.ts b/apps/desktop/src/main/session-lifecycle.ts index 71f912fa8f8..2d959204382 100644 --- a/apps/desktop/src/main/session-lifecycle.ts +++ b/apps/desktop/src/main/session-lifecycle.ts @@ -52,17 +52,12 @@ export function isLogoutNavigation(rawUrl: string, appOrigin: string): boolean { } /** - * Picks the route to load at launch: a known-signed-out session goes straight - * to the login surface, otherwise the last visited route (when safe and not - * itself an auth surface), falling back to /workspace. + * Picks the route to load at launch: the last visited route when it is safe + * and not itself an auth surface, otherwise /workspace. There is no launch-time + * session probe — the server's own redirect to /login covers a signed-out + * launch (see README "Auth model"). */ -export function decideStartRoute( - sessionState: SessionProbeResult, - lastRoute: string | undefined -): string { - if (sessionState === 'invalid') { - return '/login' - } +export function decideStartRoute(lastRoute: string | undefined): string { if (lastRoute && isSafeInternalPath(lastRoute) && !isAuthSurfacePath(lastRoute)) { return lastRoute } diff --git a/apps/desktop/src/main/tray.test.ts b/apps/desktop/src/main/tray.test.ts index 78abfb32f7e..6e2a9b3ed86 100644 --- a/apps/desktop/src/main/tray.test.ts +++ b/apps/desktop/src/main/tray.test.ts @@ -3,8 +3,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) import { session } from 'electron' -// Same module instance the vi.mock factory returns, with mock-typed statics. -import { Tray } from '@/test/electron-mock' import { buildTrayMenuTemplate, chatRoute, @@ -13,6 +11,8 @@ import { parseRecentChats, type TrayDeps, } from '@/main/tray' +// Same module instance the vi.mock factory returns, with mock-typed statics. +import { Tray } from '@/test/electron-mock' function makeDeps(overrides: Partial = {}): TrayDeps { return { diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts index b3efd941775..336b485165b 100644 --- a/apps/desktop/src/main/updater.ts +++ b/apps/desktop/src/main/updater.ts @@ -132,6 +132,28 @@ export interface UpdaterDeps { blockedVersions?: readonly string[] } +/** + * Loads the electron-updater singleton and (re)applies the channel and safety + * invariants — channel scoped to the running build, downgrades refused. Both + * the launch wiring and the manual check go through here so neither can run + * against a mis-set channel or an allowDowngrade default. `autoInstallOnAppQuit` + * is deliberately NOT touched: it is toggled per accepted download and must + * survive a manual check. + */ +function configureAutoUpdater(): typeof import('electron-updater')['autoUpdater'] | null { + try { + const { autoUpdater } = require('electron-updater') as typeof import('electron-updater') + autoUpdater.channel = resolveUpdateChannel(app.getVersion()) + autoUpdater.allowDowngrade = false + autoUpdater.autoDownload = true + autoUpdater.logger = null + return autoUpdater + } catch (error) { + logger.error('electron-updater unavailable', { error }) + return null + } +} + /** * Wires electron-updater against the GitHub Releases feed: channel-scoped * checks on launch and every four hours, delta downloads in the background, @@ -141,24 +163,17 @@ export function initUpdater(deps: UpdaterDeps): void { if (!app.isPackaged) { return } - let autoUpdater: typeof import('electron-updater')['autoUpdater'] - try { - ;({ autoUpdater } = require('electron-updater') as typeof import('electron-updater')) - } catch (error) { - logger.error('electron-updater unavailable', { error }) + const autoUpdater = configureAutoUpdater() + if (!autoUpdater) { return } const currentVersion = app.getVersion() - autoUpdater.channel = resolveUpdateChannel(currentVersion) - autoUpdater.allowDowngrade = false - autoUpdater.autoDownload = true // Never install without vetting the downloaded version first. Enabled per // download in the update-downloaded handler, but only for accepted updates // — so a blocked/downgrade build that was already downloaded is never // silently installed on quit. autoUpdater.autoInstallOnAppQuit = false - autoUpdater.logger = null autoUpdater.on('update-available', (info) => { deps.events.record('update_check', { available: info.version }) @@ -218,14 +233,23 @@ export function checkForUpdatesInteractive(deps: UpdaterDeps): void { return } deps.events.record('update_check', { manual: true }) - try { - const { autoUpdater } = require('electron-updater') as typeof import('electron-updater') - void autoUpdater.checkForUpdates().then((result) => { + const autoUpdater = configureAutoUpdater() + if (!autoUpdater) { + return + } + void autoUpdater + .checkForUpdates() + .then((result) => { if (!result || result.updateInfo.version === app.getVersion()) { void dialog.showMessageBox({ type: 'info', message: 'Sim is up to date' }) } }) - } catch (error) { - logger.error('Manual update check failed', { error }) - } + .catch((error) => { + logger.error('Manual update check failed', { error }) + void dialog.showMessageBox({ + type: 'error', + message: 'Could not check for updates', + detail: 'Something went wrong reaching the update server. Try again later.', + }) + }) } diff --git a/apps/desktop/src/main/window.ts b/apps/desktop/src/main/window.ts index 9abeddcbe7d..ce6236121fb 100644 --- a/apps/desktop/src/main/window.ts +++ b/apps/desktop/src/main/window.ts @@ -255,7 +255,11 @@ export function createMainWindow(deps: CreateMainWindowDeps): BrowserWindow { if (details.reason === 'clean-exit') { return } - deps.events.record('renderer_gone', { reason: details.reason, exitCode: details.exitCode }) + deps.events.record('renderer_gone', { + reason: details.reason, + exitCode: details.exitCode, + crashDumpDir: app.getPath('crashDumps'), + }) setTimeout(() => { if (win.isDestroyed()) { return diff --git a/apps/desktop/src/test/electron-mock.ts b/apps/desktop/src/test/electron-mock.ts index e63dd6a94c8..d09d5f7fce7 100644 --- a/apps/desktop/src/test/electron-mock.ts +++ b/apps/desktop/src/test/electron-mock.ts @@ -27,6 +27,10 @@ export const app = { dock: { downloadFinished: vi.fn() }, } +export const crashReporter = { + start: vi.fn(), +} + export const shell = { openExternal: vi.fn(() => Promise.resolve()), showItemInFolder: vi.fn(), @@ -150,6 +154,7 @@ function createWebContentsMock() { session: { setPermissionRequestHandler: vi.fn(), setPermissionCheckHandler: vi.fn(), + webRequest: { onBeforeRequest: vi.fn() }, on: vi.fn(), }, } diff --git a/apps/sim/app/api/tools/onepassword/utils.ts b/apps/sim/app/api/tools/onepassword/utils.ts index b78cf0d511c..57db60dd6a8 100644 --- a/apps/sim/app/api/tools/onepassword/utils.ts +++ b/apps/sim/app/api/tools/onepassword/utils.ts @@ -12,14 +12,12 @@ import type { Website, } from '@1password/sdk' import { createLogger } from '@sim/logger' +import { isPrivateIp } from '@sim/security/ssrf' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import * as ipaddr from 'ipaddr.js' import { isHosted } from '@/lib/core/config/env-flags' -import { - isPrivateOrReservedIP, - secureFetchWithPinnedIP, -} from '@/lib/core/security/input-validation.server' +import { secureFetchWithPinnedIP } from '@/lib/core/security/input-validation.server' /** Connect-format field type strings returned by normalization. */ type ConnectFieldType = @@ -274,7 +272,7 @@ const connectLogger = createLogger('OnePasswordConnect') */ function assertConnectIpAllowed(ip: string, hostname: string): void { if (isHosted) { - if (isPrivateOrReservedIP(ip)) { + if (isPrivateIp(ip)) { connectLogger.warn('1Password Connect server URL resolves to a private or reserved IP', { hostname, resolvedIP: ip, diff --git a/apps/sim/app/api/tools/sap_concur/shared.ts b/apps/sim/app/api/tools/sap_concur/shared.ts index e395b4123af..9936c436b4b 100644 --- a/apps/sim/app/api/tools/sap_concur/shared.ts +++ b/apps/sim/app/api/tools/sap_concur/shared.ts @@ -1,5 +1,6 @@ import { createHash } from 'node:crypto' import { createLogger } from '@sim/logger' +import { isPrivateIpHost } from '@sim/security/ssrf' import { z } from 'zod' import { secureFetchWithValidation } from '@/lib/core/security/input-validation.server' import { FileInputSchema } from '@/lib/uploads/utils/file-schemas' @@ -108,30 +109,6 @@ const FORBIDDEN_HOSTS = new Set([ '[fd00:ec2::254]', ]) -function isPrivateIPv4(host: string): boolean { - const match = host.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/) - if (!match) return false - const octets = match.slice(1, 5).map(Number) as [number, number, number, number] - if (octets.some((o) => o < 0 || o > 255)) return false - const [a, b] = octets - if (a === 10) return true - if (a === 172 && b >= 16 && b <= 31) return true - if (a === 192 && b === 168) return true - if (a === 127) return true - if (a === 169 && b === 254) return true - if (a === 0) return true - return false -} - -function isPrivateOrLoopbackIPv6(host: string): boolean { - const stripped = host.startsWith('[') && host.endsWith(']') ? host.slice(1, -1) : host - const lower = stripped.toLowerCase() - if (lower === '::' || lower === '::1') return true - if (/^fc[0-9a-f]{2}:/.test(lower) || /^fd[0-9a-f]{2}:/.test(lower)) return true - if (lower.startsWith('fe80:')) return true - return false -} - /** Validate a URL is https and not pointing to a private/loopback host. */ export function assertSafeExternalUrl(rawUrl: string, label: string): URL { let parsed: URL @@ -147,12 +124,9 @@ export function assertSafeExternalUrl(rawUrl: string, label: string): URL { if (FORBIDDEN_HOSTS.has(host) || FORBIDDEN_HOSTS.has(`[${host}]`)) { throw new Error(`${label} host is not allowed`) } - if (isPrivateIPv4(host)) { + if (isPrivateIpHost(host)) { throw new Error(`${label} host is not allowed (private/loopback range)`) } - if (isPrivateOrLoopbackIPv6(host)) { - throw new Error(`${label} host is not allowed (IPv6 private/loopback)`) - } return parsed } diff --git a/apps/sim/app/api/tools/zoominfo/shared.ts b/apps/sim/app/api/tools/zoominfo/shared.ts index 0b9f4674f06..15a3092327f 100644 --- a/apps/sim/app/api/tools/zoominfo/shared.ts +++ b/apps/sim/app/api/tools/zoominfo/shared.ts @@ -1,5 +1,6 @@ import { createHash } from 'node:crypto' import { createLogger } from '@sim/logger' +import { isPrivateIpHost } from '@sim/security/ssrf' import { z } from 'zod' import { secureFetchWithValidation } from '@/lib/core/security/input-validation.server' @@ -52,21 +53,6 @@ const FORBIDDEN_HOSTS = new Set([ '[::]', ]) -function isPrivateIPv4(host: string): boolean { - const match = host.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/) - if (!match) return false - const octets = match.slice(1, 5).map(Number) as [number, number, number, number] - if (octets.some((o) => o < 0 || o > 255)) return false - const [a, b] = octets - if (a === 10) return true - if (a === 172 && b >= 16 && b <= 31) return true - if (a === 192 && b === 168) return true - if (a === 127) return true - if (a === 169 && b === 254) return true - if (a === 0) return true - return false -} - export function assertSafeZoomInfoUrl(rawUrl: string, label: string): URL { let parsed: URL try { @@ -81,7 +67,7 @@ export function assertSafeZoomInfoUrl(rawUrl: string, label: string): URL { if (FORBIDDEN_HOSTS.has(host)) { throw new Error(`${label} host is not allowed`) } - if (isPrivateIPv4(host)) { + if (isPrivateIpHost(host)) { throw new Error(`${label} host is not allowed (private/loopback range)`) } if (host !== 'api.zoominfo.com') { diff --git a/apps/sim/app/desktop/launcher/use-launcher-chat.ts b/apps/sim/app/desktop/launcher/use-launcher-chat.ts index 542ef18160e..c32fd8c4517 100644 --- a/apps/sim/app/desktop/launcher/use-launcher-chat.ts +++ b/apps/sim/app/desktop/launcher/use-launcher-chat.ts @@ -203,6 +203,7 @@ export function useLauncherChat() { logger.info('Skipping unrecognized stream event', { reason: parsed.reason }) return undefined } + // double-cast-allowed: parsePersistedStreamEventEnvelope validates the envelope structurally; EnvelopeLike is the local narrowed view for this read-only consumer const event = parsed.event as unknown as EnvelopeLike if (event.type === 'session' && event.payload.kind === 'chat') { diff --git a/apps/sim/connectors/s3/s3.ts b/apps/sim/connectors/s3/s3.ts index 8f92d0687d5..ced6a92dc16 100644 --- a/apps/sim/connectors/s3/s3.ts +++ b/apps/sim/connectors/s3/s3.ts @@ -1,5 +1,6 @@ import crypto from 'crypto' import { createLogger } from '@sim/logger' +import { isLoopbackHostname } from '@sim/security/ssrf' import { getErrorMessage, toError } from '@sim/utils/errors' import { isHosted } from '@/lib/core/config/env-flags' import { secureFetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server' @@ -122,18 +123,6 @@ function isSupportedKey(key: string, allowedExtensions: Set): boolean { return allowedExtensions.has(getExtension(key)) } -/** - * Returns true when the host is a loopback address for which plain `http://` is - * tolerated (local MinIO development on a self-hosted deployment). Any other - * host must use `https://`. This is only an early check — the SSRF boundary is - * enforced at request time by {@link secureFetchWithRetry}, which blocks - * loopback/private targets on hosted Sim regardless of what this parser accepts. - */ -function isLoopbackHost(host: string): boolean { - const bare = host.replace(/^\[|\]$/g, '') - return bare === 'localhost' || bare === '127.0.0.1' || bare === '::1' -} - /** * Parses and validates a custom S3-compatible endpoint string. * @@ -173,7 +162,9 @@ function parseEndpoint(raw: string): S3Endpoint { const host = url.hostname if (!host) throw new Error('Endpoint is missing a host') - if (scheme === 'http' && !(isLoopbackHost(host) && !isHosted)) { + // Plain http is tolerated only for loopback on self-host (local MinIO); the + // real SSRF boundary is enforced at request time by secureFetchWithRetry. + if (scheme === 'http' && !(isLoopbackHostname(host) && !isHosted)) { throw new Error( 'Plain http:// endpoints are only allowed for localhost on self-hosted deployments — use https:// otherwise' ) diff --git a/apps/sim/lib/api/contracts/tools/sap.ts b/apps/sim/lib/api/contracts/tools/sap.ts index 38b10cbbecc..dcfa27fcaab 100644 --- a/apps/sim/lib/api/contracts/tools/sap.ts +++ b/apps/sim/lib/api/contracts/tools/sap.ts @@ -1,3 +1,4 @@ +import { isPrivateIpHost } from '@sim/security/ssrf' import { z } from 'zod' import { genericToolResponseSchema } from '@/lib/api/contracts/tools/shared' import { defineRouteContract } from '@/lib/api/contracts/types' @@ -49,54 +50,6 @@ const FORBIDDEN_SAP_HOSTS = new Set([ '[fd00:ec2::254]', ]) -function isPrivateIPv4(host: string): boolean { - const match = host.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/) - if (!match) return false - const octets = match.slice(1, 5).map(Number) as [number, number, number, number] - if (octets.some((octet) => octet < 0 || octet > 255)) return false - const [a, b] = octets - if (a === 10) return true - if (a === 172 && b >= 16 && b <= 31) return true - if (a === 192 && b === 168) return true - if (a === 127) return true - if (a === 169 && b === 254) return true - if (a === 0) return true - return false -} - -function extractIPv4MappedHost(host: string): string | null { - const stripped = host.startsWith('[') && host.endsWith(']') ? host.slice(1, -1) : host - const lower = stripped.toLowerCase() - for (const prefix of ['::ffff:', '::']) { - if (lower.startsWith(prefix)) { - const candidate = lower.slice(prefix.length) - if (/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(candidate)) return candidate - } - } - const hexMatch = lower.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/) - if (hexMatch) { - const high = Number.parseInt(hexMatch[1] as string, 16) - const low = Number.parseInt(hexMatch[2] as string, 16) - if (high >= 0 && high <= 0xffff && low >= 0 && low <= 0xffff) { - const a = (high >> 8) & 0xff - const b = high & 0xff - const c = (low >> 8) & 0xff - const d = low & 0xff - return `${a}.${b}.${c}.${d}` - } - } - return null -} - -function isPrivateOrLoopbackIPv6(host: string): boolean { - const stripped = host.startsWith('[') && host.endsWith(']') ? host.slice(1, -1) : host - const lower = stripped.toLowerCase() - if (lower === '::' || lower === '::1') return true - if (/^fc[0-9a-f]{2}:/.test(lower) || /^fd[0-9a-f]{2}:/.test(lower)) return true - if (lower.startsWith('fe80:')) return true - return false -} - export function checkSapExternalUrlSafety( rawUrl: string, label: string @@ -114,16 +67,9 @@ export function checkSapExternalUrlSafety( if (FORBIDDEN_SAP_HOSTS.has(host) || FORBIDDEN_SAP_HOSTS.has(`[${host}]`)) { return { ok: false, message: `${label} host is not allowed` } } - if (isPrivateIPv4(host)) { + if (isPrivateIpHost(host)) { return { ok: false, message: `${label} host is not allowed (private/loopback range)` } } - const mapped = extractIPv4MappedHost(host) - if (mapped && isPrivateIPv4(mapped)) { - return { ok: false, message: `${label} host is not allowed (IPv4-mapped private range)` } - } - if (isPrivateOrLoopbackIPv6(host)) { - return { ok: false, message: `${label} host is not allowed (IPv6 private/loopback)` } - } return { ok: true, url: parsed } } diff --git a/apps/sim/lib/core/security/input-validation.server.ts b/apps/sim/lib/core/security/input-validation.server.ts index dc9764990ba..a2189fd419d 100644 --- a/apps/sim/lib/core/security/input-validation.server.ts +++ b/apps/sim/lib/core/security/input-validation.server.ts @@ -3,6 +3,7 @@ import http from 'http' import https from 'https' import type { LookupFunction } from 'net' import { createLogger } from '@sim/logger' +import { isPrivateIp, isPrivateIpHost } from '@sim/security/ssrf' import { toError } from '@sim/utils/errors' import { omit } from '@sim/utils/object' import * as ipaddr from 'ipaddr.js' @@ -21,49 +22,6 @@ export interface AsyncValidationResult extends ValidationResult { originalHostname?: string } -/** - * Checks if an IP address is private or reserved (not routable on the public internet) - * Uses ipaddr.js for robust handling of all IP formats including: - * - Octal notation (0177.0.0.1) - * - Hex notation (0x7f000001) - * - IPv4-mapped IPv6 (::ffff:127.0.0.1) - * - IPv4-compatible IPv6 (::a.b.c.d / ::xxxx:xxxx, RFC 4291 §2.5.5.1, deprecated) - * - Various edge cases that regex patterns miss - */ -export function isPrivateOrReservedIP(ip: string): boolean { - try { - if (!ipaddr.isValid(ip)) { - return true - } - - const addr = ipaddr.process(ip) - const range = addr.range() - - if (range !== 'unicast') { - return true - } - - if (addr.kind() === 'ipv6') { - const v6 = addr as ipaddr.IPv6 - const parts = v6.parts - const firstSixZero = parts.slice(0, 6).every((p) => p === 0) - if (firstSixZero) { - const embedded = ipaddr.fromByteArray([ - (parts[6] >> 8) & 0xff, - parts[6] & 0xff, - (parts[7] >> 8) & 0xff, - parts[7] & 0xff, - ]) - return embedded.range() !== 'unicast' - } - } - - return false - } catch { - return true - } -} - /** * Validates a URL and resolves its DNS to prevent SSRF via DNS rebinding * @@ -114,7 +72,7 @@ export async function validateUrlWithDNS( return ip === '127.0.0.1' || ip === '::1' })() - if (isPrivateOrReservedIP(address) && !(isLocalhost && resolvedIsLoopback && !isHosted)) { + if (isPrivateIp(address) && !(isLocalhost && resolvedIsLoopback && !isHosted)) { logger.warn('URL resolves to blocked IP address', { paramName, hostname, @@ -179,18 +137,14 @@ export async function validateDatabaseHost( return { isValid: false, error: `${paramName} cannot be localhost` } } - if ( - ipaddr.isValid(cleanHost) && - isPrivateOrReservedIP(cleanHost) && - !isPrivateDatabaseHostsAllowed - ) { + if (isPrivateIpHost(cleanHost) && !isPrivateDatabaseHostsAllowed) { return { isValid: false, error: `${paramName} cannot be a private IP address` } } try { const { address } = await dns.lookup(cleanHost, { verbatim: true }) - if (isPrivateOrReservedIP(address) && !isPrivateDatabaseHostsAllowed) { + if (isPrivateIp(address) && !isPrivateDatabaseHostsAllowed) { logger.warn('Database host resolves to blocked IP address', { paramName, hostname: host, diff --git a/apps/sim/lib/core/security/input-validation.test.ts b/apps/sim/lib/core/security/input-validation.test.ts index b4c87e09374..a5358113d17 100644 --- a/apps/sim/lib/core/security/input-validation.test.ts +++ b/apps/sim/lib/core/security/input-validation.test.ts @@ -27,7 +27,6 @@ import { validateWorkdayTenantUrl, } from '@/lib/core/security/input-validation' import { - isPrivateOrReservedIP, validateDatabaseHost, validateUrlWithDNS, } from '@/lib/core/security/input-validation.server' @@ -566,147 +565,6 @@ describe('sanitizeForLogging', () => { }) }) -describe('isPrivateOrReservedIP', () => { - describe('IPv4 private/reserved ranges', () => { - it.concurrent.each([ - ['192.168.1.1'], - ['192.168.0.0'], - ['10.0.0.1'], - ['10.255.255.255'], - ['172.16.0.1'], - ['172.31.255.255'], - ['127.0.0.1'], - ['127.255.255.255'], - ['169.254.169.254'], - ['0.0.0.0'], - ['224.0.0.1'], - ])('blocks IPv4 %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(true) - }) - }) - - describe('IPv6 reserved ranges', () => { - it.concurrent.each([ - ['::1'], - ['::'], - ['fe80::1'], - ['fc00::1'], - ['fd00::1'], - ['ff02::1'], - ['2001:db8::1'], - ])('blocks IPv6 %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(true) - }) - }) - - describe('IPv4-mapped IPv6 (::ffff:0:0/96)', () => { - it.concurrent.each([ - ['::ffff:192.168.1.1'], - ['::ffff:127.0.0.1'], - ['::ffff:169.254.169.254'], - ['::ffff:c0a8:101'], - ['::ffff:0:0'], - ])('blocks mapped private/reserved %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(true) - }) - - it.concurrent('allows mapped public IPv4 ::ffff:8.8.8.8', () => { - expect(isPrivateOrReservedIP('::ffff:8.8.8.8')).toBe(false) - }) - }) - - describe('NAT64 (RFC 6052, 64:ff9b::/96)', () => { - it.concurrent('blocks NAT64-encoded private IPv4', () => { - expect(isPrivateOrReservedIP('64:ff9b::192.168.1.1')).toBe(true) - }) - }) - - describe('IPv4-compatible IPv6 (::a.b.c.d, RFC 4291 §2.5.5.1, deprecated)', () => { - it.concurrent.each([ - ['::c0a8:101', '192.168.1.1 (URL-normalized hex form)'], - ['::c0a8:0101', '192.168.1.1 (zero-padded hex form)'], - ['::a9fe:a9fe', '169.254.169.254 (cloud metadata)'], - ['::7f00:1', '127.0.0.1 (loopback)'], - ['::7f00:0001', '127.0.0.1 (zero-padded)'], - ['::a00:1', '10.0.0.1 (RFC1918)'], - ['::ac10:1', '172.16.0.1 (RFC1918)'], - ['::e000:1', '224.0.0.1 (multicast)'], - ['::192.168.1.1', 'dotted form ::192.168.1.1'], - ['::169.254.169.254', 'dotted form ::169.254.169.254'], - ['::127.0.0.1', 'dotted form ::127.0.0.1'], - ['::10.0.0.1', 'dotted form ::10.0.0.1'], - ])('blocks %s — %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(true) - }) - - it.concurrent.each([ - ['::8.8.8.8', 'dotted form embedding public IPv4'], - ['::808:808', 'hex form embedding 8.8.8.8'], - ['::0808:0808', 'zero-padded hex form embedding 8.8.8.8'], - ])('allows IPv4-compatible IPv6 with embedded public IPv4 %s — %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(false) - }) - - it.concurrent.each([ - ['::ffff:1', 'embedded 255.255.0.1 (Class E reserved) via parts[6]=0xffff'], - ['::ffff:0', 'embedded 255.255.0.0 (Class E reserved)'], - ['::ffff:abcd', 'embedded 255.255.171.205 (Class E reserved)'], - ['::f000:1', 'embedded 240.0.0.1 (Class E reserved)'], - ])('blocks IPv4-compatible IPv6 with Class E embedded IPv4 %s — %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(true) - }) - }) - - describe('non-IPv4-compat unicast IPv6 (must not over-block)', () => { - it.concurrent.each([ - ['2606:4700:4700::1111'], - ['2001:4860:4860::8888'], - ['::1:c0a8:101'], - ['1::c0a8:101'], - ['1:2:3:4:5:6:c0a8:101'], - ])('allows %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(false) - }) - }) - - describe('IPv4 public addresses', () => { - it.concurrent.each([['8.8.8.8'], ['1.1.1.1'], ['1.0.0.1']])('allows %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(false) - }) - }) - - describe('IPv4 alternate notations', () => { - it.concurrent.each([['0177.0.0.1'], ['0x7f000001']])('blocks loopback notation %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(true) - }) - }) - - describe('invalid input', () => { - it.concurrent.each([['not-an-ip'], [''], ['256.256.256.256'], ['::g']])('rejects %s', (ip) => { - expect(isPrivateOrReservedIP(ip)).toBe(true) - }) - }) -}) - -describe('URL hostname normalization (Node URL parser + isPrivateOrReservedIP integration)', () => { - it.concurrent('Node normalizes [::192.168.1.1] to [::c0a8:101] and validator blocks it', () => { - const url = new URL('http://[::192.168.1.1]/') - const cleanHostname = - url.hostname.startsWith('[') && url.hostname.endsWith(']') - ? url.hostname.slice(1, -1) - : url.hostname - expect(cleanHostname).toBe('::c0a8:101') - expect(isPrivateOrReservedIP(cleanHostname)).toBe(true) - }) - - it.concurrent('Node normalizes [::169.254.169.254] and validator blocks the metadata IP', () => { - const url = new URL('http://[::169.254.169.254]/') - const cleanHostname = url.hostname.slice(1, -1) - expect(cleanHostname).toBe('::a9fe:a9fe') - expect(isPrivateOrReservedIP(cleanHostname)).toBe(true) - }) -}) - describe('validateUrlWithDNS', () => { describe('basic validation', () => { it('should reject invalid URLs', async () => { diff --git a/apps/sim/lib/core/security/input-validation.ts b/apps/sim/lib/core/security/input-validation.ts index fb73b300560..3fef30d52f8 100644 --- a/apps/sim/lib/core/security/input-validation.ts +++ b/apps/sim/lib/core/security/input-validation.ts @@ -1,4 +1,5 @@ import { createLogger } from '@sim/logger' +import { isPrivateIp } from '@sim/security/ssrf' import * as ipaddr from 'ipaddr.js' import { isHosted } from '@/lib/core/config/env-flags' @@ -401,7 +402,7 @@ export function validateHostname( } if (ipaddr.isValid(lowerHostname)) { - if (isPrivateOrReservedIP(lowerHostname)) { + if (isPrivateIp(lowerHostname)) { logger.warn('Hostname matches blocked IP range', { paramName, hostname: hostname.substring(0, 100), @@ -754,7 +755,7 @@ export function validateExternalUrl( } if (!isLocalhost && ipaddr.isValid(cleanHostname)) { - if (isPrivateOrReservedIP(cleanHostname)) { + if (isPrivateIp(cleanHostname)) { return { isValid: false, error: `${paramName} cannot point to private IP addresses`, @@ -797,29 +798,6 @@ export function validateProxyUrl( return validateExternalUrl(url, paramName) } -/** - * Checks if an IP address is private or reserved (not routable on the public internet) - * Uses ipaddr.js for robust handling of all IP formats including: - * - Octal notation (0177.0.0.1) - * - Hex notation (0x7f000001) - * - IPv4-mapped IPv6 (::ffff:127.0.0.1) - * - Various edge cases that regex patterns miss - */ -function isPrivateOrReservedIP(ip: string): boolean { - try { - if (!ipaddr.isValid(ip)) { - return true - } - - const addr = ipaddr.process(ip) - const range = addr.range() - - return range !== 'unicast' - } catch { - return true - } -} - /** * Validates an Airtable ID (base, table, or webhook ID) * diff --git a/apps/sim/lib/mcp/domain-check.test.ts b/apps/sim/lib/mcp/domain-check.test.ts index 28f7bab6008..261b3fbcedb 100644 --- a/apps/sim/lib/mcp/domain-check.test.ts +++ b/apps/sim/lib/mcp/domain-check.test.ts @@ -1,7 +1,6 @@ /** * @vitest-environment node */ -import { inputValidationMock, inputValidationMockFns } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockGetAllowedMcpDomainsFromEnv, mockDnsLookup, hostedFlag } = vi.hoisted(() => ({ @@ -17,20 +16,6 @@ vi.mock('@/lib/core/config/env-flags', () => ({ }, })) -vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock) - -inputValidationMockFns.mockIsPrivateOrReservedIP.mockImplementation((ip: string) => { - if (ip.startsWith('10.') || ip.startsWith('192.168.')) return true - if (ip.startsWith('172.')) { - const second = Number.parseInt(ip.split('.')[1], 10) - if (second >= 16 && second <= 31) return true - } - if (ip.startsWith('169.254.')) return true - if (ip.startsWith('127.') || ip === '::1') return true - if (ip === '0.0.0.0') return true - return false -}) - vi.mock('dns/promises', () => ({ default: { lookup: mockDnsLookup }, })) diff --git a/apps/sim/lib/mcp/domain-check.ts b/apps/sim/lib/mcp/domain-check.ts index d48588ead6b..ab7df7746db 100644 --- a/apps/sim/lib/mcp/domain-check.ts +++ b/apps/sim/lib/mcp/domain-check.ts @@ -1,9 +1,8 @@ import dns from 'dns/promises' import { createLogger } from '@sim/logger' +import { isIpLiteral, isLoopbackIp, isPrivateIp, unwrapIpv6Brackets } from '@sim/security/ssrf' import { toError } from '@sim/utils/errors' -import * as ipaddr from 'ipaddr.js' import { getAllowedMcpDomainsFromEnv, isHosted } from '@/lib/core/config/env-flags' -import { isPrivateOrReservedIP } from '@/lib/core/security/input-validation.server' import { createEnvVarPattern } from '@/executor/utils/reference-validation' const logger = createLogger('McpDomainCheck') @@ -99,25 +98,13 @@ export function validateMcpDomain(url: string | undefined): void { } /** - * Returns true if the IP is a loopback address (full 127.0.0.0/8 range, or ::1). - */ -function isLoopbackIP(ip: string): boolean { - try { - if (!ipaddr.isValid(ip)) return false - return ipaddr.process(ip).range() === 'loopback' - } catch { - return false - } -} - -/** - * Returns true if the hostname is localhost or a loopback IP literal. - * Expects IPv6 brackets to already be stripped. + * Returns true if the hostname is localhost or a loopback IP literal (full + * 127.0.0.0/8 range, or ::1). Expects IPv6 brackets to already be stripped. */ function isLocalhostHostname(hostname: string): boolean { const clean = hostname.toLowerCase() if (clean === 'localhost') return true - return ipaddr.isValid(clean) && isLoopbackIP(clean) + return isIpLiteral(clean) && isLoopbackIp(clean) } /** @@ -162,8 +149,7 @@ export async function validateMcpServerSsrf(url: string | undefined): Promise { + describe('IPv4 private/reserved ranges', () => { + it.each([ + ['192.168.1.1'], + ['192.168.0.0'], + ['10.0.0.1'], + ['10.255.255.255'], + ['172.16.0.1'], + ['172.31.255.255'], + ['127.0.0.1'], + ['127.255.255.255'], + ['169.254.169.254'], + ['0.0.0.0'], + ['224.0.0.1'], + ])('blocks IPv4 %s', (ip) => { + expect(isPrivateIp(ip)).toBe(true) + }) + }) + + describe('IPv6 reserved ranges', () => { + it.each([['::1'], ['::'], ['fe80::1'], ['fc00::1'], ['fd00::1'], ['ff02::1'], ['2001:db8::1']])( + 'blocks IPv6 %s', + (ip) => { + expect(isPrivateIp(ip)).toBe(true) + } + ) + }) + + describe('IPv4-mapped IPv6 (::ffff:0:0/96)', () => { + it.each([ + ['::ffff:192.168.1.1'], + ['::ffff:127.0.0.1'], + ['::ffff:169.254.169.254'], + ['::ffff:c0a8:101'], + ['::ffff:0:0'], + ])('blocks mapped private/reserved %s', (ip) => { + expect(isPrivateIp(ip)).toBe(true) + }) + + it('allows mapped public IPv4 ::ffff:8.8.8.8', () => { + expect(isPrivateIp('::ffff:8.8.8.8')).toBe(false) + }) + }) + + describe('NAT64 (RFC 6052, 64:ff9b::/96)', () => { + it('blocks NAT64-encoded private IPv4', () => { + expect(isPrivateIp('64:ff9b::192.168.1.1')).toBe(true) + }) + }) + + describe('IPv4-compatible IPv6 (::a.b.c.d, RFC 4291 §2.5.5.1, deprecated)', () => { + it.each([ + ['::c0a8:101', '192.168.1.1 (URL-normalized hex form)'], + ['::c0a8:0101', '192.168.1.1 (zero-padded hex form)'], + ['::a9fe:a9fe', '169.254.169.254 (cloud metadata)'], + ['::7f00:1', '127.0.0.1 (loopback)'], + ['::7f00:0001', '127.0.0.1 (zero-padded)'], + ['::a00:1', '10.0.0.1 (RFC1918)'], + ['::ac10:1', '172.16.0.1 (RFC1918)'], + ['::e000:1', '224.0.0.1 (multicast)'], + ['::192.168.1.1', 'dotted form ::192.168.1.1'], + ['::169.254.169.254', 'dotted form ::169.254.169.254'], + ['::127.0.0.1', 'dotted form ::127.0.0.1'], + ['::10.0.0.1', 'dotted form ::10.0.0.1'], + ])('blocks %s — %s', (ip) => { + expect(isPrivateIp(ip)).toBe(true) + }) + + it.each([ + ['::8.8.8.8', 'dotted form embedding public IPv4'], + ['::808:808', 'hex form embedding 8.8.8.8'], + ['::0808:0808', 'zero-padded hex form embedding 8.8.8.8'], + ])('allows IPv4-compatible IPv6 with embedded public IPv4 %s — %s', (ip) => { + expect(isPrivateIp(ip)).toBe(false) + }) + + it.each([ + ['::ffff:1', 'embedded 255.255.0.1 (Class E reserved) via parts[6]=0xffff'], + ['::ffff:0', 'embedded 255.255.0.0 (Class E reserved)'], + ['::ffff:abcd', 'embedded 255.255.171.205 (Class E reserved)'], + ['::f000:1', 'embedded 240.0.0.1 (Class E reserved)'], + ])('blocks IPv4-compatible IPv6 with Class E embedded IPv4 %s — %s', (ip) => { + expect(isPrivateIp(ip)).toBe(true) + }) + }) + + describe('non-IPv4-compat unicast IPv6 (must not over-block)', () => { + it.each([ + ['2606:4700:4700::1111'], + ['2001:4860:4860::8888'], + ['::1:c0a8:101'], + ['1::c0a8:101'], + ['1:2:3:4:5:6:c0a8:101'], + ])('allows %s', (ip) => { + expect(isPrivateIp(ip)).toBe(false) + }) + }) + + describe('IPv4 public addresses', () => { + it.each([['8.8.8.8'], ['1.1.1.1'], ['1.0.0.1']])('allows %s', (ip) => { + expect(isPrivateIp(ip)).toBe(false) + }) + }) + + describe('IPv4 alternate notations', () => { + it.each([['0177.0.0.1'], ['0x7f000001'], ['2130706433']])( + 'blocks loopback notation %s', + (ip) => { + expect(isPrivateIp(ip)).toBe(true) + } + ) + }) + + describe('invalid input fails closed', () => { + it.each([['not-an-ip'], [''], ['256.256.256.256'], ['::g'], ['example.com']])( + 'rejects %s', + (ip) => { + expect(isPrivateIp(ip)).toBe(true) + } + ) + }) + + describe('URL-parser normalized IPv6 forms', () => { + it('blocks Node-normalized [::192.168.1.1] → ::c0a8:101', () => { + const hostname = new URL('http://[::192.168.1.1]/').hostname + expect(unwrapIpv6Brackets(hostname)).toBe('::c0a8:101') + expect(isPrivateIp(unwrapIpv6Brackets(hostname))).toBe(true) + }) + + it('blocks Node-normalized [::169.254.169.254] → ::a9fe:a9fe', () => { + const hostname = new URL('http://[::169.254.169.254]/').hostname + expect(unwrapIpv6Brackets(hostname)).toBe('::a9fe:a9fe') + expect(isPrivateIp(unwrapIpv6Brackets(hostname))).toBe(true) + }) + }) +}) + +describe('isPrivateIpHost', () => { + it('blocks private/reserved IP literals (IPv4 and IPv6, bracketed or bare)', () => { + expect(isPrivateIpHost('10.0.0.1')).toBe(true) + expect(isPrivateIpHost('169.254.169.254')).toBe(true) + expect(isPrivateIpHost('127.0.0.1')).toBe(true) + expect(isPrivateIpHost('[::1]')).toBe(true) + expect(isPrivateIpHost('[fd00:ec2::254]')).toBe(true) + expect(isPrivateIpHost('[::ffff:127.0.0.1]')).toBe(true) + }) + + it('allows public IP literals', () => { + expect(isPrivateIpHost('8.8.8.8')).toBe(false) + expect(isPrivateIpHost('[2606:4700:4700::1111]')).toBe(false) + }) + + it('fails open on DNS names (resolution handled separately)', () => { + expect(isPrivateIpHost('example.com')).toBe(false) + expect(isPrivateIpHost('api.zoominfo.com')).toBe(false) + expect(isPrivateIpHost('localhost')).toBe(false) + }) +}) + +describe('isLoopbackIp', () => { + it('matches the full loopback range and ::1', () => { + expect(isLoopbackIp('127.0.0.1')).toBe(true) + expect(isLoopbackIp('127.0.0.5')).toBe(true) + expect(isLoopbackIp('::1')).toBe(true) + }) + + it('rejects non-loopback and other private ranges', () => { + expect(isLoopbackIp('10.0.0.1')).toBe(false) + expect(isLoopbackIp('8.8.8.8')).toBe(false) + expect(isLoopbackIp('169.254.169.254')).toBe(false) + expect(isLoopbackIp('not-an-ip')).toBe(false) + expect(isLoopbackIp('localhost')).toBe(false) + }) +}) + +describe('isLoopbackHostname', () => { + it('matches localhost and the loopback literals, brackets optional', () => { + expect(isLoopbackHostname('localhost')).toBe(true) + expect(isLoopbackHostname('127.0.0.1')).toBe(true) + expect(isLoopbackHostname('::1')).toBe(true) + expect(isLoopbackHostname('[::1]')).toBe(true) + }) + + it('does not match other loopback-range IPs or public hosts (exact-set only)', () => { + expect(isLoopbackHostname('127.0.0.5')).toBe(false) + expect(isLoopbackHostname('example.com')).toBe(false) + expect(isLoopbackHostname('10.0.0.1')).toBe(false) + }) +}) + +describe('unwrapIpv6Brackets', () => { + it('strips brackets from IPv6 authorities', () => { + expect(unwrapIpv6Brackets('[::1]')).toBe('::1') + expect(unwrapIpv6Brackets('[2606:4700::1111]')).toBe('2606:4700::1111') + }) + + it('leaves bare hostnames untouched', () => { + expect(unwrapIpv6Brackets('example.com')).toBe('example.com') + expect(unwrapIpv6Brackets('127.0.0.1')).toBe('127.0.0.1') + }) +}) diff --git a/packages/security/src/ssrf.ts b/packages/security/src/ssrf.ts new file mode 100644 index 00000000000..7ba9027c1ac --- /dev/null +++ b/packages/security/src/ssrf.ts @@ -0,0 +1,110 @@ +import * as ipaddr from 'ipaddr.js' + +/** + * Strips the brackets the WHATWG URL parser puts around IPv6 authorities so the + * result can be handed straight to {@link isPrivateIp} / {@link isIpLiteral}. + */ +export function unwrapIpv6Brackets(host: string): string { + return host.startsWith('[') && host.endsWith(']') ? host.slice(1, -1) : host +} + +/** + * True when the (bracket-free) host is an IP literal rather than a DNS name — + * i.e. it can be classified with {@link isPrivateIp} directly, no DNS lookup. + */ +export function isIpLiteral(host: string): boolean { + return ipaddr.isValid(host) +} + +/** + * True when an IP address is loopback (127.0.0.0/8 or ::1). Narrower than + * {@link isPrivateIp}: callers that treat loopback differently from other + * private ranges (e.g. allowing local dev servers on self-host) use this. + */ +export function isLoopbackIp(ip: string): boolean { + try { + return ipaddr.isValid(ip) && ipaddr.process(ip).range() === 'loopback' + } catch { + return false + } +} + +/** + * Loopback host identifiers permitted to use plain HTTP: `localhost` and the + * canonical loopback IP literals. Compared after stripping IPv6 brackets. + */ +const LOOPBACK_HOSTNAMES: ReadonlySet = new Set(['localhost', '127.0.0.1', '::1']) + +/** + * True when a host (name or IP literal, IPv6 brackets optional) is loopback by + * exact match — `localhost`, `127.0.0.1`, or `::1`. For full-range loopback-IP + * classification (e.g. `127.0.0.5`) use {@link isLoopbackIp}. + */ +export function isLoopbackHostname(host: string): boolean { + return LOOPBACK_HOSTNAMES.has(unwrapIpv6Brackets(host)) +} + +/** + * Classifies an IP address as private or otherwise not routable on the public + * internet — the core SSRF primitive shared by every app that resolves a user- + * or model-supplied host before connecting to it. + * + * Uses ipaddr.js for robust handling of forms that regex checks miss: + * - Octal (`0177.0.0.1`) and hex (`0x7f000001`) IPv4 + * - IPv4-mapped IPv6 (`::ffff:127.0.0.1`) + * - IPv4-compatible IPv6 (`::a.b.c.d` / `::xxxx:xxxx`, RFC 4291 §2.5.5.1, deprecated) + * - Loopback, link-local (incl. the `169.254.169.254` cloud-metadata endpoint), + * unique-local, multicast, and every other non-`unicast` range + * + * Expects a bare IP (brackets already stripped). Returns `true` (blocked) for + * anything that is not a valid, publicly routable unicast address — including + * unparseable input, so callers fail closed. + */ +export function isPrivateIp(ip: string): boolean { + try { + if (!ipaddr.isValid(ip)) { + return true + } + + const addr = ipaddr.process(ip) + const range = addr.range() + + if (range !== 'unicast') { + return true + } + + if (addr.kind() === 'ipv6') { + const v6 = addr as ipaddr.IPv6 + const parts = v6.parts + const firstSixZero = parts.slice(0, 6).every((p) => p === 0) + if (firstSixZero) { + const embedded = ipaddr.fromByteArray([ + (parts[6] >> 8) & 0xff, + parts[6] & 0xff, + (parts[7] >> 8) & 0xff, + parts[7] & 0xff, + ]) + return embedded.range() !== 'unicast' + } + } + + return false + } catch { + return true + } +} + +/** + * Classifies a URL/host hostname string that may be an IP literal. Returns + * `true` only when the host is a **literal** IP that {@link isPrivateIp} blocks; + * a DNS name (which needs resolution to classify) returns `false`. + * + * Use this for the synchronous "is this host a private IP literal" guard — a + * pre-navigation check, or a per-request subresource filter — where hostnames + * are handled separately by a DNS-resolving check. IPv6 brackets are stripped + * automatically. + */ +export function isPrivateIpHost(host: string): boolean { + const clean = unwrapIpv6Brackets(host) + return isIpLiteral(clean) && isPrivateIp(clean) +} diff --git a/packages/testing/src/mocks/input-validation.mock.ts b/packages/testing/src/mocks/input-validation.mock.ts index a525df9fcfa..9a9159c27e0 100644 --- a/packages/testing/src/mocks/input-validation.mock.ts +++ b/packages/testing/src/mocks/input-validation.mock.ts @@ -16,7 +16,6 @@ export const inputValidationMockFns = { mockValidateDatabaseHost: vi.fn(), mockSecureFetchWithPinnedIP: vi.fn(), mockSecureFetchWithValidation: vi.fn(), - mockIsPrivateOrReservedIP: vi.fn().mockReturnValue(false), mockCreatePinnedLookup: vi.fn(), } @@ -33,7 +32,6 @@ export const inputValidationMock = { validateDatabaseHost: inputValidationMockFns.mockValidateDatabaseHost, secureFetchWithPinnedIP: inputValidationMockFns.mockSecureFetchWithPinnedIP, secureFetchWithValidation: inputValidationMockFns.mockSecureFetchWithValidation, - isPrivateOrReservedIP: inputValidationMockFns.mockIsPrivateOrReservedIP, createPinnedLookup: inputValidationMockFns.mockCreatePinnedLookup, SecureFetchHeaders: class { headers: Record = {} diff --git a/scripts/check-api-validation-contracts.ts b/scripts/check-api-validation-contracts.ts index 6e44238b79f..9f5553e9030 100644 --- a/scripts/check-api-validation-contracts.ts +++ b/scripts/check-api-validation-contracts.ts @@ -9,8 +9,8 @@ const QUERY_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/queries') const SELECTOR_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/selectors') const BASELINE = { - totalRoutes: 964, - zodRoutes: 964, + totalRoutes: 966, + zodRoutes: 966, nonZodRoutes: 0, } as const @@ -90,6 +90,10 @@ const INDIRECT_ZOD_ROUTES = new Set([ // Deprecated v1 headless copilot chat API: gated to always return 410 Gone // and consumes no client-supplied input. 'apps/sim/app/api/v1/copilot/chat/route.ts', + // Streaming TTS proxy: reads the raw body for a byte cap before JSON.parse, + // then validates every field inline (text presence + length, ids). The + // JSON-mode contract framework doesn't fit a raw-read streaming route. + 'apps/sim/app/api/speech/tts/route.ts', ]) /**