address comments

Author: icecrasher321

apps/sim/lib/uploads/utils/file-utils.server.ts

@@ -22,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'
@@ -89,7 +90,7 @@ export async function resolveFileInputToUrl(
 
     // Generate presigned URL if we have a key but no URL
     if (!fileUrl && userFile.key) {
-      const context = inferContextFromKey(userFile.key)
+      const context = resolveTrustedFileContext(userFile.key, userFile.context)
       const hasAccess = await verifyFileAccess(userFile.key, userId, undefined, context, false)
 
       if (!hasAccess) {
@@ -280,7 +281,7 @@ export async function downloadFileFromStorage(
     )
     buffer = await downloadExecutionFile(userFile, { maxBytes: options.maxBytes })
   } else if (userFile.key) {
-    const context = inferContextFromKey(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')

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')

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<StorageContext>([
+  '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)

apps/sim/providers/file-attachments.server.ts

@@ -3,7 +3,7 @@ import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
 import { sleep } from '@sim/utils/helpers'
 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'
@@ -84,7 +84,7 @@ export async function attachLargeFileRemoteUrls(
       )
     }
 
-    const context = 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}"`)
@@ -146,7 +146,7 @@ async function assertFileAccessForUpload(
   if (!userId) {
     throw new Error(`File "${file.name}" requires an authenticated user to upload`)
   }
-  const context = 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`)