html-embed: complete the kill-switch — strip embeds at serve time on all read paths (not just shares) #28

New Issue

Ghost · 2026-06-20T20:31:53+03:00

Ghost commented

2026-06-20 20:31:53 +03:00

Goal: make the workspace htmlEmbed toggle a complete, immediate kill-switch — turning it OFF must instantly neutralize every existing embed on all read paths, not just public shares.

Found in security review of PR #16 (merged in 7a03321d). Severity: low (the lingering embed is admin-authored, so this is a completeness gap, not a privilege escalation).

Current state

Public-share serve path DOES strip at serve time when the toggle is OFF — apps/server/src/core/share/share.service.ts:496 (commit 41f3944e). Anonymous share viewers are fully covered. ✅
The regular authenticated page-read path does NOT strip: apps/server/src/core/page/page.controller.ts /info returns pageRepo.findById(...).content as-is.
The client NodeView executes unconditionally in read-only mode: shouldExecute = !editor.isEditable || htmlEmbedEnabled (apps/client/.../html-embed/html-embed-view.tsx:73).

Consequence

After an admin turns the feature OFF, a page that still has a persisted (admin-authored) embed keeps executing for authenticated view-only viewers until that page is next saved (the next write strips it). So the kill-switch is not immediate for the in-workspace read-only path, contrary to the doc's "stops executing immediately" claim (which only holds in editable mode).

Scenario: admin enables feature → adds embed to page P → disables feature → P is not edited again → a member with view-only access to P opens it → embed still runs.

Fix (pick one)

Serve-time strip on the regular read path (preferred): mirror the share-path strip in the authenticated page-read path (page.controller /info → strip htmlEmbed when workspace.settings.htmlEmbed !== true). Makes OFF an instant, total kill-switch with no dependency on a re-save.
Client-side: in read-only mode, for authenticated viewers (who can read the toggle), gate execution on htmlEmbedEnabled; keep unconditional execution only for anonymous/share viewers (who rely on the server strip). Cheaper but leaves the raw source in the served payload.

Option 1 is the more robust "complete the kill-switch" fix.

**Goal:** make the workspace `htmlEmbed` toggle a *complete, immediate* kill-switch — turning it OFF must instantly neutralize every existing embed on **all** read paths, not just public shares. Found in security review of PR #16 (merged in 7a03321d). Severity: low (the lingering embed is admin-authored, so this is a completeness gap, not a privilege escalation). ## Current state - Public-share serve path DOES strip at serve time when the toggle is OFF — `apps/server/src/core/share/share.service.ts:496` (commit 41f3944e). Anonymous share viewers are fully covered. ✅ - The **regular authenticated page-read path does NOT strip**: `apps/server/src/core/page/page.controller.ts` `/info` returns `pageRepo.findById(...).content` as-is. - The client NodeView executes unconditionally in read-only mode: `shouldExecute = !editor.isEditable || htmlEmbedEnabled` (`apps/client/.../html-embed/html-embed-view.tsx:73`). ## Consequence After an admin turns the feature OFF, a page that still has a persisted (admin-authored) embed keeps executing for **authenticated view-only viewers** until that page is next saved (the next write strips it). So the kill-switch is not immediate for the in-workspace read-only path, contrary to the doc's "stops executing immediately" claim (which only holds in editable mode). Scenario: admin enables feature → adds embed to page P → disables feature → P is not edited again → a member with view-only access to P opens it → embed still runs. ## Fix (pick one) 1. **Serve-time strip on the regular read path** (preferred): mirror the share-path strip in the authenticated page-read path (`page.controller /info` → strip `htmlEmbed` when `workspace.settings.htmlEmbed !== true`). Makes OFF an instant, total kill-switch with no dependency on a re-save. 2. **Client-side**: in read-only mode, for *authenticated* viewers (who can read the toggle), gate execution on `htmlEmbedEnabled`; keep unconditional execution only for anonymous/share viewers (who rely on the server strip). Cheaper but leaves the raw source in the served payload. Option 1 is the more robust "complete the kill-switch" fix.

vvzvlad changed title from ~~html-embed: regular authenticated read-only path doesn't strip embeds at serve time after toggle OFF (kill-switch lag)~~ to html-embed: complete the kill-switch — strip embeds at serve time on all read paths (not just shares)

2026-06-20 20:41:53 +03:00

Ghost referenced this issue from a commit

2026-06-20 21:52:45 +03:00

fix(html-embed): strip embeds at serve time on authenticated read paths (#28)

Ghost referenced a pull request that will close this issue

2026-06-20 21:53:29 +03:00

fix(html-embed): complete kill-switch on read paths (#28) + total strip helper (#30) #46

Ghost closed this issue

2026-06-21 01:59:41 +03:00

Ghost referenced this issue from a commit

2026-06-21 01:59:42 +03:00

Merge pull request 'fix(html-embed): complete kill-switch on read paths (#28) + total strip helper (#30)' (#46) from fix/html-embed-hardening into develop