From 90d3fab4835b4ddf8dd6e2fa05d6b4928d11fae7 Mon Sep 17 00:00:00 2001 From: claude_code Date: Sat, 20 Jun 2026 23:40:40 +0300 Subject: [PATCH] test: cover features since 053a9c0d + repair test tooling Add ~330 tests across server (Jest), client (Vitest), editor-ext (Vitest) and packages/mcp (node:test) for the gitmost features added since 053a9c0d: AI chat, AI agent roles, public-share assistant, MCP per-user auth, HTML embed, page templates/embed, realtime tree, tree expand/collapse, and the AI-settings UI. Test-tooling fixes (prerequisite, were silently hiding coverage): - Repair 3 page-template specs broken by the 11-arg TransclusionService constructor; they never compiled, so template access-control / content -leak / unsync-strip coverage was fictitious. - Build @docmost/editor-ext before server tests via a `pretest` hook; the stale dist omitted the new HtmlEmbed/PageEmbed exports (TS2305). - Let jest resolve the .tsx email templates: add `tsx` to moduleFileExtensions and widen the ts-jest transform to (t|j)sx?. Behaviour-preserving "extract pure core" refactors that the tests drive: - server: resolveShareAssistantRequest + uiMessageTextLength (public-share controller), decideBasicGate + mapAuthResultToResponse (mcp), buildErrorAssistantRecord (ai-chat), jsonbObject export (roles). - client: render-raw-html + shouldExecute/canEdit, decide-embed-state, page-embed picker utils, tree-socket reducers, open/close branch maps, isEndpointConfigured/resolveKeyField; buildTreeWithChildren now treats a permission-trimmed orphan as a root instead of crashing. Deferred (need a test DB or HTTP harness, documented in the specs): repo-level Postgres integration tests and the public-share XFF E2E. Pre-existing DI/lib0-ESM suite failures are untouched and out of scope. Co-Authored-By: Claude Opus 4.8 --- .../ai-chat/utils/error-message.test.ts | 53 +++ .../ai-chat/utils/tool-parts.test.tsx | 100 ++++++ .../components/html-embed/html-embed-view.tsx | 45 +-- .../html-embed/render-raw-html.test.ts | 112 ++++++ .../components/html-embed/render-raw-html.ts | 73 ++++ .../page-embed/decide-embed-state.test.ts | 141 ++++++++ .../page-embed/decide-embed-state.ts | 58 ++++ .../page-embed-ancestry-context.test.tsx | 71 ++++ .../page-embed-lookup-context.test.tsx | 162 +++++++++ .../page-embed/page-embed-picker.tsx | 13 +- .../page-embed-picker.utils.test.ts | 43 +++ .../page-embed/page-embed-picker.utils.ts | 27 ++ .../components/page-embed/page-embed-view.tsx | 32 +- .../components/space-tree.expand-all.test.tsx | 228 ++++++++++++ .../page/tree/components/space-tree.tsx | 14 +- .../page/tree/model/tree-model.test.ts | 52 +++ .../features/page/tree/utils/utils.test.ts | 237 ++++++++++++- .../src/features/page/tree/utils/utils.ts | 37 +- .../websocket/tree-socket-reducers.test.ts | 264 ++++++++++++++ .../websocket/tree-socket-reducers.ts | 164 +++++++++ .../src/features/websocket/use-tree-socket.ts | 140 +------- .../components/ai-provider-settings.spec.tsx | 55 ++- .../components/ai-provider-settings.tsx | 65 ++-- apps/server/package.json | 6 +- .../html-embed-import-detect.spec.ts | 70 ++++ .../helpers/prosemirror/html-embed.spec.ts | 96 +++++ .../src/core/ai-chat/ai-chat.service.spec.ts | 30 ++ .../src/core/ai-chat/ai-chat.service.ts | 26 +- .../public-share-chat.controller.spec.ts | 256 ++++++++++++++ .../ai-chat/public-share-chat.controller.ts | 327 ++++++++++-------- .../core/ai-chat/public-share-chat.spec.ts | 109 +++++- .../roles/ai-agent-roles.service.spec.ts | 117 +++++++ .../core/ai-chat/roles/jsonb-object.spec.ts | 30 ++ .../roles/role-override-contract.spec.ts | 135 ++++++++ .../public-share-chat-tools.service.spec.ts | 132 +++++++ .../verify-user-credentials.live.spec.ts | 233 +++++++++++++ .../transclusion/spec/page-embed.util.spec.ts | 61 ++++ .../spec/page-template-access.spec.ts | 278 ++++++++++++++- .../spec/page-template-lookup.spec.ts | 59 ++++ .../spec/page-template.controller.spec.ts | 51 +++ .../transclusion-unsync-html-embed.spec.ts | 1 + .../src/core/share/share-html-embed.spec.ts | 128 +++++++ .../services/workspace-html-embed.spec.ts | 111 ++++++ .../ai-agent-roles/ai-agent-roles.repo.ts | 2 +- .../src/integrations/ai/ai-error.util.spec.ts | 22 ++ .../src/integrations/ai/ai.service.spec.ts | 113 ++++++ .../src/integrations/mcp/mcp-auth.helpers.ts | 105 ++++++ .../src/integrations/mcp/mcp.service.spec.ts | 195 +++++++++++ .../src/integrations/mcp/mcp.service.ts | 152 ++++---- .../src/ws/listeners/page-ws.listener.spec.ts | 137 ++++++++ apps/server/src/ws/ws-service.spec.ts | 259 ++++++++++++++ apps/server/src/ws/ws-tree.service.spec.ts | 106 ++++++ .../lib/html-embed/html-embed-codec.spec.ts | 116 +++++++ .../lib/markdown/html-embed-marked.spec.ts | 105 ++++++ .../src/lib/page-embed/page-embed.spec.ts | 88 +++++ .../mcp/test/unit/http-idle-eviction.test.mjs | 273 +++++++++++++++ 56 files changed, 5668 insertions(+), 447 deletions(-) create mode 100644 apps/client/src/features/ai-chat/utils/error-message.test.ts create mode 100644 apps/client/src/features/ai-chat/utils/tool-parts.test.tsx create mode 100644 apps/client/src/features/editor/components/html-embed/render-raw-html.test.ts create mode 100644 apps/client/src/features/editor/components/html-embed/render-raw-html.ts create mode 100644 apps/client/src/features/editor/components/page-embed/decide-embed-state.test.ts create mode 100644 apps/client/src/features/editor/components/page-embed/decide-embed-state.ts create mode 100644 apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.test.tsx create mode 100644 apps/client/src/features/editor/components/page-embed/page-embed-lookup-context.test.tsx create mode 100644 apps/client/src/features/editor/components/page-embed/page-embed-picker.utils.test.ts create mode 100644 apps/client/src/features/editor/components/page-embed/page-embed-picker.utils.ts create mode 100644 apps/client/src/features/page/tree/components/space-tree.expand-all.test.tsx create mode 100644 apps/client/src/features/websocket/tree-socket-reducers.test.ts create mode 100644 apps/client/src/features/websocket/tree-socket-reducers.ts create mode 100644 apps/server/src/common/helpers/prosemirror/html-embed-import-detect.spec.ts create mode 100644 apps/server/src/core/ai-chat/public-share-chat.controller.spec.ts create mode 100644 apps/server/src/core/ai-chat/roles/jsonb-object.spec.ts create mode 100644 apps/server/src/core/ai-chat/roles/role-override-contract.spec.ts create mode 100644 apps/server/src/core/ai-chat/tools/public-share-chat-tools.service.spec.ts create mode 100644 apps/server/src/core/auth/services/verify-user-credentials.live.spec.ts create mode 100644 apps/server/src/core/workspace/services/workspace-html-embed.spec.ts create mode 100644 apps/server/src/ws/ws-service.spec.ts create mode 100644 packages/editor-ext/src/lib/html-embed/html-embed-codec.spec.ts create mode 100644 packages/editor-ext/src/lib/markdown/html-embed-marked.spec.ts create mode 100644 packages/editor-ext/src/lib/page-embed/page-embed.spec.ts create mode 100644 packages/mcp/test/unit/http-idle-eviction.test.mjs diff --git a/apps/client/src/features/ai-chat/utils/error-message.test.ts b/apps/client/src/features/ai-chat/utils/error-message.test.ts new file mode 100644 index 00000000..83d52b3c --- /dev/null +++ b/apps/client/src/features/ai-chat/utils/error-message.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect } from "vitest"; +import { describeChatError } from "./error-message"; + +// Identity translator: assert on the raw English key so the tests do not depend +// on the i18n catalog. +const t = (key: string) => key; + +describe("describeChatError", () => { + it('surfaces a provider "402: ..." stream error verbatim', () => { + expect(describeChatError("402: Insufficient credits", t)).toBe( + "402: Insufficient credits", + ); + }); + + it('does NOT misclassify a body that merely contains "403" (no "statusCode":403)', () => { + // A provider message mentioning the number 403 must be surfaced verbatim, + // never folded into the "AI chat is disabled" gating message. + const msg = "429: rate limited after 403 attempts"; + expect(describeChatError(msg, t)).toBe(msg); + }); + + it('maps a {"statusCode":403} body to the disabled message', () => { + const body = '{"statusCode":403,"message":"Forbidden"}'; + expect(describeChatError(body, t)).toBe( + "AI chat is disabled for this workspace.", + ); + }); + + it('maps a {"statusCode":503} body to the not-configured message', () => { + const body = '{"statusCode":503,"message":"Service Unavailable"}'; + expect(describeChatError(body, t)).toBe( + "The AI provider is not configured. Ask an administrator to set it up.", + ); + }); + + it('falls back to the generic message for "An error occurred."', () => { + expect(describeChatError("An error occurred.", t)).toBe( + "The AI agent could not respond. Please try again.", + ); + }); + + it('falls back to the generic message for "Internal server error"', () => { + expect(describeChatError("Internal server error", t)).toBe( + "The AI agent could not respond. Please try again.", + ); + }); + + it("falls back to the generic message for empty input", () => { + expect(describeChatError("", t)).toBe( + "The AI agent could not respond. Please try again.", + ); + }); +}); diff --git a/apps/client/src/features/ai-chat/utils/tool-parts.test.tsx b/apps/client/src/features/ai-chat/utils/tool-parts.test.tsx new file mode 100644 index 00000000..f3c3bd4c --- /dev/null +++ b/apps/client/src/features/ai-chat/utils/tool-parts.test.tsx @@ -0,0 +1,100 @@ +import { describe, it, expect } from "vitest"; +import { + toolCitations, + toolRunState, + type ToolUiPart, +} from "./tool-parts"; + +describe("toolCitations", () => { + it("emits one citation per searchPages item with a /p/{id} href", () => { + const part: ToolUiPart = { + type: "tool-searchPages", + state: "output-available", + output: [ + { id: "p1", title: "First" }, + { id: "p2", title: "Second" }, + ], + }; + expect(toolCitations(part)).toEqual([ + { pageId: "p1", title: "First", href: "/p/p1" }, + { pageId: "p2", title: "Second", href: "/p/p2" }, + ]); + }); + + it("drops searchPages items missing an id", () => { + const part: ToolUiPart = { + type: "tool-searchPages", + state: "output-available", + output: [{ title: "No id here" }, { id: "p2", title: "Kept" }], + }; + expect(toolCitations(part)).toEqual([ + { pageId: "p2", title: "Kept", href: "/p/p2" }, + ]); + }); + + it("falls back to input.pageId / input.title for a page-op with only pageId", () => { + // The mutating tools echo `pageId` (no `id`); title is taken from the input. + const part: ToolUiPart = { + type: "tool-updatePageContent", + state: "output-available", + input: { pageId: "host-1", title: "From input" }, + output: { pageId: "host-1" }, + }; + expect(toolCitations(part)).toEqual([ + { pageId: "host-1", title: "From input", href: "/p/host-1" }, + ]); + }); + + it("prefers output.id over input.pageId when both exist", () => { + const part: ToolUiPart = { + type: "tool-getPage", + state: "output-available", + input: { pageId: "input-id", title: "Input title" }, + output: { id: "output-id", title: "Output title" }, + }; + expect(toolCitations(part)).toEqual([ + { pageId: "output-id", title: "Output title", href: "/p/output-id" }, + ]); + }); + + it("returns [] when the state is not output-available", () => { + const part: ToolUiPart = { + type: "tool-getPage", + state: "input-available", + output: { id: "p1", title: "Pending" }, + }; + expect(toolCitations(part)).toEqual([]); + }); + + it("returns [] for a page-op output with no resolvable id", () => { + const part: ToolUiPart = { + type: "tool-getPage", + state: "output-available", + input: {}, + output: { title: "Only a title" }, + }; + expect(toolCitations(part)).toEqual([]); + }); +}); + +describe("toolRunState", () => { + it('maps "output-error" to error', () => { + expect(toolRunState("output-error")).toBe("error"); + }); + + it('maps "output-denied" to error', () => { + expect(toolRunState("output-denied")).toBe("error"); + }); + + it('maps "output-available" to done', () => { + expect(toolRunState("output-available")).toBe("done"); + }); + + it('maps "input-available" to running', () => { + expect(toolRunState("input-available")).toBe("running"); + }); + + it("maps undefined to running", () => { + expect(toolRunState(undefined)).toBe("running"); + }); +}); diff --git a/apps/client/src/features/editor/components/html-embed/html-embed-view.tsx b/apps/client/src/features/editor/components/html-embed/html-embed-view.tsx index a46b383a..273fbaff 100644 --- a/apps/client/src/features/editor/components/html-embed/html-embed-view.tsx +++ b/apps/client/src/features/editor/components/html-embed/html-embed-view.tsx @@ -15,39 +15,11 @@ import { useAtomValue } from "jotai"; import useUserRole from "@/hooks/use-user-role.tsx"; import { workspaceAtom } from "@/features/user/atoms/current-user-atom.ts"; import classes from "./html-embed-view.module.css"; - -/** - * Inject raw HTML (including ", + ); + // The re-created inline script ran inside the jsdom window. + expect((dom.window as unknown as Record).__htmlEmbedFlag).toBe( + true, + ); + // The non-script markup is preserved. + expect(container.querySelector("div")?.textContent).toBe("hello"); + }); + + it("copies src/async/defer onto a re-created external ', + ); + const script = container.querySelector("script"); + expect(script).not.toBeNull(); + expect(script?.getAttribute("src")).toBe("https://example.com/t.js"); + expect(script?.hasAttribute("async")).toBe(true); + expect(script?.hasAttribute("defer")).toBe(true); + }); + + it("clears the container when the source is empty", () => { + container.innerHTML = "

stale

"; + renderRawHtml(container, ""); + expect(container.innerHTML).toBe(""); + }); + + it("clears prior content first on a re-render with new source", () => { + const win = dom.window as unknown as Record; + renderRawHtml( + container, + "one", + ); + expect(win.__htmlEmbedCount).toBe(1); + expect(container.querySelector("#first")).not.toBeNull(); + + renderRawHtml( + container, + "two", + ); + // Prior content is gone; only the new render remains. + expect(container.querySelector("#first")).toBeNull(); + expect(container.querySelector("#second")).not.toBeNull(); + expect(win.__htmlEmbedCount).toBe(2); + }); +}); + +describe("shouldExecute (execution policy)", () => { + it("read-only executes regardless of the workspace toggle", () => { + // isEditable=false → the server already gated the content. + expect(shouldExecute(false, false)).toBe(true); + expect(shouldExecute(false, true)).toBe(true); + }); + + it("editable + toggle OFF does NOT execute", () => { + expect(shouldExecute(true, false)).toBe(false); + }); + + it("editable + toggle ON executes", () => { + expect(shouldExecute(true, true)).toBe(true); + }); +}); + +describe("canEdit (edit policy)", () => { + it("a member (non-admin) can never edit", () => { + expect(canEdit(true, false, true)).toBe(false); + expect(canEdit(false, false, true)).toBe(false); + }); + + it("an admin with the toggle OFF cannot edit", () => { + expect(canEdit(true, true, false)).toBe(false); + }); + + it("an admin with the toggle ON in editable mode can edit", () => { + expect(canEdit(true, true, true)).toBe(true); + }); + + it("an admin in read-only mode cannot edit (no edit affordance)", () => { + expect(canEdit(false, true, true)).toBe(false); + }); +}); diff --git a/apps/client/src/features/editor/components/html-embed/render-raw-html.ts b/apps/client/src/features/editor/components/html-embed/render-raw-html.ts new file mode 100644 index 00000000..1b035aa6 --- /dev/null +++ b/apps/client/src/features/editor/components/html-embed/render-raw-html.ts @@ -0,0 +1,73 @@ +/** + * Pure DOM helpers for the HTML embed node view. Kept out of the React + * component so the script re-creation/execution mechanism and the execution/ + * edit policy can be unit-tested against a bare jsdom container with no + * Tiptap/Mantine providers. + */ + +/** + * Inject raw HTML (including '; + const encoded = encodeHtmlEmbedSource(source); + const md = [ + 'Hello', + '', + `
`, + '', + 'World', + ].join('\n'); + + const html = await markdownToHtml(md); + // marked preserves the raw block-level div verbatim. + expect(html).toContain('data-type="htmlEmbed"'); + + const json = htmlToJson(html); + // The div parses into a real htmlEmbed node carrying the decoded source. + expect(hasHtmlEmbedNode(json)).toBe(true); + + // Because it is detected, the write-path gate can strip it for non-admins. + const stripped = stripHtmlEmbedNodes(json); + expect(hasHtmlEmbedNode(stripped)).toBe(false); + // Surrounding non-embed content is retained. + expect(JSON.stringify(stripped)).toContain('Hello'); + expect(JSON.stringify(stripped)).toContain('World'); + }); + + it('round-trips through direct HTML conversion (htmlToJson) and is DETECTED', () => { + const source = ''; + const encoded = encodeHtmlEmbedSource(source); + const html = `

Hello

World

