Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions apps/sim/app/api/files/parse/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,68 @@ describe('Files Parse API - Path Traversal Security', () => {
}
})

it('should not treat .. inside external URLs as path traversal', async () => {
inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
isValid: true,
resolvedIP: '203.0.113.10',
})
inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValue(
new Response('slack file content', {
status: 200,
headers: { 'content-type': 'text/plain' },
})
)
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write')

// Slack truncates long titles with a literal ellipsis, so the slug contains `..`
const slackUrl =
'https://files.slack.com/files-pri/T08-F0B/_other__no_invitation_messages_get_sent_-_sim_on_railway...txt'

const request = new NextRequest('http://localhost:3000/api/files/parse', {
method: 'POST',
body: JSON.stringify({ filePath: slackUrl, workspaceId: 'workspace-id' }),
})

const response = await POST(request)
const result = await response.json()

expect(result.success).toBe(true)
// The URL reaching the pinned fetch proves it passed validation and routed
// to external-URL handling rather than being rejected as a local path.
expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).toHaveBeenCalledWith(
slackUrl,
'203.0.113.10',
expect.any(Object)
)
})

it('should still reject traversal in https URLs that look like internal serve URLs', async () => {
inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
isValid: true,
resolvedIP: '203.0.113.10',
})
inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValue(
new Response('should never be fetched', { status: 200 })
)

// Absolute https URL containing `/api/files/serve/` matches isInternalFileUrl and would
// route to handleCloudFile — so it must keep traversal protection, not be waved through
// as an external URL.
const request = new NextRequest('http://localhost:3000/api/files/parse', {
method: 'POST',
body: JSON.stringify({
filePath: 'https://attacker.com/api/files/serve/../../../etc/passwd',
}),
})

const response = await POST(request)
const result = await response.json()

expect(result.success).toBe(false)
expect(result.error).toMatch(/Access denied: path traversal detected/)
expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
})

it('should handle encoded path traversal attempts', async () => {
const encodedMaliciousPaths = [
'/api/files/serve/%2e%2e%2f%2e%2e%2fetc%2fpasswd', // ../../../etc/passwd
Expand Down
22 changes: 21 additions & 1 deletion apps/sim/app/api/files/parse/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,13 +419,33 @@ function assertParsedContentWithinLimit(content: string, maxBytes?: number): str
}

/**
* Validate file path for security - prevents null byte injection and path traversal attacks
* Validate file path for security - prevents null byte injection and path traversal attacks.
*
* External URLs (`http`/`https`) are fetched over HTTP — with SSRF protection applied
* downstream in `fetchExternalUrlToWorkspace` (DNS resolution + private/reserved IP blocking)
* — and are never resolved against the filesystem, so `..`/`~` are legal URL content and must
* not be rejected. Providers such as Slack routinely emit slugs containing a literal `...`.
*
* Internal file URLs (`/api/files/serve/...`) ARE resolved to storage keys and filesystem
* paths via `extractStorageKey`, so they keep full traversal protection. The external
* short-circuit explicitly excludes them: `parseFileSingle` routes anything matching
* `isInternalFileUrl` to `handleCloudFile` (even an absolute `https://host/api/files/serve/...`),
* so such inputs must stay subject to the `..`/`~` checks rather than being waved through as
* external URLs. Only the leading-`/` "outside allowed directory" check is relaxed for them,
* since that prefix is expected.
*/
Comment thread
waleedlatif1 marked this conversation as resolved.
function validateFilePath(filePath: string): { isValid: boolean; error?: string } {
if (filePath.includes('\0')) {
return { isValid: false, error: 'Invalid path: null byte detected' }
}

if (
(filePath.startsWith('http://') || filePath.startsWith('https://')) &&
!isInternalFileUrl(filePath)
) {
return { isValid: true }
}
Comment thread
waleedlatif1 marked this conversation as resolved.

if (filePath.includes('..')) {
return { isValid: false, error: 'Access denied: path traversal detected' }
}
Expand Down
Loading