From 5b17503df7a0990aae87da335da06fe53c8b98b9 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 05:58:48 +0300 Subject: [PATCH] docs(page): explain breadcrumb ancestors need no per-ancestor permission filter (#471) getPageBreadCrumbs returns the full ancestor chain filtered only by deletedAt; the /breadcrumbs endpoint validates validateCanView on the target page only. Document why this is safe rather than an accepted leak: page restrictions inherit down the tree, so viewing a page implies the right to view every ancestor (canUserAccessPage checks the full ancestor chain). A hidden ancestor would already hide the target itself, making per-ancestor filtering redundant. Owner decision: document, no logic change. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/server/src/core/page/page.controller.ts | 5 ++++ .../src/core/page/services/page.service.ts | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/apps/server/src/core/page/page.controller.ts b/apps/server/src/core/page/page.controller.ts index 05b363e0..6355b1d3 100644 --- a/apps/server/src/core/page/page.controller.ts +++ b/apps/server/src/core/page/page.controller.ts @@ -818,6 +818,11 @@ export class PageController { throw new NotFoundException('Page not found'); } + // Target-only validateCanView is intentional: getPageBreadCrumbs returns + // the full ancestor chain WITHOUT per-ancestor permission filtering. Safe + // because page restrictions inherit down the tree, so any ancestor the + // caller could not view would already hide the target here — see the + // getPageBreadCrumbs docstring / #471. await this.pageAccessService.validateCanView(page, user); return this.pageService.getPageBreadCrumbs(page.id); diff --git a/apps/server/src/core/page/services/page.service.ts b/apps/server/src/core/page/services/page.service.ts index 421319a4..e08788c8 100644 --- a/apps/server/src/core/page/services/page.service.ts +++ b/apps/server/src/core/page/services/page.service.ts @@ -1071,6 +1071,29 @@ export class PageService { }); } + /** + * Walk the ancestor chain of `childPageId` up to the space root, filtered + * ONLY by `deletedAt` (+ MAX_PAGE_TREE_DEPTH) — WITHOUT per-ancestor + * permission filtering. Callers that expose this to a user (the + * `/breadcrumbs` endpoint) validate `validateCanView` on the TARGET page + * only, then return the whole chain of ancestor titles (#471). + * + * This is safe — NOT a title leak — because page restrictions inherit DOWN + * the tree: to view a page the caller must hold permission on EVERY + * restricted ancestor (`validateCanView` -> `canUserAccessPage` checks the + * full ancestor chain — see page-access.service.ts / page-permission.repo.ts + * `canUserEditPage`). A restricted ancestor the caller may not see would + * therefore already hide the TARGET page itself, so every ancestor reachable + * here is one the caller is already entitled to view (content stays gated + * regardless — getPage/getNode re-check permissions). + * + * Note the guarantee is the narrow "may view a descendant => may view its + * ancestors", NOT "space membership sees every page" — restricted subtrees do + * hide pages from members. Per-ancestor permission filtering here was + * considered and declined as redundant given the inheritance invariant above + * (#471). The same chain feeds the web-UI breadcrumb bar under identical CASL + * scope. + */ async getPageBreadCrumbs(childPageId: string, trx?: KyselyTransaction) { const ancestors = await dbOrTx(this.db, trx) .withRecursive('page_ancestors', (db) =>