fix: stop cyclic breadcrumb and share traversal

apps/server/src/core/page/services/page.service.ts

@@ -55,6 +55,10 @@ import { markdownToHtml } from '@docmost/editor-ext';
 import { WatcherService } from '../../watcher/watcher.service';
 import { sql } from 'kysely';
 import { TransclusionService } from '../transclusion/transclusion.service';
+import {
+  assertAcyclicPageTraversal,
+  stripPageTraversalMetadata,
+} from '../../../database/helpers/page-hierarchy-cycle';
 
 @Injectable()
 export class PageService {
@@ -867,6 +871,8 @@ export class PageService {
             'parentPageId',
             'spaceId',
             'deletedAt',
+            sql<string[]>`ARRAY[pages.id]::uuid[]`.as('traversalPath'),
+            sql<boolean>`false`.as('isCycle'),
           ])
           .where('id', '=', childPageId)
           .where('deletedAt', 'is', null)
@@ -883,13 +889,27 @@ export class PageService {
                 'p.parentPageId',
                 'p.spaceId',
                 'p.deletedAt',
+                sql<string[]>`pa.traversal_path || p.id`.as('traversalPath'),
+                sql<boolean>`p.id = ANY(pa.traversal_path)`.as('isCycle'),
               ])
               .innerJoin('page_ancestors as pa', 'pa.parentPageId', 'p.id')
-              .where('p.deletedAt', 'is', null),
+              .where('p.deletedAt', 'is', null)
+              .where('pa.isCycle', '=', false),
           ),
       )
       .selectFrom('page_ancestors')
