Merge pull request #255 from screentinker/fix/widget-preview-stays-isolated

Keep the widget editor's Preview isolated, whatever the org setting says

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.

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
         <strong style="color:var(--text-primary)">${t('widget.preview_title')}</strong>
         <button class="btn btn-secondary btn-sm" id="pvClose">${t('widget.close')}</button>
       </div>
-      <iframe id="pvIframe" sandbox="${widgetSandboxIsolationDisabled ? 'allow-scripts allow-same-origin' : 'allow-scripts'}" style="flex:1;width:100%;border:0;background:#000"></iframe>
+      <!-- ALWAYS 'allow-scripts', never allow-same-origin, regardless of the org's
+           widget_sandbox_isolation_disabled setting. This preview loads
+           /api/widgets/preview-session/<id> from the DASHBOARD's own origin, and the
+           dashboard keeps its session JWT in localStorage.token. Granting same-origin
+           here would let anyone who can author a widget (workspace_editor and up) run
+           script in the dashboard origin and read the session of whichever admin opens
+           the preview — an editor -> admin escalation. The org setting exists to let
+           PLAYERS embed origin-strict sites; it is not a licence to de-isolate the
+           dashboard. Covered by widget-preview-stays-isolated.test.js. -->
+      <iframe id="pvIframe" sandbox="allow-scripts" style="flex:1;width:100%;border:0;background:#000"></iframe>
       ${webpageNote}
     </div>`;
   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'); }
   };
 

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}` });
 });

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.<verb>('<route>'` 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(/<iframe id="pvIframe"[^>]*>/);
+  assert.ok(iframeTag, 'could not find the #pvIframe preview iframe — test needs updating');
+  assert.match(iframeTag[0], /sandbox="allow-scripts"/, 'preview iframe must be statically sandboxed');
+  assert.doesNotMatch(
+    iframeTag[0],
+    /allow-same-origin|\$\{/,
+    'preview iframe sandbox must be a literal, not computed from the org setting'
+  );
+});