From 0df6242128e25ce7784ecc46769b194f8c63297e Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 18:07:23 +0300 Subject: [PATCH] fix(ai-chat): resolve page slugId to uuid in bound-chat, fixing 22P02 500 (#312) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/ai-chat/bound-chat 500'd with Postgres 22P02 because the client sends a page slugId (10-char nanoid) in the request `pageId` field, which the server passed straight into the UUID `page_id` column. The chat-to-document binding silently broke (client fail-softs to a new chat) and every slug-URL page open logged a 500. Fix: resolve the incoming id to a real page UUID on the server. PageRepo.findById already accepts both a uuid and a slugId (isValidUUID→slugId fallback), so boundChat now resolves the page first, guards it against a foreign/unknown workspace (returns {chatId:null} before any chat lookup — no cross-workspace probe), and looks up the latest chat by the resolved page.id (real uuid). Client: renamed the local pageId→slugId for clarity (the value is a slugId); the wire body key stays `pageId` so the DTO is unchanged. DTO left @IsString() (a @IsUUID() would only turn the 500 into a 400 and still break binding). Test: bound-chat spec asserts a slugId resolves and findLatestByPage is called with the real uuid; a foreign-workspace page → {chatId:null} without a chat lookup (no leak); an unknown id → {chatId:null}, no throw. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ai-chat/hooks/use-open-ai-chat.ts | 10 ++- .../ai-chat/services/ai-chat-service.ts | 6 +- .../ai-chat.controller.bound-chat.spec.ts | 84 +++++++++++++++---- .../ai-chat/ai-chat.controller.export.spec.ts | 1 + .../src/core/ai-chat/ai-chat.controller.ts | 20 ++++- .../ai-chat.generate-page-title.spec.ts | 1 + 6 files changed, 94 insertions(+), 28 deletions(-) diff --git a/apps/client/src/features/ai-chat/hooks/use-open-ai-chat.ts b/apps/client/src/features/ai-chat/hooks/use-open-ai-chat.ts index 604299c5..93f267db 100644 --- a/apps/client/src/features/ai-chat/hooks/use-open-ai-chat.ts +++ b/apps/client/src/features/ai-chat/hooks/use-open-ai-chat.ts @@ -27,7 +27,9 @@ export function useOpenAiChatForCurrentPage() { // AiChatWindow lives in a pathless parent layout route, so useParams() can't // see :pageSlug — match the full path against the authenticated page route. const match = useMatch("/s/:spaceSlug/p/:pageSlug"); - const pageId = extractPageSlugId(match?.params?.pageSlug); + // A page slugId (10-char nanoid), NOT a uuid; the server resolves it to the + // real page uuid (PageRepo.findById accepts slugId or uuid). + const slugId = extractPageSlugId(match?.params?.pageSlug); return useCallback(async () => { // Re-clicks while the window is already open (incl. minimized) must NOT @@ -40,9 +42,9 @@ export function useOpenAiChatForCurrentPage() { // connection the first click reads as a hung control until the POST returns. setWindowOpen(true); let resolved: string | null = activeChatId; // off-a-page: keep current - if (pageId) { + if (slugId) { try { - resolved = await getBoundChat(pageId); // null => fresh chat + resolved = await getBoundChat(slugId); // null => fresh chat } catch { resolved = null; // fail-soft: a fresh chat is always a safe fallback } @@ -58,7 +60,7 @@ export function useOpenAiChatForCurrentPage() { }, [ windowOpen, activeChatId, - pageId, + slugId, setWindowOpen, setActiveChatId, setDraft, diff --git a/apps/client/src/features/ai-chat/services/ai-chat-service.ts b/apps/client/src/features/ai-chat/services/ai-chat-service.ts index f8a57d0a..2e1528c8 100644 --- a/apps/client/src/features/ai-chat/services/ai-chat-service.ts +++ b/apps/client/src/features/ai-chat/services/ai-chat-service.ts @@ -46,9 +46,11 @@ export async function getAiChatMessages( * Resolve the chat bound to a document (the current user's most-recent chat * created on that page), or null when there is none. Drives auto-open-on-page. */ -export async function getBoundChat(pageId: string): Promise { +export async function getBoundChat(slugId: string): Promise { + // The `pageId` body field accepts a page slugId or a uuid; the server resolves + // it to the real page uuid (the wire key stays `pageId` for the DTO). const req = await api.post<{ chatId: string | null }>("/ai-chat/bound-chat", { - pageId, + pageId: slugId, }); return req.data.chatId; } diff --git a/apps/server/src/core/ai-chat/ai-chat.controller.bound-chat.spec.ts b/apps/server/src/core/ai-chat/ai-chat.controller.bound-chat.spec.ts index 769123a8..7c51e1fa 100644 --- a/apps/server/src/core/ai-chat/ai-chat.controller.bound-chat.spec.ts +++ b/apps/server/src/core/ai-chat/ai-chat.controller.bound-chat.spec.ts @@ -2,43 +2,91 @@ import { AiChatController } from './ai-chat.controller'; import type { User, Workspace } from '@docmost/db/types/entity.types'; /** - * Wiring spec for the #191 `POST /ai-chat/bound-chat` endpoint. It must forward - * the requesting user + workspace + pageId to findLatestByPage and return the - * matched chat's id, or `{ chatId: null }` when there is none. The repo already - * scopes to the caller's OWN chats, so a foreign pageId simply yields no match - * (null) — no extra page-access check is needed. Exercised with hand-rolled - * mocks, no Nest graph and no DB. + * Wiring spec for the #191 `POST /ai-chat/bound-chat` endpoint, hardened for + * #312. `dto.pageId` carries either a page slugId (10-char nanoid, off a slug + * URL) or a page uuid, so the controller must FIRST resolve it to a real page + * uuid via PageRepo.findById (which accepts both) — passing the raw slugId into + * the uuid ai_chats.page_id column caused a Postgres 22P02 500. Only then is the + * caller's most-recent OWN chat for that page looked up (by the resolved uuid), + * and a page in a different workspace (or an unknown id) yields { chatId: null } + * without ever touching the chat lookup. Exercised with hand-rolled mocks, no + * Nest graph and no DB. */ describe('AiChatController.boundChat', () => { const user = { id: 'u1' } as User; const workspace = { id: 'ws1' } as Workspace; - function makeController(chat: unknown) { + function makeController(opts: { page: unknown; chat?: unknown }) { const aiChatRepo = { - findLatestByPage: jest.fn().mockResolvedValue(chat), + findLatestByPage: jest.fn().mockResolvedValue(opts.chat), + }; + const pageRepo = { + findById: jest.fn().mockResolvedValue(opts.page), }; const controller = new AiChatController( {} as never, aiChatRepo as never, {} as never, {} as never, + pageRepo as never, ); - return { controller, aiChatRepo }; + return { controller, aiChatRepo, pageRepo }; } - it('returns the owned chat id and scopes the lookup to user + workspace + page', async () => { - const { controller, aiChatRepo } = makeController({ - id: 'c1', - creatorId: 'u1', + it('resolves a slugId to the page uuid and returns the owned chat id', async () => { + const { controller, aiChatRepo, pageRepo } = makeController({ + // findById accepts a slugId and returns the page with its real uuid. + page: { id: 'page-uuid-1', workspaceId: 'ws1' }, + chat: { id: 'c1', creatorId: 'u1' }, }); - const res = await controller.boundChat({ pageId: 'p1' }, user, workspace); - expect(aiChatRepo.findLatestByPage).toHaveBeenCalledWith('u1', 'ws1', 'p1'); + // The client sends a 10-char nanoid slugId, NOT a uuid. + const res = await controller.boundChat( + { pageId: 'i82qXsivsx' }, + user, + workspace, + ); + expect(pageRepo.findById).toHaveBeenCalledWith('i82qXsivsx'); + // findLatestByPage must receive the RESOLVED uuid, never the raw slugId. + expect(aiChatRepo.findLatestByPage).toHaveBeenCalledWith( + 'u1', + 'ws1', + 'page-uuid-1', + ); expect(res).toEqual({ chatId: 'c1' }); }); - it('returns { chatId: null } for a page with no owned chat (incl. foreign pageId)', async () => { - const { controller } = makeController(undefined); - const res = await controller.boundChat({ pageId: 'foreign' }, user, workspace); + it('returns { chatId: null } for a page in a DIFFERENT workspace without a chat lookup', async () => { + const { controller, aiChatRepo, pageRepo } = makeController({ + page: { id: 'page-uuid-2', workspaceId: 'other-ws' }, + }); + const res = await controller.boundChat( + { pageId: 'foreignSlug' }, + user, + workspace, + ); + expect(pageRepo.findById).toHaveBeenCalledWith('foreignSlug'); + // No cross-workspace leak: the chat lookup must never run. + expect(aiChatRepo.findLatestByPage).not.toHaveBeenCalled(); + expect(res).toEqual({ chatId: null }); + }); + + it('returns { chatId: null } for an unknown id without throwing or looking up a chat', async () => { + const { controller, aiChatRepo } = makeController({ page: undefined }); + const res = await controller.boundChat( + { pageId: 'does-not-exist' }, + user, + workspace, + ); + expect(aiChatRepo.findLatestByPage).not.toHaveBeenCalled(); + expect(res).toEqual({ chatId: null }); + }); + + it('returns { chatId: null } when the resolved page has no owned chat', async () => { + const { controller } = makeController({ + page: { id: 'page-uuid-3', workspaceId: 'ws1' }, + chat: undefined, + }); + const res = await controller.boundChat({ pageId: 'p3' }, user, workspace); expect(res).toEqual({ chatId: null }); }); }); diff --git a/apps/server/src/core/ai-chat/ai-chat.controller.export.spec.ts b/apps/server/src/core/ai-chat/ai-chat.controller.export.spec.ts index f46aeaa0..265a90b8 100644 --- a/apps/server/src/core/ai-chat/ai-chat.controller.export.spec.ts +++ b/apps/server/src/core/ai-chat/ai-chat.controller.export.spec.ts @@ -56,6 +56,7 @@ describe('AiChatController.export', () => { aiChatRepo as never, aiChatMessageRepo as never, {} as never, + {} as never, ); return { controller, aiChatRepo, aiChatMessageRepo }; } diff --git a/apps/server/src/core/ai-chat/ai-chat.controller.ts b/apps/server/src/core/ai-chat/ai-chat.controller.ts index 7834d6b2..40fa3d06 100644 --- a/apps/server/src/core/ai-chat/ai-chat.controller.ts +++ b/apps/server/src/core/ai-chat/ai-chat.controller.ts @@ -24,6 +24,7 @@ import { AiChat, User, Workspace } from '@docmost/db/types/entity.types'; import { PaginationOptions } from '@docmost/db/pagination/pagination-options'; import { AiChatRepo } from '@docmost/db/repos/ai-chat/ai-chat.repo'; import { AiChatMessageRepo } from '@docmost/db/repos/ai-chat/ai-chat-message.repo'; +import { PageRepo } from '@docmost/db/repos/page/page.repo'; import { UserThrottlerGuard } from '../../integrations/throttle/user-throttler.guard'; import { AI_CHAT_THROTTLER } from '../../integrations/throttle/throttler-names'; import { FileInterceptor } from '../../common/interceptors/file.interceptor'; @@ -55,6 +56,7 @@ export class AiChatController { private readonly aiChatRepo: AiChatRepo, private readonly aiChatMessageRepo: AiChatMessageRepo, private readonly aiTranscription: AiTranscriptionService, + private readonly pageRepo: PageRepo, ) {} /** List the requesting user's chats in this workspace (paginated). */ @@ -71,9 +73,15 @@ export class AiChatController { /** * Resolve the chat bound to a document for the requesting user: the most-recent * non-deleted chat created on that page (ai_chats.page_id). Returns - * { chatId: null } when the page has no owned chat (-> a fresh chat). No page - * access check needed: only the caller's OWN chats are matched, so a foreign - * pageId reveals nothing. + * { chatId: null } when the page has no owned chat (-> a fresh chat). + * + * `dto.pageId` carries EITHER a page slugId (10-char nanoid, sent by the client + * off a slug URL) OR a page uuid, so it must be resolved to a real page uuid + * before it touches the uuid ai_chats.page_id column — passing a slugId straight + * through triggered a Postgres 22P02 "invalid input syntax for type uuid" 500 + * (#312). PageRepo.findById accepts both forms. The workspace guard rejects an + * unknown or cross-workspace page (-> { chatId: null }) so a foreign id cannot + * probe another workspace's chats. Only the caller's OWN chats are then matched. */ @HttpCode(HttpStatus.OK) @Post('bound-chat') @@ -82,10 +90,14 @@ export class AiChatController { @AuthUser() user: User, @AuthWorkspace() workspace: Workspace, ): Promise<{ chatId: string | null }> { + const page = await this.pageRepo.findById(dto.pageId); // accepts slugId OR uuid + if (!page || page.workspaceId !== workspace.id) { + return { chatId: null }; // unknown or foreign-workspace page — no binding, no leak + } const chat = await this.aiChatRepo.findLatestByPage( user.id, workspace.id, - dto.pageId, + page.id, // the real uuid, never the incoming slugId ); return { chatId: chat?.id ?? null }; } diff --git a/apps/server/src/core/ai-chat/ai-chat.generate-page-title.spec.ts b/apps/server/src/core/ai-chat/ai-chat.generate-page-title.spec.ts index ef242fdf..d5f6906b 100644 --- a/apps/server/src/core/ai-chat/ai-chat.generate-page-title.spec.ts +++ b/apps/server/src/core/ai-chat/ai-chat.generate-page-title.spec.ts @@ -60,6 +60,7 @@ describe('AiChatController.generatePageTitle', () => { {} as never, {} as never, {} as never, + {} as never, ); return { controller, aiChatService }; } -- 2.52.0