fix(files): require chat ownership for output file access; duplicate error toast

Author: j15z

apps/sim/app/api/files/authorization.test.ts

@@ -210,3 +210,69 @@ describe('public-context access (profile-pictures / og-images / workspace-logos)
     expect(mockGetUserEntityPermissions).not.toHaveBeenCalled()
   })
 })
+
+/**
+ * Chat-scoped `output` files belong to a private chat: workspace membership
+ * alone must NOT grant access — the caller must also be the row's owner.
+ * These lock in the fix for the member-reads-another-member's-outputs leak,
+ * on both the explicit-context path (view route) and the inferred-workspace
+ * path (serve route — output keys share the workspace key shape).
+ */
+describe('chat output file ownership (verifyFileAccess)', () => {
+  const OUTPUT_KEY = 'workspace/ws-1/1780000000-abcd-generated-image.png'
+  const outputRow = {
+    workspaceId: 'ws-1',
+    userId: 'owner-user',
+    context: 'output',
+    deletedAt: null,
+  }
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('denies a workspace member who is not the owning chat user (explicit output context)', async () => {
+    mockGetFileMetadataByKey.mockResolvedValue(outputRow)
+    mockGetUserEntityPermissions.mockResolvedValue('read')
+
+    await expect(verifyFileAccess(OUTPUT_KEY, 'other-member', undefined, 'output')).resolves.toBe(
+      false
+    )
+  })
+
+  it('denies a non-owner member when context is inferred as workspace (serve path)', async () => {
+    mockGetFileMetadataByKey.mockResolvedValue(outputRow)
+    mockGetUserEntityPermissions.mockResolvedValue('read')
+
+    await expect(
+      verifyFileAccess(OUTPUT_KEY, 'other-member', undefined, 'workspace')
+    ).resolves.toBe(false)
+  })
+
+  it('grants the owning chat user with workspace membership', async () => {
+    mockGetFileMetadataByKey.mockResolvedValue(outputRow)
+    mockGetUserEntityPermissions.mockResolvedValue('read')
+
+    await expect(verifyFileAccess(OUTPUT_KEY, 'owner-user', undefined, 'output')).resolves.toBe(
+      true
+    )
+  })
+
+  it('still denies the owner without workspace membership (left the workspace)', async () => {
+    mockGetFileMetadataByKey.mockResolvedValue(outputRow)
+    mockGetUserEntityPermissions.mockResolvedValue(null)
+
+    await expect(verifyFileAccess(OUTPUT_KEY, 'owner-user', undefined, 'output')).resolves.toBe(
+      false
+    )
+  })
+
+  it('leaves plain workspace files on membership-only auth', async () => {
+    mockGetFileMetadataByKey.mockResolvedValue({ ...outputRow, context: 'workspace' })
+    mockGetUserEntityPermissions.mockResolvedValue('read')
+
+    await expect(
+      verifyFileAccess(OUTPUT_KEY, 'other-member', undefined, 'workspace')
+    ).resolves.toBe(true)
+  })
+})

apps/sim/app/api/files/authorization.ts

@@ -60,6 +60,21 @@ function workspacePermissionSatisfies(
   return permissionSatisfies(permission, requireWrite ? 'write' : 'read')
 }
 
