feat(html-embed): per-workspace feature toggle, default OFF
The admin-only raw HTML/JS embed is a deliberate stored-XSS surface, so gate the whole feature behind a workspace toggle that is OFF by default; it only works when a workspace admin explicitly enables it. - settings.htmlEmbed (boolean, default false) + workspace-update field htmlEmbed, persisted via WorkspaceRepo.updateSetting with an audit diff. Flipping it is admin-only (same Manage Settings CASL as other workspace toggles). - New gate htmlEmbedAllowed(featureEnabled, role) = featureEnabled && admin/owner. All 7 server write paths (create, duplicate, collab onStoreDocument, REST/MCP/AI updatePageContent, single + zip import, transclusion unsync) now read the workspace's settings.htmlEmbed and strip unless (toggle ON AND admin). OFF (default, or a failed/empty workspace lookup) strips htmlEmbed for EVERYONE including admins -> existing embeds are cleaned up on next save, none persist. - Client (defense-in-depth): the /html slash item is hidden unless toggle ON + admin; the NodeView executes nothing and shows a 'disabled in this workspace' placeholder when OFF; an admin Switch in Workspace Settings -> General with a description of the behavior. - docs/html-embed-admin.md documents the toggle + admin-only + fail-closed coedit (a non-admin save strips an admin's embed) + execution semantics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,8 @@
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import {
|
||||
canAuthorHtmlEmbed,
|
||||
hasHtmlEmbedNode,
|
||||
htmlEmbedAllowed,
|
||||
stripHtmlEmbedNodes,
|
||||
} from '../../../common/helpers/prosemirror/html-embed.util';
|
||||
|
||||
@@ -29,49 +29,74 @@ const docWithEmbed = () => ({
|
||||
],
|
||||
});
|
||||
|
||||
// The real predicate both paths apply (see SECURITY blocks in page.service.ts).
|
||||
function applyGate(json: any, role: string | null | undefined) {
|
||||
if (!canAuthorHtmlEmbed(role) && hasHtmlEmbedNode(json)) {
|
||||
// The real predicate both paths apply (see SECURITY blocks in page.service.ts):
|
||||
// toggle AND admin.
|
||||
function applyGate(
|
||||
json: any,
|
||||
featureEnabled: boolean,
|
||||
role: string | null | undefined,
|
||||
) {
|
||||
if (!htmlEmbedAllowed(featureEnabled, role) && hasHtmlEmbedNode(json)) {
|
||||
return stripHtmlEmbedNodes(json);
|
||||
}
|
||||
return json;
|
||||
}
|
||||
|
||||
describe('page create/duplicate gate decision (real helpers)', () => {
|
||||
it('non-admin (member) strips', () => {
|
||||
const result = applyGate(docWithEmbed(), 'member');
|
||||
it('toggle ON + non-admin (member) strips', () => {
|
||||
const result = applyGate(docWithEmbed(), true, 'member');
|
||||
expect(hasHtmlEmbedNode(result)).toBe(false);
|
||||
expect(result.content).toHaveLength(1);
|
||||
expect(result.content[0].content[0].text).toBe('body');
|
||||
});
|
||||
|
||||
it('unknown/empty role fails closed (strips)', () => {
|
||||
it('toggle ON + unknown/empty role fails closed (strips)', () => {
|
||||
for (const role of [null, undefined, 'viewer'] as const) {
|
||||
expect(hasHtmlEmbedNode(applyGate(docWithEmbed(), role))).toBe(false);
|
||||
expect(hasHtmlEmbedNode(applyGate(docWithEmbed(), true, role))).toBe(
|
||||
false,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('admin/owner keep', () => {
|
||||
expect(hasHtmlEmbedNode(applyGate(docWithEmbed(), 'admin'))).toBe(true);
|
||||
expect(hasHtmlEmbedNode(applyGate(docWithEmbed(), 'owner'))).toBe(true);
|
||||
it('toggle ON + admin/owner keep', () => {
|
||||
expect(hasHtmlEmbedNode(applyGate(docWithEmbed(), true, 'admin'))).toBe(
|
||||
true,
|
||||
);
|
||||
expect(hasHtmlEmbedNode(applyGate(docWithEmbed(), true, 'owner'))).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it('toggle OFF strips for everyone (admin/owner/member)', () => {
|
||||
for (const role of ['admin', 'owner', 'member'] as const) {
|
||||
expect(hasHtmlEmbedNode(applyGate(docWithEmbed(), false, role))).toBe(
|
||||
false,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
const SRC = readFileSync(join(__dirname, 'page.service.ts'), 'utf-8');
|
||||
|
||||
describe('page create/duplicate gate identity is pinned (source contract)', () => {
|
||||
it('create() gates on the caller role param before deriving content/ydoc', () => {
|
||||
it('create() gates on toggle AND the caller role param before deriving content/ydoc', () => {
|
||||
// create() receives the caller's workspace role as `callerRole` and gates on
|
||||
// it; the embed must be stripped BEFORE insertPage.
|
||||
// the combined toggle-AND-admin predicate; the embed must be stripped BEFORE
|
||||
// insertPage.
|
||||
expect(SRC).toMatch(
|
||||
/!canAuthorHtmlEmbed\(\s*callerRole\s*\)\s*&&\s*hasHtmlEmbedNode\(\s*prosemirrorJson\s*\)/,
|
||||
/!htmlEmbedAllowed\(\s*htmlEmbedEnabled\s*,\s*callerRole\s*\)\s*&&\s*hasHtmlEmbedNode\(\s*prosemirrorJson\s*\)/,
|
||||
);
|
||||
expect(SRC).toContain('prosemirrorJson = stripHtmlEmbedNodes(prosemirrorJson)');
|
||||
});
|
||||
|
||||
it('duplicatePage() gates on the duplicating user role (authUser.role)', () => {
|
||||
it('duplicatePage() gates on toggle AND the duplicating user role (authUser.role)', () => {
|
||||
expect(SRC).toMatch(
|
||||
/!canAuthorHtmlEmbed\(\s*authUser\.role\s*\)\s*&&\s*hasHtmlEmbedNode\(\s*prosemirrorJson\s*\)/,
|
||||
/!htmlEmbedAllowed\(\s*htmlEmbedEnabled\s*,\s*authUser\.role\s*\)\s*&&\s*hasHtmlEmbedNode\(\s*prosemirrorJson\s*\)/,
|
||||
);
|
||||
});
|
||||
|
||||
it('both paths resolve the toggle from the workspace settings', () => {
|
||||
expect(SRC).toContain('isHtmlEmbedFeatureEnabled(');
|
||||
expect(SRC).toContain('this.workspaceRepo.findById(');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -31,10 +31,12 @@ import {
|
||||
removeMarkTypeFromDoc,
|
||||
} from '../../../common/helpers/prosemirror/utils';
|
||||
import {
|
||||
canAuthorHtmlEmbed,
|
||||
hasHtmlEmbedNode,
|
||||
htmlEmbedAllowed,
|
||||
isHtmlEmbedFeatureEnabled,
|
||||
stripHtmlEmbedNodes,
|
||||
} from '../../../common/helpers/prosemirror/html-embed.util';
|
||||
import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo';
|
||||
import {
|
||||
htmlToJson,
|
||||
jsonToNode,
|
||||
@@ -79,6 +81,7 @@ export class PageService {
|
||||
private collaborationGateway: CollaborationGateway,
|
||||
private readonly watcherService: WatcherService,
|
||||
private readonly transclusionService: TransclusionService,
|
||||
private readonly workspaceRepo: WorkspaceRepo,
|
||||
) {}
|
||||
|
||||
async findById(
|
||||
@@ -146,9 +149,18 @@ export class PageService {
|
||||
// <!--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)) {
|
||||
// The gate is toggle-AND-admin: htmlEmbed survives only when the workspace
|
||||
// feature toggle is ON and the caller is an admin/owner. OFF (default) =>
|
||||
// stripped for everyone. Cheap settings read keyed to the workspace.
|
||||
const htmlEmbedEnabled = isHtmlEmbedFeatureEnabled(
|
||||
(await this.workspaceRepo.findById(workspaceId))?.settings,
|
||||
);
|
||||
if (
|
||||
!htmlEmbedAllowed(htmlEmbedEnabled, callerRole) &&
|
||||
hasHtmlEmbedNode(prosemirrorJson)
|
||||
) {
|
||||
this.logger.warn(
|
||||
`Stripping htmlEmbed node(s) from non-admin page creation by user ${userId} (space ${createPageDto.spaceId})`,
|
||||
`Stripping htmlEmbed node(s) from page creation by user ${userId} (space ${createPageDto.spaceId})`,
|
||||
);
|
||||
prosemirrorJson = stripHtmlEmbedNodes(prosemirrorJson);
|
||||
}
|
||||
@@ -614,6 +626,12 @@ export class PageService {
|
||||
|
||||
const attachmentMap = new Map<string, ICopyPageAttachment>();
|
||||
|
||||
// Resolve the htmlEmbed toggle ONCE for the workspace; the per-page gate
|
||||
// below is toggle-AND-admin (OFF default => stripped for everyone).
|
||||
const htmlEmbedEnabled = isHtmlEmbedFeatureEnabled(
|
||||
(await this.workspaceRepo.findById(rootPage.workspaceId))?.settings,
|
||||
);
|
||||
|
||||
const insertablePages: InsertablePage[] = await Promise.all(
|
||||
pages.map(async (page) => {
|
||||
const pageContent = getProsemirrorContent(page.content);
|
||||
@@ -724,11 +742,11 @@ export class PageService {
|
||||
// htmlEmbed node from each duplicated page when the duplicating user is
|
||||
// not an admin, BEFORE computing textContent/ydoc/insert.
|
||||
if (
|
||||
!canAuthorHtmlEmbed(authUser.role) &&
|
||||
!htmlEmbedAllowed(htmlEmbedEnabled, authUser.role) &&
|
||||
hasHtmlEmbedNode(prosemirrorJson)
|
||||
) {
|
||||
this.logger.warn(
|
||||
`Stripping htmlEmbed node(s) from non-admin page duplication by user ${authUser.id} (source page ${page.id})`,
|
||||
`Stripping htmlEmbed node(s) from page duplication by user ${authUser.id} (source page ${page.id})`,
|
||||
);
|
||||
prosemirrorJson = stripHtmlEmbedNodes(prosemirrorJson);
|
||||
}
|
||||
|
||||
@@ -21,7 +21,7 @@ const sourceContentWithEmbed = () => ({
|
||||
],
|
||||
});
|
||||
|
||||
function buildService() {
|
||||
function buildService(featureEnabled = true) {
|
||||
const pageRepo = {
|
||||
findById: jest.fn(async (id: string) => ({
|
||||
id,
|
||||
@@ -44,6 +44,13 @@ function buildService() {
|
||||
validateCanEdit: jest.fn(async () => undefined),
|
||||
validateCanView: jest.fn(async () => undefined),
|
||||
};
|
||||
// Workspace settings read used by the toggle-AND-admin gate.
|
||||
const workspaceRepo = {
|
||||
findById: jest.fn(async () => ({
|
||||
id: WS,
|
||||
settings: { htmlEmbed: featureEnabled },
|
||||
})),
|
||||
};
|
||||
|
||||
const service = new TransclusionService(
|
||||
{} as any, // db (unused on this path)
|
||||
@@ -55,6 +62,7 @@ function buildService() {
|
||||
attachmentRepo as any,
|
||||
storageService as any,
|
||||
pageAccessService as any,
|
||||
workspaceRepo as any,
|
||||
);
|
||||
return service;
|
||||
}
|
||||
@@ -90,8 +98,8 @@ describe('TransclusionService.unsyncReference htmlEmbed admin gate (real code)',
|
||||
}
|
||||
});
|
||||
|
||||
it('admin: returned content keeps the htmlEmbed', async () => {
|
||||
const service = buildService();
|
||||
it('toggle ON + admin: returned content keeps the htmlEmbed', async () => {
|
||||
const service = buildService(true);
|
||||
const { content } = await service.unsyncReference(
|
||||
REF_PAGE,
|
||||
SRC_PAGE,
|
||||
@@ -101,8 +109,8 @@ describe('TransclusionService.unsyncReference htmlEmbed admin gate (real code)',
|
||||
expect(hasHtmlEmbedNode(content)).toBe(true);
|
||||
});
|
||||
|
||||
it('owner: returned content keeps the htmlEmbed', async () => {
|
||||
const service = buildService();
|
||||
it('toggle ON + owner: returned content keeps the htmlEmbed', async () => {
|
||||
const service = buildService(true);
|
||||
const { content } = await service.unsyncReference(
|
||||
REF_PAGE,
|
||||
SRC_PAGE,
|
||||
@@ -111,4 +119,26 @@ describe('TransclusionService.unsyncReference htmlEmbed admin gate (real code)',
|
||||
);
|
||||
expect(hasHtmlEmbedNode(content)).toBe(true);
|
||||
});
|
||||
|
||||
it('toggle OFF + admin: stripped (feature disabled for everyone)', async () => {
|
||||
const service = buildService(false);
|
||||
const { content } = await service.unsyncReference(
|
||||
REF_PAGE,
|
||||
SRC_PAGE,
|
||||
TX_ID,
|
||||
userWithRole('admin'),
|
||||
);
|
||||
expect(hasHtmlEmbedNode(content)).toBe(false);
|
||||
});
|
||||
|
||||
it('toggle OFF + member: stripped', async () => {
|
||||
const service = buildService(false);
|
||||
const { content } = await service.unsyncReference(
|
||||
REF_PAGE,
|
||||
SRC_PAGE,
|
||||
TX_ID,
|
||||
userWithRole('member'),
|
||||
);
|
||||
expect(hasHtmlEmbedNode(content)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -24,10 +24,12 @@ import { TransclusionLookup } from './transclusion.types';
|
||||
import { Page, User } from '@docmost/db/types/entity.types';
|
||||
import { PageAccessService } from '../page-access/page-access.service';
|
||||
import {
|
||||
canAuthorHtmlEmbed,
|
||||
hasHtmlEmbedNode,
|
||||
htmlEmbedAllowed,
|
||||
isHtmlEmbedFeatureEnabled,
|
||||
stripHtmlEmbedNodes,
|
||||
} from '../../../common/helpers/prosemirror/html-embed.util';
|
||||
import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo';
|
||||
|
||||
type ReferencingPageInfo = {
|
||||
id: string;
|
||||
@@ -52,6 +54,7 @@ export class TransclusionService {
|
||||
private readonly attachmentRepo: AttachmentRepo,
|
||||
private readonly storageService: StorageService,
|
||||
private readonly pageAccessService: PageAccessService,
|
||||
private readonly workspaceRepo: WorkspaceRepo,
|
||||
) {}
|
||||
|
||||
async syncPageTransclusions(
|
||||
@@ -528,9 +531,12 @@ export class TransclusionService {
|
||||
// non-admin can never receive an embed payload to re-persist (the collab
|
||||
// strip on the subsequent save is debounced/race-prone and must not be the
|
||||
// only guard). Admin behavior is unchanged.
|
||||
if (!canAuthorHtmlEmbed(user.role) && hasHtmlEmbedNode(content)) {
|
||||
const htmlEmbedEnabled = isHtmlEmbedFeatureEnabled(
|
||||
(await this.workspaceRepo.findById(user.workspaceId))?.settings,
|
||||
);
|
||||
if (!htmlEmbedAllowed(htmlEmbedEnabled, user.role) && hasHtmlEmbedNode(content)) {
|
||||
this.logger.warn(
|
||||
`Stripping htmlEmbed node(s) from non-admin transclusion unsync by user ${user.id} (reference page ${referencePageId}, source page ${sourcePageId})`,
|
||||
`Stripping htmlEmbed node(s) from transclusion unsync by user ${user.id} (reference page ${referencePageId}, source page ${sourcePageId})`,
|
||||
);
|
||||
content = stripHtmlEmbedNodes(content);
|
||||
}
|
||||
|
||||
@@ -53,6 +53,12 @@ export class UpdateWorkspaceDto extends PartialType(CreateWorkspaceDto) {
|
||||
@IsBoolean()
|
||||
aiDictation: boolean;
|
||||
|
||||
// Workspace feature toggle for the admin-only HTML embed feature. Persisted at
|
||||
// settings.htmlEmbed. ABSENT/false => OFF (default).
|
||||
@IsOptional()
|
||||
@IsBoolean()
|
||||
htmlEmbed: boolean;
|
||||
|
||||
@IsOptional()
|
||||
@IsInt()
|
||||
@Min(1)
|
||||
|
||||
@@ -511,6 +511,20 @@ export class WorkspaceService {
|
||||
);
|
||||
}
|
||||
|
||||
if (typeof updateWorkspaceDto.htmlEmbed !== 'undefined') {
|
||||
const prev = settingsBefore?.htmlEmbed ?? false;
|
||||
if (prev !== updateWorkspaceDto.htmlEmbed) {
|
||||
before.htmlEmbed = prev;
|
||||
after.htmlEmbed = updateWorkspaceDto.htmlEmbed;
|
||||
}
|
||||
await this.workspaceRepo.updateSetting(
|
||||
workspaceId,
|
||||
'htmlEmbed',
|
||||
updateWorkspaceDto.htmlEmbed,
|
||||
trx,
|
||||
);
|
||||
}
|
||||
|
||||
delete updateWorkspaceDto.restrictApiToAdmins;
|
||||
delete updateWorkspaceDto.aiSearch;
|
||||
delete updateWorkspaceDto.generativeAi;
|
||||
@@ -519,6 +533,7 @@ export class WorkspaceService {
|
||||
delete updateWorkspaceDto.allowMemberTemplates;
|
||||
delete updateWorkspaceDto.aiChat;
|
||||
delete updateWorkspaceDto.aiDictation;
|
||||
delete updateWorkspaceDto.htmlEmbed;
|
||||
|
||||
await this.workspaceRepo.updateWorkspace(
|
||||
updateWorkspaceDto,
|
||||
|
||||
Reference in New Issue
Block a user