From db3083141b01d6a9c56b1447e409e0f67f338cc0 Mon Sep 17 00:00:00 2001 From: waleed Date: Fri, 3 Jul 2026 15:51:53 -0700 Subject: [PATCH] fix(uploads): gate execution-context uploads behind write/admin permission Fallback multipart upload route (/api/files/upload) had no workspace permission check for execution-context uploads, unlike the primary presigned-upload route which requires write/admin. Mirror that gate so both paths enforce the same access control. --- apps/sim/app/api/files/upload/route.test.ts | 141 ++++++++++++++++++++ apps/sim/app/api/files/upload/route.ts | 14 +- 2 files changed, 152 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/files/upload/route.test.ts b/apps/sim/app/api/files/upload/route.test.ts index 356d993d610..bbc42c5e743 100644 --- a/apps/sim/app/api/files/upload/route.test.ts +++ b/apps/sim/app/api/files/upload/route.test.ts @@ -23,6 +23,7 @@ const mocks = vi.hoisted(() => { const mockGetStorageProvider = vi.fn() const mockIsUsingCloudStorage = vi.fn() const mockUploadFile = vi.fn() + const mockUploadExecutionFile = vi.fn() return { mockVerifyFileAccess, @@ -33,6 +34,7 @@ const mocks = vi.hoisted(() => { mockGetStorageProvider, mockIsUsingCloudStorage, mockUploadFile, + mockUploadExecutionFile, } }) @@ -82,6 +84,10 @@ vi.mock('@/lib/uploads/contexts/workspace', () => ({ uploadWorkspaceFile: mocks.mockUploadWorkspaceFile, })) +vi.mock('@/lib/uploads/contexts/execution', () => ({ + uploadExecutionFile: mocks.mockUploadExecutionFile, +})) + vi.mock('@/lib/uploads', () => ({ getStorageProvider: mocks.mockGetStorageProvider, isUsingCloudStorage: mocks.mockIsUsingCloudStorage, @@ -151,6 +157,17 @@ function setupFileApiMocks( expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(), }) + mocks.mockUploadExecutionFile.mockResolvedValue({ + id: 'test-execution-file-id', + name: 'test.txt', + url: '/api/files/serve/execution/test-workspace-id/test-file.txt', + size: 100, + type: 'text/plain', + key: 'execution/test-workspace-id/1234567890-test.txt', + uploadedAt: new Date().toISOString(), + expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(), + }) + mocks.mockGetStorageProvider.mockReturnValue(storageProvider) mocks.mockIsUsingCloudStorage.mockReturnValue(cloudEnabled) mocks.mockUploadFile.mockResolvedValue({ @@ -531,6 +548,130 @@ describe('File Upload Security Tests', () => { }) }) + describe('Execution Context Permission Gate', () => { + const createExecutionFormData = ( + file: File, + workspaceId: string | null = 'test-workspace-id' + ) => { + const formData = new FormData() + formData.append('file', file) + formData.append('context', 'execution') + formData.append('workflowId', 'test-workflow-id') + formData.append('executionId', 'test-execution-id') + if (workspaceId !== null) formData.append('workspaceId', workspaceId) + return formData + } + + beforeEach(() => { + setupFileApiMocks({ + cloudEnabled: false, + storageProvider: 'local', + }) + }) + + it('rejects execution uploads without workspaceId', async () => { + const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' }) + const formData = createExecutionFormData(file, null) + + const req = new Request('http://localhost/api/files/upload', { + method: 'POST', + headers: { 'content-length': '1024' }, + body: formData, + }) + + const response = await POST(req as unknown as NextRequest) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.message).toContain('workflowId, executionId, and workspaceId') + expect(mocks.mockUploadExecutionFile).not.toHaveBeenCalled() + }) + + it('rejects execution uploads for a read-only workspace member', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('read') + + const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' }) + const formData = createExecutionFormData(file) + + const req = new Request('http://localhost/api/files/upload', { + method: 'POST', + headers: { 'content-length': '1024' }, + body: formData, + }) + + const response = await POST(req as unknown as NextRequest) + + expect(response.status).toBe(403) + const data = await response.json() + expect(data.error).toBe('Write or Admin access required for execution uploads') + expect(mocks.mockUploadExecutionFile).not.toHaveBeenCalled() + }) + + it('rejects execution uploads for a member with no workspace permission', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue(null) + + const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' }) + const formData = createExecutionFormData(file) + + const req = new Request('http://localhost/api/files/upload', { + method: 'POST', + headers: { 'content-length': '1024' }, + body: formData, + }) + + const response = await POST(req as unknown as NextRequest) + + expect(response.status).toBe(403) + expect(mocks.mockUploadExecutionFile).not.toHaveBeenCalled() + }) + + it('allows execution uploads for a write-permission workspace member', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write') + + const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' }) + const formData = createExecutionFormData(file) + + const req = new Request('http://localhost/api/files/upload', { + method: 'POST', + headers: { 'content-length': '1024' }, + body: formData, + }) + + const response = await POST(req as unknown as NextRequest) + + expect(response.status).toBe(200) + expect(mocks.mockUploadExecutionFile).toHaveBeenCalledWith( + { + workspaceId: 'test-workspace-id', + workflowId: 'test-workflow-id', + executionId: 'test-execution-id', + }, + expect.anything(), + 'test.pdf', + 'application/pdf', + 'test-user-id' + ) + }) + + it('allows execution uploads for an admin-permission workspace member', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('admin') + + const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' }) + const formData = createExecutionFormData(file) + + const req = new Request('http://localhost/api/files/upload', { + method: 'POST', + headers: { 'content-length': '1024' }, + body: formData, + }) + + const response = await POST(req as unknown as NextRequest) + + expect(response.status).toBe(200) + expect(mocks.mockUploadExecutionFile).toHaveBeenCalled() + }) + }) + describe('Authentication Requirements', () => { it('should reject uploads without authentication', async () => { authMockFns.mockGetSession.mockResolvedValue(null) diff --git a/apps/sim/app/api/files/upload/route.ts b/apps/sim/app/api/files/upload/route.ts index cb23b6ec851..a68fb9da045 100644 --- a/apps/sim/app/api/files/upload/route.ts +++ b/apps/sim/app/api/files/upload/route.ts @@ -111,16 +111,24 @@ export const POST = withRouteHandler(async (request: NextRequest) => { // Handle execution context if (context === 'execution') { - if (!workflowId || !executionId) { + if (!workflowId || !executionId || !workspaceId) { throw new InvalidRequestError( - 'Execution context requires workflowId and executionId parameters' + 'Execution context requires workflowId, executionId, and workspaceId parameters' + ) + } + + const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId) + if (permission !== 'write' && permission !== 'admin') { + return NextResponse.json( + { error: 'Write or Admin access required for execution uploads' }, + { status: 403 } ) } const { uploadExecutionFile } = await import('@/lib/uploads/contexts/execution') const userFile = await uploadExecutionFile( { - workspaceId: workspaceId || '', + workspaceId, workflowId, executionId, },