diff --git a/apps/server/src/common/helpers/prosemirror/html-embed.spec.ts b/apps/server/src/common/helpers/prosemirror/html-embed.spec.ts index 1a044a4e..ec215c87 100644 --- a/apps/server/src/common/helpers/prosemirror/html-embed.spec.ts +++ b/apps/server/src/common/helpers/prosemirror/html-embed.spec.ts @@ -106,7 +106,8 @@ describe('canAuthorHtmlEmbed', () => { }); // Replicates the write-path decision used by every non-admin persistence guard -// (collab store, single import, zip import, duplication, transclusion unsync): +// (page create, collab store, single import, zip import, duplication, +// transclusion unsync): // if !canAuthorHtmlEmbed(role) && hasHtmlEmbedNode(json) -> strip, else keep. const applyAdminGate = (json: any, role: string | null | undefined) => { if (!canAuthorHtmlEmbed(role) && hasHtmlEmbedNode(json)) { @@ -187,6 +188,55 @@ describe('admin-gate write-path decision (duplication / import / unsync)', () => }); }); +// PageService.create() now applies exactly `applyAdminGate` (above) to the +// parsed ProseMirror JSON BEFORE deriving content/textContent/ydoc and calling +// insertPage. The create controller only requires space Edit, so without this a +// regular member could POST a doc (json, or the markdown/html +// forms that parse to the same node) containing an +// htmlEmbed and store XSS for every reader. These cases pin the create-path +// decision: non-admin -> stripped, admin/owner -> kept. +describe('admin-gate write-path decision (page create)', () => { + const docWithEmbed = { + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'body' }] }, + { type: 'htmlEmbed', attrs: { source: '' } }, + ], + }; + + it('strips the embed when a non-admin (member) creates the page', () => { + const result = applyAdminGate(docWithEmbed, 'member'); + expect(hasHtmlEmbedNode(result)).toBe(false); + expect(result.content).toHaveLength(1); + expect(result.content[0].content[0].text).toBe('body'); + }); + + it('strips the embed when the creator role is unknown/empty', () => { + expect(hasHtmlEmbedNode(applyAdminGate(docWithEmbed, null))).toBe(false); + expect(hasHtmlEmbedNode(applyAdminGate(docWithEmbed, undefined))).toBe( + false, + ); + }); + + it('keeps the embed when an admin or owner creates the page', () => { + expect(hasHtmlEmbedNode(applyAdminGate(docWithEmbed, 'admin'))).toBe(true); + expect(hasHtmlEmbedNode(applyAdminGate(docWithEmbed, 'owner'))).toBe(true); + }); + + it('strips embeds smuggled via the markdown/html form', () => { + // The markdown/html create formats decode to the same htmlEmbed node, so + // the gate (run on the parsed JSON) covers them identically. + const source = ''; + const encoded = encodeHtmlEmbedSource(source); + const html = `
`; + const parsed = htmlToJson(html); + expect(hasHtmlEmbedNode(parsed)).toBe(true); + + const result = applyAdminGate(parsed, 'member'); + expect(hasHtmlEmbedNode(result)).toBe(false); + }); +}); + describe('htmlEmbed source base64 codec', () => { it('round-trips arbitrary source including UTF-8', () => { const source = ''; diff --git a/apps/server/src/core/page/page.controller.ts b/apps/server/src/core/page/page.controller.ts index 1f5163bd..29365d66 100644 --- a/apps/server/src/core/page/page.controller.ts +++ b/apps/server/src/core/page/page.controller.ts @@ -237,6 +237,9 @@ export class PageController { user.id, workspace.id, createPageDto, + // Pass the caller's workspace role so create() can enforce the htmlEmbed + // admin gate (non-admins cannot author raw-JS embeds). + user.role, provenance, ); diff --git a/apps/server/src/core/page/services/page.service.ts b/apps/server/src/core/page/services/page.service.ts index adc8b746..baf5506a 100644 --- a/apps/server/src/core/page/services/page.service.ts +++ b/apps/server/src/core/page/services/page.service.ts @@ -98,6 +98,10 @@ export class PageService { userId: string, workspaceId: string, createPageDto: CreatePageDto, + // Workspace role of the caller. Used to enforce the htmlEmbed admin gate on + // the create write path (see below). Optional/typed loosely so unknown or + // missing roles fall through to the non-admin (strip) branch by default. + callerRole?: string | null, // Optional agent-edit provenance (from the signed access claim). When the // actor is 'agent', stamp the page's source marker so a freshly created page // shows it was created by the AI agent (§14 N2) — create goes through REST, @@ -128,11 +132,27 @@ export class PageService { let ydoc = undefined; if (createPageDto?.content && createPageDto?.format) { - const prosemirrorJson = await this.parseProsemirrorContent( + let prosemirrorJson = await this.parseProsemirrorContent( createPageDto.content, createPageDto.format, ); + // SECURITY (Variant C admin gate, plain page-create write path): + // create() builds content/textContent/ydoc directly and persists them via + // insertPage, bypassing the collab onStoreDocument strip. htmlEmbed renders + // raw, unsanitized JS in readers' browsers, so only workspace admins/owners + // may author it. The create controller requires only space Edit, so a + // regular member could otherwise POST a doc (json, or the markdown/html + // forms that parse to the same node) containing an + // htmlEmbed and store XSS for every reader. Strip every htmlEmbed node when + // the caller is not an admin, BEFORE deriving textContent/ydoc/insert. + if (!canAuthorHtmlEmbed(callerRole) && hasHtmlEmbedNode(prosemirrorJson)) { + this.logger.warn( + `Stripping htmlEmbed node(s) from non-admin page creation by user ${userId} (space ${createPageDto.spaceId})`, + ); + prosemirrorJson = stripHtmlEmbedNodes(prosemirrorJson); + } + content = prosemirrorJson; textContent = jsonToText(prosemirrorJson); ydoc = createYdocFromJson(prosemirrorJson);