From 983370ee7c23bc7af84b290dbe43815ea4b62b16 Mon Sep 17 00:00:00 2001 From: waleed Date: Thu, 4 Jun 2026 09:23:02 -0700 Subject: [PATCH 1/6] fix(chat): prevent XSS in attachment preview via filename/data URL escaping Replace document.write with an escaped blob URL preview: HTML-entity encode the user-controlled filename and data URL, open with noopener,noreferrer, and revoke the blob URL after navigation. --- .../app/chat/components/message/message.tsx | 70 +++++++++++-------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/apps/sim/app/chat/components/message/message.tsx b/apps/sim/app/chat/components/message/message.tsx index 5bd6ac264bf..0178b299629 100644 --- a/apps/sim/app/chat/components/message/message.tsx +++ b/apps/sim/app/chat/components/message/message.tsx @@ -38,6 +38,44 @@ export interface ChatMessage { files?: ChatFile[] } +/** + * Escapes HTML entities so untrusted strings are safe to interpolate into markup. + */ +function escapeHtml(value: string): string { + return value.replace( + /[&<>"']/g, + (c) => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' })[c] || c + ) +} + +/** + * Opens an image attachment preview in a new tab via a blob URL, + * escaping the user-controlled filename and data URL to prevent XSS. + */ +function openAttachmentPreview(name: string, dataUrl: string): void { + const safeName = escapeHtml(name) + const safeUrl = escapeHtml(dataUrl) + const html = ` + + + + ${safeName} + + + + ${safeName} + + + ` + const blob = new Blob([html], { type: 'text/html' }) + const blobUrl = URL.createObjectURL(blob) + window.open(blobUrl, '_blank', 'noopener,noreferrer') + setTimeout(() => URL.revokeObjectURL(blobUrl), 60_000) +} + export const ClientChatMessage = memo( function ClientChatMessage({ message }: { message: ChatMessage }) { const [isCopied, setIsCopied] = useState(false) @@ -103,25 +141,7 @@ export const ClientChatMessage = memo( if (validDataUrl?.startsWith('data:')) { e.preventDefault() e.stopPropagation() - const newWindow = window.open('', '_blank') - if (newWindow) { - newWindow.document.write(` - - - - ${attachment.name} - - - - ${attachment.name} - - - `) - newWindow.document.close() - } + openAttachmentPreview(attachment.name, validDataUrl) } }} onKeyDown={(event) => { @@ -129,17 +149,7 @@ export const ClientChatMessage = memo( if (!validDataUrl?.startsWith('data:')) return if (event.key === 'Enter' || event.key === ' ') { event.preventDefault() - const newWindow = window.open('', '_blank') - if (newWindow) { - newWindow.document.write(` - - ${attachment.name} - - ${attachment.name} - - - `) - } + openAttachmentPreview(attachment.name, validDataUrl) } }} > From 94e6f882304ed38994cb7ba26a58993616ade8e5 Mon Sep 17 00:00:00 2001 From: waleed Date: Thu, 4 Jun 2026 09:23:42 -0700 Subject: [PATCH 2/6] fix(mcp): guard OAuth discovery and token revocation against SSRF Route discoverOAuthServerInfo and the RFC 7009 revocation POST through an SSRF-guarded fetch that validates every request URL via validateMcpServerSsrf (blocking private/reserved/loopback targets, honoring ALLOWED_MCP_DOMAINS and self-hosted localhost rules) and pins the connection to the resolved IP to prevent DNS-rebinding TOCTOU. Previously these fetches used unvalidated global fetch against URLs taken verbatim from attacker-controllable authorization-server metadata. --- apps/sim/lib/mcp/oauth/revoke.ts | 14 +++++-- apps/sim/lib/mcp/pinned-fetch.test.ts | 55 ++++++++++++++++++++++++++- apps/sim/lib/mcp/pinned-fetch.ts | 23 +++++++++++ 3 files changed, 87 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/mcp/oauth/revoke.ts b/apps/sim/lib/mcp/oauth/revoke.ts index dc80065a65c..6ec08d63d9c 100644 --- a/apps/sim/lib/mcp/oauth/revoke.ts +++ b/apps/sim/lib/mcp/oauth/revoke.ts @@ -1,4 +1,5 @@ import { discoverOAuthServerInfo } from '@modelcontextprotocol/sdk/client/auth.js' +import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js' import { db } from '@sim/db' import { mcpServers } from '@sim/db/schema' import { createLogger } from '@sim/logger' @@ -6,6 +7,7 @@ import { toError } from '@sim/utils/errors' import { eq } from 'drizzle-orm' import { decryptSecret } from '@/lib/core/security/encryption' import { loadOauthRow } from '@/lib/mcp/oauth/storage' +import { createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch' const logger = createLogger('McpOauthRevoke') const REVOKE_TIMEOUT_MS = 5000 @@ -30,7 +32,10 @@ export async function revokeMcpOauthTokens(mcpServerId: string): Promise { .limit(1) if (!server?.url) return - const info = await discoverOAuthServerInfo(server.url).catch(() => undefined) + const ssrfGuardedFetch = createSsrfGuardedMcpFetch() + const info = await discoverOAuthServerInfo(server.url, { fetchFn: ssrfGuardedFetch }).catch( + () => undefined + ) const metadata = info?.authorizationServerMetadata as | (Record & { revocation_endpoint?: string }) | undefined @@ -60,7 +65,7 @@ export async function revokeMcpOauthTokens(mcpServerId: string): Promise { } for (const { token, hint } of tokensToRevoke) { - await postRevoke(revocationEndpoint, token, hint, clientId, clientSecret) + await postRevoke(revocationEndpoint, token, hint, clientId, clientSecret, ssrfGuardedFetch) } } catch (error) { logger.warn(`Token revocation failed for server ${mcpServerId}`, { @@ -74,7 +79,8 @@ async function postRevoke( token: string, hint: 'refresh_token' | 'access_token', clientId: string, - clientSecret: string | undefined + clientSecret: string | undefined, + fetchFn: FetchLike ): Promise { const controller = new AbortController() const timer = setTimeout(() => controller.abort(), REVOKE_TIMEOUT_MS) @@ -89,7 +95,7 @@ async function postRevoke( } else { params.set('client_id', clientId) } - const res = await fetch(endpoint, { + const res = await fetchFn(endpoint, { method: 'POST', headers, body: params.toString(), diff --git a/apps/sim/lib/mcp/pinned-fetch.test.ts b/apps/sim/lib/mcp/pinned-fetch.test.ts index 8a4c27be0df..9f6b5919bf2 100644 --- a/apps/sim/lib/mcp/pinned-fetch.test.ts +++ b/apps/sim/lib/mcp/pinned-fetch.test.ts @@ -25,12 +25,23 @@ const { mockAgent, mockCreatePinnedLookup, mockUndiciFetch, capturedAgentOptions } }) +const { mockValidateMcpServerSsrf } = vi.hoisted(() => ({ + mockValidateMcpServerSsrf: vi.fn(), +})) + vi.mock('undici', () => ({ Agent: mockAgent, fetch: mockUndiciFetch })) vi.mock('@/lib/core/security/input-validation.server', () => ({ createPinnedLookup: mockCreatePinnedLookup, })) +vi.mock('@/lib/mcp/domain-check', () => ({ + validateMcpServerSsrf: mockValidateMcpServerSsrf, +})) -import { __resetPinnedAgentsForTests, createMcpPinnedFetch } from '@/lib/mcp/pinned-fetch' +import { + __resetPinnedAgentsForTests, + createMcpPinnedFetch, + createSsrfGuardedMcpFetch, +} from '@/lib/mcp/pinned-fetch' describe('createMcpPinnedFetch', () => { beforeEach(() => { @@ -125,3 +136,45 @@ describe('createMcpPinnedFetch', () => { expect(mockUndiciFetch).toHaveBeenCalledTimes(1) }) }) + +describe('createSsrfGuardedMcpFetch', () => { + beforeEach(() => { + vi.clearAllMocks() + capturedAgentOptions.length = 0 + __resetPinnedAgentsForTests() + mockCreatePinnedLookup.mockReturnValue('pinned-lookup-fn') + mockUndiciFetch.mockResolvedValue(new Response('ok')) + }) + + it('validates each request URL and pins to the resolved IP', async () => { + mockValidateMcpServerSsrf.mockResolvedValue('203.0.113.10') + const fetchLike = createSsrfGuardedMcpFetch() + await fetchLike('https://attacker.example/revoke', { method: 'POST' }) + + expect(mockValidateMcpServerSsrf).toHaveBeenCalledWith('https://attacker.example/revoke') + expect(mockUndiciFetch).toHaveBeenCalledTimes(1) + const [url, init] = mockUndiciFetch.mock.calls[0] + expect(url).toBe('https://attacker.example/revoke') + expect((init as { dispatcher?: unknown }).dispatcher).toBeInstanceOf(mockAgent) + expect((init as { method?: string }).method).toBe('POST') + }) + + it('rejects URLs that resolve to blocked IPs without issuing the request', async () => { + mockValidateMcpServerSsrf.mockRejectedValue(new Error('blocked')) + const fetchLike = createSsrfGuardedMcpFetch() + + await expect( + fetchLike('http://169.254.169.254/latest/meta-data/', { method: 'POST' }) + ).rejects.toThrow('blocked') + expect(mockUndiciFetch).not.toHaveBeenCalled() + }) + + it('accepts URL objects and validates their href', async () => { + mockValidateMcpServerSsrf.mockResolvedValue('203.0.113.10') + const fetchLike = createSsrfGuardedMcpFetch() + await fetchLike(new URL('https://attacker.example/discover')) + + expect(mockValidateMcpServerSsrf).toHaveBeenCalledWith('https://attacker.example/discover') + expect(mockUndiciFetch).toHaveBeenCalledTimes(1) + }) +}) diff --git a/apps/sim/lib/mcp/pinned-fetch.ts b/apps/sim/lib/mcp/pinned-fetch.ts index 236518d13ec..3ebf5c12770 100644 --- a/apps/sim/lib/mcp/pinned-fetch.ts +++ b/apps/sim/lib/mcp/pinned-fetch.ts @@ -1,6 +1,7 @@ import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js' import { Agent, type RequestInit as UndiciRequestInit, fetch as undiciFetch } from 'undici' import { createPinnedLookup } from '@/lib/core/security/input-validation.server' +import { validateMcpServerSsrf } from '@/lib/mcp/domain-check' /** * Pins outbound HTTP connections to a pre-resolved IP to prevent DNS-rebinding @@ -54,3 +55,25 @@ export function createMcpPinnedFetch(resolvedIP: string): FetchLike { return response as unknown as Response }) satisfies FetchLike } + +/** + * Builds a `FetchLike` that validates every outbound request URL against the + * MCP SSRF policy before issuing it, then pins the connection to the resolved + * IP. Unlike the live transport — where the server URL is validated once up + * front — OAuth discovery and RFC 7009 revocation follow URLs taken verbatim + * from attacker-controllable authorization-server metadata + * (`authorization_servers`, `token_endpoint`, `revocation_endpoint`, …). Each + * such hop must be re-validated, so this guard runs `validateMcpServerSsrf` + * per request and rejects private/reserved/loopback targets (honoring + * `ALLOWED_MCP_DOMAINS` and self-hosted localhost rules). + * + * @throws McpSsrfError if a request URL resolves to a blocked IP address + */ +export function createSsrfGuardedMcpFetch(): FetchLike { + return (async (url, init) => { + const target = typeof url === 'string' ? url : url.href + const resolvedIP = await validateMcpServerSsrf(target) + const pinnedFetch: FetchLike = resolvedIP ? createMcpPinnedFetch(resolvedIP) : globalThis.fetch + return pinnedFetch(url, init) + }) satisfies FetchLike +} From 80e460d4c5ed55ef139b6b974c7b2f1b6bc6dc48 Mon Sep 17 00:00:00 2001 From: waleed Date: Thu, 4 Jun 2026 09:23:45 -0700 Subject: [PATCH 3/6] fix(webhooks): verify Graph clientState on Teams chat-subscription notifications The microsoftteams_chat_subscription trigger set clientState=webhook.id when creating the Graph subscription but never validated it on inbound change notifications, so any request to the webhook path with a crafted notification body was treated as authentic (CWE-345). verifyAuth now requires every notification in the value array to carry a clientState matching the stored webhook id (constant-time compare) and rejects payloads without notifications. Validation handshakes (validationToken) are handled before auth and remain unaffected; outgoing-webhook HMAC auth is unchanged. --- .../providers/microsoft-teams.test.ts | 108 ++++++++++++++++++ .../lib/webhooks/providers/microsoft-teams.ts | 32 +++++- 2 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 apps/sim/lib/webhooks/providers/microsoft-teams.test.ts diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts new file mode 100644 index 00000000000..d60b94c9c35 --- /dev/null +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts @@ -0,0 +1,108 @@ +/** + * @vitest-environment node + */ +import { authOAuthUtilsMock, inputValidationMock } from '@sim/testing' +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock) +vi.mock('@/app/api/auth/oauth/utils', () => authOAuthUtilsMock) + +import { microsoftTeamsHandler } from '@/lib/webhooks/providers/microsoft-teams' + +const WEBHOOK_ID = 'webhook-uuid-1234' + +function makeRequest(body: string): NextRequest { + return new NextRequest('https://app.example.com/api/webhooks/trigger/abc', { + method: 'POST', + body, + headers: { 'Content-Type': 'application/json' }, + }) +} + +function makeNotificationBody(clientState?: unknown): string { + return JSON.stringify({ + value: [ + { + subscriptionId: 'sub-1', + changeType: 'created', + resource: 'chats/19:abc@thread.v2/messages/1700000000000', + resourceData: { id: '1700000000000' }, + ...(clientState !== undefined ? { clientState } : {}), + }, + ], + }) +} + +async function runVerifyAuth(rawBody: string, providerConfig: Record) { + return microsoftTeamsHandler.verifyAuth!({ + webhook: { id: WEBHOOK_ID }, + workflow: {}, + request: makeRequest(rawBody), + rawBody, + requestId: 'test-req', + providerConfig, + }) +} + +describe('microsoftTeamsHandler verifyAuth (chat subscription clientState)', () => { + const chatSubscriptionConfig = { triggerId: 'microsoftteams_chat_subscription' } + + beforeEach(() => { + vi.clearAllMocks() + }) + + it('accepts notifications whose clientState matches the webhook id', async () => { + const res = await runVerifyAuth(makeNotificationBody(WEBHOOK_ID), chatSubscriptionConfig) + expect(res).toBeNull() + }) + + it('rejects notifications with a forged clientState', async () => { + const res = await runVerifyAuth(makeNotificationBody('forged'), chatSubscriptionConfig) + expect(res?.status).toBe(401) + }) + + it('rejects notifications missing clientState', async () => { + const res = await runVerifyAuth(makeNotificationBody(), chatSubscriptionConfig) + expect(res?.status).toBe(401) + }) + + it('rejects non-string clientState values', async () => { + const res = await runVerifyAuth( + makeNotificationBody({ nested: WEBHOOK_ID }), + chatSubscriptionConfig + ) + expect(res?.status).toBe(401) + }) + + it('rejects payloads without a value array', async () => { + const res = await runVerifyAuth(JSON.stringify({ hello: 'world' }), chatSubscriptionConfig) + expect(res?.status).toBe(401) + }) + + it('rejects payloads with an empty value array', async () => { + const res = await runVerifyAuth(JSON.stringify({ value: [] }), chatSubscriptionConfig) + expect(res?.status).toBe(401) + }) + + it('rejects unparseable bodies', async () => { + const res = await runVerifyAuth('not-json', chatSubscriptionConfig) + expect(res?.status).toBe(401) + }) + + it('rejects batches where any notification has a mismatched clientState', async () => { + const rawBody = JSON.stringify({ + value: [ + { subscriptionId: 'sub-1', resourceData: { id: '1' }, clientState: WEBHOOK_ID }, + { subscriptionId: 'sub-2', resourceData: { id: '2' }, clientState: 'forged' }, + ], + }) + const res = await runVerifyAuth(rawBody, chatSubscriptionConfig) + expect(res?.status).toBe(401) + }) + + it('does not require clientState for non-subscription trigger types', async () => { + const res = await runVerifyAuth(JSON.stringify({ type: 'message', text: 'hi' }), {}) + expect(res).toBeNull() + }) +}) diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.ts index be749f4e990..b4411e6dc81 100644 --- a/apps/sim/lib/webhooks/providers/microsoft-teams.ts +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.ts @@ -477,7 +477,7 @@ export const microsoftTeamsHandler: WebhookProviderHandler = { return null }, - verifyAuth({ request, rawBody, requestId, providerConfig }: AuthContext) { + verifyAuth({ webhook, request, rawBody, requestId, providerConfig }: AuthContext) { if (providerConfig.hmacSecret) { const authHeader = request.headers.get('authorization') @@ -496,6 +496,36 @@ export const microsoftTeamsHandler: WebhookProviderHandler = { } } + if (providerConfig.triggerId === 'microsoftteams_chat_subscription') { + const expectedClientState = String(webhook.id ?? '') + let notifications: unknown[] = [] + try { + const parsed = JSON.parse(rawBody) as Record + if (Array.isArray(parsed?.value)) { + notifications = parsed.value + } + } catch { + notifications = [] + } + + if (notifications.length === 0) { + logger.warn( + `[${requestId}] Microsoft Teams chat subscription notification missing value array` + ) + return new NextResponse('Unauthorized - Invalid notification payload', { status: 401 }) + } + + for (const notification of notifications) { + const clientState = (notification as Record)?.clientState + if (typeof clientState !== 'string' || !safeCompare(clientState, expectedClientState)) { + logger.warn( + `[${requestId}] Microsoft Teams chat subscription clientState verification failed` + ) + return new NextResponse('Unauthorized - Invalid clientState', { status: 401 }) + } + } + } + return null }, From 07fd1348a535620606ed0fadd691f604a564bb15 Mon Sep 17 00:00:00 2001 From: waleed Date: Thu, 4 Jun 2026 09:36:39 -0700 Subject: [PATCH 4/6] fix(webhooks): fail closed when Teams chat-subscription webhook id is unavailable Hardens the clientState check so a missing webhook id (theoretically unreachable, since the row is looked up by primary key) can never collapse the expected value to an empty string that a forged clientState could match. --- .../lib/webhooks/providers/microsoft-teams.test.ts | 12 ++++++++++++ apps/sim/lib/webhooks/providers/microsoft-teams.ts | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts index d60b94c9c35..bb3e2c141e3 100644 --- a/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts @@ -101,6 +101,18 @@ describe('microsoftTeamsHandler verifyAuth (chat subscription clientState)', () expect(res?.status).toBe(401) }) + it('fails closed when the webhook record has no id', async () => { + const res = await microsoftTeamsHandler.verifyAuth!({ + webhook: {}, + workflow: {}, + request: makeRequest(makeNotificationBody('')), + rawBody: makeNotificationBody(''), + requestId: 'test-req', + providerConfig: chatSubscriptionConfig, + }) + expect(res?.status).toBe(401) + }) + it('does not require clientState for non-subscription trigger types', async () => { const res = await runVerifyAuth(JSON.stringify({ type: 'message', text: 'hi' }), {}) expect(res).toBeNull() diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.ts index b4411e6dc81..e073f25f184 100644 --- a/apps/sim/lib/webhooks/providers/microsoft-teams.ts +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.ts @@ -498,6 +498,13 @@ export const microsoftTeamsHandler: WebhookProviderHandler = { if (providerConfig.triggerId === 'microsoftteams_chat_subscription') { const expectedClientState = String(webhook.id ?? '') + if (!expectedClientState) { + logger.warn( + `[${requestId}] Microsoft Teams chat subscription webhook missing id for clientState verification` + ) + return new NextResponse('Unauthorized - Invalid clientState', { status: 401 }) + } + let notifications: unknown[] = [] try { const parsed = JSON.parse(rawBody) as Record From 6636456bf06295863ab5ee695cade29f79f2fead Mon Sep 17 00:00:00 2001 From: waleed Date: Thu, 4 Jun 2026 09:41:25 -0700 Subject: [PATCH 5/6] docs(mcp): note AbortSignal does not bound SSRF-guard DNS lookup --- apps/sim/lib/mcp/pinned-fetch.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/sim/lib/mcp/pinned-fetch.ts b/apps/sim/lib/mcp/pinned-fetch.ts index 3ebf5c12770..f395d6b03c5 100644 --- a/apps/sim/lib/mcp/pinned-fetch.ts +++ b/apps/sim/lib/mcp/pinned-fetch.ts @@ -67,6 +67,12 @@ export function createMcpPinnedFetch(resolvedIP: string): FetchLike { * per request and rejects private/reserved/loopback targets (honoring * `ALLOWED_MCP_DOMAINS` and self-hosted localhost rules). * + * Note: a caller-provided `AbortSignal` in `init` only bounds the HTTP request, + * not the validation DNS lookup — Node's `dns.lookup` does not accept a signal, + * so a hanging resolution can extend the overall call past the caller's timeout + * by up to the OS DNS timeout. Acceptable here because all consumers are + * best-effort, non-blocking flows (OAuth discovery and RFC 7009 revocation). + * * @throws McpSsrfError if a request URL resolves to a blocked IP address */ export function createSsrfGuardedMcpFetch(): FetchLike { From 617073c4b471b67e6839603e1926623943854bf5 Mon Sep 17 00:00:00 2001 From: waleed Date: Thu, 4 Jun 2026 09:42:27 -0700 Subject: [PATCH 6/6] improvement(chat): hoist HTML escape map to module-level constant --- apps/sim/app/chat/components/message/message.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/apps/sim/app/chat/components/message/message.tsx b/apps/sim/app/chat/components/message/message.tsx index 0178b299629..eb2f0e5c3e7 100644 --- a/apps/sim/app/chat/components/message/message.tsx +++ b/apps/sim/app/chat/components/message/message.tsx @@ -38,14 +38,19 @@ export interface ChatMessage { files?: ChatFile[] } +const HTML_ESCAPES: Record = { + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''', +} as const + /** * Escapes HTML entities so untrusted strings are safe to interpolate into markup. */ function escapeHtml(value: string): string { - return value.replace( - /[&<>"']/g, - (c) => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' })[c] || c - ) + return value.replace(/[&<>"']/g, (c) => HTML_ESCAPES[c] || c) } /**