Skip to content

Commit dac7dda

Browse files
waleedlatif1claude
andcommitted
fix: harden SSRF protections and input validation across API routes
Add DNS-based SSRF validation for MCP server URLs, secure OIDC discovery with IP-pinned fetch, strengthen OTP/chat/form input validation, sanitize 1Password vault parameters, and tighten deployment security checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 331e9fc commit dac7dda

File tree

20 files changed

+384
-73
lines changed

20 files changed

+384
-73
lines changed

apps/sim/app/api/auth/sso/register/route.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import { z } from 'zod'
44
import { auth, getSession } from '@/lib/auth'
55
import { hasSSOAccess } from '@/lib/billing'
66
import { env } from '@/lib/core/config/env'
7+
import {
8+
secureFetchWithPinnedIP,
9+
validateUrlWithDNS,
10+
} from '@/lib/core/security/input-validation.server'
711
import { REDACTED_MARKER } from '@/lib/core/security/redaction'
812

913
const logger = createLogger('SSORegisterRoute')
@@ -156,24 +160,37 @@ export async function POST(request: NextRequest) {
156160
hasJwksEndpoint: !!oidcConfig.jwksEndpoint,
157161
})
158162

159-
const discoveryResponse = await fetch(discoveryUrl, {
160-
headers: { Accept: 'application/json' },
161-
})
163+
const urlValidation = await validateUrlWithDNS(discoveryUrl, 'OIDC discovery URL')
164+
if (!urlValidation.isValid) {
165+
logger.warn('OIDC discovery URL failed SSRF validation', {
166+
discoveryUrl,
167+
error: urlValidation.error,
168+
})
169+
return NextResponse.json({ error: urlValidation.error }, { status: 400 })
170+
}
171+
172+
const discoveryResponse = await secureFetchWithPinnedIP(
173+
discoveryUrl,
174+
urlValidation.resolvedIP!,
175+
{
176+
headers: { Accept: 'application/json' },
177+
}
178+
)
162179

