fix(html-embed): execute embeds on public shares; toggle is server-side kill switch

The html-embed feature toggle was enforced CLIENT-side in the NodeView (reads
settings.htmlEmbed from the logged-in workspace), so an anonymous public-share
viewer — who has no workspace context — always saw it as OFF and got a
placeholder instead of the executing embed. That broke the whole point (a
tracker must run for anonymous visitors).

Make it server-authoritative:
- share.service prepareContentForShare (the single path both share-content
  flows use) strips htmlEmbed from served content when the workspace toggle is
  OFF; both callers (updatePublicAttachments host page + lookupTransclusionForShare)
  resolve the toggle once and pass it. Fail-closed: missing workspace -> OFF ->
  stripped.
- NodeView executes whatever it was served in read-only/share mode
  (shouldExecute = !editor.isEditable || htmlEmbedEnabled); the disabled
  placeholder now only shows in the editable editor when OFF.

Net: anonymous share + toggle ON -> server serves the (admin-authored) embed ->
it executes for everyone; toggle OFF -> stripped server-side from every
share-content path (true kill switch); a non-admin embed can never be served
(save-path strip). No XSS regression in the editable editor.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-20 19:49:57 +03:00
parent 8fcce6a674
commit 41f3944e79
3 changed files with 196 additions and 6 deletions

View File

@@ -62,6 +62,16 @@ export default function HtmlEmbedView(props: NodeViewProps) {
const workspace = useAtomValue(workspaceAtom); const workspace = useAtomValue(workspaceAtom);
const htmlEmbedEnabled = workspace?.settings?.htmlEmbed === true; 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<HTMLDivElement | null>(null); const contentRef = useRef<HTMLDivElement | null>(null);
const [modalOpen, setModalOpen] = useState(false); const [modalOpen, setModalOpen] = useState(false);
const [draft, setDraft] = useState<string>(source || ""); const [draft, setDraft] = useState<string>(source || "");
@@ -72,12 +82,12 @@ export default function HtmlEmbedView(props: NodeViewProps) {
// feature toggle is OFF we clear the container and inject/execute nothing. // feature toggle is OFF we clear the container and inject/execute nothing.
useEffect(() => { useEffect(() => {
if (!contentRef.current) return; if (!contentRef.current) return;
if (htmlEmbedEnabled) { if (shouldExecute) {
renderRawHtml(contentRef.current, source || ""); renderRawHtml(contentRef.current, source || "");
} else { } else {
contentRef.current.innerHTML = ""; contentRef.current.innerHTML = "";
} }
}, [source, htmlEmbedEnabled]); }, [source, shouldExecute]);
const openEditor = useCallback(() => { const openEditor = useCallback(() => {
setDraft(source || ""); setDraft(source || "");
@@ -116,9 +126,12 @@ export default function HtmlEmbedView(props: NodeViewProps) {
</div> </div>
)} )}
{!htmlEmbedEnabled ? ( {!shouldExecute ? (
// Feature disabled for this workspace: never inject/execute the source. // Feature disabled for this workspace AND we're in the editable editor:
// Show a neutral placeholder so an existing embed is visibly inert. // 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.
<div className={classes.htmlEmbedPlaceholder}> <div className={classes.htmlEmbedPlaceholder}>
<IconCode size={18} /> <IconCode size={18} />
<Text size="sm"> <Text size="sm">

View File

@@ -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: '<script>track()</script>' } },
],
});
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);
});
});

View File

@@ -26,6 +26,11 @@ import { validate as isValidUUID } from 'uuid';
import { sql } from 'kysely'; import { sql } from 'kysely';
import { TransclusionService } from '../page/transclusion/transclusion.service'; import { TransclusionService } from '../page/transclusion/transclusion.service';
import { TransclusionLookup } from '../page/transclusion/transclusion.types'; 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() @Injectable()
export class ShareService { export class ShareService {
@@ -38,8 +43,22 @@ export class ShareService {
@InjectKysely() private readonly db: KyselyDB, @InjectKysely() private readonly db: KyselyDB,
private readonly tokenService: TokenService, private readonly tokenService: TokenService,
private readonly transclusionService: TransclusionService, 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<boolean> {
const workspace = await this.workspaceRepo.findById(workspaceId);
return isHtmlEmbedFeatureEnabled(workspace?.settings);
}
async getShareTree(shareId: string, workspaceId: string) { async getShareTree(shareId: string, workspaceId: string) {
const share = await this.shareRepo.findById(shareId); const share = await this.shareRepo.findById(shareId);
if (!share || share.workspaceId !== workspaceId) { if (!share || share.workspaceId !== workspaceId) {
@@ -360,6 +379,11 @@ export class ShareService {
workspaceId, 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 // Sanitize each item's content for public delivery
// generate per-attachment tokens scoped to the source page // generate per-attachment tokens scoped to the source page
// and strip comment marks. // and strip comment marks.
@@ -370,6 +394,7 @@ export class ShareService {
item.content, item.content,
item.sourcePageId, item.sourcePageId,
workspaceId, workspaceId,
htmlEmbedEnabled,
); );
return { ...item, content: doc?.toJSON() ?? item.content }; return { ...item, content: doc?.toJSON() ?? item.content };
}), }),
@@ -417,10 +442,14 @@ export class ShareService {
} }
async updatePublicAttachments(page: Page): Promise<any> { async updatePublicAttachments(page: Page): Promise<any> {
const htmlEmbedEnabled = await this.isHtmlEmbedEnabledForWorkspace(
page.workspaceId,
);
const doc = await this.prepareContentForShare( const doc = await this.prepareContentForShare(
page.content, page.content,
page.id, page.id,
page.workspaceId, page.workspaceId,
htmlEmbedEnabled,
); );
return doc?.toJSON() ?? page.content; return doc?.toJSON() ?? page.content;
} }
@@ -441,6 +470,13 @@ export class ShareService {
* not leak structure (existence, location, count, resolved state, or * not leak structure (existence, location, count, resolved state, or
* comment ids) to public viewers. * 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 * Both share-content paths — the host page (`updatePublicAttachments`) and
* the share-scoped transclusion lookup (`lookupTransclusionForShare`) — * the share-scoped transclusion lookup (`lookupTransclusionForShare`) —
* call into this single helper so the two paths can never drift on * call into this single helper so the two paths can never drift on
@@ -450,8 +486,16 @@ export class ShareService {
content: unknown, content: unknown,
attachmentOwnerPageId: string, attachmentOwnerPageId: string,
workspaceId: string, workspaceId: string,
htmlEmbedEnabled: boolean,
): Promise<Node | null> { ): Promise<Node | null> {
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 attachmentIds = getAttachmentIds(pmJson);
const tokenMap = new Map<string, string>(); const tokenMap = new Map<string, string>();