Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
0ea66ec
fix(mcp): stabilize authenticated tool discovery
j15z Jul 18, 2026
ea7a66b
Address PR review feedback (#5754)
j15z Jul 18, 2026
2563ab7
Fix MCP discovery timestamp CAS precision (#5754)
j15z Jul 18, 2026
cf88f81
Harden MCP cache fallback diagnostics (#5754)
j15z Jul 18, 2026
273faff
Fail MCP discovery closed without cache ownership (#5754)
j15z Jul 18, 2026
9f5d554
Unify MCP cache and database publication order (#5754)
j15z Jul 18, 2026
405e858
Keep MCP failure counts and live results consistent (#5754)
j15z Jul 18, 2026
29bb919
Align MCP refresh responses with publication order (#5754)
j15z Jul 18, 2026
eb9b603
Preserve MCP tools across metadata races (#5754)
j15z Jul 18, 2026
3df5fc2
Verify MCP config revisions after status races (#5754)
j15z Jul 18, 2026
0fe1865
improvement(mcp): negotiate HTTP/2 for MCP transport
waleedlatif1 Jul 18, 2026
2d8118b
chore(mcp): remove unused cache CAS methods and simplify discovery he…
waleedlatif1 Jul 18, 2026
7909bb0
Fix MCP retry publication ordering (#5754)
j15z Jul 18, 2026
9743f93
fix(mcp): acquire a fresh cache mutation per discovery retry
waleedlatif1 Jul 18, 2026
253c471
Close MCP invalidation publication races (#5754)
j15z Jul 18, 2026
9043b7e
fix(mcp): verify racing refresh success
j15z Jul 18, 2026
ef7b8dc
fix(mcp): clarify OAuth authorization status
j15z Jul 18, 2026
d3c3d09
Address PR review feedback (#5754)
j15z Jul 18, 2026
3296f1f
Address PR review feedback (#5754)
j15z Jul 18, 2026
8dc843a
Address PR review feedback (#5754)
j15z Jul 18, 2026
75bb19c
Address PR review feedback (#5754)
j15z Jul 18, 2026
c0212e5
Address PR review feedback (#5754)
j15z Jul 18, 2026
5bb4402
Address PR review feedback (#5754)
j15z Jul 18, 2026
0fb9a4e
Address PR review feedback (#5754)
j15z Jul 18, 2026
b9dfe48
Address PR review feedback (#5754)
j15z Jul 18, 2026
99e235a
refactor(mcp): drop the ordered-publication consistency layer for the…
waleedlatif1 Jul 18, 2026
ea7a148
fix(mcp): reset connectionStatus when a server is switched to OAuth
waleedlatif1 Jul 18, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions apps/sim/app/api/mcp/oauth/start/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,23 @@ describe('MCP OAuth start route', () => {
expect(body.error).not.toContain('ECONNREFUSED')
expect(body.error).not.toContain('internal-db-host')
})

it('returns an actionable 4xx when the server does not support dynamic client registration', async () => {
mcpOauthMockFns.mockMcpAuthGuarded.mockRejectedValueOnce(
new Error('Incompatible auth server: does not support dynamic client registration')
)
const request = new NextRequest(
'http://localhost:3000/api/mcp/oauth/start?workspaceId=workspace-1&serverId=server-1'
)

const response = await GET(request)
const body = await response.json()

expect(response.status).toBe(422)
expect(body.error).toBe(
"This server doesn't support OAuth client registration. Configure a token instead."
)
})
})

describe('surfaceOauthError', () => {
Expand Down
13 changes: 12 additions & 1 deletion apps/sim/app/api/mcp/oauth/start/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { OAuthError, ServerError } from '@modelcontextprotocol/sdk/server/auth/e
import { db } from '@sim/db'
import { mcpServers } from '@sim/db/schema'
import { createLogger } from '@sim/logger'
import { toError } from '@sim/utils/errors'
import { getErrorMessage, toError } from '@sim/utils/errors'
import { and, eq, isNull } from 'drizzle-orm'
import type { NextRequest } from 'next/server'
import { NextResponse } from 'next/server'
Expand All @@ -25,6 +25,14 @@ import { createMcpErrorResponse } from '@/lib/mcp/utils'
const logger = createLogger('McpOauthStartAPI')
const OAUTH_START_TTL_MS = 10 * 60 * 1000
const MAX_SURFACED_ERROR_LENGTH = 250
const DCR_UNSUPPORTED_MESSAGE =
"This server doesn't support OAuth client registration. Configure a token instead."

function isDynamicClientRegistrationUnsupported(error: unknown): boolean {
return getErrorMessage(error, '')
.toLowerCase()
.includes('does not support dynamic client registration')
}

export function surfaceOauthError(error: unknown): string {
// Spec-compliant OAuth servers throw typed subclasses with clean RFC 6749 fields.
Expand Down Expand Up @@ -148,6 +156,9 @@ export const GET = withRouteHandler(
authorizationUrl: e.authorizationUrl,
})
}
if (isDynamicClientRegistrationUnsupported(e)) {
return createMcpErrorResponse(toError(e), DCR_UNSUPPORTED_MESSAGE, 422)
}
throw e
}
} catch (error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ function ServerListItem({
onViewDetails,
}: ServerListItemProps) {
const transportLabel = formatTransportLabel(server.transport || 'http')
const toolsLabel = getServerToolsLabel(tools, server.connectionStatus, server.lastError)
const toolsLabel = getServerToolsLabel(
tools,
server.connectionStatus,
server.lastError,
server.authType
)
const hasConnectionIssue =
server.connectionStatus === 'error' || server.connectionStatus === 'disconnected'

Expand Down Expand Up @@ -380,6 +385,8 @@ export function MCP() {
const refreshAction = getRefreshActionState({
mutationStatus: isCurrentRefresh ? refreshServerMutation.status : 'idle',
connectionStatus: isCurrentRefresh ? refreshServerMutation.data?.status : undefined,
authType: server.authType,
error: isCurrentRefresh ? refreshServerMutation.data?.error : undefined,
workflowsUpdated: isCurrentRefresh ? refreshServerMutation.data?.workflowsUpdated : undefined,
})

Expand Down Expand Up @@ -427,7 +434,12 @@ export function MCP() {
<div className='flex flex-col gap-2'>
<span className='text-[var(--text-muted)] text-caption'>Status</span>
<p className='text-[var(--text-error)] text-sm'>
{getServerToolsLabel([], server.connectionStatus, server.lastError)}
{getServerToolsLabel(
[],
server.connectionStatus,
server.lastError,
server.authType
)}
</p>
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,37 @@ describe('getRefreshActionState', () => {
})
})

it('shows Authorization required when an OAuth refresh finishes disconnected', () => {
expect(
getRefreshActionState({
mutationStatus: 'success',
connectionStatus: 'disconnected',
authType: 'oauth',
workflowsUpdated: 0,
})
).toEqual({
text: 'Authorization required',
textTone: 'error',
disabled: false,
})
})

it('keeps Failed when a disconnected OAuth refresh has a concrete error', () => {
expect(
getRefreshActionState({
mutationStatus: 'success',
connectionStatus: 'disconnected',
authType: 'oauth',
error: 'The MCP server took too long to respond and timed out',
workflowsUpdated: 0,
})
).toEqual({
text: 'Failed',
textTone: 'error',
disabled: false,
})
})

it('preserves successful refresh feedback', () => {
expect(
getRefreshActionState({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import type { MutationStatus } from '@tanstack/react-query'
import type { RefreshMcpServerResult } from '@/lib/api/contracts/mcp'
import type { McpServer, RefreshMcpServerResult } from '@/lib/api/contracts/mcp'
import type { SettingsAction } from '@/app/workspace/[workspaceId]/settings/components/settings-header/settings-header'

interface RefreshActionStateInput {
mutationStatus: MutationStatus
connectionStatus?: RefreshMcpServerResult['status']
authType?: McpServer['authType']
error?: RefreshMcpServerResult['error']
workflowsUpdated?: number
}

Expand All @@ -13,12 +15,23 @@ type RefreshActionState = Pick<SettingsAction, 'text' | 'textTone' | 'disabled'>
export function getRefreshActionState({
mutationStatus,
connectionStatus,
authType,
error,
workflowsUpdated,
}: RefreshActionStateInput): RefreshActionState {
if (mutationStatus === 'pending') {
return { text: 'Refreshing...', textTone: undefined, disabled: true }
}

if (
mutationStatus === 'success' &&
connectionStatus === 'disconnected' &&
authType === 'oauth' &&
!error?.trim()
) {
return { text: 'Authorization required', textTone: 'error', disabled: false }
}

if (
mutationStatus === 'error' ||
(mutationStatus === 'success' && connectionStatus !== 'connected')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,20 @@ describe('getServerToolsLabel', () => {
expect(getServerToolsLabel([], 'error', null)).toBe('Unable to connect')
})

it('shows a disconnected state when OAuth was not completed', () => {
expect(getServerToolsLabel([], 'disconnected', null)).toBe('Not Connected')
it('shows that OAuth authorization is required when OAuth was not completed', () => {
expect(getServerToolsLabel([], 'disconnected', null, 'oauth')).toBe(
'OAuth authorization required'
)
})

it('keeps the generic disconnected state for non-OAuth servers', () => {
expect(getServerToolsLabel([], 'disconnected', null, 'headers')).toBe('Not Connected')
})

it('shows the persisted error for disconnected connections', () => {
expect(getServerToolsLabel([], 'disconnected', 'Request timed out')).toBe('Request timed out')
expect(getServerToolsLabel([], 'disconnected', 'Request timed out', 'oauth')).toBe(
'Request timed out'
)
})

it('continues showing discovered tools for healthy connections', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@ interface NamedTool {
export function getServerToolsLabel(
tools: NamedTool[],
connectionStatus?: McpServer['connectionStatus'],
lastError?: McpServer['lastError']
lastError?: McpServer['lastError'],
authType?: McpServer['authType']
): string {
if (connectionStatus === 'error') {
return lastError?.trim() || 'Unable to connect'
}

if (connectionStatus === 'disconnected') {
return lastError?.trim() || 'Not Connected'
return (
lastError?.trim() || (authType === 'oauth' ? 'OAuth authorization required' : 'Not Connected')
)
}

const count = tools.length
Expand Down
16 changes: 14 additions & 2 deletions apps/sim/lib/core/security/input-validation.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,9 +433,21 @@ export function createPinnedLookup(resolvedIP: string): LookupFunction {
*
* The `Agent` is captured for the lifetime of the returned function, so repeated
* calls (e.g. a provider tool loop) reuse its keep-alive connections.
*
* `allowH2` opts the pinned Agent into HTTP/2 (ALPN-negotiated, h1.1 fallback).
* It defaults to `false` to leave existing consumers unchanged. Enabling it does
* not weaken pinning: the pinned `connect.lookup` forces every connection on the
* Agent to `resolvedIP` regardless of authority, so h2 connection coalescing can
* never reach an address other than the validated one.
*/
export function createPinnedFetch(resolvedIP: string): typeof fetch {
const dispatcher = new Agent({ connect: { lookup: createPinnedLookup(resolvedIP) } })
export function createPinnedFetch(
resolvedIP: string,
options?: { allowH2?: boolean }
): typeof fetch {
const dispatcher = new Agent({
allowH2: options?.allowH2 ?? false,
connect: { lookup: createPinnedLookup(resolvedIP) },
})

const pinned = async (input: RequestInfo | URL, init?: RequestInit): Promise<Response> => {
// double-cast-allowed: DOM RequestInfo/URL and undici fetch input types differ but are structurally compatible at runtime (Node's global fetch IS undici)
Expand Down
12 changes: 12 additions & 0 deletions apps/sim/lib/core/security/pinned-fetch.server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ describe('createPinnedFetch', () => {
expect(resolved).toEqual({ address: '203.0.113.10', family: 4 })
})

it('defaults allowH2 to false so existing consumers are unchanged', () => {
createPinnedFetch('203.0.113.10')
const opts = capturedAgentOptions[0] as { allowH2?: boolean }
expect(opts.allowH2).toBe(false)
})

it('opts the Agent into HTTP/2 when allowH2 is requested', () => {
createPinnedFetch('203.0.113.10', { allowH2: true })
const opts = capturedAgentOptions[0] as { allowH2?: boolean }
expect(opts.allowH2).toBe(true)
})

it('uses IPv6 family when the validated IP is IPv6', async () => {
createPinnedFetch('2606:4700:4700::1111')
const { connect } = capturedAgentOptions[0] as { connect: { lookup: PinnedLookup } }
Expand Down
50 changes: 50 additions & 0 deletions apps/sim/lib/mcp/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* @vitest-environment node
*/
import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js'
import { beforeEach, describe, expect, it, vi } from 'vitest'

const { mockLogger, mockSdkConnect, mockSdkListTools } = vi.hoisted(() => ({
Expand Down Expand Up @@ -265,6 +266,55 @@ describe('McpClient notification handler', () => {
expect(JSON.stringify(mockLogger.error.mock.calls)).not.toContain(secret)
})

it('does not misclassify rejected static headers as an OAuth authorization flow', async () => {
mockSdkConnect.mockRejectedValueOnce(new UnauthorizedError('Static token rejected'))
const client = new McpClient({
config: {
...createConfig(),
authType: 'headers',
headers: { Authorization: 'Bearer rejected-static-token' },
},
securityPolicy: { requireConsent: false, auditLevel: 'basic' },
})

await expect(client.connect()).rejects.toBeInstanceOf(UnauthorizedError)

expect(mockLogger.error).toHaveBeenCalledWith(
expect.stringContaining('Failed to connect'),
expect.objectContaining({ outcome: 'unauthorized' })
)
expect(client.getStatus().lastError).toBe('Authentication failed')
})

it('logs tools/list failures without echoed credentials or session identifiers', async () => {
const secret = 'opaque-tools-list-credential'
mockSdkListTools.mockRejectedValueOnce(new Error(`Upstream rejected ${secret}`))
const client = new McpClient({
config: {
...createConfig(),
authType: 'headers',
headers: { 'X-Custom-Credential': secret },
},
securityPolicy: { requireConsent: false, auditLevel: 'basic' },
})

await client.connect()
await expect(client.listTools()).rejects.toThrow(secret)

expect(mockLogger.error).toHaveBeenCalledWith(
expect.stringContaining('Failed to list tools'),
expect.objectContaining({
phase: 'tools/list',
serverId: 'server-1',
sessionIdPresent: true,
error: expect.objectContaining({ name: 'Error' }),
})
)
const logged = JSON.stringify(mockLogger.error.mock.calls)
expect(logged).not.toContain(secret)
expect(logged).not.toContain('test-session')
})

it('passes configured headers for OAuth transports as well as header auth transports', () => {
const authProvider = {} as unknown as NonNullable<McpClientOptions['authProvider']>
new McpClient({
Expand Down
Loading
Loading