Skip to content

Commit d165712

Browse files
authored
fix(files-upload): enforce workspace authorization on mothership uploads (#5604)
* 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. * 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.
1 parent 67a2f00 commit d165712

2 files changed

Lines changed: 164 additions & 7 deletions

File tree

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const mocks = vi.hoisted(() => {
2424
const mockIsUsingCloudStorage = vi.fn()
2525
const mockUploadFile = vi.fn()
2626
const mockUploadExecutionFile = vi.fn()
27+
const mockCheckStorageQuota = vi.fn()
2728

2829
return {
2930
mockVerifyFileAccess,
@@ -35,6 +36,7 @@ const mocks = vi.hoisted(() => {
3536
mockIsUsingCloudStorage,
3637
mockUploadFile,
3738
mockUploadExecutionFile,
39+
mockCheckStorageQuota,
3840
}
3941
})
4042

@@ -108,6 +110,10 @@ vi.mock('@/lib/uploads/setup.server', () => ({
108110
UPLOAD_DIR_SERVER: '/tmp/test-uploads',
109111
}))
110112

113+
vi.mock('@/lib/billing/storage', () => ({
114+
checkStorageQuota: mocks.mockCheckStorageQuota,
115+
}))
116+
111117
import { uploadWorkspaceFile } from '@/lib/uploads/contexts/workspace'
112118
import { POST } from '@/app/api/files/upload/route'
113119

@@ -183,6 +189,12 @@ function setupFileApiMocks(
183189
key: 'test-key',
184190
path: '/test/path',
185191
})
192+
193+
mocks.mockCheckStorageQuota.mockResolvedValue({
194+
allowed: true,
195+
currentUsage: 0,
196+
limit: Number.MAX_SAFE_INTEGER,
197+
})
186198
}
187199

188200
describe('File Upload API Route', () => {
@@ -640,6 +652,125 @@ describe('File Upload Security Tests', () => {
640652
})
641653
})
642654

655+
describe('Mothership Context Permission Gate', () => {
656+
const postMothershipUpload = async (workspaceId: string | null = 'test-workspace-id') => {
657+
const formData = new FormData()
658+
const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' })
659+
formData.append('file', file)
660+
formData.append('context', 'mothership')
661+
if (workspaceId !== null) formData.append('workspaceId', workspaceId)
662+
663+
const req = new Request('http://localhost/api/files/upload', {
664+
method: 'POST',
665+
headers: { 'content-length': '1024' },
666+
body: formData,
667+
})
668+
669+
return POST(req as unknown as NextRequest)
670+
}
671+
672+
beforeEach(() => {
673+
setupFileApiMocks({
674+
cloudEnabled: false,
675+
storageProvider: 'local',
676+
})
677+
})
678+
679+
it('rejects mothership uploads without workspaceId', async () => {
680+
const response = await postMothershipUpload(null)
681+
682+
expect(response.status).toBe(400)
683+
const data = await response.json()
684+
expect(data.message).toContain('workspaceId')
685+
expect(storageServiceMockFns.mockUploadFile).not.toHaveBeenCalled()
686+
})
687+
688+
it('rejects mothership uploads for a workspace the caller does not belong to', async () => {
689+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue(null)
690+
691+
const response = await postMothershipUpload()
692+
693+
expect(response.status).toBe(403)
694+
const data = await response.json()
695+
expect(data.error).toBe('Write or Admin access required for mothership uploads')
696+
expect(storageServiceMockFns.mockUploadFile).not.toHaveBeenCalled()
697+
})
698+
699+
it('rejects mothership uploads for a read-only workspace member', async () => {
700+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('read')
701+
702+
const response = await postMothershipUpload()
703+
704+
expect(response.status).toBe(403)
705+
expect(storageServiceMockFns.mockUploadFile).not.toHaveBeenCalled()
706+
})
707+
708+
it('rejects mothership uploads over the caller storage quota', async () => {
709+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write')
710+
mocks.mockCheckStorageQuota.mockResolvedValue({
711+
allowed: false,
712+
currentUsage: 100,
713+
limit: 100,
714+
error: 'Storage limit exceeded. Used: 0.00GB, Limit: 0GB',
715+
})
716+
717+
const response = await postMothershipUpload()
718+
719+
expect(response.status).toBe(413)
720+
const data = await response.json()
721+
expect(data.error).toContain('Storage limit exceeded')
722+
expect(storageServiceMockFns.mockUploadFile).not.toHaveBeenCalled()
723+
})
724+
725+
it('allows mothership uploads for a write-permission workspace member', async () => {
726+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write')
727+
728+
const response = await postMothershipUpload()
729+
730+
expect(response.status).toBe(200)
731+
expect(permissionsMockFns.mockGetUserEntityPermissions).toHaveBeenCalledWith(
732+
'test-user-id',
733+
'workspace',
734+
'test-workspace-id'
735+
)
736+
expect(storageServiceMockFns.mockUploadFile).toHaveBeenCalled()
737+
})
738+
739+
it('allows mothership uploads for an admin-permission workspace member', async () => {
740+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('admin')
741+
742+
const response = await postMothershipUpload()
743+
744+
expect(response.status).toBe(200)
745+
expect(storageServiceMockFns.mockUploadFile).toHaveBeenCalled()
746+
})
747+
748+
it('checks quota once against the combined size of a multi-file batch', async () => {
749+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write')
750+
751+
const formData = new FormData()
752+
const fileA = new File(['a'.repeat(10)], 'a.pdf', { type: 'application/pdf' })
753+
const fileB = new File(['b'.repeat(20)], 'b.pdf', { type: 'application/pdf' })
754+
formData.append('file', fileA)
755+
formData.append('file', fileB)
756+
formData.append('context', 'mothership')
757+
formData.append('workspaceId', 'test-workspace-id')
758+
759+
const req = new Request('http://localhost/api/files/upload', {
760+
method: 'POST',
761+
headers: { 'content-length': '1024' },
762+
body: formData,
763+
})
764+
765+
const response = await POST(req as unknown as NextRequest)
766+
767+
expect(response.status).toBe(200)
768+
expect(mocks.mockCheckStorageQuota).toHaveBeenCalledTimes(1)
769+
expect(mocks.mockCheckStorageQuota).toHaveBeenCalledWith('test-user-id', 30)
770+
expect(permissionsMockFns.mockGetUserEntityPermissions).toHaveBeenCalledTimes(1)
771+
})
772+
})
773+
643774
describe('Authentication Requirements', () => {
644775
it('should reject uploads without authentication', async () => {
645776
authMockFns.mockGetSession.mockResolvedValue(null)

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

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,36 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
115115
executionUploadContext = { workspaceId, workflowId, executionId }
116116
}
117117

118+
// Mothership context requires the same workspace write/admin permission check, plus a
119+
// storage quota check. Resolve both once per request (not per file) since workspaceId is
120+
// invariant across all files in the upload and quota must account for the full batch size,
121+
// not just one file.
122+
let mothershipWorkspaceId: string | undefined
123+
if (context === 'mothership') {
124+
if (!workspaceId) {
125+
throw new InvalidRequestError('Mothership context requires workspaceId parameter')
126+
}
127+
128+
const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId)
129+
if (permission !== 'write' && permission !== 'admin') {
130+
return NextResponse.json(
131+
{ error: 'Write or Admin access required for mothership uploads' },
132+
{ status: 403 }
133+
)
134+
}
135+
136+
const { checkStorageQuota } = await import('@/lib/billing/storage')
137+
const quotaCheck = await checkStorageQuota(session.user.id, totalFileSize)
138+
if (!quotaCheck.allowed) {
139+
return NextResponse.json(
140+
{ error: quotaCheck.error || 'Storage limit exceeded' },
141+
{ status: 413 }
142+
)
143+
}
144+
145+
mothershipWorkspaceId = workspaceId
146+
}
147+
118148
const uploadResults = []
119149

120150
for (const file of files) {
@@ -261,21 +291,17 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
261291
}
262292

263293
// Handle mothership context (chat-scoped uploads to workspace S3)
264-
if (context === 'mothership') {
265-
if (!workspaceId) {
266-
throw new InvalidRequestError('Chat context requires workspaceId parameter')
267-
}
268-
294+
if (context === 'mothership' && mothershipWorkspaceId) {
269295
logger.info(`Uploading mothership file: ${originalName}`)
270296

271-
const storageKey = generateWorkspaceFileKey(workspaceId, originalName)
297+
const storageKey = generateWorkspaceFileKey(mothershipWorkspaceId, originalName)
272298

273299
const metadata: Record<string, string> = {
274300
originalName: originalName,
275301
uploadedAt: new Date().toISOString(),
276302
purpose: 'mothership',
277303
userId: session.user.id,
278-
workspaceId,
304+
workspaceId: mothershipWorkspaceId,
279305
}
280306

281307
const fileInfo = await storageService.uploadFile({

0 commit comments

Comments
 (0)