Skip to content

Commit 3edaae3

Browse files
authored
fix(mcp): correct fetchFn fallback order in mcpAuthGuarded (#5423)
Spread order previously let an explicit fetchFn (including fetchFn: undefined) in options silently disable the SSRF-guarded default. Fallback is now applied after the spread so the guard always wins unless a real override is passed. fix(tools): handle non-numeric Drive file size in early size check Guard the pre-download size check against a malformed metadata.size string so it's skipped explicitly instead of relying on an incidental NaN no-op; the streaming cap on the actual download still enforces the limit either way.
1 parent f456ed9 commit 3edaae3

4 files changed

Lines changed: 42 additions & 6 deletions

File tree

apps/sim/app/api/tools/google_drive/download/route.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,30 @@ describe('POST /api/tools/google_drive/download', () => {
133133
expect(data.success).toBe(false)
134134
})
135135

136+
it('proceeds to the streamed download when metadata size is malformed', async () => {
137+
mockSecureFetchWithPinnedIP
138+
.mockResolvedValueOnce(
139+
jsonResponse({
140+
id: 'file-abc',
141+
name: 'report.pdf',
142+
mimeType: 'application/pdf',
143+
size: 'not-a-number',
144+
capabilities: { canReadRevisions: false },
145+
})
146+
)
147+
.mockResolvedValueOnce(fileResponse(1024))
148+
149+
const response = await POST(createMockRequest('POST', baseBody))
150+
expect(response.status).toBe(200)
151+
const data = (await response.json()) as { success: boolean; output: { file: { size: number } } }
152+
expect(data.success).toBe(true)
153+
expect(data.output.file.size).toBe(1024)
154+
155+
// The early size check should be skipped, but the streaming cap must still apply.
156+
const downloadCall = mockSecureFetchWithPinnedIP.mock.calls[1]
157+
expect(downloadCall[2]).toMatchObject({ maxResponseBytes: MAX_FILE_SIZE })
158+
})
159+
136160
it('does not require a metadata size for Google Workspace exports', async () => {
137161
mockSecureFetchWithPinnedIP
138162
.mockResolvedValueOnce(

apps/sim/app/api/tools/google_drive/download/route.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
188188
logger.info(`[${requestId}] Downloading regular file`, { fileId, mimeType: fileMimeType })
189189

190190
if (metadata.size) {
191-
assertKnownSizeWithinLimit(
192-
Number.parseInt(metadata.size, 10),
193-
MAX_FILE_SIZE,
194-
`Google Drive file ${fileId}`
195-
)
191+
const parsedSize = Number.parseInt(metadata.size, 10)
192+
if (Number.isFinite(parsedSize)) {
193+
assertKnownSizeWithinLimit(parsedSize, MAX_FILE_SIZE, `Google Drive file ${fileId}`)
194+
}
196195
}
197196

198197
const downloadUrl = `https://www.googleapis.com/drive/v3/files/${fileId}?alt=media&supportsAllDrives=true`

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,17 @@ describe('mcpAuthGuarded', () => {
5151
fetchFn: overrideFetch,
5252
})
5353
})
54+
55+
it('falls back to the SSRF-guarded fetch when fetchFn is explicitly undefined', async () => {
56+
await mcpAuthGuarded(provider, {
57+
serverUrl: 'https://mcp.example.com/mcp',
58+
fetchFn: undefined,
59+
})
60+
61+
expect(mockCreateSsrfGuardedMcpFetch).toHaveBeenCalledTimes(1)
62+
expect(mockAuth).toHaveBeenCalledWith(provider, {
63+
serverUrl: 'https://mcp.example.com/mcp',
64+
fetchFn: mockGuardedFetch,
65+
})
66+
})
5467
})

apps/sim/lib/mcp/oauth/auth.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@ export function mcpAuthGuarded(
1515
provider: OAuthClientProvider,
1616
options: McpAuthOptions
1717
): ReturnType<typeof auth> {
18-
return auth(provider, { fetchFn: createSsrfGuardedMcpFetch(), ...options })
18+
return auth(provider, { ...options, fetchFn: options.fetchFn ?? createSsrfGuardedMcpFetch() })
1919
}

0 commit comments

Comments
 (0)