-      .selectAll('page_ancestors')
+      .select([
+        'id',
+        'slugId',
+        'title',
+        'icon',
+        'isBase',
+        'position',
+        'parentPageId',
+        'spaceId',
+        'deletedAt',
+        'isCycle',
+      ])
       .select((eb) =>
         eb
           .exists(
@@ -903,7 +923,9 @@ export class PageService {
       )
       .execute();
 
-    return ancestors.reverse();
+    assertAcyclicPageTraversal(ancestors, childPageId);
+
+    return ancestors.reverse().map(stripPageTraversalMetadata);
   }
 
   async getRecentSpacePages(

apps/server/src/core/share/share.service.ts

@@ -26,6 +26,7 @@ import { validate as isValidUUID } from 'uuid';
 import { sql } from 'kysely';
 import { TransclusionService } from '../page/transclusion/transclusion.service';
 import { TransclusionLookup } from '../page/transclusion/transclusion.types';
+import { stripPageTraversalMetadata } from '../../database/helpers/page-hierarchy-cycle';
 
 @Injectable()
 export class ShareService {
@@ -144,7 +145,7 @@ export class ShareService {
 
   async getShareForPage(pageId: string, workspaceId: string) {
     // here we try to check if a page was shared directly or if it inherits the share from its closest shared ancestor
-    const share = await this.db
+    const traversal = await this.db
       .withRecursive('page_hierarchy', (cte) =>
         cte
           .selectFrom('pages')
@@ -164,41 +165,67 @@ export class ShareService {
             'shares.spaceId',
             'shares.workspaceId',
             'shares.createdAt',
+            sql<string[]>`ARRAY[pages.id]::uuid[]`.as('traversalPath'),
+            sql<boolean>`false`.as('isCycle'),
           ])
           .where(isValidUUID(pageId) ? 'pages.id' : 'pages.slugId', '=', pageId)
           .where('pages.deletedAt', 'is', null)
-          .unionAll(
-            (union) =>
-              union
-                .selectFrom('pages as p')
-                .innerJoin('page_hierarchy as ph', 'ph.parentPageId', 'p.id')
-                .leftJoin('shares as s', 's.pageId', 'p.id')
-                .select([
-                  'p.id',
-                  'p.slugId',
-                  'p.title',
-                  'p.icon',
-                  'p.parentPageId',
-                  sql`ph.level + 1`.as('level'),
-                  's.id as shareId',
-                  's.key as shareKey',
-                  's.includeSubPages',
-                  's.searchIndexing',
-                  's.creatorId',
-                  's.spaceId',
-                  's.workspaceId',
-                  's.createdAt',
-                ])
-                .where('p.deletedAt', 'is', null)
-                .where(sql`ph.share_id`, 'is', null) // stop if share found
-                .where(sql`ph.level`, '<', sql`25`), // prevent loop
+          .unionAll((union) =>
+            union
+              .selectFrom('pages as p')
+              .innerJoin('page_hierarchy as ph', 'ph.parentPageId', 'p.id')
+              .leftJoin('shares as s', 's.pageId', 'p.id')
+              .select([
+                'p.id',
+                'p.slugId',
+                'p.title',
+                'p.icon',
+                'p.parentPageId',
+                sql`ph.level + 1`.as('level'),
+                's.id as shareId',
+                's.key as shareKey',
+                's.includeSubPages',
+                's.searchIndexing',
+                's.creatorId',
+                's.spaceId',
+                's.workspaceId',
+                's.createdAt',
+                sql<string[]>`ph.traversal_path || p.id`.as('traversalPath'),
+                sql<boolean>`p.id = ANY(ph.traversal_path)`.as('isCycle'),
+              ])
+              .where('p.deletedAt', 'is', null)
+              .where(sql`ph.share_id`, 'is', null) // stop if share found
+              .where('ph.isCycle', '=', false),
           ),
       )
       .selectFrom('page_hierarchy')
-      .selectAll()
-      .where('shareId', 'is not', null)
-      .limit(1)
-      .executeTakeFirst();
+      .select([
+        'id',
+        'slugId',
+        'title',
+        'icon',
+        'parentPageId',
+        'level',
+        'shareId',
+        'shareKey',
+        'includeSubPages',
+        'searchIndexing',
+        'creatorId',
+        'spaceId',
+        'workspaceId',
+        'createdAt',
+        'isCycle',
+      ])
+      .execute();
+
+    if (traversal.some((row) => row.isCycle)) {
+      return undefined;
+    }
+
+    const matchedShare = traversal.find((row) => row.shareId !== null);
+    const share = matchedShare
+      ? stripPageTraversalMetadata(matchedShare)
+      : undefined;
 
     if (!share || share.workspaceId !== workspaceId) {
       return undefined;

apps/server/test/page-hierarchy-cycle.integration-spec.ts

@@ -1,21 +1,172 @@
-import { db } from './support/database';
-import { seedAcyclicPageChain } from './support/page-hierarchy-fixtures';
+import { randomUUID } from 'node:crypto';
+import { PageService } from '../src/core/page/services/page.service';
+import { ShareService } from '../src/core/share/share.service';
+import { PageHierarchyCycleError } from '../src/database/helpers/page-hierarchy-cycle';
+import { KyselyDB } from '../src/database/types/kysely.types';
+import { db, withStatementTimeout } from './support/database';
+import {
+  seedAcyclicPageChain,
+  seedSelfCycle,
+  seedTwoPageCycle,
+} from './support/page-hierarchy-fixtures';
 
-describe('page hierarchy cycle integration harness', () => {
-  it('inserts and reads an acyclic page chain through the test Kysely instance', async () => {
-    const { root, child, grandchild } = await seedAcyclicPageChain();
+function createPageService(connection: KyselyDB): PageService {
+  return new PageService(
+    undefined as never,
+    undefined as never,
+    undefined as never,
+    connection,
+    undefined as never,
+    undefined as never,
+    undefined as never,
+    undefined as never,
+    undefined as never,
+    undefined as never,
+    undefined as never,
+    undefined as never,
+  );
+}
 
-    const pages = await db
-      .selectFrom('pages')
-      .select(['id', 'parentPageId', 'title'])
-      .where('id', 'in', [root.id, child.id, grandchild.id])
-      .orderBy('title')
-      .execute();
+function createShareService(connection: KyselyDB): ShareService {
+  return new ShareService(
+    undefined as never,
+    undefined as never,
+    undefined as never,
+    connection,
+    undefined as never,
+    undefined as never,
+  );
+}
 
-    expect(pages).toEqual([
-      { id: child.id, parentPageId: root.id, title: 'Child' },
-      { id: grandchild.id, parentPageId: child.id, title: 'Grandchild' },
-      { id: root.id, parentPageId: null, title: 'Root' },
-    ]);
+async function insertShare(pageId: string, includeSubPages: boolean) {
+  const page = await db
+    .selectFrom('pages')
+    .select(['spaceId', 'workspaceId'])
+    .where('id', '=', pageId)
+    .executeTakeFirstOrThrow();
+
+  return db
+    .insertInto('shares')
+    .values({
+      includeSubPages,
+      key: `cycle-test-${randomUUID()}`,
+      pageId,
+      searchIndexing: false,
+      spaceId: page.spaceId,
+      workspaceId: page.workspaceId,
+    })
+    .returning(['id', 'workspaceId'])
+    .executeTakeFirstOrThrow();
+}
+
+describe('cycle-safe page hierarchy reads', () => {
+  describe('PageService.getPageBreadCrumbs', () => {
+    it('returns every acyclic breadcrumb exactly once in root-to-child order', async () => {
+      const { root, child, grandchild } = await seedAcyclicPageChain();
+      const pageService = createPageService(db);
+
+      const breadcrumbs = await pageService.getPageBreadCrumbs(grandchild.id);
+
+      expect(breadcrumbs.map(({ id, title }) => ({ id, title }))).toEqual([
+        { id: root.id, title: 'Root' },
+        { id: child.id, title: 'Child' },
+        { id: grandchild.id, title: 'Grandchild' },
+      ]);
+      expect(breadcrumbs).toHaveLength(3);
+      for (const breadcrumb of breadcrumbs) {
+        expect(breadcrumb).not.toHaveProperty('isCycle');
+        expect(breadcrumb).not.toHaveProperty('traversalPath');
+      }
+    });
+
+    it('raises PageHierarchyCycleError for a self-cycle', async () => {
+      const { self } = await seedSelfCycle();
+
+      await withStatementTimeout(async (connection) => {
+        const pageService = createPageService(connection);
+
+        await expect(pageService.getPageBreadCrumbs(self.id)).rejects.toEqual(
+          expect.objectContaining({
+            code: 'PAGE_HIERARCHY_CYCLE',
+            rootPageId: self.id,
+          }) satisfies Partial<PageHierarchyCycleError>,
+        );
+      });
+    });
+
+    it('raises PageHierarchyCycleError for a two-page cycle', async () => {
+      const { a } = await seedTwoPageCycle();
+
+      await withStatementTimeout(async (connection) => {
+        const pageService = createPageService(connection);
+
+        await expect(pageService.getPageBreadCrumbs(a.id)).rejects.toEqual(
+          expect.objectContaining({
+            code: 'PAGE_HIERARCHY_CYCLE',
+            rootPageId: a.id,
+          }) satisfies Partial<PageHierarchyCycleError>,
+        );
+      });
+    });
+  });
+
+  describe('ShareService.getShareForPage', () => {
+    it('returns the nearest valid inherited share without a depth limit', async () => {
+      const { root, grandchild } = await seedAcyclicPageChain();
+      const context = await db
+        .selectFrom('pages')
+        .select(['spaceId', 'workspaceId'])
+        .where('id', '=', root.id)
+        .executeTakeFirstOrThrow();
+      let descendantId = grandchild.id;
+
+      for (let depth = 3; depth <= 26; depth += 1) {
+        const descendant = await db
+          .insertInto('pages')
+          .values({
+            parentPageId: descendantId,
+            slugId: randomUUID(),
+            spaceId: context.spaceId,
+            title: `Descendant ${depth}`,
+            workspaceId: context.workspaceId,
+          })
+          .returning('id')
+          .executeTakeFirstOrThrow();
+        descendantId = descendant.id;
+      }
+
+      const storedShare = await insertShare(root.id, true);
+      const shareService = createShareService(db);
+
+      const share = await shareService.getShareForPage(
+        descendantId,
+        context.workspaceId,
+      );
+
+      expect(share).toEqual(
+        expect.objectContaining({
+          id: storedShare.id,
+          pageId: root.id,
+          level: 26,
+        }),
+      );
+    });
+
+    it('returns undefined for a cyclic chain with no reachable share', async () => {
+      const { a } = await seedTwoPageCycle();
+      const { workspaceId } = await db
+        .selectFrom('pages')
+        .select('workspaceId')
+        .where('id', '=', a.id)
+        .executeTakeFirstOrThrow();
+
+      await withStatementTimeout(async (connection) => {
+        const shareService = createShareService(connection);
+
+        await expect(
+          shareService.getShareForPage(a.id, workspaceId),
+        ).resolves.toBeUndefined();
+      });
+    });
   });
 });