fix(security): bind copilot chat attachment keys to their owner (#6179)

* fix(security): bind copilot chat attachment keys to their owner

`POST /api/copilot/chat` accepted a client-supplied `fileAttachments[].key`
and passed it straight into `trackChatUpload`, which wrote it into the
`workspace_files` ownership binding with no permission check and no
key-ownership validation. That binding is what `verifyFileAccess` and the
Files feature resolve authorization from, so any workspace member —
including read-only — could hand in another member's key and have their
file re-parented to a private chat: removed from the Files listing, from
folders and from download-by-id, and destroyable through the chat-delete
FK cascade. The sibling register route already enforced these invariants;
the copilot path did not.

- `trackChatUpload` now rejects keys that do not address the target
  workspace, only re-links a chat-upload row the caller already owns
  (matched by row id, not by raw key), and only mints a new binding when
  the key has no prior record at all — including soft-deleted ones, which
  the partial active-key unique index would otherwise let it insert over.
- Minting a new binding verifies the object exists in storage, matching
  `registerUploadedWorkspaceFile`.
- `buildCopilotRequestPayload` gates attachment tracking on write/admin,
  the same grant the upload routes that issue these keys require.

* fix(security): make the chat-upload ownership check atomic with its write

The ownership lookup and the binding UPDATE were separate statements, and the
UPDATE matched on the captured row id alone. A concurrent `materialize_file`
sets `context='workspace'` and clears `chatId` on that same row, so the
tracking write still matched and dragged the saved file back into chat scope —
hiding it from every workspace-file listing and re-exposing it to the
chat-delete cascade, with materialize's storage-usage increment left stranded.

Re-assert every ownership predicate in the UPDATE so the statement is itself
the atomic check. The lookup now only decides UPDATE-vs-INSERT and is no
longer load-bearing for authorization, which also makes the existing
`updated.length === 0` fail-closed branch correct rather than dead.

* fix(uploads): tolerate transient storage probes and reject unowned keys with 403

Follow-ups from a backward-compatibility audit of the attachment-key hardening.

The existence probe is hygiene, not authorization — the key-format and
no-prior-record guards already carry that, and a binding to a nonexistent
object grants nothing readable. But `headObject` rethrows non-404 provider
errors, so a transient 5xx or throttle would drop a legitimate attachment.
Only a definitive not-found now rejects; a thrown error logs and proceeds on
the ownership guards. This path is reached solely by >50MB multipart uploads,
the one flow that persists no metadata row at upload time.

The stage route mapped an ownership rejection to a 500. It is a client error;
return 403 instead.

* fix(security): compare-and-swap the chat binding on chat uploads

Two overlapping chat requests could both observe the same claimable row with
`chat_id IS NULL` and both satisfy the update predicate, so the later write
silently moved the upload to its own chat — taking over the delete-cascade
lifecycle of a file the first chat had already bound.

Scope the update to `chat_id IS NULL OR chat_id = <target>` so the statement
is a compare-and-swap: the loser matches zero rows and fails closed. The
resolver applies the same rule so the update-vs-insert decision stays
coherent.

This also makes an upload bind to exactly one chat, matching the 409 the
sibling `local-files/stage` route already returns for the same case; verified
no client flow relinks a key across chats (drafts are per-chat, every retry
path replays under a pinned chat id, forking copies blobs to fresh keys).

* refactor(copilot): gate attachment tracking with the shared permission predicate

`permissionSatisfies` is the documented single source of truth for permission
comparisons and exists to replace hand-written `=== 'admin' || === 'write'`
ladders. `userPermission` is typed `string` for legacy reasons, so narrow it
with `isPermissionType` first — an unrecognized value fails the gate instead of
ranking below every level. Behavior is unchanged for all three levels.

Imported from the dependency-free `/predicates` subpath rather than
`/workspace`, which would pull `@sim/db` onto the chat request path.

Author: waleedlatif1

apps/sim/app/api/mothership/local-files/stage/route.ts

@@ -13,7 +13,10 @@ import {
 } from '@/lib/copilot/request/http'
 import { encodeVfsSegment } from '@/lib/copilot/vfs/path-utils'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
-import { trackChatUpload } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
+import {
+  trackChatUpload,
+  WorkspaceFileKeyOwnershipError,
+} from '@/lib/uploads/contexts/workspace/workspace-file-manager'
 import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
 
 const logger = createLogger('StageLocalFileUploadAPI')
@@ -95,6 +98,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       uploadPath: `uploads/${encodeVfsSegment(displayName)}`,
     })
   } catch (error) {
+    if (error instanceof WorkspaceFileKeyOwnershipError) {
+      // The caller supplied a key they may not bind — a client error, not ours.
+      logger.warn('Rejected chat upload staging for an unowned storage key', {
+        error: error.message,
+      })
+      return NextResponse.json({ error: 'Storage key is not available' }, { status: 403 })
+    }
     logger.error('Failed to stage local file upload', error)
     return createInternalServerErrorResponse('Failed to stage local file upload')
   }

apps/sim/lib/copilot/chat/payload.test.ts

