Skip to content

Commit ff2a152

Browse files
authored
fix(security): add SSRF protection to database tools and webhook delivery (#3500)
* fix(security): add SSRF protection to database tools and webhook delivery * fix(security): address review comments on SSRF PR - Remove Promise.race timeout pattern to avoid unhandled rejections (http.request timeout is sufficient for webhook delivery) - Use safeCompare in verifyCronAuth instead of inline HMAC logic - Strip IPv6 brackets before validateDatabaseHost in Redis route * fix(security): allow HTTP webhooks and fix misleading MCP error docs - Add allowHttp option to validateExternalUrl, validateUrlWithDNS, and secureFetchWithValidation to support HTTP webhook URLs - Pass allowHttp: true for webhook delivery and test endpoints - Fix misleading JSDoc on createMcpErrorResponse (doesn't log errors) - Mark unused error param with underscore prefix * fix(security): forward allowHttp option through redirect validation Pass allowHttp to validateUrlWithDNS in the redirect handler of secureFetchWithPinnedIP so HTTP-to-HTTP redirects work when allowHttp is enabled for webhook delivery. * fix(security): block localhost when allowHttp is enabled When allowHttp is true (user-supplied webhook URLs), explicitly block localhost/loopback in both validateExternalUrl and validateUrlWithDNS to prevent SSRF against internal services. * fix(security): always strip multi-line content in sanitizeConnectionError Take the first line of the error message regardless of length to prevent leaking sensitive data from multi-line error messages.
1 parent 2e1c639 commit ff2a152

File tree

37 files changed

+259
-112
lines changed

37 files changed

+259
-112
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ export const POST = withMcpAuth<{ id: string }>('read')(
192192
)
193193
} catch (error) {
194194
connectionStatus = 'error'
195-
lastError = error instanceof Error ? error.message : 'Connection test failed'
195+
lastError =
196+
error instanceof Error ? error.message.split('\n')[0].slice(0, 200) : 'Connection failed'
196197
logger.warn(`[${requestId}] Failed to connect to server ${serverId}:`, error)
197198
}
198199

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,20 @@ interface TestConnectionResult {
4141
warnings?: string[]
4242
}
4343

44+
/**
45+
* Extracts a user-friendly error message from connection errors.
46+
* Keeps diagnostic info (timeout, DNS, HTTP status) but strips
47+
* verbose internals (Zod details, full response bodies, stack traces).
48+
*/
49+
function sanitizeConnectionError(error: unknown): string {
50+
if (!(error instanceof Error)) {
51+
return 'Unknown connection error'
52+
}
53+
54+
const firstLine = error.message.split('\n')[0]
55+
return firstLine.length > 200 ? `${firstLine.slice(0, 200)}...` : firstLine
56+
}
57+
4458
/**
4559
* POST - Test connection to an MCP server before registering it
4660
*/
@@ -137,8 +151,7 @@ export const POST = withMcpAuth('write')(
137151
} catch (toolError) {
138152
logger.warn(`[${requestId}] Connection established but could not list tools:`, toolError)
139153
result.success = false
140-
const errorMessage = toolError instanceof Error ? toolError.message : 'Unknown error'
141-
result.error = `Connection established but could not list tools: ${errorMessage}`
154+
result.error = 'Connection established but could not list tools'
142155
result.warnings = result.warnings || []
143156
result.warnings.push(
144157
'Server connected but tool listing failed - connection may be incomplete'
@@ -163,11 +176,7 @@ export const POST = withMcpAuth('write')(
163176
logger.warn(`[${requestId}] MCP server test failed:`, error)
164177

165178
result.success = false
166-
if (error instanceof Error) {
167-
result.error = error.message
168-
} else {
169-
result.error = 'Unknown connection error'
170-
}
179+
result.error = sanitizeConnectionError(error)
171180
} finally {
172181
if (client) {
173182
try {

apps/sim/app/api/mcp/tools/execute/route.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,12 @@ export const POST = withMcpAuth('read')(
8989
tool = tools.find((t) => t.name === toolName) ?? null
9090

9191
if (!tool) {
92+
logger.warn(`[${requestId}] Tool ${toolName} not found on server ${serverId}`, {
93+
availableTools: tools.map((t) => t.name),
94+
})
9295
return createMcpErrorResponse(
93-
new Error(
94-
`Tool ${toolName} not found on server ${serverId}. Available tools: ${tools.map((t) => t.name).join(', ')}`
95-
),
96-
'Tool not found',
96+
new Error('Tool not found'),
97+
'Tool not found on the specified server',
9798
404
9899
)
99100
}

apps/sim/app/api/tools/a2a/cancel-task/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export async function POST(request: NextRequest) {
7676
return NextResponse.json(
7777
{
7878
success: false,
79-
error: error instanceof Error ? error.message : 'Failed to cancel task',
79+
error: 'Failed to cancel task',
8080
},
8181
{ status: 500 }
8282
)

apps/sim/app/api/tools/a2a/delete-push-notification/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export async function POST(request: NextRequest) {
8686
return NextResponse.json(
8787
{
8888
success: false,
89-
error: error instanceof Error ? error.message : 'Failed to delete push notification',
89+
error: 'Failed to delete push notification',
9090
},
9191
{ status: 500 }
9292
)

apps/sim/app/api/tools/a2a/get-agent-card/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export async function POST(request: NextRequest) {
8484
return NextResponse.json(
8585
{
8686
success: false,
87-
error: error instanceof Error ? error.message : 'Failed to fetch Agent Card',
87+
error: 'Failed to fetch Agent Card',
8888
},
8989
{ status: 500 }
9090
)

apps/sim/app/api/tools/a2a/get-push-notification/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export async function POST(request: NextRequest) {
107107
return NextResponse.json(
108108
{
109109
success: false,
110-
error: error instanceof Error ? error.message : 'Failed to get push notification',
110+
error: 'Failed to get push notification',
111111
},
112112
{ status: 500 }
113113
)

apps/sim/app/api/tools/a2a/get-task/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export async function POST(request: NextRequest) {
8787
return NextResponse.json(
8888
{
8989
success: false,
90-
error: error instanceof Error ? error.message : 'Failed to get task',
90+
error: 'Failed to get task',
9191
},
9292
{ status: 500 }
9393
)

apps/sim/app/api/tools/a2a/resubscribe/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export async function POST(request: NextRequest) {
111111
return NextResponse.json(
112112
{
113113
success: false,
114-
error: error instanceof Error ? error.message : 'Failed to resubscribe',
114+
error: 'Failed to resubscribe',
115115
},
116116
{ status: 500 }
117117
)

apps/sim/app/api/tools/a2a/send-message/route.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export async function POST(request: NextRequest) {
7070
return NextResponse.json(
7171
{
7272
success: false,
73-
error: `Failed to connect to agent: ${clientError instanceof Error ? clientError.message : 'Unknown error'}`,
73+
error: 'Failed to connect to agent',
7474
},
7575
{ status: 502 }
7676
)
@@ -158,7 +158,7 @@ export async function POST(request: NextRequest) {
158158
return NextResponse.json(
159159
{
160160
success: false,
161-
error: `Failed to send message: ${sendError instanceof Error ? sendError.message : 'Unknown error'}`,
161+
error: 'Failed to send message to agent',
162162
},
163163
{ status: 502 }
164164
)
@@ -218,7 +218,7 @@ export async function POST(request: NextRequest) {
218218
return NextResponse.json(
219219
{
220220
success: false,
221-
error: error instanceof Error ? error.message : 'Internal server error',
221+
error: 'Internal server error',
222222
},
223223
{ status: 500 }
224224
)

0 commit comments

Comments
 (0)