From 68c604f1f00707be40c4285642380989111900ce Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 11 Jul 2026 11:31:37 -0700 Subject: [PATCH 1/2] fix(files-upload): enforce workspace authorization on mothership uploads The mothership context in POST /api/files/upload skipped the workspace permission and storage quota checks that every sibling context enforces, letting a caller write files into a workspace they have no access to. --- apps/sim/app/api/files/upload/route.test.ts | 106 ++++++++++++++++++++ apps/sim/app/api/files/upload/route.ts | 17 ++++ 2 files changed, 123 insertions(+) diff --git a/apps/sim/app/api/files/upload/route.test.ts b/apps/sim/app/api/files/upload/route.test.ts index f6df57eb59d..6750165d51f 100644 --- a/apps/sim/app/api/files/upload/route.test.ts +++ b/apps/sim/app/api/files/upload/route.test.ts @@ -24,6 +24,7 @@ const mocks = vi.hoisted(() => { const mockIsUsingCloudStorage = vi.fn() const mockUploadFile = vi.fn() const mockUploadExecutionFile = vi.fn() + const mockCheckStorageQuota = vi.fn() return { mockVerifyFileAccess, @@ -35,6 +36,7 @@ const mocks = vi.hoisted(() => { mockIsUsingCloudStorage, mockUploadFile, mockUploadExecutionFile, + mockCheckStorageQuota, } }) @@ -108,6 +110,10 @@ vi.mock('@/lib/uploads/setup.server', () => ({ UPLOAD_DIR_SERVER: '/tmp/test-uploads', })) +vi.mock('@/lib/billing/storage', () => ({ + checkStorageQuota: mocks.mockCheckStorageQuota, +})) + import { uploadWorkspaceFile } from '@/lib/uploads/contexts/workspace' import { POST } from '@/app/api/files/upload/route' @@ -183,6 +189,12 @@ function setupFileApiMocks( key: 'test-key', path: '/test/path', }) + + mocks.mockCheckStorageQuota.mockResolvedValue({ + allowed: true, + currentUsage: 0, + limit: Number.MAX_SAFE_INTEGER, + }) } describe('File Upload API Route', () => { @@ -640,6 +652,100 @@ describe('File Upload Security Tests', () => { }) }) + describe('Mothership Context Permission Gate', () => { + const postMothershipUpload = async (workspaceId: string | null = 'test-workspace-id') => { + const formData = new FormData() + const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' }) + formData.append('file', file) + formData.append('context', 'mothership') + if (workspaceId !== null) formData.append('workspaceId', workspaceId) + + const req = new Request('http://localhost/api/files/upload', { + method: 'POST', + headers: { 'content-length': '1024' }, + body: formData, + }) + + return POST(req as unknown as NextRequest) + } + + beforeEach(() => { + setupFileApiMocks({ + cloudEnabled: false, + storageProvider: 'local', + }) + }) + + it('rejects mothership uploads without workspaceId', async () => { + const response = await postMothershipUpload(null) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.message).toContain('workspaceId') + expect(storageServiceMockFns.mockUploadFile).not.toHaveBeenCalled() + }) + + it('rejects mothership uploads for a workspace the caller does not belong to', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue(null) + + const response = await postMothershipUpload() + + expect(response.status).toBe(403) + const data = await response.json() + expect(data.error).toBe('Write or Admin access required for mothership uploads') + expect(storageServiceMockFns.mockUploadFile).not.toHaveBeenCalled() + }) + + it('rejects mothership uploads for a read-only workspace member', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('read') + + const response = await postMothershipUpload() + + expect(response.status).toBe(403) + expect(storageServiceMockFns.mockUploadFile).not.toHaveBeenCalled() + }) + + it('rejects mothership uploads over the caller storage quota', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write') + mocks.mockCheckStorageQuota.mockResolvedValue({ + allowed: false, + currentUsage: 100, + limit: 100, + error: 'Storage limit exceeded. Used: 0.00GB, Limit: 0GB', + }) + + const response = await postMothershipUpload() + + expect(response.status).toBe(413) + const data = await response.json() + expect(data.error).toContain('Storage limit exceeded') + expect(storageServiceMockFns.mockUploadFile).not.toHaveBeenCalled() + }) + + it('allows mothership uploads for a write-permission workspace member', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write') + + const response = await postMothershipUpload() + + expect(response.status).toBe(200) + expect(permissionsMockFns.mockGetUserEntityPermissions).toHaveBeenCalledWith( + 'test-user-id', + 'workspace', + 'test-workspace-id' + ) + expect(storageServiceMockFns.mockUploadFile).toHaveBeenCalled() + }) + + it('allows mothership uploads for an admin-permission workspace member', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('admin') + + const response = await postMothershipUpload() + + expect(response.status).toBe(200) + expect(storageServiceMockFns.mockUploadFile).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 64914af8970..23e09ede401 100644 --- a/apps/sim/app/api/files/upload/route.ts +++ b/apps/sim/app/api/files/upload/route.ts @@ -266,6 +266,23 @@ export const POST = withRouteHandler(async (request: NextRequest) => { throw new InvalidRequestError('Chat context requires workspaceId parameter') } + const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId) + if (permission !== 'write' && permission !== 'admin') { + return NextResponse.json( + { error: 'Write or Admin access required for mothership uploads' }, + { status: 403 } + ) + } + + const { checkStorageQuota } = await import('@/lib/billing/storage') + const quotaCheck = await checkStorageQuota(session.user.id, buffer.length) + if (!quotaCheck.allowed) { + return NextResponse.json( + { error: quotaCheck.error || 'Storage limit exceeded' }, + { status: 413 } + ) + } + logger.info(`Uploading mothership file: ${originalName}`) const storageKey = generateWorkspaceFileKey(workspaceId, originalName) From eb714024e301211ed1ca24e5a84f4db82abf6143 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 11 Jul 2026 12:41:46 -0700 Subject: [PATCH 2/2] fix(files-upload): check mothership quota once against the full batch Resolve the mothership permission and quota check once per request (mirroring the existing execution-context pattern) instead of per file: a per-file quota check let a multi-file batch exceed the caller's quota since each file's own size fit even when the combined total did not. Also corrects the missing-workspaceId error message, which named the chat context instead of mothership. --- apps/sim/app/api/files/upload/route.test.ts | 25 +++++++++ apps/sim/app/api/files/upload/route.ts | 57 ++++++++++++--------- 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/apps/sim/app/api/files/upload/route.test.ts b/apps/sim/app/api/files/upload/route.test.ts index 6750165d51f..aa810049034 100644 --- a/apps/sim/app/api/files/upload/route.test.ts +++ b/apps/sim/app/api/files/upload/route.test.ts @@ -744,6 +744,31 @@ describe('File Upload Security Tests', () => { expect(response.status).toBe(200) expect(storageServiceMockFns.mockUploadFile).toHaveBeenCalled() }) + + it('checks quota once against the combined size of a multi-file batch', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write') + + const formData = new FormData() + const fileA = new File(['a'.repeat(10)], 'a.pdf', { type: 'application/pdf' }) + const fileB = new File(['b'.repeat(20)], 'b.pdf', { type: 'application/pdf' }) + formData.append('file', fileA) + formData.append('file', fileB) + formData.append('context', 'mothership') + formData.append('workspaceId', 'test-workspace-id') + + 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.mockCheckStorageQuota).toHaveBeenCalledTimes(1) + expect(mocks.mockCheckStorageQuota).toHaveBeenCalledWith('test-user-id', 30) + expect(permissionsMockFns.mockGetUserEntityPermissions).toHaveBeenCalledTimes(1) + }) }) describe('Authentication Requirements', () => { diff --git a/apps/sim/app/api/files/upload/route.ts b/apps/sim/app/api/files/upload/route.ts index 23e09ede401..ab934a64960 100644 --- a/apps/sim/app/api/files/upload/route.ts +++ b/apps/sim/app/api/files/upload/route.ts @@ -115,6 +115,36 @@ export const POST = withRouteHandler(async (request: NextRequest) => { executionUploadContext = { workspaceId, workflowId, executionId } } + // Mothership context requires the same workspace write/admin permission check, plus a + // storage quota check. Resolve both once per request (not per file) since workspaceId is + // invariant across all files in the upload and quota must account for the full batch size, + // not just one file. + let mothershipWorkspaceId: string | undefined + if (context === 'mothership') { + if (!workspaceId) { + throw new InvalidRequestError('Mothership context requires workspaceId parameter') + } + + const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId) + if (permission !== 'write' && permission !== 'admin') { + return NextResponse.json( + { error: 'Write or Admin access required for mothership uploads' }, + { status: 403 } + ) + } + + const { checkStorageQuota } = await import('@/lib/billing/storage') + const quotaCheck = await checkStorageQuota(session.user.id, totalFileSize) + if (!quotaCheck.allowed) { + return NextResponse.json( + { error: quotaCheck.error || 'Storage limit exceeded' }, + { status: 413 } + ) + } + + mothershipWorkspaceId = workspaceId + } + const uploadResults = [] for (const file of files) { @@ -261,38 +291,17 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } // Handle mothership context (chat-scoped uploads to workspace S3) - if (context === 'mothership') { - if (!workspaceId) { - throw new InvalidRequestError('Chat context requires workspaceId parameter') - } - - const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId) - if (permission !== 'write' && permission !== 'admin') { - return NextResponse.json( - { error: 'Write or Admin access required for mothership uploads' }, - { status: 403 } - ) - } - - const { checkStorageQuota } = await import('@/lib/billing/storage') - const quotaCheck = await checkStorageQuota(session.user.id, buffer.length) - if (!quotaCheck.allowed) { - return NextResponse.json( - { error: quotaCheck.error || 'Storage limit exceeded' }, - { status: 413 } - ) - } - + if (context === 'mothership' && mothershipWorkspaceId) { logger.info(`Uploading mothership file: ${originalName}`) - const storageKey = generateWorkspaceFileKey(workspaceId, originalName) + const storageKey = generateWorkspaceFileKey(mothershipWorkspaceId, originalName) const metadata: Record = { originalName: originalName, uploadedAt: new Date().toISOString(), purpose: 'mothership', userId: session.user.id, - workspaceId, + workspaceId: mothershipWorkspaceId, } const fileInfo = await storageService.uploadFile({