diff --git a/apps/sim/app/chat/components/message/message.tsx b/apps/sim/app/chat/components/message/message.tsx index 5bd6ac264bf..eb2f0e5c3e7 100644 --- a/apps/sim/app/chat/components/message/message.tsx +++ b/apps/sim/app/chat/components/message/message.tsx @@ -38,6 +38,49 @@ 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) => HTML_ESCAPES[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 +146,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 +154,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) } }} > 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..f395d6b03c5 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,31 @@ 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). + * + * 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 { + 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 +} 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..bb3e2c141e3 --- /dev/null +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts @@ -0,0 +1,120 @@ +/** + * @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('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 be749f4e990..e073f25f184 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,43 @@ 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 + 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 },