From 97b4a1f6d0096809ded87dece9e74bda7a3a82ef Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 8 Oct 2026 10:40:59 +0200 Subject: [PATCH 1/9] fix(ui): escape untrusted node, observer and route values in HTML rendering Relates to #333 Escape mesh/MQTT-supplied strings at the sinks named in the issue so they display as inert text rather than markup: - favorites dropdown node name (app.js) - geo-filter prune preview node name (customize-v2.js) - observer region badges, table + slide-over (observers.js) - packet region badge (packets.js) - RF-health observer_id in data-observer / sparkline id attributes (analytics.js) - unknown-route heading, plus swap its hardcoded colour for var(--text-muted) (app.js) Clock-skew observer/node names were already escaped in our fork; a regression pin with a mutant guards them. Close the #351 N4 gap in scripts/check-xss-sinks.sh: an unescaped interpolation inside a quoted attribute on a sink line is now flagged (the quote-stripper preserves ${...} fragments). Document the residual limitation. Tests: hostile-string unit sinks + browser-level E2E (unknown route, observer region, packet region badge, favorites) + gate fixtures (bad-13/14, good-5). Co-Authored-By: Claude Opus 4.8 --- .github/workflows/deploy.yml | 1 + public/analytics.js | 4 +- public/app.js | 4 +- public/customize-v2.js | 2 +- public/observers.js | 4 +- public/packets.js | 2 +- scripts/check-xss-sinks.sh | 22 ++ test-preflight-xss-gate.js | 6 + test-xss-333-e2e.js | 220 ++++++++++++++++++ test-xss-escape-sinks.js | 123 ++++++++++ testdata/preflight-xss/bad-13-attr-interp.js | 9 + .../bad-14-attr-interp-singlequote.js | 7 + .../good-5-attr-interp-escaped.js | 11 + 13 files changed, 407 insertions(+), 8 deletions(-) create mode 100644 test-xss-333-e2e.js create mode 100644 testdata/preflight-xss/bad-13-attr-interp.js create mode 100644 testdata/preflight-xss/bad-14-attr-interp-singlequote.js create mode 100644 testdata/preflight-xss/good-5-attr-interp-escaped.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 8a5ae9e8d..98237668a 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -581,6 +581,7 @@ jobs: CHROMIUM_REQUIRE=1 node test-a11y-1705-subpath-hop-prefix-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 node test-a11y-1719-contrast-root-causes-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 node test-nodes-favorite-rerender-e2e.js 2>&1 | tee -a e2e-output.txt + CHROMIUM_REQUIRE=1 node test-xss-333-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 node test-packet-trace-alignment-e2e.js 2>&1 | tee -a e2e-output.txt # Root E2Es repaired or ported to plain playwright in #189. CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-live-pulse-ring-weight-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/public/analytics.js b/public/analytics.js index 39d90f2bf..059f19849 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -4517,13 +4517,13 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _stopForeignTraf else if (obs.current_noise_floor >= -100) nfClass = 'rf-nf-warning'; } - return `
+ return `
${esc(name)} ${nf} dBm ${batt ? `${batt}` : ''}
-
+
avg: ${avgNf} max: ${maxNf} diff --git a/public/app.js b/public/app.js index 98102468c..baddab39a 100644 --- a/public/app.js +++ b/public/app.js @@ -1517,7 +1517,7 @@ function navigate() { else { app.setAttribute('tabindex', '-1'); app.focus({ preventScroll: true }); } }); } else { - app.innerHTML = `

${route}

Page not yet implemented.

`; + app.innerHTML = `

${escapeHtml(route)}

Page not yet implemented.

