fix(mcp): make SSRF-guarded fetch structurally mandatory in OAuth auth() calls

apps/sim/app/api/mcp/oauth/callback/route.test.ts

@@ -13,13 +13,9 @@ import {
 import { NextRequest } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockMcpAuth, mockCreateSsrfGuardedMcpFetch, mockGuardedFetch, mockDiscoverServerTools } =
-  vi.hoisted(() => ({
-    mockMcpAuth: vi.fn(),
-    mockCreateSsrfGuardedMcpFetch: vi.fn(),
-    mockGuardedFetch: vi.fn(),
-    mockDiscoverServerTools: vi.fn(),
-  }))
+const { mockDiscoverServerTools } = vi.hoisted(() => ({
+  mockDiscoverServerTools: vi.fn(),
+}))
 
 vi.mock('@sim/db', () => dbChainMock)
 vi.mock('@sim/db/schema', () => schemaMock)
@@ -28,13 +24,7 @@ vi.mock('drizzle-orm', () => ({
   eq: vi.fn(),
   isNull: vi.fn(),
 }))
-vi.mock('@modelcontextprotocol/sdk/client/auth.js', () => ({
-  auth: mockMcpAuth,
-}))
 vi.mock('@/lib/mcp/oauth', () => mcpOauthMock)
-vi.mock('@/lib/mcp/pinned-fetch', () => ({
-  createSsrfGuardedMcpFetch: mockCreateSsrfGuardedMcpFetch,
-}))
 vi.mock('@/lib/mcp/service', () => ({
   mcpService: { discoverServerTools: mockDiscoverServerTools },
 }))
@@ -45,7 +35,6 @@ describe('MCP OAuth callback route', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     resetDbChainMock()
-    mockCreateSsrfGuardedMcpFetch.mockReturnValue(mockGuardedFetch)
     authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-1' } })
     mcpOauthMockFns.mockLoadOauthRowByState.mockResolvedValue({
       id: 'oauth-row-1',
@@ -61,24 +50,25 @@ describe('MCP OAuth callback route', () => {
       },
     ])
     mcpOauthMockFns.mockLoadPreregisteredClient.mockResolvedValue(undefined)
-    mockMcpAuth.mockResolvedValue('AUTHORIZED')
+    mcpOauthMockFns.mockMcpAuthGuarded.mockResolvedValue('AUTHORIZED')
     mockDiscoverServerTools.mockResolvedValue(undefined)
   })
 
