diff --git a/apps/server/src/core/ai-chat/public-share-chat.controller.ts b/apps/server/src/core/ai-chat/public-share-chat.controller.ts index d142215c..6ed6691b 100644 --- a/apps/server/src/core/ai-chat/public-share-chat.controller.ts +++ b/apps/server/src/core/ai-chat/public-share-chat.controller.ts @@ -19,6 +19,8 @@ import { AuthWorkspace } from '../../common/decorators/auth-workspace.decorator' import { SkipTransform } from '../../common/decorators/skip-transform.decorator'; import { PUBLIC_SHARE_AI_THROTTLER } from '../../integrations/throttle/throttler-names'; import { ShareService } from '../share/share.service'; +import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo'; +import { PageRepo } from '@docmost/db/repos/page/page.repo'; import { AiSettingsService } from '../../integrations/ai/ai-settings.service'; import { AiNotConfiguredException } from '../../integrations/ai/ai-not-configured.exception'; import { @@ -50,6 +52,8 @@ export class PublicShareChatController { constructor( private readonly shareService: ShareService, + private readonly pagePermissionRepo: PagePermissionRepo, + private readonly pageRepo: PageRepo, private readonly aiSettings: AiSettingsService, private readonly publicShareChat: PublicShareChatService, ) {} @@ -78,18 +82,26 @@ export class PublicShareChatController { ); // 2. Share usable? Resolved via the page's share membership, since the page - // resolution (getShareForPage) ALSO yields the share + workspace + - // restricted checks. We still need basic input to attempt it. + // resolution (getShareForPage) ALSO yields the share + workspace. We + // still need basic input to attempt it. // 3. Page in share? The same getShareForPage lookup confirms the opened page - // actually resolves to THIS share tree (shareUsable + pageInShare are set - // together below; the funnel grades them as distinct ordered steps). + // resolves to THIS share tree, PLUS an explicit restricted-ancestor gate + // (getShareForPage itself does NOT exclude restricted descendants) so a + // restricted page hidden from the public view is graded not-in-share. + // (shareUsable + pageInShare are set together below; the funnel grades + // them as distinct ordered steps.) let share: Awaited> | undefined; let shareUsable = false; let pageInShare = false; if (assistantEnabled && shareId && pageId) { // getShareForPage walks up the tree to the nearest ancestor share, // enforces share.workspaceId === workspaceId and includeSubPages, and - // returns undefined when the page is not publicly reachable. + // returns undefined when the page is not publicly reachable. NOTE: it + // joins only the `shares` table — it does NOT exclude restricted + // descendants — so a restricted page inside an includeSubPages share + // still resolves here. We add an explicit restricted-ancestor gate below + // (same as the public view) so the opened page's title never leaks into + // the system prompt for a page the public view 404s. share = await this.shareService.getShareForPage(pageId, workspace.id); if (share && share.id === shareId) { // Confirm sharing is still allowed for the share's space (and not @@ -99,7 +111,20 @@ export class PublicShareChatController { share.spaceId, ); shareUsable = sharingAllowed; - pageInShare = sharingAllowed; + // A restricted descendant is hidden from the public share view; treat + // the opened page as not-in-share so the funnel returns the SAME 404 it + // returns for an out-of-tree page (uniform, no existence leak). + // hasRestrictedAncestor matches on the page UUID only, while the + // opened pageId may be a slugId, so resolve to the UUID first (cheap + // base-fields lookup, mirroring how getSharedPage resolves the page + // before its restricted check). + const openedPageRow = await this.pageRepo.findById(pageId); + const restricted = openedPageRow + ? await this.pagePermissionRepo.hasRestrictedAncestor( + openedPageRow.id, + ) + : true; // unresolvable opened page => fail closed (treat as not-in-share) + pageInShare = sharingAllowed && !restricted; } } diff --git a/apps/server/src/core/ai-chat/public-share-chat.spec.ts b/apps/server/src/core/ai-chat/public-share-chat.spec.ts index 44cf9d17..c6699591 100644 --- a/apps/server/src/core/ai-chat/public-share-chat.spec.ts +++ b/apps/server/src/core/ai-chat/public-share-chat.spec.ts @@ -67,6 +67,65 @@ describe('evaluateShareAssistantFunnel ordering', () => { }); }); +describe('controller funnel: restricted opened page is graded not-in-share', () => { + /** + * Mirrors the controller's pageInShare decision for the opened page: + * pageInShare = sharingAllowed && !hasRestrictedAncestor(resolvedPageId) + * A restricted descendant inside an includeSubPages share resolves via + * getShareForPage but must be graded not-in-share so the funnel returns the + * SAME 404 it returns for an out-of-tree page (uniform, no existence leak). + */ + function decidePageInShare( + sharingAllowed: boolean, + restricted: boolean, + ): boolean { + return sharingAllowed && !restricted; + } + + it('a restricted descendant funnels to the SAME 404 as an out-of-tree page', () => { + // Out-of-tree page: getShareForPage returns a different/no share => the + // controller never sets pageInShare (stays false). + const outOfTree = evaluateShareAssistantFunnel({ + assistantEnabled: true, + shareUsable: true, + pageInShare: false, + providerConfigured: true, + }); + + // Restricted descendant: share resolves, sharing allowed, but the explicit + // restricted-ancestor gate flips pageInShare to false. + const restrictedPageInShare = decidePageInShare(true, /* restricted */ true); + const restricted = evaluateShareAssistantFunnel({ + assistantEnabled: true, + shareUsable: true, + pageInShare: restrictedPageInShare, + providerConfigured: true, + }); + + expect(restrictedPageInShare).toBe(false); + // Same outcome, same reason, same status: indistinguishable. + expect(restricted).toEqual(outOfTree); + expect(restricted).toEqual({ + ok: false, + status: 404, + reason: 'page-not-in-share', + }); + }); + + it('an unrestricted page inside the share is allowed through the funnel', () => { + const pageInShare = decidePageInShare(true, /* restricted */ false); + expect(pageInShare).toBe(true); + expect( + evaluateShareAssistantFunnel({ + assistantEnabled: true, + shareUsable: true, + pageInShare, + providerConfigured: true, + }), + ).toEqual({ ok: true }); + }); +}); + describe('buildShareSystemPrompt locking', () => { it('always includes the immutable read-only / share-scope safety rules', () => { const prompt = buildShareSystemPrompt({ share: null, openedPage: null }); @@ -173,10 +232,12 @@ describe('PublicShareChatToolsService share scoping', () => { updatePublicAttachments: jest.fn(), }; const pageRepo = { findById: jest.fn() }; + const pagePermissionRepo = { hasRestrictedAncestor: jest.fn() }; const svc = new PublicShareChatToolsService( shareService as never, {} as never, pageRepo as never, + pagePermissionRepo as never, ); const tools = svc.forShare('THIS-SHARE', 'ws-1'); @@ -190,6 +251,50 @@ describe('PublicShareChatToolsService share scoping', () => { // It must NOT have fetched/returned any content for an out-of-share page. expect(pageRepo.findById).not.toHaveBeenCalled(); expect(shareService.updatePublicAttachments).not.toHaveBeenCalled(); + // The restricted check is never even reached for an out-of-share page. + expect(pagePermissionRepo.hasRestrictedAncestor).not.toHaveBeenCalled(); + }); + + it('getSharePage BLOCKS a restricted descendant inside THIS share with the SAME generic error (content leak fix)', async () => { + const shareService = { + // The restricted page DOES resolve to this share (includeSubPages tree)... + getShareForPage: jest.fn().mockResolvedValue({ id: 'THIS-SHARE' }), + updatePublicAttachments: jest.fn(), + }; + // ...and the page itself exists and is not deleted. + const pageRepo = { + findById: jest + .fn() + .mockResolvedValue({ id: 'p-restricted', title: 'Secret', content: {} }), + }; + // ...but it has a restricted ancestor (its own page_permissions row), so the + // public view 404s it — the tool must NOT return its content. + const pagePermissionRepo = { + hasRestrictedAncestor: jest + .fn() + .mockImplementation(async (id: string) => id === 'p-restricted'), + }; + const svc = new PublicShareChatToolsService( + shareService as never, + {} as never, + pageRepo as never, + pagePermissionRepo as never, + ); + + const tools = svc.forShare('THIS-SHARE', 'ws-1'); + const getSharePage = tools.getSharePage as { + execute: (args: { pageId: string }) => Promise; + }; + + await expect( + getSharePage.execute({ pageId: 'p-restricted' }), + ).rejects.toThrow(/not part of this published share/i); + // The restricted check ran on the resolved page id... + expect(pagePermissionRepo.hasRestrictedAncestor).toHaveBeenCalledWith( + 'p-restricted', + ); + // ...and no content was ever sanitized/returned. + expect(shareService.updatePublicAttachments).not.toHaveBeenCalled(); }); it('searchSharePages forwards the share scope (shareId, no spaceId/userId) to the FTS branch', async () => { @@ -202,6 +307,7 @@ describe('PublicShareChatToolsService share scoping', () => { {} as never, searchService as never, {} as never, + {} as never, ); const tools = svc.forShare('THIS-SHARE', 'ws-1'); const searchSharePages = tools.searchSharePages as { diff --git a/apps/server/src/core/ai-chat/tools/public-share-chat-tools.service.ts b/apps/server/src/core/ai-chat/tools/public-share-chat-tools.service.ts index 3d2284d8..b15a6f69 100644 --- a/apps/server/src/core/ai-chat/tools/public-share-chat-tools.service.ts +++ b/apps/server/src/core/ai-chat/tools/public-share-chat-tools.service.ts @@ -4,6 +4,7 @@ import { z } from 'zod'; import { ShareService } from '../../share/share.service'; import { SearchService } from '../../search/search.service'; import { PageRepo } from '@docmost/db/repos/page/page.repo'; +import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo'; import { jsonToMarkdown } from '../../../collaboration/collaboration.util'; /** @@ -22,7 +23,9 @@ import { jsonToMarkdown } from '../../../collaboration/collaboration.util'; * (`shareId && !spaceId && !userId`), which itself restricts results to the * share's pages and excludes restricted descendants; * - reading a page first confirms, via `getShareForPage`, that the page - * resolves to THIS share before returning any content. + * resolves to THIS share AND (because getShareForPage does NOT itself + * exclude restricted descendants) that the page has no restricted ancestor, + * before returning any content. */ @Injectable() export class PublicShareChatToolsService { @@ -32,6 +35,7 @@ export class PublicShareChatToolsService { private readonly shareService: ShareService, private readonly searchService: SearchService, private readonly pageRepo: PageRepo, + private readonly pagePermissionRepo: PagePermissionRepo, ) {} /** @@ -96,7 +100,10 @@ export class PublicShareChatToolsService { throw new Error('A pageId is required.'); } // Confirm the page resolves to THIS share (recursive CTE up the tree, - // honouring includeSubPages + restricted exclusion + workspace check). + // honouring includeSubPages + workspace check). NOTE: getShareForPage + // joins only the `shares` table — it does NOT exclude restricted + // descendants — so membership alone is not sufficient (see the + // explicit restricted check below, which the public view also does). // Not in this share => tool error WITHOUT leaking whether the page // exists at all. const share = await this.shareService.getShareForPage( @@ -114,6 +121,17 @@ export class PublicShareChatToolsService { throw new Error('That page is not part of this published share.'); } + // A restricted descendant (a page with its own page_permissions / + // pageAccess row) is hidden from the public share view even when it + // sits inside an includeSubPages share. getShareForPage does NOT + // exclude it, so we must replicate the public view's restricted- + // ancestor gate here (ShareService.getSharedPage). Use the SAME + // generic message as an out-of-share page so the model cannot + // distinguish "restricted" from "not in share" (no info leak). + if (await this.pagePermissionRepo.hasRestrictedAncestor(page.id)) { + throw new Error('That page is not part of this published share.'); + } + // Reuse the public share-content sanitizer: strips comment marks and // tokenizes attachments for public delivery, exactly as the public // shared-page view does.