diff --git a/apps/sim/app/api/files/download/route.ts b/apps/sim/app/api/files/download/route.ts index 33f1ce61146..62bdbfe6e6a 100644 --- a/apps/sim/app/api/files/download/route.ts +++ b/apps/sim/app/api/files/download/route.ts @@ -4,8 +4,8 @@ import { fileDownloadContract } from '@/lib/api/contracts/storage-transfer' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import type { StorageContext } from '@/lib/uploads/config' import { hasCloudStorage } from '@/lib/uploads/core/storage-service' +import { inferContextFromKey } from '@/lib/uploads/utils/file-utils' import { verifyFileAccess } from '@/app/api/files/authorization' import { createErrorResponse, FileNotFoundError } from '@/app/api/files/utils' @@ -40,7 +40,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) if (!parsed.success) return parsed.response - const { key, name, isExecutionFile, context, url } = parsed.data.body + const { key, name, url } = parsed.data.body if (!key) { return createErrorResponse(new Error('File key is required'), 400) @@ -58,12 +58,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => { }) } - let storageContext: StorageContext | 'general' | undefined = context - - if (isExecutionFile && !context) { - storageContext = 'execution' - logger.info(`Using execution context for file: ${key}`) - } + // Derive context from the trusted key prefix, mirroring the serve route this URL + // delegates to, which re-derives context from the key and ignores any client-supplied value. + const storageContext = inferContextFromKey(key) const hasAccess = await verifyFileAccess( key, @@ -79,10 +76,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } const { getBaseUrl } = await import('@/lib/core/utils/urls') - const contextQuery = storageContext ? `?context=${storageContext}` : '' - const downloadUrl = `${getBaseUrl()}/api/files/serve/${encodeURIComponent(key)}${contextQuery}` + const downloadUrl = `${getBaseUrl()}/api/files/serve/${encodeURIComponent(key)}?context=${storageContext}` - logger.info(`Generated download URL for ${storageContext ?? 'inferred'} file: ${key}`) + logger.info(`Generated download URL for ${storageContext} file: ${key}`) return NextResponse.json({ downloadUrl, diff --git a/apps/sim/app/api/files/parse/route.ts b/apps/sim/app/api/files/parse/route.ts index b925a366033..033f180818a 100644 --- a/apps/sim/app/api/files/parse/route.ts +++ b/apps/sim/app/api/files/parse/route.ts @@ -13,7 +13,7 @@ import { checkInternalAuth } from '@/lib/auth/hybrid' import { sanitizeUrlForLog } from '@/lib/core/utils/logging' import { assertKnownSizeWithinLimit, isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { isSupportedFileType, parseFile } from '@/lib/file-parsers' -import { isUsingCloudStorage, type StorageContext, StorageService } from '@/lib/uploads' +import { isUsingCloudStorage, StorageService } from '@/lib/uploads' import { uploadExecutionFile } from '@/lib/uploads/contexts/execution' import { ExternalUrlValidationError, @@ -303,7 +303,6 @@ async function parseFileSingle( return handleCloudFile( filePath, fileType, - undefined, userId, executionContext, maxDownloadBytes, @@ -329,7 +328,6 @@ async function parseFileSingle( return handleCloudFile( filePath, fileType, - undefined, userId, executionContext, maxDownloadBytes, @@ -608,7 +606,6 @@ async function handleExternalUrl( async function handleCloudFile( filePath: string, fileType: string, - explicitContext: string | undefined, userId: string, executionContext?: ExecutionContext, maxDownloadBytes = MAX_DOWNLOAD_SIZE_BYTES, @@ -619,7 +616,7 @@ async function handleCloudFile( logger.info('Extracted cloud key:', cloudKey) - const context = (explicitContext as StorageContext) || inferContextFromKey(cloudKey) + const context = inferContextFromKey(cloudKey) const hasAccess = await verifyFileAccess( cloudKey, @@ -658,7 +655,7 @@ async function handleCloudFile( maxBytes: maxDownloadBytes, }) logger.info( - `Downloaded file from ${context} storage (${explicitContext ? 'explicit' : 'inferred'}): ${cloudKey}, size: ${fileBuffer.length} bytes` + `Downloaded file from ${context} storage: ${cloudKey}, size: ${fileBuffer.length} bytes` ) const filename = originalFilename || cloudKey.split('/').pop() || cloudKey diff --git a/apps/sim/lib/uploads/utils/file-utils.server.test.ts b/apps/sim/lib/uploads/utils/file-utils.server.test.ts new file mode 100644 index 00000000000..40e9ec97044 --- /dev/null +++ b/apps/sim/lib/uploads/utils/file-utils.server.test.ts @@ -0,0 +1,47 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockDownloadFile } = vi.hoisted(() => ({ + mockDownloadFile: vi.fn(), +})) + +vi.mock('@/lib/uploads/core/storage-service', () => ({ + downloadFile: mockDownloadFile, + hasCloudStorage: vi.fn(() => true), +})) + +vi.mock('@/app/api/files/authorization', () => ({ + verifyFileAccess: vi.fn(), +})) + +import { createLogger } from '@sim/logger' +import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server' +import type { UserFile } from '@/executor/types' + +describe('downloadFileFromStorage context derivation', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDownloadFile.mockResolvedValue(Buffer.from('bytes')) + }) + + it('downloads with the key-derived context, ignoring a caller-supplied public context', async () => { + const userFile: UserFile = { + id: 'f1', + name: 'report.pdf', + url: '', + size: 5, + type: 'application/pdf', + key: 'workspace/ws-1/1700000000000-abc1234-report.pdf', + context: 'og-images', + } + + await downloadFileFromStorage(userFile, 'req-1', createLogger('test')) + + expect(mockDownloadFile).toHaveBeenCalledTimes(1) + expect(mockDownloadFile).toHaveBeenCalledWith( + expect.objectContaining({ key: userFile.key, context: 'workspace' }) + ) + }) +}) diff --git a/apps/sim/lib/uploads/utils/file-utils.server.ts b/apps/sim/lib/uploads/utils/file-utils.server.ts index e495b94274a..1077512cc97 100644 --- a/apps/sim/lib/uploads/utils/file-utils.server.ts +++ b/apps/sim/lib/uploads/utils/file-utils.server.ts @@ -12,7 +12,6 @@ import { consumeOrCancelBody, readResponseToBufferWithLimit, } from '@/lib/core/utils/stream-limits' -import type { StorageContext } from '@/lib/uploads' import { StorageService } from '@/lib/uploads' import { isExecutionFile } from '@/lib/uploads/contexts/execution/utils' import { @@ -23,6 +22,7 @@ import { isInternalFileUrl, processSingleFileToUserFile, type RawFileInput, + resolveTrustedFileContext, } from '@/lib/uploads/utils/file-utils' import { verifyFileAccess } from '@/app/api/files/authorization' import type { UserFile } from '@/executor/types' @@ -90,7 +90,7 @@ export async function resolveFileInputToUrl( // Generate presigned URL if we have a key but no URL if (!fileUrl && userFile.key) { - const context = (userFile.context as StorageContext) || inferContextFromKey(userFile.key) + const context = resolveTrustedFileContext(userFile.key, userFile.context) const hasAccess = await verifyFileAccess(userFile.key, userId, undefined, context, false) if (!hasAccess) { @@ -281,10 +281,8 @@ export async function downloadFileFromStorage( ) buffer = await downloadExecutionFile(userFile, { maxBytes: options.maxBytes }) } else if (userFile.key) { - const context = (userFile.context as StorageContext) || inferContextFromKey(userFile.key) - logger.info( - `[${requestId}] Downloading from ${context} storage (${userFile.context ? 'explicit' : 'inferred'}): ${userFile.key}` - ) + const context = resolveTrustedFileContext(userFile.key, userFile.context) + logger.info(`[${requestId}] Downloading from ${context} storage: ${userFile.key}`) const { downloadFile } = await import('@/lib/uploads/core/storage-service') buffer = await downloadFile({ diff --git a/apps/sim/lib/uploads/utils/file-utils.test.ts b/apps/sim/lib/uploads/utils/file-utils.test.ts index d982f33e2a4..0e7581bb454 100644 --- a/apps/sim/lib/uploads/utils/file-utils.test.ts +++ b/apps/sim/lib/uploads/utils/file-utils.test.ts @@ -9,6 +9,7 @@ import { isInternalFileUrl, isNetworkError, processSingleFileToUserFile, + resolveTrustedFileContext, } from '@/lib/uploads/utils/file-utils' const logger = createLogger('FileUtilsTest') @@ -74,6 +75,26 @@ describe('inferContextFromKey', () => { }) }) +describe('resolveTrustedFileContext', () => { + it('derives from the key prefix and ignores a mismatched caller context', () => { + expect(resolveTrustedFileContext('workspace/ws/1700000000000-abc-x.pdf', 'og-images')).toBe( + 'workspace' + ) + expect(resolveTrustedFileContext('chat/x', 'workspace-logos')).toBe('chat') + expect(resolveTrustedFileContext('workspace/ws/x', 'mothership')).toBe('workspace') + }) + + it('honors the caller context for legacy keys with no inferrable prefix', () => { + expect(resolveTrustedFileContext('legacy/ws/wf/ex/report.pdf', 'execution')).toBe('execution') + }) + + it('never resolves an un-inferrable key to a world-readable context', () => { + expect(() => resolveTrustedFileContext('legacy/report.pdf', 'og-images')).toThrow() + expect(() => resolveTrustedFileContext('legacy/report.pdf', 'profile-pictures')).toThrow() + expect(() => resolveTrustedFileContext('legacy/report.pdf')).toThrow() + }) +}) + describe('isAbortError', () => { it('returns true for AbortError-named errors', () => { const err = new Error('aborted') diff --git a/apps/sim/lib/uploads/utils/file-utils.ts b/apps/sim/lib/uploads/utils/file-utils.ts index 0fd254f2e25..642e48aafd2 100644 --- a/apps/sim/lib/uploads/utils/file-utils.ts +++ b/apps/sim/lib/uploads/utils/file-utils.ts @@ -596,6 +596,43 @@ export function inferContextFromKey(key: string): StorageContext { ) } +/** + * World-readable storage contexts. Reads for these short-circuit file + * authorization and can resolve to the shared bucket, so a caller-supplied + * context must never select one for a key that does not carry the matching + * prefix. + */ +const PUBLIC_STORAGE_CONTEXTS = new Set([ + 'profile-pictures', + 'og-images', + 'workspace-logos', +]) + +/** + * Resolve the storage context for a stored file from its trusted key prefix. + * + * The storage key is written server-side at upload time and cannot be forged to + * change tenant, whereas a file's `context` field is attacker-authorable in a + * workflow. When the key carries a recognized prefix that prefix is + * authoritative and the caller-supplied `context` is ignored — this prevents a + * private `workspace/…` key from being relabeled with a world-readable context + * to bypass authorization and read the shared bucket. + * + * Legacy keys predating context-prefixed keys cannot be inferred; for those the + * persisted `context` is honored so existing files stay resolvable — except a + * world-readable context, which would reopen the bypass on an un-inferrable key. + */ +export function resolveTrustedFileContext(key: string, context?: string): StorageContext { + try { + return inferContextFromKey(key) + } catch (error) { + if (context && !PUBLIC_STORAGE_CONTEXTS.has(context as StorageContext)) { + return context as StorageContext + } + throw error + } +} + /** * Extract storage key and context from an internal file URL * @param fileUrl - Internal file URL (e.g., /api/files/serve/key?context=workspace) diff --git a/apps/sim/providers/file-attachments.server.ts b/apps/sim/providers/file-attachments.server.ts index 8f2e2dfac5e..4ca03ef07aa 100644 --- a/apps/sim/providers/file-attachments.server.ts +++ b/apps/sim/providers/file-attachments.server.ts @@ -2,9 +2,8 @@ import { FileState, GoogleGenAI } from '@google/genai' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { sleep } from '@sim/utils/helpers' -import type { StorageContext } from '@/lib/uploads' import { StorageService } from '@/lib/uploads' -import { inferContextFromKey } from '@/lib/uploads/utils/file-utils' +import { resolveTrustedFileContext } from '@/lib/uploads/utils/file-utils' import { downloadServableFileFromStorage } from '@/lib/uploads/utils/file-utils.server' import { verifyFileAccess } from '@/app/api/files/authorization' import type { UserFile } from '@/executor/types' @@ -85,7 +84,7 @@ export async function attachLargeFileRemoteUrls( ) } - const context = (file.context as StorageContext) || inferContextFromKey(file.key) + const context = resolveTrustedFileContext(file.key, file.context) const hasAccess = await verifyFileAccess(file.key, request.userId, undefined, context, false) if (!hasAccess) { throw new Error(`File "${file.name}" is not accessible for provider "${providerId}"`) @@ -147,7 +146,7 @@ async function assertFileAccessForUpload( if (!userId) { throw new Error(`File "${file.name}" requires an authenticated user to upload`) } - const context = (file.context as StorageContext) || inferContextFromKey(file.key) + const context = resolveTrustedFileContext(file.key, file.context) const hasAccess = await verifyFileAccess(file.key, userId, undefined, context, false) if (!hasAccess) { throw new Error(`File "${file.name}" is not accessible`)