A git push to a page with an OPEN editor was silently reverted: the git
commit landed and the DB body updated, but the page in the browser stayed
on the old content and the editor's next autosave overwrote the git change.
Root cause (distributed, not in the merge): writeBody applied the body
merge via collabGateway.openDirectConnection on whichever instance/process
runs git-sync (the api/worker). When an editor is connected to a DIFFERENT
collab instance/process, that opens a SEPARATE, detached Y.Doc. The merge
landed in the detached doc + DB, but the live editor's Y.Doc never received
the Yjs update; its debounced autosave then persisted its STALE state over
the DB, reverting the git change (and, for concurrent edits to different
paragraphs, losing the git side). In one process the bug is invisible
because the direct connection already shares the editor's doc.
Fix: route the body write through the existing custom-event channel (the
same mechanism comment-marks and updatePageContent use) so the merge runs
on the instance that OWNS the live doc. Its update is then broadcast to
every connection (Document.handleUpdate) and the editor's CRDT converges on
the merged result. New CollaborationGateway.writePageBody dispatches to a
new gitSyncWriteBody handler (builds incoming/base docs before opening the
connection — crash-safe — then 3-way/2-way merges into the live fragment);
without redis it runs locally on the single (owning) instance. writeBody
now just forwards the converted ProseMirror bodies + service userId.
Evidence:
- git-ingest-convergence.spec.ts: deterministic two-Y.Doc repro. PATH B
(undelivered update) asserts the LOSS (the bug); PATH A (update delivered,
as the owner-routed write does) asserts the git change SURVIVES and that
concurrent edits to different paragraphs both survive.
- collaboration.handler.git-sync.spec.ts: exercises the real gitSyncWriteBody
against a shared doc wired to a connected "editor" doc (models the
owning-instance broadcast) — editor converges, concurrent edit preserved,
crash-safe on transform failure.
- gitmost-datasource.service.spec.ts: writeBody now routes via writePageBody
(RED before this change — it called openDirectConnection).
Honest scope: the failure is cross-instance; full multi-instance convergence
needs a live Hocuspocus + redis and is not provable in a unit test, so the
convergence invariant is captured at the Yjs update-exchange level.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert the htmlEmbed node from same-origin raw-HTML execution to a sandboxed
iframe (sandbox="allow-scripts allow-popups allow-forms", no allow-same-origin,
srcdoc) with postMessage auto-resize (validated by event.source) and an optional
manual height attr. The block now runs in an opaque origin and cannot reach the
viewer's cookies/session/API, so it is safe for any member.
Because the block is now harmless, remove the entire admin/role gating apparatus:
drop htmlEmbedAllowed/canAuthorHtmlEmbed/stripDisallowedHtmlEmbedNodes/
collectHtmlEmbedSources and every role-based strip on the write paths (collab
REST/MCP + socket, page create/duplicate, import x2, transclusion unsync), along
with the now-unused WorkspaceRepo/UserRepo injections and the PageService.create
callerRole param. Keep one strip: prepareContentForShare still removes htmlEmbed
on the anonymous public-share read path when the workspace master toggle is OFF.
The workspace settings.htmlEmbed toggle is now a plain feature switch (gates the
slash-menu and share rendering); when ON the block is available to all members.
Add settings.trackerHead: an admin-only raw HTML/JS analytics snippet injected
verbatim into the <head> of public share pages only (ShareSeoController), for
trackers that genuinely need same-origin. Admin-gated via the existing CASL
Manage/Settings ability; never injected into the authenticated app shell.
Closes security-review findings #1, #2, #4, #5, #10 (and #3 as a security issue).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
Adds an htmlEmbed block node that renders and executes raw HTML/CSS/JS in the
wiki origin (e.g. an analytics tracker) — the owner-chosen variant C. Because
this is stored-XSS by design, only workspace admins/owners may get such a node
persisted; everyone executes it when reading.
- Node (editor-ext): htmlEmbed atom/isolating block; source stored base64 in
data-source for lossless HTML<->JSON round-trip. renderHTML emits only the
encoded marker (never inlines raw markup), so generateHTML/export/search are
not themselves injection vectors. Registered in BOTH client extensions and
server tiptapExtensions. Markdown round-trip via an <!--html-embed:b64-->
comment (turndown) + a marked rule.
- Client NodeView: injects source and re-creates <script> elements so they
actually run; edit modal; renders in read-only/share too. Slash item is
admin-gated (adminOnly filtered by the user's workspace role).
- SERVER ENFORCEMENT (the real control — UI gating alone is insufficient):
stripHtmlEmbedNodes() removes htmlEmbed from any document persisted by a
non-admin, applied at every write path that introduces content from an
untrusted author: collab onStoreDocument, REST/MCP/AI updatePageContent,
single-file import, zip/multi-file import, page duplication, and transclusion
unsync. Page restore introduces no new content. Public share/readonly viewers
render fetched (already-stripped) content and do NOT open a collab socket, so
the only residual is a transient broadcast window to concurrent authenticated
editors (documented).
Implements docs/arbitrary-html-embed-plan.md (variant C).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
