fix: validate cycles in permission fast path

This commit is contained in:
Philipinho
2026-09-02 12:16:46 +01:00
parent 82011fb417
commit e3843e7178
2 changed files with 104 additions and 1 deletions
@@ -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<string[]>`ARRAY[pages.id]::uuid[]`.as('traversalPath'),
sql<boolean>`false`.as('isCycle'),
])
.where(sql<SqlBool>`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<string[]>`all_ancestors.traversal_path || pages.id`.as(
'traversalPath',
),
sql<boolean>`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));
}
}
@@ -65,6 +65,45 @@ async function insertTestUser(pageId: string): Promise<string> {
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();