`; + + const json = htmlToJson(html); + expect(hasHtmlEmbedNode(json)).toBe(true); + expect(hasHtmlEmbedNode(stripHtmlEmbedNodes(json))).toBe(false); + }); + + it('is still DETECTED even when the data-source is NOT valid base64', async () => { + // A naive raw inline source (HTML-escaped, not base64) still parses as an + // htmlEmbed NODE — the decoder just yields an empty source. Detection (and + // therefore stripping) does not depend on the source being well-formed, so + // the bypass cannot be hidden by sending a malformed data-source. + const md = `
`; + const html = await markdownToHtml(md); + const json = htmlToJson(html); + expect(hasHtmlEmbedNode(json)).toBe(true); + expect(hasHtmlEmbedNode(stripHtmlEmbedNodes(json))).toBe(false); + }); +}); diff --git a/apps/server/src/common/helpers/prosemirror/html-embed.spec.ts b/apps/server/src/common/helpers/prosemirror/html-embed.spec.ts index 6b07ec0b..28a59ea3 100644 --- a/apps/server/src/common/helpers/prosemirror/html-embed.spec.ts +++ b/apps/server/src/common/helpers/prosemirror/html-embed.spec.ts @@ -92,6 +92,102 @@ describe('stripHtmlEmbedNodes', () => { const result = stripHtmlEmbedNodes(doc); expect(result).toEqual(doc); }); + + it('strips a deeply nested htmlEmbed (3+ levels: callout > column > paragraph-sibling)', () => { + // htmlEmbed sits as a sibling of a paragraph, nested four containers deep. + const doc = { + type: 'doc', + content: [ + { + type: 'callout', + content: [ + { + type: 'columns', + content: [ + { + type: 'column', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'deep keep' }], + }, + { type: 'htmlEmbed', attrs: { source: '' } }, + ], + }, + ], + }, + ], + }, + ], + }; + + const result = stripHtmlEmbedNodes(doc); + expect(hasHtmlEmbedNode(result)).toBe(false); + const col = findFirstChild(result, 'column'); + // Sibling paragraph survives; only the embed is removed. + expect(col.content).toHaveLength(1); + expect(col.content[0].type).toBe('paragraph'); + expect(col.content[0].content[0].text).toBe('deep keep'); + }); + + it('returns non-object / null / array-without-content nodes unchanged', () => { + // Non-object inputs are returned as-is (callers persist what they got). + expect(stripHtmlEmbedNodes(null as any)).toBeNull(); + expect(stripHtmlEmbedNodes(undefined as any)).toBeUndefined(); + expect(stripHtmlEmbedNodes('not-a-node' as any)).toBe('not-a-node'); + expect(stripHtmlEmbedNodes(42 as any)).toBe(42); + + // An object node with no `content` array is returned shallow-cloned, equal. + const leaf = { type: 'paragraph', attrs: { id: 'x' } }; + const out = stripHtmlEmbedNodes(leaf); + expect(out).toEqual(leaf); + expect(out).not.toBe(leaf); // new object, input not mutated + }); + + it('yields empty content (not null/undefined) for a doc whose only child is an htmlEmbed', () => { + const doc = { + type: 'doc', + content: [{ type: 'htmlEmbed', attrs: { source: 'only' } }], + }; + const result = stripHtmlEmbedNodes(doc) as any; + expect(Array.isArray(result.content)).toBe(true); + expect(result.content).toHaveLength(0); + expect(result.content).not.toBeNull(); + expect(result.content).not.toBeUndefined(); + expect(hasHtmlEmbedNode(result)).toBe(false); + }); +}); + +describe('hasHtmlEmbedNode (root/odd-shape detection)', () => { + it('returns true when the ROOT node itself is an htmlEmbed (not only a child)', () => { + const rootEmbed = { type: 'htmlEmbed', attrs: { source: '' } }; + expect(hasHtmlEmbedNode(rootEmbed)).toBe(true); + }); + + it('returns false for a doc with embed-like TEXT but no htmlEmbed node', () => { + // The literal string "htmlEmbed" appears only as text content, not as a + // node type, so it must NOT be detected. + const doc = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'type: htmlEmbed
' }, + ], + }, + ], + }; + expect(hasHtmlEmbedNode(doc)).toBe(false); + }); + + it('returns false for non-object / null / array inputs', () => { + expect(hasHtmlEmbedNode(null)).toBe(false); + expect(hasHtmlEmbedNode(undefined)).toBe(false); + expect(hasHtmlEmbedNode('htmlEmbed')).toBe(false); + // A bare array (no `content` wrapper) has no node `type`, so it's false. + expect(hasHtmlEmbedNode([{ type: 'htmlEmbed' }] as any)).toBe(false); + }); }); describe('canAuthorHtmlEmbed', () => { diff --git a/apps/server/src/core/ai-chat/ai-chat.service.spec.ts b/apps/server/src/core/ai-chat/ai-chat.service.spec.ts index 2756df77..b788646e 100644 --- a/apps/server/src/core/ai-chat/ai-chat.service.spec.ts +++ b/apps/server/src/core/ai-chat/ai-chat.service.spec.ts @@ -4,6 +4,7 @@ import { serializeSteps, rowToUiMessage, prepareAgentStep, + buildErrorAssistantRecord, MAX_AGENT_STEPS, FINAL_STEP_INSTRUCTION, } from './ai-chat.service'; @@ -229,3 +230,32 @@ describe('prepareAgentStep', () => { expect(atBoundary?.toolChoice).toBe('none'); }); }); + +/** + * Unit test for buildErrorAssistantRecord: the pure helper that shapes the + * assistant-message record persisted on a first-turn (or any) stream failure. + * The streamText onError callback builds the formatted error text via + * describeProviderError (tested separately) and hands it to this helper; pinning + * the record shape here covers the persist-assistant-on-error logic without + * having to seam streamText itself. + */ +describe('buildErrorAssistantRecord', () => { + it('records an empty turn with the error text in metadata (finishReason=error)', () => { + const rec = buildErrorAssistantRecord('401: Unauthorized'); + expect(rec).toEqual({ + text: '', + toolCalls: null, + metadata: { finishReason: 'error', parts: [], error: '401: Unauthorized' }, + }); + }); + + it('always produces empty text + empty parts so a failed turn is still recorded', () => { + const rec = buildErrorAssistantRecord('boom'); + // No partial text and no UI parts: the turn exists in history but renders as + // an error, with the cause preserved in metadata.error. + expect(rec.text).toBe(''); + expect(rec.metadata.parts).toEqual([]); + expect(rec.toolCalls).toBeNull(); + expect(rec.metadata.error).toBe('boom'); + }); +}); diff --git a/apps/server/src/core/ai-chat/ai-chat.service.ts b/apps/server/src/core/ai-chat/ai-chat.service.ts index 4c4bc6f4..f492ca03 100644 --- a/apps/server/src/core/ai-chat/ai-chat.service.ts +++ b/apps/server/src/core/ai-chat/ai-chat.service.ts @@ -384,11 +384,7 @@ export class AiChatService { this.logger.error(`AI chat stream error: ${errorText}`, e?.stack); // Persist whatever text we have (likely empty) so the turn is recorded, // and record the error text in metadata so it is visible in history. - await persistAssistant({ - text: '', - toolCalls: null, - metadata: { finishReason: 'error', parts: [], error: errorText }, - }); + await persistAssistant(buildErrorAssistantRecord(errorText)); await closeExternalClients(); }, onAbort: async ({ steps }) => { @@ -710,6 +706,26 @@ export function rowToUiMessage(row: AiChatMessage): Omit & { return { id: row.id, role, parts: parts as UIMessage['parts'] }; } +/** + * Build the assistant-message record persisted when a turn fails before any text + * is produced (the streamText onError path). Pure: it takes the formatted error + * text and returns the exact `{ text, toolCalls, metadata }` payload handed to + * persistAssistant, so the first-turn-failure recording shape is unit-testable + * without seaming streamText. The empty text + empty parts mean the failed turn + * is still recorded in history, with the provider cause visible in metadata. + */ +export function buildErrorAssistantRecord(errorText: string): { + text: string; + toolCalls: null; + metadata: { finishReason: 'error'; parts: []; error: string }; +} { + return { + text: '', + toolCalls: null, + metadata: { finishReason: 'error', parts: [], error: errorText }, + }; +} + /** * Reduce SDK step objects to a compact, JSON-serializable trace for the * `tool_calls` column. Stores only what the UI action-log and history need — 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 new file mode 100644 index 00000000..83f6252e --- /dev/null +++ b/apps/server/src/core/ai-chat/public-share-chat.controller.spec.ts @@ -0,0 +1,256 @@ +import { HttpException } from '@nestjs/common'; +import { + resolveShareAssistantRequest, + uiMessageTextLength, + type ShareAssistantDeps, +} from './public-share-chat.controller'; +import { AiNotConfiguredException } from '../../integrations/ai/ai-not-configured.exception'; +import { + MAX_SHARE_MESSAGES, + MAX_SHARE_MESSAGE_CHARS, +} from './public-share-chat.service'; +import type { UIMessage } from 'ai'; + +/** + * Unit tests for the extracted pre-hijack funnel (resolveShareAssistantRequest) + * and the exported size helper (uiMessageTextLength). The funnel order is + * security-relevant: the first failing gate must win, every failure must throw + * BEFORE any stream/hijack, and the access-shaped failures must all 404 (no + * existence leak). These exercise each branch with hand-rolled mocks — no Nest + * module graph, no DB. + */ +describe('resolveShareAssistantRequest (extracted controller funnel)', () => { + /** A fully-passing dep set; individual tests override single collaborators. */ + function makeDeps(over: { + assistantEnabled?: boolean; + getShareForPage?: jest.Mock; + isSharingAllowed?: jest.Mock; + findById?: jest.Mock; + hasRestrictedAncestor?: jest.Mock; + resolveShareRole?: jest.Mock; + getShareChatModel?: jest.Mock; + tryConsumeWorkspaceQuota?: jest.Mock; + } = {}) { + const aiSettings = { + isPublicShareAssistantEnabled: jest + .fn() + .mockResolvedValue(over.assistantEnabled ?? true), + }; + const shareService = { + getShareForPage: + over.getShareForPage ?? + jest.fn().mockResolvedValue({ + id: 'SHARE-A', + pageId: 'root-page', + spaceId: 'space-1', + sharedPage: { id: 'root-page', title: 'Root' }, + }), + isSharingAllowed: + over.isSharingAllowed ?? jest.fn().mockResolvedValue(true), + }; + const pageRepo = { + findById: + over.findById ?? jest.fn().mockResolvedValue({ id: 'opened-uuid' }), + }; + const pagePermissionRepo = { + hasRestrictedAncestor: + over.hasRestrictedAncestor ?? jest.fn().mockResolvedValue(false), + }; + const publicShareChat = { + resolveShareRole: + over.resolveShareRole ?? jest.fn().mockResolvedValue(null), + getShareChatModel: + over.getShareChatModel ?? jest.fn().mockResolvedValue('MODEL'), + tryConsumeWorkspaceQuota: + over.tryConsumeWorkspaceQuota ?? jest.fn().mockResolvedValue(true), + }; + const deps: ShareAssistantDeps = { + aiSettings: aiSettings as never, + shareService: shareService as never, + pageRepo: pageRepo as never, + pagePermissionRepo: pagePermissionRepo as never, + publicShareChat: publicShareChat as never, + }; + return { + deps, + aiSettings, + shareService, + pageRepo, + pagePermissionRepo, + publicShareChat, + }; + } + + const body = (over: Record = {}) => ({ + shareId: 'SHARE-A', + pageId: 'opened-page', + messages: [], + ...over, + }); + + /** Run the funnel and capture the thrown HttpException status (or null). */ + async function statusOf( + deps: ShareAssistantDeps, + b: Record, + ): Promise { + try { + await resolveShareAssistantRequest(deps, { + workspaceId: 'ws-1', + body: b as never, + }); + return null; + } catch (err) { + if (err instanceof HttpException) return err.getStatus(); + throw err; + } + } + + it('happy path: returns the resolved, non-null request', async () => { + const { deps } = makeDeps(); + const out = await resolveShareAssistantRequest(deps, { + workspaceId: 'ws-1', + body: body() as never, + }); + expect(out.shareId).toBe('SHARE-A'); + expect(out.share.id).toBe('SHARE-A'); + expect(out.model).toBe('MODEL'); + expect(out.role).toBeNull(); + expect(out.openedPage).toEqual({ id: 'opened-page', title: 'Root' }); + }); + + it('assistant disabled => 404 and NO share/page/model lookups', async () => { + const { deps, shareService, pageRepo, publicShareChat } = makeDeps({ + assistantEnabled: false, + }); + expect(await statusOf(deps, body())).toBe(404); + expect(shareService.getShareForPage).not.toHaveBeenCalled(); + expect(pageRepo.findById).not.toHaveBeenCalled(); + expect(publicShareChat.getShareChatModel).not.toHaveBeenCalled(); + }); + + it('share.id !== body.shareId => 404 (cross-share id swap rejected)', async () => { + const { deps, publicShareChat } = makeDeps({ + getShareForPage: jest.fn().mockResolvedValue({ + id: 'OTHER-SHARE', + pageId: 'root', + spaceId: 'space-1', + sharedPage: null, + }), + }); + expect(await statusOf(deps, body({ shareId: 'SHARE-A' }))).toBe(404); + // Never reached the model resolution for an unusable share. + expect(publicShareChat.getShareChatModel).not.toHaveBeenCalled(); + }); + + it('opened page unresolvable (pageRepo.findById -> null) => fail-closed 404', async () => { + const { deps } = makeDeps({ + findById: jest.fn().mockResolvedValue(null), + }); + expect(await statusOf(deps, body())).toBe(404); + }); + + it('restricted descendant => 404 (same as out-of-tree, no existence leak)', async () => { + const { deps, pagePermissionRepo } = makeDeps({ + hasRestrictedAncestor: jest.fn().mockResolvedValue(true), + }); + expect(await statusOf(deps, body())).toBe(404); + expect(pagePermissionRepo.hasRestrictedAncestor).toHaveBeenCalled(); + }); + + it('getShareChatModel throws AiNotConfiguredException => 503', async () => { + const { deps } = makeDeps({ + getShareChatModel: jest + .fn() + .mockRejectedValue(new AiNotConfiguredException()), + }); + expect(await statusOf(deps, body())).toBe(503); + }); + + it('getShareChatModel throws a non-AiNotConfigured error => re-thrown (not a 503/404)', async () => { + const boom = new Error('boom'); + const { deps } = makeDeps({ + getShareChatModel: jest.fn().mockRejectedValue(boom), + }); + await expect( + resolveShareAssistantRequest(deps, { + workspaceId: 'ws-1', + body: body() as never, + }), + ).rejects.toBe(boom); + }); + + it('tryConsumeWorkspaceQuota false => 429 thrown BEFORE any stream', async () => { + const { deps, publicShareChat } = makeDeps({ + tryConsumeWorkspaceQuota: jest.fn().mockResolvedValue(false), + }); + expect(await statusOf(deps, body())).toBe(429); + // The quota gate ran AFTER the model resolved (provider configured) but the + // function returns/throws before producing a streamable request. + expect(publicShareChat.tryConsumeWorkspaceQuota).toHaveBeenCalledWith('ws-1'); + }); + + it('messages over MAX_SHARE_MESSAGES => 413', async () => { + const { deps } = makeDeps(); + const tooMany = Array.from({ length: MAX_SHARE_MESSAGES + 1 }, () => ({ + role: 'user', + parts: [{ type: 'text', text: 'hi' }], + })); + expect(await statusOf(deps, body({ messages: tooMany }))).toBe(413); + }); + + it('a single message over MAX_SHARE_MESSAGE_CHARS => 413 (uiMessageTextLength)', async () => { + const { deps } = makeDeps(); + const huge = { + role: 'user', + parts: [{ type: 'text', text: 'x'.repeat(MAX_SHARE_MESSAGE_CHARS + 1) }], + }; + expect(await statusOf(deps, body({ messages: [huge] }))).toBe(413); + }); + + 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. + const { deps } = makeDeps({ + tryConsumeWorkspaceQuota: jest.fn().mockResolvedValue(false), + }); + const huge = { + role: 'user', + parts: [{ type: 'text', text: 'x'.repeat(MAX_SHARE_MESSAGE_CHARS + 1) }], + }; + expect(await statusOf(deps, body({ messages: [huge] }))).toBe(429); + }); +}); + +describe('uiMessageTextLength', () => { + it('returns 0 for an undefined / parts-less / non-array message', () => { + expect(uiMessageTextLength(undefined)).toBe(0); + expect(uiMessageTextLength({} as UIMessage)).toBe(0); + expect(uiMessageTextLength({ parts: 'nope' } as never)).toBe(0); + }); + + it('sums the lengths of ONLY the text parts', () => { + const msg = { + role: 'user', + parts: [ + { type: 'text', text: 'hello' }, // 5 + { type: 'tool-call', text: 'IGNORED' }, // non-text: ignored + { type: 'text', text: 'world!' }, // 6 + { type: 'text' }, // no text field: ignored + ], + } as unknown as UIMessage; + expect(uiMessageTextLength(msg)).toBe(11); + }); + + it('matches the 413 boundary used by the funnel', () => { + const atCap = { + role: 'user', + parts: [{ type: 'text', text: 'x'.repeat(MAX_SHARE_MESSAGE_CHARS) }], + } as unknown as UIMessage; + const overCap = { + role: 'user', + parts: [{ type: 'text', text: 'x'.repeat(MAX_SHARE_MESSAGE_CHARS + 1) }], + } as unknown as UIMessage; + expect(uiMessageTextLength(atCap)).toBe(MAX_SHARE_MESSAGE_CHARS); + expect(uiMessageTextLength(overCap)).toBeGreaterThan(MAX_SHARE_MESSAGE_CHARS); + }); +}); diff --git a/apps/server/src/core/ai-chat/public-share-chat.controller.ts b/apps/server/src/core/ai-chat/public-share-chat.controller.ts index fa5a0a5f..4c8d0a39 100644 --- a/apps/server/src/core/ai-chat/public-share-chat.controller.ts +++ b/apps/server/src/core/ai-chat/public-share-chat.controller.ts @@ -77,142 +77,25 @@ export class PublicShareChatController { @AuthWorkspace() workspace: Workspace, ): Promise { const body = (req.body ?? {}) as PublicShareChatStreamBody; - const shareId = typeof body.shareId === 'string' ? body.shareId.trim() : ''; - const pageId = typeof body.pageId === 'string' ? body.pageId.trim() : ''; // ---- Guardrail funnel (order matters; each failure exits before stream) ---- - - // 1. Workspace master toggle. 404 (do not reveal the feature exists). - const assistantEnabled = await this.aiSettings.isPublicShareAssistantEnabled( - workspace.id, + // The whole pre-hijack fact-resolution + cap-ordering block is a pure-ish + // helper (collaborators passed in) so every funnel branch — 404 disabled / + // share-mismatch / page-unresolvable / restricted, 503 unconfigured, 429 + // over-cap, 413 too many/too long — is unit-testable against the red-team + // boundaries without the full Nest/DB graph. It throws the SAME HttpException + // the controller would, and never starts streaming. + const resolved = await resolveShareAssistantRequest( + { + aiSettings: this.aiSettings, + shareService: this.shareService, + pageRepo: this.pageRepo, + pagePermissionRepo: this.pagePermissionRepo, + publicShareChat: this.publicShareChat, + }, + { workspaceId: workspace.id, body }, ); - - // 2. Share usable? Resolved via the page's share membership, since the page - // resolution (getShareForPage) ALSO yields the share + workspace. We - // still need basic input to attempt it. - // 3. Page in share? The same getShareForPage lookup confirms the opened page - // resolves to THIS share tree, PLUS an explicit restricted-ancestor gate - // (getShareForPage itself does NOT exclude restricted descendants) so a - // restricted page hidden from the public view is graded not-in-share. - // (shareUsable + pageInShare are set together below; the funnel grades - // them as distinct ordered steps.) - let share: Awaited> | undefined; - let shareUsable = false; - let pageInShare = false; - if (assistantEnabled && shareId && pageId) { - // getShareForPage walks up the tree to the nearest ancestor share, - // enforces share.workspaceId === workspaceId and includeSubPages, and - // returns undefined when the page is not publicly reachable. NOTE: it - // joins only the `shares` table — it does NOT exclude restricted - // descendants — so a restricted page inside an includeSubPages share - // still resolves here. We add an explicit restricted-ancestor gate below - // (same as the public view) so the opened page's title never leaks into - // the system prompt for a page the public view 404s. - share = await this.shareService.getShareForPage(pageId, workspace.id); - if (share && share.id === shareId) { - // Confirm sharing is still allowed for the share's space (and not - // disabled at workspace/space level) — same gate the public views use. - const sharingAllowed = await this.shareService.isSharingAllowed( - workspace.id, - share.spaceId, - ); - // A restricted descendant is hidden from the public share view; treat - // the opened page as not-in-share so the funnel returns the SAME 404 it - // returns for an out-of-tree page (uniform, no existence leak). - // hasRestrictedAncestor matches on the page UUID only, while the - // opened pageId may be a slugId, so resolve to the UUID first (cheap - // base-fields lookup, mirroring how getSharedPage resolves the page - // before its restricted check). - const openedPageRow = await this.pageRepo.findById(pageId); - const restricted = openedPageRow - ? await this.pagePermissionRepo.hasRestrictedAncestor( - openedPageRow.id, - ) - : true; // unresolvable opened page => fail closed (treat as not-in-share) - // The security-relevant combination (server-resolved share id === - // requested shareId, + sharingAllowed, + the restricted gate) is a pure, - // unit-tested helper so the access join point can be exercised against - // the red-team boundaries without the full Nest/DB graph. - ({ shareUsable, pageInShare } = deriveShareAccess({ - resolvedShareId: share.id, - requestedShareId: shareId, - sharingAllowed, - restricted, - })); - } - } - - // 4. Provider configured? Resolve the model now so an unconfigured provider - // yields a clean 503 (AiNotConfiguredException) BEFORE hijack. Only - // attempt this once the earlier gates passed, to avoid leaking timing. - let model: Awaited> | undefined; - // Admin-selected identity (agent role) for the anonymous assistant, resolved - // server-authoritatively. null = built-in locked persona. - let role: AiAgentRole | null = null; - let providerConfigured = false; - if (assistantEnabled && shareUsable && pageInShare) { - try { - role = await this.publicShareChat.resolveShareRole(workspace.id); - model = await this.publicShareChat.getShareChatModel(workspace.id, role); - providerConfigured = true; - } catch (err) { - if (err instanceof AiNotConfiguredException) { - providerConfigured = false; - } else { - throw err; - } - } - } - - const outcome = evaluateShareAssistantFunnel({ - assistantEnabled, - shareUsable, - pageInShare, - providerConfigured, - }); - if (outcome.ok === false) { - // 404 for everything access-shaped (feature/share/page); 503 for config. - if (outcome.status === 503) { - throw new ServiceUnavailableException('AI is not configured'); - } - throw new NotFoundException('Not found'); - } - - // 5. Per-WORKSPACE anti-abuse cap (IP-independent; defense in depth). The - // per-IP @Throttle above can be evaded by an attacker rotating - // `X-Forwarded-For` (the app runs with trustProxy), and each evaded call - // spends REAL tokens on the workspace owner's paid AI provider. This cap - // is keyed by the server-resolved workspace id (never attacker- - // controllable), so it bounds the owner's bill even when the per-IP limit - // is fully defeated via XFF spoofing. Checked here, BEFORE res.hijack(), - // so an over-cap workspace gets a clean 429 and spends nothing. NOTE: - // production should ALSO front this endpoint with a trusted proxy that - // REWRITES (not appends) XFF so the per-IP throttle stays meaningful. - if (!(await this.publicShareChat.tryConsumeWorkspaceQuota(workspace.id))) { - throw new HttpException( - 'This documentation assistant is temporarily busy. Please try again later.', - HttpStatus.TOO_MANY_REQUESTS, - ); - } - - // ---- Validate / bound the payload (cheap caps; ephemeral, never stored) ---- - const messages = Array.isArray(body.messages) - ? (body.messages as UIMessage[]) - : []; - if (messages.length > MAX_SHARE_MESSAGES) { - throw new HttpException('Too many messages', 413); - } - for (const m of messages) { - const text = uiMessageTextLength(m); - if (text > MAX_SHARE_MESSAGE_CHARS) { - throw new HttpException('Message too long', 413); - } - } - - const openedPage = { - id: pageId, - title: share?.sharedPage?.title ?? undefined, - }; + const { shareId, share, model, role, messages, openedPage } = resolved; // Abort the agent loop when the client disconnects (mirrors ai-chat). const controller = new AbortController(); @@ -230,15 +113,15 @@ export class PublicShareChatController { workspaceId: workspace.id, shareId, share: { - id: share!.id, - pageId: share!.pageId, - sharedPage: share!.sharedPage, + id: share.id, + pageId: share.pageId, + sharedPage: share.sharedPage, }, openedPage, messages, res, signal: controller.signal, - model: model!, + model, role, }); } catch (err) { @@ -255,8 +138,174 @@ export class PublicShareChatController { } } -/** Sum of the text-part lengths of a UIMessage (cheap, for the size cap). */ -function uiMessageTextLength(message: UIMessage | undefined): number { +/** + * The collaborators the pre-hijack funnel needs. Declared as the minimal slice + * of each injected service it actually calls, so the resolver can be unit-tested + * with hand-rolled mocks (no Nest module graph, no DB). + */ +export interface ShareAssistantDeps { + aiSettings: Pick; + shareService: Pick< + ShareService, + 'getShareForPage' | 'isSharingAllowed' + >; + pageRepo: Pick; + pagePermissionRepo: Pick; + publicShareChat: Pick< + PublicShareChatService, + | 'resolveShareRole' + | 'getShareChatModel' + | 'tryConsumeWorkspaceQuota' + >; +} + +/** The resolved, validated request ready to stream (everything is non-null). */ +export interface ResolvedShareAssistantRequest { + shareId: string; + share: NonNullable>>; + model: Awaited>; + role: AiAgentRole | null; + messages: UIMessage[]; + openedPage: { id: string; title?: string }; +} + +/** + * Pre-hijack fact-resolution + cap-ordering for the anonymous public-share + * assistant, extracted from the controller so every funnel branch is unit- + * testable without the Nest/DB graph. Order is security-relevant and each + * failure exits BEFORE any stream/hijack: + * 1. assistant toggle off => 404 (no share/page/model lookups); + * 2. share/page access (deriveShareAccess + evaluateShareAssistantFunnel) => + * 404 (uniform; restricted descendant and out-of-tree look identical); + * 3. provider unconfigured => 503 (AiNotConfiguredException), other errors + * re-thrown; + * 4. per-workspace quota exhausted => 429 (BEFORE any stream/hijack); + * 5. payload caps => 413 (too many messages / a single message too long). + * Throws the SAME HttpException the controller would; returns the resolved, + * non-null request otherwise. + */ +export async function resolveShareAssistantRequest( + deps: ShareAssistantDeps, + input: { workspaceId: string; body: PublicShareChatStreamBody }, +): Promise { + const { workspaceId, body } = input; + const shareId = typeof body.shareId === 'string' ? body.shareId.trim() : ''; + const pageId = typeof body.pageId === 'string' ? body.pageId.trim() : ''; + + // 1. Workspace master toggle. 404 (do not reveal the feature exists). + const assistantEnabled = + await deps.aiSettings.isPublicShareAssistantEnabled(workspaceId); + + // 2/3. Share usable? Page in share? Resolved via the page's share membership, + // since getShareForPage ALSO yields the share + workspace. The opened + // page is then gated by an explicit restricted-ancestor check (which + // getShareForPage does NOT do) so a restricted page hidden from the + // public view is graded not-in-share. + let share: Awaited> | undefined; + let shareUsable = false; + let pageInShare = false; + if (assistantEnabled && shareId && pageId) { + share = await deps.shareService.getShareForPage(pageId, workspaceId); + if (share && share.id === shareId) { + const sharingAllowed = await deps.shareService.isSharingAllowed( + workspaceId, + share.spaceId, + ); + // hasRestrictedAncestor matches on the page UUID only, while the opened + // pageId may be a slugId, so resolve to the UUID first (cheap base-fields + // lookup). An unresolvable opened page fails closed (not-in-share). + const openedPageRow = await deps.pageRepo.findById(pageId); + const restricted = openedPageRow + ? await deps.pagePermissionRepo.hasRestrictedAncestor(openedPageRow.id) + : true; + ({ shareUsable, pageInShare } = deriveShareAccess({ + resolvedShareId: share.id, + requestedShareId: shareId, + sharingAllowed, + restricted, + })); + } + } + + // 4. Provider configured? Resolve the model now so an unconfigured provider + // yields a clean 503 BEFORE hijack. Only after the access gates pass, to + // avoid leaking timing. + let model: + | Awaited> + | undefined; + let role: AiAgentRole | null = null; + let providerConfigured = false; + if (assistantEnabled && shareUsable && pageInShare) { + try { + role = await deps.publicShareChat.resolveShareRole(workspaceId); + model = await deps.publicShareChat.getShareChatModel(workspaceId, role); + providerConfigured = true; + } catch (err) { + if (err instanceof AiNotConfiguredException) { + providerConfigured = false; + } else { + throw err; + } + } + } + + const outcome = evaluateShareAssistantFunnel({ + assistantEnabled, + shareUsable, + pageInShare, + providerConfigured, + }); + if (outcome.ok === false) { + // 404 for everything access-shaped (feature/share/page); 503 for config. + if (outcome.status === 503) { + throw new ServiceUnavailableException('AI is not configured'); + } + throw new NotFoundException('Not found'); + } + + // 5. Per-WORKSPACE anti-abuse cap (IP-independent; defense in depth). Checked + // BEFORE res.hijack(), so an over-cap workspace gets a clean 429 and spends + // nothing. + if (!(await deps.publicShareChat.tryConsumeWorkspaceQuota(workspaceId))) { + throw new HttpException( + 'This documentation assistant is temporarily busy. Please try again later.', + HttpStatus.TOO_MANY_REQUESTS, + ); + } + + // ---- Validate / bound the payload (cheap caps; ephemeral, never stored) ---- + const messages = Array.isArray(body.messages) + ? (body.messages as UIMessage[]) + : []; + if (messages.length > MAX_SHARE_MESSAGES) { + throw new HttpException('Too many messages', 413); + } + for (const m of messages) { + if (uiMessageTextLength(m) > MAX_SHARE_MESSAGE_CHARS) { + throw new HttpException('Message too long', 413); + } + } + + const openedPage = { + id: pageId, + title: share?.sharedPage?.title ?? undefined, + }; + + // The funnel passed, so share/model are guaranteed present. + return { + shareId, + share: share!, + model: model!, + role, + messages, + openedPage, + }; +} + +/** Sum of the text-part lengths of a UIMessage (cheap, for the size cap). + * Exported so the 413 size-cap logic is unit-testable without the Nest/DB graph. + */ +export function uiMessageTextLength(message: UIMessage | undefined): number { if (!message?.parts || !Array.isArray(message.parts)) return 0; let total = 0; for (const p of message.parts) { diff --git a/apps/server/src/core/ai-chat/public-share-chat.spec.ts b/apps/server/src/core/ai-chat/public-share-chat.spec.ts index 623852fb..2be6a5f4 100644 --- a/apps/server/src/core/ai-chat/public-share-chat.spec.ts +++ b/apps/server/src/core/ai-chat/public-share-chat.spec.ts @@ -7,7 +7,11 @@ import { filterShareTranscript, } from './public-share-chat.service'; import { PublicShareChatToolsService } from './tools/public-share-chat-tools.service'; -import { PublicShareWorkspaceLimiter } from './public-share-workspace-limiter'; +import { + PublicShareWorkspaceLimiter, + resolveShareAiWorkspaceMax, + SHARE_AI_WORKSPACE_MAX_PER_WINDOW, +} from './public-share-workspace-limiter'; /** * Minimal in-memory fake of the slice of ioredis the sliding-window limiter @@ -195,6 +199,54 @@ describe('buildShareSystemPrompt locking', () => { expect(prompt).toContain('read-only assistant'); expect(prompt).toContain('anti prompt-injection'); }); + + it('an opened page with a title injects both the pageId and the title', () => { + const prompt = buildShareSystemPrompt({ + share: null, + openedPage: { id: 'page-123', title: 'Getting Started' }, + }); + expect(prompt).toContain('(pageId: page-123)'); + expect(prompt).toContain('"Getting Started"'); + expect(prompt).toContain('the current page'); + }); + + it('an opened page with a blank/whitespace title falls back to "Untitled"', () => { + const prompt = buildShareSystemPrompt({ + share: null, + openedPage: { id: 'page-123', title: ' ' }, + }); + expect(prompt).toContain('(pageId: page-123)'); + expect(prompt).toContain('"Untitled"'); + }); + + it('an empty / blank pageId omits the opened-page context line entirely', () => { + const emptyId = buildShareSystemPrompt({ + share: null, + openedPage: { id: '', title: 'Ignored' }, + }); + expect(emptyId).not.toContain('pageId:'); + expect(emptyId).not.toContain('the current page'); + + const blankId = buildShareSystemPrompt({ + share: null, + openedPage: { id: ' ', title: 'Ignored' }, + }); + expect(blankId).not.toContain('pageId:'); + }); + + it('a present share title is injected; a blank share title is omitted', () => { + const withTitle = buildShareSystemPrompt({ + share: { sharedPageTitle: 'Product Docs' }, + openedPage: null, + }); + expect(withTitle).toContain('titled "Product Docs"'); + + const blankTitle = buildShareSystemPrompt({ + share: { sharedPageTitle: ' ' }, + openedPage: null, + }); + expect(blankTitle).not.toContain('This published documentation is titled'); + }); }); describe('PublicShareChatService model fallback', () => { @@ -306,6 +358,44 @@ describe('PublicShareChatService model fallback', () => { }); }); +describe('resolveShareAiWorkspaceMax (env-overridable per-workspace cap)', () => { + const ENV = 'SHARE_AI_WORKSPACE_MAX_PER_HOUR'; + const original = process.env[ENV]; + + afterEach(() => { + if (original === undefined) delete process.env[ENV]; + else process.env[ENV] = original; + }); + + it('uses a valid positive integer from the env', () => { + process.env[ENV] = '42'; + expect(resolveShareAiWorkspaceMax()).toBe(42); + }); + + it('floors a float value', () => { + process.env[ENV] = '99.9'; + expect(resolveShareAiWorkspaceMax()).toBe(99); + }); + + it('falls back to the default for an unparseable / NaN value', () => { + process.env[ENV] = 'not-a-number'; + expect(resolveShareAiWorkspaceMax()).toBe(SHARE_AI_WORKSPACE_MAX_PER_WINDOW); + expect(SHARE_AI_WORKSPACE_MAX_PER_WINDOW).toBe(300); + }); + + it('falls back to the default when unset', () => { + delete process.env[ENV]; + expect(resolveShareAiWorkspaceMax()).toBe(SHARE_AI_WORKSPACE_MAX_PER_WINDOW); + }); + + it('falls back to the default for zero or a negative value (no unlimited / negative cap)', () => { + process.env[ENV] = '0'; + expect(resolveShareAiWorkspaceMax()).toBe(SHARE_AI_WORKSPACE_MAX_PER_WINDOW); + process.env[ENV] = '-5'; + expect(resolveShareAiWorkspaceMax()).toBe(SHARE_AI_WORKSPACE_MAX_PER_WINDOW); + }); +}); + describe('PublicShareWorkspaceLimiter (cluster-wide sliding-window per-workspace cap)', () => { it('allows up to the cap within a window, then 429s (returns false)', async () => { const limiter = makeLimiter(3, 60_000, () => 1_000); @@ -353,6 +443,23 @@ describe('PublicShareWorkspaceLimiter (cluster-wide sliding-window per-workspace expect(await limiter.tryConsume('ws-1')).toBe(true); }); + it('consumes a distinct member slot per call at one FIXED clock value (no same-ms score-collision under-count)', async () => { + // All calls happen at the SAME millisecond. The limiter mints a unique member + // id per attempt, so distinct calls in the same ms must NOT collide on the + // sorted-set score and under-count: exactly `cap` calls are admitted, the + // rest rejected — even though every score is identical. + const cap = 5; + const limiter = makeLimiter(cap, 60_000, () => 7_000); // clock never advances + const results: boolean[] = []; + for (let i = 0; i < cap + 3; i++) { + results.push(await limiter.tryConsume('ws-1')); + } + // First `cap` admitted, the remaining 3 rejected. + expect(results.slice(0, cap)).toEqual(Array(cap).fill(true)); + expect(results.slice(cap)).toEqual([false, false, false]); + expect(results.filter(Boolean)).toHaveLength(cap); + }); + it('keeps separate budgets per workspace (one over-cap ws cannot starve another)', async () => { const limiter = makeLimiter(1, 60_000, () => 1_000); expect(await limiter.tryConsume('ws-a')).toBe(true); diff --git a/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.spec.ts b/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.spec.ts index d2cf6004..e86cbbf5 100644 --- a/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.spec.ts +++ b/apps/server/src/core/ai-chat/roles/ai-agent-roles.service.spec.ts @@ -93,6 +93,56 @@ describe('AiAgentRolesService guards', () => { ).rejects.toBeInstanceOf(BadRequestException); expect(repo.update).not.toHaveBeenCalled(); }); + + it('instructions cleared to whitespace => BadRequest, repo.update NOT called', async () => { + const { service, repo } = makeService({ existing: makeRow() }); + await expect( + service.update('ws-1', 'r1', { + instructions: ' ', + } as UpdateAgentRoleDto), + ).rejects.toBeInstanceOf(BadRequestException); + expect(repo.update).not.toHaveBeenCalled(); + }); + + it('concurrent soft-delete: row exists on the pre-update lookup but the re-fetch is undefined => BadRequest (not a TypeError)', async () => { + // findById returns the live row FIRST (pre-update guard passes), then the + // role is soft-deleted concurrently, so the POST-update re-fetch returns + // undefined. The service must surface a clean 400, never dereference + // undefined (which would throw a TypeError in toView). + const { service, repo } = makeService(); + repo.findById + .mockResolvedValueOnce(makeRow()) + .mockResolvedValueOnce(undefined); + await expect( + service.update('ws-1', 'r1', { name: 'X' } as UpdateAgentRoleDto), + ).rejects.toBeInstanceOf(BadRequestException); + // The UPDATE ran (the row existed pre-update), but the re-fetch failed. + expect(repo.update).toHaveBeenCalled(); + expect(repo.findById).toHaveBeenCalledTimes(2); + }); + + it('emoji/description tri-state: emoji:"" => null (clear), emoji omitted => undefined (unchanged), description:" " => null', async () => { + const { service, repo } = makeService({ existing: makeRow() }); + + // emoji explicitly emptied => clear to null; description whitespace => null. + await service.update('ws-1', 'r1', { + emoji: '', + description: ' ', + } as UpdateAgentRoleDto); + const patch1 = repo.update.mock.calls[0][2]; + expect(patch1.emoji).toBeNull(); + expect(patch1.description).toBeNull(); + + repo.update.mockClear(); + + // emoji omitted => unchanged (undefined passed through to the repo patch). + await service.update('ws-1', 'r1', { + name: 'Renamed', + } as UpdateAgentRoleDto); + const patch2 = repo.update.mock.calls[0][2]; + expect(patch2.emoji).toBeUndefined(); + expect(patch2.description).toBeUndefined(); + }); }); describe('remove', () => { @@ -136,6 +186,51 @@ describe('AiAgentRolesService guards', () => { expect(repo.insert).not.toHaveBeenCalled(); }); + it('modelConfig:{chatModel} only persists {chatModel} (no driver key)', async () => { + const { service, repo } = makeService(); + await service.create('ws-1', 'u1', { + name: 'R', + instructions: 'do', + modelConfig: { chatModel: 'gpt-4o' }, + } as CreateAgentRoleDto); + const values = repo.insert.mock.calls[0][0]; + expect(values.modelConfig).toEqual({ chatModel: 'gpt-4o' }); + expect('driver' in values.modelConfig).toBe(false); + }); + + it('modelConfig:{} (empty) normalizes to null', async () => { + const { service, repo } = makeService(); + await service.create('ws-1', 'u1', { + name: 'R', + instructions: 'do', + modelConfig: {}, + } as CreateAgentRoleDto); + expect(repo.insert.mock.calls[0][0].modelConfig).toBeNull(); + }); + + it('modelConfig:{chatModel:" "} (whitespace-only) normalizes to null', async () => { + const { service, repo } = makeService(); + await service.create('ws-1', 'u1', { + name: 'R', + instructions: 'do', + modelConfig: { chatModel: ' ' }, + } as CreateAgentRoleDto); + expect(repo.insert.mock.calls[0][0].modelConfig).toBeNull(); + }); + + it('modelConfig:{driver,chatModel} round-trips both fields (trimmed)', async () => { + const { service, repo } = makeService(); + await service.create('ws-1', 'u1', { + name: 'R', + instructions: 'do', + modelConfig: { driver: 'gemini', chatModel: ' gemini-2.0-flash ' }, + } as CreateAgentRoleDto); + expect(repo.insert.mock.calls[0][0].modelConfig).toEqual({ + driver: 'gemini', + chatModel: 'gemini-2.0-flash', + }); + }); + it('duplicate name (Postgres 23505) => ConflictException (409), not 500', async () => { const { service, repo } = makeService(); // The partial unique (workspace_id, name) index rejects the insert. @@ -148,6 +243,28 @@ describe('AiAgentRolesService guards', () => { ).rejects.toBeInstanceOf(ConflictException); }); + it('duplicate name 409 message contains the TRIMMED submitted name', async () => { + const { service, repo } = makeService(); + repo.insert.mockRejectedValueOnce({ code: '23505' }); + await service + .create('ws-1', 'u1', { + name: ' Researcher ', + instructions: 'do', + } as CreateAgentRoleDto) + .then( + () => { + throw new Error('expected create to throw'); + }, + (err: unknown) => { + expect(err).toBeInstanceOf(ConflictException); + const message = (err as ConflictException).message; + // The trimmed name appears verbatim; the untrimmed padding does not. + expect(message).toContain('"Researcher"'); + expect(message).not.toContain(' Researcher '); + }, + ); + }); + it('non-unique-violation error is NOT swallowed (re-thrown as-is)', async () => { const { service, repo } = makeService(); const other = Object.assign(new Error('boom'), { code: '23502' }); diff --git a/apps/server/src/core/ai-chat/roles/jsonb-object.spec.ts b/apps/server/src/core/ai-chat/roles/jsonb-object.spec.ts new file mode 100644 index 00000000..96875748 --- /dev/null +++ b/apps/server/src/core/ai-chat/roles/jsonb-object.spec.ts @@ -0,0 +1,30 @@ +import { jsonbObject } from '@docmost/db/repos/ai-agent-roles/ai-agent-roles.repo'; + +/** + * Unit tests for jsonbObject: the repo helper that encodes a model_config object + * as a jsonb bind (or null when there is nothing to persist). It is the last + * line of defence before the column write, so the null-vs-bind decision is what + * matters here. We assert only null vs non-null because the non-null value is a + * kysely `sql` template fragment whose internal shape is an implementation + * detail of the SQL tag. + */ +describe('jsonbObject', () => { + it('returns null for null', () => { + expect(jsonbObject(null)).toBeNull(); + }); + + it('returns null for undefined', () => { + expect(jsonbObject(undefined)).toBeNull(); + }); + + it('returns null for an empty object (nothing to persist)', () => { + expect(jsonbObject({})).toBeNull(); + }); + + it('returns a (non-null) jsonb bind for a non-empty object', () => { + const out = jsonbObject({ driver: 'gemini', chatModel: 'gemini-2.0-flash' }); + // A real sql fragment is produced, never null/undefined. + expect(out).not.toBeNull(); + expect(out).toBeDefined(); + }); +}); diff --git a/apps/server/src/core/ai-chat/roles/role-override-contract.spec.ts b/apps/server/src/core/ai-chat/roles/role-override-contract.spec.ts new file mode 100644 index 00000000..c5165b26 --- /dev/null +++ b/apps/server/src/core/ai-chat/roles/role-override-contract.spec.ts @@ -0,0 +1,135 @@ +import { AiService } from '../../../integrations/ai/ai.service'; +import { AiNotConfiguredException } from '../../../integrations/ai/ai-not-configured.exception'; +import { roleModelOverride } from './role-model-config'; +import type { AiAgentRole } from '@docmost/db/types/entity.types'; + +/** + * Contract test for the override SHAPE that travels from a role's persisted + * `model_config` (via roleModelOverride) into AiService.getChatModel. + * + * This is the seam between the two halves of the role-model feature: + * - roleModelOverride (pure) turns model_config into a ChatModelOverride; + * - getChatModel consumes that override to build the model (or to 503). + * Wiring the REAL roleModelOverride output into a unit-constructed AiService + * (with stubbed deps, no DB) pins that the two agree on the override contract: + * - a cross-driver override whose creds are absent => AiNotConfiguredException + * naming the role + driver; + * - a chatModel-only override keeps the workspace driver/creds (no creds + * lookup, no decrypt); + * - an ollama cross-driver override => 503 (no silent baseUrl reuse). + */ +describe('role override -> AiService.getChatModel contract', () => { + function role(modelConfig: unknown, name = 'Researcher'): AiAgentRole { + return { id: 'r1', name, modelConfig } as unknown as AiAgentRole; + } + + function makeService(opts: { + workspaceDriver: string; + baseUrl?: string; + credsApiKeyEnc?: string; + }) { + const aiSettings = { + resolve: jest.fn().mockResolvedValue({ + driver: opts.workspaceDriver, + chatModel: 'gpt-4o-mini', + apiKey: 'workspace-key', + baseUrl: opts.baseUrl, + }), + }; + const aiProviderCredentialsRepo = { + find: jest + .fn() + .mockResolvedValue( + opts.credsApiKeyEnc ? { apiKeyEnc: opts.credsApiKeyEnc } : undefined, + ), + }; + const secretBox = { decryptSecret: jest.fn().mockReturnValue('decrypted') }; + const service = new AiService( + aiSettings as never, + aiProviderCredentialsRepo as never, + secretBox as never, + ); + return { service, aiSettings, aiProviderCredentialsRepo, secretBox }; + } + + it('cross-driver override with NO creds => 503 naming the role and the override driver', async () => { + const override = roleModelOverride( + role({ driver: 'gemini', chatModel: 'gemini-2.0-flash' }), + ); + expect(override).toEqual({ + driver: 'gemini', + chatModel: 'gemini-2.0-flash', + roleName: 'Researcher', + }); + + // Workspace is openai; the gemini override has no configured creds. + const { service, aiProviderCredentialsRepo } = makeService({ + workspaceDriver: 'openai', + }); + + await service.getChatModel('ws-1', override).then( + () => { + throw new Error('expected getChatModel to throw'); + }, + (err: unknown) => { + expect(err).toBeInstanceOf(AiNotConfiguredException); + const message = (err as AiNotConfiguredException).message; + expect(message).toContain('gemini'); + expect(message).toContain('Researcher'); + }, + ); + expect(aiProviderCredentialsRepo.find).toHaveBeenCalledWith('ws-1', 'gemini'); + }); + + it('chatModel-only override keeps the workspace driver/creds (no creds lookup, no decrypt)', async () => { + const override = roleModelOverride(role({ chatModel: 'gpt-4o' })); + // No driver in the override => the workspace driver/creds are reused. + expect(override).toEqual({ + driver: undefined, + chatModel: 'gpt-4o', + roleName: 'Researcher', + }); + + const { service, aiProviderCredentialsRepo, secretBox } = makeService({ + workspaceDriver: 'openai', + }); + + const model = await service.getChatModel('ws-1', override); + expect(model).toBeDefined(); + expect(aiProviderCredentialsRepo.find).not.toHaveBeenCalled(); + expect(secretBox.decryptSecret).not.toHaveBeenCalled(); + }); + + it('ollama cross-driver override (workspace driver != ollama) => 503, no baseUrl reuse', async () => { + const override = roleModelOverride( + role({ driver: 'ollama', chatModel: 'llama3' }, 'Local'), + ); + expect(override).toEqual({ + driver: 'ollama', + chatModel: 'llama3', + roleName: 'Local', + }); + + const { service, aiProviderCredentialsRepo } = makeService({ + workspaceDriver: 'openai', + baseUrl: 'https://openrouter.example/v1', + }); + + await service.getChatModel('ws-1', override).then( + () => { + throw new Error('expected getChatModel to throw'); + }, + (err: unknown) => { + expect(err).toBeInstanceOf(AiNotConfiguredException); + const message = (err as AiNotConfiguredException).message; + expect(message).toContain('ollama'); + expect(message).toContain('openai'); + expect(message).toContain('Local'); + // The workspace gateway baseUrl must never be reused for ollama. + expect(message).not.toContain('openrouter.example'); + }, + ); + // No creds lookup for ollama: we fail before reaching the creds branch. + expect(aiProviderCredentialsRepo.find).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/server/src/core/ai-chat/tools/public-share-chat-tools.service.spec.ts b/apps/server/src/core/ai-chat/tools/public-share-chat-tools.service.spec.ts new file mode 100644 index 00000000..dd46b527 --- /dev/null +++ b/apps/server/src/core/ai-chat/tools/public-share-chat-tools.service.spec.ts @@ -0,0 +1,132 @@ +import { PublicShareChatToolsService } from './public-share-chat-tools.service'; + +/** + * Mock-based integration tests for the anonymous public-share toolset built by + * forShare(). Constructed directly with hand-rolled collaborators (no Nest/DB): + * - listSharePages tree assembly (dedupe, single-page root fallback, fail-soft); + * - the blank-input guards on search / read. + */ +describe('PublicShareChatToolsService.forShare', () => { + type ToolExec = { execute: (args: unknown) => Promise }; + + function makeService(over: { + getShareTree?: jest.Mock; + findById?: jest.Mock; + searchPage?: jest.Mock; + getShareForPage?: jest.Mock; + } = {}) { + const shareService = { + getShareTree: over.getShareTree ?? jest.fn(), + getShareForPage: over.getShareForPage ?? jest.fn(), + updatePublicAttachments: jest.fn(), + }; + const searchService = { searchPage: over.searchPage ?? jest.fn() }; + const pageRepo = { findById: over.findById ?? jest.fn() }; + const pagePermissionRepo = { hasRestrictedAncestor: jest.fn() }; + const svc = new PublicShareChatToolsService( + shareService as never, + searchService as never, + pageRepo as never, + pagePermissionRepo as never, + ); + return { svc, shareService, searchService, pageRepo, pagePermissionRepo }; + } + + describe('listSharePages', () => { + it('includeSubPages tree: returns deduped, titled pages (root already in tree)', async () => { + // getShareTree returns the share root + descendants; the root IS in the + // tree, so no extra title lookup is needed and the tree is listed as-is. + const { svc, pageRepo } = makeService({ + getShareTree: jest.fn().mockResolvedValue({ + share: { pageId: 'root' }, + pageTree: [ + { id: 'root', title: 'Home' }, + { id: 'child-1', title: 'Child One' }, + { id: 'child-2', title: 'Child Two' }, + ], + }), + }); + const tools = svc.forShare('SHARE-A', 'ws-1'); + const out = (await (tools.listSharePages as unknown as ToolExec).execute( + {}, + )) as Array<{ id: string; title: string }>; + expect(out).toEqual([ + { id: 'root', title: 'Home' }, + { id: 'child-1', title: 'Child One' }, + { id: 'child-2', title: 'Child Two' }, + ]); + // The root was already in the tree => no fallback title lookup. + expect(pageRepo.findById).not.toHaveBeenCalled(); + }); + + it('single-page share (empty tree): falls back to the root title and PREPENDS it', async () => { + const { svc, pageRepo } = makeService({ + getShareTree: jest.fn().mockResolvedValue({ + share: { pageId: 'root' }, + pageTree: [], // includeSubPages=false => empty tree + }), + findById: jest.fn().mockResolvedValue({ id: 'root', title: 'Solo Page' }), + }); + const tools = svc.forShare('SHARE-A', 'ws-1'); + const out = (await (tools.listSharePages as unknown as ToolExec).execute( + {}, + )) as Array<{ id: string; title: string }>; + expect(out).toEqual([{ id: 'root', title: 'Solo Page' }]); + expect(pageRepo.findById).toHaveBeenCalledWith('root'); + }); + + it('de-duplicates pages by id, keeping the first (titled) occurrence', async () => { + const { svc } = makeService({ + getShareTree: jest.fn().mockResolvedValue({ + share: { pageId: 'root' }, + pageTree: [ + { id: 'root', title: 'Home' }, + { id: 'dup', title: 'First' }, + { id: 'dup', title: 'Second (dropped)' }, + { id: 'root', title: 'Home again (dropped)' }, + ], + }), + }); + const tools = svc.forShare('SHARE-A', 'ws-1'); + const out = (await (tools.listSharePages as unknown as ToolExec).execute( + {}, + )) as Array<{ id: string; title: string }>; + expect(out).toEqual([ + { id: 'root', title: 'Home' }, + { id: 'dup', title: 'First' }, + ]); + }); + + it('getShareTree throws => returns [] (fail-soft, never throws to the model)', async () => { + const { svc } = makeService({ + getShareTree: jest.fn().mockRejectedValue(new Error('db down')), + }); + const tools = svc.forShare('SHARE-A', 'ws-1'); + await expect( + (tools.listSharePages as unknown as ToolExec).execute({}), + ).resolves.toEqual([]); + }); + }); + + describe('searchSharePages blank guard', () => { + it('blank query => [] WITHOUT calling searchService', async () => { + const { svc, searchService } = makeService({ searchPage: jest.fn() }); + const tools = svc.forShare('SHARE-A', 'ws-1'); + await expect( + (tools.searchSharePages as unknown as ToolExec).execute({ query: ' ' }), + ).resolves.toEqual([]); + expect(searchService.searchPage).not.toHaveBeenCalled(); + }); + }); + + describe('getSharePage blank guard', () => { + it('blank pageId => throws "A pageId is required." WITHOUT calling getShareForPage', async () => { + const { svc, shareService } = makeService({ getShareForPage: jest.fn() }); + const tools = svc.forShare('SHARE-A', 'ws-1'); + await expect( + (tools.getSharePage as unknown as ToolExec).execute({ pageId: ' ' }), + ).rejects.toThrow('A pageId is required.'); + expect(shareService.getShareForPage).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/server/src/core/auth/services/verify-user-credentials.live.spec.ts b/apps/server/src/core/auth/services/verify-user-credentials.live.spec.ts new file mode 100644 index 00000000..5504b3bd --- /dev/null +++ b/apps/server/src/core/auth/services/verify-user-credentials.live.spec.ts @@ -0,0 +1,233 @@ +import { UnauthorizedException } from '@nestjs/common'; +import { AuthService } from './auth.service'; +import { CREDENTIALS_MISMATCH_MESSAGE } from '../auth.constants'; +import { hashPassword } from '../../../common/helpers'; + +/** + * LIVE security contract for AuthService.verifyUserCredentials / login (M4 + * item 5). + * + * The (now-fixed) jest config CAN import AuthService at the module level (the + * `^src/(.*)$` moduleNameMapper resolves the transitive `src/...` imports and the + * ts-jest transform loads the graph). AuthService cannot be `.compile()`-d via + * the Nest TestingModule (its full provider graph is not wired here), but it can + * be constructed directly with mocked collaborators — which is exactly what we + * need to exercise the credential-check decision live. + * + * The load-bearing property: verifyUserCredentials (and login(), which reuses it) + * throws EXACTLY the shared CREDENTIALS_MISMATCH_MESSAGE for all three + * credentials-failure cases — unknown email, disabled user, wrong password. The + * /mcp Basic brute-force limiter only counts a failure when it recognises THIS + * exact message (isCredentialsFailure in mcp-auth.helpers matches the same shared + * constant); a reword that diverged here would silently turn /mcp Basic into an + * unthrottled password-guessing oracle. + */ + +const WORKSPACE_ID = 'ws-1'; + +// Build an AuthService with the dependencies verifyUserCredentials/login touch +// stubbed, and a userRepo whose findByEmail is overridable per test. Only the +// collaborators actually reached on these paths need real behaviour; the rest +// are inert mocks (constructor wiring only). +function makeAuthService(over: { + findByEmail?: jest.Mock; +} = {}): { + service: AuthService; + userRepo: { findByEmail: jest.Mock; updateLastLogin: jest.Mock }; + sessionService: { createSessionAndToken: jest.Mock }; + auditService: { log: jest.Mock }; +} { + const userRepo = { + findByEmail: over.findByEmail ?? jest.fn(), + updateLastLogin: jest.fn().mockResolvedValue(undefined), + }; + const sessionService = { + createSessionAndToken: jest.fn().mockResolvedValue('issued-token'), + }; + const auditService = { log: jest.fn() }; + // environmentService: isCloud() false (so throwIfEmailNotVerified does not + // require verification) + a stable app secret. + const environmentService = { + isCloud: jest.fn().mockReturnValue(false), + getAppSecret: jest.fn().mockReturnValue('test-secret'), + }; + + // Constructor signature (auth.service.ts): signupService, tokenService, + // sessionService, userSessionRepo, userRepo, userTokenRepo, mailService, + // domainService, environmentService, db, auditService. + const service = new (AuthService as unknown as new (...args: unknown[]) => AuthService)( + {}, // signupService + {}, // tokenService + sessionService, // sessionService + {}, // userSessionRepo + userRepo, // userRepo + {}, // userTokenRepo + {}, // mailService + {}, // domainService + environmentService, // environmentService + {}, // db + auditService, // auditService + ); + + return { service, userRepo, sessionService, auditService }; +} + +describe('AuthService.verifyUserCredentials (live credentials-mismatch contract)', () => { + it('UNKNOWN email -> throws exactly CREDENTIALS_MISMATCH_MESSAGE', async () => { + const { service } = makeAuthService({ + findByEmail: jest.fn().mockResolvedValue(undefined), + }); + + await expect( + service.verifyUserCredentials( + { email: 'nobody@example.com', password: 'whatever' }, + WORKSPACE_ID, + ), + ).rejects.toMatchObject({ message: CREDENTIALS_MISMATCH_MESSAGE }); + await expect( + service.verifyUserCredentials( + { email: 'nobody@example.com', password: 'whatever' }, + WORKSPACE_ID, + ), + ).rejects.toBeInstanceOf(UnauthorizedException); + }); + + it('DISABLED user -> throws exactly CREDENTIALS_MISMATCH_MESSAGE (no password oracle)', async () => { + // A deactivated user must be indistinguishable from a wrong password: same + // message, before any password comparison. + const passwordHash = await hashPassword('correct-horse'); + const disabledUser = { + id: 'u-1', + email: 'disabled@example.com', + password: passwordHash, + deactivatedAt: new Date(), + deletedAt: null, + emailVerifiedAt: new Date(), + }; + const { service } = makeAuthService({ + findByEmail: jest.fn().mockResolvedValue(disabledUser), + }); + + await expect( + service.verifyUserCredentials( + { email: 'disabled@example.com', password: 'correct-horse' }, + WORKSPACE_ID, + ), + ).rejects.toMatchObject({ message: CREDENTIALS_MISMATCH_MESSAGE }); + }); + + it('WRONG password -> throws exactly CREDENTIALS_MISMATCH_MESSAGE', async () => { + const passwordHash = await hashPassword('correct-horse'); + const user = { + id: 'u-1', + email: 'user@example.com', + password: passwordHash, + deactivatedAt: null, + deletedAt: null, + emailVerifiedAt: new Date(), + }; + const { service } = makeAuthService({ + findByEmail: jest.fn().mockResolvedValue(user), + }); + + await expect( + service.verifyUserCredentials( + { email: 'user@example.com', password: 'wrong-password' }, + WORKSPACE_ID, + ), + ).rejects.toMatchObject({ message: CREDENTIALS_MISMATCH_MESSAGE }); + }); + + it('CORRECT credentials -> resolves the matched user (no side effects here)', async () => { + const passwordHash = await hashPassword('correct-horse'); + const user = { + id: 'u-1', + email: 'user@example.com', + password: passwordHash, + deactivatedAt: null, + deletedAt: null, + emailVerifiedAt: new Date(), + }; + const { service, sessionService, auditService, userRepo } = + makeAuthService({ findByEmail: jest.fn().mockResolvedValue(user) }); + + const result = await service.verifyUserCredentials( + { email: 'user@example.com', password: 'correct-horse' }, + WORKSPACE_ID, + ); + expect(result).toBe(user); + // verifyUserCredentials is non-side-effecting: no session/audit/lastLogin. + expect(sessionService.createSessionAndToken).not.toHaveBeenCalled(); + expect(auditService.log).not.toHaveBeenCalled(); + expect(userRepo.updateLastLogin).not.toHaveBeenCalled(); + }); +}); + +describe('AuthService.login (live credentials-mismatch contract via verifyUserCredentials)', () => { + it('UNKNOWN email -> login throws exactly CREDENTIALS_MISMATCH_MESSAGE, mints NO session', async () => { + const { service, sessionService } = makeAuthService({ + findByEmail: jest.fn().mockResolvedValue(undefined), + }); + + await expect( + service.login( + { email: 'nobody@example.com', password: 'whatever' }, + WORKSPACE_ID, + ), + ).rejects.toMatchObject({ message: CREDENTIALS_MISMATCH_MESSAGE }); + expect(sessionService.createSessionAndToken).not.toHaveBeenCalled(); + }); + + it('WRONG password -> login throws exactly CREDENTIALS_MISMATCH_MESSAGE', async () => { + const passwordHash = await hashPassword('correct-horse'); + const user = { + id: 'u-1', + email: 'user@example.com', + password: passwordHash, + deactivatedAt: null, + deletedAt: null, + emailVerifiedAt: new Date(), + }; + const { service } = makeAuthService({ + findByEmail: jest.fn().mockResolvedValue(user), + }); + + await expect( + service.login( + { email: 'user@example.com', password: 'wrong-password' }, + WORKSPACE_ID, + ), + ).rejects.toMatchObject({ message: CREDENTIALS_MISMATCH_MESSAGE }); + }); + + it('CORRECT credentials -> login mints the session (the side-effecting path)', async () => { + const passwordHash = await hashPassword('correct-horse'); + const user = { + id: 'u-1', + email: 'user@example.com', + password: passwordHash, + deactivatedAt: null, + deletedAt: null, + emailVerifiedAt: new Date(), + }; + const { service, sessionService, auditService, userRepo } = + makeAuthService({ findByEmail: jest.fn().mockResolvedValue(user) }); + + await expect( + service.login( + { email: 'user@example.com', password: 'correct-horse' }, + WORKSPACE_ID, + ), + ).resolves.toBe('issued-token'); + // login() reuses verifyUserCredentials but DOES run the three side effects. + expect(userRepo.updateLastLogin).toHaveBeenCalledWith('u-1', WORKSPACE_ID); + expect(auditService.log).toHaveBeenCalled(); + expect(sessionService.createSessionAndToken).toHaveBeenCalledWith(user); + }); + + it('the message login throws is the SAME shared constant the /mcp limiter matches', () => { + // Cross-file coupling lock: the constant is the single source of truth shared + // by AuthService and mcp-auth.helpers.isCredentialsFailure. + expect(CREDENTIALS_MISMATCH_MESSAGE).toBe('Email or password does not match'); + }); +}); diff --git a/apps/server/src/core/page/transclusion/spec/page-embed.util.spec.ts b/apps/server/src/core/page/transclusion/spec/page-embed.util.spec.ts index 2bdca7b7..9219154c 100644 --- a/apps/server/src/core/page/transclusion/spec/page-embed.util.spec.ts +++ b/apps/server/src/core/page/transclusion/spec/page-embed.util.spec.ts @@ -80,6 +80,67 @@ describe('collectPageEmbedsFromPmJson', () => { }; expect(collectPageEmbedsFromPmJson(doc)).toEqual([]); }); + + it('ignores a pageEmbed whose sourcePageId is not a string', () => { + const doc = { + type: 'doc', + content: [ + { type: 'pageEmbed', attrs: { sourcePageId: 123 as any } }, + { type: 'pageEmbed', attrs: { sourcePageId: null as any } }, + { type: 'pageEmbed', attrs: { sourcePageId: { nested: true } as any } }, + { type: 'pageEmbed', attrs: { sourcePageId: ['arr'] as any } }, + // a valid one mixed in proves only the bad ones are dropped + { type: 'pageEmbed', attrs: { sourcePageId: 'good' } }, + ], + }; + expect(collectPageEmbedsFromPmJson(doc)).toEqual([ + { sourcePageId: 'good' }, + ]); + }); + + it('collects a pageEmbed nested under multiple block containers', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'callout', + content: [ + { + type: 'columns', + content: [ + { + type: 'column', + content: [ + { + type: 'details', + content: [ + { + type: 'pageEmbed', + attrs: { sourcePageId: 'deep' }, + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }; + expect(collectPageEmbedsFromPmJson(doc)).toEqual([{ sourcePageId: 'deep' }]); + }); + + it('terminates (does not silently hang) on a self-referencing/cyclic object', () => { + // FINDING: there is NO explicit cycle guard. A hand-built cyclic JS object + // (which cannot arise from JSON parsing — the real input path) makes the + // recursive walk overflow the stack and throw a RangeError. It TERMINATES + // with a controlled error rather than recursing unboundedly forever, and a + // non-cyclic (JSON-shaped) document is never affected. + const node: any = { type: 'doc', content: [] }; + node.content.push(node); // content array references its own parent node + expect(() => collectPageEmbedsFromPmJson(node)).toThrow(RangeError); + }); }); describe('pageEmbed HTML <-> JSON round-trip (server schema)', () => { diff --git a/apps/server/src/core/page/transclusion/spec/page-template-access.spec.ts b/apps/server/src/core/page/transclusion/spec/page-template-access.spec.ts index 3c497d80..2f37eb97 100644 --- a/apps/server/src/core/page/transclusion/spec/page-template-access.spec.ts +++ b/apps/server/src/core/page/transclusion/spec/page-template-access.spec.ts @@ -68,6 +68,7 @@ describe('TransclusionService — template access core (real filter)', () => { {} as any, // attachmentRepo {} as any, // storageService {} as any, // pageAccessService + {} as any, // workspaceRepo ); return { service, db, pageRepo, spaceMemberRepo, pagePermissionRepo }; @@ -187,8 +188,103 @@ describe('TransclusionService — template access core (real filter)', () => { }); }); +describe('TransclusionService.filterViewerAccessiblePageIds — AND ordering (content-leak control)', () => { + function makeDb(executeRows: Array<{ id: string }>) { + const builder: any = {}; + builder.selectFrom = jest.fn(() => builder); + builder.select = jest.fn(() => builder); + builder.where = jest.fn(() => builder); + builder.execute = jest.fn(async () => executeRows); + return builder; + } + + function makeService(opts: { + spaceVisibleRows: Array<{ id: string }>; + permissionAccessibleIds: string[]; + }) { + const db = makeDb(opts.spaceVisibleRows); + const spaceMemberRepo = { + getUserSpaceIdsQuery: jest.fn(() => ({ __subquery: true })), + }; + const filterAccessiblePageIds = jest + .fn() + .mockResolvedValue(opts.permissionAccessibleIds); + const pagePermissionRepo = { filterAccessiblePageIds }; + + const service = new TransclusionService( + db as any, // db + {} as any, // pageTransclusionsRepo + {} as any, // pageTransclusionReferencesRepo + {} as any, // pageTemplateReferencesRepo + {} as any, // pageRepo + pagePermissionRepo as any, + spaceMemberRepo as any, + {} as any, // attachmentRepo + {} as any, // storageService + {} as any, // pageAccessService + {} as any, // workspaceRepo + ); + + return { service, filterAccessiblePageIds }; + } + + it('space-visible AND permission-accessible → returned', async () => { + const { service } = makeService({ + spaceVisibleRows: [{ id: 'p1' }], + permissionAccessibleIds: ['p1'], + }); + const out = await service.filterViewerAccessiblePageIds( + ['p1'], + 'u1', + 'w1', + ); + expect(out).toEqual(['p1']); + }); + + it('space-visible but permission-rejected → dropped', async () => { + const { service, filterAccessiblePageIds } = makeService({ + spaceVisibleRows: [{ id: 'p1' }], + permissionAccessibleIds: [], + }); + const out = await service.filterViewerAccessiblePageIds( + ['p1'], + 'u1', + 'w1', + ); + expect(out).toEqual([]); + // The permission filter only ever sees the space-visible candidate. + expect(filterAccessiblePageIds).toHaveBeenCalledWith({ + pageIds: ['p1'], + userId: 'u1', + }); + }); + + it('NOT space-visible but permission-accessible → STILL dropped (AND-ordering enforced)', async () => { + // The page would pass page-level permission filtering, but it is not visible + // at the space level (e.g. a private space the viewer is not a member of). + // The space-visibility gate runs FIRST and short-circuits, so the page-level + // permission filter is never even consulted — preventing a private-space + // content leak via an unrestricted source page. + const { service, filterAccessiblePageIds } = makeService({ + spaceVisibleRows: [], + permissionAccessibleIds: ['private-but-permitted'], + }); + const out = await service.filterViewerAccessiblePageIds( + ['private-but-permitted'], + 'u1', + 'w1', + ); + expect(out).toEqual([]); + expect(filterAccessiblePageIds).not.toHaveBeenCalled(); + }); +}); + describe('TransclusionService.syncPageTemplateReferences — workspace scoping', () => { - function makeService(opts: { inWorkspaceIds: string[] }) { + function makeService(opts: { + inWorkspaceIds: string[]; + /** existing rows already persisted for the reference page */ + existingSourceIds?: string[]; + }) { // db stub: the in-workspace existence query returns only allowed ids. const builder: any = {}; builder.selectFrom = jest.fn(() => builder); @@ -201,25 +297,37 @@ describe('TransclusionService.syncPageTemplateReferences — workspace scoping', const insertMany = jest.fn().mockResolvedValue(undefined); const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined); const pageTemplateReferencesRepo = { - findByReferencePageId: jest.fn().mockResolvedValue([]), + findByReferencePageId: jest + .fn() + .mockResolvedValue( + (opts.existingSourceIds ?? []).map((sourcePageId) => ({ + sourcePageId, + })), + ), insertMany, deleteByReferenceAndSources, }; const service = new TransclusionService( - builder as any, - {} as any, - {} as any, + builder as any, // db + {} as any, // pageTransclusionsRepo + {} as any, // pageTransclusionReferencesRepo pageTemplateReferencesRepo as any, - {} as any, - {} as any, - {} as any, - {} as any, - {} as any, - {} as any, + {} as any, // pageRepo + {} as any, // pagePermissionRepo + {} as any, // spaceMemberRepo + {} as any, // attachmentRepo + {} as any, // storageService + {} as any, // pageAccessService + {} as any, // workspaceRepo ); - return { service, insertMany, pageTemplateReferencesRepo }; + return { + service, + insertMany, + deleteByReferenceAndSources, + pageTemplateReferencesRepo, + }; } function docWithEmbeds(sourceIds: string[]) { @@ -264,4 +372,150 @@ describe('TransclusionService.syncPageTemplateReferences — workspace scoping', expect(result.inserted).toBe(0); expect(insertMany).not.toHaveBeenCalled(); }); + + it('DELETE branch: an existing in-workspace ref removed from the doc is deleted', async () => { + // 'gone' was referenced before but is no longer in the doc; 'stay' remains. + const { service, insertMany, deleteByReferenceAndSources } = makeService({ + inWorkspaceIds: ['stay'], + existingSourceIds: ['stay', 'gone'], + }); + + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + docWithEmbeds(['stay']), + ); + + expect(result.deleted).toBe(1); + expect(result.inserted).toBe(0); // 'stay' already existed + expect(insertMany).not.toHaveBeenCalled(); + expect(deleteByReferenceAndSources).toHaveBeenCalledTimes(1); + expect(deleteByReferenceAndSources).toHaveBeenCalledWith( + 'host', + ['gone'], + undefined, // no trx supplied + ); + }); + + it('does NOT delete a stale ref whose source is now cross-workspace if it is also still embedded', async () => { + // Edge: 'x' is still embedded in the doc but no longer in-workspace. It is + // not in desiredIds (filtered out) AND it exists → it should be deleted, not + // kept, because the reference graph must drop the cross-workspace edge. + const { service, deleteByReferenceAndSources } = makeService({ + inWorkspaceIds: [], // 'x' no longer in-workspace + existingSourceIds: ['x'], + }); + + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + docWithEmbeds(['x']), + ); + + expect(result.deleted).toBe(1); + expect(deleteByReferenceAndSources).toHaveBeenCalledWith( + 'host', + ['x'], + undefined, + ); + }); +}); + +describe('TransclusionService.insertTemplateReferencesForPages — per-workspace existence validation', () => { + /** + * Smart db stub: each existence query is `.where('id','in', ids)` + + * `.where('workspaceId','=', wsId)`; `.execute()` returns only the ids that + * `validByWorkspace[wsId]` declares in-workspace. The builder snapshots the + * last `id`-in list and `workspaceId` value per chain (selectFrom resets). + */ + function makeDb(validByWorkspace: Record) { + const builder: any = {}; + let curIds: string[] = []; + let curWs: string | undefined; + builder.selectFrom = jest.fn(() => { + curIds = []; + curWs = undefined; + return builder; + }); + builder.select = jest.fn(() => builder); + builder.where = jest.fn((col: string, op: string, val: any) => { + if (col === 'id' && op === 'in') curIds = val; + if (col === 'workspaceId' && op === '=') curWs = val; + return builder; + }); + builder.execute = jest.fn(async () => { + const valid = new Set(validByWorkspace[curWs ?? ''] ?? []); + return curIds.filter((id) => valid.has(id)).map((id) => ({ id })); + }); + return builder; + } + + function makeService(validByWorkspace: Record) { + const insertMany = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { insertMany }; + const service = new TransclusionService( + makeDb(validByWorkspace) as any, // db + {} as any, // pageTransclusionsRepo + {} as any, // pageTransclusionReferencesRepo + pageTemplateReferencesRepo as any, + {} as any, // pageRepo + {} as any, // pagePermissionRepo + {} as any, // spaceMemberRepo + {} as any, // attachmentRepo + {} as any, // storageService + {} as any, // pageAccessService + {} as any, // workspaceRepo + ); + return { service, insertMany }; + } + + const embedDoc = (ids: string[]) => ({ + type: 'doc', + content: ids.map((id) => ({ + type: 'pageEmbed', + attrs: { sourcePageId: id }, + })), + }); + + it('validates each workspace separately: a source in-ws for A but cross-ws for B inserts only the valid delta', async () => { + // 'shared' is in-workspace for wA but NOT for wB. Page A embeds 'shared' + // (valid → inserted). Page B embeds 'shared' (cross-ws for wB → dropped). + const { service, insertMany } = makeService({ + wA: ['shared'], + wB: [], // 'shared' is not a page in wB + }); + + const result = await service.insertTemplateReferencesForPages([ + { id: 'pageA', workspaceId: 'wA', content: embedDoc(['shared']) }, + { id: 'pageB', workspaceId: 'wB', content: embedDoc(['shared']) }, + ]); + + expect(result.inserted).toBe(1); + expect(insertMany).toHaveBeenCalledTimes(1); + expect(insertMany.mock.calls[0][0]).toEqual([ + { workspaceId: 'wA', referencePageId: 'pageA', sourcePageId: 'shared' }, + ]); + }); + + it('inserts the in-workspace deltas for both pages when each is valid in its own workspace', async () => { + const { service, insertMany } = makeService({ + wA: ['a-src'], + wB: ['b-src'], + }); + + const result = await service.insertTemplateReferencesForPages([ + { id: 'pageA', workspaceId: 'wA', content: embedDoc(['a-src']) }, + { id: 'pageB', workspaceId: 'wB', content: embedDoc(['b-src']) }, + ]); + + expect(result.inserted).toBe(2); + const rows = insertMany.mock.calls[0][0]; + expect(rows).toEqual( + expect.arrayContaining([ + { workspaceId: 'wA', referencePageId: 'pageA', sourcePageId: 'a-src' }, + { workspaceId: 'wB', referencePageId: 'pageB', sourcePageId: 'b-src' }, + ]), + ); + expect(rows).toHaveLength(2); + }); }); diff --git a/apps/server/src/core/page/transclusion/spec/page-template-lookup.spec.ts b/apps/server/src/core/page/transclusion/spec/page-template-lookup.spec.ts index f62a047c..fbcd9486 100644 --- a/apps/server/src/core/page/transclusion/spec/page-template-lookup.spec.ts +++ b/apps/server/src/core/page/transclusion/spec/page-template-lookup.spec.ts @@ -1,4 +1,5 @@ import { TransclusionService } from '../transclusion.service'; +import * as collabUtil from '../../../../collaboration/collaboration.util'; /** * Exercises the pure access/mapping logic of `lookupTemplate`: @@ -34,6 +35,7 @@ describe('TransclusionService.lookupTemplate (access mapping)', () => { {} as any, // attachmentRepo {} as any, // storageService {} as any, // pageAccessService + {} as any, // workspaceRepo ); jest @@ -110,4 +112,61 @@ describe('TransclusionService.lookupTemplate (access mapping)', () => { expect((items[1] as any).status).toBeUndefined(); expect((items[2] as any).status).toBe('no_access'); }); + + // Content-prep failure path: if jsonToNode throws for an accessible page, the + // item must degrade to not_found and NEVER return content (which would + // otherwise carry the source's un-stripped comment marks). + describe('content-prep failure → not_found', () => { + let jsonToNodeSpy: jest.SpyInstance; + + afterEach(() => { + jsonToNodeSpy?.mockRestore(); + }); + + it('maps to not_found and returns no content when jsonToNode throws', async () => { + // The page is accessible and present, but content preparation blows up. + jsonToNodeSpy = jest + .spyOn(collabUtil, 'jsonToNode') + .mockImplementation(() => { + throw new Error('boom'); + }); + + const contentWithComment = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'text', + text: 'secret', + marks: [{ type: 'comment', attrs: { commentId: 'leak' } }], + }, + ], + }, + ], + }; + + const { service } = makeService({ + accessibleIds: ['p1'], + pages: [ + { + id: 'p1', + title: 'T', + icon: null, + content: contentWithComment, + updatedAt: now, + }, + ], + }); + + // Silence the service's error logger for the expected throw. + jest.spyOn((service as any).logger, 'error').mockImplementation(() => {}); + + const { items } = await service.lookupTemplate(['p1'], 'u1', 'w1'); + expect(items).toEqual([{ sourcePageId: 'p1', status: 'not_found' }]); + // Crucially: no content field, so no comment mark can leak. + expect((items[0] as any).content).toBeUndefined(); + }); + }); }); diff --git a/apps/server/src/core/page/transclusion/spec/page-template.controller.spec.ts b/apps/server/src/core/page/transclusion/spec/page-template.controller.spec.ts index 2de644e0..df340b13 100644 --- a/apps/server/src/core/page/transclusion/spec/page-template.controller.spec.ts +++ b/apps/server/src/core/page/transclusion/spec/page-template.controller.spec.ts @@ -1,7 +1,10 @@ import { Test } from '@nestjs/testing'; import { ForbiddenException, NotFoundException } from '@nestjs/common'; +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; import { PageTemplateController } from '../page-template.controller'; import { TransclusionService } from '../transclusion.service'; +import { TemplateLookupDto } from '../dto/template-lookup.dto'; import { PageRepo } from '@docmost/db/repos/page/page.repo'; import { PageAccessService } from '../../page-access/page-access.service'; import { JwtAuthGuard } from '../../../../common/guards/jwt-auth.guard'; @@ -90,4 +93,52 @@ describe('PageTemplateController.toggleTemplate', () => { ); expect(out).toEqual({ pageId: 'p1', isTemplate: false }); }); + + it('lookup forwards dto.sourcePageIds + user.id + user.workspaceId to the service', async () => { + const expected = { items: [] }; + (transclusionService.lookupTemplate as jest.Mock).mockResolvedValue( + expected, + ); + + const dto = { sourcePageIds: ['s1', 's2'] } as any; + const out = await controller.lookup(dto, user); + + expect(transclusionService.lookupTemplate).toHaveBeenCalledWith( + ['s1', 's2'], + 'u1', // user.id + 'w1', // user.workspaceId + ); + expect(out).toBe(expected); + }); +}); + +describe('TemplateLookupDto validation (class-validator)', () => { + const uuid = (n: number) => + `00000000-0000-4000-8000-${String(n).padStart(12, '0')}`; + + it('accepts an array of <=50 valid UUIDs', async () => { + const dto = plainToInstance(TemplateLookupDto, { + sourcePageIds: [uuid(1), uuid(2)], + }); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + }); + + it('rejects an over-cap array (ArrayMaxSize 50)', async () => { + const dto = plainToInstance(TemplateLookupDto, { + sourcePageIds: Array.from({ length: 51 }, (_, i) => uuid(i)), + }); + const errors = await validate(dto); + expect(errors).toHaveLength(1); + expect(errors[0].constraints).toHaveProperty('arrayMaxSize'); + }); + + it('rejects a non-UUID member (IsUUID each)', async () => { + const dto = plainToInstance(TemplateLookupDto, { + sourcePageIds: [uuid(1), 'not-a-uuid'], + }); + const errors = await validate(dto); + expect(errors).toHaveLength(1); + expect(errors[0].constraints).toHaveProperty('isUuid'); + }); }); diff --git a/apps/server/src/core/page/transclusion/spec/transclusion-unsync-html-embed.spec.ts b/apps/server/src/core/page/transclusion/spec/transclusion-unsync-html-embed.spec.ts index 8ad13121..4d149369 100644 --- a/apps/server/src/core/page/transclusion/spec/transclusion-unsync-html-embed.spec.ts +++ b/apps/server/src/core/page/transclusion/spec/transclusion-unsync-html-embed.spec.ts @@ -56,6 +56,7 @@ function buildService(featureEnabled = true) { {} as any, // db (unused on this path) pageTransclusionsRepo as any, pageTransclusionReferencesRepo as any, + {} as any, // pageTemplateReferencesRepo (unused on this path) pageRepo as any, {} as any, // pagePermissionRepo (unused) {} as any, // spaceMemberRepo (unused) diff --git a/apps/server/src/core/share/share-html-embed.spec.ts b/apps/server/src/core/share/share-html-embed.spec.ts index 1bfeff1c..162ba4ae 100644 --- a/apps/server/src/core/share/share-html-embed.spec.ts +++ b/apps/server/src/core/share/share-html-embed.spec.ts @@ -131,3 +131,131 @@ describe('ShareService htmlEmbed server-authoritative kill-switch (real code)', expect(hasHtmlEmbedNode(out)).toBe(true); }); }); + +// Exercises the REAL ShareService.lookupTransclusionForShare post-processing for +// the share-served transclusion path: the same server-authoritative htmlEmbed +// kill-switch must apply to each transcluded item's content, and a not_found +// item must never be run through prepareContentForShare (so its absent content +// can't be serialized/leaked). The access graph (shareRepo / isSharingAllowed / +// getShareForPage / restricted-ancestor) is stubbed so the strip/serve mapping +// runs deterministically; lookupWithAccessSet is mocked to control the items. +describe('ShareService.lookupTransclusionForShare htmlEmbed kill-switch (real code)', () => { + const SHARE = 'share-1'; + const SPACE = 'space-1'; + const SRC = 'src-page'; + + function buildTransclusionService(opts: { + htmlEmbed?: boolean | undefined; + items: any[]; + }) { + const shareRepo = { + findById: jest.fn(async () => ({ + id: SHARE, + workspaceId: WS, + spaceId: SPACE, + })), + }; + const pageRepo = { findById: jest.fn() }; + const pagePermissionRepo = { + hasRestrictedAncestor: jest.fn(async () => false), + }; + const tokenService = { + generateAttachmentToken: jest.fn(async () => 'tok'), + }; + const lookupWithAccessSet = jest.fn(async () => ({ items: opts.items })); + const transclusionService = { lookupWithAccessSet }; + const workspaceRepo = { + findById: jest.fn(async () => ({ + id: WS, + settings: { htmlEmbed: opts.htmlEmbed }, + })), + }; + + const service = new ShareService( + shareRepo as any, + pageRepo as any, + pagePermissionRepo as any, + {} as any, // db (unused — isSharingAllowed stubbed below) + tokenService as any, + transclusionService as any, + workspaceRepo as any, + ); + + // isSharingAllowed and getShareForPage hit the raw db; stub them so the + // access chain resolves SRC as reachable and prepareContentForShare runs. + jest.spyOn(service, 'isSharingAllowed').mockResolvedValue(true); + jest + .spyOn(service, 'getShareForPage') + .mockResolvedValue({ pageId: SRC, spaceId: SPACE, id: 's2' } as any); + + return { service, transclusionService, lookupWithAccessSet }; + } + + const transcludedItemWithEmbed = () => ({ + sourcePageId: SRC, + transclusionId: 't1', + content: { + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'block body' }] }, + { type: 'htmlEmbed', attrs: { source: '' } }, + ], + }, + sourceUpdatedAt: new Date('2026-06-20T00:00:00.000Z'), + }); + + const refs = [{ sourcePageId: SRC, transclusionId: 't1' }]; + + it('toggle OFF: strips htmlEmbed from each transcluded item content', async () => { + const { service } = buildTransclusionService({ + htmlEmbed: false, + items: [transcludedItemWithEmbed()], + }); + + const { items } = await service.lookupTransclusionForShare(SHARE, refs, WS); + expect(items).toHaveLength(1); + const item = items[0] as any; + expect(item.status).toBeUndefined(); + expect(hasHtmlEmbedNode(item.content)).toBe(false); + // Non-embed body of the transcluded block is preserved. + expect(JSON.stringify(item.content)).toContain('block body'); + }); + + it('toggle ON: serves htmlEmbed in the transcluded item content', async () => { + const { service } = buildTransclusionService({ + htmlEmbed: true, + items: [transcludedItemWithEmbed()], + }); + + const { items } = await service.lookupTransclusionForShare(SHARE, refs, WS); + const item = items[0] as any; + expect(item.status).toBeUndefined(); + expect(hasHtmlEmbedNode(item.content)).toBe(true); + expect(JSON.stringify(item.content)).toContain('block body'); + }); + + it('a not_found item is NOT run through prepareContentForShare (no token minting)', async () => { + const notFoundItem = { + sourcePageId: SRC, + transclusionId: 't1', + status: 'not_found' as const, + }; + const { service } = buildTransclusionService({ + htmlEmbed: true, + items: [notFoundItem], + }); + // tokenService is reachable via the service; spy on it to assert it is never + // touched for a status item (prepareContentForShare mints tokens). + const tokenSpy = jest.spyOn( + (service as any).tokenService, + 'generateAttachmentToken', + ); + + const { items } = await service.lookupTransclusionForShare(SHARE, refs, WS); + // not_found is collapsed to no_access for share viewers and carries NO content. + const item = items[0] as any; + expect(item.status).toBe('no_access'); + expect(item.content).toBeUndefined(); + expect(tokenSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/server/src/core/workspace/services/workspace-html-embed.spec.ts b/apps/server/src/core/workspace/services/workspace-html-embed.spec.ts new file mode 100644 index 00000000..fda0f5fa --- /dev/null +++ b/apps/server/src/core/workspace/services/workspace-html-embed.spec.ts @@ -0,0 +1,111 @@ +import { WorkspaceService } from './workspace.service'; + +/** + * Exercises the REAL WorkspaceService.update htmlEmbed-toggle persistence at the + * service seam: an update carrying `htmlEmbed` must call + * `workspaceRepo.updateSetting(workspaceId, 'htmlEmbed', value, trx)`, and an + * update WITHOUT it must not touch that setting. The repo, db transaction, and + * audit service are mocked; `executeTx` runs the callback against a fake trx. + * + * DEFERRED (DB-only): the "does not clobber sibling settings" guarantee is a + * jsonb merge property of `updateSetting`'s SQL and needs a real Postgres to + * assert. This spec only asserts the service-level CALL SHAPE. + */ +describe('WorkspaceService.update — htmlEmbed toggle persistence (real code)', () => { + function buildService(opts: { settingsBefore?: Record }) { + const updateSetting = jest.fn().mockResolvedValue(undefined); + const updateWorkspace = jest.fn().mockResolvedValue(undefined); + const workspaceRepo = { + // First call: read settingsBefore. Second call: return the updated + // workspace (must include a licenseKey because update() destructures it). + findById: jest + .fn() + .mockResolvedValueOnce({ id: 'w1', settings: opts.settingsBefore ?? {} }) + .mockResolvedValueOnce({ id: 'w1', name: 'WS', licenseKey: null }), + updateSetting, + updateWorkspace, + }; + + // Fake kysely db: only .transaction().execute(cb) is used on this path. + const db = { + transaction: jest.fn(() => ({ + execute: jest.fn(async (cb: any) => cb({ __trx: true })), + })), + }; + + const auditService = { log: jest.fn() }; + + const service = new WorkspaceService( + workspaceRepo as any, // workspaceRepo + {} as any, // spaceService + {} as any, // spaceMemberService + {} as any, // groupRepo + {} as any, // groupUserRepo + {} as any, // userRepo + {} as any, // environmentService + {} as any, // domainService + {} as any, // licenseCheckService + {} as any, // shareRepo + {} as any, // watcherRepo + {} as any, // favoriteRepo + db as any, // db (InjectKysely) + {} as any, // attachmentQueue + {} as any, // billingQueue + {} as any, // aiQueue + auditService as any, // auditService + {} as any, // userSessionRepo + ); + + return { service, workspaceRepo, updateSetting, auditService }; + } + + it('persists htmlEmbed:true via updateSetting with the htmlEmbed key', async () => { + const { service, updateSetting } = buildService({}); + + await service.update('w1', { htmlEmbed: true } as any); + + expect(updateSetting).toHaveBeenCalledTimes(1); + expect(updateSetting).toHaveBeenCalledWith( + 'w1', + 'htmlEmbed', + true, + expect.anything(), // the transaction handle + ); + }); + + it('persists htmlEmbed:false (explicit disable is not dropped)', async () => { + const { service, updateSetting } = buildService({ + settingsBefore: { htmlEmbed: true }, + }); + + await service.update('w1', { htmlEmbed: false } as any); + + expect(updateSetting).toHaveBeenCalledWith( + 'w1', + 'htmlEmbed', + false, + expect.anything(), + ); + }); + + it('does NOT call updateSetting when htmlEmbed is undefined in the dto', async () => { + const { service, updateSetting } = buildService({}); + + await service.update('w1', { name: 'New name' } as any); + + expect(updateSetting).not.toHaveBeenCalled(); + }); + + it('audits the htmlEmbed change (before/after) when the value actually changes', async () => { + const { service, auditService } = buildService({ + settingsBefore: { htmlEmbed: false }, + }); + + await service.update('w1', { htmlEmbed: true } as any); + + expect(auditService.log).toHaveBeenCalledTimes(1); + const logged = auditService.log.mock.calls[0][0]; + expect(logged.changes.before.htmlEmbed).toBe(false); + expect(logged.changes.after.htmlEmbed).toBe(true); + }); +}); diff --git a/apps/server/src/database/repos/ai-agent-roles/ai-agent-roles.repo.ts b/apps/server/src/database/repos/ai-agent-roles/ai-agent-roles.repo.ts index b5e4ed98..4adfa677 100644 --- a/apps/server/src/database/repos/ai-agent-roles/ai-agent-roles.repo.ts +++ b/apps/server/src/database/repos/ai-agent-roles/ai-agent-roles.repo.ts @@ -132,7 +132,7 @@ export class AiAgentRoleRepo { * generated column type is the broad `JsonValue` union, which a concrete object * type is not structurally assignable to. */ -function jsonbObject(value: ModelConfigValue | undefined) { +export function jsonbObject(value: ModelConfigValue | undefined) { if (value === null || value === undefined || Object.keys(value).length === 0) { return null; } diff --git a/apps/server/src/integrations/ai/ai-error.util.spec.ts b/apps/server/src/integrations/ai/ai-error.util.spec.ts index c9b7fb3e..414701d4 100644 --- a/apps/server/src/integrations/ai/ai-error.util.spec.ts +++ b/apps/server/src/integrations/ai/ai-error.util.spec.ts @@ -58,4 +58,26 @@ describe('describeProviderError', () => { // 'e | response body: ' + 300 chars + '…' expect(out.length).toBeLessThan('e | response body: '.length + 305); }); + + it('uses the fallback for a numeric or boolean (non-object, non-string) error', () => { + // typeof number / boolean is neither 'object' nor a non-empty 'string', so + // the early branch returns the fallback verbatim. + expect(describeProviderError(500, 'AI stream error')).toBe('AI stream error'); + expect(describeProviderError(0, 'AI stream error')).toBe('AI stream error'); + expect(describeProviderError(true)).toBe('Unknown error'); + expect(describeProviderError(false, 'fb')).toBe('fb'); + }); + + it('statusCode present but message undefined => ":" with no trailing space', () => { + // `${code}: ${undefined ?? ''}`.trim() collapses to just ":". + expect(describeProviderError({ statusCode: 503 })).toBe('503:'); + // The trailing space after the colon is trimmed away. + expect(describeProviderError({ statusCode: 503 }).endsWith(': ')).toBe(false); + }); + + it('object with neither message nor statusCode nor body => fallback', () => { + expect(describeProviderError({}, 'AI stream error')).toBe('AI stream error'); + // An object carrying only unrelated keys is still treated as message-less. + expect(describeProviderError({ foo: 'bar' } as never)).toBe('Unknown error'); + }); }); diff --git a/apps/server/src/integrations/ai/ai.service.spec.ts b/apps/server/src/integrations/ai/ai.service.spec.ts index 7bedc23a..ef44a59d 100644 --- a/apps/server/src/integrations/ai/ai.service.spec.ts +++ b/apps/server/src/integrations/ai/ai.service.spec.ts @@ -171,4 +171,117 @@ describe('AiService.getChatModel role model override', () => { expect(aiProviderCredentialsRepo.find).not.toHaveBeenCalled(); expect(secretBox.decryptSecret).not.toHaveBeenCalled(); }); + + /** + * Build a service whose workspace driver is ollama (no apiKey, with a baseUrl). + * Complements makeService (which configures openai) for the same-driver and + * not-configured ollama cases. + */ + function makeOllamaService(over: { baseUrl?: string } = {}) { + const aiSettings = { + resolve: jest.fn().mockResolvedValue({ + driver: 'ollama', + chatModel: 'llama3', + apiKey: undefined, + baseUrl: over.baseUrl ?? 'http://localhost:11434/v1', + }), + }; + const aiProviderCredentialsRepo = { find: jest.fn() }; + const secretBox = { decryptSecret: jest.fn() }; + const service = new AiService( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + aiSettings as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + aiProviderCredentialsRepo as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + secretBox as any, + ); + return { service, aiSettings, aiProviderCredentialsRepo, secretBox }; + } + + it('same-driver ollama override (workspace driver=ollama): reuses the workspace ollama baseUrl, no creds lookup/decrypt', async () => { + // Workspace driver IS ollama. A role that overrides to ollama (same driver) + // legitimately reuses the workspace's configured ollama endpoint — it must + // NOT hit the cross-driver 503 path, NOT query ai_provider_credentials, and + // NOT decrypt anything (ollama needs no key). + const { service, aiProviderCredentialsRepo, secretBox } = makeOllamaService(); + + const model = await service.getChatModel('ws-1', { + driver: 'ollama', + chatModel: 'llama3.1', + roleName: 'Local', + }); + + expect(model).toBeDefined(); + expect(aiProviderCredentialsRepo.find).not.toHaveBeenCalled(); + expect(secretBox.decryptSecret).not.toHaveBeenCalled(); + }); + + it('chatModel-only override on an ollama workspace: reuses the workspace ollama baseUrl, no creds lookup', async () => { + // No override.driver on an ollama workspace => the workspace ollama driver + + // baseUrl are reused; no creds lookup, no decrypt (the cheap public-share + // model-only override path against an ollama workspace). + const { service, aiProviderCredentialsRepo, secretBox } = makeOllamaService(); + + const model = await service.getChatModel('ws-1', { chatModel: 'mistral' }); + + expect(model).toBeDefined(); + expect(aiProviderCredentialsRepo.find).not.toHaveBeenCalled(); + expect(secretBox.decryptSecret).not.toHaveBeenCalled(); + }); + + it('blank chatModel guard: workspace has a driver but a blank chatModel and no override chatModel => AiNotConfiguredException', async () => { + // cfg.driver passes the first guard, but cfg.chatModel is blank and the + // override carries no chatModel, so the effective chatModel is empty. + const aiSettings = { + resolve: jest.fn().mockResolvedValue({ + driver: 'openai', + chatModel: '', + apiKey: 'workspace-key', + baseUrl: undefined, + }), + }; + const aiProviderCredentialsRepo = { find: jest.fn() }; + const secretBox = { decryptSecret: jest.fn() }; + const service = new AiService( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + aiSettings as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + aiProviderCredentialsRepo as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + secretBox as any, + ); + + await expect( + // Override has only a roleName, no chatModel to fill the blank. + service.getChatModel('ws-1', { roleName: 'Writer' }), + ).rejects.toBeInstanceOf(AiNotConfiguredException); + }); + + it('non-ollama driver with a missing apiKey => AiNotConfiguredException', async () => { + // Workspace is openai (non-ollama) with a model but NO apiKey: the combined + // `driver !== ollama && !apiKey` guard must 503. + const aiSettings = { + resolve: jest.fn().mockResolvedValue({ + driver: 'openai', + chatModel: 'gpt-4o-mini', + apiKey: undefined, + baseUrl: undefined, + }), + }; + const aiProviderCredentialsRepo = { find: jest.fn() }; + const secretBox = { decryptSecret: jest.fn() }; + const service = new AiService( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + aiSettings as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + aiProviderCredentialsRepo as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + secretBox as any, + ); + + await expect(service.getChatModel('ws-1')).rejects.toBeInstanceOf( + AiNotConfiguredException, + ); + }); }); diff --git a/apps/server/src/integrations/mcp/mcp-auth.helpers.ts b/apps/server/src/integrations/mcp/mcp-auth.helpers.ts index 4a0b5be1..0d1237e7 100644 --- a/apps/server/src/integrations/mcp/mcp-auth.helpers.ts +++ b/apps/server/src/integrations/mcp/mcp-auth.helpers.ts @@ -359,6 +359,111 @@ export function isInitializeRequestBody(body: unknown): boolean { return (body as { method?: unknown }).method === 'initialize'; } +/** + * The outcome of McpService.handle's pre-hijack gauntlet, as a pure value the + * caller acts on. Either send a JSON error with a fixed status (`respond`), or + * proceed to hijack the response and delegate to the MCP transport (`hijack`). + * Keeping this a pure decision (no FastifyReply, no res.hijack) makes the + * status/body mapping unit-testable, and guarantees no error path can leak the + * password or Authorization header — the body is only ever a fixed string or the + * UnauthorizedException's own message. + */ +export type McpHandleDecision = + | { kind: 'respond'; status: number; body: { error: string } } + | { kind: 'hijack' }; + +/** + * Pure mapping of McpService.handle's auth/enablement gauntlet to a response + * decision. Precedence mirrors handle(): + * 1. shared X-MCP-Token mismatch -> 401 {error:'Unauthorized'} (no hijack). + * 2. workspace MCP disabled -> 403 {error:'MCP is disabled ...'}. + * 3. resolveSessionConfig threw: + * - an UnauthorizedException -> 401 with err.message (a SPECIFIC reason; + * never the password/header — the message is the only thing surfaced). + * - any other error -> 500 generic 'Internal server error'. + * 4. otherwise (auth resolved) -> hijack and delegate to the transport. + */ +export function mapAuthResultToResponse(input: { + sharedTokenOk: boolean; + enabled: boolean; + error?: unknown; +}): McpHandleDecision { + if (!input.sharedTokenOk) { + return { kind: 'respond', status: 401, body: { error: 'Unauthorized' } }; + } + + if (!input.enabled) { + return { + kind: 'respond', + status: 403, + body: { error: 'MCP is disabled for this workspace' }, + }; + } + + if (input.error !== undefined) { + if (input.error instanceof UnauthorizedException) { + return { + kind: 'respond', + status: 401, + body: { error: input.error.message }, + }; + } + return { + kind: 'respond', + status: 500, + body: { error: 'Internal server error' }, + }; + } + + return { kind: 'hijack' }; +} + +// Result of the EE MFA module's requirement check for the Basic gate. Both +// flags absent/false means MFA does not block the password login. +export interface BasicGateMfaResult { + userHasMfa?: boolean; + requiresMfaSetup?: boolean; +} + +/** + * Pure decision logic for the /mcp HTTP-Basic pre-token gate, replicating EXACTLY + * what AuthController.login enforces before issuing a token, so the Basic path is + * not an SSO/MFA bypass. Framework-free (no ModuleRef, no on-disk EE MFA module) + * so the SSO/MFA decision is unit-testable in isolation: + * + * - `ssoEnforced` true -> throw Unauthorized ("enforced SSO"); a password + * login is not allowed on an SSO-enforced workspace. + * - otherwise, `mfa` is the EE MFA module's requirement result (or undefined + * when no EE MFA module is bundled — a community/fork build). If MFA is + * present and the user has MFA enabled OR needs MFA setup, throw Unauthorized + * telling the caller to use a Bearer access token (Basic cannot complete MFA). + * - no SSO + no MFA gate -> resolve (the Basic login is allowed to proceed). + * + * McpService.enforceBasicLoginGate wires the concrete `validateSsoEnforcement` + * result and the lazily-loaded MFA module result into this, so the gate decision + * itself carries no framework dependencies. Throws UnauthorizedException on + * rejection (surfaced as a clean 401); never logs the password. + */ +export function decideBasicGate(input: { + ssoEnforced: boolean; + mfa?: BasicGateMfaResult; +}): void { + if (input.ssoEnforced) { + throw new UnauthorizedException( + 'This workspace has enforced SSO login. Use SSO; MCP HTTP Basic is not allowed.', + ); + } + + const mfa = input.mfa; + if (mfa && (mfa.userHasMfa || mfa.requiresMfaSetup)) { + throw new UnauthorizedException( + 'This account requires multi-factor authentication. MCP HTTP Basic ' + + 'cannot complete MFA — log in normally and use a Bearer access token ' + + 'instead.', + ); + } +} + /** Extract a Bearer token from an Authorization header (case-insensitive). */ export function extractBearer( authHeader: string | undefined, diff --git a/apps/server/src/integrations/mcp/mcp.service.spec.ts b/apps/server/src/integrations/mcp/mcp.service.spec.ts index bf4c8a24..e8a57748 100644 --- a/apps/server/src/integrations/mcp/mcp.service.spec.ts +++ b/apps/server/src/integrations/mcp/mcp.service.spec.ts @@ -9,6 +9,9 @@ import { sharedTokenMatches, clientIp, bindAccessJwtVerifier, + extractBearer, + decideBasicGate, + mapAuthResultToResponse, McpAuthDeps, } from './mcp-auth.helpers'; import { JwtType } from '../../core/auth/dto/jwt-payload'; @@ -79,6 +82,26 @@ describe('parseBasicAuth', () => { }); }); +describe('extractBearer', () => { + it('extracts the token from a "Bearer " header', () => { + expect(extractBearer('Bearer abc.def.ghi')).toBe('abc.def.ghi'); + }); + + it('is case-insensitive on the scheme (lowercase + uppercase)', () => { + // The split keeps the token as-is; only the scheme is compared lowercased. + expect(extractBearer('bearer abc')).toBe('abc'); + expect(extractBearer('BEARER abc')).toBe('abc'); + }); + + it('returns undefined for a non-Bearer scheme (e.g. Basic)', () => { + expect(extractBearer('Basic abc')).toBeUndefined(); + }); + + it('returns undefined for an undefined header', () => { + expect(extractBearer(undefined)).toBeUndefined(); + }); +}); + describe('isCredentialsFailure', () => { it('is true for the credentials-mismatch UnauthorizedException', () => { expect( @@ -185,6 +208,43 @@ describe('FailedLoginLimiter', () => { expect(lim.isBlocked(k, 0)).toBe(true); expect(lim.isBlocked(k, 1000)).toBe(false); }); + + describe('sweep (expired-bucket eviction, injectable clock)', () => { + // sweep() drops buckets whose windowStart is older than windowMs so + // never-revisited keys cannot accumulate forever. It takes an injectable + // `now` so the behaviour is deterministic without faking timers. + it('drops a bucket strictly older than windowMs', () => { + const lim = new FailedLoginLimiter(5, 1000); + // Seed a bucket at t=0 (windowStart=0). + lim.recordFailure('stale', 0); + // Sweep well past the window: now - windowStart = 5000 >= 1000 -> dropped. + lim.sweep(5000); + // A dropped bucket means a brand-new bucket is created on next touch, so + // the prior failure count is gone (a single fresh failure is far from 5). + lim.recordFailure('stale', 5001); + expect(lim.isBlocked('stale', 5001)).toBe(false); + }); + + it('drops a bucket exactly at the windowMs boundary (>= is inclusive)', () => { + const lim = new FailedLoginLimiter(1, 1000); + lim.recordFailure('boundary', 0); // windowStart=0, blocked at threshold 1 + expect(lim.isBlocked('boundary', 0)).toBe(true); + // now - windowStart = 1000 == windowMs -> the >= check evicts it. + lim.sweep(1000); + // Re-touch at the same instant: a fresh bucket (count 0) is created, so the + // key is no longer blocked, proving the boundary bucket was swept. + expect(lim.isBlocked('boundary', 1000)).toBe(false); + }); + + it('retains a fresh bucket still within the window', () => { + const lim = new FailedLoginLimiter(1, 1000); + lim.recordFailure('fresh', 0); // windowStart=0 + // now - windowStart = 999 < 1000 -> the bucket survives the sweep. + lim.sweep(999); + // Still blocked because the bucket (and its count) was retained. + expect(lim.isBlocked('fresh', 999)).toBe(true); + }); + }); }); describe('verifyBearerAccess (Bearer revocation/disabled checks)', () => { @@ -769,3 +829,138 @@ describe('bindAccessJwtVerifier enforces JwtType.ACCESS (item 3)', () => { expect(res).toEqual({ sub: 'user-1', email: undefined }); }); }); + +describe('decideBasicGate (pure SSO/MFA pre-token gate, refactor R1)', () => { + // The pure decision extracted out of McpService.enforceBasicLoginGate. It is + // tested WITHOUT ModuleRef and WITHOUT an on-disk EE MFA module: the SSO verdict + // and the MFA requirement result are passed in as plain values. + + it('SSO enforced -> throws Unauthorized ("enforced SSO")', () => { + expect(() => decideBasicGate({ ssoEnforced: true })).toThrow( + UnauthorizedException, + ); + expect(() => decideBasicGate({ ssoEnforced: true })).toThrow(/enforced SSO/); + // SSO takes precedence even if MFA flags are also set. + expect(() => + decideBasicGate({ ssoEnforced: true, mfa: { userHasMfa: true } }), + ).toThrow(/enforced SSO/); + }); + + it('no SSO + no MFA module (mfa undefined) -> resolves (Basic allowed)', () => { + // A community/fork build with no EE MFA module passes mfa: undefined and the + // gate must allow the password login (same as the controller with no MFA). + expect(() => decideBasicGate({ ssoEnforced: false })).not.toThrow(); + expect(() => + decideBasicGate({ ssoEnforced: false, mfa: undefined }), + ).not.toThrow(); + }); + + it('MFA present + userHasMfa -> rejects ("use a Bearer access token")', () => { + expect(() => + decideBasicGate({ ssoEnforced: false, mfa: { userHasMfa: true } }), + ).toThrow(/use a Bearer access token/); + expect(() => + decideBasicGate({ ssoEnforced: false, mfa: { userHasMfa: true } }), + ).toThrow(UnauthorizedException); + }); + + it('MFA present + requiresMfaSetup -> rejects', () => { + expect(() => + decideBasicGate({ ssoEnforced: false, mfa: { requiresMfaSetup: true } }), + ).toThrow(/use a Bearer access token/); + }); + + it('MFA present but none required (both flags false) -> resolves', () => { + expect(() => + decideBasicGate({ + ssoEnforced: false, + mfa: { userHasMfa: false, requiresMfaSetup: false }, + }), + ).not.toThrow(); + }); +}); + +describe('mapAuthResultToResponse (handle status/body mapping, refactor R2)', () => { + // The pure response decision extracted out of McpService.handle. It maps the + // pre-hijack gauntlet (shared token, enablement, auth error) to either a fixed + // JSON error response or the hijack path — never leaking the password/header. + + it('wrong X-MCP-Token -> 401 {error:"Unauthorized"} and NOT the hijack path', () => { + const d = mapAuthResultToResponse({ sharedTokenOk: false, enabled: true }); + expect(d).toEqual({ + kind: 'respond', + status: 401, + body: { error: 'Unauthorized' }, + }); + }); + + it('workspace MCP disabled -> 403', () => { + const d = mapAuthResultToResponse({ sharedTokenOk: true, enabled: false }); + expect(d.kind).toBe('respond'); + if (d.kind === 'respond') { + expect(d.status).toBe(403); + expect(d.body).toEqual({ error: 'MCP is disabled for this workspace' }); + } + }); + + it('an UnauthorizedException -> 401 with err.message; no password/header leaked', () => { + // Construct an UnauthorizedException whose message is the SPECIFIC auth reason. + const err = new UnauthorizedException('Email or password does not match'); + const d = mapAuthResultToResponse({ + sharedTokenOk: true, + enabled: true, + error: err, + }); + expect(d).toEqual({ + kind: 'respond', + status: 401, + body: { error: 'Email or password does not match' }, + }); + // The surfaced body is ONLY the exception message — never the raw secret. + if (d.kind === 'respond') { + const serialized = JSON.stringify(d.body); + expect(serialized).not.toContain('password='); + expect(serialized).not.toContain('Authorization'); + expect(serialized).not.toContain('Basic '); + expect(serialized).not.toContain('Bearer '); + } + }); + + it('a non-Unauthorized error -> 500 generic (no error detail surfaced)', () => { + const err = new Error('db blew up: connection string secret'); + const d = mapAuthResultToResponse({ + sharedTokenOk: true, + enabled: true, + error: err, + }); + expect(d).toEqual({ + kind: 'respond', + status: 500, + body: { error: 'Internal server error' }, + }); + // The generic body must NOT echo the underlying error message. + if (d.kind === 'respond') { + expect(d.body.error).not.toContain('secret'); + } + }); + + it('happy path (auth resolved, no error) -> hijack', () => { + const d = mapAuthResultToResponse({ sharedTokenOk: true, enabled: true }); + expect(d).toEqual({ kind: 'hijack' }); + }); + + it('shared-token failure takes precedence over disabled/error', () => { + // Even with a disabled workspace and an error, a bad shared token is the + // first gate, so the response is the uniform 401 Unauthorized. + const d = mapAuthResultToResponse({ + sharedTokenOk: false, + enabled: false, + error: new UnauthorizedException('should not surface'), + }); + expect(d).toEqual({ + kind: 'respond', + status: 401, + body: { error: 'Unauthorized' }, + }); + }); +}); diff --git a/apps/server/src/integrations/mcp/mcp.service.ts b/apps/server/src/integrations/mcp/mcp.service.ts index 7ac16fb6..0af88c65 100644 --- a/apps/server/src/integrations/mcp/mcp.service.ts +++ b/apps/server/src/integrations/mcp/mcp.service.ts @@ -25,6 +25,8 @@ import { sharedTokenMatches, clientIp, bindAccessJwtVerifier, + decideBasicGate, + mapAuthResultToResponse, DocmostMcpConfig, ResolvedMcpAuth, } from './mcp-auth.helpers'; @@ -231,49 +233,54 @@ export class McpService implements OnModuleDestroy { workspace: Workspace, creds: { email: string; password: string }, ): Promise { - // 1) SSO enforcement. validateSsoEnforcement throws BadRequestException; we - // re-surface it as Unauthorized so the /mcp 401 path is consistent and a - // token is never issued. + // 1) SSO enforcement. validateSsoEnforcement throws when the workspace + // enforces SSO; we only need the boolean verdict for the pure decision. + let ssoEnforced = false; try { validateSsoEnforcement(workspace); } catch { - throw new UnauthorizedException( - 'This workspace has enforced SSO login. Use SSO; MCP HTTP Basic is not allowed.', - ); + ssoEnforced = true; } // 2) MFA gate — lazy-require the EE module exactly like AuthController.login. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let MfaModule: any; - try { - // eslint-disable-next-line @typescript-eslint/no-require-imports - MfaModule = require('./../../ee/mfa/services/mfa.service'); - } catch { - // No EE MFA module bundled in this build: same as the controller -> no - // MFA gate. (A community/fork build has no MFA, so Basic is allowed.) - return; + // On a fork WITHOUT the EE module bundled, mfaResult stays undefined and the + // pure gate behaves exactly like the controller (no MFA module -> no MFA + // gate). We only LOAD the module + read the requirement flags here; the + // accept/reject decision lives in the framework-free decideBasicGate so the + // SSO/MFA logic is unit-testable without ModuleRef or the on-disk EE module. + let mfaResult: { userHasMfa?: boolean; requiresMfaSetup?: boolean } | undefined; + // Only consult the MFA module when SSO has not already disqualified the + // request (SSO short-circuits, and skipping the load avoids a needless + // require on the SSO-reject path). + if (!ssoEnforced) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let MfaModule: any; + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + MfaModule = require('./../../ee/mfa/services/mfa.service'); + } catch { + // No EE MFA module bundled in this build: same as the controller -> no + // MFA gate. (A community/fork build has no MFA, so Basic is allowed.) + MfaModule = undefined; + } + + if (MfaModule) { + const mfaService = this.moduleRef.get(MfaModule.MfaService, { + strict: false, + }); + // Same requirement check the controller uses. We pass NO FastifyReply + // (the controller passes `res` only to set a cookie on the no-MFA happy + // path, which we never take here): we only read the requirement flags. + mfaResult = await mfaService.checkMfaRequirements( + creds, + workspace, + undefined, + ); + } } - const mfaService = this.moduleRef.get(MfaModule.MfaService, { - strict: false, - }); - // Use the same requirement check the controller uses. We pass NO FastifyReply - // (the controller passes `res` only to set a cookie on the no-MFA happy path, - // which we never take here): we only read the requirement flags. Be tolerant - // of either a (loginInput, workspace) or (loginInput, workspace, res) shape. - const mfaResult = await mfaService.checkMfaRequirements( - creds, - workspace, - undefined, - ); - - if (mfaResult && (mfaResult.userHasMfa || mfaResult.requiresMfaSetup)) { - throw new UnauthorizedException( - 'This account requires multi-factor authentication. MCP HTTP Basic ' + - 'cannot complete MFA — log in normally and use a Bearer access token ' + - 'instead.', - ); - } + // Pure accept/reject decision (throws UnauthorizedException on rejection). + decideBasicGate({ ssoEnforced, mfa: mfaResult }); } // Lazily create the HTTP handler exactly once. The import is indirected so @@ -333,52 +340,61 @@ export class McpService implements OnModuleDestroy { // matching `X-MCP-Token` header. It now lives in its OWN header so it never // collides with `Authorization`, which carries the per-user credentials. const sharedToken = process.env.MCP_TOKEN; - if (sharedToken) { - const provided = req.headers['x-mcp-token']; - if (!sharedTokenMatches(sharedToken, provided)) { - res.status(401).send({ error: 'Unauthorized' }); - return; - } - } + const sharedTokenOk = sharedToken + ? sharedTokenMatches(sharedToken, req.headers['x-mcp-token']) + : true; - if (!(await this.isEnabled())) { - res.status(403).send({ error: 'MCP is disabled for this workspace' }); - return; - } + // Short-circuit checks (shared token, enablement) that do not need the auth + // resolution. Compute them up front so the response mapping is a single pure + // decision (mapAuthResultToResponse) that cannot leak the password/header. + const enabled = sharedTokenOk ? await this.isEnabled() : false; // Resolve + validate the per-session identity BEFORE hijacking the response // so bad credentials surface as a clean 401 JSON (never a torn response and // never a generic "MCP error"). The resolved config/identity is stashed on // the raw request for the package's resolver + identify hook to read back. - let resolved: ResolvedMcpAuth; - try { - resolved = await this.resolveSessionConfig(req); - } catch (err) { - if (err instanceof UnauthorizedException) { - // Warn once if the only thing missing is the service account, to keep - // the original operator hint. - if ( - !this.credsConfigured() && - !req.headers['authorization'] && - !this.warnedMissingCreds - ) { - this.warnedMissingCreds = true; - this.logger.warn( - 'MCP is enabled but received a request with no credentials and no ' + - 'MCP_DOCMOST_EMAIL/MCP_DOCMOST_PASSWORD service account configured.', - ); + let resolved: ResolvedMcpAuth | undefined; + let authError: unknown; + if (sharedTokenOk && enabled) { + try { + resolved = await this.resolveSessionConfig(req); + } catch (err) { + authError = err; + if (err instanceof UnauthorizedException) { + // Warn once if the only thing missing is the service account, to keep + // the original operator hint. + if ( + !this.credsConfigured() && + !req.headers['authorization'] && + !this.warnedMissingCreds + ) { + this.warnedMissingCreds = true; + this.logger.warn( + 'MCP is enabled but received a request with no credentials and no ' + + 'MCP_DOCMOST_EMAIL/MCP_DOCMOST_PASSWORD service account configured.', + ); + } + } else { + this.logger.error('MCP auth resolution failed', err as Error); } - res.status(401).send({ error: err.message }); - return; } - this.logger.error('MCP auth resolution failed', err as Error); - res.status(500).send({ error: 'Internal server error' }); + } + + // Pure status/body mapping for the whole pre-hijack gauntlet. + const decision = mapAuthResultToResponse({ + sharedTokenOk, + enabled, + error: authError, + }); + if (decision.kind === 'respond') { + res.status(decision.status).send(decision.body); return; } // Stash the resolved auth on the raw request so the package's resolver + // identify hook (wired in getHandler) read it back instead of re-parsing. - (req.raw as unknown as Record)[MCP_RESOLVED] = resolved; + (req.raw as unknown as Record)[MCP_RESOLVED] = + resolved as ResolvedMcpAuth; // Hand the raw Node req/res to the MCP transport. hijack() tells Fastify // to stop managing this response so the transport can write to it directly. diff --git a/apps/server/src/ws/listeners/page-ws.listener.spec.ts b/apps/server/src/ws/listeners/page-ws.listener.spec.ts index 734e8228..3282d318 100644 --- a/apps/server/src/ws/listeners/page-ws.listener.spec.ts +++ b/apps/server/src/ws/listeners/page-ws.listener.spec.ts @@ -3,6 +3,7 @@ import { PageWsListener } from './page-ws.listener'; import { WsTreeService } from '../ws-tree.service'; import { PageEvent, + PageMovedEvent, TreeNodeSnapshot, } from '../../database/listeners/page.listener'; @@ -93,3 +94,139 @@ describe('PageWsListener.onPageCreated', () => { expect(wsTree.broadcastRefetchRoot).not.toHaveBeenCalled(); }); }); + +describe('PageWsListener delete/move/restore handlers', () => { + let listener: PageWsListener; + let wsTree: { + broadcastPageCreated: jest.Mock; + broadcastPageDeleted: jest.Mock; + broadcastPageMoved: jest.Mock; + broadcastRefetchRoot: jest.Mock; + }; + let warnSpy: jest.SpyInstance; + + const secondSnapshot: TreeNodeSnapshot = { + id: 'page-2', + slugId: 'slug-2', + title: 'World', + icon: '📁', + position: 'a2', + spaceId: 'space-1', + parentPageId: null, + }; + + beforeEach(async () => { + wsTree = { + broadcastPageCreated: jest.fn().mockResolvedValue(undefined), + broadcastPageDeleted: jest.fn().mockResolvedValue(undefined), + broadcastPageMoved: jest.fn().mockResolvedValue(undefined), + broadcastRefetchRoot: jest.fn().mockResolvedValue(undefined), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + PageWsListener, + { provide: WsTreeService, useValue: wsTree }, + ], + }).compile(); + + listener = module.get(PageWsListener); + // The PAGE_RESTORED-without-spaceId branch logs a warning; silence + assert. + warnSpy = jest + .spyOn(listener['logger'], 'warn') + .mockImplementation(() => undefined); + }); + + afterEach(() => { + warnSpy.mockRestore(); + }); + + // --- onPageDeleted (PAGE_SOFT_DELETED / PAGE_DELETED) --- + + it('onPageDeleted with N `pages`: one broadcastPageDeleted per page', async () => { + const event: PageEvent = { + pageIds: ['page-1', 'page-2'], + workspaceId: 'ws-1', + pages: [snapshot, secondSnapshot], + }; + + await listener.onPageDeleted(event); + + expect(wsTree.broadcastPageDeleted).toHaveBeenCalledTimes(2); + expect(wsTree.broadcastPageDeleted).toHaveBeenNthCalledWith(1, snapshot); + expect(wsTree.broadcastPageDeleted).toHaveBeenNthCalledWith( + 2, + secondSnapshot, + ); + }); + + it('onPageDeleted with an EMPTY `pages` array: no broadcast', async () => { + const event: PageEvent = { + pageIds: ['page-1'], + workspaceId: 'ws-1', + pages: [], + }; + + await listener.onPageDeleted(event); + + expect(wsTree.broadcastPageDeleted).not.toHaveBeenCalled(); + }); + + it('onPageDeleted with UNDEFINED `pages`: no broadcast (no crash)', async () => { + const event: PageEvent = { + pageIds: ['page-1'], + workspaceId: 'ws-1', + }; + + await listener.onPageDeleted(event); + + expect(wsTree.broadcastPageDeleted).not.toHaveBeenCalled(); + }); + + // --- onPageMoved (PAGE_MOVED) --- + + it('onPageMoved: forwards the whole event to a single broadcastPageMoved', async () => { + const event: PageMovedEvent = { + workspaceId: 'ws-1', + oldParentId: 'old-parent', + hasChildren: false, + node: { ...snapshot, parentPageId: 'new-parent', position: 'a5' }, + }; + + await listener.onPageMoved(event); + + expect(wsTree.broadcastPageMoved).toHaveBeenCalledTimes(1); + expect(wsTree.broadcastPageMoved).toHaveBeenCalledWith(event); + }); + + // --- onPageRestored (PAGE_RESTORED) --- + + it('onPageRestored WITHOUT spaceId: warns and does NOT refetch', async () => { + const event: PageEvent = { + pageIds: ['page-1'], + workspaceId: 'ws-1', + }; + + await listener.onPageRestored(event); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('PAGE_RESTORED'), + ); + expect(wsTree.broadcastRefetchRoot).not.toHaveBeenCalled(); + }); + + it('onPageRestored WITH spaceId: one broadcastRefetchRoot scoped to the space', async () => { + const event: PageEvent = { + pageIds: ['page-1'], + workspaceId: 'ws-1', + spaceId: 'space-9', + }; + + await listener.onPageRestored(event); + + expect(warnSpy).not.toHaveBeenCalled(); + expect(wsTree.broadcastRefetchRoot).toHaveBeenCalledTimes(1); + expect(wsTree.broadcastRefetchRoot).toHaveBeenCalledWith('space-9'); + }); +}); diff --git a/apps/server/src/ws/ws-service.spec.ts b/apps/server/src/ws/ws-service.spec.ts new file mode 100644 index 00000000..c87d1493 --- /dev/null +++ b/apps/server/src/ws/ws-service.spec.ts @@ -0,0 +1,259 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; +import { WsService } from './ws.service'; +import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo'; +import { + getSpaceRoomName, + WS_SPACE_RESTRICTION_CACHE_PREFIX, + WS_CACHE_TTL_MS, +} from './ws.utils'; + +/** + * WsService server-side unit tests (M7 item 2): + * - spaceHasRestrictions cache lifecycle (miss -> read+set with TTL; hit -> + * no re-read; documents the stale-false window). + * - broadcastToAuthorizedUsers fan-out (authorized-only delivery, multi-socket + * fan-out per user, sockets with no userId skipped). + * + * Both private methods are exercised through their public entry points: + * spaceHasRestrictions via emitTreeEvent, broadcastToAuthorizedUsers via + * emitToAuthorizedUsers. WsService is constructed with mocked cache + repo and a + * mocked socket.io server, so no live infra is needed. + */ + +describe('WsService.spaceHasRestrictions (cache lifecycle, via emitTreeEvent)', () => { + let service: WsService; + let pagePermissionRepo: { + hasRestrictedPagesInSpace: jest.Mock; + hasRestrictedAncestor: jest.Mock; + getUserIdsWithPageAccess: jest.Mock; + }; + let cache: { get: jest.Mock; set: jest.Mock; del: jest.Mock }; + let roomEmit: jest.Mock; + + beforeEach(async () => { + pagePermissionRepo = { + hasRestrictedPagesInSpace: jest.fn(), + hasRestrictedAncestor: jest.fn(), + getUserIdsWithPageAccess: jest.fn(), + }; + cache = { + get: jest.fn().mockResolvedValue(null), + set: jest.fn().mockResolvedValue(undefined), + del: jest.fn().mockResolvedValue(undefined), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + WsService, + { provide: PagePermissionRepo, useValue: pagePermissionRepo }, + { provide: CACHE_MANAGER, useValue: cache }, + ], + }).compile(); + + service = module.get(WsService); + + roomEmit = jest.fn(); + const server = { + to: jest.fn().mockReturnValue({ emit: roomEmit }), + in: jest.fn().mockReturnValue({ fetchSockets: jest.fn() }), + }; + service.setServer(server as never); + }); + + const cacheKey = (spaceId: string): string => + `${WS_SPACE_RESTRICTION_CACHE_PREFIX}${spaceId}`; + + it('first call MISSES the cache -> reads the repo and sets it with WS_CACHE_TTL_MS', async () => { + cache.get.mockResolvedValue(null); // miss + pagePermissionRepo.hasRestrictedPagesInSpace.mockResolvedValue(true); + pagePermissionRepo.hasRestrictedAncestor.mockResolvedValue(false); + + await service.emitTreeEvent('space-1', 'page-1', { op: 'x' }); + + expect(cache.get).toHaveBeenCalledWith(cacheKey('space-1')); + expect(pagePermissionRepo.hasRestrictedPagesInSpace).toHaveBeenCalledTimes(1); + expect(pagePermissionRepo.hasRestrictedPagesInSpace).toHaveBeenCalledWith( + 'space-1', + ); + // The freshly-read verdict is cached with the 30s TTL. + expect(cache.set).toHaveBeenCalledWith( + cacheKey('space-1'), + true, + WS_CACHE_TTL_MS, + ); + }); + + it('second call HITS the cache -> the repo is NOT re-read', async () => { + // Cache hit returns false (no restrictions) -> open-space fast path. + cache.get.mockResolvedValue(false); + + await service.emitTreeEvent('space-1', 'page-1', { op: 'x' }); + + expect(cache.get).toHaveBeenCalledWith(cacheKey('space-1')); + // The whole point of the cache: no repo read on a hit. + expect(pagePermissionRepo.hasRestrictedPagesInSpace).not.toHaveBeenCalled(); + expect(cache.set).not.toHaveBeenCalled(); + // false verdict -> broadcast to the whole room (open-space fast path). + expect(roomEmit).toHaveBeenCalledWith('message', { op: 'x' }); + }); + + it('a cached `false` is returned even when restrictions now exist (the stale window)', async () => { + // The cache says "no restrictions" (false) but the repo, if asked, would now + // say true. spaceHasRestrictions trusts the cached false and never re-reads — + // this documents the up-to-TTL stale window the production comment warns about + // (a payload can fan out room-wide until the cache is invalidated/expires). + cache.get.mockResolvedValue(false); + pagePermissionRepo.hasRestrictedPagesInSpace.mockResolvedValue(true); + + await service.emitTreeEvent('space-1', 'page-1', { op: 'stale' }); + + expect(pagePermissionRepo.hasRestrictedPagesInSpace).not.toHaveBeenCalled(); + // Treated as open -> the event is broadcast to the WHOLE room. + expect(roomEmit).toHaveBeenCalledWith('message', { op: 'stale' }); + }); + + it('caches a `false` verdict too (so the next emit hits, not re-reads)', async () => { + cache.get.mockResolvedValueOnce(null); // first call: miss + pagePermissionRepo.hasRestrictedPagesInSpace.mockResolvedValue(false); + + await service.emitTreeEvent('space-2', 'page-9', { op: 'y' }); + + expect(cache.set).toHaveBeenCalledWith( + cacheKey('space-2'), + false, + WS_CACHE_TTL_MS, + ); + }); +}); + +describe('WsService.broadcastToAuthorizedUsers fan-out (via emitToAuthorizedUsers)', () => { + let service: WsService; + let pagePermissionRepo: { + hasRestrictedPagesInSpace: jest.Mock; + hasRestrictedAncestor: jest.Mock; + getUserIdsWithPageAccess: jest.Mock; + }; + let cache: { get: jest.Mock; set: jest.Mock; del: jest.Mock }; + let fetchSockets: jest.Mock; + let serverIn: jest.Mock; + + beforeEach(async () => { + pagePermissionRepo = { + hasRestrictedPagesInSpace: jest.fn(), + hasRestrictedAncestor: jest.fn(), + getUserIdsWithPageAccess: jest.fn(), + }; + cache = { + get: jest.fn().mockResolvedValue(null), + set: jest.fn().mockResolvedValue(undefined), + del: jest.fn().mockResolvedValue(undefined), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + WsService, + { provide: PagePermissionRepo, useValue: pagePermissionRepo }, + { provide: CACHE_MANAGER, useValue: cache }, + ], + }).compile(); + + service = module.get(WsService); + + fetchSockets = jest.fn(); + serverIn = jest.fn().mockReturnValue({ fetchSockets }); + const server = { + to: jest.fn().mockReturnValue({ emit: jest.fn() }), + in: serverIn, + }; + service.setServer(server as never); + }); + + it('only sockets whose userId is in getUserIdsWithPageAccess receive the event', async () => { + pagePermissionRepo.getUserIdsWithPageAccess.mockResolvedValue(['user-ok']); + + const okEmit = jest.fn(); + const noEmit = jest.fn(); + fetchSockets.mockResolvedValue([ + { id: 's1', data: { userId: 'user-ok' }, emit: okEmit }, + { id: 's2', data: { userId: 'user-no' }, emit: noEmit }, + ]); + + const data = { operation: 'moveTreeNode' }; + await service.emitToAuthorizedUsers('space-1', 'page-1', data); + + // The authorized set is resolved from the candidate userIds present on the + // sockets (deduped), then only those users' sockets get the event. + expect(pagePermissionRepo.getUserIdsWithPageAccess).toHaveBeenCalledWith( + 'page-1', + expect.arrayContaining(['user-ok', 'user-no']), + ); + expect(okEmit).toHaveBeenCalledWith('message', data); + expect(noEmit).not.toHaveBeenCalled(); + }); + + it('a user with TWO sockets receives the event on BOTH (userSocketMap fan-out)', async () => { + pagePermissionRepo.getUserIdsWithPageAccess.mockResolvedValue(['user-ok']); + + const tab1 = jest.fn(); + const tab2 = jest.fn(); + fetchSockets.mockResolvedValue([ + { id: 's1', data: { userId: 'user-ok' }, emit: tab1 }, + { id: 's2', data: { userId: 'user-ok' }, emit: tab2 }, + ]); + + const data = { operation: 'moveTreeNode' }; + await service.emitToAuthorizedUsers('space-1', 'page-1', data); + + // Both of the authorized user's sockets (e.g. two browser tabs) receive it. + expect(tab1).toHaveBeenCalledWith('message', data); + expect(tab2).toHaveBeenCalledWith('message', data); + // The candidate set is deduped to a single userId even with two sockets. + expect(pagePermissionRepo.getUserIdsWithPageAccess).toHaveBeenCalledWith( + 'page-1', + ['user-ok'], + ); + }); + + it('a socket with NO userId is skipped (not a candidate, never emitted to)', async () => { + pagePermissionRepo.getUserIdsWithPageAccess.mockResolvedValue(['user-ok']); + + const okEmit = jest.fn(); + const anonEmit = jest.fn(); + fetchSockets.mockResolvedValue([ + { id: 's1', data: { userId: 'user-ok' }, emit: okEmit }, + // Unauthenticated socket: no userId -> excluded from the candidate map. + { id: 's2', data: {}, emit: anonEmit }, + ]); + + const data = { operation: 'moveTreeNode' }; + await service.emitToAuthorizedUsers('space-1', 'page-1', data); + + expect(okEmit).toHaveBeenCalledWith('message', data); + expect(anonEmit).not.toHaveBeenCalled(); + // The no-userId socket is not even offered as a candidate to the repo. + expect(pagePermissionRepo.getUserIdsWithPageAccess).toHaveBeenCalledWith( + 'page-1', + ['user-ok'], + ); + }); + + it('no sockets in the room -> no repo lookup, no emit', async () => { + fetchSockets.mockResolvedValue([]); + + await service.emitToAuthorizedUsers('space-1', 'page-1', { op: 'x' }); + + expect(pagePermissionRepo.getUserIdsWithPageAccess).not.toHaveBeenCalled(); + }); + + it('routes through the space room name', async () => { + pagePermissionRepo.getUserIdsWithPageAccess.mockResolvedValue([]); + fetchSockets.mockResolvedValue([ + { id: 's1', data: { userId: 'u' }, emit: jest.fn() }, + ]); + + await service.emitToAuthorizedUsers('space-7', 'page-1', { op: 'x' }); + + expect(serverIn).toHaveBeenCalledWith(getSpaceRoomName('space-7')); + }); +}); diff --git a/apps/server/src/ws/ws-tree.service.spec.ts b/apps/server/src/ws/ws-tree.service.spec.ts index 0c511223..973e6b00 100644 --- a/apps/server/src/ws/ws-tree.service.spec.ts +++ b/apps/server/src/ws/ws-tree.service.spec.ts @@ -329,3 +329,109 @@ describe('WsService.emitTreeEvent', () => { expect(anonEmit).toHaveBeenCalledWith('message', data); }); }); + +describe('move-into-restricted disjointness contract (WsTreeService + real WsService)', () => { + // CONTRACT: a move under a restricted ancestor PARTITIONS the room. The + // authorized set (gets the moveTreeNode via emitToAuthorizedUsers) and its + // complement (gets the deleteTreeNode via emitDeleteToUnauthorized) are + // disjoint and together cover every socket — and an anonymous (no-userId) + // socket lands in the delete set. We wire a REAL WsService (only its repo, + // cache and socket server mocked) so both broadcasts run against the SAME fixed + // socket set, the way they do in production. + let treeService: WsTreeService; + let pagePermissionRepo: { + hasRestrictedPagesInSpace: jest.Mock; + hasRestrictedAncestor: jest.Mock; + getUserIdsWithPageAccess: jest.Mock; + }; + + // Fixed room: two authorized users (one with two sockets), one unauthorized + // user, one anonymous socket. + const moveSeen: string[] = []; + const deleteSeen: string[] = []; + + const mkSocket = (id: string, userId: string | undefined) => ({ + id, + data: userId ? { userId } : {}, + emit: jest.fn((_event: string, payload: { operation: string }) => { + if (payload.operation === 'moveTreeNode') moveSeen.push(id); + if (payload.operation === 'deleteTreeNode') deleteSeen.push(id); + }), + }); + + const sockets = [ + mkSocket('s-ok-1', 'user-ok'), // authorized, tab 1 + mkSocket('s-ok-2', 'user-ok'), // authorized, tab 2 (fan-out) + mkSocket('s-no', 'user-no'), // unauthorized + mkSocket('s-anon', undefined), // anonymous (no userId) + ]; + + beforeEach(async () => { + moveSeen.length = 0; + deleteSeen.length = 0; + + pagePermissionRepo = { + hasRestrictedPagesInSpace: jest.fn().mockResolvedValue(true), + // The move destination IS under a restricted ancestor. + hasRestrictedAncestor: jest.fn().mockResolvedValue(true), + // Only user-ok is authorized to see the page. + getUserIdsWithPageAccess: jest.fn().mockResolvedValue(['user-ok']), + }; + const cache = { + get: jest.fn().mockResolvedValue(null), + set: jest.fn().mockResolvedValue(undefined), + del: jest.fn().mockResolvedValue(undefined), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + WsTreeService, + WsService, + { provide: PagePermissionRepo, useValue: pagePermissionRepo }, + { provide: CACHE_MANAGER, useValue: cache }, + ], + }).compile(); + + const wsService = module.get(WsService); + const server = { + to: jest.fn().mockReturnValue({ emit: jest.fn() }), + in: jest.fn().mockReturnValue({ + fetchSockets: jest.fn().mockResolvedValue(sockets), + }), + }; + wsService.setServer(server as never); + + treeService = module.get(WsTreeService); + }); + + it('authorized set (move) and complement (delete) partition the room; anon is in delete', async () => { + const event: PageMovedEvent = { + workspaceId: 'ws-1', + oldParentId: 'old-parent', + hasChildren: false, + node: { ...snapshot, parentPageId: 'restricted-parent', position: 'a5' }, + }; + + await treeService.broadcastPageMoved(event); + + const moveSet = new Set(moveSeen); + const deleteSet = new Set(deleteSeen); + + // Authorized user's BOTH sockets got the move; nobody else did. + expect(moveSet).toEqual(new Set(['s-ok-1', 's-ok-2'])); + // Everyone else (unauthorized + anonymous) got the delete. + expect(deleteSet).toEqual(new Set(['s-no', 's-anon'])); + + // DISJOINT: no socket received both a move and a delete. + const intersection = [...moveSet].filter((id) => deleteSet.has(id)); + expect(intersection).toEqual([]); + + // PARTITION: the two sets together cover every socket in the room exactly. + const union = new Set([...moveSet, ...deleteSet]); + expect(union).toEqual(new Set(sockets.map((s) => s.id))); + + // The anonymous socket specifically lands in the DELETE set, never the move. + expect(deleteSet.has('s-anon')).toBe(true); + expect(moveSet.has('s-anon')).toBe(false); + }); +}); diff --git a/packages/editor-ext/src/lib/html-embed/html-embed-codec.spec.ts b/packages/editor-ext/src/lib/html-embed/html-embed-codec.spec.ts new file mode 100644 index 00000000..fbee45d2 --- /dev/null +++ b/packages/editor-ext/src/lib/html-embed/html-embed-codec.spec.ts @@ -0,0 +1,116 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { + encodeHtmlEmbedSource, + decodeHtmlEmbedSource, +} from "./html-embed"; + +// Unit coverage for the base64 codec used by the htmlEmbed node's +// data-source attribute (html-embed.ts). The codec has two branches: +// - the BROWSER branch: btoa(encodeURIComponent(s)) / decodeURIComponent(atob(s)); +// - the NODE fallback: Buffer.from(..).toString("base64") / Buffer.from(s,"base64"). +// Server-side schema parsing (htmlToJson with no global btoa/atob) hits the +// fallback, so both branches must round-trip identically; otherwise an embed +// encoded in the browser would decode wrong on the server (or vice versa). +// +// We force the fallback by temporarily DELETING globalThis.btoa/atob (jsdom +// provides them in this env), restoring them after each test so the suite stays +// hermetic. + +const realBtoa = globalThis.btoa; +const realAtob = globalThis.atob; + +function deleteBase64Globals(): void { + // @ts-expect-error — intentionally removing the globals to exercise the + // `typeof btoa !== "function"` Node fallback branch in the codec. + delete globalThis.btoa; + // @ts-expect-error — see above. + delete globalThis.atob; +} + +afterEach(() => { + // Always restore so one test's stubbing never leaks into another. + globalThis.btoa = realBtoa; + globalThis.atob = realAtob; +}); + +describe("html-embed codec — browser btoa/atob branch", () => { + it("round-trips ASCII source", () => { + const src = ""; + const enc = encodeHtmlEmbedSource(src); + expect(enc).not.toBe(""); + // base64 of the encodeURIComponent form never contains a raw '<'. + expect(enc).not.toContain("<"); + expect(decodeHtmlEmbedSource(enc)).toBe(src); + }); + + it("round-trips UTF-8 / non-Latin1 source (the reason for encodeURIComponent)", () => { + const src = '

héllo → 世界 𝕏

'; + const enc = encodeHtmlEmbedSource(src); + expect(decodeHtmlEmbedSource(enc)).toBe(src); + }); +}); + +describe("html-embed codec — Node Buffer fallback branch", () => { + it("encode uses the Buffer fallback when btoa is unavailable and still round-trips (UTF-8)", () => { + const src = '
héllo → 世界 𝕏
'; + + deleteBase64Globals(); + // With the globals gone, encode must take the Buffer path... + const encFallback = encodeHtmlEmbedSource(src); + expect(encFallback).not.toBe(""); + // ...and decode (also via Buffer) must recover the exact source. + expect(decodeHtmlEmbedSource(encFallback)).toBe(src); + }); + + it("the Buffer fallback produces the SAME bytes the browser branch does (cross-env parity)", () => { + const src = 'café — 日本語'; + + // Browser branch (globals intact). + const encBrowser = encodeHtmlEmbedSource(src); + + // Fallback branch. + deleteBase64Globals(); + const encFallback = encodeHtmlEmbedSource(src); + + // Identical base64 => an embed encoded in either environment decodes + // identically in the other (server <-> client losslessness). + expect(encFallback).toBe(encBrowser); + + // And the fallback can decode what the browser produced. + expect(decodeHtmlEmbedSource(encBrowser)).toBe(src); + }); + + it("empty string -> '' on both encode and decode in the fallback (early return, branch never reached)", () => { + deleteBase64Globals(); + expect(encodeHtmlEmbedSource("")).toBe(""); + expect(decodeHtmlEmbedSource("")).toBe(""); + }); + + it("decode of malformed base64 -> '' via the catch branch (fallback)", () => { + // In the Buffer fallback, Buffer.from(..,'base64') is lenient and never + // throws, so to hit the catch we need a payload whose DECODED bytes are an + // invalid percent-escape, which makes decodeURIComponent throw. base64 of a + // lone '%' decodes back to '%', and decodeURIComponent('%') is a URIError. + const badBase64 = Buffer.from("%", "utf-8").toString("base64"); // "JQ==" + + deleteBase64Globals(); + // Sanity: the raw decode really does throw, so we're exercising the catch. + expect(() => + decodeURIComponent(Buffer.from(badBase64, "base64").toString("utf-8")), + ).toThrow(); + // The codec swallows it and returns "" rather than propagating. + expect(decodeHtmlEmbedSource(badBase64)).toBe(""); + }); +}); + +describe("html-embed codec — decode of malformed input (browser branch)", () => { + it("returns '' for input atob rejects (catch branch)", () => { + // atob throws on characters outside the base64 alphabet; the codec catches + // it and returns "" instead of throwing. + expect(decodeHtmlEmbedSource("@@not-base64@@")).toBe(""); + }); + + it("empty string short-circuits to '' (never calls atob)", () => { + expect(decodeHtmlEmbedSource("")).toBe(""); + }); +}); diff --git a/packages/editor-ext/src/lib/markdown/html-embed-marked.spec.ts b/packages/editor-ext/src/lib/markdown/html-embed-marked.spec.ts new file mode 100644 index 00000000..7904f063 --- /dev/null +++ b/packages/editor-ext/src/lib/markdown/html-embed-marked.spec.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from "vitest"; +import { htmlEmbedExtension } from "./utils/html-embed.marked"; +import { markdownToHtml } from "./index"; +import { encodeHtmlEmbedSource } from "../html-embed/html-embed"; + +// CONTRACT tests for the marked block tokenizer that rebuilds an htmlEmbed node +// from the `` marker (html-embed.marked.ts), plus the +// observable round-trip through markdownToHtml. +// +// These pin the REAL tokenizer behaviour the import path depends on: +// - the tokenizer rule is anchored (^) and only accepts the base64 alphabet +// [A-Za-z0-9+/=], so a marker with non-base64 chars is NOT tokenized and +// survives as a literal HTML comment (not silently turned into something the +// server's strip no longer recognizes); +// - start() reports the correct index of the next marker so marked invokes the +// tokenizer at the right offset when a marker sits mid-document / after text; +// - a marker with surrounding text on the SAME line is split out into its own +// embed div while the surrounding text becomes ordinary paragraphs. +// +// The contract is asserted against the actual exported extension and pipeline — +// no behaviour is invented; the expectations were read off the real tokenizer. + +const SAMPLE = "x"; +const ENC = encodeHtmlEmbedSource(SAMPLE); + +describe("htmlEmbed marked tokenizer — start()", () => { + it("returns the index of a marker that sits mid-document", () => { + const src = `hello world `; + expect(htmlEmbedExtension.start(src)).toBe(src.indexOf("`)).toBe(0); + }); + + it("returns -1 when there is no marker", () => { + expect(htmlEmbedExtension.start("no marker here")).toBe(-1); + }); +}); + +describe("htmlEmbed marked tokenizer — tokenizer()", () => { + it("tokenizes a marker at the start of the input, capturing the base64 payload", () => { + const token = htmlEmbedExtension.tokenizer(``); + expect(token).toBeTruthy(); + expect(token!.type).toBe("htmlEmbed"); + expect(token!.raw).toBe(``); + expect(token!.encoded).toBe(ENC); + }); + + it("tokenizes an EMPTY marker (the [A-Za-z0-9+/=]* class allows zero chars)", () => { + const token = htmlEmbedExtension.tokenizer(""); + expect(token).toBeTruthy(); + expect(token!.encoded).toBe(""); + expect(token!.raw).toBe(""); + }); + + it("does NOT tokenize when text precedes the marker (rule is anchored ^)", () => { + // marked relies on start() to advance to the marker; the tokenizer itself + // only matches at offset 0, so a non-anchored call returns undefined. + expect( + htmlEmbedExtension.tokenizer(`hello `), + ).toBeUndefined(); + }); + + it("does NOT tokenize a marker containing a non-base64 char ('$')", () => { + expect( + htmlEmbedExtension.tokenizer(""), + ).toBeUndefined(); + }); + + it("does NOT tokenize a marker containing a space", () => { + expect( + htmlEmbedExtension.tokenizer(""), + ).toBeUndefined(); + }); + + it("renderer emits the embed div the node's parseHTML recognizes", () => { + const token = htmlEmbedExtension.tokenizer(``)!; + const html = htmlEmbedExtension.renderer(token as any); + expect(html).toBe( + `
`, + ); + }); +}); + +describe("htmlEmbed marked tokenizer — markdownToHtml round-trip", () => { + it("splits a marker out of surrounding same-line text into its own embed div", async () => { + const html = await markdownToHtml(`before after`); + // The marker became the embed div... + expect(html).toContain( + `
`, + ); + // ...and the surrounding text survived as ordinary paragraph content. + expect(html).toContain("before"); + expect(html).toContain("after"); + }); + + it("leaves a marker with non-base64 chars as a literal comment (NOT an embed div)", async () => { + const html = await markdownToHtml(""); + // It is NOT tokenized into an embed div the server would strip... + expect(html).not.toContain('data-type="htmlEmbed"'); + // ...it passes through unchanged as a literal HTML comment. + expect(html).toContain(""); + }); +}); diff --git a/packages/editor-ext/src/lib/page-embed/page-embed.spec.ts b/packages/editor-ext/src/lib/page-embed/page-embed.spec.ts new file mode 100644 index 00000000..95638090 --- /dev/null +++ b/packages/editor-ext/src/lib/page-embed/page-embed.spec.ts @@ -0,0 +1,88 @@ +import { describe, expect, it } from "vitest"; +import { getSchema } from "@tiptap/core"; +import { generateHTML, generateJSON } from "@tiptap/html"; +import { Document } from "@tiptap/extension-document"; +import { Paragraph } from "@tiptap/extension-paragraph"; +import { Text } from "@tiptap/extension-text"; +import { PageEmbed } from "./page-embed"; + +// CONTRACT tests for the PageEmbed node's parse/render round-trip +// (page-embed.ts). The whole-page live embed stores ONLY a `sourcePageId` +// reference; renderHTML must serialize it as `data-source-page-id` and parseHTML +// must recover it. If this attribute mapping drifts, an embed saved to HTML loses +// its target page on reload (the node view would have nothing to fetch). +// +// We assert at the editor-ext schema level using the same Tiptap utilities the +// other editor-ext tests use (getSchema + @tiptap/html generateHTML/generateJSON +// over a jsdom DOM), driving a real HTML -> node JSON -> HTML round-trip through +// the node's actual addAttributes()/parseHTML()/renderHTML(). + +// Minimal schema: a doc of blocks, plus the PageEmbed block node under test. +const extensions = [Document, Paragraph, Text, PageEmbed]; + +describe("PageEmbed schema", () => { + it("registers the pageEmbed node in the schema", () => { + const schema = getSchema(extensions); + expect(schema.nodes.pageEmbed).toBeTruthy(); + }); +}); + +describe("PageEmbed parse/render round-trip", () => { + it("recovers sourcePageId from data-source-page-id on parse (HTML -> JSON)", () => { + const html = `
`; + const json = generateJSON(html, extensions); + + const node = json.content?.[0]; + expect(node?.type).toBe("pageEmbed"); + expect(node?.attrs?.sourcePageId).toBe("pg-123"); + }); + + it("emits data-source-page-id on render (JSON -> HTML)", () => { + const json = { + type: "doc", + content: [{ type: "pageEmbed", attrs: { sourcePageId: "pg-456" } }], + }; + const html = generateHTML(json, extensions); + + expect(html).toContain('data-type="pageEmbed"'); + expect(html).toContain('data-source-page-id="pg-456"'); + }); + + it("survives a full HTML -> node -> HTML round-trip (attribute preserved)", () => { + const start = `
`; + + // HTML -> node JSON -> HTML. + const json = generateJSON(start, extensions); + const html = generateHTML(json, extensions); + + // The id survived the round-trip in the serialized HTML... + expect(html).toContain('data-source-page-id="pg-789"'); + + // ...and re-parsing the round-tripped HTML yields the same id (stable across + // an extra pass — no loss, no duplication). + const json2 = generateJSON(html, extensions); + expect(json2.content?.[0]?.attrs?.sourcePageId).toBe("pg-789"); + }); + + it("omits data-source-page-id entirely when sourcePageId is null (renderHTML guard)", () => { + // The renderHTML maps a null/empty id to {} (no attribute), so an embed + // without a target page does not emit a stray empty attribute. + const json = { + type: "doc", + content: [{ type: "pageEmbed", attrs: { sourcePageId: null } }], + }; + const html = generateHTML(json, extensions); + + expect(html).toContain('data-type="pageEmbed"'); + expect(html).not.toContain("data-source-page-id"); + }); + + it("parses a div without the attribute to a null sourcePageId (default)", () => { + const html = `
`; + const json = generateJSON(html, extensions); + + expect(json.content?.[0]?.type).toBe("pageEmbed"); + // getAttribute returns null when absent; parseHTML returns it verbatim. + expect(json.content?.[0]?.attrs?.sourcePageId).toBeNull(); + }); +}); diff --git a/packages/mcp/test/unit/http-idle-eviction.test.mjs b/packages/mcp/test/unit/http-idle-eviction.test.mjs new file mode 100644 index 00000000..6521f268 --- /dev/null +++ b/packages/mcp/test/unit/http-idle-eviction.test.mjs @@ -0,0 +1,273 @@ +// Unit tests for createMcpHttpHandler's idle-session eviction (http.ts). +// +// http.ts keeps one transport per MCP session alive between requests, keyed by +// the mcp-session-id header, and runs a periodic sweep (setInterval, every 5 +// min) that closes any transport idle longer than the idle TTL +// (MCP_SESSION_IDLE_MS, default 30 min) and drops its lastSeen + sessionIdentity +// bookkeeping. Routing a request to an existing transport refreshes its +// lastSeen. +// +// We drive this DETERMINISTICALLY rather than waiting wall-clock: the env knob +// MCP_SESSION_IDLE_MS is read ONCE when the handler is created, so we set it +// small; and node:test's mock.timers lets us mock both `setInterval` (the sweep) +// and `Date` (the lastSeen comparison clock) so ticking advances the clock and +// fires the sweep on demand. +// +// IMPORTANT mock.timers semantics: when a tick spans MULTIPLE timer fires (or +// overshoots a fire), the callbacks all observe Date.now() == the FINAL ticked +// time, not their individual scheduled times. So to make the sweep's +// `now - lastSeen` comparison meaningful we tick EXACTLY to a sweep boundary +// (a multiple of the sweep interval): then Date.now() inside the sweep equals +// that boundary. The mocked clock starts at 0, so sweeps fire at SWEEP, 2*SWEEP, +// ... We pin each session's lastSeen by establishing/touching it at a known +// pre-boundary clock, then tick the remaining delta to land exactly on the +// boundary. +// +// Sessions are established over a real loopback http server (so the SDK's +// StreamableHTTPServerTransport gets genuine Node req/res and a real +// mcp-session-id), exactly like http-resolver.test.mjs, and the server is closed +// in a finally. +// +// Eviction is asserted via its OBSERVABLE effect: once a session is evicted its +// transport is gone from the handler's internal map, so a subsequent non-init +// request replaying that session id is treated as unknown (400 "no valid +// session ID") — the same response an id that was never established would get. +// An active (recently-seen) session is retained and its subsequent request is +// NOT a 400. +import { test, mock } from "node:test"; +import assert from "node:assert/strict"; + +const INIT_BODY = { + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-03-26", + capabilities: {}, + clientInfo: { name: "test", version: "0.0.0" }, + }, +}; + +const SWEEP_MS = 5 * 60 * 1000; // setInterval cadence in http.ts. + +// Spin a loopback http server bridging every request into the MCP handler with +// its JSON body parsed, mirroring the embedding host. Returns { call, close }. +async function startLoopback(handler) { + const http = await import("node:http"); + const server = http.createServer((req, res) => { + let raw = ""; + req.on("data", (c) => (raw += c)); + req.on("end", () => { + const body = raw ? JSON.parse(raw) : undefined; + handler.handleRequest(req, res, body).catch(() => { + if (!res.headersSent) { + res.statusCode = 500; + res.end(); + } + }); + }); + }); + await new Promise((r) => server.listen(0, "127.0.0.1", r)); + const { port } = server.address(); + + const call = (headers, body) => + new Promise((resolve) => { + const r = http.request( + { + host: "127.0.0.1", + port, + method: "POST", + path: "/mcp", + headers: { + "Content-Type": "application/json", + Accept: "application/json, text/event-stream", + ...headers, + }, + }, + (resp) => { + let data = ""; + resp.on("data", (c) => (data += c)); + resp.on("end", () => + resolve({ + statusCode: resp.statusCode, + sessionId: resp.headers["mcp-session-id"], + body: data, + }), + ); + }, + ); + r.end(JSON.stringify(body)); + }); + + return { call, close: () => new Promise((r) => server.close(r)) }; +} + +// The sweep closes transports asynchronously (void transport.close()), whose +// onclose then removes the entry from the internal map. Yield to the event loop +// so those microtasks settle before we assert the observable effect. +const settle = () => new Promise((r) => setImmediate(r)); + +// Set the idle TTL env knob (read once at handler creation) and enable mocked +// setInterval + Date BEFORE creating the handler, so the sweep interval and +// every Date.now() (lastSeen at init, lastSeen on routing, and the sweep's +// comparison) all run on the same mocked clock. Returns restore() to undo it. +function withMockedTimers(idleMs) { + const prevIdle = process.env.MCP_SESSION_IDLE_MS; + process.env.MCP_SESSION_IDLE_MS = String(idleMs); + mock.timers.enable({ apis: ["setInterval", "Date"] }); + return () => { + mock.timers.reset(); + if (prevIdle === undefined) delete process.env.MCP_SESSION_IDLE_MS; + else process.env.MCP_SESSION_IDLE_MS = prevIdle; + }; +} + +test("idle session is evicted by the sweep; an active session is retained", async () => { + // A small TTL: idle longer than 1s triggers eviction. Both sessions start at + // clock 0; we keep one fresh (touch it just before the sweep) and leave the + // other idle, then fire ONE sweep exactly on its boundary. + const idleMs = 1000; + const restore = withMockedTimers(idleMs); + + const { createMcpHttpHandler } = await import("../../build/http.js"); + const handler = createMcpHttpHandler(() => ({ + apiUrl: "http://127.0.0.1:3000/api", + getToken: async () => "t", + })); + + const lb = await startLoopback(handler); + try { + // T0 (clock 0): establish both sessions; lastSeen(A) = lastSeen(B) = 0. + const a = await lb.call({}, INIT_BODY); + const b = await lb.call({}, INIT_BODY); + assert.ok(a.sessionId, "session A must get an mcp-session-id"); + assert.ok(b.sessionId, "session B must get an mcp-session-id"); + assert.notEqual(a.sessionId, b.sessionId, "distinct sessions"); + + // Advance to just before the first sweep boundary (SWEEP - 1ms): no sweep + // fires yet (boundary not reached). lastSeen(A) stays 0. + mock.timers.tick(SWEEP_MS - 1); + // Touch ONLY B here, refreshing lastSeen(B) to SWEEP-1 (active); A is left + // idle since clock 0. + const touchB = await lb.call( + { "mcp-session-id": b.sessionId }, + { jsonrpc: "2.0", method: "ping", id: 5 }, + ); + assert.notEqual(touchB.statusCode, 400, "B alive right before the sweep"); + + // Land EXACTLY on the sweep boundary (clock = SWEEP). Inside the sweep + // Date.now() == SWEEP, so: + // idle(A) = SWEEP - 0 = SWEEP > TTL(1s) -> A EVICTED + // idle(B) = SWEEP - (SWEEP-1) = 1ms < TTL(1s) -> B RETAINED + mock.timers.tick(1); + await settle(); + + // OBSERVABLE EFFECT 1 — A evicted: replaying its session id on a non-init + // request is now treated as unknown (400, no valid session). + const aAfter = await lb.call( + { "mcp-session-id": a.sessionId }, + { jsonrpc: "2.0", method: "ping", id: 10 }, + ); + assert.equal(aAfter.statusCode, 400, "evicted session id is unknown -> 400"); + assert.match(aAfter.body, /no valid session ID/); + + // OBSERVABLE EFFECT 2 — B retained: a subsequent request on its session id + // is routed to the live transport, NOT rejected as an unknown session. + const bAfter = await lb.call( + { "mcp-session-id": b.sessionId }, + { jsonrpc: "2.0", method: "ping", id: 11 }, + ); + assert.notEqual( + bAfter.statusCode, + 400, + "active session must survive the sweep (not 400)", + ); + } finally { + await lb.close(); + restore(); + } +}); + +test("a session left idle past the TTL is dropped so its id becomes unknown", async () => { + // Simplest single-session eviction: establish a session, let it go idle past + // the TTL, fire the sweep on its boundary, and confirm its id is now unknown + // (400). Pins the core "lastSeen older than TTL -> closed and dropped" path. + const idleMs = 1000; + const restore = withMockedTimers(idleMs); + + const { createMcpHttpHandler } = await import("../../build/http.js"); + const handler = createMcpHttpHandler(() => ({ + apiUrl: "http://127.0.0.1:3000/api", + getToken: async () => "t", + })); + + const lb = await startLoopback(handler); + try { + const s = await lb.call({}, INIT_BODY); + assert.ok(s.sessionId, "session must get an mcp-session-id"); + + // Fire the first sweep exactly on its boundary: Date.now() == SWEEP, idle = + // SWEEP - 0 = SWEEP > TTL, so the untouched session is evicted. + mock.timers.tick(SWEEP_MS); + await settle(); + + const after = await lb.call( + { "mcp-session-id": s.sessionId }, + { jsonrpc: "2.0", method: "ping", id: 30 }, + ); + assert.equal(after.statusCode, 400, "idle session id is unknown -> 400"); + assert.match(after.body, /no valid session ID/); + } finally { + await lb.close(); + restore(); + } +}); + +test("activity refreshes lastSeen so a busy session is never evicted", async () => { + // A session kept busy (a request just before the sweep) refreshes its + // lastSeen, so even though it was created long ago the sweep must not evict + // it. Pins the "routing to an existing transport refreshes its idle + // timestamp" branch of http.ts. + const idleMs = 1000; + const restore = withMockedTimers(idleMs); + + const { createMcpHttpHandler } = await import("../../build/http.js"); + const handler = createMcpHttpHandler(() => ({ + apiUrl: "http://127.0.0.1:3000/api", + getToken: async () => "t", + })); + + const lb = await startLoopback(handler); + try { + const s = await lb.call({}, INIT_BODY); + assert.ok(s.sessionId, "session must get an mcp-session-id"); + + // Age to just before the sweep boundary, then touch the session so its + // lastSeen is refreshed to SWEEP-1 (well within the TTL of the imminent + // sweep). + mock.timers.tick(SWEEP_MS - 1); + const touch = await lb.call( + { "mcp-session-id": s.sessionId }, + { jsonrpc: "2.0", method: "ping", id: 40 }, + ); + assert.notEqual(touch.statusCode, 400, "session still alive before sweep"); + + // Land exactly on the sweep boundary: idle = SWEEP - (SWEEP-1) = 1ms < TTL, + // so the busy session is retained. + mock.timers.tick(1); + await settle(); + + const after = await lb.call( + { "mcp-session-id": s.sessionId }, + { jsonrpc: "2.0", method: "ping", id: 41 }, + ); + assert.notEqual( + after.statusCode, + 400, + "a session touched just before the sweep must not be evicted", + ); + } finally { + await lb.close(); + restore(); + } +});