harden(page-templates): throttle lookup/toggle; workspace-scope ref writes

Release-cycle review: POST /pages/template/lookup had only JwtAuthGuard and the
embed depth cap was client-only, so a scripted client could drive heavy
full-content fan-out (access control holds per-id, but a cost/DoS gap). And
page_template_references rows were written for any sourcePageId with no
workspace check at sync time (no leak today since lookup re-checks access, but
the graph could accumulate cross-space rows).

- Apply the standard per-user throttler (PAGE_TEMPLATE_THROTTLER, 30/min) to
  /pages/template/lookup and /pages/toggle-template (mirrors ai-chat); auth +
  the toggle's validateCanEdit CASL are unchanged.
- syncPageTemplateReferences / insertTemplateReferencesForPages now restrict
  inserts to in-workspace source ids (filterInWorkspaceSourceIds, workspace +
  not-deleted scoped, trx-aware) and still delete stale out-of-workspace rows
  (self-heal). SECURITY comment: the ref table is NOT access-filtered; every
  consumer must permission-filter at read time (as lookupTemplate does).
- Tests: lookup access exercises the REAL filterViewerAccessiblePageIds
  (no_access / cross-workspace excluded / accessible+comment-stripped / <=50);
  toggle controller CASL (cannot-edit -> Forbidden, flag not flipped); ref-sync
  excludes cross-workspace and keeps in-workspace.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

apps/server/src/core/page/transclusion/page-template.controller.ts

@@ -7,6 +7,7 @@ import {
   Post,
   UseGuards,
 } from '@nestjs/common';
+import { Throttle } from '@nestjs/throttler';
 import { JwtAuthGuard } from '../../../common/guards/jwt-auth.guard';
 import { AuthUser } from '../../../common/decorators/auth-user.decorator';
 import { User } from '@docmost/db/types/entity.types';
@@ -15,6 +16,8 @@ import { TemplateLookupDto } from './dto/template-lookup.dto';
 import { PageRepo } from '@docmost/db/repos/page/page.repo';
 import { PageAccessService } from '../page-access/page-access.service';
 import { ToggleTemplateDto } from './dto/toggle-template.dto';
+import { UserThrottlerGuard } from '../../../integrations/throttle/user-throttler.guard';
+import { PAGE_TEMPLATE_THROTTLER } from '../../../integrations/throttle/throttler-names';
 
 @UseGuards(JwtAuthGuard)
 @Controller('pages')
@@ -28,7 +31,14 @@ export class PageTemplateController {
   /**
    * Whole-page live embed lookup for authenticated viewers. Returns current
    * content (comment marks stripped) for accessible source pages.
+   *
+   * DoS note: the embed cycle/depth cap (PAGE_EMBED_MAX_DEPTH=5) is enforced
+   * CLIENT-side only — a scripted client could otherwise drive heavy full-doc
+   * fan-out. The server bounds the cost with this per-user throttle plus the
+   * DTO's ArrayMaxSize(50) cap; server-side recursive expansion is out of scope.
    */
+  @UseGuards(JwtAuthGuard, UserThrottlerGuard)
+  @Throttle({ [PAGE_TEMPLATE_THROTTLER]: { limit: 30, ttl: 60000 } })
   @HttpCode(HttpStatus.OK)
   @Post('template/lookup')
   async lookup(@Body() dto: TemplateLookupDto, @AuthUser() user: User) {
@@ -44,6 +54,8 @@ export class PageTemplateController {
    * inside `validateCanEdit`). The flag only affects template picker discovery;
    * it does not restrict editing or embedding.
    */
+  @UseGuards(JwtAuthGuard, UserThrottlerGuard)
+  @Throttle({ [PAGE_TEMPLATE_THROTTLER]: { limit: 30, ttl: 60000 } })
   @HttpCode(HttpStatus.OK)
   @Post('toggle-template')
   async toggleTemplate(

apps/server/src/core/page/transclusion/spec/page-template-access.spec.ts

@@ -0,0 +1,267 @@
+import { TransclusionService } from '../transclusion.service';
+
+/**
+ * Exercises the REAL security core of the whole-page template feature rather
+ * than mocking it away:
+ *  - `filterViewerAccessiblePageIds` runs for real (space-visibility query +
+ *    page-permission filter are stubbed, but the branching/AND-ing is real), so
+ *    `lookupTemplate` actually maps no_access vs content based on it.
+ *  - the workspace scoping of `page_template_references` writes is verified to
+ *    drop cross-workspace source ids before they are persisted.
+ */
+describe('TransclusionService — template access core (real filter)', () => {
+  /**
+   * Build a chainable kysely `db` stub. `selectFrom(...).select(...).where(...)`
+   * all return the same builder; `.execute()` resolves the supplied rows. The
+   * `where('spaceId','in', getUserSpaceIdsQuery(...))` sub-query argument is
+   * ignored — space visibility is decided by what `execute()` returns.
+   */
+  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;
+  }
+
+  function makeService(opts: {
+    /** rows returned by the space-visibility query (workspace + space scoped) */
+    spaceVisibleRows: Array<{ id: string }>;
+    /** ids that survive page-level permission filtering */
+    permissionAccessibleIds: string[];
+    pages?: Array<{
+      id: string;
+      slugId?: string;
+      title: string | null;
+      icon: string | null;
+      content: unknown;
+      updatedAt: Date;
+    }>;
+  }) {
+    const db = makeDb(opts.spaceVisibleRows);
+
+    const spaceMemberRepo = {
+      // The real code only passes this query object into `.where(...)`; our db
+      // stub ignores it, so a sentinel is fine.
+      getUserSpaceIdsQuery: jest.fn(() => ({ __subquery: true })),
+    };
+
+    const pagePermissionRepo = {
+      filterAccessiblePageIds: jest
+        .fn()
+        .mockResolvedValue(opts.permissionAccessibleIds),
+    };
+
+    const pageRepo = {
+      findManyByIds: jest.fn().mockResolvedValue(opts.pages ?? []),
+    };
+
+    const service = new TransclusionService(
+      db as any,
+      {} as any, // pageTransclusionsRepo
+      {} as any, // pageTransclusionReferencesRepo
+      {} as any, // pageTemplateReferencesRepo
+      pageRepo as any,
+      pagePermissionRepo as any,
+      spaceMemberRepo as any,
+      {} as any, // attachmentRepo
+      {} as any, // storageService
+      {} as any, // pageAccessService
+    );
+
+    return { service, db, pageRepo, spaceMemberRepo, pagePermissionRepo };
+  }
+
+  const now = new Date('2026-06-20T00:00:00.000Z');
+
+  it('returns no_access when the viewer fails the page-permission filter (real filter runs)', async () => {
+    // Space-visible, but page-permission filter rejects it.
+    const { service, pagePermissionRepo } = makeService({
+      spaceVisibleRows: [{ id: 'p1' }],
+      permissionAccessibleIds: [],
+    });
+
+    const { items } = await service.lookupTemplate(['p1'], 'u1', 'w1');
+    expect(items).toEqual([{ sourcePageId: 'p1', status: 'no_access' }]);
+    // proves the real filter executed and consulted page permissions
+    expect(pagePermissionRepo.filterAccessiblePageIds).toHaveBeenCalledWith({
+      pageIds: ['p1'],
+      userId: 'u1',
+    });
+  });
+
+  it('returns no_access for a cross-workspace id (space-visibility query excludes it)', async () => {
+    // The workspace/space-scoped query returns nothing → permission filter is
+    // never reached and the id is not returned as accessible.
+    const { service, pagePermissionRepo } = makeService({
+      spaceVisibleRows: [],
+      permissionAccessibleIds: ['cross-ws'],
+    });
+
+    const { items } = await service.lookupTemplate(['cross-ws'], 'u1', 'w1');
+    expect(items).toEqual([{ sourcePageId: 'cross-ws', status: 'no_access' }]);
+    // short-circuited before page-permission filtering
+    expect(pagePermissionRepo.filterAccessiblePageIds).not.toHaveBeenCalled();
+  });
+
+  it('returns content with comment marks stripped for an accessible page', async () => {
+    const content = {
+      type: 'doc',
+      content: [
+        {
+          type: 'paragraph',
+          content: [
+            {
+              type: 'text',
+              text: 'hello',
+              marks: [{ type: 'comment', attrs: { commentId: 'c1' } }],
+            },
+          ],
+        },
+      ],
+    };
+
+    const { service } = makeService({
+      spaceVisibleRows: [{ id: 'p1' }],
+      permissionAccessibleIds: ['p1'],
+      pages: [
+        {
+          id: 'p1',
+          slugId: 's1',
+          title: 'Tmpl',
+          icon: '📄',
+          content,
+          updatedAt: now,
+        },
+      ],
+    });
+
+    const { items } = await service.lookupTemplate(['p1'], 'u1', 'w1');
+    const item = items[0] as any;
+    expect(item.status).toBeUndefined();
+    expect(item.title).toBe('Tmpl');
+    const json = JSON.stringify(item.content);
+    expect(json).not.toContain('comment');
+    expect(json).toContain('hello');
+  });
+
+  it('mixes accessible and inaccessible ids in one batch positionally', async () => {
+    const { service } = makeService({
+      spaceVisibleRows: [{ id: 'ok' }, { id: 'denied' }],
+      permissionAccessibleIds: ['ok'],
+      pages: [
+        {
+          id: 'ok',
+          slugId: 's',
+          title: 'A',
+          icon: null,
+          content: { type: 'doc', content: [] },
+          updatedAt: now,
+        },
+      ],
+    });
+
+    const { items } = await service.lookupTemplate(
+      ['denied', 'ok', 'cross'],
+      'u1',
+      'w1',
+    );
+    expect((items[0] as any).status).toBe('no_access'); // space-visible but no perm
+    expect((items[1] as any).status).toBeUndefined(); // accessible
+    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.
+    const ids = ['a', 'a', 'b'];
+    const { service, db } = makeService({
+      spaceVisibleRows: [],
+      permissionAccessibleIds: [],
+    });
+
+    await service.lookupTemplate(ids, 'u1', 'w1');
+    // db.where('id','in', <uniqueIds>) — verify the in-clause got deduped ids
+    const inCall = db.where.mock.calls.find((c: any[]) => c[0] === 'id');
+    expect(inCall?.[2]).toEqual(['a', 'b']);
+  });
+});
+
+describe('TransclusionService.syncPageTemplateReferences — workspace scoping', () => {
+  function makeService(opts: { inWorkspaceIds: string[] }) {
+    // db stub: the in-workspace existence query returns only allowed ids.
+    const builder: any = {};
+    builder.selectFrom = jest.fn(() => builder);
+    builder.select = jest.fn(() => builder);
+    builder.where = jest.fn(() => builder);
+    builder.execute = jest.fn(async () =>
+      opts.inWorkspaceIds.map((id) => ({ id })),
+    );
+
+    const insertMany = jest.fn().mockResolvedValue(undefined);
+    const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined);
+    const pageTemplateReferencesRepo = {
+      findByReferencePageId: jest.fn().mockResolvedValue([]),
+      insertMany,
+      deleteByReferenceAndSources,
+    };
+
+    const service = new TransclusionService(
+      builder as any,
+      {} as any,
+      {} as any,
+      pageTemplateReferencesRepo as any,
+      {} as any,
+      {} as any,
+      {} as any,
+      {} as any,
+      {} as any,
+      {} as any,
+    );
+
+    return { service, insertMany, pageTemplateReferencesRepo };
+  }
+
+  function docWithEmbeds(sourceIds: string[]) {
+    return {
+      type: 'doc',
+      content: sourceIds.map((id) => ({
+        type: 'pageEmbed',
+        attrs: { sourcePageId: id },
+      })),
+    };
+  }
+
+  it('does NOT write a row for a cross-workspace sourcePageId, but writes the in-workspace one', async () => {
+    const { service, insertMany } = makeService({
+      // only the in-workspace id survives the existence query
+      inWorkspaceIds: ['in-ws'],
+    });
+
+    const result = await service.syncPageTemplateReferences(
+      'host',
+      'w1',
+      docWithEmbeds(['in-ws', 'cross-ws']),
+    );
+
+    expect(result.inserted).toBe(1);
+    expect(insertMany).toHaveBeenCalledTimes(1);
+    const rows = insertMany.mock.calls[0][0];
+    expect(rows).toEqual([
+      { workspaceId: 'w1', referencePageId: 'host', sourcePageId: 'in-ws' },
+    ]);
+  });
+
+  it('inserts nothing when every embed points at a cross-workspace source', async () => {
+    const { service, insertMany } = makeService({ inWorkspaceIds: [] });
+
+    const result = await service.syncPageTemplateReferences(
+      'host',
+      'w1',
+      docWithEmbeds(['cross-a', 'cross-b']),
+    );
+
+    expect(result.inserted).toBe(0);
+    expect(insertMany).not.toHaveBeenCalled();
+  });
+});

apps/server/src/core/page/transclusion/spec/page-template.controller.spec.ts

@@ -0,0 +1,93 @@
+import { Test } from '@nestjs/testing';
+import { ForbiddenException, NotFoundException } from '@nestjs/common';
+import { PageTemplateController } from '../page-template.controller';
+import { TransclusionService } from '../transclusion.service';
+import { PageRepo } from '@docmost/db/repos/page/page.repo';
+import { PageAccessService } from '../../page-access/page-access.service';
+import { JwtAuthGuard } from '../../../../common/guards/jwt-auth.guard';
+import { UserThrottlerGuard } from '../../../../integrations/throttle/user-throttler.guard';
+
+describe('PageTemplateController.toggleTemplate', () => {
+  let controller: PageTemplateController;
+  let pageRepo: { findById: jest.Mock; updatePage: jest.Mock };
+  let pageAccessService: { validateCanEdit: jest.Mock };
+  let transclusionService: Partial<jest.Mocked<TransclusionService>>;
+
+  const user = { id: 'u1', workspaceId: 'w1' } as any;
+  const page = {
+    id: 'p1',
+    workspaceId: 'w1',
+    deletedAt: null,
+    isTemplate: false,
+  } as any;
+
+  beforeEach(async () => {
+    pageRepo = {
+      findById: jest.fn().mockResolvedValue(page),
+      updatePage: jest.fn().mockResolvedValue(undefined),
+    };
+    pageAccessService = {
+      validateCanEdit: jest.fn().mockResolvedValue(undefined),
+    };
+    transclusionService = { lookupTemplate: jest.fn() };
+
+    const module = await Test.createTestingModule({
+      controllers: [PageTemplateController],
+      providers: [
+        { provide: TransclusionService, useValue: transclusionService },
+        { provide: PageRepo, useValue: pageRepo },
+        { provide: PageAccessService, useValue: pageAccessService },
+      ],
+    })
+      .overrideGuard(JwtAuthGuard)
+      .useValue({ canActivate: () => true })
+      .overrideGuard(UserThrottlerGuard)
+      .useValue({ canActivate: () => true })
+      .compile();
+
+    controller = module.get(PageTemplateController);
+  });
+
+  it('throws NotFound and does not touch the page when the page is missing', async () => {
+    pageRepo.findById.mockResolvedValue(null);
+    await expect(
+      controller.toggleTemplate({ pageId: 'p1' } as any, user),
+    ).rejects.toBeInstanceOf(NotFoundException);
+    expect(pageAccessService.validateCanEdit).not.toHaveBeenCalled();
+    expect(pageRepo.updatePage).not.toHaveBeenCalled();
+  });
+
+  it('enforces CASL edit: when validateCanEdit throws, the flag is NOT flipped', async () => {
+    pageAccessService.validateCanEdit.mockRejectedValue(
+      new ForbiddenException(),
+    );
+    await expect(
+      controller.toggleTemplate({ pageId: 'p1' } as any, user),
+    ).rejects.toBeInstanceOf(ForbiddenException);
+    expect(pageAccessService.validateCanEdit).toHaveBeenCalledWith(page, user);
+    expect(pageRepo.updatePage).not.toHaveBeenCalled();
+  });
+
+  it('flips is_template (toggle) when the user can edit', async () => {
+    const out = await controller.toggleTemplate(
+      { pageId: 'p1' } as any,
+      user,
+    );
+    expect(pageAccessService.validateCanEdit).toHaveBeenCalledWith(page, user);
+    // page.isTemplate was false → toggled to true
+    expect(pageRepo.updatePage).toHaveBeenCalledWith({ isTemplate: true }, 'p1');
+    expect(out).toEqual({ pageId: 'p1', isTemplate: true });
+  });
+
+  it('respects an explicit isTemplate flag instead of toggling', async () => {
+    const out = await controller.toggleTemplate(
+      { pageId: 'p1', isTemplate: false } as any,
+      user,
+    );
+    expect(pageRepo.updatePage).toHaveBeenCalledWith(
+      { isTemplate: false },
+      'p1',
+    );
+    expect(out).toEqual({ pageId: 'p1', isTemplate: false });
+  });
+});

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

@@ -232,10 +232,41 @@ export class TransclusionService {
   // Whole-page live embeds (pageEmbed node)
   // ---------------------------------------------------------------------------
 
+  /**
+   * Restrict a set of candidate `pageEmbed` source ids to the pages that
+   * actually live in `workspaceId` (and are not soft-deleted). Defense in depth:
+   * `page_template_references` is NOT access-filtered, so we must never persist a
+   * reference to a cross-workspace source page. This is a single workspace-scoped
+   * existence query; it does NOT do per-viewer permission filtering (that stays
+   * the job of `lookupTemplate` at read time — see the warning below).
+   */
+  private async filterInWorkspaceSourceIds(
+    sourceIds: string[],
+    workspaceId: string,
+    trx?: KyselyTransaction,
+  ): Promise<Set<string>> {
+    if (sourceIds.length === 0) return new Set();
+    const db = trx ?? this.db;
+    const rows = await db
+      .selectFrom('pages')
+      .select('id')
+      .where('id', 'in', sourceIds)
+      .where('workspaceId', '=', workspaceId)
+      .where('deletedAt', 'is', null)
+      .execute();
+    return new Set(rows.map((r) => r.id));
+  }
+
   /**
    * Diff `page_template_references` for a host page against the `pageEmbed`
    * nodes currently in its content. Mirror of `syncPageReferences` but keyed by
    * `sourcePageId` only (whole-page, no transclusionId). Idempotent.
+   *
+   * SECURITY: `page_template_references` rows are NOT access-filtered. Inserts
+   * are restricted here to in-workspace source pages so the graph can never
+   * accumulate cross-workspace edges, but rows are still NOT per-viewer
+   * permission-filtered. EVERY consumer of these rows MUST permission-filter at
+   * read time (as `lookupTemplate` does via `filterViewerAccessiblePageIds`).
    */
   async syncPageTemplateReferences(
     referencePageId: string,
@@ -244,7 +275,14 @@ export class TransclusionService {
     trx?: KyselyTransaction,
   ): Promise<{ inserted: number; deleted: number }> {
     const desired = collectPageEmbedsFromPmJson(pmJson);
-    const desiredIds = new Set(desired.map((d) => d.sourcePageId));
+    const inWorkspace = await this.filterInWorkspaceSourceIds(
+      desired.map((d) => d.sourcePageId),
+      workspaceId,
+      trx,
+    );
+    const desiredIds = new Set(
+      desired.map((d) => d.sourcePageId).filter((id) => inWorkspace.has(id)),
+    );
 
     const existing =
       await this.pageTemplateReferencesRepo.findByReferencePageId(
@@ -253,12 +291,12 @@ export class TransclusionService {
       );
     const existingIds = new Set(existing.map((e) => e.sourcePageId));
 
-    const toInsert = desired
-      .filter((d) => !existingIds.has(d.sourcePageId))
-      .map((d) => ({
+    const toInsert = Array.from(desiredIds)
+      .filter((id) => !existingIds.has(id))
+      .map((sourcePageId) => ({
         workspaceId,
         referencePageId,
-        sourcePageId: d.sourcePageId,
+        sourcePageId,
       }));
 
     const toDelete = existing
@@ -282,23 +320,57 @@ export class TransclusionService {
   /**
    * Bulk-insert `page_template_references` for brand-new pages (duplication,
    * import) where there is nothing to diff against.
+   *
+   * SECURITY: like `syncPageTemplateReferences`, inserts are restricted to
+   * in-workspace source pages so the (non-access-filtered) reference graph never
+   * gains a cross-workspace edge. Read-time per-viewer permission filtering is
+   * still required by every consumer.
    */
   async insertTemplateReferencesForPages(
     pages: Array<{ id: string; workspaceId: string; content: unknown }>,
     trx?: KyselyTransaction,
   ): Promise<{ inserted: number }> {
+    // Collect candidate source ids per workspace, then validate each workspace's
+    // set in a single existence query before building insert rows.
+    const candidatesByWorkspace = new Map<string, Set<string>>();
+    const pageEmbeds = pages.map((page) => {
+      const sourceIds = collectPageEmbedsFromPmJson(page.content).map(
+        (e) => e.sourcePageId,
+      );
+      let set = candidatesByWorkspace.get(page.workspaceId);
+      if (!set) {
+        set = new Set();
+        candidatesByWorkspace.set(page.workspaceId, set);
+      }
+      for (const id of sourceIds) set.add(id);
+      return { page, sourceIds };
+    });
+
+    const inWorkspaceByWorkspace = new Map<string, Set<string>>();
+    for (const [workspaceId, candidates] of candidatesByWorkspace) {
+      inWorkspaceByWorkspace.set(
+        workspaceId,
+        await this.filterInWorkspaceSourceIds(
+          Array.from(candidates),
+          workspaceId,
+          trx,
+        ),
+      );
+    }
+
     const rows: Array<{
       workspaceId: string;
       referencePageId: string;
       sourcePageId: string;
     }> = [];
-    for (const page of pages) {
-      const embeds = collectPageEmbedsFromPmJson(page.content);
-      for (const e of embeds) {
+    for (const { page, sourceIds } of pageEmbeds) {
+      const inWorkspace = inWorkspaceByWorkspace.get(page.workspaceId);
+      for (const sourcePageId of sourceIds) {
+        if (!inWorkspace?.has(sourcePageId)) continue;
         rows.push({
           workspaceId: page.workspaceId,
           referencePageId: page.id,
-          sourcePageId: e.sourcePageId,
+          sourcePageId,
         });
       }
     }

apps/server/src/integrations/throttle/throttle.module.ts

@@ -4,7 +4,11 @@ import { ThrottlerStorageRedisService } from '@nest-lab/throttler-storage-redis'
 import { EnvironmentService } from '../environment/environment.service';
 import { EnvironmentModule } from '../environment/environment.module';
 import { parseRedisUrl } from '../../common/helpers';
-import { AUTH_THROTTLER, AI_CHAT_THROTTLER } from './throttler-names';
+import {
+  AUTH_THROTTLER,
+  AI_CHAT_THROTTLER,
+  PAGE_TEMPLATE_THROTTLER,
+} from './throttler-names';
 import Redis from 'ioredis';
 
 @Module({
@@ -18,6 +22,11 @@ import Redis from 'ioredis';
           throttlers: [
             { name: AUTH_THROTTLER, ttl: 60_000, limit: 10 },
             { name: AI_CHAT_THROTTLER, ttl: 60_000, limit: 25 },
+            // Whole-page template lookup returns full ProseMirror docs for up
+            // to 50 ids per call and the embed depth cap is client-side only, so
+            // a scripted client could drive heavy content fan-out. 30 req/min
+            // per user is plenty for legitimate render-time batched lookups.
+            { name: PAGE_TEMPLATE_THROTTLER, ttl: 60_000, limit: 30 },
           ],
           errorMessage: 'Too many requests',
           storage: new ThrottlerStorageRedisService(

apps/server/src/integrations/throttle/throttler-names.ts

@@ -1,2 +1,3 @@
 export const AUTH_THROTTLER = 'auth';
 export const AI_CHAT_THROTTLER = 'ai-chat';
+export const PAGE_TEMPLATE_THROTTLER = 'page-template';