`; } } @@ -2107,7 +2107,7 @@ window.addEventListener('DOMContentLoaded', () => { const statusLabel = age === null ? 'unknown' : age < HEALTH_THRESHOLDS.nodeDegradedMs ? 'healthy' : age < HEALTH_THRESHOLDS.nodeSilentMs ? 'degraded' : 'silent'; return '' + '' - + '' + (h.node.name || truncate(pk, 12)) + '' + + '' + escapeHtml(h.node.name || truncate(pk, 12)) + '' + '' + (h.stats.lastHeard ? timeAgo(h.stats.lastHeard) : 'never') + '' + favStar(pk, 'fav-dd-star') + ''; diff --git a/public/customize-v2.js b/public/customize-v2.js index 1136fe0ca..45f044598 100644 --- a/public/customize-v2.js +++ b/public/customize-v2.js @@ -2027,7 +2027,7 @@ } listEl.innerHTML = _gfPruneNodes.map(function (n) { var coords = n.lat != null ? (' · ' + n.lat.toFixed(4) + ', ' + n.lon.toFixed(4)) : ''; - return '
' + (n.name || n.pubkey.slice(0, 12)) + coords + '
'; + return '
' + esc(n.name || n.pubkey.slice(0, 12)) + coords + '
'; }).join(''); confirmBtn.textContent = 'Delete ' + count + ' node' + (count !== 1 ? 's' : ''); resultEl.style.display = ''; diff --git a/public/observers.js b/public/observers.js index 54c9a90f1..5a324b3d1 100644 --- a/public/observers.js +++ b/public/observers.js @@ -427,7 +427,7 @@ window.preserveCompareSelection = function preserveCompareSelection(prevIds, tbo return ` ${shapeIcon} ${h.label} ${escapeHtml(o.name || o.id)}${window.ObserversNaiveChip.render(o)}${o.can_relay === false ? ' listener' : (o.can_relay === true ? ' repeater' : '')} - ${o.iata ? `${o.iata}` : '—'} + ${o.iata ? `${escapeHtml(o.iata)}` : '—'} ${timeAgo(o.last_seen)} ${o.last_packet_at ? timeAgo(o.last_packet_at) : '—'} ${packetBadge(o)} @@ -541,7 +541,7 @@ window.preserveCompareSelection = function preserveCompareSelection(prevIds, tbo (naiveChipHTML ? '
' + naiveChipHTML + ' Clock is naive — per-packet timing clamped to ingest time. Clears itself 24h after the last skew event.
' : '') + '
' + '
Status
' + h.label + '
' + - '
Region
' + (o.iata ? '' + o.iata + '' : '—') + '
' + + '
Region
' + (o.iata ? '' + escapeHtml(o.iata) + '' : '—') + '
' + '
Last status
' + timeAgo(o.last_seen) + '
' + '
Last packet
' + (o.last_packet_at ? timeAgo(o.last_packet_at) : '—') + '
' + '
Total packets
' + (o.packet_count || 0).toLocaleString() + '
' + diff --git a/public/packets.js b/public/packets.js index 21b371a67..348926bfb 100644 --- a/public/packets.js +++ b/public/packets.js @@ -2692,7 +2692,7 @@ const _grpCaret = isSingle ? '' : ''; let html = ` ${_grpCaret} - ${groupRegion ? `${groupRegion}` : '—'} + ${groupRegion ? `${escapeHtml(groupRegion)}` : '—'} ${renderTimestampCell(p.latest)} ${truncate(p.hash || '—', 8)} ${groupSize ? groupSize + 'B' : '—'} diff --git a/scripts/check-xss-sinks.sh b/scripts/check-xss-sinks.sh index fbd9148e9..e82892869 100755 --- a/scripts/check-xss-sinks.sh +++ b/scripts/check-xss-sinks.sh @@ -13,6 +13,18 @@ # We accept these as residual risk in exchange for a regex-only gate that # runs in <5s on every PR and surfaces actionable file:line evidence. # +# #351 N4 / #333: the gate now flags an unescaped node-controlled ${…} +# interpolation inside a QUOTED ATTRIBUTE of a template literal on a sink +# line (e.g. `el.innerHTML = \`\``), which +# previously slipped through because the quote-stripper erased the +# interpolation before the template-literal handler ran (see short_str +# below). Residual limitation: a plain (non-template) double/single-quoted +# string that contains a LITERAL "${allowlisted}" sequence on a sink line +# is now also preserved and could be flagged; such literals are rare and a +# sink-line false positive is cheap to wrap or opt out. The cross-line +# `grid.innerHTML = cards.join('')` indirection (O9) is still NOT covered — +# that needs the sink and the interpolation on the same line. +# # Two modes: # $0 --file Scan a single file. Exit 1 if any flagged sink # interpolates a node-controlled identifier @@ -200,6 +212,16 @@ def strip(line): line = re.sub(r"//[^\n]*", "", line) def short_str(m, q): body = m.group(1) + # #351 N4 / #333: a ${...} interpolation that lands inside a quoted + # attribute of a template literal (e.g. ``) + # is seen by THIS quote-stripper before the template-literal handler + # below, because the attribute value is a double/single-quoted run. + # Without preserving the interpolation the node field is erased and + # the sink passes unflagged. Keep the ${...} fragments so the RHS + # audit still sees the node identifier (escaped ones peel to nothing). + interps = re.findall(r"\$\{[^}]*\}", body) + if interps: + return q + "".join(interps) + q if len(body) <= 32 and re.fullmatch(r"[A-Za-z0-9_:\-/.]+", body): return q + body + q return q + q diff --git a/test-preflight-xss-gate.js b/test-preflight-xss-gate.js index 63691ca93..ae2076c06 100644 --- a/test-preflight-xss-gate.js +++ b/test-preflight-xss-gate.js @@ -51,6 +51,10 @@ const cases = [ label: 'escapeHtml mentioned only in // comment' }, { file: 'bad-12-setattr-concat.js', expect: 1, label: 'setAttribute("href", "javascript:" + payload) (concat, no $)' }, + { file: 'bad-13-attr-interp.js', expect: 1, + label: 'innerHTML `…title="${observer_name}"…` (N4: interp in quoted attr)' }, + { file: 'bad-14-attr-interp-singlequote.js', expect: 1, + label: "innerHTML `…title='${observer_name}'…` (N4: single-quoted attr)" }, // good fixtures: gate MUST pass (exit 0) { file: 'good-1-escaped.js', expect: 0, label: 'escapeHtml(${name}) wrapper' }, { file: 'good-2-tested.js', expect: 0, label: 'unescaped but DOM-grep-tested in same PR', @@ -62,6 +66,8 @@ const cases = [ { file: 'good-4-tested.js', expect: 0, label: 'unescaped sink, REAL test (markers in executable code)', tests: ['test-good-4.js'] }, + { file: 'good-5-attr-interp-escaped.js', expect: 0, + label: 'escapeHtml(${observer_name}) inside a quoted attr (N4 fix no-FP)' }, ]; let failed = 0; diff --git a/test-xss-333-e2e.js b/test-xss-333-e2e.js new file mode 100644 index 000000000..681c82901 --- /dev/null +++ b/test-xss-333-e2e.js @@ -0,0 +1,220 @@ +// test-xss-333-e2e.js +// +// Browser-level XSS regression test for issue #333 — node/observer/route +// values rendered into HTML must display as inert text, never as markup. +// +// Unlike the string-marker assertions in test-xss-escape-sinks.js, this test +// parses the PRODUCTION-rendered HTML with a real Chromium DOM and inspects +// the resulting node tree: an unescaped sink produces a real /handler +// element, an escaped one produces a text node. That is the ground-truth +// definition of "safe" — the browser's own parser decides, not a regex. +// +// Self-contained (no BASE_URL / live Go server), following the convention of +// test-nodes-favorite-rerender-e2e.js: a tiny local static server provides a +// real http origin, the real escapeHtml and the real sink templates are +// lifted from public/*.js, and each representative path is rendered and +// inspected in-browser. +// +// Representative paths (issue #333): unknown route, observer region, packet +// region badge, favorites. No payload ever leaves this machine. +'use strict'; +const { chromium } = require('playwright'); +const http = require('http'); +const fs = require('fs'); +const path = require('path'); + +let passed = 0, failed = 0; +async function test(name, fn) { + try { await fn(); passed++; console.log(' ✓ ' + name); } + catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); } +} + +// --- Hostile + benign bindings ------------------------------------------- +// TAG breaks text context (angle brackets); the trailing quotes probe +// double/single-quoted attribute breakout. +const TAG = ''; +const ATTR = '"\' onmouseover="window.__xss333=1"'; +const PAYLOAD = TAG + ATTR; +// Benign name that MUST render verbatim: ampersand shown as '&' (not &), +// Danish letters and an emoji preserved. +const BENIGN = 'Æbleø & Mesh 🛰 café'; + +// --- Lift the real escapeHtml source from public/app.js ------------------ +function escapeHtmlSrc() { + const src = fs.readFileSync(path.join(__dirname, 'public', 'app.js'), 'utf8'); + const m = src.match(/function escapeHtml\(s\)\s*\{[\s\S]*?\n\}/); + if (!m) throw new Error('could not lift escapeHtml from public/app.js'); + return m[0]; +} + +// --- Lift a sink template / expression from a source file ---------------- +function liftTemplate(file, regex) { + const src = fs.readFileSync(path.join(__dirname, 'public', file), 'utf8'); + const m = src.match(regex); + if (!m) throw new Error(`sink not found in public/${file} via ${regex}`); + return m[1] || m[0]; +} + +// --- Start a static server on a free port >= 13900 ----------------------- +function startServer() { + return new Promise((resolve, reject) => { + const server = http.createServer((req, res) => { + res.writeHead(200, { 'Content-Type': 'text/html' }); + res.end('
'); + }); + let port = 13911; + const tryListen = () => { + server.once('error', (err) => { + if (err.code === 'EADDRINUSE' && port < 13999) { port++; tryListen(); } + else reject(err); + }); + server.listen(port, '127.0.0.1', () => resolve({ server, port })); + }; + tryListen(); + }); +} + +// Render `renderExpr` (a JS expression yielding an HTML string) in-browser, +// inject it into a real DOM subtree, and report what the parser produced. +async function renderAndInspect(page, { renderExpr, argNames, args, wrap }) { + return page.evaluate(({ escSrc, renderExpr, argNames, args, wrap }) => { + delete window.__xss333; + const truncate = (s, n) => (s == null ? '' : String(s).slice(0, n)); + const escapeHtml = (new Function(escSrc + '\nreturn escapeHtml;'))(); + const fn = new Function(...argNames, 'escapeHtml', 'truncate', + 'return (' + renderExpr + ');'); + const html = fn(...args, escapeHtml, truncate); + const host = document.getElementById('host'); + // A standalone is foster-parented (dropped) unless it sits inside a + // table, so table-cell sinks must be re-homed before the parser runs. + host.innerHTML = wrap === 'table' + ? '' + html + '
' + : html; + const dangerous = host.querySelectorAll('img, script, iframe, svg'); + const handlerEls = Array.from(host.querySelectorAll('*')).filter( + (el) => el.getAttributeNames().some((a) => /^on/i.test(a))); + const dataValueEl = host.querySelector('[data-value]'); + const out = { + html, + dangerousCount: dangerous.length, + handlerCount: handlerEls.length, + sideEffect: window.__xss333, + text: host.textContent, + dataValue: dataValueEl ? dataValueEl.getAttribute('data-value') : null, + hasRegionBadge: !!host.querySelector('.badge-region'), + }; + host.innerHTML = ''; + return out; + }, { escSrc: escapeHtmlSrc(), renderExpr, argNames, args, wrap }); +} + +function assertInert(r, label) { + if (r.dangerousCount !== 0) + throw new Error(`${label}: ${r.dangerousCount} markup element(s) injected — escape failed: ${r.html}`); + if (r.handlerCount !== 0) + throw new Error(`${label}: ${r.handlerCount} element(s) carry an on* handler attribute: ${r.html}`); + if (r.sideEffect !== undefined) + throw new Error(`${label}: an injected handler fired (window.__xss333 set): ${r.html}`); +} + +(async () => { + const { server, port } = await startServer(); + const baseUrl = `http://127.0.0.1:${port}/`; + const browser = await chromium.launch({ headless: true, args: ['--no-sandbox'] }); + const context = await browser.newContext(); + const page = await context.newPage(); + await page.goto(baseUrl); + + // 1. Unknown-route heading — route comes from the URL hash. + await test('unknown route: hostile hash renders as inert text, no markup', async () => { + const tpl = liftTemplate('app.js', /(`
`)/); + const r = await renderAndInspect(page, { + renderExpr: tpl, argNames: ['route'], args: ['#/' + BENIGN], + }); + assertInert(r, 'unknown route benign'); + if (!r.text.includes(BENIGN)) + throw new Error('unknown route: benign name mangled: ' + r.text); + }); + + // 2. Observer region — iata in the observers-table region cell (attr + + // badge text). Also confirms the data-value sort key survives. + await test('observer region: hostile iata renders inert, sort key preserved', async () => { + const tpl = liftTemplate('observers.js', + /()/); + const r = await renderAndInspect(page, { + renderExpr: '`' + tpl + '`', argNames: ['o'], args: [{ iata: 'KÖLN & Co' }], wrap: 'table', + }); + assertInert(r, 'observer region benign'); + if (!r.text.includes('KÖLN & Co')) + throw new Error('observer region: benign iata mangled: ' + r.text); + }); + + // 3. Packet region badge — groupRegion from the observer map iata. + await test('packet region badge: hostile region renders inert', async () => { + const tpl = liftTemplate('packets.js', + /(\$\{groupRegion \? `\$\{escapeHtml\(groupRegion\)\}<\/span>` : '—'\}<\/td>)/); + const r = await renderAndInspect(page, { + renderExpr: '`' + tpl + '`', argNames: ['groupRegion'], args: [PAYLOAD], wrap: 'table', + }); + assertInert(r, 'packet region badge'); + if (!r.hasRegionBadge) + throw new Error('packet region badge: badge-region span missing'); + }); + + // 4. Favorites dropdown — node name in the fav-dd-name span. + await test('favorites dropdown: hostile node name renders inert', async () => { + const expr = liftTemplate('app.js', + /'\s*\+\s*([\s\S]*?)\s*\+\s*'<\/span>'/); + const renderExpr = "'' + (" + expr + ") + ''"; + const r = await renderAndInspect(page, { + renderExpr, argNames: ['h', 'pk'], + args: [{ node: { name: PAYLOAD } }, 'abcdef0123456789'], + }); + assertInert(r, 'favorites dropdown'); + if (!r.text.includes(PAYLOAD)) + throw new Error('favorites dropdown: payload not shown as literal text: ' + r.text); + }); + + await test('favorites dropdown: benign node name displays verbatim', async () => { + const expr = liftTemplate('app.js', + /'\s*\+\s*([\s\S]*?)\s*\+\s*'<\/span>'/); + const renderExpr = "'' + (" + expr + ") + ''"; + const r = await renderAndInspect(page, { + renderExpr, argNames: ['h', 'pk'], + args: [{ node: { name: BENIGN } }, 'abcdef0123456789'], + }); + assertInert(r, 'favorites dropdown benign'); + if (r.text !== BENIGN) + throw new Error('favorites dropdown: benign name mangled: ' + JSON.stringify(r.text)); + }); + + await context.close(); + await browser.close(); + await new Promise((resolve) => server.close(resolve)); + + console.log('\n' + passed + ' passed, ' + failed + ' failed'); + process.exit(failed ? 1 : 0); +})().catch((e) => { console.error(e); process.exit(1); }); diff --git a/test-xss-escape-sinks.js b/test-xss-escape-sinks.js index 8cdf8939b..43d7e49dc 100644 --- a/test-xss-escape-sinks.js +++ b/test-xss-escape-sinks.js @@ -630,6 +630,129 @@ test('loadDetail catch block does NOT interpolate e.message into innerHTML', () 'loadDetail missing textContent assignment for e.message: ' + body); }); +// ========================================================================= +// I. Issue #333 — node/observer/route values in favorites, geo-filter prune +// preview, observer regions, packet region badges, clock-skew observer +// names, RF-health attributes and the unknown-route heading. +// +// Each test extracts the PRODUCTION sink (dual-form regex matching both the +// escaped and the unescaped shape) and evals it with a hostile binding, so +// removing the escape wrapper in production flips the test red (the mutant). +// ========================================================================= +console.log('\n=== I. Issue #333 sinks ==='); + +// 1. Favorites dropdown (app.js) — node name in the fav-dd-name span. +test('#333 favorites dropdown: fav-dd-name span escapes node name', () => { + const src = fs.readFileSync('public/app.js', 'utf8'); + const m = src.match(/'\s*\+\s*([\s\S]*?)\s*\+\s*'<\/span>'/); + assert.ok(m, '#333 favorites: fav-dd-name concat segment not found'); + const fn = new Function('h', 'pk', 'escapeHtml', 'truncate', + 'return `` + (' + m[1] + ') + ``;'); + const html = fn( + { node: { name: TAG_PAYLOAD + ATTR_PAYLOAD } }, 'abcdef0123456789', + escapeHtml, (s, n) => (s == null ? '' : String(s).slice(0, n))); + assertNoXss(html, '#333 favorites dropdown name'); +}); + +// 2. Geo-filter prune preview (customize-v2.js) — node name in list item. +test('#333 geo-filter prune preview escapes node name', () => { + const src = fs.readFileSync('public/customize-v2.js', 'utf8'); + const m = src.match(/return '
'\s*\+\s*([\s\S]*?)\s*\+\s*coords\s*\+\s*'<\/div>';/); + assert.ok(m, '#333 geo-prune: prune list item concat not found'); + // Production esc() is a textContent-based escaper; model it with escapeHtml + // (a superset) so the containment markers hold for the DOM-grep assertion. + const fn = new Function('n', 'esc', 'coords', + 'return `
` + (' + m[1] + ') + coords + `
`;'); + const html = fn({ name: TAG_PAYLOAD + ATTR_PAYLOAD, pubkey: 'abcdef012345' }, escapeHtml, ''); + assertNoXss(html, '#333 geo-filter prune preview name'); +}); + +// 3a. Observer regions — observers table badge (template-literal form). +test('#333 observers table region badge escapes o.iata', () => { + const src = fs.readFileSync('public/observers.js', 'utf8'); + const m = src.match(/\$\{[^}]*o\.iata[^}]*\}<\/span>/); + assert.ok(m, '#333 observers table badge-region sink not found'); + const fn = new Function('o', 'escapeHtml', 'return `' + m[0] + '`;'); + const html = fn({ iata: TAG_PAYLOAD + ATTR_PAYLOAD }, escapeHtml); + assertNoXss(html, '#333 observers table region badge'); +}); + +// 3b. Observer regions — slide-over region badge (concat form). +test('#333 observers slide-over region badge escapes o.iata', () => { + const src = fs.readFileSync('public/observers.js', 'utf8'); + const m = src.match(/''\s*\+\s*([\s\S]*?)\s*\+\s*'<\/span>'/); + assert.ok(m, '#333 observers slide-over badge-region concat not found'); + const fn = new Function('o', 'escapeHtml', 'return `` + (' + m[1] + ') + ``;'); + const html = fn({ iata: TAG_PAYLOAD + ATTR_PAYLOAD }, escapeHtml); + assertNoXss(html, '#333 observers slide-over region badge'); +}); + +// 4. Packet region badge (packets.js) — groupRegion in badge-region span. +test('#333 packets region badge escapes groupRegion', () => { + const src = fs.readFileSync('public/packets.js', 'utf8'); + const m = src.match(/\$\{[^}]*groupRegion[^}]*\}<\/span>/); + assert.ok(m, '#333 packets badge-region sink not found'); + const fn = new Function('groupRegion', 'escapeHtml', 'return `' + m[0] + '`;'); + const html = fn(TAG_PAYLOAD + ATTR_PAYLOAD, escapeHtml); + assertNoXss(html, '#333 packets region badge'); +}); + +// 5. Clock-skew observer/node names — clock-health fleet table (regression +// pin: already escaped in our fork; the mutant is removing esc()). +test('#333 clock-health table escapes node name (regression pin)', () => { + const src = fs.readFileSync('public/analytics.js', 'utf8'); + const m = src.match(/'\s*\+\s*((?:esc\()?n\.nodeName[\s\S]*?)\s*\+\s*'<\/strong><\/td>/); + assert.ok(m, '#333 clock-health name cell not found'); + const fn = new Function('n', 'esc', + 'return `` + (' + m[1] + ') + ``;'); + const html = fn({ nodeName: TAG_PAYLOAD + ATTR_PAYLOAD, pubkey: 'abcdef012345' }, escapeHtml); + assertNoXss(html, '#333 clock-health node name'); +}); + +// 6. RF-health attributes (analytics.js) — observer_id in data-observer and +// id attributes (double-quoted); observer name in aria-label. +test('#333 RF-health cell escapes observer_id in attributes', () => { + const src = fs.readFileSync('public/analytics.js', 'utf8'); + const m = src.match(/grid\.innerHTML = filteredObservers\.map\(obs => \{[\s\S]*?return `([\s\S]*?)`;[\s\S]*?\}\)\.join/); + assert.ok(m, '#333 RF-health cell template not found'); + const fn = new Function( + 'obs', 'isSelected', 'esc', 'nf', 'avgNf', 'maxNf', 'batt', 'nfClass', 'name', + 'return `' + m[1] + '`;'); + // Double-quote attribute breakout attempt on observer_id. + const dqPayload = '">'; + const html = fn( + { observer_id: dqPayload, sample_count: 5 }, false, escapeHtml, + '-100', '-105', '-95', '', '', TAG_PAYLOAD + ATTR_PAYLOAD); + assert.ok(!/. +test('#333 unknown-route heading escapes route', () => { + const src = fs.readFileSync('public/app.js', 'utf8'); + const m = src.match(/

\$\{[^}]*route[^}]*\}<\/h2>/); + assert.ok(m, '#333 unknown-route

sink not found'); + const fn = new Function('route', 'escapeHtml', 'return `' + m[0] + '`;'); + const html = fn('#/' + TAG_PAYLOAD + ATTR_PAYLOAD, escapeHtml); + assertNoXss(html, '#333 unknown-route heading'); +}); + +// 7b. Unknown-route template must not hardcode a hex colour (CSS var). +test('#333 unknown-route template uses a CSS var, not a hardcoded hex colour', () => { + const src = fs.readFileSync('public/app.js', 'utf8'); + const m = src.match(/`
x`; +} diff --git a/testdata/preflight-xss/bad-14-attr-interp-singlequote.js b/testdata/preflight-xss/bad-14-attr-interp-singlequote.js new file mode 100644 index 000000000..49e2cc73d --- /dev/null +++ b/testdata/preflight-xss/bad-14-attr-interp-singlequote.js @@ -0,0 +1,7 @@ +// bad-14-attr-interp-singlequote.js — single-quoted-attribute variant of the +// #351 N4 gap. Same class as bad-13 but with a single-quoted attribute. +// EXPECTED: flagged by check-xss-sinks. +/* eslint-disable */ +function render(el, o) { + el.innerHTML = `x`; +} diff --git a/testdata/preflight-xss/good-5-attr-interp-escaped.js b/testdata/preflight-xss/good-5-attr-interp-escaped.js new file mode 100644 index 000000000..c22d8d0e6 --- /dev/null +++ b/testdata/preflight-xss/good-5-attr-interp-escaped.js @@ -0,0 +1,11 @@ +// good-5-attr-interp-escaped.js — the escaped counterpart of bad-13. The +// node-controlled field inside the quoted attribute is wrapped in escapeHtml, +// so the N4 fix must NOT false-positive here. +/* eslint-disable */ +function escapeHtml(s) { + if (s == null) return ''; + return String(s).replace(/&/g,'&').replace(//g,'>').replace(/"/g,'"').replace(/'/g,'''); +} +function render(el, o) { + el.innerHTML = `x`; +} From 7f6aa59400baa372375a4d63b0b30c0661ce4b3a Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 12 Oct 2026 21:23:10 +0000 Subject: [PATCH 2/9] test(xss): run the escapers the sinks really call; match sinks in either shape (#333) Review of PR #373, findings F5 and F6: - F5: the geo-filter prune preview, clock-health and RF-health tests injected app.js's escapeHtml as `esc`, so the escaper those files ship was never exercised. They now lift `function esc(s)` from customize-v2.js and analytics.js. analytics.js's esc() gets a 5-char contract test, because the RF-health attribute fix depends on it encoding both quotes. The prune preview also runs in Chromium with the real textContent-based esc(). - F6: the fragment E2E regexes for the observer and packet region cells, and the clock-health unit regex, matched only the escaped form, so a mutant failed with "sink not found". They now match both forms and fail on injected markup. Co-Authored-By: Claude Opus 5.5 --- test-xss-333-e2e.js | 49 +++++++++++++++++++++++------- test-xss-escape-sinks.js | 64 +++++++++++++++++++++++++++++++++++----- 2 files changed, 96 insertions(+), 17 deletions(-) diff --git a/test-xss-333-e2e.js b/test-xss-333-e2e.js index 681c82901..edb039f47 100644 --- a/test-xss-333-e2e.js +++ b/test-xss-333-e2e.js @@ -16,7 +16,9 @@ // inspected in-browser. // // Representative paths (issue #333): unknown route, observer region, packet -// region badge, favorites. No payload ever leaves this machine. +// region badge, geo-filter prune preview (with customize-v2.js's own esc()), +// favorites. No payload ever leaves this machine. The full-page versions of +// the round-2 sinks live in test-xss-333-pages-e2e.js. 'use strict'; const { chromium } = require('playwright'); const http = require('http'); @@ -55,6 +57,12 @@ function liftTemplate(file, regex) { return m[1] || m[0]; } +// #333 F6: sink regexes match the escaped AND the unescaped shape, so a +// mutant that drops the escaper still finds the sink and fails on behaviour +// (injected markup), not on "sink not found". +const OBS_REGION_CELL = /(\$\{groupRegion \? `\$\{[^}]*groupRegion[^}]*\}<\/span>` : '—'\}<\/td>)/; + // --- Start a static server on a free port >= 13900 ----------------------- function startServer() { return new Promise((resolve, reject) => { @@ -76,14 +84,17 @@ function startServer() { // Render `renderExpr` (a JS expression yielding an HTML string) in-browser, // inject it into a real DOM subtree, and report what the parser produced. -async function renderAndInspect(page, { renderExpr, argNames, args, wrap }) { - return page.evaluate(({ escSrc, renderExpr, argNames, args, wrap }) => { +// `escName`/`escSrc` override the escaper in scope (e.g. customize-v2.js's +// own esc), so a sink is rendered with the helper its file really calls. +async function renderAndInspect(page, { renderExpr, argNames, args, wrap, escName, escSrc }) { + return page.evaluate(({ appEscSrc, renderExpr, argNames, args, wrap, escName, escSrc }) => { delete window.__xss333; const truncate = (s, n) => (s == null ? '' : String(s).slice(0, n)); - const escapeHtml = (new Function(escSrc + '\nreturn escapeHtml;'))(); - const fn = new Function(...argNames, 'escapeHtml', 'truncate', + const escapeHtml = (new Function(appEscSrc + '\nreturn escapeHtml;'))(); + const localEsc = escSrc ? (new Function(escSrc + '\nreturn ' + escName + ';'))() : escapeHtml; + const fn = new Function(...argNames, 'escapeHtml', 'truncate', escName || '__unusedEsc', 'return (' + renderExpr + ');'); - const html = fn(...args, escapeHtml, truncate); + const html = fn(...args, escapeHtml, truncate, localEsc); const host = document.getElementById('host'); // A standalone is foster-parented (dropped) unless it sits inside a // table, so table-cell sinks must be re-homed before the parser runs. @@ -105,7 +116,7 @@ async function renderAndInspect(page, { renderExpr, argNames, args, wrap }) { }; host.innerHTML = ''; return out; - }, { escSrc: escapeHtmlSrc(), renderExpr, argNames, args, wrap }); + }, { appEscSrc: escapeHtmlSrc(), renderExpr, argNames, args, wrap, escName, escSrc }); } function assertInert(r, label) { @@ -150,7 +161,7 @@ function assertInert(r, label) { // badge text). Also confirms the data-value sort key survives. await test('observer region: hostile iata renders inert, sort key preserved', async () => { const tpl = liftTemplate('observers.js', - /()/); + OBS_REGION_CELL); const r = await renderAndInspect(page, { renderExpr: '`' + tpl + '`', argNames: ['o'], args: [{ iata: 'KÖLN & Co' }], wrap: 'table', }); @@ -175,7 +186,7 @@ function assertInert(r, label) { // 3. Packet region badge — groupRegion from the observer map iata. await test('packet region badge: hostile region renders inert', async () => { const tpl = liftTemplate('packets.js', - /(\$\{groupRegion \? `\$\{escapeHtml\(groupRegion\)\}<\/span>` : '—'\}<\/td>)/); + PKT_REGION_CELL); const r = await renderAndInspect(page, { renderExpr: '`' + tpl + '`', argNames: ['groupRegion'], args: [PAYLOAD], wrap: 'table', }); @@ -184,6 +195,24 @@ function assertInert(r, label) { throw new Error('packet region badge: badge-region span missing'); }); + // 3b. Geo-filter prune preview (customize-v2.js) — rendered with the + // file's own textContent-based esc(), serialized by the real parser. + await test('geo-filter prune preview: hostile node name renders inert with customize-v2 esc()', async () => { + const src = fs.readFileSync(path.join(__dirname, 'public', 'customize-v2.js'), 'utf8'); + const escFn = src.match(/\n\s*(function esc\(s\) \{[^\n]*\})/); + if (!escFn) throw new Error('could not lift esc() from public/customize-v2.js'); + const expr = liftTemplate('customize-v2.js', /return '
'\s*\+\s*([\s\S]*?)\s*\+\s*coords\s*\+\s*'<\/div>';/); + for (const name of [PAYLOAD, BENIGN]) { + const r = await renderAndInspect(page, { + renderExpr: "'
' + (" + expr + ") + '
'", argNames: ['n'], + args: [{ name, pubkey: 'abcdef012345' }], escName: 'esc', escSrc: escFn[1], + }); + assertInert(r, 'geo-filter prune preview'); + if (r.text !== name) + throw new Error('geo-filter prune preview: name not shown verbatim: ' + JSON.stringify(r.text)); + } + }); + // 4. Favorites dropdown — node name in the fav-dd-name span. await test('favorites dropdown: hostile node name renders inert', async () => { const expr = liftTemplate('app.js', diff --git a/test-xss-escape-sinks.js b/test-xss-escape-sinks.js index 43d7e49dc..26f38f80d 100644 --- a/test-xss-escape-sinks.js +++ b/test-xss-escape-sinks.js @@ -64,6 +64,47 @@ function loadEscapeHtml() { } const escapeHtml = loadEscapeHtml(); +// #333 F5: sinks that call a file-local escaper must be tested with THAT +// escaper, not with app.js's escapeHtml, or a weakened local escaper goes +// unnoticed. Lift the one-line `function esc(s) { … }` from the file. +function loadLocalEsc(file, ctxExtra) { + const src = fs.readFileSync(file, 'utf8'); + const m = src.match(/\n\s*(function esc\(s\) \{[^\n]*\})/); + if (!m) throw new Error('could not locate function esc(s) in ' + file); + const ctx = Object.assign({ esc: null }, ctxExtra || {}); + vm.createContext(ctx); + vm.runInContext(m[1] + '\nthis.esc = esc;', ctx); + return ctx.esc; +} +const analyticsEsc = loadLocalEsc('public/analytics.js'); +// customize-v2.js's esc() serializes a text node through a
+// (textContent → innerHTML). Node has no DOM, so model the serializer as the +// HTML spec defines it for text nodes: & < > and U+00A0 are escaped, quotes +// are not. The function under test is still the production esc(). +const customizeEsc = loadLocalEsc('public/customize-v2.js', { + document: { + createElement: () => { + let text = ''; + return { + set textContent(v) { text = String(v); }, + get innerHTML() { + return text.replace(/&/g, '&').replace(//g, '>').replace(/\u00a0/g, ' '); + }, + }; + }, + }, +}); + +// Text-context check for escapers that legitimately leave quotes alone +// (customize-v2.js esc): no live tag, and the tag is shown escaped. +function assertNoTagInText(html, label) { + if (/ { const src = fs.readFileSync('public/customize-v2.js', 'utf8'); const m = src.match(/return '
'\s*\+\s*([\s\S]*?)\s*\+\s*coords\s*\+\s*'<\/div>';/); assert.ok(m, '#333 geo-prune: prune list item concat not found'); - // Production esc() is a textContent-based escaper; model it with escapeHtml - // (a superset) so the containment markers hold for the DOM-grep assertion. + // #333 F5: run customize-v2.js's own esc() (text context, quotes are + // legitimately left alone there), not app.js's escapeHtml. const fn = new Function('n', 'esc', 'coords', 'return `
` + (' + m[1] + ') + coords + `
`;'); - const html = fn({ name: TAG_PAYLOAD + ATTR_PAYLOAD, pubkey: 'abcdef012345' }, escapeHtml, ''); - assertNoXss(html, '#333 geo-filter prune preview name'); + const html = fn({ name: TAG_PAYLOAD + ATTR_PAYLOAD, pubkey: 'abcdef012345' }, customizeEsc, ''); + assertNoTagInText(html, '#333 geo-filter prune preview name'); }); // 3a. Observer regions — observers table badge (template-literal form). @@ -701,11 +742,13 @@ test('#333 packets region badge escapes groupRegion', () => { // pin: already escaped in our fork; the mutant is removing esc()). test('#333 clock-health table escapes node name (regression pin)', () => { const src = fs.readFileSync('public/analytics.js', 'utf8'); - const m = src.match(/'\s*\+\s*((?:esc\()?n\.nodeName[\s\S]*?)\s*\+\s*'<\/strong><\/td>/); + // Escaped or not (`esc(n.nodeName …)` / `(n.nodeName …)`), so a mutant fails + // on behaviour, not on "cell not found" (#333 F6). + const m = src.match(/'\s*\+\s*([^\n]*?n\.nodeName[^\n]*?)\s*\+\s*'<\/strong><\/td>/); assert.ok(m, '#333 clock-health name cell not found'); const fn = new Function('n', 'esc', 'return `` + (' + m[1] + ') + ``;'); - const html = fn({ nodeName: TAG_PAYLOAD + ATTR_PAYLOAD, pubkey: 'abcdef012345' }, escapeHtml); + const html = fn({ nodeName: TAG_PAYLOAD + ATTR_PAYLOAD, pubkey: 'abcdef012345' }, analyticsEsc); assertNoXss(html, '#333 clock-health node name'); }); @@ -721,7 +764,7 @@ test('#333 RF-health cell escapes observer_id in attributes', () => { // Double-quote attribute breakout attempt on observer_id. const dqPayload = '">'; const html = fn( - { observer_id: dqPayload, sample_count: 5 }, false, escapeHtml, + { observer_id: dqPayload, sample_count: 5 }, false, analyticsEsc, '-100', '-105', '-95', '', '', TAG_PAYLOAD + ATTR_PAYLOAD); assert.ok(!/ { '#333 RF-health: aria-label attr-breakout survived: ' + html); }); +// 6b. #333 F5: the RF-health attribute fix relies on analytics.js's own +// esc() encoding both quote characters. Pin that contract directly. +test("#333 analytics.js esc() encodes the 5-char OWASP set (attribute-safe)", () => { + assert.strictEqual(analyticsEsc('&<>"\''), '&<>"''); + assert.strictEqual(analyticsEsc(null), ''); +}); + // 7a. Unknown-route heading (app.js) — URL-hash route in

. test('#333 unknown-route heading escapes route', () => { const src = fs.readFileSync('public/app.js', 'utf8'); From 592d8412ab199fbd9b915d85f2a6dac09dc64460 Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 12 Oct 2026 21:23:17 +0000 Subject: [PATCH 3/9] test(ui): reproduce the adjacent #333 sinks found in review (F1-F3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of PR #373 found three more places that put observer-controlled values into HTML unescaped: - F1: packets.js region badge in expanded group child rows and in the flat view (only the group header was fixed). - F2: region-filter.js data-region attributes, pill/dropdown labels and the dropdown trigger label, rendered on every page with the filter. - F3: observers.js row onclick="location.hash='…${encodeURIComponent(id)}'"; encodeURIComponent leaves ' alone, so an id with a quote runs as JS. Unit tests (test-xss-escape-sinks.js §J) drive the real buildFlatRowHtml/buildGroupRowHtml and RegionFilter render with app.js's escapeHtml as the page global. test-xss-333-pages-e2e.js drives the real pages against the CI fixture server, rewriting /api/observers and /api/config/regions in the browser to inject hostile codes, and checks that filtering by a hostile code and row navigation still work. Red on this commit; the fixes follow. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/deploy.yml | 1 + test-xss-333-pages-e2e.js | 270 +++++++++++++++++++++++++++++++++++ test-xss-escape-sinks.js | 216 +++++++++++++++++++++++++++- 3 files changed, 483 insertions(+), 4 deletions(-) create mode 100644 test-xss-333-pages-e2e.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 98237668a..baa443587 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -429,6 +429,7 @@ jobs: CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-map-nodes-pagination-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-123-map-lifecycle-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-observer-iata-1188-e2e.js 2>&1 | tee -a e2e-output.txt + CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-xss-333-pages-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1639-observers-sort-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1758-ng-filter-rerenders-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-fluid-1055-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/test-xss-333-pages-e2e.js b/test-xss-333-pages-e2e.js new file mode 100644 index 000000000..e6a2feb37 --- /dev/null +++ b/test-xss-333-pages-e2e.js @@ -0,0 +1,270 @@ +// test-xss-333-pages-e2e.js +// +// Full-page XSS regression test for issue #333 round 2 — the adjacent sinks +// found in review of PR #373: +// F1 packets region badge in the flat view and in expanded group children +// F2 the shared region filter (pills, dropdown items, dropdown trigger) +// F3 observers-row navigation with an observer id that contains a quote +// +// Unlike test-xss-333-e2e.js (production sink fragments rendered on a blank +// page), this drives the REAL pages against a running Go server. The hostile +// values are injected by rewriting two API responses in the browser +// (/api/observers, /api/config/regions) with page.route — the fixture DB is +// untouched and no payload leaves the machine. Every injected element would +// run `window.__xss333++`, so the check is "did any handler fire" plus "is +// there any in the DOM", not a string match on the source. +// +// Requires a server with the CI-prepared e2e fixture (see deploy.yml): SJC +// observers exist, and the #1486 seed adds hash fae0c9e6d357a814 with three +// observations (observers OAK, SJC, OAK). +// +// Usage: BASE_URL=http://localhost:13581 node test-xss-333-pages-e2e.js +'use strict'; +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +const SEED_HASH = 'fae0c9e6d357a814'; + +// Breaks out of a double-quoted attribute and carries a tag for the text +// context; the trailing quote probes single-quoted contexts. +const HOSTILE = '">\''; +// Observer id that breaks out of a single-quoted JS string (F3). Only +// characters encodeURIComponent leaves alone (' ( ) - letters digits), so +// URL-encoding cannot neutralise it; a run shows up as an alert dialog. +const QUOTE_ID = "xss333'-alert(333)-'"; +const BENIGN_REGION = 'KØB & Æ'; + +let passed = 0, failed = 0; +async function step(name, fn) { + try { await fn(); passed++; console.log(' ✓ ' + name); } + catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); } +} +function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } + +// Rewrite /api/observers (SJC → HOSTILE, optional extra observers) and +// /api/config/regions (whatever `regions` returns for the real response). +async function hostileContext(browser, { regions, extraObservers = [], iataMap = { SJC: HOSTILE } }) { + const ctx = await browser.newContext({ viewport: { width: 1400, height: 900 } }); + await ctx.route((url) => url.pathname === '/api/observers', async (route) => { + const res = await route.fetch(); + const body = await res.json(); + for (const o of body.observers || []) if (o.iata in iataMap) o.iata = iataMap[o.iata]; + body.observers = (body.observers || []).concat(extraObservers); + await route.fulfill({ response: res, json: body }); + }); + await ctx.route((url) => url.pathname === '/api/config/regions', async (route) => { + const res = await route.fetch(); + await route.fulfill({ response: res, json: regions(await res.json()) }); + }); + const page = await ctx.newPage(); + page.setDefaultTimeout(15000); + page.on('pageerror', (e) => console.error(' [pageerror]', e.message)); + page._xssDialogs = 0; + page.on('dialog', (d) => { page._xssDialogs++; d.dismiss().catch(() => {}); }); + return { ctx, page }; +} + +// Real regions with SJC replaced by the hostile code (4 codes → pill mode). +const hostileRegions = (real) => { + const out = {}; + for (const k of Object.keys(real)) out[k === 'SJC' ? HOSTILE : k] = k === 'SJC' ? HOSTILE : real[k]; + return out; +}; + +// Count live injections anywhere in the document. +async function injections(page) { + const r = await page.evaluate(() => ({ + fired: window.__xss333 || 0, + imgs: document.querySelectorAll('img[src="x"]').length, + })); + r.dialogs = page._xssDialogs; + return r; +} +function assertInert(r, label) { + assert(r.fired === 0 && r.imgs === 0 && r.dialogs === 0, + `${label}: ${r.imgs} injected , ${r.fired} handler call(s), ${r.dialogs} dialog(s)`); +} + +(async () => { + const browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'], + }); + console.log(`\n=== #333 round 2 full-page XSS E2E against ${BASE} ===`); + + // ---- F1: packets flat view -------------------------------------------- + await step('F1 packets flat view: hostile observer IATA renders as badge text', async () => { + const { ctx, page } = await hostileContext(browser, { regions: (r) => r }); + try { + // 24 h window: the fixture is freshened to ~now before the run, and a + // 15-minute default would empty the list on a slow runner (cf. #180). + await page.goto(BASE + '/#/packets?timeWindow=1440', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#fGroup'); + await page.waitForSelector('#pktBody tr[data-hash]'); + await page.click('#fGroup'); // Group by Hash off → flat rows + await page.waitForFunction(() => !document.getElementById('fGroup').classList.contains('active')); + // Rows from SJC observers carry the hostile region; wait until the flat + // rows have rendered region cells, then judge the DOM. + await page.waitForFunction(() => document.querySelectorAll( + '#pktBody tr[data-action="select-hash"] td.col-region').length > 0); + await page.waitForTimeout(500); + assertInert(await injections(page), 'flat view'); + const r = await page.evaluate((h) => { + const rows = Array.from(document.querySelectorAll('#pktBody tr[data-action="select-hash"]')); + const badges = Array.from(document.querySelectorAll('#pktBody .badge-region')).map((b) => b.textContent); + return { rows: rows.length, hostile: badges.filter((t) => t === h).length, + groupHeaders: document.querySelectorAll('#pktBody tr.group-header').length }; + }, HOSTILE); + assert(r.groupHeaders === 0, 'expected the flat view, found group headers'); + assert(r.hostile > 0, 'no flat row shows the hostile region as text'); + } finally { await ctx.close(); } + }); + + // ---- F1: packets group child rows --------------------------------------- + await step('F1 packets expanded group: child-row region badges render as text', async () => { + const { ctx, page } = await hostileContext(browser, { regions: (r) => r }); + try { + await page.goto(BASE + '/#/packets?hash=' + SEED_HASH + '&timeWindow=0', { waitUntil: 'domcontentloaded' }); + const sel = `#pktBody tr[data-hash="${SEED_HASH}"][data-action="toggle-select"]`; + await page.waitForSelector(sel); + await page.click(sel + ' td.col-expand'); + await page.waitForFunction(() => document.querySelectorAll('#pktBody tr.group-child').length === 3); + const badges = await page.$$eval('#pktBody tr.group-child .badge-region', (els) => els.map((e) => e.textContent)); + assert(JSON.stringify(badges.slice().sort()) === JSON.stringify([HOSTILE, 'OAK', 'OAK'].sort()), + 'child badges must be OAK, SJC→hostile, OAK as text, got ' + JSON.stringify(badges)); + assertInert(await injections(page), 'group children'); + } finally { await ctx.close(); } + }); + + // ---- F2: region filter pills (Observers) + filtering still works -------- + await step('F2 region filter pills: hostile code inert, selecting it filters the observers table', async () => { + const { ctx, page } = await hostileContext(browser, { regions: hostileRegions }); + try { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsRegionFilter .region-pill'); + await page.waitForSelector('#obsTable tbody tr'); + const pills = await page.$$eval('#obsRegionFilter .region-pill', + (els) => els.map((e) => ({ region: e.dataset.region, text: e.textContent }))); + const hostilePill = pills.find((p) => p.region === HOSTILE); + assert(hostilePill, 'no pill carries the hostile code in data-region: ' + JSON.stringify(pills)); + assert(hostilePill.text === HOSTILE, 'hostile pill label is not the literal code: ' + hostilePill.text); + assertInert(await injections(page), 'region pills'); + + const total = await page.$$eval('#obsTable tbody tr[data-action="navigate"]', (els) => els.length); + await page.locator('#obsRegionFilter .region-pill').filter({ hasText: 'onerror' }).click(); + await page.waitForFunction((n) => document.querySelectorAll('#obsTable tbody tr[data-action="navigate"]').length < n, total); + const r = await page.evaluate(() => ({ + stored: localStorage.getItem('meshcore-region-filter'), + regions: Array.from(document.querySelectorAll('#obsTable tbody tr[data-action="navigate"] .badge-region')) + .map((b) => b.textContent), + active: Array.from(document.querySelectorAll('#obsRegionFilter .region-pill-active')).map((e) => e.dataset.region), + })); + assert(r.stored === JSON.stringify([HOSTILE]), 'persisted selection is not the exact code: ' + r.stored); + assert(r.regions.length > 0 && r.regions.every((t) => t === HOSTILE), + 'filtered table must only show the hostile region: ' + JSON.stringify(r.regions)); + assert(JSON.stringify(r.active) === JSON.stringify([HOSTILE]), 'active pill: ' + JSON.stringify(r.active)); + assertInert(await injections(page), 'region pills after selection'); + } finally { await ctx.close(); } + }); + + // ---- F2: region filter dropdown (Observers, 5 regions) ----------------- + await step('F2 region filter dropdown: items + trigger label inert, selection filters', async () => { + const { ctx, page } = await hostileContext(browser, { + regions: (real) => Object.assign(hostileRegions(real), { [BENIGN_REGION]: BENIGN_REGION }), + }); + try { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsRegionFilter .region-dropdown-trigger'); + await page.waitForSelector('#obsTable tbody tr'); + await page.click('#obsRegionFilter .region-dropdown-trigger'); + const items = await page.$$eval('#obsRegionFilter .region-dropdown-item', + (els) => els.map((e) => ({ region: e.querySelector('input').dataset.region, text: e.textContent.trim() }))); + const h = items.find((i) => i.region === HOSTILE); + assert(h && h.text === HOSTILE + ' - ' + HOSTILE, 'hostile dropdown item: ' + JSON.stringify(items)); + const b = items.find((i) => i.region === BENIGN_REGION); + assert(b && b.text === BENIGN_REGION + ' - ' + BENIGN_REGION, 'benign item mangled: ' + JSON.stringify(b)); + assertInert(await injections(page), 'region dropdown'); + + await page.evaluate((code) => { + const input = Array.from(document.querySelectorAll('#obsRegionFilter input[data-region]')) + .find((i) => i.dataset.region === code); + input.click(); + }, HOSTILE); + await page.waitForFunction(() => document.querySelector('#obsRegionFilter .region-dropdown-trigger') + .textContent !== 'All Regions ▾'); + const label = await page.$eval('#obsRegionFilter .region-dropdown-trigger', (e) => e.textContent); + assert(label === HOSTILE + ' ▾', 'trigger label is not the literal code: ' + JSON.stringify(label)); + const regions = await page.$$eval('#obsTable tbody tr[data-action="navigate"] .badge-region', (els) => els.map((e) => e.textContent)); + assert(regions.length > 0 && regions.every((t) => t === HOSTILE), + 'filtered table must only show the hostile region: ' + JSON.stringify(regions)); + assertInert(await injections(page), 'region dropdown trigger label'); + } finally { await ctx.close(); } + }); + + // ---- F2: the filter on the other pages that render it ------------------ + for (const [hash, sel] of [['#/packets', '#packetsRegionFilter'], ['#/nodes', '#nodesRegionFilter'], ['#/analytics', '#analyticsRegionFilter']]) { + await step(`F2 region filter on ${hash}: hostile code inert`, async () => { + const { ctx, page } = await hostileContext(browser, { regions: hostileRegions }); + try { + await page.goto(BASE + '/' + hash, { waitUntil: 'domcontentloaded' }); + await page.waitForFunction((s) => { + const el = document.querySelector(s); + return el && Array.from(el.querySelectorAll('[data-region]')).length >= 4; + }, sel); + const codes = await page.$$eval(sel + ' [data-region]', (els) => els.map((e) => e.dataset.region)); + assert(codes.includes(HOSTILE), hash + ': hostile code lost from data-region: ' + JSON.stringify(codes)); + assertInert(await injections(page), hash + ' region filter'); + } finally { await ctx.close(); } + }); + } + + // ---- F3: observers row with a quote in the id -------------------------- + await step('F3 observers row: id with a quote navigates without running code', async () => { + const { ctx, page } = await hostileContext(browser, { + regions: (r) => r, + extraObservers: [{ id: QUOTE_ID, name: 'xss333 quote', iata: 'SJC', last_seen: new Date().toISOString() }], + }); + try { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsTable tbody tr'); + const idx = await page.$$eval('#obsTable tbody tr[data-action="navigate"]', + (els, id) => els.findIndex((e) => e.dataset.observerId === id), QUOTE_ID); + assert(idx >= 0, 'synthetic observer row not rendered'); + await page.locator('#obsTable tbody tr[data-action="navigate"]').nth(idx).locator('[data-testid="obs-cell-name"]').click(); + await page.waitForFunction((want) => location.hash === want, '#/observers/' + encodeURIComponent(QUOTE_ID)); + assertInert(await injections(page), 'observer row click'); + } finally { await ctx.close(); } + }); + + await step('F3 observers row: plain id still navigates to the detail page', async () => { + const { ctx, page } = await hostileContext(browser, { regions: (r) => r }); + try { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsTable tbody tr[data-action="navigate"]'); + const id = await page.$eval('#obsTable tbody tr[data-action="navigate"]', (e) => e.dataset.observerId); + await page.locator('#obsTable tbody tr[data-action="navigate"]').first().locator('[data-testid="obs-cell-name"]').click(); + await page.waitForFunction((want) => location.hash === want, '#/observers/' + encodeURIComponent(id)); + } finally { await ctx.close(); } + }); + + // The inline onclick also navigated on narrow viewports, where the + // delegated handler is meant to open the slide-over INSTEAD (#1056 AC#4). + await step('F3 observers row on a narrow viewport: opens the slide-over, list stays', async () => { + const ctx = await browser.newContext({ viewport: { width: 500, height: 900 } }); + try { + const page = await ctx.newPage(); + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsTable tbody tr[data-action="navigate"]'); + await page.locator('#obsTable tbody tr[data-action="navigate"]').first().locator('[data-testid="obs-cell-name"]').click(); + await page.waitForFunction(() => window.SlideOver && window.SlideOver.isOpen()); + await page.waitForTimeout(500); // a stray navigation would land by now + const hash = await page.evaluate(() => location.hash); + assert(hash === '#/observers', 'narrow row click navigated away: ' + hash); + } finally { await ctx.close(); } + }); + + await browser.close(); + console.log('\n' + passed + ' passed, ' + failed + ' failed'); + process.exit(failed ? 1 : 0); +})().catch((e) => { console.error(e); process.exit(1); }); diff --git a/test-xss-escape-sinks.js b/test-xss-escape-sinks.js index 26f38f80d..8ce791324 100644 --- a/test-xss-escape-sinks.js +++ b/test-xss-escape-sinks.js @@ -803,10 +803,218 @@ test('#333 unknown-route template uses a CSS var, not a hardcoded hex colour', ( '#333 unknown-route div does not use a CSS variable for colour: ' + m[0]); }); +// ========================================================================= +// J. Issue #333 round 2 — adjacent sinks found in review: packet region +// badges in group child rows and the flat view (F1), the shared region +// filter (F2) and the observers-row navigation (F3). +// +// These drive the REAL render functions (packets.js buildGroupRowHtml / +// buildFlatRowHtml, region-filter.js render) in a vm sandbox with app.js's +// real escapeHtml as the page global, so a missing wrapper at any call site +// turns the test red — no template is lifted by regex. +// ========================================================================= +const asyncTests = []; +function asyncTest(name, fn) { asyncTests.push({ name, fn }); } + +// Hostile region code: breaks out of a double-quoted attribute AND carries a +// tag for the text context; the single quote probes single-quoted contexts. +const REGION_PAYLOAD = '">' + ATTR_PAYLOAD; +const BENIGN_REGION = 'KØB & Æ'; + +function decodeEntities(s) { + return s.replace(/</g, '<').replace(/>/g, '>').replace(/"/g, '"') + .replace(/'/g, "'").replace(/&/g, '&'); +} + +// Real public/packets.js in a sandbox (same shape as +// test-issue-165-hop-resolution-per-observer.js). `observers` is what +// /api/observers returns; resolveHops() seeds the page's observerMap from it. +async function loadPacketsWithObservers(observers) { + const store = {}; + const ctx = { + window: { addEventListener() {}, dispatchEvent() {}, innerWidth: 1280 }, + document: { + readyState: 'complete', + createElement: () => ({ style: {}, dataset: {}, classList: { add() {}, remove() {} }, appendChild() {}, setAttribute() {} }), + head: { appendChild() {} }, body: { appendChild() {}, removeChild() {} }, + getElementById: () => null, querySelector: () => null, querySelectorAll: () => [], + addEventListener() {}, removeEventListener() {}, + }, + localStorage: { getItem: k => (k in store ? store[k] : null), setItem: (k, v) => { store[k] = String(v); }, removeItem: k => { delete store[k]; } }, + console, Math, Object, Array, Number, Date, Map, Set, JSON, String, Boolean, RegExp, Error, TypeError, Promise, + parseInt, parseFloat, isNaN, isFinite, encodeURIComponent, decodeURIComponent, URLSearchParams, + setTimeout, clearTimeout, setInterval: () => 0, clearInterval() {}, + requestAnimationFrame: () => 0, cancelAnimationFrame() {}, + performance: { now: () => Date.now() }, + location: { hash: '' }, + fetch: () => Promise.resolve({ ok: true, json: () => Promise.resolve({}) }), + registerPage() {}, onWS() {}, offWS() {}, debouncedOnWS: fn => fn, + CLIENT_TTL: {}, + fetchAllNodes: () => Promise.resolve({ nodes: [] }), + api: (p) => Promise.resolve(p.startsWith('/observers') ? { observers } : p.startsWith('/iata-coords') ? { coords: {} } : {}), + invalidateApiCache() {}, + }; + ctx.window.localStorage = ctx.localStorage; + vm.createContext(ctx); + const load = (f) => { + vm.runInContext(fs.readFileSync('public/' + f, 'utf8'), ctx, { filename: f }); + for (const k of Object.keys(ctx.window)) ctx[k] = ctx.window[k]; + }; + // roles.js + app.js supply the page globals the row builders call + // (payloadTypeName, …); app.js's own escapeHtml becomes the global. + for (const f of ['payload-labels.js', 'roles.js', 'app.js']) load(f); + ctx.api = (p) => Promise.resolve(p.startsWith('/observers') ? { observers } : p.startsWith('/iata-coords') ? { coords: {} } : {}); + ctx.fetchAllNodes = () => Promise.resolve({ nodes: [] }); + for (const f of ['packet-helpers.js', 'hop-resolver.js', 'hop-display.js', 'packets.js']) load(f); + const T = ctx.window._packetsTestAPI; + await T.resolveHops(['ef'], observers[0].id); // seeds observerMap via ensureHopResolver + return T; +} + +// Every .badge-region in a rendered row, decoded the way the browser would. +function regionBadges(html) { + return Array.from(html.matchAll(/([^<]*)<\/span>/g), m => decodeEntities(m[1])); +} + +const RAW_HEX = '15000102030405060708090a0b0c0d0e0f'; +function pkt(extra) { + return Object.assign({ hash: 'fae0c9e6d357a814', raw_hex: RAW_HEX, payload_type: 5, route_type: 1, + decoded_json: '{}', path_json: '[]', timestamp: '', latest: '' }, extra); +} + +asyncTest('#333 F1 packets flat view: region badge escapes the observer IATA', async () => { + const T = await loadPacketsWithObservers([{ id: 'OBS-H', name: 'h', iata: REGION_PAYLOAD }]); + const html = T.buildFlatRowHtml(pkt({ id: 7, observer_id: 'OBS-H' })); + assertNoXss(html, '#333 F1 flat view region'); + assert.deepStrictEqual(regionBadges(html), [REGION_PAYLOAD], 'flat view badge must show the code as text'); +}); + +asyncTest('#333 F1 packets group child rows: region badge escapes the observer IATA', async () => { + const T = await loadPacketsWithObservers([ + { id: 'OBS-B', name: 'b', iata: 'SJC' }, + { id: 'OBS-H', name: 'h', iata: REGION_PAYLOAD }, + ]); + const p = pkt({ observer_id: 'OBS-B', count: 2, observation_count: 2, observer_count: 2, + _children: [pkt({ id: 1, observer_id: 'OBS-B' }), pkt({ id: 2, observer_id: 'OBS-H' })] }); + T._setExpanded(p.hash, true); + const html = T.buildGroupRowHtml(p); + assert.ok(html.includes('class="group-child"'), 'expanded group must render child rows'); + assertNoXss(html, '#333 F1 group child region'); + assert.deepStrictEqual(regionBadges(html), ['SJC', 'SJC', REGION_PAYLOAD], + 'header + both child badges must show their codes as text'); +}); + +asyncTest('#333 F1 packets group header: region badge escapes the observer IATA', async () => { + const T = await loadPacketsWithObservers([{ id: 'OBS-H', name: 'h', iata: REGION_PAYLOAD }]); + const html = T.buildGroupRowHtml(pkt({ observer_id: 'OBS-H', count: 1, observation_count: 1 })); + assertNoXss(html, '#333 F1 group header region'); + assert.deepStrictEqual(regionBadges(html), [REGION_PAYLOAD]); +}); + +asyncTest('#333 F1 packets region badges show a benign code verbatim (no double escape)', async () => { + const T = await loadPacketsWithObservers([{ id: 'OBS-K', name: 'k', iata: BENIGN_REGION }]); + const flat = T.buildFlatRowHtml(pkt({ id: 3, observer_id: 'OBS-K' })); + assert.ok(flat.includes('KØB & Æ'), flat); + assert.deepStrictEqual(regionBadges(flat), [BENIGN_REGION]); +}); + +// Real public/region-filter.js in a sandbox. `regions` is what +// /api/config/regions returns; `stored` pre-seeds the persisted selection. +async function loadRegionFilter(regions, opts) { + const store = {}; + if (opts && opts.stored) store['meshcore-region-filter'] = JSON.stringify(opts.stored); + const el = () => ({ hidden: true, onclick: null, setAttribute() {} }); + const container = { innerHTML: '', style: {}, onclick: null, querySelector: el }; + const ctx = { + window: {}, + document: { addEventListener() {}, removeEventListener() {} }, + localStorage: { getItem: k => (k in store ? store[k] : null), setItem: (k, v) => { store[k] = String(v); }, removeItem: k => { delete store[k]; } }, + fetch: async () => ({ json: async () => regions }), + console, setTimeout, clearTimeout, + escapeHtml, // app.js's real global escaper + }; + ctx.window = ctx; + vm.createContext(ctx); + vm.runInContext(fs.readFileSync('public/region-filter.js', 'utf8'), ctx, { filename: 'region-filter.js' }); + await ctx.RegionFilter.init(container, opts && opts.dropdown ? { dropdown: true } : undefined); + return { RF: ctx.RegionFilter, container, store }; +} + +// The value the browser would hand to toggleRegion via el.dataset.region. +function dataRegions(html) { + return Array.from(html.matchAll(/data-region="([^"]*)"/g), m => decodeEntities(m[1])); +} + +asyncTest('#333 F2 region filter pills: hostile code escaped in data-region and label', async () => { + const { container } = await loadRegionFilter({ SJC: 'San Jose', [REGION_PAYLOAD]: REGION_PAYLOAD }); + const html = container.innerHTML; + assert.ok(html.includes('region-pill'), 'expected pill mode for 2 regions: ' + html); + assertNoXss(html, '#333 F2 region pills'); + assert.deepStrictEqual(dataRegions(html), ['__all__', 'SJC', REGION_PAYLOAD], + 'each pill data-region must round-trip to its code'); + assert.ok(decodeEntities(html).includes('>' + REGION_PAYLOAD + ''), 'pill label must show the code as text'); +}); + +asyncTest('#333 F2 region filter dropdown: hostile code + label escaped, trigger label escaped', async () => { + const { container } = await loadRegionFilter( + { SJC: 'San Jose', [REGION_PAYLOAD]: REGION_PAYLOAD + ' city' }, + { dropdown: true, stored: [REGION_PAYLOAD] }); + const html = container.innerHTML; + assert.ok(html.includes('region-dropdown-trigger'), 'expected dropdown mode: ' + html); + assertNoXss(html, '#333 F2 region dropdown'); + assert.deepStrictEqual(dataRegions(html), ['__all__', 'SJC', REGION_PAYLOAD]); + const trigger = html.match(/'), container.innerHTML); + assert.deepStrictEqual(dataRegions(container.innerHTML), ['__all__', 'SJC', BENIGN_REGION]); +}); + +// F3: observers row — an observer id containing ' must not reach JS code. +test('#333 F3 observers row: no inline handler interpolates the observer id', () => { + const src = fs.readFileSync('public/observers.js', 'utf8'); + const m = src.match(/return `(]*data-action="navigate"[^>]*>)/); + assert.ok(m, '#333 F3 observers navigate row not found'); + const fn = new Function('o', 'escapeHtml', 'return `' + m[1] + '`;'); + const id = "Q'-console.error(1)-'"; + const tag = fn({ id }, escapeHtml); + const handlers = Array.from(tag.matchAll(/\son[a-z]+="([^"]*)"/gi), x => x[1]); + for (const h of handlers) { + assert.ok(!h.includes(id) && !h.includes(encodeURIComponent(id)), + '#333 F3: observer id reaches an inline JS handler: ' + tag); + } + // Navigation target stays on the row for the delegated handler. + const dv = tag.match(/data-value="([^"]*)"/); + assert.ok(dv && decodeEntities(dv[1]) === '#/observers/' + encodeURIComponent(id), + '#333 F3: data-value navigation target lost: ' + tag); +}); + // ========================================================================= // SUMMARY // ========================================================================= -console.log('\n' + '═'.repeat(48)); -console.log(` XSS escape sinks: ${passed} passed, ${failed} failed`); -console.log('═'.repeat(48)); -if (failed > 0) process.exit(1); +(async () => { + for (const t of asyncTests) { + try { await t.fn(); passed++; console.log(` ✅ ${t.name}`); } + catch (e) { failed++; console.log(` ❌ ${t.name}: ${e.message}`); } + } + console.log('\n' + '═'.repeat(48)); + console.log(` XSS escape sinks: ${passed} passed, ${failed} failed`); + console.log('═'.repeat(48)); + if (failed > 0) process.exit(1); +})(); From 8401fa822a744a8066abf77171b35b61f589006c Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 12 Oct 2026 21:23:29 +0000 Subject: [PATCH 4/9] fix(packets): escape the region badge in group child rows and the flat view (#333) Review of PR #373, finding F1. The group-header badge was escaped in round 1, but the child rows (childRegion) and the flat view (region) put the same observer IATA into the same sink raw. Wrap both in escapeHtml, as the header does. Co-Authored-By: Claude Opus 5.5 --- public/packets.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/packets.js b/public/packets.js index 348926bfb..5e17c1d06 100644 --- a/public/packets.js +++ b/public/packets.js @@ -2727,7 +2727,7 @@ const childPathStr = renderPath(childPath, c.observer_id, { summary: true }); const _childHashStripe = _hashStripeStyle(c.hash || p.hash); html += ` - ${childRegion ? `${childRegion}` : '—'} + ${childRegion ? `${escapeHtml(childRegion)}` : '—'} ${renderTimestampCell(c.timestamp)} ${truncate(c.hash || '', 8)} ${size}B @@ -2764,7 +2764,7 @@ const _flatHashStripe = _hashStripeStyle(p.hash); const _flatStyle = _flatHashStripe + _chanStyle; return ` - ${region ? `${region}` : '—'} + ${region ? `${escapeHtml(region)}` : '—'} ${renderTimestampCell(p.timestamp)} ${truncate(p.hash || String(p.id), 8)} ${size}B From d4bdbfd0a0a0a796b8a2ee9e887772bd1401ebd6 Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 12 Oct 2026 21:23:29 +0000 Subject: [PATCH 5/9] fix(region-filter): escape observer region codes and labels (#333) Review of PR #373, finding F2. Region codes include observer IATA values from MQTT and were concatenated raw into data-region attributes, the pill/dropdown label text and the dropdown trigger label, rendering on page load on Observers, Packets, Nodes, Analytics, Channels and Live. Escape each with the global escapeHtml (5-char OWASP set, so it is safe in the double-quoted attributes too). Selection keeps working because toggleRegion() reads btn.dataset.region, which the parser has already decoded back to the exact code. The setSelected sandbox in test-frontend-helpers.js now loads roles.js + app.js like the page does, so the global escaper exists there. Co-Authored-By: Claude Opus 5.5 --- public/region-filter.js | 14 +++++++++----- test-frontend-helpers.js | 4 ++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/public/region-filter.js b/public/region-filter.js index 69f88fc7d..fe2d5557e 100644 --- a/public/region-filter.js +++ b/public/region-filter.js @@ -86,12 +86,16 @@ _listeners.forEach(function (fn) { fn(getSelected()); }); } - /** Build summary label for dropdown trigger */ + // #333: region codes include observer IATA values from MQTT, so every code + // and label is escaped for its attribute or text slot. dataset.region hands + // toggleRegion() the decoded code, so selection still matches _regions. + + /** Build summary label for dropdown trigger (HTML-escaped) */ function dropdownLabel(codes) { if (!_selected) return 'All Regions'; var sel = Array.from(_selected); if (sel.length === 0) return 'All Regions'; - if (sel.length <= 2) return sel.join(', '); + if (sel.length <= 2) return escapeHtml(sel.join(', ')); return sel.length + ' Regions'; } @@ -106,7 +110,7 @@ var label = _regions[code] || code; var active = allSelected || (_selected && _selected.has(code)); html += ''; + '" data-region="' + escapeHtml(code) + '" role="checkbox" aria-checked="' + !!active + '">' + escapeHtml(label) + ''; }); html += '

