Skip to content

Commit ccde12e

Browse files
committed
fix(mcp): keep self-hosted private-resolving hosts unguarded (old-pin parity)
A DNS alias resolving to loopback/private is only reachable on self-hosted, where the policy explicitly permits it; the guarded lookup would filter the address and strand the connect where the old pin connected. Both MCP gates (transport + OAuth guard) now route private/loopback resolutions over the unguarded path, same as the localhost carve-out. Test IPs moved off RFC-5737 TEST-NET (which is correctly classified reserved).
1 parent b5adb51 commit ccde12e

5 files changed

Lines changed: 40 additions & 6 deletions

File tree

apps/sim/lib/mcp/client.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ describe('McpClient notification handler', () => {
276276
const client = new McpClient({
277277
config: createConfig(),
278278
securityPolicy: { requireConsent: false, auditLevel: 'basic' },
279-
resolvedIP: '203.0.113.10',
279+
resolvedIP: '93.184.216.34',
280280
})
281281

282282
// A failed connect discards the client without a disconnect(), so the Agent
@@ -290,7 +290,7 @@ describe('McpClient notification handler', () => {
290290
const client = new McpClient({
291291
config: createConfig(),
292292
securityPolicy: { requireConsent: false, auditLevel: 'basic' },
293-
resolvedIP: '203.0.113.10',
293+
resolvedIP: '93.184.216.34',
294294
})
295295

296296
await client.connect()
@@ -304,7 +304,7 @@ describe('McpClient notification handler', () => {
304304
const client = new McpClient({
305305
config: createConfig(),
306306
securityPolicy: { requireConsent: false, auditLevel: 'basic' },
307-
resolvedIP: '203.0.113.10',
307+
resolvedIP: '93.184.216.34',
308308
})
309309

310310
await expect(client.connect()).rejects.toThrow()

apps/sim/lib/mcp/client.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
import { createLogger } from '@sim/logger'
1212
import { getErrorMessage } from '@sim/utils/errors'
1313
import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'
14+
import { isPrivateOrReservedIP } from '@/lib/core/security/input-validation.server'
1415
import { getMcpSafeErrorDiagnostics } from '@/lib/mcp/error-diagnostics'
1516
import { McpOauthRedirectRequired } from '@/lib/mcp/oauth'
1617
import { createGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
@@ -98,7 +99,11 @@ export class McpClient {
9899
const useOauth = this.config.authType === 'oauth'
99100
// `resolvedIP` non-null signals the SSRF policy is active for this server (it is null in
100101
// allowlist mode / localhost-on-self-hosted); the guard validates addresses per-connect.
101-
const guarded = resolvedIP ? createGuardedMcpFetch() : undefined
102+
// A private/loopback resolvedIP only reaches here on self-hosted (where the policy
103+
// permits it) — the guarded lookup would filter it, so those connect unguarded like the
104+
// localhost carve-out.
105+
const guarded =
106+
resolvedIP && !isPrivateOrReservedIP(resolvedIP) ? createGuardedMcpFetch() : undefined
102107
this.closeGuardedTransport = guarded?.close
103108
this.transport = new StreamableHTTPClientTransport(new URL(this.config.url), {
104109
authProvider: useOauth ? this.authProvider : undefined,

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ const {
3131
}))
3232

3333
vi.mock('@/lib/core/security/input-validation.server', () => ({
34+
isPrivateOrReservedIP: (ip: string) =>
35+
ip.startsWith('127.') || ip.startsWith('10.') || ip === '::1',
3436
createSsrfGuardedFetchWithDispatcher: vi.fn(() => ({
3537
fetch: mockUndiciFetch,
3638
dispatcher: { destroy: vi.fn(() => Promise.resolve()) },

apps/sim/lib/mcp/pinned-fetch.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ const {
1818

1919
vi.mock('@/lib/core/security/input-validation.server', () => ({
2020
createSsrfGuardedFetchWithDispatcher: mockCreateGuardedFetchWithDispatcher,
21+
isPrivateOrReservedIP: (ip: string) =>
22+
ip.startsWith('127.') || ip.startsWith('10.') || ip === '::1',
2123
}))
2224
vi.mock('@/lib/mcp/domain-check', () => ({
2325
validateMcpServerSsrf: mockValidateMcpServerSsrf,
@@ -345,3 +347,22 @@ describe('createSsrfGuardedMcpFetch', () => {
345347
}
346348
})
347349
})
350+
351+
describe('self-hosted private-resolution carve-out', () => {
352+
it('routes a loopback-resolving host over global fetch (guarded lookup would filter it)', async () => {
353+
// Self-hosted DNS alias -> 127.0.0.1: policy allows it, so the guard must not
354+
// strand the connect. Falls back to global fetch, same as the allowlist path.
355+
mockValidateMcpServerSsrf.mockResolvedValue('127.0.0.1')
356+
const globalFetch = vi
357+
.spyOn(globalThis, 'fetch')
358+
.mockImplementation(async () => new Response('ok'))
359+
try {
360+
const fetchLike = createSsrfGuardedMcpFetch()
361+
await fetchLike('https://my-local-alias/mcp')
362+
expect(mockCreateGuardedFetchWithDispatcher).not.toHaveBeenCalled()
363+
expect(globalFetch).toHaveBeenCalledTimes(1)
364+
} finally {
365+
globalFetch.mockRestore()
366+
}
367+
})
368+
})

apps/sim/lib/mcp/pinned-fetch.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'
22
import { createLogger } from '@sim/logger'
33
import type { Agent } from 'undici'
4-
import { createSsrfGuardedFetchWithDispatcher } from '@/lib/core/security/input-validation.server'
4+
import {
5+
createSsrfGuardedFetchWithDispatcher,
6+
isPrivateOrReservedIP,
7+
} from '@/lib/core/security/input-validation.server'
58
import { validateMcpServerSsrf } from '@/lib/mcp/domain-check'
69
import { McpError } from '@/lib/mcp/types'
710

@@ -216,7 +219,10 @@ export function createSsrfGuardedMcpFetch(timeoutMs: number = OAUTH_FETCH_TIMEOU
216219
const resolvedIP = await withDeadline(validateMcpServerSsrf(target), signal)
217220
logger.info('OAuth guarded fetch: requesting', { host, guarded: Boolean(resolvedIP) })
218221
let response: Response
219-
if (resolvedIP) {
222+
// A private/loopback resolvedIP only occurs on self-hosted where the policy permits
223+
// it — the guarded lookup would filter it out, so those go over global fetch like the
224+
// allowlist carve-out below.
225+
if (resolvedIP && !isPrivateOrReservedIP(resolvedIP)) {
220226
const guarded = createSsrfGuardedFetchWithDispatcher({
221227
maxResponseSize: MAX_OAUTH_RESPONSE_BYTES,
222228
})

0 commit comments

Comments
 (0)