163180
if (!discoveryResponse.ok) {
164181
logger.error('Failed to fetch OIDC discovery document', {
165182
status: discoveryResponse.status,
166-
statusText: discoveryResponse.statusText,
167183
})
168184
return NextResponse.json(
169185
{
170-
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Status: ${discoveryResponse.status}. Provide all endpoints explicitly or verify the issuer URL.`,
186+
error:
187+
'Failed to fetch OIDC discovery document. Provide all endpoints explicitly or verify the issuer URL.',
171188
},
172189
{ status: 400 }
173190
)
174191
}
175192

176-
const discovery = await discoveryResponse.json()
193+
const discovery = (await discoveryResponse.json()) as Record<string, unknown>
177194

178195
oidcConfig.authorizationEndpoint =
179196
oidcConfig.authorizationEndpoint || discovery.authorization_endpoint
@@ -196,7 +213,8 @@ export async function POST(request: NextRequest) {
196213
})
197214
return NextResponse.json(
198215
{
199-
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Please verify the issuer URL is correct or provide all endpoints explicitly.`,
216+
error:
217+
'Failed to fetch OIDC discovery document. Please verify the issuer URL is correct or provide all endpoints explicitly.',
200218
},
201219
{ status: 400 }
202220
)

apps/sim/app/api/files/authorization.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,9 @@ export async function verifyFileAccess(
114114
// Infer context from key if not explicitly provided
115115
const inferredContext = context || inferContextFromKey(cloudKey)
116116

117-
// 0. Profile pictures: Public access (anyone can view creator profile pictures)
118-
if (inferredContext === 'profile-pictures') {
119-
logger.info('Profile picture access allowed (public)', { cloudKey })
117+
// 0. Public contexts: profile pictures and OG images are publicly accessible
118+
if (inferredContext === 'profile-pictures' || inferredContext === 'og-images') {
119+
logger.info('Public file access allowed', { cloudKey, context: inferredContext })
120120
return true
121121
}
122122

apps/sim/app/api/files/serve/[...path]/route.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,6 @@ export async function GET(
9494
const isCloudPath = isS3Path || isBlobPath
9595
const cloudKey = isCloudPath ? path.slice(1).join('/') : fullPath
9696

97-
const contextParam = request.nextUrl.searchParams.get('context')
98-
const raw = request.nextUrl.searchParams.get('raw') === '1'
99-
10097
const isPublicByKeyPrefix =
10198
cloudKey.startsWith('profile-pictures/') || cloudKey.startsWith('og-images/')
10299

@@ -109,6 +106,9 @@ export async function GET(
109106
return await handleLocalFilePublic(fullPath)
110107
}
111108

109+
const contextParam = request.nextUrl.searchParams.get('context')
110+
const raw = request.nextUrl.searchParams.get('raw') === '1'
111+
112112
const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
113113

114114
if (!authResult.success || !authResult.userId) {

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@ import { createLogger } from '@sim/logger'
44
import { and, eq, isNull } from 'drizzle-orm'
55
import type { NextRequest } from 'next/server'
66
import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
7-
import { McpDomainNotAllowedError, validateMcpDomain } from '@/lib/mcp/domain-check'
7+
import {
8+
McpDomainNotAllowedError,
9+
McpSsrfError,
10+
validateMcpDomain,
11+
validateMcpServerSsrf,
12+
} from '@/lib/mcp/domain-check'
813
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
914
import { mcpService } from '@/lib/mcp/service'
1015
import { createMcpErrorResponse, createMcpSuccessResponse } from '@/lib/mcp/utils'
@@ -44,6 +49,15 @@ export const PATCH = withMcpAuth<{ id: string }>('write')(
4449
}
4550
throw e
4651
}
52+
53+
try {
54+
await validateMcpServerSsrf(updateData.url)
55+
} catch (e) {
56+
if (e instanceof McpSsrfError) {
57+
return createMcpErrorResponse(e, e.message, 403)
58+
}
59+
throw e
60+
}
4761
}
4862

4963
// Get the current server to check if URL is changing

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@ import { createLogger } from '@sim/logger'
44
import { and, eq, isNull } from 'drizzle-orm'
55
import type { NextRequest } from 'next/server'
66
import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
7-
import { McpDomainNotAllowedError, validateMcpDomain } from '@/lib/mcp/domain-check'
7+
import {
8+
McpDomainNotAllowedError,
9+
McpSsrfError,
10+
validateMcpDomain,
11+
validateMcpServerSsrf,
12+
} from '@/lib/mcp/domain-check'
813
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
914
import { mcpService } from '@/lib/mcp/service'
1015
import {
@@ -83,6 +88,15 @@ export const POST = withMcpAuth('write')(
8388
throw e
8489
}
8590

91+
try {
92+
await validateMcpServerSsrf(body.url)
93+
} catch (e) {
94+
if (e instanceof McpSsrfError) {
95+
return createMcpErrorResponse(e, e.message, 403)
96+
}
97+
throw e
98+
}
99+
86100
const serverId = body.url ? generateMcpServerId(workspaceId, body.url) : crypto.randomUUID()
87101

88102
const [existingServer] = await db

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

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
import { createLogger } from '@sim/logger'
22
import type { NextRequest } from 'next/server'
33
import { McpClient } from '@/lib/mcp/client'
4-
import { McpDomainNotAllowedError, validateMcpDomain } from '@/lib/mcp/domain-check'
4+
import {
5+
McpDomainNotAllowedError,
6+
McpSsrfError,
7+
validateMcpDomain,
8+
validateMcpServerSsrf,
9+
} from '@/lib/mcp/domain-check'
510
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
611
import { resolveMcpConfigEnvVars } from '@/lib/mcp/resolve-config'
712
import type { McpTransport } from '@/lib/mcp/types'
@@ -95,6 +100,15 @@ export const POST = withMcpAuth('write')(
95100
throw e
96101
}
97102

103+
try {
104+
await validateMcpServerSsrf(body.url)
105+
} catch (e) {
106+
if (e instanceof McpSsrfError) {
107+
return createMcpErrorResponse(e, e.message, 403)
108+
}
109+
throw e
110+
}
111+
98112
// Build initial config for resolution
99113
const initialConfig = {
100114
id: `test-${requestId}`,
@@ -119,7 +133,7 @@ export const POST = withMcpAuth('write')(
119133
logger.warn(`[${requestId}] Some environment variables not found:`, { missingVars })
120134
}
121135

122-
// Re-validate domain after env var resolution
136+
// Re-validate domain and SSRF after env var resolution
123137
try {
124138
validateMcpDomain(testConfig.url)
125139
} catch (e) {
@@ -129,6 +143,15 @@ export const POST = withMcpAuth('write')(
129143
throw e
130144
}
131145

146+
try {
147+
await validateMcpServerSsrf(testConfig.url)
148+
} catch (e) {
149+
if (e instanceof McpSsrfError) {
150+
return createMcpErrorResponse(e, e.message, 403)
151+
}
152+
throw e
153+
}
154+
132155
const testSecurityPolicy = {
133156
requireConsent: false,
134157
auditLevel: 'none' as const,

apps/sim/app/api/tools/onepassword/utils.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import dns from 'dns/promises'
12
import type {
23
Item,
34
ItemCategory,
@@ -8,6 +9,8 @@ import type {
89
VaultOverview,
910
Website,
1011
} from '@1password/sdk'
12+
import { createLogger } from '@sim/logger'
13+
import * as ipaddr from 'ipaddr.js'
1114

1215
/** Connect-format field type strings returned by normalization. */
1316
type ConnectFieldType =
@@ -238,6 +241,49 @@ export async function createOnePasswordClient(serviceAccountToken: string) {
238241
})
239242
}
240243

244+
const connectLogger = createLogger('OnePasswordConnect')
245+
246+
/**
247+
* Validates that a Connect server URL does not target cloud metadata endpoints.
248+
* Allows private IPs and localhost since 1Password Connect is designed to be self-hosted.
249+
*/
250+
async function validateConnectServerUrl(serverUrl: string): Promise<void> {
251+
let hostname: string
252+
try {
253+
hostname = new URL(serverUrl).hostname
254+
} catch {
255+
throw new Error('1Password server URL is not a valid URL')
256+
}
257+
258+
const clean = hostname.startsWith('[') && hostname.endsWith(']') ? hostname.slice(1, -1) : hostname
259+
260+
if (ipaddr.isValid(clean)) {
261+
const addr = ipaddr.process(clean)
262+
if (addr.range() === 'linkLocal') {
263+
throw new Error('1Password server URL cannot point to a link-local address')
264+
}
265+
return
266+
}
267+
268+
try {
269+
const { address } = await dns.lookup(clean, { verbatim: true })
270+
if (ipaddr.isValid(address) && ipaddr.process(address).range() === 'linkLocal') {
271+
connectLogger.warn('1Password Connect server URL resolves to link-local IP', {
272+
hostname: clean,
273+
resolvedIP: address,
274+
})
275+
throw new Error('1Password server URL resolves to a link-local address')
276+
}
277+
} catch (error) {
278+
if (error instanceof Error && error.message.startsWith('1Password')) throw error
279+
connectLogger.warn('DNS lookup failed for 1Password Connect server URL', {
280+
hostname: clean,
281+
error: error instanceof Error ? error.message : String(error),
282+
})
283+
throw new Error('1Password server URL hostname could not be resolved')
284+
}
285+
}
286+
241287
/** Proxy a request to the 1Password Connect Server. */
242288
export async function connectRequest(options: {
243289
serverUrl: string
@@ -247,6 +293,8 @@ export async function connectRequest(options: {
247293
body?: unknown
248294
query?: string
249295
}): Promise<Response> {
296+
await validateConnectServerUrl(options.serverUrl)
297+
250298
const base = options.serverUrl.replace(/\/$/, '')
251299
const queryStr = options.query ? `?${options.query}` : ''
252300
const url = `${base}${options.path}${queryStr}`

apps/sim/background/schedule-execution.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ export async function executeJobInline(payload: JobExecutionPayload) {
913913

914914
try {
915915
const url = buildAPIUrl('/api/mothership/execute')
916-
const headers = await buildAuthHeaders()
916+
const headers = await buildAuthHeaders(jobRecord.sourceUserId)
917917

918918
const body = {
919919
messages: [{ role: 'user', content: promptText }],

apps/sim/executor/handlers/agent/agent-handler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ export class AgentBlockHandler implements BlockHandler {
289289
}
290290

291291
try {
292-
const headers = await buildAuthHeaders()
292+
const headers = await buildAuthHeaders(ctx.userId)
293293
const params: Record<string, string> = {}
294294

295295
if (ctx.workspaceId) {
@@ -467,7 +467,7 @@ export class AgentBlockHandler implements BlockHandler {
467467
throw new Error('workflowId is required for internal JWT authentication')
468468
}
469469

470-
const headers = await buildAuthHeaders()
470+
const headers = await buildAuthHeaders(ctx.userId)
471471
const url = buildAPIUrl('/api/mcp/tools/discover', {
472472
serverId,
473473
workspaceId: ctx.workspaceId,

apps/sim/executor/handlers/evaluator/evaluator-handler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ export class EvaluatorBlockHandler implements BlockHandler {
134134

135135
const response = await fetch(url.toString(), {
136136
method: 'POST',
137-
headers: await buildAuthHeaders(),
137+
headers: await buildAuthHeaders(ctx.userId),
138138
body: stringifyJSON(providerRequest),
139139
})
140140

0 commit comments

Comments
 (0)