'; container.innerHTML = html; @@ -132,8 +136,8 @@ var cityName = configLabel || (window.IATA_CITIES && window.IATA_CITIES[code]); var label = cityName ? (code + ' - ' + cityName) : code; var active = allSelected || (_selected && _selected.has(code)); - html += ''; + html += ''; }); html += '
'; container.innerHTML = html; diff --git a/test-frontend-helpers.js b/test-frontend-helpers.js index 37598b44c..0df1b3319 100644 --- a/test-frontend-helpers.js +++ b/test-frontend-helpers.js @@ -5734,6 +5734,10 @@ console.log('\n=== region-filter.js: setSelected ==='); removeEventListener: () => {}, }); + // region-filter.js escapes codes/labels with app.js's global escapeHtml + // (#333), as on the page, where app.js is always loaded. + loadInCtx(ctx, 'public/roles.js'); + loadInCtx(ctx, 'public/app.js'); loadInCtx(ctx, 'public/area-filter.js'); loadInCtx(ctx, 'public/region-filter.js'); From 4eb76dcce1d1e59921795519e9f421729d02854b Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 12 Oct 2026 21:23:29 +0000 Subject: [PATCH 6/9] fix(observers): drop the inline onclick on observer rows (#333) Review of PR #373, finding F3. The row carried onclick="location.hash='#/observers/${encodeURIComponent(o.id)}'". encodeURIComponent does not encode ', so an observer id containing a quote broke out of the JS string and ran (observer ids come from the MQTT topic). The row already has data-action="navigate" with the same target in data-value, handled by the delegated click/keydown listeners, so the inline handler is removed. This also restores the #1056 behaviour on narrow viewports: the inline handler navigated to the detail page even when the delegated handler opened the slide-over instead. Co-Authored-By: Claude Opus 5.5 --- public/observers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/observers.js b/public/observers.js index 5a324b3d1..da225ec40 100644 --- a/public/observers.js +++ b/public/observers.js @@ -424,7 +424,7 @@ window.preserveCompareSelection = function preserveCompareSelection(prevIds, tbo const _packetCount = (o.packet_count != null) ? o.packet_count : ''; const _packetsHour = (o.packetsLastHour != null) ? o.packetsLastHour : ''; const _neighborsReportMs = o.last_neighbors_report_at ? new Date(o.last_neighbors_report_at).getTime() : ''; - return ` + return ` ${shapeIcon} ${h.label} ${escapeHtml(o.name || o.id)}${window.ObserversNaiveChip.render(o)}${o.can_relay === false ? ' listener' : (o.can_relay === true ? ' repeater' : '')} ${o.iata ? `${escapeHtml(o.iata)}` : '—'} From ed9c45c15653f7afcdfc9b187aa7e352f16792d6 Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 12 Oct 2026 21:23:43 +0000 Subject: [PATCH 7/9] test(xss-gate): reproduce the concat-into-attribute and URL-in-string misses (#333) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of PR #373, finding F4. New fixtures for scripts/check-xss-sinks.sh: - bad-15: el.innerHTML = '' passes the gate: the "…" pass takes `"' + o.name + '"` for one string and erases the field. - bad-16: the mirror with "…" pieces around a '…' attribute (already flagged; pinned). - bad-17: '' + o.name passes: `//` inside the string is stripped as a comment before strings are parsed. - good-6: the escaped forms must stay clean. - good-7: a plain '…${name}…' literal is flagged by the round-1 N4 workaround although it is not an interpolation. Red on this commit (bad-15, bad-17, good-7); the gate fix follows. Co-Authored-By: Claude Opus 5.5 --- test-preflight-xss-gate.js | 10 ++++++++++ testdata/preflight-xss/bad-15-attr-concat.js | 10 ++++++++++ .../bad-16-attr-concat-doublequote-pieces.js | 8 ++++++++ testdata/preflight-xss/bad-17-url-in-string.js | 9 +++++++++ testdata/preflight-xss/good-6-attr-concat-escaped.js | 12 ++++++++++++ .../preflight-xss/good-7-literal-dollar-brace.js | 7 +++++++ 6 files changed, 56 insertions(+) create mode 100644 testdata/preflight-xss/bad-15-attr-concat.js create mode 100644 testdata/preflight-xss/bad-16-attr-concat-doublequote-pieces.js create mode 100644 testdata/preflight-xss/bad-17-url-in-string.js create mode 100644 testdata/preflight-xss/good-6-attr-concat-escaped.js create mode 100644 testdata/preflight-xss/good-7-literal-dollar-brace.js diff --git a/test-preflight-xss-gate.js b/test-preflight-xss-gate.js index ae2076c06..9bb22734b 100644 --- a/test-preflight-xss-gate.js +++ b/test-preflight-xss-gate.js @@ -55,6 +55,12 @@ const cases = [ label: 'innerHTML `…title="${observer_name}"…` (N4: interp in quoted attr)' }, { file: 'bad-14-attr-interp-singlequote.js', expect: 1, label: "innerHTML `…title='${observer_name}'…` (N4: single-quoted attr)" }, + { file: 'bad-15-attr-concat.js', expect: 1, + label: `innerHTML '…title="' + o.name + '"…' (F4: concat into quoted attr)` }, + { file: 'bad-16-attr-concat-doublequote-pieces.js', expect: 1, + label: "innerHTML \"…title='\" + observer_name + \"'…\" (F4: mirror of bad-15)" }, + { file: 'bad-17-url-in-string.js', expect: 1, + label: `innerHTML '' + o.name (F4: // inside a string is not a comment)` }, // good fixtures: gate MUST pass (exit 0) { file: 'good-1-escaped.js', expect: 0, label: 'escapeHtml(${name}) wrapper' }, { file: 'good-2-tested.js', expect: 0, label: 'unescaped but DOM-grep-tested in same PR', @@ -68,6 +74,10 @@ const cases = [ tests: ['test-good-4.js'] }, { file: 'good-5-attr-interp-escaped.js', expect: 0, label: 'escapeHtml(${observer_name}) inside a quoted attr (N4 fix no-FP)' }, + { file: 'good-6-attr-concat-escaped.js', expect: 0, + label: 'escaped concat into quoted attrs + URL string (F4 no-FP)' }, + { file: 'good-7-literal-dollar-brace.js', expect: 0, + label: "plain '…${name}…' string literal is not an interpolation (F4 no-FP)" }, ]; let failed = 0; diff --git a/testdata/preflight-xss/bad-15-attr-concat.js b/testdata/preflight-xss/bad-15-attr-concat.js new file mode 100644 index 000000000..5ba37ea38 --- /dev/null +++ b/testdata/preflight-xss/bad-15-attr-concat.js @@ -0,0 +1,10 @@ +// bad-15-attr-concat.js — XSS fixture for check-xss-sinks (#333 F4). +// The concatenation form of N4: a node-controlled field concatenated into a +// double-quoted attribute built from single-quoted string pieces. The old +// quote-stripper ran its "…" pass first, so `"' + o.name + '"` looked like +// one double-quoted string and the field was erased before the audit. +// EXPECTED: flagged by check-xss-sinks. +/* eslint-disable */ +function render(el, o) { + el.innerHTML = 'x'; +} diff --git a/testdata/preflight-xss/bad-16-attr-concat-doublequote-pieces.js b/testdata/preflight-xss/bad-16-attr-concat-doublequote-pieces.js new file mode 100644 index 000000000..bc9eb69b2 --- /dev/null +++ b/testdata/preflight-xss/bad-16-attr-concat-doublequote-pieces.js @@ -0,0 +1,8 @@ +// bad-16-attr-concat-doublequote-pieces.js — XSS fixture (#333 F4). +// Mirror of bad-15: double-quoted string pieces around a single-quoted +// attribute. The '…' pass used to swallow `' + o.observer_name + "'`. +// EXPECTED: flagged by check-xss-sinks. +/* eslint-disable */ +function render(el, o) { + el.innerHTML = "x"; +} diff --git a/testdata/preflight-xss/bad-17-url-in-string.js b/testdata/preflight-xss/bad-17-url-in-string.js new file mode 100644 index 000000000..6281ee98a --- /dev/null +++ b/testdata/preflight-xss/bad-17-url-in-string.js @@ -0,0 +1,9 @@ +// bad-17-url-in-string.js — XSS fixture (#333 F4). +// A `//` inside a string literal (a URL) is not a comment. The old stripper +// removed `//…` before it parsed strings, cutting the line at `https:` and +// dropping the unescaped node field that followed. +// EXPECTED: flagged by check-xss-sinks. +/* eslint-disable */ +function render(el, o) { + el.innerHTML = '' + o.name + ''; +} diff --git a/testdata/preflight-xss/good-6-attr-concat-escaped.js b/testdata/preflight-xss/good-6-attr-concat-escaped.js new file mode 100644 index 000000000..487a8fbe5 --- /dev/null +++ b/testdata/preflight-xss/good-6-attr-concat-escaped.js @@ -0,0 +1,12 @@ +// good-6-attr-concat-escaped.js — the escaped counterpart of bad-15/bad-16. +// The left-to-right string parse must NOT false-positive on these. +/* eslint-disable */ +function escapeHtml(s) { + if (s == null) return ''; + return String(s).replace(/&/g,'&').replace(//g,'>').replace(/"/g,'"').replace(/'/g,'''); +} +function render(el, o) { + el.innerHTML = 'x'; + el.innerHTML = "x"; + el.innerHTML = '' + escapeHtml(o.name) + ''; +} diff --git a/testdata/preflight-xss/good-7-literal-dollar-brace.js b/testdata/preflight-xss/good-7-literal-dollar-brace.js new file mode 100644 index 000000000..3d8d1009e --- /dev/null +++ b/testdata/preflight-xss/good-7-literal-dollar-brace.js @@ -0,0 +1,7 @@ +// good-7-literal-dollar-brace.js — a plain quoted string whose TEXT contains +// "${name}" is not an interpolation. The round-1 N4 workaround preserved such +// literals and could flag them; the left-to-right parse does not. +/* eslint-disable */ +function render(el) { + el.innerHTML = '${name} is a template placeholder'; +} From a03151319d41a3077d3fee56c9432ce1b16b617d Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 12 Oct 2026 21:23:44 +0000 Subject: [PATCH 8/9] fix(xss-gate): parse strings left to right; document the same-line limit (#333) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of PR #373, finding F4. strip() erased "…" runs, then '…' runs, then // to end of line, each across the whole line, so one quote kind could swallow code between strings of the other kind and a URL was cut at `//`. It now walks the line once, recognising '…', "…", `…` and // comments in order. A quote inside a string or template is then just text, so the round-1 workaround that kept ${…} inside "…" runs is gone, along with its false positive on plain '…${x}…' literals. Measured on every public/*.js: the gate reports the same 3 pre-existing flags (bottom-nav.js x2, nav-drawer.js) before and after. The gate still only audits the line that holds the sink. Every #333 sink builds HTML on another line (row builders returning strings), so it cannot catch those mutants; the DOM-level tests do. Flagging every HTML-building line was measured at 224 hits on public/*.js, nearly all pre-escaped locals or static text, so it is not enabled. The script header documents this. Co-Authored-By: Claude Opus 5.5 --- scripts/check-xss-sinks.sh | 82 ++++++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 26 deletions(-) diff --git a/scripts/check-xss-sinks.sh b/scripts/check-xss-sinks.sh index e82892869..7211e4894 100755 --- a/scripts/check-xss-sinks.sh +++ b/scripts/check-xss-sinks.sh @@ -13,17 +13,25 @@ # We accept these as residual risk in exchange for a regex-only gate that # runs in <5s on every PR and surfaces actionable file:line evidence. # -# #351 N4 / #333: the gate now flags an unescaped node-controlled ${…} -# interpolation inside a QUOTED ATTRIBUTE of a template literal on a sink -# line (e.g. `el.innerHTML = \`\``), which -# previously slipped through because the quote-stripper erased the -# interpolation before the template-literal handler ran (see short_str -# below). Residual limitation: a plain (non-template) double/single-quoted -# string that contains a LITERAL "${allowlisted}" sequence on a sink line -# is now also preserved and could be flagged; such literals are rare and a -# sink-line false positive is cheap to wrap or opt out. The cross-line -# `grid.innerHTML = cards.join('')` indirection (O9) is still NOT covered — -# that needs the sink and the interpolation on the same line. +# #351 N4 / #333: the gate flags an unescaped node-controlled field inside +# a QUOTED ATTRIBUTE on a sink line, in both the template form +# (`el.innerHTML = \`\``) and the concat +# form (`el.innerHTML = ''`), because strings, +# templates and // comments are parsed in one left-to-right pass (see +# strip() below). +# +# KNOWN LIMITATION — same-line sinks only. The gate audits the line that +# holds the sink (innerHTML = …, insertAdjacentHTML(…), bindPopup(…), …). +# HTML built on OTHER lines and assigned later is not seen, e.g. +# function row(o) { return '' + o.iata + ''; } // not audited +# tbody.innerHTML = rows.map(row).join(''); // no field here +# This is how every #333 sink was written (packets.js / observers.js / +# region-filter.js row builders), so the gate does not catch those +# mutants; the DOM-level tests (test-xss-escape-sinks.js, +# test-xss-333-*e2e.js) do. Flagging every line that builds an HTML +# fragment was measured on public/*.js (#333 round 2): 224 lines, nearly +# all pre-escaped locals or static text, i.e. false positives, so it is +# not enabled. Closing this gap needs data flow, not a line regex. # # Two modes: # $0 --file Scan a single file. Exit 1 if any flagged sink @@ -207,26 +215,48 @@ def sink_match(line): # Comment/string strip — preserves short identifier-like string contents # (so setAttribute('href', ...) keeps its 'href' marker) and template # literal ${...} interpolations (so the RHS audit can still see node IDs). +# +# #333 F4: strings, template literals and // comments are recognised in ONE +# left-to-right pass. The earlier version erased "…" runs, then '…' runs, +# then // to end of line, each over the whole line, so a run of the other +# quote kind could swallow code: in +# el.innerHTML = '' +# the "…" pass took `"' + o.name + '"` for one string and erased the field, +# and a URL such as 'https://…' was cut at `//` as if it were a comment. +# Scanning in order means a quote inside a string or a template is just +# text, so the round-1 N4 workaround (keeping ${…} found inside "…" runs) is +# no longer needed, and a plain '…${x}…' literal is no longer mistaken for +# an interpolation. def strip(line): line = re.sub(r"/\*.*?\*/", "", line) - line = re.sub(r"//[^\n]*", "", line) - def short_str(m, q): - body = m.group(1) - # #351 N4 / #333: a ${...} interpolation that lands inside a quoted - # attribute of a template literal (e.g. ``) - # is seen by THIS quote-stripper before the template-literal handler - # below, because the attribute value is a double/single-quoted run. - # Without preserving the interpolation the node field is erased and - # the sink passes unflagged. Keep the ${...} fragments so the RHS - # audit still sees the node identifier (escaped ones peel to nothing). - interps = re.findall(r"\$\{[^}]*\}", body) - if interps: - return q + "".join(interps) + q + def short_str(body, q): if len(body) <= 32 and re.fullmatch(r"[A-Za-z0-9_:\-/.]+", body): return q + body + q return q + q - line = re.sub(r"\"((?:[^\"\\]|\\.)*)\"", lambda m: short_str(m, '"'), line) - line = re.sub(r"'((?:[^'\\]|\\.)*)'", lambda m: short_str(m, "'"), line) + def scan_to(i, q): + # Index of the closing quote q for a literal opened just before i, + # skipping backslash escapes; len(line) if it is not closed here. + while i < len(line) and line[i] != q: + i += 2 if line[i] == "\\" else 1 + return i + out = [] + i = 0 + while i < len(line): + c = line[i] + if c == "/" and line.startswith("//", i): + break # a real comment: we are outside every literal here + if c == "`": + j = scan_to(i + 1, "`") + out.append(line[i:j + 1]) # left whole for tpl() below + i = j + 1 + elif c in "'\"": + j = scan_to(i + 1, c) + out.append(short_str(line[i + 1:j], c)) + i = j + 1 + else: + out.append(c) + i += 1 + line = "".join(out) def tpl(m): body = m.group(1) parts = re.findall(r"\$\{[^}]*\}", body) From 8474b4947b21fafdd582e7290b0ba8d90527d39a Mon Sep 17 00:00:00 2001 From: dborup Date: Tue, 13 Oct 2026 01:25:38 +0200 Subject: [PATCH 9/9] fix(ui): escape region summaries and drop the repeater inline onclick (#333) Round 3 of PR #373, addressing the re-review (N1-N4). N1 nodes.js: the node-detail panel's "Regions:" summary joined observers.map(o => o.iata) raw into panel.innerHTML, one line above the already-escaped per-observer badge. Wrap each code in escapeHtml, matching the full-page view at nodes.js:970. N2 live.js: showNodeDetail's "Heard By - Regions: ..." heading joined the same observer IATAs raw into content.innerHTML; the per-observer rows one line below were already escaped. Same escapeHtml(join) fix. N3 analytics.js: renderRepeaterTable's row carried onclick="location.hash='#/nodes/${encodeURIComponent(r.pubkey)}'". encodeURIComponent leaves ' ( ) alone, so a pubkey quote breaks out of the JS string - the pattern F3 removed from observers.js. Drop the inline handler, navigate via data-action="navigate"/data-value, and broaden the delegated handler's selector from tr[...] to [...] so the div row is covered. tabindex/role match the sibling table rows for keyboard access. N4 test-xss-escape-sinks.js: loadLocalEsc lifts the first one-line function esc(s); assert exactly one esc() definition per file so a second one added above the real escaper fails loudly instead of silently retargeting the contract test. Tests: Section K drives the real extracted production expressions for N1/N2/N3 with a hostile IATA/pubkey (red before, green after); a new N1 step in test-xss-333-pages-e2e.js injects the hostile IATA into /api/nodes/{pk}/health and asserts the detail panel stays inert. Relates to #333 Co-Authored-By: Claude Opus 4.8 --- public/analytics.js | 4 +-- public/live.js | 2 +- public/nodes.js | 2 +- test-xss-333-pages-e2e.js | 53 ++++++++++++++++++++++++++++ test-xss-escape-sinks.js | 73 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 4 deletions(-) diff --git a/public/analytics.js b/public/analytics.js index 059f19849..9abe5751d 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -330,7 +330,7 @@ const analyticsContent = document.getElementById('analyticsContent'); if (analyticsContent) { const handler = (e) => { - const row = e.target.closest('tr[data-action="navigate"]'); + const row = e.target.closest('[data-action="navigate"]'); if (!row) return; if (e.type === 'keydown' && e.key !== 'Enter' && e.key !== ' ') return; if (e.type === 'keydown') e.preventDefault(); @@ -1102,7 +1102,7 @@ let html = '
'; repeaters.slice(0, 15).forEach(r => { const pct = (r.count / max * 100).toFixed(0); - html += `
+ html += `
${r.name ? '' + esc(r.name) + '' : '' + r.hop + ''}
${r.count.toLocaleString()}
diff --git a/public/live.js b/public/live.js index 20ffc7e15..dfe354fe4 100644 --- a/public/live.js +++ b/public/live.js @@ -2724,7 +2724,7 @@ if (observers.length) { const regions = [...new Set(observers.map(o => o.iata).filter(Boolean))]; - html += `

