diff --git a/apps/sim/app/api/files/utils.test.ts b/apps/sim/app/api/files/utils.test.ts index d0ad4567ac..b3deae47bd 100644 --- a/apps/sim/app/api/files/utils.test.ts +++ b/apps/sim/app/api/files/utils.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { createFileResponse, extractFilename } from './utils' +import { createFileResponse, extractFilename, findLocalFile } from './utils' describe('extractFilename', () => { describe('legitimate file paths', () => { @@ -325,3 +325,91 @@ describe('extractFilename', () => { }) }) }) + +describe('findLocalFile - Path Traversal Security Tests', () => { + describe('path traversal attack prevention', () => { + it.concurrent('should reject classic path traversal attacks', () => { + const maliciousInputs = [ + '../../../etc/passwd', + '..\\..\\..\\windows\\system32\\config\\sam', + '../../../../etc/shadow', + '../config.json', + '..\\config.ini', + ] + + maliciousInputs.forEach((input) => { + const result = findLocalFile(input) + expect(result).toBeNull() + }) + }) + + it.concurrent('should reject encoded path traversal attempts', () => { + const encodedInputs = [ + '%2e%2e%2f%2e%2e%2f%65%74%63%2f%70%61%73%73%77%64', // ../../../etc/passwd + '..%2f..%2fetc%2fpasswd', + '..%5c..%5cconfig.ini', + ] + + encodedInputs.forEach((input) => { + const result = findLocalFile(input) + expect(result).toBeNull() + }) + }) + + it.concurrent('should reject mixed path separators', () => { + const mixedInputs = ['../..\\config.txt', '..\\../secret.ini', '/..\\..\\system32'] + + mixedInputs.forEach((input) => { + const result = findLocalFile(input) + expect(result).toBeNull() + }) + }) + + it.concurrent('should reject filenames with dangerous characters', () => { + const dangerousInputs = [ + 'file:with:colons.txt', + 'file|with|pipes.txt', + 'file?with?questions.txt', + 'file*with*asterisks.txt', + ] + + dangerousInputs.forEach((input) => { + const result = findLocalFile(input) + expect(result).toBeNull() + }) + }) + + it.concurrent('should reject null and empty inputs', () => { + expect(findLocalFile('')).toBeNull() + expect(findLocalFile(' ')).toBeNull() + expect(findLocalFile('\t\n')).toBeNull() + }) + + it.concurrent('should reject filenames that become empty after sanitization', () => { + const emptyAfterSanitization = ['../..', '..\\..\\', '////', '....', '..'] + + emptyAfterSanitization.forEach((input) => { + const result = findLocalFile(input) + expect(result).toBeNull() + }) + }) + }) + + describe('security validation passes for legitimate files', () => { + it.concurrent('should accept properly formatted filenames without throwing errors', () => { + const legitimateInputs = [ + 'document.pdf', + 'image.png', + 'data.csv', + 'report-2024.doc', + 'file_with_underscores.txt', + 'file-with-dashes.json', + ] + + legitimateInputs.forEach((input) => { + // Should not throw security errors for legitimate filenames + expect(() => findLocalFile(input)).not.toThrow() + }) + }) + }) +}) diff --git a/apps/sim/app/api/files/utils.ts b/apps/sim/app/api/files/utils.ts index 4e427bb77c..2e88f0c9c4 100644 --- a/apps/sim/app/api/files/utils.ts +++ b/apps/sim/app/api/files/utils.ts @@ -1,8 +1,11 @@ import { existsSync } from 'fs' -import { join } from 'path' +import { join, resolve, sep } from 'path' import { NextResponse } from 'next/server' +import { createLogger } from '@/lib/logs/console/logger' import { UPLOAD_DIR } from '@/lib/uploads/setup' +const logger = createLogger('FilesUtils') + /** * Response type definitions */ @@ -192,18 +195,71 @@ export function extractFilename(path: string): string { } /** - * Find a file in possible local storage locations + * Sanitize filename to prevent path traversal attacks + */ +function sanitizeFilename(filename: string): string { + if (!filename || typeof filename !== 'string') { + throw new Error('Invalid filename provided') + } + + const sanitized = filename + .replace(/\.\./g, '') // Remove .. sequences + .replace(/[/\\]/g, '') // Remove path separators + .replace(/^\./g, '') // Remove leading dots + .trim() + + if (!sanitized || sanitized.length === 0) { + throw new Error('Invalid or empty filename after sanitization') + } + + if ( + sanitized.includes(':') || + sanitized.includes('|') || + sanitized.includes('?') || + sanitized.includes('*') || + sanitized.includes('\x00') || // Null bytes + /[\x00-\x1F\x7F]/.test(sanitized) // Control characters + ) { + throw new Error('Filename contains invalid characters') + } + + return sanitized +} + +/** + * Find a file in possible local storage locations with proper path validation */ export function findLocalFile(filename: string): string | null { - const possiblePaths = [join(UPLOAD_DIR, filename), join(process.cwd(), 'uploads', filename)] + try { + const sanitizedFilename = sanitizeFilename(filename) + + const possiblePaths = [ + join(UPLOAD_DIR, sanitizedFilename), + join(process.cwd(), 'uploads', sanitizedFilename), + ] + + for (const path of possiblePaths) { + const resolvedPath = resolve(path) + const allowedDirs = [resolve(UPLOAD_DIR), resolve(process.cwd(), 'uploads')] - for (const path of possiblePaths) { - if (existsSync(path)) { - return path + const isWithinAllowedDir = allowedDirs.some( + (allowedDir) => resolvedPath.startsWith(allowedDir + sep) || resolvedPath === allowedDir + ) + + if (!isWithinAllowedDir) { + continue // Skip this path as it's outside allowed directories + } + + if (existsSync(resolvedPath)) { + return resolvedPath + } } - } - return null + return null + } catch (error) { + logger.error('Error in findLocalFile:', error) + return null + } } const SAFE_INLINE_TYPES = new Set([ diff --git a/apps/sim/app/api/function/execute/route.test.ts b/apps/sim/app/api/function/execute/route.test.ts index 5ca4eeb36a..8e32ae9cc0 100644 --- a/apps/sim/app/api/function/execute/route.test.ts +++ b/apps/sim/app/api/function/execute/route.test.ts @@ -48,8 +48,52 @@ describe('Function Execute API Route', () => { vi.clearAllMocks() }) + describe('Security Tests', () => { + it.concurrent('should create secure fetch in VM context', async () => { + const req = createMockRequest('POST', { + code: 'return "test"', + useLocalVM: true, + }) + + const { POST } = await import('@/app/api/function/execute/route') + await POST(req) + + expect(mockCreateContext).toHaveBeenCalled() + const contextArgs = mockCreateContext.mock.calls[0][0] + expect(contextArgs).toHaveProperty('fetch') + expect(typeof contextArgs.fetch).toBe('function') + + expect(contextArgs.fetch.name).toBe('secureFetch') + }) + + it.concurrent('should block SSRF attacks through secure fetch wrapper', async () => { + const { validateProxyUrl } = await import('@/lib/security/url-validation') + + expect(validateProxyUrl('http://169.254.169.254/latest/meta-data/').isValid).toBe(false) + expect(validateProxyUrl('http://127.0.0.1:8080/admin').isValid).toBe(false) + expect(validateProxyUrl('http://192.168.1.1/config').isValid).toBe(false) + expect(validateProxyUrl('http://10.0.0.1/internal').isValid).toBe(false) + }) + + it.concurrent('should allow legitimate external URLs', async () => { + const { validateProxyUrl } = await import('@/lib/security/url-validation') + + expect(validateProxyUrl('https://api.github.com/user').isValid).toBe(true) + expect(validateProxyUrl('https://httpbin.org/get').isValid).toBe(true) + expect(validateProxyUrl('http://example.com/api').isValid).toBe(true) + }) + + it.concurrent('should block dangerous protocols', async () => { + const { validateProxyUrl } = await import('@/lib/security/url-validation') + + expect(validateProxyUrl('file:///etc/passwd').isValid).toBe(false) + expect(validateProxyUrl('ftp://internal.server/files').isValid).toBe(false) + expect(validateProxyUrl('gopher://old.server/menu').isValid).toBe(false) + }) + }) + describe('Basic Function Execution', () => { - it('should execute simple JavaScript code successfully', async () => { + it.concurrent('should execute simple JavaScript code successfully', async () => { const req = createMockRequest('POST', { code: 'return "Hello World"', timeout: 5000, @@ -66,7 +110,7 @@ describe('Function Execute API Route', () => { expect(data.output).toHaveProperty('executionTime') }) - it('should handle missing code parameter', async () => { + it.concurrent('should handle missing code parameter', async () => { const req = createMockRequest('POST', { timeout: 5000, }) @@ -80,7 +124,7 @@ describe('Function Execute API Route', () => { expect(data).toHaveProperty('error') }) - it('should use default timeout when not provided', async () => { + it.concurrent('should use default timeout when not provided', async () => { const req = createMockRequest('POST', { code: 'return "test"', useLocalVM: true, @@ -100,7 +144,7 @@ describe('Function Execute API Route', () => { }) describe('Template Variable Resolution', () => { - it('should resolve environment variables with {{var_name}} syntax', async () => { + it.concurrent('should resolve environment variables with {{var_name}} syntax', async () => { const req = createMockRequest('POST', { code: 'return {{API_KEY}}', useLocalVM: true, @@ -116,7 +160,7 @@ describe('Function Execute API Route', () => { // The code should be resolved to: return "secret-key-123" }) - it('should resolve tag variables with syntax', async () => { + it.concurrent('should resolve tag variables with syntax', async () => { const req = createMockRequest('POST', { code: 'return ', useLocalVM: true, @@ -132,7 +176,7 @@ describe('Function Execute API Route', () => { // The code should be resolved with the email object }) - it('should NOT treat email addresses as template variables', async () => { + it.concurrent('should NOT treat email addresses as template variables', async () => { const req = createMockRequest('POST', { code: 'return "Email sent to user"', useLocalVM: true, @@ -151,7 +195,7 @@ describe('Function Execute API Route', () => { // Should not try to replace as a template variable }) - it('should only match valid variable names in angle brackets', async () => { + it.concurrent('should only match valid variable names in angle brackets', async () => { const req = createMockRequest('POST', { code: 'return + "" + ', useLocalVM: true, @@ -170,64 +214,70 @@ describe('Function Execute API Route', () => { }) describe('Gmail Email Data Handling', () => { - it('should handle Gmail webhook data with email addresses containing angle brackets', async () => { - const gmailData = { - email: { - id: '123', - from: 'Waleed Latif ', - to: 'User ', - subject: 'Test Email', - bodyText: 'Hello world', - }, - rawEmail: { - id: '123', - payload: { - headers: [ - { name: 'From', value: 'Waleed Latif ' }, - { name: 'To', value: 'User ' }, - ], + it.concurrent( + 'should handle Gmail webhook data with email addresses containing angle brackets', + async () => { + const gmailData = { + email: { + id: '123', + from: 'Waleed Latif ', + to: 'User ', + subject: 'Test Email', + bodyText: 'Hello world', }, - }, - } - - const req = createMockRequest('POST', { - code: 'return ', - useLocalVM: true, - params: gmailData, - }) + rawEmail: { + id: '123', + payload: { + headers: [ + { name: 'From', value: 'Waleed Latif ' }, + { name: 'To', value: 'User ' }, + ], + }, + }, + } - const { POST } = await import('@/app/api/function/execute/route') - const response = await POST(req) + const req = createMockRequest('POST', { + code: 'return ', + useLocalVM: true, + params: gmailData, + }) - expect(response.status).toBe(200) - const data = await response.json() - expect(data.success).toBe(true) - }) + const { POST } = await import('@/app/api/function/execute/route') + const response = await POST(req) - it('should properly serialize complex email objects with special characters', async () => { - const complexEmailData = { - email: { - from: 'Test User ', - bodyHtml: '
HTML content with "quotes" and \'apostrophes\'
', - bodyText: 'Text with\nnewlines\tand\ttabs', - }, + expect(response.status).toBe(200) + const data = await response.json() + expect(data.success).toBe(true) } + ) - const req = createMockRequest('POST', { - code: 'return ', - useLocalVM: true, - params: complexEmailData, - }) + it.concurrent( + 'should properly serialize complex email objects with special characters', + async () => { + const complexEmailData = { + email: { + from: 'Test User ', + bodyHtml: '
HTML content with "quotes" and \'apostrophes\'
', + bodyText: 'Text with\nnewlines\tand\ttabs', + }, + } - const { POST } = await import('@/app/api/function/execute/route') - const response = await POST(req) + const req = createMockRequest('POST', { + code: 'return ', + useLocalVM: true, + params: complexEmailData, + }) - expect(response.status).toBe(200) - }) + const { POST } = await import('@/app/api/function/execute/route') + const response = await POST(req) + + expect(response.status).toBe(200) + } + ) }) describe('Custom Tools', () => { - it('should handle custom tool execution with direct parameter access', async () => { + it.concurrent('should handle custom tool execution with direct parameter access', async () => { const req = createMockRequest('POST', { code: 'return location + " weather is sunny"', useLocalVM: true, @@ -246,7 +296,7 @@ describe('Function Execute API Route', () => { }) describe('Security and Edge Cases', () => { - it('should handle malformed JSON in request body', async () => { + it.concurrent('should handle malformed JSON in request body', async () => { const req = new NextRequest('http://localhost:3000/api/function/execute', { method: 'POST', body: 'invalid json{', @@ -259,7 +309,7 @@ describe('Function Execute API Route', () => { expect(response.status).toBe(500) }) - it('should handle timeout parameter', async () => { + it.concurrent('should handle timeout parameter', async () => { const req = createMockRequest('POST', { code: 'return "test"', useLocalVM: true, @@ -277,7 +327,7 @@ describe('Function Execute API Route', () => { ) }) - it('should handle empty parameters object', async () => { + it.concurrent('should handle empty parameters object', async () => { const req = createMockRequest('POST', { code: 'return "no params"', useLocalVM: true, @@ -485,7 +535,7 @@ SyntaxError: Invalid or unexpected token expect(data.debug.lineContent).toBe('return a + b + c + d;') }) - it('should provide helpful suggestions for common syntax errors', async () => { + it.concurrent('should provide helpful suggestions for common syntax errors', async () => { const mockScript = vi.fn().mockImplementation(() => { const error = new Error('Unexpected end of input') error.name = 'SyntaxError' @@ -517,7 +567,7 @@ SyntaxError: Invalid or unexpected token }) describe('Utility Functions', () => { - it('should properly escape regex special characters', async () => { + it.concurrent('should properly escape regex special characters', async () => { // This tests the escapeRegExp function indirectly const req = createMockRequest('POST', { code: 'return {{special.chars+*?}}', @@ -534,7 +584,7 @@ SyntaxError: Invalid or unexpected token // Should handle special regex characters in variable names }) - it('should handle JSON serialization edge cases', async () => { + it.concurrent('should handle JSON serialization edge cases', async () => { // Test with complex but not circular data first const req = createMockRequest('POST', { code: 'return ', diff --git a/apps/sim/app/api/function/execute/route.ts b/apps/sim/app/api/function/execute/route.ts index 2182078b15..a046fa3c39 100644 --- a/apps/sim/app/api/function/execute/route.ts +++ b/apps/sim/app/api/function/execute/route.ts @@ -4,6 +4,7 @@ import { env, isTruthy } from '@/lib/env' import { executeInE2B } from '@/lib/execution/e2b' import { CodeLanguage, DEFAULT_CODE_LANGUAGE, isValidCodeLanguage } from '@/lib/execution/languages' import { createLogger } from '@/lib/logs/console/logger' +import { validateProxyUrl } from '@/lib/security/url-validation' import { generateRequestId } from '@/lib/utils' export const dynamic = 'force-dynamic' export const runtime = 'nodejs' @@ -11,6 +12,29 @@ export const maxDuration = 60 const logger = createLogger('FunctionExecuteAPI') +function createSecureFetch(requestId: string) { + const originalFetch = (globalThis as any).fetch || require('node-fetch').default + + return async function secureFetch(input: any, init?: any) { + const url = typeof input === 'string' ? input : input?.url || input + + if (!url || typeof url !== 'string') { + throw new Error('Invalid URL provided to fetch') + } + + const validation = validateProxyUrl(url) + if (!validation.isValid) { + logger.warn(`[${requestId}] Blocked fetch request due to SSRF validation`, { + url: url.substring(0, 100), + error: validation.error, + }) + throw new Error(`Security Error: ${validation.error}`) + } + + return originalFetch(input, init) + } +} + // Constants for E2B code wrapping line counts const E2B_JS_WRAPPER_LINES = 3 // Lines before user code: ';(async () => {', ' try {', ' const __sim_result = await (async () => {' const E2B_PYTHON_WRAPPER_LINES = 1 // Lines before user code: 'def __sim_main__():' @@ -737,7 +761,7 @@ export async function POST(req: NextRequest) { params: executionParams, environmentVariables: envVars, ...contextVariables, - fetch: (globalThis as any).fetch || require('node-fetch').default, + fetch: createSecureFetch(requestId), console: { log: (...args: any[]) => { const logMessage = `${args