Skip to content

Commit 499e834

Browse files
committed
fix(files): keep traversal protection for https URLs matching internal serve paths
1 parent a9a5503 commit 499e834

2 files changed

Lines changed: 37 additions & 3 deletions

File tree

apps/sim/app/api/files/parse/route.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -831,6 +831,33 @@ describe('Files Parse API - Path Traversal Security', () => {
831831
)
832832
})
833833

834+
it('should still reject traversal in https URLs that look like internal serve URLs', async () => {
835+
inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
836+
isValid: true,
837+
resolvedIP: '203.0.113.10',
838+
})
839+
inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValue(
840+
new Response('should never be fetched', { status: 200 })
841+
)
842+
843+
// Absolute https URL containing `/api/files/serve/` matches isInternalFileUrl and would
844+
// route to handleCloudFile — so it must keep traversal protection, not be waved through
845+
// as an external URL.
846+
const request = new NextRequest('http://localhost:3000/api/files/parse', {
847+
method: 'POST',
848+
body: JSON.stringify({
849+
filePath: 'https://attacker.com/api/files/serve/../../../etc/passwd',
850+
}),
851+
})
852+
853+
const response = await POST(request)
854+
const result = await response.json()
855+
856+
expect(result.success).toBe(false)
857+
expect(result.error).toMatch(/Access denied: path traversal detected/)
858+
expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
859+
})
860+
834861
it('should handle encoded path traversal attempts', async () => {
835862
const encodedMaliciousPaths = [
836863
'/api/files/serve/%2e%2e%2f%2e%2e%2fetc%2fpasswd', // ../../../etc/passwd

apps/sim/app/api/files/parse/route.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -427,15 +427,22 @@ function assertParsedContentWithinLimit(content: string, maxBytes?: number): str
427427
* not be rejected. Providers such as Slack routinely emit slugs containing a literal `...`.
428428
*
429429
* Internal file URLs (`/api/files/serve/...`) ARE resolved to storage keys and filesystem
430-
* paths via `extractStorageKey`, so they keep full traversal protection — only the leading-`/`
431-
* "outside allowed directory" check is relaxed for them, since that prefix is expected.
430+
* paths via `extractStorageKey`, so they keep full traversal protection. The external
431+
* short-circuit explicitly excludes them: `parseFileSingle` routes anything matching
432+
* `isInternalFileUrl` to `handleCloudFile` (even an absolute `https://host/api/files/serve/...`),
433+
* so such inputs must stay subject to the `..`/`~` checks rather than being waved through as
434+
* external URLs. Only the leading-`/` "outside allowed directory" check is relaxed for them,
435+
* since that prefix is expected.
432436
*/
433437
function validateFilePath(filePath: string): { isValid: boolean; error?: string } {
434438
if (filePath.includes('\0')) {
435439
return { isValid: false, error: 'Invalid path: null byte detected' }
436440
}
437441

438-
if (filePath.startsWith('http://') || filePath.startsWith('https://')) {
442+
if (
443+
(filePath.startsWith('http://') || filePath.startsWith('https://')) &&
444+
!isInternalFileUrl(filePath)
445+
) {
439446
return { isValid: true }
440447
}
441448

0 commit comments

Comments
 (0)