Heard By${regions.length ? ' — Regions: ' + regions.join(', ') : ''}

+ html += `

Heard By${regions.length ? ' — Regions: ' + regions.map(escapeHtml).join(', ') : ''}

` + observers.map(o => ``).join('') + '
'; diff --git a/public/nodes.js b/public/nodes.js index f0c360d66..d419030b1 100644 --- a/public/nodes.js +++ b/public/nodes.js @@ -2002,7 +2002,7 @@
${observers.length ? `
- ${(() => { const regions = [...new Set(observers.map(o => o.iata).filter(Boolean))]; return regions.length ? `
Regions: ${regions.join(', ')}
` : ''; })()} + ${(() => { const regions = [...new Set(observers.map(o => o.iata).filter(Boolean))]; return regions.length ? `
Regions: ${regions.map(escapeHtml).join(', ')}
` : ''; })()}

Heard By (${observers.length} observer${observers.length > 1 ? 's' : ''})

${observers.map(o => { diff --git a/test-xss-333-pages-e2e.js b/test-xss-333-pages-e2e.js index e6a2feb37..85b561d6d 100644 --- a/test-xss-333-pages-e2e.js +++ b/test-xss-333-pages-e2e.js @@ -5,6 +5,7 @@ // F1 packets region badge in the flat view and in expanded group children // F2 the shared region filter (pills, dropdown items, dropdown trigger) // F3 observers-row navigation with an observer id that contains a quote +// N1 (round 3) the nodes-page detail-panel "Regions:" summary line // // Unlike test-xss-333-e2e.js (production sink fragments rendered on a blank // page), this drives the REAL pages against a running Go server. The hostile @@ -264,6 +265,58 @@ function assertInert(r, label) { } finally { await ctx.close(); } }); + // ---- N1 (round 3): nodes-page detail panel "Regions:" summary ---------- + // The detail panel (renderDetail → #nodesRight) joins the observer IATAs + // from /api/nodes/{pk}/health into a "Regions:" line, one line above the + // already-escaped per-observer badge. Select a node, inject the hostile + // IATA into its health response, and judge the DOM. + await step('N1 nodes detail panel: Regions summary renders the hostile IATA as text', async () => { + const ctx = await browser.newContext({ viewport: { width: 1400, height: 900 } }); + let dialogs = 0; + try { + await ctx.route((url) => /\/api\/nodes\/[^/]+\/health$/.test(url.pathname), async (route) => { + const res = await route.fetch(); + const body = await res.json(); + // Guarantee at least one observer so the Regions section renders, + // whichever node the first row happens to be. + body.observers = (body.observers && body.observers.length) + ? body.observers + : [{ observer_id: 'OBS-H', observer_name: 'h', packetCount: 1 }]; + for (const o of body.observers) o.iata = HOSTILE; + await route.fulfill({ response: res, json: body }); + }); + const page = await ctx.newPage(); + page.setDefaultTimeout(15000); + page.on('dialog', (d) => { dialogs++; d.dismiss().catch(() => {}); }); + await page.goto(BASE + '/#/nodes', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#nodesBody tr[data-action="select"]'); + await page.click('#nodesBody tr[data-action="select"]'); + await page.waitForFunction(() => { + const r = document.getElementById('nodesRight'); + return Array.from(r ? r.querySelectorAll('div') : []) + .some((d) => d.innerHTML.trim().startsWith('Regions:')); + }); + await page.waitForTimeout(400); + const r = await page.evaluate((h) => { + const right = document.getElementById('nodesRight'); + // The exact summary div, not an ancestor (whose innerHTML could carry + // an unrelated such as a node avatar). + const line = Array.from(right.querySelectorAll('div')) + .find((d) => d.innerHTML.trim().startsWith('Regions:')); + return { + fired: window.__xss333 || 0, + imgs: document.querySelectorAll('img[src="x"]').length, + showsText: !!line && line.textContent.includes(h), + hasRawImg: !!line && / survived in the Regions summary'); + } finally { await ctx.close(); } + }); + await browser.close(); console.log('\n' + passed + ' passed, ' + failed + ' failed'); process.exit(failed ? 1 : 0); diff --git a/test-xss-escape-sinks.js b/test-xss-escape-sinks.js index 8ce791324..836781b8a 100644 --- a/test-xss-escape-sinks.js +++ b/test-xss-escape-sinks.js @@ -69,6 +69,14 @@ const escapeHtml = loadEscapeHtml(); // unnoticed. Lift the one-line `function esc(s) { … }` from the file. function loadLocalEsc(file, ctxExtra) { const src = fs.readFileSync(file, 'utf8'); + // #333 N4: the regex below lifts the *first* one-line `function esc(s)`. + // If a second file-local esc were added above the real sink's escaper, it + // would silently retarget this contract test. Pin the assumption: there + // must be exactly one esc() definition in the file, else fail loudly. + const defs = src.match(/function esc\s*\(/g) || []; + if (defs.length !== 1) + throw new Error(`expected exactly one function esc() in ${file}, found ${defs.length}; ` + + 'loadLocalEsc would silently pick the first — add a line anchor to disambiguate'); const m = src.match(/\n\s*(function esc\(s\) \{[^\n]*\})/); if (!m) throw new Error('could not locate function esc(s) in ' + file); const ctx = Object.assign({ esc: null }, ctxExtra || {}); @@ -1005,6 +1013,71 @@ test('#333 F3 observers row: no inline handler interpolates the observer id', () '#333 F3: data-value navigation target lost: ' + tag); }); +// ========================================================================= +// K. Issue #333 round 3 — the two remaining adjacent region sinks the round-2 +// review flagged (N1, N2) plus the analytics repeater-row inline handler +// (N3). N1 and N2 both render the SAME value — observer IATA from +// /api/nodes/{pk}/health, via observers.map(o => o.iata) — raw into a +// summary line one or two lines above an already-escaped sibling badge. +// Each test lifts the REAL production expression by regex and renders it +// with a hostile IATA, so dropping the escaper (the mutant) turns it red. +// ========================================================================= + +// N1: public/nodes.js detail panel (renderDetail → panel.innerHTML). The +// "Regions:" summary line joins observers.map(o => o.iata) into text. The +// sibling full-page view at nodes.js:970 and the "Heard By" rows below both +// escape the IATA; only this summary line is at issue. +test('#333 N1 nodes detail panel: Regions summary escapes the observer IATA', () => { + const src = fs.readFileSync('public/nodes.js', 'utf8'); + const m = src.match(/\(\(\) => \{ const regions = \[\.\.\.new Set\(observers\.map\(o => o\.iata\)\.filter\(Boolean\)\)\]; return regions\.length \? `
Regions:<\/strong> [^`]*<\/div>` : ''; \}\)\(\)/); + assert.ok(m, '#333 N1 nodes detail Regions summary IIFE not found'); + const render = new Function('observers', 'escapeHtml', 'return ' + m[0] + ';'); + const html = render([{ iata: 'SJC' }, { iata: REGION_PAYLOAD }], escapeHtml); + assertNoXss(html, '#333 N1 nodes detail Regions summary'); + assert.ok(decodeEntities(html).includes('SJC, ' + REGION_PAYLOAD), + '#333 N1: region codes must render as comma-joined text: ' + html); +}); + +// N2: public/live.js showNodeDetail (content.innerHTML). The "Heard By" +// heading appends ' — Regions: ' + the same observers.map(o => o.iata) join. +// The per-observer rows one line below already escape o.iata. +test('#333 N2 live node detail: Heard By heading escapes the observer IATA', () => { + const src = fs.readFileSync('public/live.js', 'utf8'); + // The heading template continues past

