diff --git a/apps/server/src/database/repos/page/page-permission.repo.ts b/apps/server/src/database/repos/page/page-permission.repo.ts index 4fbad3563..4c240905c 100644 --- a/apps/server/src/database/repos/page/page-permission.repo.ts +++ b/apps/server/src/database/repos/page/page-permission.repo.ts @@ -728,7 +728,48 @@ export class PagePermissionRepo { if (spaceId) { const hasRestrictions = await this.hasRestrictedPagesInSpace(spaceId); if (!hasRestrictions) { - return pageIds; + const cyclicPages = await this.db + .withRecursive('allAncestors', (qb) => + qb + .selectFrom('pages') + .select([ + 'pages.id as pageId', + 'pages.id as ancestorId', + 'pages.parentPageId', + sql`ARRAY[pages.id]::uuid[]`.as('traversalPath'), + sql`false`.as('isCycle'), + ]) + .where(sql`pages.id = ANY(${pageIds}::uuid[])`) + .unionAll((eb) => + eb + .selectFrom('pages') + .innerJoin( + 'allAncestors', + 'allAncestors.parentPageId', + 'pages.id', + ) + .select([ + 'allAncestors.pageId', + 'pages.id as ancestorId', + 'pages.parentPageId', + sql`all_ancestors.traversal_path || pages.id`.as( + 'traversalPath', + ), + sql`pages.id = ANY(all_ancestors.traversal_path)`.as( + 'isCycle', + ), + ]) + .where('allAncestors.isCycle', '=', false), + ), + ) + .selectFrom('allAncestors') + .select('allAncestors.pageId') + .distinct() + .where('allAncestors.isCycle', '=', true) + .execute(); + const cyclicPageIds = new Set(cyclicPages.map((page) => page.pageId)); + + return pageIds.filter((pageId) => !cyclicPageIds.has(pageId)); } } diff --git a/apps/server/test/page-hierarchy-cycle.integration-spec.ts b/apps/server/test/page-hierarchy-cycle.integration-spec.ts index 4b192f6b3..95d9ad632 100644 --- a/apps/server/test/page-hierarchy-cycle.integration-spec.ts +++ b/apps/server/test/page-hierarchy-cycle.integration-spec.ts @@ -65,6 +65,45 @@ async function insertTestUser(pageId: string): Promise { return user.id; } +async function insertTwoPageCycleInPageSpace( + pageId: string, +): Promise<{ cyclePageId: string; spaceId: string }> { + const context = await db + .selectFrom('pages') + .select(['spaceId', 'workspaceId']) + .where('id', '=', pageId) + .executeTakeFirstOrThrow(); + const a = await db + .insertInto('pages') + .values({ + slugId: randomUUID(), + spaceId: context.spaceId, + title: 'Same-space cycle A', + workspaceId: context.workspaceId, + }) + .returning('id') + .executeTakeFirstOrThrow(); + const b = await db + .insertInto('pages') + .values({ + parentPageId: a.id, + slugId: randomUUID(), + spaceId: context.spaceId, + title: 'Same-space cycle B', + workspaceId: context.workspaceId, + }) + .returning('id') + .executeTakeFirstOrThrow(); + + await db + .updateTable('pages') + .set({ parentPageId: b.id }) + .where('id', '=', a.id) + .execute(); + + return { cyclePageId: a.id, spaceId: context.spaceId }; +} + async function restrictPage( pageId: string, permittedUserId?: string, @@ -474,6 +513,29 @@ describe('cycle-safe page hierarchy reads', () => { }); describe('bulk permissions', () => { + it('excludes a cyclic page from the unrestricted-space fast path', async () => { + const { grandchild } = await seedAcyclicPageChain(); + const { cyclePageId, spaceId } = await insertTwoPageCycleInPageSpace( + grandchild.id, + ); + const userId = await insertTestUser(grandchild.id); + + await withStatementTimeout(async (connection) => { + const repo = createPagePermissionRepo(connection); + + await expect(repo.hasRestrictedPagesInSpace(spaceId)).resolves.toBe( + false, + ); + await expect( + repo.filterAccessiblePageIds({ + pageIds: [grandchild.id, cyclePageId], + userId, + spaceId, + }), + ).resolves.toEqual([grandchild.id]); + }); + }); + it('keeps an accessible acyclic page while excluding a cyclic page', async () => { const { grandchild } = await seedAcyclicPageChain(); const { a } = await seedTwoPageCycle();