Skip to content

Commit 999b80d

Browse files
waleedlatif1waleedlatif1
committed
fix(files): authorize presigned upload contexts per type
The batch presign endpoint validated its type param against the shared seven-value upload enum while only authorizing knowledge-base, so any authenticated user could mint an S3 presigned PUT into workspace-logos, profile-pictures, execution, mothership, chat and copilot prefixes — objects that are then served unauthenticated from the app origin with a one-year public cache. The single presign endpoint had the same gap for chat, which has no authorization predicate and no client. - Batch presign now accepts only knowledge-base, and always requires a workspaceId the caller has write/admin on before anything is minted - Single presign drops chat from its accepted contexts; every remaining context has a per-context predicate - Both allowlists live in the contract module so the enforced enum and the documented one cannot drift apart again
1 parent 5686b7b commit 999b80d

5 files changed

Lines changed: 395 additions & 160 deletions

File tree

‎apps/sim/app/api/files/presigned/batch/route.test.ts‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
/**
2+
* Tests for the batch presigned upload API route
3+
*
4+
* @vitest-environment node
5+
*/
6+
7+
import { authMockFns, storageServiceMock, storageServiceMockFns } from '@sim/testing'
8+
import { NextRequest } from 'next/server'
9+
import { beforeEach, describe, expect, it, vi } from 'vitest'
10+
11+
const {
12+
mockValidateFileType,
13+
mockGetUserEntityPermissions,
14+
mockRecordKnowledgeBaseFileOwnershipMany,
15+
} = vi.hoisted(() => ({
16+
mockValidateFileType: vi.fn().mockReturnValue(null),
17+
mockGetUserEntityPermissions: vi.fn().mockResolvedValue('write'),
18+
mockRecordKnowledgeBaseFileOwnershipMany: vi.fn().mockResolvedValue(undefined),
19+
}))
20+
21+
vi.mock('@/lib/uploads/config', () => ({
22+
getServeStoragePrefix: () => 's3',
23+
}))
24+
25+
vi.mock('@/lib/uploads/core/storage-service', () => storageServiceMock)
26+
27+
vi.mock('@/lib/uploads/utils/validation', () => ({
28+
validateFileType: mockValidateFileType,
29+
SUPPORTED_ARCHIVE_EXTENSIONS: ['zip'] as const,
30+
}))
31+
32+
vi.mock('@/lib/workspaces/permissions/utils', () => ({
33+
getUserEntityPermissions: mockGetUserEntityPermissions,
34+
}))
35+
36+
vi.mock('@/lib/uploads/server/metadata', () => ({
37+
recordKnowledgeBaseFileOwnershipMany: mockRecordKnowledgeBaseFileOwnershipMany,
38+
}))
39+
40+
import { POST } from '@/app/api/files/presigned/batch/route'
41+
42+
const KB_QUERY = 'type=knowledge-base&workspaceId=ws-1'
43+
44+
const buildRequest = (query: string, files?: unknown) =>
45+
new NextRequest(`http://localhost:3000/api/files/presigned/batch?${query}`, {
46+
method: 'POST',
47+
body: JSON.stringify({
48+
files: files ?? [{ fileName: 'doc.pdf', contentType: 'application/pdf', fileSize: 1024 }],
49+
}),
50+
})
51+
52+
describe('/api/files/presigned/batch', () => {
53+
beforeEach(() => {
54+
vi.clearAllMocks()
55+
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-1' } })
56+
mockValidateFileType.mockReturnValue(null)
57+
mockGetUserEntityPermissions.mockResolvedValue('write')
58+
mockRecordKnowledgeBaseFileOwnershipMany.mockResolvedValue(undefined)
59+
storageServiceMockFns.mockHasCloudStorage.mockReturnValue(true)
60+
storageServiceMockFns.mockGenerateBatchPresignedUploadUrls.mockImplementation(
61+
async (files: Array<{ fileName: string }>, context: string) =>
62+
files.map((file) => ({
63+
url: `https://example.com/${context}/${file.fileName}`,
64+
key: `${context}/${file.fileName}`,
65+
}))
66+
)
67+
})
68+
69+
it('returns 401 when the caller has no session', async () => {
70+
authMockFns.mockGetSession.mockResolvedValue(null)
71+
72+
const response = await POST(buildRequest(KB_QUERY))
73+
74+
expect(response.status).toBe(401)
75+
expect(storageServiceMockFns.mockGenerateBatchPresignedUploadUrls).not.toHaveBeenCalled()
76+
})
77+
78+
it.each([
79+
'workspace-logos',
80+
'profile-pictures',
81+
'execution',
82+
'mothership',
83+
'chat',
84+
'copilot',
85+
'workspace',
86+
])('refuses to presign the %s context', async (type) => {
87+
const response = await POST(buildRequest(`type=${type}&workspaceId=ws-1`))
88+
const data = await response.json()
89+
90+
expect(response.status).toBe(400)
91+
expect(data.error).toContain('Invalid type parameter')
92+
expect(storageServiceMockFns.mockGenerateBatchPresignedUploadUrls).not.toHaveBeenCalled()
93+
})
94+
95+
it('returns 400 when type is missing', async () => {
96+
const response = await POST(buildRequest('workspaceId=ws-1'))
97+
98+
expect(response.status).toBe(400)
99+
expect(storageServiceMockFns.mockGenerateBatchPresignedUploadUrls).not.toHaveBeenCalled()
100+
})
101+
102+
it('returns 400 when workspaceId is missing', async () => {
103+
const response = await POST(buildRequest('type=knowledge-base'))
104+
const data = await response.json()
105+
106+
expect(response.status).toBe(400)
107+
expect(data.error).toContain('workspaceId')
108+
expect(mockGetUserEntityPermissions).not.toHaveBeenCalled()
109+
expect(storageServiceMockFns.mockGenerateBatchPresignedUploadUrls).not.toHaveBeenCalled()
110+
})
111+
112+
it.each([['read'], [null]])(
113+
'returns 403 when the caller has %s access to the workspace',
114+
async (permission) => {
115+
mockGetUserEntityPermissions.mockResolvedValue(permission)
116+
117+
const response = await POST(buildRequest(KB_QUERY))
118+
119+
expect(response.status).toBe(403)
120+
expect(storageServiceMockFns.mockGenerateBatchPresignedUploadUrls).not.toHaveBeenCalled()
121+
}
122+
)
123+
124+
it('authorizes the workspace before returning the local-storage fallback', async () => {
125+
storageServiceMockFns.mockHasCloudStorage.mockReturnValue(false)
126+
mockGetUserEntityPermissions.mockResolvedValue('read')
127+
128+
const response = await POST(buildRequest(KB_QUERY))
129+
130+
expect(response.status).toBe(403)
131+
})
132+
133+
it('rejects unsupported file types before minting any URL', async () => {
134+
mockValidateFileType.mockReturnValue({
135+
code: 'UNSUPPORTED_FILE_TYPE',
136+
message: 'Unsupported file type: html.',
137+
supportedTypes: ['pdf'],
138+
})
139+
140+
const response = await POST(
141+
buildRequest(KB_QUERY, [{ fileName: 'poc.html', contentType: 'text/html', fileSize: 41 }])
142+
)
143+
const data = await response.json()
144+
145+
expect(response.status).toBe(400)
146+
expect(data.code).toBe('UNSUPPORTED_FILE_TYPE')
147+
expect(storageServiceMockFns.mockGenerateBatchPresignedUploadUrls).not.toHaveBeenCalled()
148+
})
149+
150+
it('mints knowledge-base URLs and records workspace ownership for a permitted caller', async () => {
151+
const response = await POST(buildRequest(KB_QUERY))
152+
const data = await response.json()
153+
154+
expect(response.status).toBe(200)
155+
expect(mockGetUserEntityPermissions).toHaveBeenCalledWith('user-1', 'workspace', 'ws-1')
156+
expect(storageServiceMockFns.mockGenerateBatchPresignedUploadUrls).toHaveBeenCalledWith(
157+
[{ fileName: 'doc.pdf', contentType: 'application/pdf', fileSize: 1024 }],
158+
'knowledge-base',
159+
'user-1',
160+
3600
161+
)
162+
expect(data.files).toHaveLength(1)
163+
expect(data.files[0].fileInfo.key).toBe('knowledge-base/doc.pdf')
164+
expect(data.files[0].fileInfo.path).toContain('?context=knowledge-base')
165+
expect(data.directUploadSupported).toBe(true)
166+
expect(mockRecordKnowledgeBaseFileOwnershipMany).toHaveBeenCalledWith([
167+
{
168+
key: 'knowledge-base/doc.pdf',
169+
userId: 'user-1',
170+
workspaceId: 'ws-1',
171+
originalName: 'doc.pdf',
172+
contentType: 'application/pdf',
173+
size: 1024,
174+
},
175+
])
176+
})
177+
178+
it('returns the fallback response when cloud storage is not configured', async () => {
179+
storageServiceMockFns.mockHasCloudStorage.mockReturnValue(false)
180+
181+
const response = await POST(buildRequest(KB_QUERY))
182+
const data = await response.json()
183+
184+
expect(response.status).toBe(200)
185+
expect(data.directUploadSupported).toBe(false)
186+
expect(data.files[0].presignedUrl).toBe('')
187+
expect(storageServiceMockFns.mockGenerateBatchPresignedUploadUrls).not.toHaveBeenCalled()
188+
})
189+
})