-  it('performs the token exchange through the SSRF-guarded fetch', async () => {
+  it('performs the token exchange through the SSRF-guarded mcpAuthGuarded wrapper', async () => {
     const request = new NextRequest(
       'http://localhost:3000/api/mcp/oauth/callback?state=state-1&code=auth-code-1'
     )
 
     await GET(request)
 
-    expect(mockCreateSsrfGuardedMcpFetch).toHaveBeenCalledTimes(1)
-    expect(mockMcpAuth).toHaveBeenCalledWith(
+    // The route must call the guarded wrapper (which defaults fetchFn to the
+    // SSRF-guarded fetch internally) rather than the raw SDK `auth()` — see
+    // apps/sim/lib/mcp/oauth/auth.test.ts for the wrapper's own fetchFn coverage.
+    expect(mcpOauthMockFns.mockMcpAuthGuarded).toHaveBeenCalledWith(
       expect.anything(),
       expect.objectContaining({
         serverUrl: 'https://mcp.example.com/mcp',
         authorizationCode: 'auth-code-1',
-        fetchFn: mockGuardedFetch,
       })
     )
   })

apps/sim/app/api/mcp/oauth/callback/route.ts

@@ -1,4 +1,3 @@
-import { auth as mcpAuth } from '@modelcontextprotocol/sdk/client/auth.js'
 import { db } from '@sim/db'
 import { mcpServers } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
@@ -17,9 +16,9 @@ import {
   loadOauthRowByState,
   loadPreregisteredClient,
   type McpOauthCallbackReason,
+  mcpAuthGuarded,
   SimMcpOauthProvider,
 } from '@/lib/mcp/oauth'
-import { createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
 import { mcpService } from '@/lib/mcp/service'
 
 const logger = createLogger('McpOauthCallbackAPI')
@@ -145,12 +144,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
 
     const preregistered = await loadPreregisteredClient(server.id)
     const provider = new SimMcpOauthProvider({ row, preregistered })
-    let result: Awaited<ReturnType<typeof mcpAuth>>
+    let result: Awaited<ReturnType<typeof mcpAuthGuarded>>
     try {
-      result = await mcpAuth(provider, {
+      result = await mcpAuthGuarded(provider, {
         serverUrl: server.url,
         authorizationCode: code,
-        fetchFn: createSsrfGuardedMcpFetch(),
       })
     } catch (e) {
       logger.error('Token exchange failed during MCP OAuth callback', e)

apps/sim/app/api/mcp/oauth/start/route.test.ts

@@ -17,25 +17,13 @@ import {
 import { NextRequest } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockMcpAuth, mockCreateSsrfGuardedMcpFetch, mockGuardedFetch } = vi.hoisted(() => ({
-  mockMcpAuth: vi.fn(),
-  mockCreateSsrfGuardedMcpFetch: vi.fn(),
-  mockGuardedFetch: vi.fn(),
-}))
-
 vi.mock('@sim/db', () => dbChainMock)
 vi.mock('@sim/db/schema', () => schemaMock)
 vi.mock('drizzle-orm', () => ({
   and: vi.fn(),
   eq: vi.fn(),
   isNull: vi.fn(),
 }))
-vi.mock('@modelcontextprotocol/sdk/client/auth.js', () => ({
-  auth: mockMcpAuth,
-}))
-vi.mock('@/lib/mcp/pinned-fetch', () => ({
-  createSsrfGuardedMcpFetch: mockCreateSsrfGuardedMcpFetch,
-}))
 vi.mock('@/lib/auth/hybrid', () => hybridAuthMock)
 vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
 vi.mock('@/lib/mcp/oauth', () => mcpOauthMock)
@@ -77,21 +65,24 @@ describe('MCP OAuth start route', () => {
       updatedAt: new Date(),
     })
     mcpOauthMockFns.mockLoadPreregisteredClient.mockResolvedValue(undefined)
-    mockMcpAuth.mockRejectedValue(new McpOauthRedirectRequiredMock('https://mcp.exa.ai/authorize'))
-    mockCreateSsrfGuardedMcpFetch.mockReturnValue(mockGuardedFetch)
+    mcpOauthMockFns.mockMcpAuthGuarded.mockRejectedValue(
+      new McpOauthRedirectRequiredMock('https://mcp.exa.ai/authorize')
+    )
   })
 
-  it('routes OAuth discovery through the SSRF-guarded fetch', async () => {
+  it('routes OAuth discovery through the SSRF-guarded mcpAuthGuarded wrapper', async () => {
     const request = new NextRequest(
       'http://localhost:3000/api/mcp/oauth/start?workspaceId=workspace-1&serverId=server-1'
     )
 
     await GET(request)
 
-    expect(mockCreateSsrfGuardedMcpFetch).toHaveBeenCalledTimes(1)
-    expect(mockMcpAuth).toHaveBeenCalledWith(
+    // The route must call the guarded wrapper (which defaults fetchFn to the
+    // SSRF-guarded fetch internally) rather than the raw SDK `auth()` — see
+    // apps/sim/lib/mcp/oauth/auth.test.ts for the wrapper's own fetchFn coverage.
+    expect(mcpOauthMockFns.mockMcpAuthGuarded).toHaveBeenCalledWith(
       expect.anything(),
-      expect.objectContaining({ serverUrl: 'https://mcp.exa.ai/mcp', fetchFn: mockGuardedFetch })
+      expect.objectContaining({ serverUrl: 'https://mcp.exa.ai/mcp' })
     )
   })
 
@@ -152,7 +143,7 @@ describe('MCP OAuth start route', () => {
 
     expect(response.status).toBe(409)
     expect(body.error).toBe('OAuth authorization already in progress for this server')
-    expect(mockMcpAuth).not.toHaveBeenCalled()
+    expect(mcpOauthMockFns.mockMcpAuthGuarded).not.toHaveBeenCalled()
   })
 
   it('does not leak non-OAuth internal error details to the client', async () => {

apps/sim/app/api/mcp/oauth/start/route.ts

@@ -1,4 +1,3 @@
-import { auth as mcpAuth } from '@modelcontextprotocol/sdk/client/auth.js'
 import { OAuthError, ServerError } from '@modelcontextprotocol/sdk/server/auth/errors.js'
 import { db } from '@sim/db'
 import { mcpServers } from '@sim/db/schema'
@@ -17,10 +16,10 @@ import {
   loadPreregisteredClient,
   McpOauthInsecureUrlError,
   McpOauthRedirectRequired,
+  mcpAuthGuarded,
   SimMcpOauthProvider,
   setOauthRowUser,
 } from '@/lib/mcp/oauth'
-import { createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
 import { createMcpErrorResponse } from '@/lib/mcp/utils'
 
 const logger = createLogger('McpOauthStartAPI')
@@ -130,9 +129,8 @@ export const GET = withRouteHandler(
       const provider = new SimMcpOauthProvider({ row, preregistered })
 
       try {
-        const result = await mcpAuth(provider, {
+        const result = await mcpAuthGuarded(provider, {
           serverUrl: server.url,
-          fetchFn: createSsrfGuardedMcpFetch(),
         })
         if (result === 'AUTHORIZED') {
           return NextResponse.json({ status: 'already_authorized' })

apps/sim/lib/mcp/oauth/auth.test.ts

@@ -0,0 +1,54 @@
+/**
+ * @vitest-environment node
+ */
+import type { OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockAuth, mockCreateSsrfGuardedMcpFetch, mockGuardedFetch } = vi.hoisted(() => ({
+  mockAuth: vi.fn(),
+  mockCreateSsrfGuardedMcpFetch: vi.fn(),
+  mockGuardedFetch: vi.fn(),
+}))
+
+vi.mock('@modelcontextprotocol/sdk/client/auth.js', () => ({
+  auth: mockAuth,
+}))
+vi.mock('@/lib/mcp/pinned-fetch', () => ({
+  createSsrfGuardedMcpFetch: mockCreateSsrfGuardedMcpFetch,
+}))
+
+import { mcpAuthGuarded } from '@/lib/mcp/oauth/auth'
+
+describe('mcpAuthGuarded', () => {
+  const provider = {} as OAuthClientProvider
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockCreateSsrfGuardedMcpFetch.mockReturnValue(mockGuardedFetch)
+    mockAuth.mockResolvedValue('AUTHORIZED')
+  })
+
+  it('defaults fetchFn to the SSRF-guarded fetch', async () => {
+    await mcpAuthGuarded(provider, { serverUrl: 'https://mcp.example.com/mcp' })
+
+    expect(mockCreateSsrfGuardedMcpFetch).toHaveBeenCalledTimes(1)
+    expect(mockAuth).toHaveBeenCalledWith(provider, {
+      serverUrl: 'https://mcp.example.com/mcp',
+      fetchFn: mockGuardedFetch,
+    })
+  })
+
+  it('lets a caller-supplied fetchFn override the default', async () => {
+    const overrideFetch = vi.fn()
+
+    await mcpAuthGuarded(provider, {
+      serverUrl: 'https://mcp.example.com/mcp',
+      fetchFn: overrideFetch,
+    })
+
+    expect(mockAuth).toHaveBeenCalledWith(provider, {
+      serverUrl: 'https://mcp.example.com/mcp',
+      fetchFn: overrideFetch,
+    })
+  })
+})

apps/sim/lib/mcp/oauth/auth.ts

@@ -0,0 +1,19 @@
+import { auth, type OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'
+import { createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
+
+type McpAuthOptions = Parameters<typeof auth>[1]
+
+/**
+ * Wraps the MCP SDK's `auth()` and defaults `fetchFn` to the SSRF-guarded
+ * fetch. Every URL touched during an MCP OAuth exchange — discovery,
+ * authorization, token, and revocation endpoints — can come from
+ * attacker-controllable authorization-server metadata, so callers must not
+ * be able to omit the guard by forgetting to pass `fetchFn` explicitly.
+ * Pass `fetchFn` in `options` to override (e.g. in tests).
+ */
+export function mcpAuthGuarded(
+  provider: OAuthClientProvider,
+  options: McpAuthOptions
+): ReturnType<typeof auth> {
+  return auth(provider, { fetchFn: createSsrfGuardedMcpFetch(), ...options })
+}

apps/sim/lib/mcp/oauth/index.ts

@@ -1,3 +1,4 @@
+export { mcpAuthGuarded } from './auth'
 export type {
   McpOauthCallbackMessage,
   McpOauthCallbackReason,

packages/testing/src/mocks/mcp-oauth.mock.ts

@@ -12,6 +12,7 @@ import { vi } from 'vitest'
  */
 export const mcpOauthMockFns = {
   mockAssertSafeOauthServerUrl: vi.fn(),
+  mockMcpAuthGuarded: vi.fn(),
   mockGetOrCreateOauthRow: vi.fn(),
   mockLoadOauthRow: vi.fn(),
   mockLoadOauthRowByState: vi.fn(),
@@ -63,6 +64,7 @@ function buildSimMcpOauthProvider(value: object) {
  */
 export const mcpOauthMock = {
   assertSafeOauthServerUrl: mcpOauthMockFns.mockAssertSafeOauthServerUrl,
+  mcpAuthGuarded: mcpOauthMockFns.mockMcpAuthGuarded,
   getOrCreateOauthRow: mcpOauthMockFns.mockGetOrCreateOauthRow,
   loadOauthRow: mcpOauthMockFns.mockLoadOauthRow,
   loadOauthRowByState: mcpOauthMockFns.mockLoadOauthRowByState,