into the observer list, so lift + // just the real join expression that builds the " — Regions: …" text. + const m = src.match(/Heard By\$\{regions\.length \? ' — Regions: ' \+ ([^}]*?) : ''\}<\/h4>/); + assert.ok(m, '#333 N2 live Heard By heading not found'); + const render = new Function('regions', 'escapeHtml', "return 'Heard By — Regions: ' + (" + m[1] + ');'); + const regions = [...new Set([{ iata: 'SJC' }, { iata: REGION_PAYLOAD }].map(o => o.iata).filter(Boolean))]; + const html = render(regions, escapeHtml); + assertNoXss(html, '#333 N2 live Heard By heading'); + assert.ok(decodeEntities(html).includes('Regions: SJC, ' + REGION_PAYLOAD), + '#333 N2: region codes must render as text in the heading: ' + html); +}); + +// N3: public/analytics.js renderRepeaterTable. The row carried +// onclick="location.hash='#/nodes/${encodeURIComponent(r.pubkey)}'". +// encodeURIComponent leaves ' ( ) alone, so a pubkey quote breaks out of the +// JS string — the exact pattern F3 removed from observers.js. The fix drops +// the inline handler and navigates via data-action="navigate"/data-value. +test('#333 N3 analytics repeater row: no inline handler carries the node pubkey', () => { + const src = fs.readFileSync('public/analytics.js', 'utf8'); + const m = src.match(/html \+= `
String(s), 50, encodeURIComponent); + const handlers = Array.from(html.matchAll(/\son[a-z]+\s*=\s*"([^"]*)"/gi), x => x[1]); + for (const h of handlers) { + assert.ok(!h.includes(pubkey) && !h.includes(encodeURIComponent(pubkey)), + '#333 N3: node pubkey reaches an inline JS handler: ' + html); + } + const dv = html.match(/data-value="([^"]*)"/); + assert.ok(dv && decodeEntities(dv[1]) === '#/nodes/' + encodeURIComponent(pubkey), + '#333 N3: data-value navigation target lost: ' + html); +}); + // ========================================================================= // SUMMARY // =========================================================================