From 5849e77697d36816a16d96c0a9b8277cebcfbe83 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 15 May 2026 18:41:20 -0700 Subject: [PATCH 1/2] improvement(copilot): trim copilot_chats reads to lean projections --- .../app/api/copilot/chat/delete/route.test.ts | 5 +- apps/sim/app/api/copilot/chat/delete/route.ts | 4 +- apps/sim/app/api/copilot/chat/queries.ts | 20 ++++-- apps/sim/app/api/copilot/chat/rename/route.ts | 4 +- .../api/copilot/chat/update-messages/route.ts | 4 +- .../copilot/checkpoints/revert/route.test.ts | 1 + .../api/copilot/checkpoints/revert/route.ts | 4 +- .../app/api/copilot/checkpoints/route.test.ts | 1 + apps/sim/app/api/copilot/checkpoints/route.ts | 6 +- .../mothership/chats/[chatId]/route.test.ts | 1 + .../api/mothership/chats/[chatId]/route.ts | 7 +- apps/sim/lib/api/contracts/copilot.ts | 4 -- apps/sim/lib/copilot/chat/lifecycle.ts | 65 +++++++++++++++++-- 13 files changed, 97 insertions(+), 29 deletions(-) diff --git a/apps/sim/app/api/copilot/chat/delete/route.test.ts b/apps/sim/app/api/copilot/chat/delete/route.test.ts index 785cc66a63d..dd93b33367b 100644 --- a/apps/sim/app/api/copilot/chat/delete/route.test.ts +++ b/apps/sim/app/api/copilot/chat/delete/route.test.ts @@ -7,14 +7,16 @@ import { authMockFns, dbChainMock, dbChainMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockGetAccessibleCopilotChat } = vi.hoisted(() => ({ +const { mockGetAccessibleCopilotChat, mockGetAccessibleCopilotChatAuth } = vi.hoisted(() => ({ mockGetAccessibleCopilotChat: vi.fn(), + mockGetAccessibleCopilotChatAuth: vi.fn(), })) vi.mock('@sim/db', () => dbChainMock) vi.mock('@/lib/copilot/chat/lifecycle', () => ({ getAccessibleCopilotChat: mockGetAccessibleCopilotChat, + getAccessibleCopilotChatAuth: mockGetAccessibleCopilotChatAuth, })) vi.mock('@/lib/copilot/tasks', () => ({ @@ -39,6 +41,7 @@ describe('Copilot Chat Delete API Route', () => { dbChainMockFns.returning.mockResolvedValue([{ workspaceId: 'ws-1' }]) mockGetAccessibleCopilotChat.mockResolvedValue({ id: 'chat-123', userId: 'user-123' }) + mockGetAccessibleCopilotChatAuth.mockResolvedValue({ id: 'chat-123', userId: 'user-123' }) }) afterEach(() => { diff --git a/apps/sim/app/api/copilot/chat/delete/route.ts b/apps/sim/app/api/copilot/chat/delete/route.ts index 87452df78d6..fd06735a91f 100644 --- a/apps/sim/app/api/copilot/chat/delete/route.ts +++ b/apps/sim/app/api/copilot/chat/delete/route.ts @@ -6,7 +6,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { deleteCopilotChatContract } from '@/lib/api/contracts/copilot' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { getAccessibleCopilotChat } from '@/lib/copilot/chat/lifecycle' +import { getAccessibleCopilotChatAuth } from '@/lib/copilot/chat/lifecycle' import { taskPubSub } from '@/lib/copilot/tasks' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -30,7 +30,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest) => { if (!validated.success) return validated.response const parsed = validated.data.body - const chat = await getAccessibleCopilotChat(parsed.chatId, session.user.id) + const chat = await getAccessibleCopilotChatAuth(parsed.chatId, session.user.id) if (!chat) { return NextResponse.json({ success: true }) } diff --git a/apps/sim/app/api/copilot/chat/queries.ts b/apps/sim/app/api/copilot/chat/queries.ts index bc475a8bc09..41ff9ec4bbf 100644 --- a/apps/sim/app/api/copilot/chat/queries.ts +++ b/apps/sim/app/api/copilot/chat/queries.ts @@ -51,6 +51,21 @@ function transformChat(chat: { } } +type CopilotChatListRow = Pick< + typeof copilotChats.$inferSelect, + 'id' | 'title' | 'model' | 'createdAt' | 'updatedAt' +> + +function transformChatListItem(chat: CopilotChatListRow) { + return { + id: chat.id, + title: chat.title, + model: chat.model, + createdAt: chat.createdAt, + updatedAt: chat.updatedAt, + } +} + export async function GET(req: NextRequest) { try { const { searchParams } = new URL(req.url) @@ -166,9 +181,6 @@ export async function GET(req: NextRequest) { id: copilotChats.id, title: copilotChats.title, model: copilotChats.model, - messages: copilotChats.messages, - planArtifact: copilotChats.planArtifact, - config: copilotChats.config, createdAt: copilotChats.createdAt, updatedAt: copilotChats.updatedAt, }) @@ -181,7 +193,7 @@ export async function GET(req: NextRequest) { return NextResponse.json({ success: true, - chats: chats.map(transformChat), + chats: chats.map(transformChatListItem), }) } catch (error) { logger.error('Error fetching copilot chats:', error) diff --git a/apps/sim/app/api/copilot/chat/rename/route.ts b/apps/sim/app/api/copilot/chat/rename/route.ts index 97874ba2806..6886f568be6 100644 --- a/apps/sim/app/api/copilot/chat/rename/route.ts +++ b/apps/sim/app/api/copilot/chat/rename/route.ts @@ -6,7 +6,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { renameCopilotChatContract } from '@/lib/api/contracts/copilot' import { parseRequest, validationErrorResponse } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { getAccessibleCopilotChat } from '@/lib/copilot/chat/lifecycle' +import { getAccessibleCopilotChatAuth } from '@/lib/copilot/chat/lifecycle' import { taskPubSub } from '@/lib/copilot/tasks' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -30,7 +30,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest) => { if (!parsed.success) return parsed.response const { chatId, title } = parsed.data.body - const chat = await getAccessibleCopilotChat(chatId, session.user.id) + const chat = await getAccessibleCopilotChatAuth(chatId, session.user.id) if (!chat) { return NextResponse.json({ success: false, error: 'Chat not found' }, { status: 404 }) } diff --git a/apps/sim/app/api/copilot/chat/update-messages/route.ts b/apps/sim/app/api/copilot/chat/update-messages/route.ts index f58691fd121..7107883f2d1 100644 --- a/apps/sim/app/api/copilot/chat/update-messages/route.ts +++ b/apps/sim/app/api/copilot/chat/update-messages/route.ts @@ -5,7 +5,7 @@ import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { updateCopilotMessagesContract } from '@/lib/api/contracts/copilot' import { parseRequest } from '@/lib/api/server' -import { getAccessibleCopilotChat } from '@/lib/copilot/chat/lifecycle' +import { getAccessibleCopilotChatAuth } from '@/lib/copilot/chat/lifecycle' import { normalizeMessage, type PersistedMessage } from '@/lib/copilot/chat/persisted-message' import { authenticateCopilotRequestSessionOnly, @@ -66,7 +66,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => { } // Verify that the chat belongs to the user - const chat = await getAccessibleCopilotChat(chatId, userId) + const chat = await getAccessibleCopilotChatAuth(chatId, userId) if (!chat) { return createNotFoundResponse('Chat not found or unauthorized') diff --git a/apps/sim/app/api/copilot/checkpoints/revert/route.test.ts b/apps/sim/app/api/copilot/checkpoints/revert/route.test.ts index a05d3df1131..251578bbd80 100644 --- a/apps/sim/app/api/copilot/checkpoints/revert/route.test.ts +++ b/apps/sim/app/api/copilot/checkpoints/revert/route.test.ts @@ -36,6 +36,7 @@ vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock) vi.mock('@/lib/copilot/chat/lifecycle', () => ({ getAccessibleCopilotChat: mockGetAccessibleCopilotChat, + getAccessibleCopilotChatAuth: mockGetAccessibleCopilotChat, })) vi.mock('@sim/db', () => ({ diff --git a/apps/sim/app/api/copilot/checkpoints/revert/route.ts b/apps/sim/app/api/copilot/checkpoints/revert/route.ts index 1e3025a672f..5ccda51212d 100644 --- a/apps/sim/app/api/copilot/checkpoints/revert/route.ts +++ b/apps/sim/app/api/copilot/checkpoints/revert/route.ts @@ -7,7 +7,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { revertCopilotCheckpointContract } from '@/lib/api/contracts/copilot' import type { CleanedWorkflowState } from '@/lib/api/contracts/workflows' import { parseRequest } from '@/lib/api/server' -import { getAccessibleCopilotChat } from '@/lib/copilot/chat/lifecycle' +import { getAccessibleCopilotChatAuth } from '@/lib/copilot/chat/lifecycle' import { authenticateCopilotRequestSessionOnly, createInternalServerErrorResponse, @@ -57,7 +57,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { return createNotFoundResponse('Checkpoint not found or access denied') } - const chat = await getAccessibleCopilotChat(checkpoint.chatId, userId) + const chat = await getAccessibleCopilotChatAuth(checkpoint.chatId, userId) if (!chat) { return createNotFoundResponse('Checkpoint not found or access denied') } diff --git a/apps/sim/app/api/copilot/checkpoints/route.test.ts b/apps/sim/app/api/copilot/checkpoints/route.test.ts index 1001f87701e..f7dc748786b 100644 --- a/apps/sim/app/api/copilot/checkpoints/route.test.ts +++ b/apps/sim/app/api/copilot/checkpoints/route.test.ts @@ -44,6 +44,7 @@ vi.mock('drizzle-orm', () => ({ vi.mock('@/lib/copilot/chat/lifecycle', () => ({ getAccessibleCopilotChat: mockGetAccessibleCopilotChat, + getAccessibleCopilotChatAuth: mockGetAccessibleCopilotChat, })) vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock) diff --git a/apps/sim/app/api/copilot/checkpoints/route.ts b/apps/sim/app/api/copilot/checkpoints/route.ts index 5198933f4f5..985a95a71a3 100644 --- a/apps/sim/app/api/copilot/checkpoints/route.ts +++ b/apps/sim/app/api/copilot/checkpoints/route.ts @@ -9,7 +9,7 @@ import { listCopilotCheckpointsContract, } from '@/lib/api/contracts/copilot' import { getValidationErrorMessage, parseRequest, validationErrorResponse } from '@/lib/api/server' -import { getAccessibleCopilotChat } from '@/lib/copilot/chat/lifecycle' +import { getAccessibleCopilotChatAuth } from '@/lib/copilot/chat/lifecycle' import { authenticateCopilotRequestSessionOnly, createBadRequestResponse, @@ -60,7 +60,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => { }) // Verify that the chat belongs to the user - const chat = await getAccessibleCopilotChat(chatId, userId) + const chat = await getAccessibleCopilotChatAuth(chatId, userId) if (!chat) { return createBadRequestResponse('Chat not found or unauthorized') @@ -159,7 +159,7 @@ export const GET = withRouteHandler(async (req: NextRequest) => { chatId, }) - const chat = await getAccessibleCopilotChat(chatId, userId) + const chat = await getAccessibleCopilotChatAuth(chatId, userId) if (!chat) { return createBadRequestResponse('Chat not found or unauthorized') } diff --git a/apps/sim/app/api/mothership/chats/[chatId]/route.test.ts b/apps/sim/app/api/mothership/chats/[chatId]/route.test.ts index ad0efd9a946..3d54f4fad00 100644 --- a/apps/sim/app/api/mothership/chats/[chatId]/route.test.ts +++ b/apps/sim/app/api/mothership/chats/[chatId]/route.test.ts @@ -48,6 +48,7 @@ vi.mock('@/lib/copilot/request/http', () => copilotHttpMock) vi.mock('@/lib/copilot/chat/lifecycle', () => ({ getAccessibleCopilotChat: mockGetAccessibleCopilotChat, + getAccessibleCopilotChatAuth: mockGetAccessibleCopilotChat, })) vi.mock('@/lib/copilot/chat/stream-liveness', () => ({ diff --git a/apps/sim/app/api/mothership/chats/[chatId]/route.ts b/apps/sim/app/api/mothership/chats/[chatId]/route.ts index 91c88712cfd..121a01b5fde 100644 --- a/apps/sim/app/api/mothership/chats/[chatId]/route.ts +++ b/apps/sim/app/api/mothership/chats/[chatId]/route.ts @@ -12,7 +12,10 @@ import { import { parseRequest } from '@/lib/api/server' import { getLatestRunForStream } from '@/lib/copilot/async-runs/repository' import { buildEffectiveChatTranscript } from '@/lib/copilot/chat/effective-transcript' -import { getAccessibleCopilotChat } from '@/lib/copilot/chat/lifecycle' +import { + getAccessibleCopilotChat, + getAccessibleCopilotChatAuth, +} from '@/lib/copilot/chat/lifecycle' import { normalizeMessage } from '@/lib/copilot/chat/persisted-message' import { reconcileChatStreamMarkers } from '@/lib/copilot/chat/stream-liveness' import { @@ -238,7 +241,7 @@ export const DELETE = withRouteHandler( if (!parsed.success) return parsed.response const { chatId } = parsed.data.params - const chat = await getAccessibleCopilotChat(chatId, userId) + const chat = await getAccessibleCopilotChatAuth(chatId, userId) if (!chat || chat.type !== 'mothership') { return NextResponse.json({ success: true }) } diff --git a/apps/sim/lib/api/contracts/copilot.ts b/apps/sim/lib/api/contracts/copilot.ts index 365ae41b9f5..41bffb5f3be 100644 --- a/apps/sim/lib/api/contracts/copilot.ts +++ b/apps/sim/lib/api/contracts/copilot.ts @@ -418,10 +418,6 @@ const copilotChatGetListItemSchema = z id: z.string(), title: z.string().nullable(), model: z.string().nullable(), - messages: z.array(z.unknown()), - messageCount: z.number(), - planArtifact: z.unknown().nullable(), - config: z.unknown().nullable(), createdAt: z.string().nullable(), updatedAt: z.string().nullable(), }) diff --git a/apps/sim/lib/copilot/chat/lifecycle.ts b/apps/sim/lib/copilot/chat/lifecycle.ts index c74a90317b6..5224ffdd611 100644 --- a/apps/sim/lib/copilot/chat/lifecycle.ts +++ b/apps/sim/lib/copilot/chat/lifecycle.ts @@ -20,13 +20,30 @@ export interface ChatLoadResult { isNew: boolean } -export async function getAccessibleCopilotChat(chatId: string, userId: string) { - const [chat] = await db - .select() - .from(copilotChats) - .where(and(eq(copilotChats.id, chatId), eq(copilotChats.userId, userId))) - .limit(1) - +/** + * Minimal column set needed to perform workflow/workspace authorization for a + * copilot chat. Heavy TOAST-able columns (messages, planArtifact, previewYaml, + * config, resources) are intentionally excluded — callers that only need to + * verify ownership should not pay the detoast cost for those fields. + */ +const copilotChatAuthColumns = { + id: copilotChats.id, + userId: copilotChats.userId, + workflowId: copilotChats.workflowId, + workspaceId: copilotChats.workspaceId, + type: copilotChats.type, +} as const + +type CopilotChatAuthRow = Pick< + typeof copilotChats.$inferSelect, + 'id' | 'userId' | 'workflowId' | 'workspaceId' | 'type' +> + +async function authorizeCopilotChatRow( + chat: T | undefined, + chatId: string, + userId: string +): Promise { if (!chat) { logger.warn('Copilot chat not found or not owned by user', { chatId, userId }) return null @@ -61,6 +78,40 @@ export async function getAccessibleCopilotChat(chatId: string, userId: string) { return chat } +/** + * Verify a copilot chat exists, is owned by the user, and the user has access + * to its workflow/workspace. Selects only the columns required for the + * authorization check — use this for routes that only need ownership + * verification before a mutation (rename, delete, update-messages). + */ +export async function getAccessibleCopilotChatAuth( + chatId: string, + userId: string +): Promise { + const [chat] = await db + .select(copilotChatAuthColumns) + .from(copilotChats) + .where(and(eq(copilotChats.id, chatId), eq(copilotChats.userId, userId))) + .limit(1) + + return authorizeCopilotChatRow(chat, chatId, userId) +} + +/** + * Load the full copilot chat row after authorization. Use this only when the + * caller actually consumes the heavy columns (`messages`, `planArtifact`, + * `config`, etc.) — for example, chat resume or the GET-by-id endpoint. + */ +export async function getAccessibleCopilotChat(chatId: string, userId: string) { + const [chat] = await db + .select() + .from(copilotChats) + .where(and(eq(copilotChats.id, chatId), eq(copilotChats.userId, userId))) + .limit(1) + + return authorizeCopilotChatRow(chat, chatId, userId) +} + /** * Resolve or create a copilot chat session. * If chatId is provided, loads the existing chat. Otherwise creates a new one. From 59d25fe49b345a561d683d12fb299ac11aae586f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 15 May 2026 18:48:59 -0700 Subject: [PATCH 2/2] fix(copilot): exercise idempotent-delete guard via the lean auth mock --- apps/sim/app/api/copilot/chat/delete/route.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/sim/app/api/copilot/chat/delete/route.test.ts b/apps/sim/app/api/copilot/chat/delete/route.test.ts index dd93b33367b..ccabaf7c521 100644 --- a/apps/sim/app/api/copilot/chat/delete/route.test.ts +++ b/apps/sim/app/api/copilot/chat/delete/route.test.ts @@ -143,7 +143,7 @@ describe('Copilot Chat Delete API Route', () => { it('should delete chat even if it does not exist (idempotent)', async () => { authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-123' } }) - mockGetAccessibleCopilotChat.mockResolvedValueOnce(null) + mockGetAccessibleCopilotChatAuth.mockResolvedValueOnce(null) const req = createMockRequest('DELETE', { chatId: 'non-existent-chat',