mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
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.
This commit is contained in:
@@ -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,125 @@ 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()
|
||||
})
|
||||
|
||||
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', () => {
|
||||
it('should reject uploads without authentication', async () => {
|
||||
authMockFns.mockGetSession.mockResolvedValue(null)
|
||||
|
||||
@@ -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,21 +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')
|
||||
}
|
||||
|
||||
if (context === 'mothership' && mothershipWorkspaceId) {
|
||||
logger.info(`Uploading mothership file: ${originalName}`)
|
||||
|
||||
const storageKey = generateWorkspaceFileKey(workspaceId, originalName)
|
||||
const storageKey = generateWorkspaceFileKey(mothershipWorkspaceId, originalName)
|
||||
|
||||
const metadata: Record<string, string> = {
|
||||
originalName: originalName,
|
||||
uploadedAt: new Date().toISOString(),
|
||||
purpose: 'mothership',
|
||||
userId: session.user.id,
|
||||
workspaceId,
|
||||
workspaceId: mothershipWorkspaceId,
|
||||
}
|
||||
|
||||
const fileInfo = await storageService.uploadFile({
|
||||
|
||||
Reference in New Issue
Block a user