From 85db20f9f29c8495f4c4297774cbe9a1e7958460 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 21 Jun 2026 05:39:35 +0300 Subject: [PATCH 01/12] test(page): cover movePage server-side cycle guard (#102) Adds the missing tests for the #67 guard: self-move and a destination inside the moved page's subtree both throw BadRequestException before updatePage; a legitimate move proceeds. Mocks pageRepo + spies getPageBreadCrumbs. Co-Authored-By: Claude Opus 4.8 --- .../core/page/services/page.service.spec.ts | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/apps/server/src/core/page/services/page.service.spec.ts b/apps/server/src/core/page/services/page.service.spec.ts index ec3a39db..f923c432 100644 --- a/apps/server/src/core/page/services/page.service.spec.ts +++ b/apps/server/src/core/page/services/page.service.spec.ts @@ -1,4 +1,7 @@ +import { BadRequestException } from '@nestjs/common'; import { PageService } from './page.service'; +import { MovePageDto } from '../dto/move-page.dto'; +import { Page } from '@docmost/db/types/entity.types'; // Direct instantiation with stub deps. The Test.createTestingModule form failed // to resolve the @InjectKysely()/@InjectQueue() tokens at compile(), and this @@ -26,4 +29,122 @@ describe('PageService', () => { it('should be defined', () => { expect(service).toBeDefined(); }); + + describe('movePage cycle guard (#67)', () => { + // A valid fractional-indexing key — movePage validates `position` by feeding + // it to generateJitteredKeyBetween(position, null) before anything else. + const VALID_POSITION = 'a0'; + const SPACE_ID = 'space-1'; + + // Build a PageService whose pageRepo (findById/updatePage) and own + // getPageBreadCrumbs are mockable, while every other collaborator stays a + // bare stub. We only need to drive the three cycle-guard branches, so we + // mock minimally rather than standing up the whole DI graph. + const makeService = (overrides?: { + breadcrumbs?: Array<{ id: string }>; + }) => { + const pageRepo = { + // Destination parent lookup: a valid, non-deleted, same-space page. + findById: jest.fn().mockResolvedValue({ + id: 'dest-parent', + deletedAt: null, + spaceId: SPACE_ID, + }), + // numUpdatedRows must be 1n so the #64 phantom-broadcast gate passes and + // movePage proceeds to emit PAGE_MOVED instead of early-returning. + updatePage: jest.fn().mockResolvedValue({ numUpdatedRows: 1n }), + }; + + const eventEmitter = { emit: jest.fn() }; + + const svc = new PageService( + pageRepo as any, // pageRepo + {} as any, // pagePermissionRepo + {} as any, // attachmentRepo + {} as any, // db + {} as any, // storageService + {} as any, // attachmentQueue + {} as any, // aiQueue + {} as any, // generalQueue + eventEmitter as any, // eventEmitter + {} as any, // collaborationGateway + {} as any, // watcherService + {} as any, // transclusionService + ); + + // getPageBreadCrumbs is a method on PageService itself (it runs a recursive + // ancestor CTE against the db). Spy on the instance method so we can return + // a synthetic ancestor chain without a real database. + jest + .spyOn(svc, 'getPageBreadCrumbs') + .mockResolvedValue((overrides?.breadcrumbs ?? []) as any); + + return { svc, pageRepo, eventEmitter }; + }; + + // movePage takes `movedPage` as a param. Keep its parentPageId distinct from + // the dto's parentPageId so the re-parent branch (and thus the cycle guard) + // actually runs instead of short-circuiting to a same-parent reorder. + const makeMovedPage = (): Page => + ({ + id: 'page-1', + parentPageId: 'old-parent', + spaceId: SPACE_ID, + workspaceId: 'ws-1', + slugId: 'slug-1', + title: 'Page 1', + icon: null, + }) as any; + + it('rejects a self-move (parentPageId === pageId) without updating', async () => { + const { svc, pageRepo } = makeService(); + const dto: MovePageDto = { + pageId: 'page-1', + position: VALID_POSITION, + parentPageId: 'page-1', // moving the page into itself + }; + + await expect(svc.movePage(dto, makeMovedPage())).rejects.toThrow( + BadRequestException, + ); + expect(pageRepo.updatePage).not.toHaveBeenCalled(); + }); + + it('rejects moving a page into its own subtree (cycle) before updating', async () => { + // Destination's ancestor chain includes the page being moved -> the + // destination lives inside the moved page's subtree -> cycle. + const { svc, pageRepo } = makeService({ + breadcrumbs: [ + { id: 'dest-parent' }, + { id: 'page-1' }, // the moved page appears among the destination's ancestors + { id: 'root' }, + ], + }); + const dto: MovePageDto = { + pageId: 'page-1', + position: VALID_POSITION, + parentPageId: 'dest-parent', + }; + + await expect(svc.movePage(dto, makeMovedPage())).rejects.toThrow( + BadRequestException, + ); + expect(pageRepo.updatePage).not.toHaveBeenCalled(); + }); + + it('allows a legitimate move when the destination is not in the subtree', async () => { + // Destination's ancestor chain does NOT contain the moved page -> no cycle. + const { svc, pageRepo } = makeService({ + breadcrumbs: [{ id: 'dest-parent' }, { id: 'root' }], + }); + const dto: MovePageDto = { + pageId: 'page-1', + position: VALID_POSITION, + parentPageId: 'dest-parent', + }; + + await expect(svc.movePage(dto, makeMovedPage())).resolves.not.toThrow(); + expect(pageRepo.updatePage).toHaveBeenCalledTimes(1); + }); + }); }); From 33c52045a24abfed88c1f7b53f7c2c617ba8f75c Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 21 Jun 2026 05:39:35 +0300 Subject: [PATCH 02/12] test(share-ai): drive the non-text message-part 400 path (#103) Covers the #63 guard: a message with a non-text part -> 400 'Unsupported message content'; a message mixing text + a non-text part still 400s (before the 413 size check). Co-Authored-By: Claude Opus 4.8 --- .../public-share-chat.controller.spec.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/apps/server/src/core/ai-chat/public-share-chat.controller.spec.ts b/apps/server/src/core/ai-chat/public-share-chat.controller.spec.ts index 13c19f8b..08b20b43 100644 --- a/apps/server/src/core/ai-chat/public-share-chat.controller.spec.ts +++ b/apps/server/src/core/ai-chat/public-share-chat.controller.spec.ts @@ -209,6 +209,56 @@ describe('resolveShareAssistantRequest (extracted controller funnel)', () => { expect(await statusOf(deps, body({ messages: [huge] }))).toBe(413); }); + it('a message with a non-text part => 400 Unsupported message content', async () => { + // The anonymous path runs no tools, so a client-supplied tool/file/data part + // is never legitimate and is rejected before it can reach the model context. + const { deps } = makeDeps(); + const nonText = { + role: 'user', + parts: [{ type: 'tool-call' }], + }; + let caught: HttpException | null = null; + try { + await resolveShareAssistantRequest(deps, { + workspaceId: 'ws-1', + body: body({ messages: [nonText] }) as never, + }); + } catch (err) { + caught = err instanceof HttpException ? err : null; + } + expect(caught).toBeInstanceOf(HttpException); + expect(caught!.getStatus()).toBe(400); + expect(caught!.message).toBe('Unsupported message content'); + }); + + it('a message mixing a text part AND a non-text part => still 400 (rejected before the 413 size check)', async () => { + // A forged non-text part smuggled alongside a legit text part is still + // rejected: the non-text guard runs BEFORE the char-cap (413) check, so even + // an over-long mixed message surfaces the 400, not the size error. + const { deps } = makeDeps(); + const mixed = { + role: 'user', + parts: [ + { type: 'text', text: 'x'.repeat(MAX_SHARE_MESSAGE_CHARS + 1) }, + { type: 'tool-call' }, + ], + }; + let caught: HttpException | null = null; + try { + await resolveShareAssistantRequest(deps, { + workspaceId: 'ws-1', + body: body({ messages: [mixed] }) as never, + }); + } catch (err) { + caught = err instanceof HttpException ? err : null; + } + expect(caught).toBeInstanceOf(HttpException); + // The non-text guard wins over the 413 size cap even though the text part + // alone would exceed MAX_SHARE_MESSAGE_CHARS. + expect(caught!.getStatus()).toBe(400); + expect(caught!.message).toBe('Unsupported message content'); + }); + it('the quota gate is checked BEFORE the payload caps (429 wins over 413)', async () => { // Over-cap workspace AND an over-long message: the 429 must surface first, so // an over-cap caller is rejected without even paying the payload-cap scan. From ec4622a1b8cb65c87672559020631d715e86d651 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 21 Jun 2026 05:39:35 +0300 Subject: [PATCH 03/12] test(security): export + unit-test resolveTrustProxy (#105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relocate resolveTrustProxy from main.ts (untestable — bootstraps on import) to integrations/environment/trust-proxy.util.ts and import it back. Unit-test every branch (empty/undefined -> safe loopback/private default; true/false; hop count; trim; CIDR/negative passthrough) so a regression can't silently re-open the XFF spoofing hole (#61). Co-Authored-By: Claude Opus 4.8 --- .../environment/trust-proxy.util.spec.ts | 50 +++++++++++++++++++ .../environment/trust-proxy.util.ts | 14 ++++++ apps/server/src/main.ts | 14 +----- 3 files changed, 65 insertions(+), 13 deletions(-) create mode 100644 apps/server/src/integrations/environment/trust-proxy.util.spec.ts create mode 100644 apps/server/src/integrations/environment/trust-proxy.util.ts diff --git a/apps/server/src/integrations/environment/trust-proxy.util.spec.ts b/apps/server/src/integrations/environment/trust-proxy.util.spec.ts new file mode 100644 index 00000000..294c6fba --- /dev/null +++ b/apps/server/src/integrations/environment/trust-proxy.util.spec.ts @@ -0,0 +1,50 @@ +import { resolveTrustProxy } from './trust-proxy.util'; + +/** + * Unit tests for resolveTrustProxy: the helper that turns the TRUST_PROXY env + * string into a Fastify trustProxy value. The contract is: empty/undefined + * falls back to the safe loopback/linklocal/uniquelocal default (so a public-IP + * client cannot spoof X-Forwarded-For); 'true'/'false' become booleans; a + * non-negative integer becomes a hop count (number); anything else (CIDR/IP + * lists, negative numbers, named keywords) is passed through verbatim as a + * trimmed string. + */ +describe('resolveTrustProxy', () => { + const SAFE_DEFAULT = 'loopback, linklocal, uniquelocal'; + + it('returns the safe default for an empty string', () => { + expect(resolveTrustProxy('')).toBe(SAFE_DEFAULT); + }); + + it('returns the safe default for undefined', () => { + expect(resolveTrustProxy(undefined)).toBe(SAFE_DEFAULT); + }); + + it("returns the boolean true for 'true'", () => { + expect(resolveTrustProxy('true')).toBe(true); + }); + + it("returns the boolean false for 'false'", () => { + expect(resolveTrustProxy('false')).toBe(false); + }); + + it("returns the number 2 for '2'", () => { + expect(resolveTrustProxy('2')).toBe(2); + }); + + it("trims surrounding whitespace and returns the number 3 for ' 3 '", () => { + expect(resolveTrustProxy(' 3 ')).toBe(3); + }); + + it('passes a CIDR string through unchanged', () => { + expect(resolveTrustProxy('10.0.0.0/8')).toBe('10.0.0.0/8'); + }); + + it("passes a negative number through as a string ('-1' is not a valid hop count)", () => { + expect(resolveTrustProxy('-1')).toBe('-1'); + }); + + it('passes a non-numeric keyword through unchanged', () => { + expect(resolveTrustProxy('loopback')).toBe('loopback'); + }); +}); diff --git a/apps/server/src/integrations/environment/trust-proxy.util.ts b/apps/server/src/integrations/environment/trust-proxy.util.ts new file mode 100644 index 00000000..176e0654 --- /dev/null +++ b/apps/server/src/integrations/environment/trust-proxy.util.ts @@ -0,0 +1,14 @@ +// Trust X-Forwarded-For ONLY from real proxies on private/loopback nets by +// default, so a public-IP client cannot spoof its IP via X-Forwarded-For. +// TRUST_PROXY env overrides: 'true'/'false', a hop count (integer), or a +// CIDR/IP list string passed through to Fastify/proxy-addr. +export function resolveTrustProxy( + rawInput?: string, +): boolean | number | string { + const raw = rawInput?.trim(); + if (raw == null || raw === '') return 'loopback, linklocal, uniquelocal'; + if (raw === 'true') return true; + if (raw === 'false') return false; + const n = Number(raw); + return Number.isInteger(n) && n >= 0 ? n : raw; +} diff --git a/apps/server/src/main.ts b/apps/server/src/main.ts index 3588f045..05968d09 100644 --- a/apps/server/src/main.ts +++ b/apps/server/src/main.ts @@ -14,19 +14,7 @@ import fastifyIp from 'fastify-ip'; import { InternalLogFilter } from './common/logger/internal-log-filter'; import { EnvironmentService } from './integrations/environment/environment.service'; import { resolveFrameHeader } from './common/helpers'; - -// Trust X-Forwarded-For ONLY from real proxies on private/loopback nets by -// default, so a public-IP client cannot spoof its IP via X-Forwarded-For. -// TRUST_PROXY env overrides: 'true'/'false', a hop count (integer), or a -// CIDR/IP list string passed through to Fastify/proxy-addr. -function resolveTrustProxy(rawInput?: string): boolean | number | string { - const raw = rawInput?.trim(); - if (raw == null || raw === '') return 'loopback, linklocal, uniquelocal'; - if (raw === 'true') return true; - if (raw === 'false') return false; - const n = Number(raw); - return Number.isInteger(n) && n >= 0 ? n : raw; -} +import { resolveTrustProxy } from './integrations/environment/trust-proxy.util'; async function bootstrap() { const app = await NestFactory.create( From e8775c45b0f2467574a4bb633524bd21d23079c7 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 21 Jun 2026 05:39:35 +0300 Subject: [PATCH 04/12] test(ai-chat): cover the conditional assistant-name signature (#108) Extract the shared assistant-name predicate (resolveAssistantName: trimmed name or null) used by typing-indicator + message-item, and unit-test the branches (name shown; whitespace-only -> 'AI agent' fallback; undefined -> fallback). Behavior-identical (|| -> ?? since the helper returns null). Co-Authored-By: Claude Opus 4.8 --- .../ai-chat/components/message-item.tsx | 3 ++- .../ai-chat/components/typing-indicator.tsx | 5 ++-- .../ai-chat/utils/assistant-name.test.ts | 24 +++++++++++++++++++ .../features/ai-chat/utils/assistant-name.ts | 16 +++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 apps/client/src/features/ai-chat/utils/assistant-name.test.ts create mode 100644 apps/client/src/features/ai-chat/utils/assistant-name.ts diff --git a/apps/client/src/features/ai-chat/components/message-item.tsx b/apps/client/src/features/ai-chat/components/message-item.tsx index a0c37089..2feaa8de 100644 --- a/apps/client/src/features/ai-chat/components/message-item.tsx +++ b/apps/client/src/features/ai-chat/components/message-item.tsx @@ -5,6 +5,7 @@ import type { UIMessage } from "@ai-sdk/react"; import ToolCallCard from "@/features/ai-chat/components/tool-call-card.tsx"; import { ToolUiPart, isToolPart } from "@/features/ai-chat/utils/tool-parts.tsx"; import { renderChatMarkdown } from "@/features/ai-chat/utils/markdown.ts"; +import { resolveAssistantName } from "@/features/ai-chat/utils/assistant-name.ts"; import { describeChatError } from "@/features/ai-chat/utils/error-message.ts"; import classes from "@/features/ai-chat/components/ai-chat.module.css"; @@ -67,7 +68,7 @@ export default function MessageItem({ return ( - {assistantName?.trim() || t("AI agent")} + {resolveAssistantName(assistantName) ?? t("AI agent")} {message.parts.map((part, index) => { if (part.type === "text") { diff --git a/apps/client/src/features/ai-chat/components/typing-indicator.tsx b/apps/client/src/features/ai-chat/components/typing-indicator.tsx index ba6c6db0..27373a3e 100644 --- a/apps/client/src/features/ai-chat/components/typing-indicator.tsx +++ b/apps/client/src/features/ai-chat/components/typing-indicator.tsx @@ -1,5 +1,6 @@ import { Box, Group, Text } from "@mantine/core"; import { useTranslation } from "react-i18next"; +import { resolveAssistantName } from "@/features/ai-chat/utils/assistant-name.ts"; import classes from "@/features/ai-chat/components/ai-chat.module.css"; interface TypingIndicatorProps { @@ -23,12 +24,12 @@ interface TypingIndicatorProps { */ export default function TypingIndicator({ assistantName }: TypingIndicatorProps) { const { t } = useTranslation(); - const name = assistantName?.trim(); + const name = resolveAssistantName(assistantName); return ( - {name || t("AI agent")} + {name ?? t("AI agent")}