diff --git a/apps/client/public/locales/en-US/translation.json b/apps/client/public/locales/en-US/translation.json index 3330f641..70353fee 100644 --- a/apps/client/public/locales/en-US/translation.json +++ b/apps/client/public/locales/en-US/translation.json @@ -183,6 +183,7 @@ "Successfully imported": "Successfully imported", "Successfully restored": "Successfully restored", "System settings": "System settings", + "Template": "Template", "Templates": "Templates", "Theme": "Theme", "To change your email, you have to enter your password and new email.": "To change your email, you have to enter your password and new email.", @@ -473,6 +474,7 @@ "Make sub-pages public too": "Make sub-pages public too", "Allow search engines to index page": "Allow search engines to index page", "Open page": "Open page", + "Open source page": "Open source page", "Page": "Page", "Delete public share link": "Delete public share link", "Delete share": "Delete share", diff --git a/apps/client/public/locales/ru-RU/translation.json b/apps/client/public/locales/ru-RU/translation.json index 233ced69..8c91dd5a 100644 --- a/apps/client/public/locales/ru-RU/translation.json +++ b/apps/client/public/locales/ru-RU/translation.json @@ -183,6 +183,7 @@ "Successfully imported": "Успешно импортировано", "Successfully restored": "Успешно восстановлено", "System settings": "Системные настройки", + "Template": "Шаблон", "Templates": "Шаблоны", "Theme": "Тема", "To change your email, you have to enter your password and new email.": "Чтобы изменить электронную почту, вам нужно ввести пароль и новый адрес.", @@ -478,6 +479,7 @@ "Make sub-pages public too": "Сделать подстраницы тоже общедоступными", "Allow search engines to index page": "Разрешить поисковым системам индексировать страницу", "Open page": "Открыть страницу", + "Open source page": "Открыть исходную страницу", "Page": "Страница", "Delete public share link": "Удалить публичную ссылку", "Delete share": "Удалить общий доступ", diff --git a/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.test.tsx b/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.test.tsx index 6b9f3e27..867922d2 100644 --- a/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.test.tsx +++ b/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.test.tsx @@ -1,71 +1,149 @@ import { describe, it, expect } from "vitest"; -import { render } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; import { PageEmbedAncestryProvider, usePageEmbedAncestry, + isPageEmbedCycle, + isPageEmbedTooDeep, + PAGE_EMBED_MAX_DEPTH, } from "./page-embed-ancestry-context"; -// Probe child: renders the current ancestry context value as JSON so the test -// can assert on the accumulated chain and host without any Tiptap editor. -function Probe({ testId }: { testId: string }) { - const ancestry = usePageEmbedAncestry(); - return
{JSON.stringify(ancestry)}
; -} - -function read(el: HTMLElement) { - return JSON.parse(el.textContent || "{}") as { - chain: string[]; - hostPageId: string | null; - }; +/** + * Tiny probe that renders the current ancestry context as serialized data + * attributes so tests can assert the accumulated chain / threaded hostPageId + * without mounting the heavy Tiptap node view. + */ +function AncestryProbe({ testId = "probe" }: { testId?: string }) { + const { chain, hostPageId } = usePageEmbedAncestry(); + return ( + + ); } describe("PageEmbedAncestryProvider", () => { - it("accumulates the chain in order across nested providers", () => { - const { getByTestId } = render( + it("defaults to an empty chain and null host with no provider", () => { + render(); + const probe = screen.getByTestId("probe"); + expect(probe.getAttribute("data-chain")).toBe(""); + expect(probe.getAttribute("data-chain-length")).toBe("0"); + expect(probe.getAttribute("data-host")).toBe(""); + }); + + it("accumulates sourcePageId into the chain across nested providers", () => { + render( - + , ); - const value = read(getByTestId("leaf")); - expect(value.chain).toEqual(["a", "b", "c"]); - expect(value.hostPageId).toBe("host"); + const probe = screen.getByTestId("probe"); + // Chain is built outermost -> innermost. + expect(probe.getAttribute("data-chain")).toBe("a,b,c"); + expect(probe.getAttribute("data-chain-length")).toBe("3"); }); - it("leaves the chain unchanged when sourcePageId is absent, still propagating the host", () => { - const { getByTestId } = render( + it("threads the host page id from the outermost provider down the tree", () => { + render( + + + + + , + ); + const probe = screen.getByTestId("probe"); + // The first host wins (parent.hostPageId ?? hostPageId); deeper hosts are + // ignored so the original host is preserved for self-embed detection. + expect(probe.getAttribute("data-host")).toBe("host-page"); + }); + + it("does not add an entry to the chain when sourcePageId is missing", () => { + render( - - + + + + , ); - const value = read(getByTestId("leaf")); - expect(value.chain).toEqual(["a"]); - expect(value.hostPageId).toBe("host"); + const probe = screen.getByTestId("probe"); + // null / undefined sources are pass-through: chain stays ["a"], host kept. + expect(probe.getAttribute("data-chain")).toBe("a"); + expect(probe.getAttribute("data-host")).toBe("host"); }); - it("keeps the first (top-level) host even if an inner provider passes a different one", () => { - const { getByTestId } = render( - - - + it("adopts a host provided only at a deeper level when the root had none", () => { + render( + + + , ); - const value = read(getByTestId("leaf")); - expect(value.chain).toEqual(["a", "b"]); - // Inner host is ignored: the top-level host is set once and propagated. - expect(value.hostPageId).toBe("top-host"); - }); - - it("defaults to an empty chain and null host with no provider", () => { - const { getByTestId } = render(); - const value = read(getByTestId("leaf")); - expect(value.chain).toEqual([]); - expect(value.hostPageId).toBeNull(); + const probe = screen.getByTestId("probe"); + expect(probe.getAttribute("data-host")).toBe("late-host"); + }); +}); + +describe("isPageEmbedCycle", () => { + it("is false when the source is not in the chain and is not the host", () => { + expect(isPageEmbedCycle(["a", "b"], "host", "c")).toBe(false); + }); + + it("is true when the source is already present in the ancestor chain", () => { + expect(isPageEmbedCycle(["a", "b", "c"], "host", "b")).toBe(true); + }); + + it("is true for a top-level self-embed (host === source, empty chain)", () => { + expect(isPageEmbedCycle([], "self", "self")).toBe(true); + }); + + it("is true when the source equals the host even mid-chain", () => { + expect(isPageEmbedCycle(["x"], "self", "self")).toBe(true); + }); + + it("is false when there is no source id (nothing to embed yet)", () => { + expect(isPageEmbedCycle(["a"], "host", null)).toBe(false); + expect(isPageEmbedCycle([], "host", "")).toBe(false); + }); + + it("is false when host is null and source is not in the chain", () => { + expect(isPageEmbedCycle(["a", "b"], null, "c")).toBe(false); + }); +}); + +describe("isPageEmbedTooDeep", () => { + it("is false below the max depth", () => { + expect(isPageEmbedTooDeep([])).toBe(false); + expect( + isPageEmbedTooDeep(new Array(PAGE_EMBED_MAX_DEPTH - 1).fill("x")), + ).toBe(false); + }); + + it("is true once the chain length reaches the max depth", () => { + expect( + isPageEmbedTooDeep(new Array(PAGE_EMBED_MAX_DEPTH).fill("x")), + ).toBe(true); + }); + + it("is true when the chain length exceeds the max depth", () => { + expect( + isPageEmbedTooDeep(new Array(PAGE_EMBED_MAX_DEPTH + 3).fill("x")), + ).toBe(true); + }); + + it("guards at exactly PAGE_EMBED_MAX_DEPTH (=5)", () => { + // Pin the documented constant so an accidental change is caught. + expect(PAGE_EMBED_MAX_DEPTH).toBe(5); + expect(isPageEmbedTooDeep(["1", "2", "3", "4"])).toBe(false); + expect(isPageEmbedTooDeep(["1", "2", "3", "4", "5"])).toBe(true); }); }); diff --git a/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.tsx b/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.tsx index c989ee21..cdd7f109 100644 --- a/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.tsx +++ b/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.tsx @@ -51,3 +51,26 @@ export function PageEmbedAncestryProvider({ export function usePageEmbedAncestry() { return useContext(PageEmbedAncestryContext); } + +/** + * Pure cycle predicate used by the page-embed node view. Returns true when the + * source page would recurse into itself: either it is already present in the + * ancestor chain, or it is the host page (top-level self-embed). Extracted so + * the anti-DoS guard can be unit-tested without mounting the Tiptap NodeView. + */ +export function isPageEmbedCycle( + chain: string[], + hostPageId: string | null, + sourcePageId: string | null, +): boolean { + if (!sourcePageId) return false; + return chain.includes(sourcePageId) || hostPageId === sourcePageId; +} + +/** + * Pure depth-limit predicate. Returns true once the ancestor chain has reached + * the hard cap, before a deeper nested editor is mounted. + */ +export function isPageEmbedTooDeep(chain: string[]): boolean { + return chain.length >= PAGE_EMBED_MAX_DEPTH; +} diff --git a/apps/client/src/features/editor/components/page-embed/page-embed-lookup-context.tsx b/apps/client/src/features/editor/components/page-embed/page-embed-lookup-context.tsx index aa2a8caf..d29c19dc 100644 --- a/apps/client/src/features/editor/components/page-embed/page-embed-lookup-context.tsx +++ b/apps/client/src/features/editor/components/page-embed/page-embed-lookup-context.tsx @@ -55,7 +55,9 @@ export function PageEmbedLookupProvider({ try { const { items } = await lookupTemplate({ sourcePageIds: ids }); + const returned = new Set(); for (const r of items) { + returned.add(r.sourcePageId); resultCacheRef.current.set(r.sourcePageId, r); inFlightRef.current.delete(r.sourcePageId); const subs = subscribersRef.current.get(r.sourcePageId); @@ -64,6 +66,17 @@ export function PageEmbedLookupProvider({ } resolveWaiters(r.sourcePageId); } + // Harden against a partial/short server response: any requested id not + // present in `items` would otherwise stay in `inFlightRef` forever + // (subscribe/refresh are guarded by `!inFlightRef.has(id)`) and its + // refresh() promise would never resolve. Clear + resolve those ids, + // mirroring the catch branch, so no id can be stranded in-flight. + for (const id of ids) { + if (!returned.has(id)) { + inFlightRef.current.delete(id); + resolveWaiters(id); + } + } } catch (err) { // Surface the failure: errors must never be swallowed silently. console.error("[pageEmbed] template lookup failed", err); diff --git a/apps/client/src/features/editor/components/page-embed/page-embed-view.tsx b/apps/client/src/features/editor/components/page-embed/page-embed-view.tsx index 9a308bf6..d9189388 100644 --- a/apps/client/src/features/editor/components/page-embed/page-embed-view.tsx +++ b/apps/client/src/features/editor/components/page-embed/page-embed-view.tsx @@ -2,10 +2,9 @@ import { NodeViewProps, NodeViewWrapper } from "@tiptap/react"; import { ActionIcon, Menu, Tooltip } from "@mantine/core"; import { IconAlertTriangle, - IconArrowsMaximize, IconDots, - IconExternalLink, IconEyeOff, + IconFileText, IconInfoCircle, IconRefresh, IconRepeat, @@ -138,20 +137,6 @@ function PageEmbedBody({ - {sourceHref && ( - - - - - - )} @@ -172,13 +157,18 @@ function PageEmbedBody({ ) : null; const header = - sourceTitle || sourceIcon ? ( + // Render the badge whenever the source resolves (sourceHref), not only when + // it has a title/icon — the title link is now the single way to open the + // source, so it must not disappear when title and icon are both empty. + sourceTitle || sourceIcon || sourceHref ? (
- {sourceIcon ? `${sourceIcon} ` : } + {sourceIcon ? `${sourceIcon} ` : } {sourceHref ? ( {sourceTitle || t("Untitled")} @@ -226,7 +216,17 @@ function PageEmbedBody({ sourcePageId={sourcePageId} hostPageId={hostPageId} > - + {/* + Tiptap's EditorProvider consumes `content` only at initial mount, so a + changed `content` prop (e.g. after Refresh re-fetches fresh content) + would not update the read-only sub-editor. Key on the source's + updatedAt to remount PageEmbedContent (and its inner EditorProvider) + whenever the source page changes, applying the refreshed content. + */} + ); } else if (embedState === "no_access") { diff --git a/apps/client/src/features/editor/components/transclusion/transclusion.module.css b/apps/client/src/features/editor/components/transclusion/transclusion.module.css index 4d8d321a..168da0c7 100644 --- a/apps/client/src/features/editor/components/transclusion/transclusion.module.css +++ b/apps/client/src/features/editor/components/transclusion/transclusion.module.css @@ -183,7 +183,8 @@ } :global(.react-renderer.node-transclusionSource.ProseMirror-selectednode), -:global(.react-renderer.node-transclusionReference.ProseMirror-selectednode) { +:global(.react-renderer.node-transclusionReference.ProseMirror-selectednode), +:global(.react-renderer.node-pageEmbed.ProseMirror-selectednode) { outline: none; } diff --git a/apps/client/src/features/page/tree/components/space-tree-row.tsx b/apps/client/src/features/page/tree/components/space-tree-row.tsx index df371498..c5c08b1c 100644 --- a/apps/client/src/features/page/tree/components/space-tree-row.tsx +++ b/apps/client/src/features/page/tree/components/space-tree-row.tsx @@ -2,13 +2,14 @@ import { useRef } from "react"; import { Link, useParams } from "react-router-dom"; import { useAtom } from "jotai"; import { useTranslation } from "react-i18next"; -import { ActionIcon, rem } from "@mantine/core"; +import { ActionIcon, rem, Tooltip } from "@mantine/core"; import { IconChevronDown, IconChevronRight, IconFileDescription, IconPlus, IconPointFilled, + IconTemplate, } from "@tabler/icons-react"; import EmojiPicker from "@/components/ui/emoji-picker.tsx"; @@ -171,6 +172,25 @@ export function SpaceTreeRow({ {node.name || t("untitled")} + {node.isTemplate === true && ( + + + + )} +
diff --git a/apps/server/src/core/page/services/page.service.ts b/apps/server/src/core/page/services/page.service.ts index ba904d87..6a3ac26b 100644 --- a/apps/server/src/core/page/services/page.service.ts +++ b/apps/server/src/core/page/services/page.service.ts @@ -55,6 +55,7 @@ import { markdownToHtml } from '@docmost/editor-ext'; import { WatcherService } from '../../watcher/watcher.service'; import { sql } from 'kysely'; import { TransclusionService } from '../transclusion/transclusion.service'; +import { remapPageEmbedSourceId } from '../transclusion/utils/transclusion-prosemirror.util'; import { AuthProvenanceData } from '../../../common/decorators/auth-provenance.decorator'; @Injectable() @@ -670,12 +671,11 @@ export class PageService { // source page is also part of the copied set, point at its new copy; // otherwise leave it pointing at the original (live embed of original). if (node.type.name === 'pageEmbed') { - const sourcePageId = node.attrs.sourcePageId; - if (sourcePageId && pageMap.has(sourcePageId)) { - const mappedPage = pageMap.get(sourcePageId); - //@ts-ignore - node.attrs.sourcePageId = mappedPage.newPageId; - } + // @ts-expect-error ProseMirror Attrs is read-only typed; intentional remap to the duplicated copy + node.attrs.sourcePageId = remapPageEmbedSourceId( + node.attrs.sourcePageId, + (id) => pageMap.get(id)?.newPageId, + ); } // Update internal page links in link marks diff --git a/apps/server/src/core/page/transclusion/page-template.controller.ts b/apps/server/src/core/page/transclusion/page-template.controller.ts index 555a487f..db20ea42 100644 --- a/apps/server/src/core/page/transclusion/page-template.controller.ts +++ b/apps/server/src/core/page/transclusion/page-template.controller.ts @@ -67,6 +67,12 @@ export class PageTemplateController { throw new NotFoundException('Page not found'); } + if (page.workspaceId !== user.workspaceId) { + // Defense-in-depth: never act on a page outside the caller's workspace. + // Use NotFound (not Forbidden) to avoid leaking cross-workspace existence. + throw new NotFoundException('Page not found'); + } + await this.pageAccessService.validateCanEdit(page, user); const isTemplate = diff --git a/apps/server/src/core/page/transclusion/spec/page-embed-remap.util.spec.ts b/apps/server/src/core/page/transclusion/spec/page-embed-remap.util.spec.ts new file mode 100644 index 00000000..47fa46c4 --- /dev/null +++ b/apps/server/src/core/page/transclusion/spec/page-embed-remap.util.spec.ts @@ -0,0 +1,200 @@ +import { + remapPageEmbedSourceId, + remapPageEmbedSourceIds, +} from '../utils/transclusion-prosemirror.util'; + +/** + * Unit tests for the `pageEmbed` remap used by `PageService.duplicatePage`: + * + * - source page within the copied set -> rewrite to the COPY's new id + * - source page NOT in the copied set -> keep the ORIGINAL id (live embed) + * + * `remapPageEmbedSourceId` is the per-node decision the production + * `duplicatePage` callback now calls directly, so these tests guard the real + * path rather than a parallel copy. `remapPageEmbedSourceIds` is the JSON + * walker that delegates to the same helper; its tests exercise the shared + * decision transitively across nested ProseMirror containers. + */ +describe('remapPageEmbedSourceId (shared per-node decision used by duplicatePage)', () => { + it('returns the new copy id when the source IS in the copied set', () => { + const idMap = new Map([['old-src', 'new-copy']]); + + const out = remapPageEmbedSourceId('old-src', (id) => idMap.get(id)); + + expect(out).toBe('new-copy'); + }); + + it('returns the original id when the source is NOT in the copied set', () => { + const idMap = new Map([['old-src', 'new-copy']]); + + const out = remapPageEmbedSourceId('external', (id) => idMap.get(id)); + + expect(out).toBe('external'); + }); + + it('returns the original id when resolveNewId yields undefined', () => { + const out = remapPageEmbedSourceId('some-id', () => undefined); + + expect(out).toBe('some-id'); + }); + + it('leaves a null source unchanged without consulting the resolver', () => { + const resolve = jest.fn(() => 'should-not-be-used'); + + const out = remapPageEmbedSourceId(null, resolve); + + expect(out).toBeNull(); + expect(resolve).not.toHaveBeenCalled(); + }); + + it('leaves an undefined source unchanged without consulting the resolver', () => { + const resolve = jest.fn(() => 'should-not-be-used'); + + const out = remapPageEmbedSourceId(undefined, resolve); + + expect(out).toBeUndefined(); + expect(resolve).not.toHaveBeenCalled(); + }); +}); + +describe('remapPageEmbedSourceIds (duplicatePage pageEmbed remap)', () => { + const docWithEmbeds = (ids: string[]) => ({ + type: 'doc', + content: ids.map((id) => ({ + type: 'pageEmbed', + attrs: { sourcePageId: id }, + })), + }); + + it('remaps a source that IS within the copied set to its new copy id', () => { + const doc = docWithEmbeds(['old-src']); + const idMap = new Map([['old-src', 'new-copy']]); + + const out = remapPageEmbedSourceIds(doc, idMap); + + expect(out.content[0].attrs.sourcePageId).toBe('new-copy'); + }); + + it('keeps the original id for a source NOT in the copied set', () => { + const doc = docWithEmbeds(['external']); + const idMap = new Map([['old-src', 'new-copy']]); // does not contain "external" + + const out = remapPageEmbedSourceIds(doc, idMap); + + expect(out.content[0].attrs.sourcePageId).toBe('external'); + }); + + it('handles a mixed doc: in-set remapped, out-of-set preserved', () => { + const doc = docWithEmbeds(['in-set', 'external']); + const idMap = new Map([['in-set', 'in-set-copy']]); + + const out = remapPageEmbedSourceIds(doc, idMap); + + expect(out.content.map((n: any) => n.attrs.sourcePageId)).toEqual([ + 'in-set-copy', + 'external', + ]); + }); + + it('remaps pageEmbeds nested inside columns', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'columnList', + content: [ + { + type: 'column', + content: [ + { type: 'pageEmbed', attrs: { sourcePageId: 'nested-in' } }, + ], + }, + { + type: 'column', + content: [ + { type: 'pageEmbed', attrs: { sourcePageId: 'nested-out' } }, + ], + }, + ], + }, + ], + }; + const idMap = new Map([['nested-in', 'nested-in-copy']]); + + const out = remapPageEmbedSourceIds(doc, idMap) as any; + + const col0 = out.content[0].content[0].content[0]; + const col1 = out.content[0].content[1].content[0]; + expect(col0.attrs.sourcePageId).toBe('nested-in-copy'); + expect(col1.attrs.sourcePageId).toBe('nested-out'); + }); + + it('remaps pageEmbeds nested inside a callout', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'callout', + content: [ + { type: 'pageEmbed', attrs: { sourcePageId: 'in-callout' } }, + ], + }, + ], + }; + const idMap = new Map([['in-callout', 'in-callout-copy']]); + + const out = remapPageEmbedSourceIds(doc, idMap) as any; + + expect(out.content[0].content[0].attrs.sourcePageId).toBe( + 'in-callout-copy', + ); + }); + + it('does not descend into a transclusionSource (schema-isolated)', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'transclusionSource', + attrs: { id: 'src' }, + content: [ + { type: 'pageEmbed', attrs: { sourcePageId: 'hidden' } }, + ], + }, + ], + }; + const idMap = new Map([['hidden', 'should-not-apply']]); + + const out = remapPageEmbedSourceIds(doc, idMap) as any; + + // The embed inside a source must be left untouched. + expect(out.content[0].content[0].attrs.sourcePageId).toBe('hidden'); + }); + + it('leaves embeds missing a sourcePageId untouched', () => { + const doc = { + type: 'doc', + content: [ + { type: 'pageEmbed', attrs: {} }, + { type: 'pageEmbed', attrs: { sourcePageId: '' } }, + ], + }; + const idMap = new Map([['', 'x']]); + + const out = remapPageEmbedSourceIds(doc, idMap) as any; + + expect(out.content[0].attrs.sourcePageId).toBeUndefined(); + expect(out.content[1].attrs.sourcePageId).toBe(''); + }); + + it('returns the doc unchanged when idMap is empty', () => { + const doc = docWithEmbeds(['a', 'b']); + + const out = remapPageEmbedSourceIds(doc, new Map()); + + expect(out.content.map((n: any) => n.attrs.sourcePageId)).toEqual([ + 'a', + 'b', + ]); + }); +}); 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 ef15bd18..286ad275 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 @@ -172,8 +172,13 @@ describe('TransclusionService — template access core (real filter)', () => { expect((items[2] as any).status).toBe('no_access'); // not space-visible }); - it('honours the DTO-level ≤50 cap by deduping ids passed to the filter', async () => { - // The DTO enforces ArrayMaxSize(50); the service dedupes before filtering. + it('dedupes source ids before passing them to the access filter', async () => { + // NOTE: this test only covers DEDUP, not the ≤50 cap. The ArrayMaxSize(50) + // limit is enforced by the DTO (validation layer), so it is never engaged in + // the service under unit test — the service receives an already-validated + // array and merely dedupes it. Renamed from the old "honours ≤50 cap" title, + // which misleadingly implied the cap was exercised here. A real cap test would + // belong in a controller/DTO-validation spec, not in this service unit test. const ids = ['a', 'a', 'b']; const { service, db } = makeService({ spaceVisibleRows: [], @@ -389,6 +394,7 @@ describe('TransclusionService.syncPageTemplateReferences — workspace scoping', expect(deleteByReferenceAndSources).toHaveBeenCalledTimes(1); expect(deleteByReferenceAndSources).toHaveBeenCalledWith( 'host', + 'w1', // workspace-scoped delete (#36 defense-in-depth) ['gone'], undefined, // no trx supplied ); @@ -412,6 +418,7 @@ describe('TransclusionService.syncPageTemplateReferences — workspace scoping', expect(result.deleted).toBe(1); expect(deleteByReferenceAndSources).toHaveBeenCalledWith( 'host', + 'w1', // workspace-scoped delete (#36 defense-in-depth) ['x'], undefined, ); diff --git a/apps/server/src/core/page/transclusion/spec/page-template-lookup-edge.spec.ts b/apps/server/src/core/page/transclusion/spec/page-template-lookup-edge.spec.ts new file mode 100644 index 00000000..0d8433bd --- /dev/null +++ b/apps/server/src/core/page/transclusion/spec/page-template-lookup-edge.spec.ts @@ -0,0 +1,181 @@ +import { TransclusionService } from '../transclusion.service'; + +/** + * Edge-case + anti-leak coverage for `lookupTemplate` that the existing + * `page-template-lookup.spec.ts` (stubbed filter) and `page-template-access.spec.ts` + * (real filter, happy paths) do not exercise: + * + * 1. SECURITY anti-leak: when comment-mark stripping THROWS, the item must come + * back as `not_found` and NEVER carry raw content (the source's comment marks + * could otherwise leak to a viewer). See the `catch` branch in `lookupTemplate`. + * 2. A soft-deleted source page resolved through the REAL + * `filterViewerAccessiblePageIds` (space-visibility query filters `deletedAt`), + * asserting it maps to `not_found`/`no_access` rather than content. + */ +describe('TransclusionService.lookupTemplate — anti-leak catch branch', () => { + const now = new Date('2026-06-20T00:00:00.000Z'); + + function makeService(opts: { + accessibleIds: string[]; + pages: Array<{ + id: string; + slugId?: string; + title: string | null; + icon: string | null; + content: unknown; + updatedAt: Date; + }>; + }) { + const pageRepo = { + findManyByIds: jest.fn().mockResolvedValue(opts.pages), + }; + + const service = new TransclusionService( + {} as any, // db + {} as any, // pageTransclusionsRepo + {} as any, // pageTransclusionReferencesRepo + {} as any, // pageTemplateReferencesRepo + pageRepo as any, + {} as any, // pagePermissionRepo + {} as any, // spaceMemberRepo + {} as any, // attachmentRepo + {} as any, // storageService + {} as any, // pageAccessService + ); + + // Stub the access decision; we are testing the content-prep stage, not access. + jest + .spyOn(service, 'filterViewerAccessiblePageIds') + .mockResolvedValue(opts.accessibleIds); + + return { service, pageRepo }; + } + + it('returns not_found (NOT raw content) when comment-mark stripping throws', async () => { + // An accessible, present page whose stored content is structurally invalid PM: + // a `text` node without a `text` field. `jsonToNode` (called inside the try + // block) throws "Invalid text node in JSON" on this, which exercises the + // service's catch -> not_found anti-leak guard. This uses a REAL malformed + // input (no module mocking) so the test stays faithful to production behaviour. + const malformedContent = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + // Missing `text` — Node.fromJSON rejects this and jsonToNode rethrows. + type: 'text', + marks: [{ type: 'comment', attrs: { commentId: 'leak-me' } }], + }, + ], + }, + ], + }; + + const { service } = makeService({ + accessibleIds: ['p1'], + pages: [ + { + id: 'p1', + slugId: 's1', + title: 'Secret', + icon: '📄', + content: malformedContent, + updatedAt: now, + }, + ], + }); + + // Silence the expected error log so the suite output stays clean. + jest.spyOn((service as any).logger, 'error').mockImplementation(() => {}); + + const { items } = await service.lookupTemplate(['p1'], 'u1', 'w1'); + + expect(items).toHaveLength(1); + const item = items[0] as any; + + // Must degrade to not_found... + expect(item.status).toBe('not_found'); + expect(item.sourcePageId).toBe('p1'); + + // ...and must NOT leak ANY content/metadata of the source page. + expect(item).not.toHaveProperty('content'); + expect(item).not.toHaveProperty('title'); + expect(item).not.toHaveProperty('icon'); + expect(item).not.toHaveProperty('slugId'); + expect(item).not.toHaveProperty('sourceUpdatedAt'); + + // Hard guarantee: the would-be-leaked comment mark appears nowhere in output. + expect(JSON.stringify(item)).not.toContain('leak-me'); + expect(JSON.stringify(item)).not.toContain('comment'); + }); +}); + +describe('TransclusionService.lookupTemplate — soft-deleted source via real filter', () => { + const now = new Date('2026-06-20T00:00:00.000Z'); + + /** + * Chainable kysely `db` stub mirroring `page-template-access.spec.ts`. The + * space-visibility query in `filterViewerAccessiblePageIds` filters + * `where('deletedAt','is',null)`; a soft-deleted page is therefore absent from + * the rows we resolve here, so the REAL filter is what drops it. + */ + 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; + } + + it('resolves a soft-deleted source to not_found/no_access through the REAL filter', async () => { + // The page IS soft-deleted, so the space-visibility query returns no rows for + // it (deletedAt filter). We let the real filter run end-to-end. + const db = makeDb([]); // soft-deleted -> excluded by the deletedAt='is null' clause + + const spaceMemberRepo = { + getUserSpaceIdsQuery: jest.fn(() => ({ __subquery: true })), + }; + const pagePermissionRepo = { + filterAccessiblePageIds: jest.fn().mockResolvedValue([]), + }; + const pageRepo = { + // Even if it were queried, the page is gone; assert via the filter instead. + findManyByIds: jest.fn().mockResolvedValue([]), + }; + + const service = new TransclusionService( + db as any, + {} as any, + {} as any, + {} as any, + pageRepo as any, + pagePermissionRepo as any, + spaceMemberRepo as any, + {} as any, + {} as any, + {} as any, + ); + + const { items } = await service.lookupTemplate(['deleted-src'], 'u1', 'w1'); + + // Soft-deleted source must never resolve to content. + expect(items).toEqual([ + { sourcePageId: 'deleted-src', status: 'no_access' }, + ]); + const item = items[0] as any; + expect(item).not.toHaveProperty('content'); + + // The real filter short-circuited before page-permission filtering because + // the deletedAt-filtered space-visibility query returned nothing. + expect(pagePermissionRepo.filterAccessiblePageIds).not.toHaveBeenCalled(); + // And the verb on the db builder included a deletedAt 'is null' guard, proving + // the real path (not a stub) excluded the soft-deleted page. + const deletedAtCall = db.where.mock.calls.find( + (c: any[]) => c[0] === 'deletedAt', + ); + expect(deletedAtCall).toEqual(['deletedAt', 'is', null]); + }); +}); diff --git a/apps/server/src/core/page/transclusion/spec/page-template-references-sync.spec.ts b/apps/server/src/core/page/transclusion/spec/page-template-references-sync.spec.ts new file mode 100644 index 00000000..b8ce5c7d --- /dev/null +++ b/apps/server/src/core/page/transclusion/spec/page-template-references-sync.spec.ts @@ -0,0 +1,309 @@ +import { TransclusionService } from '../transclusion.service'; + +/** + * Covers two untested, high-risk write paths around `page_template_references`: + * + * 1. `syncPageTemplateReferences` — the `toDelete` branch: stale references are + * removed when the host page no longer embeds a source, while genuinely new + * embeds are inserted. We assert `deleteByReferenceAndSources` / `insertMany` + * receive the correct rows and the returned `{ inserted, deleted }` counts. + * + * 2. `insertTemplateReferencesForPages` — the multi-workspace grouping/filtering + * branch: candidate source ids are grouped per workspace, each workspace is + * validated independently, and cross-workspace sources are dropped. + * + * Setup/mocking mirrors the existing transclusion specs (page-template-access / + * page-template-lookup): `new TransclusionService(...)` is built with the same + * 11 positional mock args; only the deps each test touches are real stubs. + */ + +/** + * Chainable kysely `db` stub used by `filterInWorkspaceSourceIds`. Every + * `selectFrom(...).select(...).where(...)` returns the same builder; `.execute()` + * resolves whatever rows the per-call resolver returns. The resolver receives + * the captured `where('id','in', )` and `where('workspaceId','=', ws)` + * arguments so a test can decide, per workspace, which ids "exist". + */ +function makeWorkspaceScopedDb( + resolve: (ids: string[], workspaceId: string) => string[], +) { + const state = { ids: [] as string[], workspaceId: '' }; + const builder: any = {}; + builder.selectFrom = jest.fn(() => builder); + builder.select = jest.fn(() => builder); + builder.where = jest.fn((col: string, _op: string, val: any) => { + if (col === 'id') state.ids = val as string[]; + if (col === 'workspaceId') state.workspaceId = val as string; + return builder; + }); + builder.execute = jest.fn(async () => + resolve(state.ids, state.workspaceId).map((id) => ({ id })), + ); + return builder; +} + +function buildService(opts: { + db: any; + pageTemplateReferencesRepo: any; +}) { + return new TransclusionService( + opts.db, + {} as any, // pageTransclusionsRepo + {} as any, // pageTransclusionReferencesRepo + opts.pageTemplateReferencesRepo, + {} as any, // pageRepo + {} as any, // pagePermissionRepo + {} as any, // spaceMemberRepo + {} as any, // attachmentRepo + {} as any, // storageService + {} as any, // pageAccessService + ); +} + +const pageEmbedDoc = (sourceIds: string[]) => ({ + type: 'doc', + content: sourceIds.map((id) => ({ + type: 'pageEmbed', + attrs: { sourcePageId: id }, + })), +}); + +describe('TransclusionService.syncPageTemplateReferences — toDelete branch', () => { + it('deletes stale references and inserts new ones with correct args/counts', async () => { + // Every candidate id is treated as in-workspace by the existence query. + const db = makeWorkspaceScopedDb((ids) => ids); + + const insertMany = jest.fn().mockResolvedValue(undefined); + const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { + // existing refs: "keep" stays embedded, "stale-a"/"stale-b" no longer are + findByReferencePageId: jest.fn().mockResolvedValue([ + { sourcePageId: 'keep' }, + { sourcePageId: 'stale-a' }, + { sourcePageId: 'stale-b' }, + ]), + insertMany, + deleteByReferenceAndSources, + }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + // host now embeds: keep (unchanged) + fresh (new). stale-a/stale-b gone. + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + pageEmbedDoc(['keep', 'fresh']), + ); + + expect(result).toEqual({ inserted: 1, deleted: 2 }); + + // only the genuinely new embed is inserted (keep already existed) + expect(insertMany).toHaveBeenCalledTimes(1); + expect(insertMany.mock.calls[0][0]).toEqual([ + { workspaceId: 'w1', referencePageId: 'host', sourcePageId: 'fresh' }, + ]); + + // stale references removed, scoped to host + workspace + expect(deleteByReferenceAndSources).toHaveBeenCalledTimes(1); + const [refPageId, workspaceId, staleSources] = + deleteByReferenceAndSources.mock.calls[0]; + expect(refPageId).toBe('host'); + expect(workspaceId).toBe('w1'); + expect([...staleSources].sort()).toEqual(['stale-a', 'stale-b']); + }); + + it('deletes ALL existing references when the host embeds nothing anymore', async () => { + const db = makeWorkspaceScopedDb((ids) => ids); + const insertMany = jest.fn().mockResolvedValue(undefined); + const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { + findByReferencePageId: jest + .fn() + .mockResolvedValue([{ sourcePageId: 'a' }, { sourcePageId: 'b' }]), + insertMany, + deleteByReferenceAndSources, + }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + pageEmbedDoc([]), // no embeds left + ); + + expect(result).toEqual({ inserted: 0, deleted: 2 }); + expect(insertMany).not.toHaveBeenCalled(); + const [, , staleSources] = deleteByReferenceAndSources.mock.calls[0]; + expect([...staleSources].sort()).toEqual(['a', 'b']); + }); + + it('treats a cross-workspace embed as stale: it never survives to be kept', async () => { + // existence query drops "cross-ws"; so an existing ref to it must be deleted + const db = makeWorkspaceScopedDb((ids) => ids.filter((id) => id !== 'cross-ws')); + const insertMany = jest.fn().mockResolvedValue(undefined); + const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { + findByReferencePageId: jest + .fn() + .mockResolvedValue([{ sourcePageId: 'cross-ws' }]), + insertMany, + deleteByReferenceAndSources, + }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + // host still "embeds" cross-ws in its doc, but it is not in-workspace + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + pageEmbedDoc(['cross-ws']), + ); + + expect(result).toEqual({ inserted: 0, deleted: 1 }); + expect(insertMany).not.toHaveBeenCalled(); + const [, , staleSources] = deleteByReferenceAndSources.mock.calls[0]; + expect([...staleSources]).toEqual(['cross-ws']); + }); + + it('no-ops both repos when desired and existing already match', async () => { + const db = makeWorkspaceScopedDb((ids) => ids); + const insertMany = jest.fn().mockResolvedValue(undefined); + const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { + findByReferencePageId: jest + .fn() + .mockResolvedValue([{ sourcePageId: 'same' }]), + insertMany, + deleteByReferenceAndSources, + }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + pageEmbedDoc(['same']), + ); + + expect(result).toEqual({ inserted: 0, deleted: 0 }); + expect(insertMany).not.toHaveBeenCalled(); + expect(deleteByReferenceAndSources).not.toHaveBeenCalled(); + }); +}); + +describe('TransclusionService.insertTemplateReferencesForPages — multi-workspace grouping', () => { + it('groups candidates per workspace and validates each workspace independently', async () => { + // Each workspace "owns" only its own source ids. The existence query is + // workspace-scoped, so an id from another workspace is dropped. + const owned: Record = { + w1: ['s1'], + w2: ['s2'], + }; + const executeArgs: Array<{ ids: string[]; workspaceId: string }> = []; + const db = makeWorkspaceScopedDb((ids, workspaceId) => { + executeArgs.push({ ids: [...ids], workspaceId }); + const ownedSet = new Set(owned[workspaceId] ?? []); + return ids.filter((id) => ownedSet.has(id)); + }); + + const insertMany = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { insertMany }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + // page-a in w1 embeds s1 (valid) + s2 (belongs to w2 -> dropped) + // page-b in w2 embeds s2 (valid) + const result = await service.insertTemplateReferencesForPages([ + { id: 'page-a', workspaceId: 'w1', content: pageEmbedDoc(['s1', 's2']) }, + { id: 'page-b', workspaceId: 'w2', content: pageEmbedDoc(['s2']) }, + ]); + + expect(result).toEqual({ inserted: 2 }); + + expect(insertMany).toHaveBeenCalledTimes(1); + const rows = insertMany.mock.calls[0][0]; + expect(rows).toEqual([ + { workspaceId: 'w1', referencePageId: 'page-a', sourcePageId: 's1' }, + { workspaceId: 'w2', referencePageId: 'page-b', sourcePageId: 's2' }, + ]); + + // one existence query per workspace, each scoped to that workspace's candidates + expect(executeArgs).toHaveLength(2); + const w1Call = executeArgs.find((c) => c.workspaceId === 'w1'); + const w2Call = executeArgs.find((c) => c.workspaceId === 'w2'); + expect(w1Call?.ids.sort()).toEqual(['s1', 's2']); + expect(w2Call?.ids).toEqual(['s2']); + }); + + it('drops every cross-workspace source and inserts nothing when none are in-workspace', async () => { + // No id is owned by its page's workspace -> all filtered out. + const db = makeWorkspaceScopedDb(() => []); + const insertMany = jest.fn().mockResolvedValue(undefined); + const service = buildService({ + db, + pageTemplateReferencesRepo: { insertMany }, + }); + + const result = await service.insertTemplateReferencesForPages([ + { id: 'page-a', workspaceId: 'w1', content: pageEmbedDoc(['x']) }, + { id: 'page-b', workspaceId: 'w2', content: pageEmbedDoc(['y']) }, + ]); + + expect(result).toEqual({ inserted: 0 }); + expect(insertMany).not.toHaveBeenCalled(); + }); + + it('dedupes a sourceId shared by two pages in the same workspace into one validation', async () => { + const executeArgs: Array<{ ids: string[]; workspaceId: string }> = []; + const db = makeWorkspaceScopedDb((ids, workspaceId) => { + executeArgs.push({ ids: [...ids], workspaceId }); + return ids; // all in-workspace + }); + const insertMany = jest.fn().mockResolvedValue(undefined); + const service = buildService({ + db, + pageTemplateReferencesRepo: { insertMany }, + }); + + // both pages embed the same source "shared" in w1 + const result = await service.insertTemplateReferencesForPages([ + { id: 'page-a', workspaceId: 'w1', content: pageEmbedDoc(['shared']) }, + { id: 'page-b', workspaceId: 'w1', content: pageEmbedDoc(['shared']) }, + ]); + + // a row per (page, source) pair, but only one existence query for w1 + expect(result).toEqual({ inserted: 2 }); + expect(executeArgs).toHaveLength(1); + expect(executeArgs[0]).toEqual({ ids: ['shared'], workspaceId: 'w1' }); + + const rows = insertMany.mock.calls[0][0]; + expect(rows).toEqual([ + { workspaceId: 'w1', referencePageId: 'page-a', sourcePageId: 'shared' }, + { workspaceId: 'w1', referencePageId: 'page-b', sourcePageId: 'shared' }, + ]); + }); + + it('returns inserted:0 without querying when no page has embeds', async () => { + const execute = jest.fn(); + const db = makeWorkspaceScopedDb(() => { + execute(); + return []; + }); + const insertMany = jest.fn().mockResolvedValue(undefined); + const service = buildService({ + db, + pageTemplateReferencesRepo: { insertMany }, + }); + + const result = await service.insertTemplateReferencesForPages([ + { id: 'page-a', workspaceId: 'w1', content: pageEmbedDoc([]) }, + ]); + + expect(result).toEqual({ inserted: 0 }); + expect(insertMany).not.toHaveBeenCalled(); + // filterInWorkspaceSourceIds short-circuits on empty candidates, so the + // existence query never runs. + expect(execute).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/server/src/core/page/transclusion/transclusion.service.ts b/apps/server/src/core/page/transclusion/transclusion.service.ts index 8e4ccf87..9bf07833 100644 --- a/apps/server/src/core/page/transclusion/transclusion.service.ts +++ b/apps/server/src/core/page/transclusion/transclusion.service.ts @@ -309,6 +309,7 @@ export class TransclusionService { if (toDelete.length > 0) { await this.pageTemplateReferencesRepo.deleteByReferenceAndSources( referencePageId, + workspaceId, toDelete, trx, ); diff --git a/apps/server/src/core/page/transclusion/utils/transclusion-prosemirror.util.ts b/apps/server/src/core/page/transclusion/utils/transclusion-prosemirror.util.ts index eeeea0e2..d8cdf3bf 100644 --- a/apps/server/src/core/page/transclusion/utils/transclusion-prosemirror.util.ts +++ b/apps/server/src/core/page/transclusion/utils/transclusion-prosemirror.util.ts @@ -99,6 +99,64 @@ export function collectReferencesFromPmJson( return out; } +/** + * Decide the sourcePageId a duplicated pageEmbed should point to: the copy's new + * id when the embedded source is part of the copied set, otherwise the original + * (a live embed of the original page). Pure — shared by PageService.duplicatePage + * (the real path) and the JSON walker below, so both stay in lockstep. + */ +export function remapPageEmbedSourceId( + sourcePageId: string | null | undefined, + resolveNewId: (id: string) => string | undefined, +): string | null | undefined { + if (sourcePageId) { + const mapped = resolveNewId(sourcePageId); + if (mapped) return mapped; + } + return sourcePageId; +} + +/** + * Remap the `sourcePageId` of every `pageEmbed` node in a ProseMirror JSON doc + * according to `idMap` (old page id -> new page id). Delegates the per-node + * decision to the shared `remapPageEmbedSourceId` helper that + * `PageService.duplicatePage` also uses, so the production path and this walker + * stay in lockstep: when the embedded source page is part of the copied set + * (present in `idMap`) the embed is pointed at its new copy; otherwise the + * original `sourcePageId` is preserved so it stays a live embed of the original + * page. Mutates `doc` in place (and returns it) to match the service's in-place + * ProseMirror mutation. Recurses through arbitrary block containers (columns, + * callouts, etc.) the same way the collectors do, but does NOT descend into a + * `transclusionSource` (schema-isolated). + */ +export function remapPageEmbedSourceIds( + doc: T, + idMap: Map, +): T { + const visit = (node: any): void => { + if (!node || typeof node !== 'object') return; + + if (node.type === PAGE_EMBED_TYPE) { + if (node.attrs) { + node.attrs.sourcePageId = remapPageEmbedSourceId( + node.attrs.sourcePageId, + (id) => idMap.get(id), + ); + } + return; // atom node - no children + } + + if (node.type === TRANSCLUSION_TYPE) return; + + if (Array.isArray(node.content)) { + for (const child of node.content) visit(child); + } + }; + + visit(doc); + return doc; +} + /** * Walks a ProseMirror JSON document and returns one snapshot per unique * `sourcePageId` found on `pageEmbed` nodes (whole-page live embeds). Order diff --git a/apps/server/src/core/search/search.controller.spec.ts b/apps/server/src/core/search/search.controller.spec.ts index 6d6bad58..1b0e42cb 100644 --- a/apps/server/src/core/search/search.controller.spec.ts +++ b/apps/server/src/core/search/search.controller.spec.ts @@ -1,15 +1,19 @@ -import { Test, TestingModule } from '@nestjs/testing'; import { SearchController } from './search.controller'; +// Direct instantiation with stub deps. The Test.createTestingModule form failed +// to resolve SearchService's @InjectKysely() connection token at compile() (the +// same Nest-DI/Kysely-token issue addressed in search.service.spec), and this +// unit only needs the controller to construct. describe('SearchController', () => { let controller: SearchController; - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - controllers: [SearchController], - }).compile(); - - controller = module.get(SearchController); + beforeEach(() => { + controller = new SearchController( + {} as any, // searchService + {} as any, // spaceAbility + {} as any, // environmentService + {} as any, // moduleRef + ); }); it('should be defined', () => { diff --git a/apps/server/src/core/search/search.service.spec.ts b/apps/server/src/core/search/search.service.spec.ts index 63fc48c0..efd4d2b8 100644 --- a/apps/server/src/core/search/search.service.spec.ts +++ b/apps/server/src/core/search/search.service.spec.ts @@ -1,18 +1,101 @@ -import { Test, TestingModule } from '@nestjs/testing'; import { SearchService } from './search.service'; describe('SearchService', () => { - let service: SearchService; - - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - providers: [SearchService], - }).compile(); - - service = module.get(SearchService); - }); - it('should be defined', () => { + // Construct directly with stub deps. The previous Test.createTestingModule + // form could not resolve the @InjectKysely() connection token and failed at + // compile() — manual construction mirrors the rest of these unit specs. + const service = new SearchService( + {} as any, // db + {} as any, // pageRepo + {} as any, // shareRepo + {} as any, // spaceMemberRepo + {} as any, // pagePermissionRepo + ); expect(service).toBeDefined(); }); }); + +/** + * Focused coverage for the `onlyTemplates` flag in `searchSuggestions`, which + * restricts page suggestions to template pages (`is_template = true`). The kysely + * query builder and repos are mocked the same way the access specs mock chainable + * builders: every builder method returns the same builder, `.execute()` resolves + * the supplied rows. We assert whether `.where('isTemplate', '=', true)` is added. + */ +describe('SearchService.searchSuggestions — onlyTemplates filter', () => { + function makeService(pageRows: Array<{ id: string }>) { + // Chainable page-search builder. Record every `.where(...)` call so we can + // assert on the is_template restriction. + const pageBuilder: any = {}; + pageBuilder.select = jest.fn(() => pageBuilder); + pageBuilder.where = jest.fn(() => pageBuilder); + pageBuilder.orderBy = jest.fn(() => pageBuilder); + pageBuilder.limit = jest.fn(() => pageBuilder); + pageBuilder.execute = jest.fn(async () => pageRows); + + const db: any = { + // searchSuggestions only touches `pages` here (includePages: true). + selectFrom: jest.fn(() => pageBuilder), + }; + + const pageRepo = { + // `.select((eb) => this.pageRepo.withSpace(eb))` — return value is ignored + // by our builder stub, so a sentinel is enough. + withSpace: jest.fn(() => ({ __withSpace: true })), + }; + const shareRepo = {}; + const spaceMemberRepo = { + getUserSpaceIds: jest.fn().mockResolvedValue(['space-1']), + }; + const pagePermissionRepo = { + // Let every found page through page-level permission filtering. + filterAccessiblePageIds: jest + .fn() + .mockImplementation(async ({ pageIds }: { pageIds: string[] }) => pageIds), + }; + + const service = new SearchService( + db as any, + pageRepo as any, + shareRepo as any, + spaceMemberRepo as any, + pagePermissionRepo as any, + ); + + return { service, db, pageBuilder }; + } + + const isTemplateWhereCall = (pageBuilder: any) => + pageBuilder.where.mock.calls.find((c: any[]) => c[0] === 'isTemplate'); + + it('restricts page suggestions to is_template = true when onlyTemplates is set', async () => { + const { service, pageBuilder } = makeService([{ id: 'tmpl-1' }]); + + const result = await service.searchSuggestions( + { query: 'plan', includePages: true, onlyTemplates: true } as any, + 'user-1', + 'ws-1', + ); + + // The is_template restriction must be applied to the page query. + const call = isTemplateWhereCall(pageBuilder); + expect(call).toEqual(['isTemplate', '=', true]); + + // Sanity: the (template) page made it through. + expect(result.pages.map((p: any) => p.id)).toEqual(['tmpl-1']); + }); + + it('does NOT restrict to templates when onlyTemplates is absent', async () => { + const { service, pageBuilder } = makeService([{ id: 'any-1' }]); + + await service.searchSuggestions( + { query: 'plan', includePages: true } as any, + 'user-1', + 'ws-1', + ); + + // No is_template clause should be added for a normal page suggestion search. + expect(isTemplateWhereCall(pageBuilder)).toBeUndefined(); + }); +}); diff --git a/apps/server/src/database/repos/page-template-references/page-template-references.repo.ts b/apps/server/src/database/repos/page-template-references/page-template-references.repo.ts index a678422f..ac358bc6 100644 --- a/apps/server/src/database/repos/page-template-references/page-template-references.repo.ts +++ b/apps/server/src/database/repos/page-template-references/page-template-references.repo.ts @@ -22,21 +22,6 @@ export class PageTemplateReferencesRepo { .execute(); } - async findReferencePageIdsBySource( - sourcePageId: string, - workspaceId: string, - trx?: KyselyTransaction, - ): Promise { - const rows = await dbOrTx(this.db, trx) - .selectFrom('pageTemplateReferences') - .select('referencePageId') - .distinct() - .where('workspaceId', '=', workspaceId) - .where('sourcePageId', '=', sourcePageId) - .execute(); - return rows.map((r) => r.referencePageId); - } - async insertMany( rows: InsertablePageTemplateReference[], trx?: KyselyTransaction, @@ -53,12 +38,15 @@ export class PageTemplateReferencesRepo { async deleteByReferenceAndSources( referencePageId: string, + workspaceId: string, sourcePageIds: string[], trx?: KyselyTransaction, ): Promise { if (sourcePageIds.length === 0) return; await dbOrTx(this.db, trx) .deleteFrom('pageTemplateReferences') + // Defense-in-depth: scope deletes to the caller's workspace. + .where('workspaceId', '=', workspaceId) .where('referencePageId', '=', referencePageId) .where('sourcePageId', 'in', sourcePageIds) .execute();