Skip to content

Commit 7e56894

Browse files
waleedlatif1claude
andcommitted
fix(mcp): distinguish DNS resolution failures from SSRF policy blocks
DNS lookup failures now throw McpDnsResolutionError (502) instead of McpSsrfError (403), so transient DNS hiccups surface as retryable upstream errors rather than confusing permission rejections. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent dea9fbe commit 7e56894

File tree

5 files changed

+26
-3
lines changed

5 files changed

+26
-3
lines changed

apps/sim/app/api/mcp/servers/[id]/route.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { and, eq, isNull } from 'drizzle-orm'
55
import type { NextRequest } from 'next/server'
66
import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
77
import {
8+
McpDnsResolutionError,
89
McpDomainNotAllowedError,
910
McpSsrfError,
1011
validateMcpDomain,
@@ -53,6 +54,9 @@ export const PATCH = withMcpAuth<{ id: string }>('write')(
5354
try {
5455
await validateMcpServerSsrf(updateData.url)
5556
} catch (e) {
57+
if (e instanceof McpDnsResolutionError) {
58+
return createMcpErrorResponse(e, e.message, 502)
59+
}
5660
if (e instanceof McpSsrfError) {
5761
return createMcpErrorResponse(e, e.message, 403)
5862
}

apps/sim/app/api/mcp/servers/route.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { and, eq, isNull } from 'drizzle-orm'
55
import type { NextRequest } from 'next/server'
66
import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
77
import {
8+
McpDnsResolutionError,
89
McpDomainNotAllowedError,
910
McpSsrfError,
1011
validateMcpDomain,
@@ -91,6 +92,9 @@ export const POST = withMcpAuth('write')(
9192
try {
9293
await validateMcpServerSsrf(body.url)
9394
} catch (e) {
95+
if (e instanceof McpDnsResolutionError) {
96+
return createMcpErrorResponse(e, e.message, 502)
97+
}
9498
if (e instanceof McpSsrfError) {
9599
return createMcpErrorResponse(e, e.message, 403)
96100
}

apps/sim/app/api/mcp/servers/test-connection/route.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
22
import type { NextRequest } from 'next/server'
33
import { McpClient } from '@/lib/mcp/client'
44
import {
5+
McpDnsResolutionError,
56
McpDomainNotAllowedError,
67
McpSsrfError,
78
validateMcpDomain,
@@ -103,6 +104,9 @@ export const POST = withMcpAuth('write')(
103104
try {
104105
await validateMcpServerSsrf(body.url)
105106
} catch (e) {
107+
if (e instanceof McpDnsResolutionError) {
108+
return createMcpErrorResponse(e, e.message, 502)
109+
}
106110
if (e instanceof McpSsrfError) {
107111
return createMcpErrorResponse(e, e.message, 403)
108112
}
@@ -146,6 +150,9 @@ export const POST = withMcpAuth('write')(
146150
try {
147151
await validateMcpServerSsrf(testConfig.url)
148152
} catch (e) {
153+
if (e instanceof McpDnsResolutionError) {
154+
return createMcpErrorResponse(e, e.message, 502)
155+
}
149156
if (e instanceof McpSsrfError) {
150157
return createMcpErrorResponse(e, e.message, 403)
151158
}

apps/sim/lib/mcp/domain-check.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ vi.mock('@/executor/utils/reference-validation', () => ({
3636

3737
import {
3838
isMcpDomainAllowed,
39+
McpDnsResolutionError,
3940
McpDomainNotAllowedError,
4041
McpSsrfError,
4142
validateMcpDomain,
@@ -393,10 +394,10 @@ describe('validateMcpServerSsrf', () => {
393394
)
394395
})
395396

396-
it('throws McpSsrfError when DNS lookup fails', async () => {
397+
it('throws McpDnsResolutionError when DNS lookup fails', async () => {
397398
mockDnsLookup.mockRejectedValue(new Error('ENOTFOUND'))
398399
await expect(validateMcpServerSsrf('https://nonexistent.invalid/mcp')).rejects.toThrow(
399-
'MCP server URL hostname could not be resolved'
400+
McpDnsResolutionError
400401
)
401402
})
402403

apps/sim/lib/mcp/domain-check.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ export class McpSsrfError extends Error {
2121
}
2222
}
2323

24+
export class McpDnsResolutionError extends Error {
25+
constructor(hostname: string) {
26+
super(`MCP server URL hostname "${hostname}" could not be resolved`)
27+
this.name = 'McpDnsResolutionError'
28+
}
29+
}
30+
2431
/**
2532
* Core domain check. Returns null if the URL is allowed, or the hostname/url
2633
* string to use in the rejection error.
@@ -168,6 +175,6 @@ export async function validateMcpServerSsrf(url: string | undefined): Promise<vo
168175
hostname,
169176
error: error instanceof Error ? error.message : String(error),
170177
})
171-
throw new McpSsrfError('MCP server URL hostname could not be resolved')
178+
throw new McpDnsResolutionError(cleanHostname)
172179
}
173180
}

0 commit comments

Comments
 (0)