Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions apps/sim/app/api/copilot/chat/delete/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand All @@ -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(() => {
Expand Down Expand Up @@ -140,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',
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/app/api/copilot/chat/delete/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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 })
}
Expand Down
20 changes: 16 additions & 4 deletions apps/sim/app/api/copilot/chat/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
})
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/app/api/copilot/chat/rename/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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 })
}
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/app/api/copilot/chat/update-messages/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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')
Expand Down
1 change: 1 addition & 0 deletions apps/sim/app/api/copilot/checkpoints/revert/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock)

vi.mock('@/lib/copilot/chat/lifecycle', () => ({
getAccessibleCopilotChat: mockGetAccessibleCopilotChat,
getAccessibleCopilotChatAuth: mockGetAccessibleCopilotChat,
}))

vi.mock('@sim/db', () => ({
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/app/api/copilot/checkpoints/revert/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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')
}
Expand Down
1 change: 1 addition & 0 deletions apps/sim/app/api/copilot/checkpoints/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ vi.mock('drizzle-orm', () => ({

vi.mock('@/lib/copilot/chat/lifecycle', () => ({
getAccessibleCopilotChat: mockGetAccessibleCopilotChat,
getAccessibleCopilotChatAuth: mockGetAccessibleCopilotChat,
}))

vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock)
Expand Down
6 changes: 3 additions & 3 deletions apps/sim/app/api/copilot/checkpoints/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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')
}
Expand Down
1 change: 1 addition & 0 deletions apps/sim/app/api/mothership/chats/[chatId]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand Down
7 changes: 5 additions & 2 deletions apps/sim/app/api/mothership/chats/[chatId]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 })
}
Expand Down
4 changes: 0 additions & 4 deletions apps/sim/lib/api/contracts/copilot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
Expand Down
65 changes: 58 additions & 7 deletions apps/sim/lib/copilot/chat/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T extends CopilotChatAuthRow>(
chat: T | undefined,
chatId: string,
userId: string
): Promise<T | null> {
if (!chat) {
logger.warn('Copilot chat not found or not owned by user', { chatId, userId })
return null
Expand Down Expand Up @@ -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<CopilotChatAuthRow | null> {
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.
Expand Down
Loading