feat: the announcement banner renders Markdown, safely
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
parent
881534234d
commit
e2444066a0
2 changed files with 49 additions and 1 deletions
|
|
@ -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';
|
||||
|
|
|
|||
|
|
@ -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\(/);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue