test(page-templates): cover lookupTemplate anti-leak + edge cases (#33)

- The security-relevant catch->not_found branch in lookupTemplate (returns
  not_found instead of raw content when comment-mark stripping throws) is now
  tested by forcing the strip to throw with a malformed text node, asserting no
  content/marks leak.
- not_found for a soft-deleted source resolved through the REAL
  filterViewerAccessiblePageIds (deletedAt-excluded), not the stubbed filter.
- Rename the misleading 'honours <=50 cap' test to reflect it only exercises
  dedup (the cap lives in the DTO, never engaged in the service unit).
- Cover the onlyTemplates search filter (restricts to is_template=true).
Also fix two pre-existing failing 'should be defined' specs (search service +
controller) that couldn't resolve the @InjectKysely token via createTestingModule.

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

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

@@ -173,8 +173,13 @@ describe('TransclusionService — template access core (real filter)', () => {
     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.
+  it('dedupes source ids before passing them to the access filter', async () => {
+    // NOTE: this test only covers DEDUP, not the ≤50 cap. The ArrayMaxSize(50)
+    // limit is enforced by the DTO (validation layer), so it is never engaged in
+    // the service under unit test — the service receives an already-validated
+    // array and merely dedupes it. Renamed from the old "honours ≤50 cap" title,
+    // which misleadingly implied the cap was exercised here. A real cap test would
+    // belong in a controller/DTO-validation spec, not in this service unit test.
     const ids = ['a', 'a', 'b'];
     const { service, db } = makeService({
       spaceVisibleRows: [],

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

@@ -0,0 +1,183 @@
+import { TransclusionService } from '../transclusion.service';
+
+/**
+ * Edge-case + anti-leak coverage for `lookupTemplate` that the existing
+ * `page-template-lookup.spec.ts` (stubbed filter) and `page-template-access.spec.ts`
+ * (real filter, happy paths) do not exercise:
+ *
+ *  1. SECURITY anti-leak: when comment-mark stripping THROWS, the item must come
+ *     back as `not_found` and NEVER carry raw content (the source's comment marks
+ *     could otherwise leak to a viewer). See the `catch` branch in `lookupTemplate`.
+ *  2. A soft-deleted source page resolved through the REAL
+ *     `filterViewerAccessiblePageIds` (space-visibility query filters `deletedAt`),
+ *     asserting it maps to `not_found`/`no_access` rather than content.
+ */
+describe('TransclusionService.lookupTemplate — anti-leak catch branch', () => {
+  const now = new Date('2026-06-20T00:00:00.000Z');
+
+  function makeService(opts: {
+    accessibleIds: string[];
+    pages: Array<{
+      id: string;
+      slugId?: string;
+      title: string | null;
+      icon: string | null;
+      content: unknown;
+      updatedAt: Date;
+    }>;
+  }) {
+    const pageRepo = {
+      findManyByIds: jest.fn().mockResolvedValue(opts.pages),
+    };
+
+    const service = new TransclusionService(
+      {} as any, // db
+      {} as any, // pageTransclusionsRepo
+      {} as any, // pageTransclusionReferencesRepo
+      {} as any, // pageTemplateReferencesRepo
+      pageRepo as any,
+      {} as any, // pagePermissionRepo
+      {} as any, // spaceMemberRepo
+      {} as any, // attachmentRepo
+      {} as any, // storageService
+      {} as any, // pageAccessService
+      {} as any, // workspaceRepo
+    );
+
+    // Stub the access decision; we are testing the content-prep stage, not access.
+    jest
+      .spyOn(service, 'filterViewerAccessiblePageIds')
+      .mockResolvedValue(opts.accessibleIds);
+
+    return { service, pageRepo };
+  }
+
+  it('returns not_found (NOT raw content) when comment-mark stripping throws', async () => {
+    // An accessible, present page whose stored content is structurally invalid PM:
+    // a `text` node without a `text` field. `jsonToNode` (called inside the try
+    // block) throws "Invalid text node in JSON" on this, which exercises the
+    // service's catch -> not_found anti-leak guard. This uses a REAL malformed
+    // input (no module mocking) so the test stays faithful to production behaviour.
+    const malformedContent = {
+      type: 'doc',
+      content: [
+        {
+          type: 'paragraph',
+          content: [
+            {
+              // Missing `text` — Node.fromJSON rejects this and jsonToNode rethrows.
+              type: 'text',
+              marks: [{ type: 'comment', attrs: { commentId: 'leak-me' } }],
+            },
+          ],
+        },
+      ],
+    };
+
+    const { service } = makeService({
+      accessibleIds: ['p1'],
+      pages: [
+        {
+          id: 'p1',
+          slugId: 's1',
+          title: 'Secret',
+          icon: '📄',
+          content: malformedContent,
+          updatedAt: now,
+        },
+      ],
+    });
+
+    // Silence the expected error log so the suite output stays clean.
+    jest.spyOn((service as any).logger, 'error').mockImplementation(() => {});
+
+    const { items } = await service.lookupTemplate(['p1'], 'u1', 'w1');
+
+    expect(items).toHaveLength(1);
+    const item = items[0] as any;
+
+    // Must degrade to not_found...
+    expect(item.status).toBe('not_found');
+    expect(item.sourcePageId).toBe('p1');
+
+    // ...and must NOT leak ANY content/metadata of the source page.
+    expect(item).not.toHaveProperty('content');
+    expect(item).not.toHaveProperty('title');
+    expect(item).not.toHaveProperty('icon');
+    expect(item).not.toHaveProperty('slugId');
+    expect(item).not.toHaveProperty('sourceUpdatedAt');
+
+    // Hard guarantee: the would-be-leaked comment mark appears nowhere in output.
+    expect(JSON.stringify(item)).not.toContain('leak-me');
+    expect(JSON.stringify(item)).not.toContain('comment');
+  });
+});
+
+describe('TransclusionService.lookupTemplate — soft-deleted source via real filter', () => {
+  const now = new Date('2026-06-20T00:00:00.000Z');
+
+  /**
+   * Chainable kysely `db` stub mirroring `page-template-access.spec.ts`. The
+   * space-visibility query in `filterViewerAccessiblePageIds` filters
+   * `where('deletedAt','is',null)`; a soft-deleted page is therefore absent from
+   * the rows we resolve here, so the REAL filter is what drops it.
+   */
+  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;
+  }
+
+  it('resolves a soft-deleted source to not_found/no_access through the REAL filter', async () => {
+    // The page IS soft-deleted, so the space-visibility query returns no rows for
+    // it (deletedAt filter). We let the real filter run end-to-end.
+    const db = makeDb([]); // soft-deleted -> excluded by the deletedAt='is null' clause
+
+    const spaceMemberRepo = {
+      getUserSpaceIdsQuery: jest.fn(() => ({ __subquery: true })),
+    };
+    const pagePermissionRepo = {
+      filterAccessiblePageIds: jest.fn().mockResolvedValue([]),
+    };
+    const pageRepo = {
+      // Even if it were queried, the page is gone; assert via the filter instead.
+      findManyByIds: jest.fn().mockResolvedValue([]),
+    };
+
+    const service = new TransclusionService(
+      db as any,
+      {} as any,
+      {} as any,
+      {} as any,
+      pageRepo as any,
+      pagePermissionRepo as any,
+      spaceMemberRepo as any,
+      {} as any,
+      {} as any,
+      {} as any,
+      {} as any,
+    );
+
+    const { items } = await service.lookupTemplate(['deleted-src'], 'u1', 'w1');
+
+    // Soft-deleted source must never resolve to content.
+    expect(items).toEqual([
+      { sourcePageId: 'deleted-src', status: 'no_access' },
+    ]);
+    const item = items[0] as any;
+    expect(item).not.toHaveProperty('content');
+
+    // The real filter short-circuited before page-permission filtering because
+    // the deletedAt-filtered space-visibility query returned nothing.
+    expect(pagePermissionRepo.filterAccessiblePageIds).not.toHaveBeenCalled();
+    // And the verb on the db builder included a deletedAt 'is null' guard, proving
+    // the real path (not a stub) excluded the soft-deleted page.
+    const deletedAtCall = db.where.mock.calls.find(
+      (c: any[]) => c[0] === 'deletedAt',
+    );
+    expect(deletedAtCall).toEqual(['deletedAt', 'is', null]);
+  });
+});

apps/server/src/core/search/search.controller.spec.ts

@@ -1,15 +1,19 @@
-import { Test, TestingModule } from '@nestjs/testing';
 import { SearchController } from './search.controller';
 
+// Direct instantiation with stub deps. The Test.createTestingModule form failed
+// to resolve SearchService's @InjectKysely() connection token at compile() (the
+// same Nest-DI/Kysely-token issue addressed in search.service.spec), and this
+// unit only needs the controller to construct.
 describe('SearchController', () => {
   let controller: SearchController;
 
-  beforeEach(async () => {
-    const module: TestingModule = await Test.createTestingModule({
-      controllers: [SearchController],
-    }).compile();
-
-    controller = module.get<SearchController>(SearchController);
+  beforeEach(() => {
+    controller = new SearchController(
+      {} as any, // searchService
+      {} as any, // spaceAbility
+      {} as any, // environmentService
+      {} as any, // moduleRef
+    );
   });
 
   it('should be defined', () => {

apps/server/src/core/search/search.service.spec.ts

@@ -1,18 +1,101 @@
-import { Test, TestingModule } from '@nestjs/testing';
 import { SearchService } from './search.service';
 
 describe('SearchService', () => {
-  let service: SearchService;
-
-  beforeEach(async () => {
-    const module: TestingModule = await Test.createTestingModule({
-      providers: [SearchService],
-    }).compile();
-
-    service = module.get<SearchService>(SearchService);
-  });
-
   it('should be defined', () => {
+    // Construct directly with stub deps. The previous Test.createTestingModule
+    // form could not resolve the @InjectKysely() connection token and failed at
+    // compile() — manual construction mirrors the rest of these unit specs.
+    const service = new SearchService(
+      {} as any, // db
+      {} as any, // pageRepo
+      {} as any, // shareRepo
+      {} as any, // spaceMemberRepo
+      {} as any, // pagePermissionRepo
+    );
     expect(service).toBeDefined();
   });
 });
+
+/**
+ * Focused coverage for the `onlyTemplates` flag in `searchSuggestions`, which
+ * restricts page suggestions to template pages (`is_template = true`). The kysely
+ * query builder and repos are mocked the same way the access specs mock chainable
+ * builders: every builder method returns the same builder, `.execute()` resolves
+ * the supplied rows. We assert whether `.where('isTemplate', '=', true)` is added.
+ */
+describe('SearchService.searchSuggestions — onlyTemplates filter', () => {
+  function makeService(pageRows: Array<{ id: string }>) {
+    // Chainable page-search builder. Record every `.where(...)` call so we can
+    // assert on the is_template restriction.
+    const pageBuilder: any = {};
+    pageBuilder.select = jest.fn(() => pageBuilder);
+    pageBuilder.where = jest.fn(() => pageBuilder);
+    pageBuilder.orderBy = jest.fn(() => pageBuilder);
+    pageBuilder.limit = jest.fn(() => pageBuilder);
+    pageBuilder.execute = jest.fn(async () => pageRows);
+
+    const db: any = {
+      // searchSuggestions only touches `pages` here (includePages: true).
+      selectFrom: jest.fn(() => pageBuilder),
+    };
+
+    const pageRepo = {
+      // `.select((eb) => this.pageRepo.withSpace(eb))` — return value is ignored
+      // by our builder stub, so a sentinel is enough.
+      withSpace: jest.fn(() => ({ __withSpace: true })),
+    };
+    const shareRepo = {};
+    const spaceMemberRepo = {
+      getUserSpaceIds: jest.fn().mockResolvedValue(['space-1']),
+    };
+    const pagePermissionRepo = {
+      // Let every found page through page-level permission filtering.
+      filterAccessiblePageIds: jest
+        .fn()
+        .mockImplementation(async ({ pageIds }: { pageIds: string[] }) => pageIds),
+    };
+
+    const service = new SearchService(
+      db as any,
+      pageRepo as any,
+      shareRepo as any,
+      spaceMemberRepo as any,
+      pagePermissionRepo as any,
+    );
+
+    return { service, db, pageBuilder };
+  }
+
+  const isTemplateWhereCall = (pageBuilder: any) =>
+    pageBuilder.where.mock.calls.find((c: any[]) => c[0] === 'isTemplate');
+
+  it('restricts page suggestions to is_template = true when onlyTemplates is set', async () => {
+    const { service, pageBuilder } = makeService([{ id: 'tmpl-1' }]);
+
+    const result = await service.searchSuggestions(
+      { query: 'plan', includePages: true, onlyTemplates: true } as any,
+      'user-1',
+      'ws-1',
+    );
+
+    // The is_template restriction must be applied to the page query.
+    const call = isTemplateWhereCall(pageBuilder);
+    expect(call).toEqual(['isTemplate', '=', true]);
+
+    // Sanity: the (template) page made it through.
+    expect(result.pages.map((p: any) => p.id)).toEqual(['tmpl-1']);
+  });
+
+  it('does NOT restrict to templates when onlyTemplates is absent', async () => {
+    const { service, pageBuilder } = makeService([{ id: 'any-1' }]);
+
+    await service.searchSuggestions(
+      { query: 'plan', includePages: true } as any,
+      'user-1',
+      'ws-1',
+    );
+
+    // No is_template clause should be added for a normal page suggestion search.
+    expect(isTemplateWhereCall(pageBuilder)).toBeUndefined();
+  });
+});