‎apps/sim/app/api/files/presigned/batch/route.ts‎

Lines changed: 42 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ import { createLogger } from '@sim/logger'
22
import { type NextRequest, NextResponse } from 'next/server'
33
import {
44
batchPresignedUploadBodyContract,
5-
uploadTypeSchema,
5+
batchPresignedUploadTypeSchema,
6+
batchPresignedUploadTypes,
67
} from '@/lib/api/contracts/storage-transfer'
78
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
89
import { getSession } from '@/lib/auth'
910
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
10-
import type { StorageContext } from '@/lib/uploads/config'
1111
import { getServeStoragePrefix } from '@/lib/uploads/config'
1212
import {
1313
generateBatchPresignedUploadUrls,
@@ -20,8 +20,13 @@ import { createErrorResponse } from '@/app/api/files/utils'
2020

2121
const logger = createLogger('BatchPresignedUploadAPI')
2222

23-
const VALID_UPLOAD_TYPES = ['knowledge-base', 'chat', 'copilot', 'profile-pictures'] as const
24-
23+
/**
24+
* Mints presigned upload URLs for knowledge-base ingest, the only context this
25+
* endpoint can authorize. Every request must name a workspace the caller has
26+
* write access to; other storage contexts are rejected rather than presigned,
27+
* because a presigned PUT is a write grant into a bucket served from a trusted
28+
* origin.
29+
*/
2530
export const POST = withRouteHandler(async (request: NextRequest) => {
2631
try {
2732
const session = await getSession()
@@ -52,59 +57,46 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
5257
return NextResponse.json({ error: 'type query parameter is required' }, { status: 400 })
5358
}
5459

55-
const uploadTypeResult = uploadTypeSchema.safeParse(uploadTypeParam)
60+
const uploadTypeResult = batchPresignedUploadTypeSchema.safeParse(uploadTypeParam)
5661
if (!uploadTypeResult.success) {
5762
return NextResponse.json(
58-
{ error: `Invalid type parameter. Must be one of: ${VALID_UPLOAD_TYPES.join(', ')}` },
63+
{
64+
error: `Invalid type parameter. Must be one of: ${batchPresignedUploadTypes.join(', ')}`,
65+
},
5966
{ status: 400 }
6067
)
6168
}
6269

63-
const uploadType = uploadTypeResult.data as StorageContext
64-
70+
const uploadType = uploadTypeResult.data
6571
const sessionUserId = session.user.id
6672

67-
let knowledgeBaseWorkspaceId: string | null = null
68-
if (uploadType === 'knowledge-base') {
69-
for (const file of files) {
70-
const fileValidationError = validateFileType(file.fileName, file.contentType)
71-
if (fileValidationError) {
72-
return NextResponse.json(
73-
{
74-
error: fileValidationError.message,
75-
code: fileValidationError.code,
76-
supportedTypes: fileValidationError.supportedTypes,
77-
},
78-
{ status: 400 }
79-
)
80-
}
81-
}
82-
83-
knowledgeBaseWorkspaceId = request.nextUrl.searchParams.get('workspaceId')
84-
if (!knowledgeBaseWorkspaceId?.trim()) {
73+
for (const file of files) {
74+
const fileValidationError = validateFileType(file.fileName, file.contentType)
75+
if (fileValidationError) {
8576
return NextResponse.json(
86-
{ error: 'workspaceId query parameter is required for knowledge-base uploads' },
77+
{
78+
error: fileValidationError.message,
79+
code: fileValidationError.code,
80+
supportedTypes: fileValidationError.supportedTypes,
81+
},
8782
{ status: 400 }
8883
)
8984
}
85+
}
9086

91-
const permission = await getUserEntityPermissions(
92-
sessionUserId,
93-
'workspace',
94-
knowledgeBaseWorkspaceId
87+
const workspaceId = request.nextUrl.searchParams.get('workspaceId')
88+
if (!workspaceId?.trim()) {
89+
return NextResponse.json(
90+
{ error: 'workspaceId query parameter is required for knowledge-base uploads' },
91+
{ status: 400 }
9592
)
96-
if (permission !== 'write' && permission !== 'admin') {
97-
return NextResponse.json(
98-
{ error: 'Write or Admin access required for knowledge-base uploads' },
99-
{ status: 403 }
100-
)
101-
}
10293
}
10394

104-
if (uploadType === 'copilot' && !sessionUserId?.trim()) {
95+
const permission = await getUserEntityPermissions(sessionUserId, 'workspace', workspaceId)
96+
if (permission !== 'write' && permission !== 'admin') {
10597
return NextResponse.json(
106-
{ error: 'Authenticated user session is required for copilot uploads' },
107-
{ status: 400 }
98+
{ error: 'Write or Admin access required for knowledge-base uploads' },
99+
{ status: 403 }
108100
)
109101
}
110102

@@ -149,19 +141,16 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
149141
`Generated ${files.length} presigned URLs in ${duration}ms (avg ${Math.round(duration / files.length)}ms per file)`
150142
)
151143

152-
if (uploadType === 'knowledge-base' && knowledgeBaseWorkspaceId) {
153-
const ownerWorkspaceId = knowledgeBaseWorkspaceId
154-
await recordKnowledgeBaseFileOwnershipMany(
155-
presignedUrls.map((urlResponse, index) => ({
156-
key: urlResponse.key,
157-
userId: sessionUserId,
158-
workspaceId: ownerWorkspaceId,
159-
originalName: files[index].fileName,
160-
contentType: files[index].contentType,
161-
size: files[index].fileSize,
162-
}))
163-
)
164-
}
144+
await recordKnowledgeBaseFileOwnershipMany(
145+
presignedUrls.map((urlResponse, index) => ({
146+
key: urlResponse.key,
147+
userId: sessionUserId,
148+
workspaceId,
149+
originalName: files[index].fileName,
150+
contentType: files[index].contentType,
151+
size: files[index].fileSize,
152+
}))
153+
)
165154

166155
const storagePrefix = getServeStoragePrefix()
167156

0 commit comments

Comments
 (0)