fix(html-embed): strip htmlEmbed on the plain page-create path too
Release-cycle red-team found the admin-only gate missed PageService.create(): content/textContent/ydoc were derived and persisted without the strip, so any space member could POST /pages/create with an htmlEmbed node (incl. the markdown/html <!--html-embed:BASE64--> form) and store executing JS for every reader. Add the same gate used by duplicatePage: strip htmlEmbed when the caller is not a workspace admin/owner. Role is plumbed from the controller (user.role); unknown role => non-admin (strip). All four create paths (create, duplicate, single import, zip import) plus the update paths are now guarded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
// <!--html-embed:BASE64--> 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: '<script>alert(1)</script>' } },
|
||||
],
|
||||
};
|
||||
|
||||
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 <!--html-embed--> 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 = '<script>steal()</script>';
|
||||
const encoded = encodeHtmlEmbedSource(source);
|
||||
const html = `<div data-type="htmlEmbed" data-source="${encoded}"></div>`;
|
||||
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 = '<script>console.log("héllo → 世界")</script>';
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
|
||||
|
||||
@@ -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
|
||||
// <!--html-embed:BASE64--> 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);
|
||||
|
||||
Reference in New Issue
Block a user