From e2444066a01a7c28feecd655b3574b69233b7699 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 10 Sep 2026 19:08:12 +0200 Subject: [PATCH] feat: the announcement banner renders Markdown, safely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was textContent, so formatting appeared literally. It now renders inline Markdown only — bold, italic, code, links, breaks — because this text is injected into every page and must not be able to introduce headings, images or block layout that shifts the app around. parseInline rather than parse, a tag allow-list, event-handler and style attributes forbidden, data attributes off, and one innerHTML assignment that can only be reached through DOMPurify. Missing either library, or any failure while rendering, falls back to the literal text rather than to unsanitised markup. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- public/js/app.js | 26 +++++++++++++++++++++++++- test/frontend-rendering-safety.test.js | 24 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/public/js/app.js b/public/js/app.js index 7434ba5b..1fb3f32c 100644 --- a/public/js/app.js +++ b/public/js/app.js @@ -684,6 +684,30 @@ function applyUserFeatures(features) { document.addEventListener('tabChanged', loadUserFeatures); // ── Announcement banner ──────────────────────────────────── +// The banner is admin-authored and shown on every page, so it renders +// Markdown but only ever emits inline formatting and links — no images, no +// headings, no block layout that could push the app around. Anything else, +// and any failure to sanitise, falls back to the literal text. +function renderAnnouncement(el, raw) { + var markdown = String(raw == null ? '' : raw); + if (!window.marked || typeof window.marked.parseInline !== 'function' || + !window.DOMPurify || typeof window.DOMPurify.sanitize !== 'function') { + el.textContent = markdown; + return; + } + try { + el.innerHTML = window.DOMPurify.sanitize(window.marked.parseInline(markdown, { gfm: true }), { + ALLOWED_TAGS: ['strong', 'em', 'b', 'i', 'u', 's', 'code', 'a', 'br'], + ALLOWED_ATTR: ['href', 'target', 'rel'], + ADD_ATTR: ['target'], + FORBID_ATTR: ['style', 'onerror', 'onload', 'onclick', 'onmouseover'], + ALLOW_DATA_ATTR: false + }); + } catch (e) { + el.textContent = markdown; + } +} + function loadAnnouncement() { loadUserFeatures(); fetch('/api/admin/config/announcement', { headers: getAuthHeaders() }) @@ -695,7 +719,7 @@ function loadAnnouncement() { var icon = document.getElementById('announcement-icon'); if (!banner || !text) return; if (data.enabled && data.text && data.text.trim()) { - text.textContent = data.text; + renderAnnouncement(text, data.text); // type → icon + class var icons = { info: 'fa-info-circle', warning: 'fa-triangle-exclamation', error: 'fa-circle-xmark', success: 'fa-circle-check' }; var type = data.type || 'info'; diff --git a/test/frontend-rendering-safety.test.js b/test/frontend-rendering-safety.test.js index 53cd6daa..ab5ff2c3 100644 --- a/test/frontend-rendering-safety.test.js +++ b/test/frontend-rendering-safety.test.js @@ -18,3 +18,27 @@ test('billing E/M suggestion fields are escaped before innerHTML insertion', () assert.match(source, /escHtml\(data\.emLevel\.rosCount\)/); assert.match(source, /escHtml\(data\.emLevel\.peCount\)/); }); + +test('the announcement banner formats without becoming an injection point', () => { + const fs = require('node:fs'); + const path = require('node:path'); + const app = fs.readFileSync(path.join(__dirname, '..', 'public/js/app.js'), 'utf8'); + const fn = app.slice(app.indexOf('function renderAnnouncement(el, raw)'), app.indexOf('function loadAnnouncement()')); + + // Inline only: this text sits on every page, so it must not be able to + // introduce headings, images or block layout that shifts the app around. + assert.match(fn, /window\.marked\.parseInline\(markdown, \{ gfm: true \}\)/); + assert.match(fn, /ALLOWED_TAGS: \['strong', 'em', 'b', 'i', 'u', 's', 'code', 'a', 'br'\]/); + assert.doesNotMatch(fn, /'img'|'script'|'iframe'|'h1'|'h2'/); + assert.match(fn, /FORBID_ATTR: \['style', 'onerror', 'onload', 'onclick', 'onmouseover'\]/); + assert.match(fn, /ALLOW_DATA_ATTR: false/); + + // Sanitising is not optional: without either library, or on any failure, the + // text is shown literally rather than as markup. + assert.match(fn, /if \(!window\.marked \|\| typeof window\.marked\.parseInline !== 'function' \|\|\s*\n\s*!window\.DOMPurify \|\| typeof window\.DOMPurify\.sanitize !== 'function'\) \{\s*\n\s*el\.textContent = markdown;/); + assert.match(fn, /catch \(e\) \{\s*\n\s*el\.textContent = markdown;/); + // innerHTML is only ever reached through sanitize(). + const assignments = fn.match(/\.innerHTML\s*=/g) || []; + assert.equal(assignments.length, 1, 'one innerHTML assignment'); + assert.match(fn, /el\.innerHTML = window\.DOMPurify\.sanitize\(/); +});