@@ -4,10 +4,12 @@
 import { workflowsUtilsMock } from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockCreateUserToolSchema, mockGetHighestPrioritySubscription } = vi.hoisted(() => ({
-  mockCreateUserToolSchema: vi.fn(() => ({ type: 'object', properties: {} })),
-  mockGetHighestPrioritySubscription: vi.fn(),
-}))
+const { mockCreateUserToolSchema, mockGetHighestPrioritySubscription, mockTrackChatUpload } =
+  vi.hoisted(() => ({
+    mockCreateUserToolSchema: vi.fn(() => ({ type: 'object', properties: {} })),
+    mockGetHighestPrioritySubscription: vi.fn(),
+    mockTrackChatUpload: vi.fn(),
+  }))
 
 vi.mock('@/lib/billing/core/subscription', () => ({
   getHighestPrioritySubscription: mockGetHighestPrioritySubscription,
@@ -104,6 +106,10 @@ vi.mock('@/tools/params', () => ({
   createUserToolSchema: mockCreateUserToolSchema,
 }))
 
+vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
+  trackChatUpload: mockTrackChatUpload,
+}))
+
 import {
   buildCopilotRequestPayload,
   buildIntegrationToolSchemas,
@@ -209,6 +215,53 @@ describe('buildIntegrationToolSchemas', () => {
 describe('buildCopilotRequestPayload', () => {
   beforeEach(() => {
     vi.clearAllMocks()
+    mockTrackChatUpload.mockResolvedValue({ displayName: 'payroll.xlsx' })
+  })
+
+  describe('file attachment tracking', () => {
+    const attachmentParams = {
+      message: 'hi',
+      userId: 'mallory',
+      userMessageId: 'msg-1',
+      mode: 'agent',
+      model: 'claude-opus-4-8',
+      workspaceId: 'ws-1',
+      chatId: 'chat-1',
+      fileAttachments: [
+        { id: 'a1', key: 'workspace/ws-1/1731000000000-ab12cd34-payroll.xlsx', size: 1 },
+      ],
+    }
+
+    /**
+     * Tracking writes `workspace_files` rows. A read-only member reaching the
+     * chat endpoint must not gain that write through an attachment.
+     */
+    it.each(['read', undefined])('does not track attachments for permission %s', async (perm) => {
+      await buildCopilotRequestPayload(
+        { ...attachmentParams, userPermission: perm },
+        { selectedModel: 'claude-opus-4-8' }
+      )
+
+      expect(mockTrackChatUpload).not.toHaveBeenCalled()
+    })
+
+    it.each(['write', 'admin'])('tracks attachments for permission %s', async (perm) => {
+      await buildCopilotRequestPayload(
+        { ...attachmentParams, userPermission: perm },
+        { selectedModel: 'claude-opus-4-8' }
+      )
+
+      expect(mockTrackChatUpload).toHaveBeenCalledWith(
+        'ws-1',
+        'mallory',
+        'chat-1',
+        'workspace/ws-1/1731000000000-ab12cd34-payroll.xlsx',
+        expect.anything(),
+        expect.anything(),
+        1,
+        'msg-1'
+      )
+    })
   })
 
   it('passes workspaceContext through to the Go request payload', async () => {

apps/sim/lib/copilot/chat/payload.ts

@@ -1,5 +1,6 @@
 import type { BrowserKnownSession } from '@sim/browser-protocol'
 import { createLogger } from '@sim/logger'
+import { isPermissionType, permissionSatisfies } from '@sim/platform-authz/predicates'
 import { toError } from '@sim/utils/errors'
 import { LRUCache } from 'lru-cache'
 import { getHighestPrioritySubscription } from '@/lib/billing/core/subscription'
@@ -333,10 +334,25 @@ export async function buildCopilotRequestPayload(
   const effectiveMode = mode === 'agent' ? 'build' : mode
   const transportMode = effectiveMode === 'build' ? 'agent' : effectiveMode
 
-  // Track uploaded files in the DB and build context tags instead of base64 inlining
+  // Track uploaded files in the DB and build context tags instead of base64 inlining.
+  // Tracking writes `workspace_files` rows, so it needs the same write grant the
+  // upload routes that issue these keys already require — reaching the chat
+  // endpoint with `read` must not confer a file-write capability.
   const uploadContexts: Array<{ type: string; content: string; tag?: string; path?: string }> = []
+  // `userPermission` is typed `string` for legacy reasons, so narrow it before
+  // comparing — an unrecognized value must fail the gate, not rank below it.
+  const canWriteWorkspaceFiles =
+    isPermissionType(params.userPermission) && permissionSatisfies(params.userPermission, 'write')
   if (chatId && params.workspaceId && fileAttachments && fileAttachments.length > 0) {
-    for (const f of fileAttachments) {
+    if (!canWriteWorkspaceFiles) {
+      logger.warn('Dropping chat file attachments without workspace write access', {
+        chatId,
+        workspaceId: params.workspaceId,
+        attachmentCount: fileAttachments.length,
+      })
+    }
+    const trackableAttachments = canWriteWorkspaceFiles ? fileAttachments : []
+    for (const f of trackableAttachments) {
       const filename = (f.filename ?? f.name ?? 'file') as string
       const mediaType = (f.media_type ?? f.mimeType ?? 'application/octet-stream') as string
       try {