diff --git a/apps/client/src/features/editor/components/html-embed/html-embed-view.tsx b/apps/client/src/features/editor/components/html-embed/html-embed-view.tsx index 7128497b..a46b383a 100644 --- a/apps/client/src/features/editor/components/html-embed/html-embed-view.tsx +++ b/apps/client/src/features/editor/components/html-embed/html-embed-view.tsx @@ -62,6 +62,16 @@ export default function HtmlEmbedView(props: NodeViewProps) { const workspace = useAtomValue(workspaceAtom); const htmlEmbedEnabled = workspace?.settings?.htmlEmbed === true; + // Execution policy split by editor mode: + // - READ-ONLY / public-share view: the SERVER already decided whether to + // include the embed (it strips htmlEmbed from shared content when the + // workspace toggle is OFF). An anonymous viewer has no workspace and thus + // reads `htmlEmbedEnabled` as false, so we must NOT gate execution on it + // here — we execute exactly the `source` the server chose to serve. + // - EDITABLE editor (admin authoring): keep gating on the per-workspace + // toggle so an admin sees the inert placeholder when the feature is OFF. + const shouldExecute = !editor.isEditable || htmlEmbedEnabled; + const contentRef = useRef(null); const [modalOpen, setModalOpen] = useState(false); const [draft, setDraft] = useState(source || ""); @@ -72,12 +82,12 @@ export default function HtmlEmbedView(props: NodeViewProps) { // feature toggle is OFF we clear the container and inject/execute nothing. useEffect(() => { if (!contentRef.current) return; - if (htmlEmbedEnabled) { + if (shouldExecute) { renderRawHtml(contentRef.current, source || ""); } else { contentRef.current.innerHTML = ""; } - }, [source, htmlEmbedEnabled]); + }, [source, shouldExecute]); const openEditor = useCallback(() => { setDraft(source || ""); @@ -116,9 +126,12 @@ export default function HtmlEmbedView(props: NodeViewProps) { )} - {!htmlEmbedEnabled ? ( - // Feature disabled for this workspace: never inject/execute the source. - // Show a neutral placeholder so an existing embed is visibly inert. + {!shouldExecute ? ( + // Feature disabled for this workspace AND we're in the editable editor: + // never inject/execute the source. Show a neutral placeholder so an + // existing embed is visibly inert for the authoring admin. Read-only / + // share viewers never hit this branch (`shouldExecute` is always true + // there) — they execute exactly the source the server chose to serve.
diff --git a/apps/server/src/core/share/share-html-embed.spec.ts b/apps/server/src/core/share/share-html-embed.spec.ts new file mode 100644 index 00000000..1bfeff1c --- /dev/null +++ b/apps/server/src/core/share/share-html-embed.spec.ts @@ -0,0 +1,133 @@ +import { ShareService } from './share.service'; +import { hasHtmlEmbedNode } from '../../common/helpers/prosemirror/html-embed.util'; + +// Exercises the REAL ShareService server-authoritative htmlEmbed kill-switch for +// shared content. An anonymous public-share viewer cannot read the per-workspace +// htmlEmbed toggle, so the SERVER must decide what to serve: when the toggle is +// OFF, htmlEmbed nodes are stripped from the shared doc; when ON they are kept so +// the read-only client executes them. All repos / token service are mocked so the +// real prepareContentForShare logic runs end-to-end via getSharedPage. + +const WS = 'ws-1'; +const PAGE = 'page-1'; + +const pageContentWithEmbed = () => ({ + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'shared body' }] }, + { type: 'htmlEmbed', attrs: { source: '' } }, + ], +}); + +function buildService(opts: { + // undefined => workspaceRepo.findById returns undefined (fail-closed case) + htmlEmbed?: boolean | undefined; + workspaceMissing?: boolean; +}) { + const shareRepo = { findById: jest.fn() }; + + const pageRepo = { + findById: jest.fn(async () => ({ + id: PAGE, + workspaceId: WS, + spaceId: 'space-1', + deletedAt: null, + content: pageContentWithEmbed(), + })), + }; + + const pagePermissionRepo = { + hasRestrictedAncestor: jest.fn(async () => false), + }; + + const tokenService = { + generateAttachmentToken: jest.fn(async () => 'tok'), + }; + + const workspaceRepo = { + findById: jest.fn(async () => + opts.workspaceMissing + ? undefined + : { id: WS, settings: { htmlEmbed: opts.htmlEmbed } }, + ), + }; + + const service = new ShareService( + shareRepo as any, + pageRepo as any, + pagePermissionRepo as any, + {} as any, // db (unused on this path) + tokenService as any, + {} as any, // transclusionService (unused) + workspaceRepo as any, + ); + + // getSharedPage resolves the share via getShareForPage (a raw db query). + // Stub it so we exercise prepareContentForShare deterministically. + jest + .spyOn(service, 'getShareForPage') + .mockResolvedValue({ pageId: PAGE, key: 'k', id: 's1' } as any); + + return { service, workspaceRepo }; +} + +describe('ShareService htmlEmbed server-authoritative kill-switch (real code)', () => { + it('toggle ON: shared content keeps the htmlEmbed (served to anonymous viewer)', async () => { + const { service } = buildService({ htmlEmbed: true }); + const { page } = await service.getSharedPage( + { pageId: PAGE } as any, + WS, + ); + expect(hasHtmlEmbedNode(page.content)).toBe(true); + expect(JSON.stringify(page.content)).toContain('shared body'); + }); + + it('toggle OFF: htmlEmbed stripped from shared content', async () => { + const { service } = buildService({ htmlEmbed: false }); + const { page } = await service.getSharedPage( + { pageId: PAGE } as any, + WS, + ); + expect(hasHtmlEmbedNode(page.content)).toBe(false); + // Non-embed content is preserved. + expect(JSON.stringify(page.content)).toContain('shared body'); + }); + + it('toggle ABSENT: defaults OFF and strips', async () => { + const { service } = buildService({ htmlEmbed: undefined }); + const { page } = await service.getSharedPage( + { pageId: PAGE } as any, + WS, + ); + expect(hasHtmlEmbedNode(page.content)).toBe(false); + }); + + it('workspace missing: fails closed (stripped)', async () => { + const { service } = buildService({ workspaceMissing: true }); + const { page } = await service.getSharedPage( + { pageId: PAGE } as any, + WS, + ); + expect(hasHtmlEmbedNode(page.content)).toBe(false); + }); + + it('updatePublicAttachments strips htmlEmbed when toggle OFF', async () => { + const { service } = buildService({ htmlEmbed: false }); + const out = await service.updatePublicAttachments({ + id: PAGE, + workspaceId: WS, + content: pageContentWithEmbed(), + } as any); + expect(hasHtmlEmbedNode(out)).toBe(false); + }); + + it('updatePublicAttachments keeps htmlEmbed when toggle ON', async () => { + const { service } = buildService({ htmlEmbed: true }); + const out = await service.updatePublicAttachments({ + id: PAGE, + workspaceId: WS, + content: pageContentWithEmbed(), + } as any); + expect(hasHtmlEmbedNode(out)).toBe(true); + }); +}); diff --git a/apps/server/src/core/share/share.service.ts b/apps/server/src/core/share/share.service.ts index 03ee3155..846cc96a 100644 --- a/apps/server/src/core/share/share.service.ts +++ b/apps/server/src/core/share/share.service.ts @@ -26,6 +26,11 @@ import { validate as isValidUUID } from 'uuid'; import { sql } from 'kysely'; import { TransclusionService } from '../page/transclusion/transclusion.service'; import { TransclusionLookup } from '../page/transclusion/transclusion.types'; +import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo'; +import { + isHtmlEmbedFeatureEnabled, + stripHtmlEmbedNodes, +} from '../../common/helpers/prosemirror/html-embed.util'; @Injectable() export class ShareService { @@ -38,8 +43,22 @@ export class ShareService { @InjectKysely() private readonly db: KyselyDB, private readonly tokenService: TokenService, private readonly transclusionService: TransclusionService, + private readonly workspaceRepo: WorkspaceRepo, ) {} + /** + * Resolve whether the htmlEmbed feature toggle is ON for a workspace. + * Fail-closed: a missing workspace (or absent/non-true setting) => OFF, so + * share content gets the embed stripped when we can't positively confirm the + * feature is enabled. + */ + private async isHtmlEmbedEnabledForWorkspace( + workspaceId: string, + ): Promise { + const workspace = await this.workspaceRepo.findById(workspaceId); + return isHtmlEmbedFeatureEnabled(workspace?.settings); + } + async getShareTree(shareId: string, workspaceId: string) { const share = await this.shareRepo.findById(shareId); if (!share || share.workspaceId !== workspaceId) { @@ -360,6 +379,11 @@ export class ShareService { workspaceId, ); + // Resolve the workspace htmlEmbed toggle once for this share request; all + // transcluded items belong to the same workspace as the host share. + const htmlEmbedEnabled = + await this.isHtmlEmbedEnabledForWorkspace(workspaceId); + // Sanitize each item's content for public delivery // generate per-attachment tokens scoped to the source page // and strip comment marks. @@ -370,6 +394,7 @@ export class ShareService { item.content, item.sourcePageId, workspaceId, + htmlEmbedEnabled, ); return { ...item, content: doc?.toJSON() ?? item.content }; }), @@ -417,10 +442,14 @@ export class ShareService { } async updatePublicAttachments(page: Page): Promise { + const htmlEmbedEnabled = await this.isHtmlEmbedEnabledForWorkspace( + page.workspaceId, + ); const doc = await this.prepareContentForShare( page.content, page.id, page.workspaceId, + htmlEmbedEnabled, ); return doc?.toJSON() ?? page.content; } @@ -441,6 +470,13 @@ export class ShareService { * not leak structure (existence, location, count, resolved state, or * comment ids) to public viewers. * + * 3. Strip `htmlEmbed` nodes when the workspace feature toggle is OFF. This + * makes the toggle a SERVER-AUTHORITATIVE kill-switch for shared content: + * when OFF the embed is never served to the anonymous viewer (who can't + * read the per-workspace toggle), when ON the embed is served so the + * read-only client executes it. `htmlEmbedEnabled` is resolved fail-closed + * by the callers (missing workspace => OFF => strip). + * * Both share-content paths — the host page (`updatePublicAttachments`) and * the share-scoped transclusion lookup (`lookupTransclusionForShare`) — * call into this single helper so the two paths can never drift on @@ -450,8 +486,16 @@ export class ShareService { content: unknown, attachmentOwnerPageId: string, workspaceId: string, + htmlEmbedEnabled: boolean, ): Promise { - const pmJson = getProsemirrorContent(content); + let pmJson = getProsemirrorContent(content); + + // Kill-switch: when the workspace toggle is OFF, never serve htmlEmbed + // nodes to public viewers. Strip before tokenizing/serializing. + if (!htmlEmbedEnabled) { + pmJson = stripHtmlEmbedNodes(pmJson); + } + const attachmentIds = getAttachmentIds(pmJson); const tokenMap = new Map();