+/**
+ * Chat-scoped `output` rows belong to a PRIVATE chat: workspace membership
+ * alone is not enough — the caller must also be the row's owner (stamped from
+ * the chat owner at creation and re-stamped on fork/duplicate). Without this,
+ * any workspace member who learns a file id or storage key could read another
+ * member's agent outputs, even though listing outputs requires chat ownership.
+ * Non-output contexts pass through unchanged.
+ */
+function outputOwnershipSatisfied(
+  record: { context: string; uploadedBy: string },
+  userId: string
+): boolean {
+  return record.context !== 'output' || record.uploadedBy === userId
+}
+
 /**
  * Lookup workspace file by storage key from database
  * @param key Storage key to lookup
@@ -68,15 +83,17 @@ function workspacePermissionSatisfies(
 async function lookupWorkspaceFileByKey(
   key: string,
   options?: { includeDeleted?: boolean }
-): Promise<{ workspaceId: string; uploadedBy: string } | null> {
+): Promise<{ workspaceId: string; uploadedBy: string; context: string } | null> {
   try {
     const { includeDeleted = false } = options ?? {}
     // Priority 1: Check new workspaceFiles table. Look up by key across
     // WORKSPACE_FILE_LOOKUP_CONTEXTS (`workspace` + `output`): both share the
-    // `workspace/<id>/...` key shape and authorize by workspace membership. Filtering
-    // to `workspace` alone made `output` files unservable (broken previews); scoping to
-    // this explicit set (rather than dropping the filter) keeps outputs servable while
-    // leaving upload (`mothership`) authorization and the owner-scoped contexts untouched.
+    // `workspace/<id>/...` key shape. `workspace` rows authorize by workspace
+    // membership; `output` rows additionally require ownership (see
+    // outputOwnershipSatisfied). Filtering to `workspace` alone made `output`
+    // files unservable (broken previews); scoping to this explicit set (rather
+    // than dropping the filter) keeps outputs servable while leaving upload
+    // (`mothership`) authorization and the owner-scoped contexts untouched.
     const fileRecord = await getFileMetadataByKey(key, WORKSPACE_FILE_LOOKUP_CONTEXTS, {
       includeDeleted,
     })
@@ -85,6 +102,7 @@ async function lookupWorkspaceFileByKey(
       return {
         workspaceId: fileRecord.workspaceId || '',
         uploadedBy: fileRecord.userId,
+        context: fileRecord.context,
       }
     }
 
@@ -107,6 +125,7 @@ async function lookupWorkspaceFileByKey(
         return {
           workspaceId: legacyFile.workspaceId,
           uploadedBy: legacyFile.uploadedBy,
+          context: 'workspace',
         }
       }
     } catch (legacyError) {
@@ -181,8 +200,15 @@ export async function verifyFileAccess(
       return true
     }
 
-    // 1. Workspace / mothership files: Check database first (most reliable for both local and cloud)
-    if (inferredContext === 'workspace' || inferredContext === 'mothership') {
+    // 1. Workspace / mothership / chat-output files: check database first (most
+    // reliable for both local and cloud). `output` shares the workspace key
+    // shape; explicitly routing it here (instead of letting it fall through to
+    // verifyRegularFileAccess) keeps its ownership rule applied on every path.
+    if (
+      inferredContext === 'workspace' ||
+      inferredContext === 'mothership' ||
+      inferredContext === 'output'
+    ) {
       return await verifyWorkspaceFileAccess(cloudKey, userId, customConfig, isLocal, requireWrite)
     }
 
@@ -242,6 +268,14 @@ async function verifyWorkspaceFileAccess(
     // Priority 1: Check database (most reliable, works for both local and cloud)
     const workspaceFileRecord = await lookupWorkspaceFileByKey(cloudKey)
     if (workspaceFileRecord) {
+      if (!outputOwnershipSatisfied(workspaceFileRecord, userId)) {
+        logger.warn('Chat output file access denied: caller is not the owning chat user', {
+          userId,
+          workspaceId: workspaceFileRecord.workspaceId,
+          cloudKey,
+        })
+        return false
+      }
       const permission = await getUserEntityPermissions(
         userId,
         'workspace',
@@ -671,6 +705,14 @@ async function verifyRegularFileAccess(
     // This handles legacy files that might not have metadata
     const workspaceFileRecord = await lookupWorkspaceFileByKey(cloudKey)
     if (workspaceFileRecord) {
+      if (!outputOwnershipSatisfied(workspaceFileRecord, userId)) {
+        logger.warn('Chat output file access denied: caller is not the owning chat user', {
+          userId,
+          workspaceId: workspaceFileRecord.workspaceId,
+          cloudKey,
+        })
+        return false
+      }
       const permission = await getUserEntityPermissions(
         userId,
         'workspace',

apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts

@@ -51,7 +51,7 @@ export const GET = withRouteHandler(
         return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 })
       }
 
-      const file = await getPreviewableWorkspaceFile(workspaceId, fileId)
+      const file = await getPreviewableWorkspaceFile(workspaceId, fileId, session.user.id)
       if (!file) {
         return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
       }

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx

@@ -36,6 +36,7 @@ import {
   Workflow,
 } from '@sim/emcn/icons'
 import { createLogger } from '@sim/logger'
+import { getErrorMessage } from '@sim/utils/errors'
 import { MoreHorizontal, Pin } from 'lucide-react'
 import Link from 'next/link'
 import { useParams, usePathname, useRouter } from 'next/navigation'
@@ -982,6 +983,9 @@ export const Sidebar = memo(function Sidebar({ isCollapsed }: SidebarProps) {
           useFolderStore.getState().clearChatSelection()
           navigateToPage(`/workspace/${workspaceId}/chat/${result.id}`)
         },
+        onError: (error) => {
+          toast.error(getErrorMessage(error, 'Failed to duplicate chat'))
+        },
       }
     )
   }, [navigateToPage, workspaceId])

apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts

@@ -989,16 +989,19 @@ export async function getWorkspaceFile(
  * Fetch a single file record by id for PREVIEW, including the chat-scoped `output`
  * context (agent-generated outputs) that never appears in the workspace Files list.
  * Returns the same shape as {@link listWorkspaceFiles} so the resource panel can
- * render an output that {@link getWorkspaceFile}/list would miss. Authorization
- * (workspace membership) is the caller's responsibility.
+ * render an output that {@link getWorkspaceFile}/list would miss. Workspace
+ * membership is the caller's responsibility; chat-output OWNERSHIP is enforced
+ * here — `output` rows belong to a private chat, so only the owning chat's user
+ * may resolve them (non-owners get null, indistinguishable from a missing id).
  *
  * `mothership` chat uploads are intentionally not included here — surfacing uploads
  * through this preview path is out of scope for the outputs feature (see
  * outputs-vfs-followups.md #2/#7) and can be added later if wanted.
  */
 export async function getPreviewableWorkspaceFile(
   workspaceId: string,
-  fileId: string
+  fileId: string,
+  requestingUserId: string
 ): Promise<WorkspaceFileRecord | null> {
   try {
     const [file] = await db
@@ -1015,6 +1018,14 @@ export async function getPreviewableWorkspaceFile(
       .limit(1)
 
     if (!file) return null
+    if (file.context === 'output' && file.userId !== requestingUserId) {
+      logger.warn('Chat output preview denied: caller is not the owning chat user', {
+        fileId,
+        workspaceId,
+        requestingUserId,
+      })
+      return null
+    }
     return mapSingleWorkspaceFileRecord(file, workspaceId)
   } catch (error) {
     logger.error(`Failed to get previewable workspace file ${fileId}:`, error)