fix(chat): review round 1 — restore workspace authz, purge recheck, archived-list invalidation

Author: waleedlatif1

apps/sim/app/api/mothership/chats/[chatId]/restore/route.ts

@@ -8,19 +8,25 @@ import { parseRequest } from '@/lib/api/server'
 import { chatPubSub } from '@/lib/copilot/chat-status'
 import {
   authenticateCopilotRequestSessionOnly,
+  createForbiddenResponse,
   createInternalServerErrorResponse,
   createUnauthorizedResponse,
 } from '@/lib/copilot/request/http'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { captureServerEvent } from '@/lib/posthog/server'
+import {
+  assertActiveWorkspaceAccess,
+  isWorkspaceAccessDeniedError,
+} from '@/lib/workspaces/permissions/utils'
 
 const logger = createLogger('RestoreMothershipChatAPI')
 
 /**
  * POST /api/mothership/chats/[chatId]/restore
  * Restores a soft-deleted mothership chat back into the sidebar. Ownership is
- * enforced by scoping the update to the authenticated user's rows, matching
- * the delete path.
+ * enforced by scoping the update to the authenticated user's rows, and the
+ * caller must still have access to the chat's workspace, matching the delete
+ * path.
  */
 export const POST = withRouteHandler(
   async (request: NextRequest, context: { params: Promise<{ chatId: string }> }) => {
@@ -34,6 +40,26 @@ export const POST = withRouteHandler(
       if (!parsed.success) return parsed.response
       const { chatId } = parsed.data.params
 
+      const [chat] = await db
+        .select({ workspaceId: copilotChats.workspaceId })
+        .from(copilotChats)
+        .where(
+          and(
+            eq(copilotChats.id, chatId),
+            eq(copilotChats.userId, userId),
+            eq(copilotChats.type, 'mothership'),
+            isNotNull(copilotChats.deletedAt)
+          )
+        )
+        .limit(1)
+
+      if (!chat) {
+        return NextResponse.json({ success: false, error: 'Chat not found' }, { status: 404 })
+      }
+      if (chat.workspaceId) {
+        await assertActiveWorkspaceAccess(chat.workspaceId, userId)
+      }
+
       const [restoredChat] = await db
         .update(copilotChats)
         .set({ deletedAt: null })
@@ -71,6 +97,9 @@ export const POST = withRouteHandler(
 
       return NextResponse.json({ success: true })
     } catch (error) {
+      if (isWorkspaceAccessDeniedError(error)) {
+        return createForbiddenResponse('Workspace access denied')
+      }
       logger.error('Error restoring mothership chat:', error)
       return createInternalServerErrorResponse('Failed to restore chat')
     }

apps/sim/background/cleanup-soft-deletes.ts

@@ -600,13 +600,26 @@ export async function runCleanupSoftDeletes(payload: CleanupJobPayload): Promise
 
   // Workflow-scoped chats above are removed by the workflow FK cascade;
   // soft-deleted mothership chats have no workflow and need their own delete.
-  const chatResult = await deleteRowsById(
-    copilotChats,
-    copilotChats.id,
-    softDeletedChatIds,
-    `${label}/copilotChats`
-  )
-  totalDeleted += chatResult.deleted
+  // Re-check the soft-delete cutoff in the DELETE itself so a chat restored
+  // between selection and this point survives (chatCleanup.execute() below
+  // also re-checks row existence before purging external data).
+  for (const batch of chunkArray(softDeletedChatIds, DEFAULT_DELETE_CHUNK_SIZE)) {
+    try {
+      const deleted = await db
+        .delete(copilotChats)
+        .where(
+          and(
+            inArray(copilotChats.id, batch),
+            isNotNull(copilotChats.deletedAt),
+            lt(copilotChats.deletedAt, retentionDate)
+          )
+        )
+        .returning({ id: copilotChats.id })
+      totalDeleted += deleted.length
+    } catch (error) {
+      logger.error(`[${label}/copilotChats] Soft-deleted chat delete failed`, { error })
+    }
+  }
 
   const legacyFileResult = await deleteExpiredLegacyWorkspaceFileRows(
     fileCleanup.legacyRows,

apps/sim/hooks/use-mothership-chat-events.ts

@@ -101,7 +101,9 @@ export function handleMothershipChatStatusEvent(
     return
   }
 
-  queryClient.invalidateQueries({ queryKey: mothershipChatKeys.list(workspaceId) })
+  // workspaceLists covers both the active and archived (Recently Deleted)
+  // lists: delete/restore events move chats between the two scopes.
+  queryClient.invalidateQueries({ queryKey: mothershipChatKeys.workspaceLists(workspaceId) })
   if (!payload.chatId) return
   if (payload.type === 'deleted') {
     queryClient.removeQueries({ queryKey: mothershipChatKeys.detail(payload.chatId) })

apps/sim/lib/cleanup/chat-cleanup.ts

@@ -1,5 +1,5 @@
 import { db } from '@sim/db'
-import { copilotMessages, workspaceFiles } from '@sim/db/schema'
+import { copilotChats, copilotMessages, workspaceFiles } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { and, inArray, isNull } from 'drizzle-orm'
 import { chunkArray } from '@/lib/cleanup/batch-delete'
@@ -26,6 +26,7 @@ type ChatScopedContext = (typeof CHAT_SCOPED_CONTEXTS)[number]
 interface FileRef {
   key: string
   context: ChatScopedContext
+  chatId: string
 }
 
 /**
@@ -42,7 +43,11 @@ export async function collectChatFiles(chatIds: string[]): Promise<FileRef[]> {
   for (const chunk of chunkArray(chatIds, CHAT_FILE_COLLECT_CHUNK_SIZE)) {
     const [linkedFiles, messageRows] = await Promise.all([
       db
-        .select({ key: workspaceFiles.key, context: workspaceFiles.context })
+        .select({
+          key: workspaceFiles.key,
+          context: workspaceFiles.context,
+          chatId: workspaceFiles.chatId,
+        })
         .from(workspaceFiles)
         .where(
           and(
@@ -54,15 +59,15 @@ export async function collectChatFiles(chatIds: string[]): Promise<FileRef[]> {
       // Scan every message row for the chat (no deleted_at filter): this is a
       // deletion path collecting blob keys, so attachments on any row count.
       db
-        .select({ content: copilotMessages.content })
+        .select({ content: copilotMessages.content, chatId: copilotMessages.chatId })
         .from(copilotMessages)
         .where(inArray(copilotMessages.chatId, chunk)),
     ])
 
     for (const f of linkedFiles) {
-      if (!seen.has(f.key)) {
+      if (f.chatId && !seen.has(f.key)) {
         seen.add(f.key)
-        files.push({ key: f.key, context: f.context as ChatScopedContext })
+        files.push({ key: f.key, context: f.context as ChatScopedContext, chatId: f.chatId })
       }
     }
 
@@ -80,7 +85,7 @@ export async function collectChatFiles(chatIds: string[]): Promise<FileRef[]> {
           const key = (attachment as Record<string, unknown>).key as string
           if (!seen.has(key)) {
             seen.add(key)
-            files.push({ key, context: 'copilot' })
+            files.push({ key, context: 'copilot', chatId: row.chatId })
           }
         }
       }
@@ -195,17 +200,36 @@ export async function prepareChatCleanup(
 
   return {
     execute: async () => {
+      // A chat can be restored (or its delete can fail) between selection and
+      // the caller's row delete. Purge backend data and files only for chats
+      // whose rows are actually gone, so a surviving row never loses its data.
+      const survivors = new Set<string>()
+      for (const chunk of chunkArray(chatIds, CHAT_FILE_COLLECT_CHUNK_SIZE)) {
+        const rows = await db
+          .select({ id: copilotChats.id })
+          .from(copilotChats)
+          .where(inArray(copilotChats.id, chunk))
+        for (const row of rows) survivors.add(row.id)
+      }
+      if (survivors.size > 0) {
+        logger.info(
+          `[${label}] Skipping external cleanup for ${survivors.size} chats whose rows still exist`
+        )
+      }
+      const confirmedChatIds = chatIds.filter((id) => !survivors.has(id))
+      const confirmedFiles = files.filter((file) => !survivors.has(file.chatId))
+
       // Call copilot backend
-      if (chatIds.length > 0) {
-        const copilotResult = await cleanupCopilotBackend(chatIds, label)
+      if (confirmedChatIds.length > 0) {
+        const copilotResult = await cleanupCopilotBackend(confirmedChatIds, label)
         logger.info(
           `[${label}] Copilot backend: ${copilotResult.deleted} deleted, ${copilotResult.failed} failed`
         )
       }
 
       // Delete storage files with correct context per file
-      if (files.length > 0) {
-        const fileStats = await deleteStorageFiles(files, label)
+      if (confirmedFiles.length > 0) {
+        const fileStats = await deleteStorageFiles(confirmedFiles, label)
         logger.info(
           `[${label}] Storage cleanup: ${fileStats.filesDeleted} deleted, ${fileStats.filesFailed} failed`
         )