From 226c96c17e08fa9a2b97442641cd1848cbb7a894 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Tue, 11 Aug 2026 15:53:57 -0500 Subject: [PATCH] Keep the widget editor's Preview isolated, whatever the org setting says #254 lets an organization opt out of widget iframe isolation so that players can embed origin-strict third-party sites. It applied that opt-out to the widget editor's Preview as well. Preview is framed by the dashboard, from the dashboard's own origin, and the dashboard keeps its session JWT in localStorage. So with the setting on, anyone who can author a widget -- workspace_editor and up; viewers are refused at the create route -- could put script in a text widget and read the session of whichever admin clicked Preview. That is an editor -> admin escalation, and it is not the risk the confirmation modal asks the admin to accept: a player runs on a kiosk with a device token, an admin's dashboard session is a different thing entirely. The org setting is what makes players able to embed those sites, so the /render path keeps consulting it. Preview is pinned to allow-scripts in both places that build it -- the dashboard iframe and the server-side render -- so neither a frontend change nor a new server caller can re-grant it alone. Also correct the modal copy, which claimed same-origin would expose the session of anyone viewing "a display or preview". Preview is now excluded, and the display case is really the device token, so say that instead. widget-preview-stays-isolated.test.js fails if either half is reverted; both mutations were checked to fail before committing. --- frontend/js/views/settings.js | 10 ++- frontend/js/views/widgets.js | 17 +++-- server/routes/widgets.js | 20 ++++- .../widget-preview-stays-isolated.test.js | 73 +++++++++++++++++++ 4 files changed, 109 insertions(+), 11 deletions(-) create mode 100644 server/test/widget-preview-stays-isolated.test.js diff --git a/frontend/js/views/settings.js b/frontend/js/views/settings.js index 264daf4..7a01f6d 100644 --- a/frontend/js/views/settings.js +++ b/frontend/js/views/settings.js @@ -1230,12 +1230,18 @@ Turning this off re-enables allow-same-origin. Widget HTML will then run with the same privileges as ScreenTinker itself. Any script in any widget in this organization will be able to: - - Read the session token of every logged-in user who views a display or - preview + - Read the device token of every display that shows the widget, and act as + that display against the ScreenTinker API + - Read the session token of any logged-in user who opens a display in their + own browser - Call the ScreenTinker API as that user, including admin actions - Read and modify content on every other display in this organization - Silently exfiltrate all of the above to any server it likes +The widget editor's Preview is NOT affected: it renders inside the dashboard, +where your session lives, so it stays isolated whatever this setting says. A +widget may therefore behave differently in Preview than on a display. + Because allow-scripts is also required for widgets to function, a widget can remove its own sandbox entirely once same-origin is granted. There is no partial protection left after this point. diff --git a/frontend/js/views/widgets.js b/frontend/js/views/widgets.js index e24de5c..88ffd0c 100644 --- a/frontend/js/views/widgets.js +++ b/frontend/js/views/widgets.js @@ -292,7 +292,7 @@ function openContentPicker({ multiple = false, title } = {}) { }); } -function showPreviewModal(sessionId, widgetType, widgetSandboxIsolationDisabled = false) { +function showPreviewModal(sessionId, widgetType) { const overlay = document.createElement('div'); overlay.style.cssText = 'position:fixed;inset:0;background:rgba(0,0,0,0.85);display:flex;align-items:center;justify-content:center;z-index:10000;padding:16px'; // #104: webpage widgets pointing at frame-denying sites (X-Frame-Options) can't be @@ -307,7 +307,16 @@ function showPreviewModal(sessionId, widgetType, widgetSandboxIsolationDisabled ${t('widget.preview_title')} - + + ${webpageNote} `; document.body.appendChild(overlay); @@ -1004,9 +1013,7 @@ export async function render(container) { }); if (!res.ok) throw new Error(t('widget.toast.preview_failed')); const { id } = await res.json(); - let user = null; - try { user = JSON.parse(localStorage.getItem('user') || 'null'); } catch (_) { user = null; } - showPreviewModal(id, type, !!user?.current_organization?.widget_sandbox_isolation_disabled); + showPreviewModal(id, type); } catch (err) { showToast(err.message, 'error'); } }; diff --git a/server/routes/widgets.js b/server/routes/widgets.js index 9cd2060..0e09157 100644 --- a/server/routes/widgets.js +++ b/server/routes/widgets.js @@ -210,6 +210,17 @@ function renderWidgetHtml(type, config, opts = {}) { } } +// The widget editor's Preview is framed by the DASHBOARD, from the dashboard's own +// origin, and the dashboard keeps its session JWT in localStorage. So preview HTML is +// pinned to the isolating sandbox and never consults the org setting: otherwise anyone +// who can author a widget (workspace_editor and up) could run script in the dashboard +// origin and lift the session of whichever admin clicked Preview. +// +// The org setting exists so PLAYERS can embed origin-strict third-party sites. A player +// runs on a kiosk with a device token, which is the risk the confirmation modal +// describes; an admin's dashboard session is not. +const PREVIEW_IFRAME_SANDBOX = 'allow-scripts'; + function widgetIframeSandboxForWorkspace(workspaceId) { if (!workspaceId) return 'allow-scripts'; try { @@ -328,8 +339,9 @@ router.post('/preview', (req, res) => { const { widget_type, config } = req.body || {}; if (!widget_type || typeof widget_type !== 'string') return res.status(400).json({ error: 'widget_type required' }); if (!KNOWN_WIDGET_TYPES.has(widget_type)) return res.status(400).json({ error: 'Unknown widget_type' }); - const iframeSandbox = widgetIframeSandboxForWorkspace(req.workspaceId); - let html = renderWidgetHtml(widget_type, config || {}, { iframeSandbox }); + // Preview renders inside the DASHBOARD origin, so it never opts into same-origin — + // see PREVIEW_IFRAME_SANDBOX. + let html = renderWidgetHtml(widget_type, config || {}, { iframeSandbox: PREVIEW_IFRAME_SANDBOX }); if (req.workspaceId) html = inlineUserContent(html, req.workspaceId); res.setHeader('Content-Type', 'text/html'); res.send(html); @@ -351,8 +363,8 @@ router.post('/preview-session', (req, res) => { if (!widget_type || typeof widget_type !== 'string') return res.status(400).json({ error: 'widget_type required' }); if (!KNOWN_WIDGET_TYPES.has(widget_type)) return res.status(400).json({ error: 'Unknown widget_type' }); const id = uuidv4(); - const iframeSandbox = widgetIframeSandboxForWorkspace(req.workspaceId); - const html = renderWidgetHtml(widget_type, config || {}, { iframeSandbox }); + // Same reasoning as /preview — dashboard origin, never same-origin. + const html = renderWidgetHtml(widget_type, config || {}, { iframeSandbox: PREVIEW_IFRAME_SANDBOX }); previewStore.set(id, { html, widget_type, created: Date.now() }); res.json({ id, url: `/api/widgets/preview-session/${id}` }); }); diff --git a/server/test/widget-preview-stays-isolated.test.js b/server/test/widget-preview-stays-isolated.test.js new file mode 100644 index 0000000..2ad75cc --- /dev/null +++ b/server/test/widget-preview-stays-isolated.test.js @@ -0,0 +1,73 @@ +'use strict'; + +// Guards the boundary added on top of #254 (org-level widget sandbox toggle). +// +// #254 let an org opt out of widget iframe isolation so PLAYERS can embed +// origin-strict third-party sites. As merged it applied the same opt-out to the +// widget editor's Preview — which is framed by the DASHBOARD, from the dashboard's +// own origin, where the admin's session JWT lives in localStorage. That turned +// "my kiosks are less isolated" into "anyone who can author a widget can lift the +// session of whichever admin clicks Preview" (workspace_editor and up; viewers are +// refused at the create route). +// +// These tests fail if the preview path ever consults the org setting again. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const WIDGETS_ROUTE = fs.readFileSync(path.join(__dirname, '..', 'routes', 'widgets.js'), 'utf8'); +const WIDGETS_VIEW = fs.readFileSync( + path.join(__dirname, '..', '..', 'frontend', 'js', 'views', 'widgets.js'), + 'utf8' +); + +// Body of a router handler, from its `router.(''` to the next `router.` +function handlerBody(source, verb, route) { + const start = source.indexOf(`router.${verb}('${route}'`); + assert.notEqual(start, -1, `could not find router.${verb}('${route}') — test needs updating`); + const rest = source.slice(start + 1); + const end = rest.indexOf('\nrouter.'); + return end === -1 ? rest : rest.slice(0, end); +} + +test('preview sandbox constant is the isolating one', () => { + assert.match( + WIDGETS_ROUTE, + /const PREVIEW_IFRAME_SANDBOX = 'allow-scripts';/, + 'PREVIEW_IFRAME_SANDBOX must be exactly allow-scripts (no allow-same-origin)' + ); +}); + +for (const [verb, route] of [['post', '/preview'], ['post', '/preview-session']]) { + test(`${verb.toUpperCase()} ${route} pins the isolating sandbox and ignores the org setting`, () => { + const body = handlerBody(WIDGETS_ROUTE, verb, route); + assert.match(body, /PREVIEW_IFRAME_SANDBOX/, `${route} must render with PREVIEW_IFRAME_SANDBOX`); + assert.doesNotMatch( + body, + /widgetIframeSandboxForWorkspace/, + `${route} must NOT consult the org widget-sandbox setting — it renders in the dashboard origin` + ); + }); +} + +test('player render path still honours the org setting (the feature itself)', () => { + const body = handlerBody(WIDGETS_ROUTE, 'get', '/:id/render'); + assert.match( + body, + /widgetIframeSandboxForWorkspace\(widget\.workspace_id\)/, + 'the /render path is what #254 is for — it must keep consulting the org setting' + ); +}); + +test('dashboard preview iframe is hard-coded to allow-scripts', () => { + const iframeTag = WIDGETS_VIEW.match(/