Skip to content

Commit 88330b4

Browse files
authored
fix(uploads): gate execution-context uploads behind write/admin permission (#5404)
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.
1 parent afe3d32 commit 88330b4

2 files changed

Lines changed: 152 additions & 3 deletions

File tree

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

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const mocks = vi.hoisted(() => {
2323
const mockGetStorageProvider = vi.fn()
2424
const mockIsUsingCloudStorage = vi.fn()
2525
const mockUploadFile = vi.fn()
26+
const mockUploadExecutionFile = vi.fn()
2627

2728
return {
2829
mockVerifyFileAccess,
@@ -33,6 +34,7 @@ const mocks = vi.hoisted(() => {
3334
mockGetStorageProvider,
3435
mockIsUsingCloudStorage,
3536
mockUploadFile,
37+
mockUploadExecutionFile,
3638
}
3739
})
3840

@@ -82,6 +84,10 @@ vi.mock('@/lib/uploads/contexts/workspace', () => ({
8284
uploadWorkspaceFile: mocks.mockUploadWorkspaceFile,
8385
}))
8486

87+
vi.mock('@/lib/uploads/contexts/execution', () => ({
88+
uploadExecutionFile: mocks.mockUploadExecutionFile,
89+
}))
90+
8591
vi.mock('@/lib/uploads', () => ({
8692
getStorageProvider: mocks.mockGetStorageProvider,
8793
isUsingCloudStorage: mocks.mockIsUsingCloudStorage,
@@ -151,6 +157,17 @@ function setupFileApiMocks(
151157
expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(),
152158
})
153159

160+
mocks.mockUploadExecutionFile.mockResolvedValue({
161+
id: 'test-execution-file-id',
162+
name: 'test.txt',
163+
url: '/api/files/serve/execution/test-workspace-id/test-file.txt',
164+
size: 100,
165+
type: 'text/plain',
166+
key: 'execution/test-workspace-id/1234567890-test.txt',
167+
uploadedAt: new Date().toISOString(),
168+
expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(),
169+
})
170+
154171
mocks.mockGetStorageProvider.mockReturnValue(storageProvider)
155172
mocks.mockIsUsingCloudStorage.mockReturnValue(cloudEnabled)
156173
mocks.mockUploadFile.mockResolvedValue({
@@ -531,6 +548,130 @@ describe('File Upload Security Tests', () => {
531548
})
532549
})
533550

551+
describe('Execution Context Permission Gate', () => {
552+
const createExecutionFormData = (
553+
file: File,
554+
workspaceId: string | null = 'test-workspace-id'
555+
) => {
556+
const formData = new FormData()
557+
formData.append('file', file)
558+
formData.append('context', 'execution')
559+
formData.append('workflowId', 'test-workflow-id')
560+
formData.append('executionId', 'test-execution-id')
561+
if (workspaceId !== null) formData.append('workspaceId', workspaceId)
562+
return formData
563+
}
564+
565+
beforeEach(() => {
566+
setupFileApiMocks({
567+
cloudEnabled: false,
568+
storageProvider: 'local',
569+
})
570+
})
571+
572+
it('rejects execution uploads without workspaceId', async () => {
573+
const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' })
574+
const formData = createExecutionFormData(file, null)
575+
576+
const req = new Request('http://localhost/api/files/upload', {
577+
method: 'POST',
578+
headers: { 'content-length': '1024' },
579+
body: formData,
580+
})
581+
582+
const response = await POST(req as unknown as NextRequest)
583+
584+
expect(response.status).toBe(400)
585+
const data = await response.json()
586+
expect(data.message).toContain('workflowId, executionId, and workspaceId')
587+
expect(mocks.mockUploadExecutionFile).not.toHaveBeenCalled()
588+
})
589+
590+
it('rejects execution uploads for a read-only workspace member', async () => {
591+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('read')
592+
593+
const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' })
594+
const formData = createExecutionFormData(file)
595+
596+
const req = new Request('http://localhost/api/files/upload', {
597+
method: 'POST',
598+
headers: { 'content-length': '1024' },
599+
body: formData,
600+
})
601+
602+
const response = await POST(req as unknown as NextRequest)
603+
604+
expect(response.status).toBe(403)
605+
const data = await response.json()
606+
expect(data.error).toBe('Write or Admin access required for execution uploads')
607+
expect(mocks.mockUploadExecutionFile).not.toHaveBeenCalled()
608+
})
609+
610+
it('rejects execution uploads for a member with no workspace permission', async () => {
611+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue(null)
612+
613+
const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' })
614+
const formData = createExecutionFormData(file)
615+
616+
const req = new Request('http://localhost/api/files/upload', {
617+
method: 'POST',
618+
headers: { 'content-length': '1024' },
619+
body: formData,
620+
})
621+
622+
const response = await POST(req as unknown as NextRequest)
623+
624+
expect(response.status).toBe(403)
625+
expect(mocks.mockUploadExecutionFile).not.toHaveBeenCalled()
626+
})
627+
628+
it('allows execution uploads for a write-permission workspace member', async () => {
629+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write')
630+
631+
const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' })
632+
const formData = createExecutionFormData(file)
633+
634+
const req = new Request('http://localhost/api/files/upload', {
635+
method: 'POST',
636+
headers: { 'content-length': '1024' },
637+
body: formData,
638+
})
639+
640+
const response = await POST(req as unknown as NextRequest)
641+
642+
expect(response.status).toBe(200)
643+
expect(mocks.mockUploadExecutionFile).toHaveBeenCalledWith(
644+
{
645+
workspaceId: 'test-workspace-id',
646+
workflowId: 'test-workflow-id',
647+
executionId: 'test-execution-id',
648+
},
649+
expect.anything(),
650+
'test.pdf',
651+
'application/pdf',
652+
'test-user-id'
653+
)
654+
})
655+
656+
it('allows execution uploads for an admin-permission workspace member', async () => {
657+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('admin')
658+
659+
const file = new File(['test content'], 'test.pdf', { type: 'application/pdf' })
660+
const formData = createExecutionFormData(file)
661+
662+
const req = new Request('http://localhost/api/files/upload', {
663+
method: 'POST',
664+
headers: { 'content-length': '1024' },
665+
body: formData,
666+
})
667+
668+
const response = await POST(req as unknown as NextRequest)
669+
670+
expect(response.status).toBe(200)
671+
expect(mocks.mockUploadExecutionFile).toHaveBeenCalled()
672+
})
673+
})
674+
534675
describe('Authentication Requirements', () => {
535676
it('should reject uploads without authentication', async () => {
536677
authMockFns.mockGetSession.mockResolvedValue(null)

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,16 +111,24 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
111111

112112
// Handle execution context
113113
if (context === 'execution') {
114-
if (!workflowId || !executionId) {
114+
if (!workflowId || !executionId || !workspaceId) {
115115
throw new InvalidRequestError(
116-
'Execution context requires workflowId and executionId parameters'
116+
'Execution context requires workflowId, executionId, and workspaceId parameters'
117+
)
118+
}
119+
120+
const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId)
121+
if (permission !== 'write' && permission !== 'admin') {
122+
return NextResponse.json(
123+
{ error: 'Write or Admin access required for execution uploads' },
124+
{ status: 403 }
117125
)
118126
}
119127

120128
const { uploadExecutionFile } = await import('@/lib/uploads/contexts/execution')
121129
const userFile = await uploadExecutionFile(
122130
{
123-
workspaceId: workspaceId || '',
131+
workspaceId,
124132
workflowId,
125133
executionId,
126134
},

0 commit comments

Comments
 (0)