fix(ui): escape untrusted node, observer and route values in HTML rendering

.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
@@ -581,6 +582,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

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 = '<div class="repeater-list">';
     repeaters.slice(0, 15).forEach(r => {
       const pct = (r.count / max * 100).toFixed(0);
-      html += `<div class="repeater-row ${r.pubkey ? 'clickable-row' : ''}" ${r.pubkey ? `onclick="location.hash='#/nodes/${encodeURIComponent(r.pubkey)}'"` : ''}>
+      html += `<div class="repeater-row ${r.pubkey ? 'clickable-row' : ''}" ${r.pubkey ? `data-action="navigate" data-value="#/nodes/${encodeURIComponent(r.pubkey)}" tabindex="0" role="row"` : ''}>
         <div class="repeater-name">${r.name ? '<strong>' + esc(r.name) + '</strong>' : '<span class="mono">' + r.hop + '</span>'}</div>
         <div class="repeater-bar"><div class="hash-bar-track"><div class="hash-bar-fill" style="width:${pct}%;background:var(--accent)"></div></div></div>
         <div class="repeater-count">${r.count.toLocaleString()}</div>
@@ -4517,13 +4517,13 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _stopForeignTraf
           else if (obs.current_noise_floor >= -100) nfClass = 'rf-nf-warning';
         }
 
-        return `<div class="rf-cell${isSelected ? ' rf-cell-selected' : ''}" data-observer="${obs.observer_id}" tabindex="0" role="button" aria-label="Observer ${esc(name)}, noise floor ${nf} dBm">
+        return `<div class="rf-cell${isSelected ? ' rf-cell-selected' : ''}" data-observer="${esc(obs.observer_id)}" tabindex="0" role="button" aria-label="Observer ${esc(name)}, noise floor ${nf} dBm">
           <div class="rf-cell-header">
             <span class="rf-cell-name">${esc(name)}</span>
             <span class="rf-cell-nf ${nfClass}">${nf} dBm</span>
             ${batt ? `<span class="rf-cell-batt">${batt}</span>` : ''}
           </div>
-          <div class="rf-cell-sparkline" id="rf-spark-${obs.observer_id}"></div>
+          <div class="rf-cell-sparkline" id="rf-spark-${esc(obs.observer_id)}"></div>
           <div class="rf-cell-stats">
             <span>avg: ${avgNf}</span>
             <span>max: ${maxNf}</span>

public/app.js

@@ -1517,7 +1517,7 @@ function navigate() {
       else { app.setAttribute('tabindex', '-1'); app.focus({ preventScroll: true }); }
     });
   } else {
-    app.innerHTML = `<div style="padding:40px;text-align:center;color:#6b7280"><h2>${route}</h2><p>Page not yet implemented.</p></div>`;
+    app.innerHTML = `<div style="padding:40px;text-align:center;color:var(--text-muted)"><h2>${escapeHtml(route)}</h2><p>Page not yet implemented.</p></div>`;
   }
 }
 
@@ -2107,7 +2107,7 @@ window.addEventListener('DOMContentLoaded', () => {
         const statusLabel = age === null ? 'unknown' : age < HEALTH_THRESHOLDS.nodeDegradedMs ? 'healthy' : age < HEALTH_THRESHOLDS.nodeSilentMs ? 'degraded' : 'silent';
         return '<a href="#/nodes/' + pk + '" class="fav-dd-item" data-key="' + pk + '">'
           + '<span class="fav-dd-status ' + statusCls + '" title="' + statusLabel + '" aria-label="' + statusLabel + '"><svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-circle-fill"/></svg></span>'
-          + '<span class="fav-dd-name">' + (h.node.name || truncate(pk, 12)) + '</span>'
+          + '<span class="fav-dd-name">' + escapeHtml(h.node.name || truncate(pk, 12)) + '</span>'
           + '<span class="fav-dd-meta">' + (h.stats.lastHeard ? timeAgo(h.stats.lastHeard) : 'never') + '</span>'
           + favStar(pk, 'fav-dd-star')
           + '</a>';

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 '<div>' + (n.name || n.pubkey.slice(0, 12)) + coords + '</div>';
+        return '<div>' + esc(n.name || n.pubkey.slice(0, 12)) + coords + '</div>';
       }).join('');
       confirmBtn.textContent = 'Delete ' + count + ' node' + (count !== 1 ? 's' : '');
       resultEl.style.display = '';

public/live.js

@@ -2724,7 +2724,7 @@
 
       if (observers.length) {
         const regions = [...new Set(observers.map(o => o.iata).filter(Boolean))];
-        html += `<h4 style="font-size:12px;margin:12px 0 6px;color:var(--text-muted);">Heard By${regions.length ? ' — Regions: ' + regions.join(', ') : ''}</h4>
+        html += `<h4 style="font-size:12px;margin:12px 0 6px;color:var(--text-muted);">Heard By${regions.length ? ' — Regions: ' + regions.map(escapeHtml).join(', ') : ''}</h4>
           <div style="font-size:11px;">` +
           observers.map(o => `<div style="padding:2px 0;"><a href="#/observers/${encodeURIComponent(o.observer_id)}" style="color:var(--link-color);text-decoration:none;">${escapeHtml(o.observer_name || o.observer_id.slice(0, 12))}${o.iata ? ' (' + escapeHtml(o.iata) + ')' : ''}</a> — ${o.packetCount || o.count || 0} pkts</div>`).join('') +
           '</div>';

public/nodes.js

@@ -2002,7 +2002,7 @@
         </div>
 
         ${observers.length ? `<div class="node-detail-section">
-          ${(() => { const regions = [...new Set(observers.map(o => o.iata).filter(Boolean))]; return regions.length ? `<div style="margin-bottom:6px;font-size:12px"><strong>Regions:</strong> ${regions.join(', ')}</div>` : ''; })()}
+          ${(() => { const regions = [...new Set(observers.map(o => o.iata).filter(Boolean))]; return regions.length ? `<div style="margin-bottom:6px;font-size:12px"><strong>Regions:</strong> ${regions.map(escapeHtml).join(', ')}</div>` : ''; })()}
           <h4>Heard By (${observers.length} observer${observers.length > 1 ? 's' : ''})</h4>
           <div class="observer-list">
             ${observers.map(o => {

public/observers.js

@@ -424,10 +424,10 @@ 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 `<tr style="cursor:pointer" tabindex="0" role="row" data-action="navigate" data-value="#/observers/${encodeURIComponent(o.id)}" data-observer-id="${escapeHtml(o.id)}" onclick="location.hash='#/observers/${encodeURIComponent(o.id)}'">
+          return `<tr style="cursor:pointer" tabindex="0" role="row" data-action="navigate" data-value="#/observers/${encodeURIComponent(o.id)}" data-observer-id="${escapeHtml(o.id)}">
             <td data-value="${_healthRank}"><span class="health-dot ${h.cls}" title="${h.label}">${shapeIcon}</span> ${h.label}</td>
             <td data-testid="obs-cell-name" data-value="${escapeHtml(String(o.name || o.id))}" class="mono">${escapeHtml(o.name || o.id)}${window.ObserversNaiveChip.render(o)}${o.can_relay === false ? ' <span class="badge-listener" title="Firmware reported repeat:off — listener-only; excluded from path-hop disambiguator (issue #1290)">listener</span>' : (o.can_relay === true ? ' <span class="badge-repeater" title="Firmware reported repeat:on — eligible as a path hop">repeater</span>' : '')}</td>
-            <td data-value="${escapeHtml(o.iata || '')}">${o.iata ? `<span class="badge-region">${o.iata}</span>` : '—'}</td>
+            <td data-value="${escapeHtml(o.iata || '')}">${o.iata ? `<span class="badge-region">${escapeHtml(o.iata)}</span>` : '—'}</td>
             <td data-value="${_lastSeenMs}">${timeAgo(o.last_seen)}</td>
             <td data-testid="obs-cell-last-packet" data-value="${_lastPktMs}">${o.last_packet_at ? timeAgo(o.last_packet_at) : '<span class="text-muted">—</span>'}</td>
             <td data-value="${_healthRank}">${packetBadge(o)}</td>
@@ -541,7 +541,7 @@ window.preserveCompareSelection = function preserveCompareSelection(prevIds, tbo
       (naiveChipHTML ? '<div style="margin-bottom:10px">' + naiveChipHTML + ' <span class="text-muted">Clock is naive — per-packet timing clamped to ingest time. Clears itself 24h after the last skew event.</span></div>' : '') +
       '<dl class="slide-over-dl" style="margin:0;display:grid;grid-template-columns:auto 1fr;gap:6px 12px;font-size:13px">' +
         '<dt>Status</dt><dd><span class="health-dot ' + h.cls + '"><svg class="ph-icon" aria-hidden="true" focusable="false"><use href="/icons/phosphor-sprite.svg#ph-circle-fill"></use></svg></span> ' + h.label + '</dd>' +
-        '<dt>Region</dt><dd>' + (o.iata ? '<span class="badge-region">' + o.iata + '</span>' : '—') + '</dd>' +
+        '<dt>Region</dt><dd>' + (o.iata ? '<span class="badge-region">' + escapeHtml(o.iata) + '</span>' : '—') + '</dd>' +
         '<dt>Last status</dt><dd>' + timeAgo(o.last_seen) + '</dd>' +
         '<dt>Last packet</dt><dd>' + (o.last_packet_at ? timeAgo(o.last_packet_at) : '—') + '</dd>' +
         '<dt>Total packets</dt><dd>' + (o.packet_count || 0).toLocaleString() + '</dd>' +

public/packets.js

@@ -2692,7 +2692,7 @@
     const _grpCaret = isSingle ? '' : '<svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-caret-' + (isExpanded ? 'down' : 'right') + '"/></svg>';
     let html = `<tr class="${isSingle ? '' : 'group-header'} ${isExpanded ? 'expanded' : ''}" data-hash="${p.hash}" data-action="${_grpToggles ? 'toggle-select' : 'select-hash'}" data-value="${p.hash}" data-entry-idx="${entryIdx}" tabindex="0" role="row"${_grpToggles ? ' aria-expanded="' + isExpanded + '"' : ''}${_grpStyle ? ' style="' + _grpStyle + '"' : ''}>
           <td class="col-expand" style="text-align:center;cursor:pointer">${_grpCaret}</td>
-          <td class="col-region">${groupRegion ? `<span class="badge-region">${groupRegion}</span>` : '—'}</td>
+          <td class="col-region">${groupRegion ? `<span class="badge-region">${escapeHtml(groupRegion)}</span>` : '—'}</td>
           <td class="col-time">${renderTimestampCell(p.latest)}</td>
           <td class="mono col-hash" data-filter-field="hash" data-filter-value="${escapeHtml(p.hash || '')}">${truncate(p.hash || '—', 8)}</td>
           <td class="col-size" data-filter-field="size" data-filter-value="${groupSize || ''}">${groupSize ? groupSize + 'B' : '—'}</td>
@@ -2727,7 +2727,7 @@
         const childPathStr = renderPath(childPath, c.observer_id, { summary: true });
         const _childHashStripe = _hashStripeStyle(c.hash || p.hash);
         html += `<tr class="group-child" data-id="${c.id}" data-hash="${c.hash || ''}" data-action="select-observation" data-value="${c.id}" data-parent-hash="${p.hash}" data-entry-idx="${entryIdx}" tabindex="0" role="row"${_childHashStripe ? ' style="' + _childHashStripe + '"' : ''}>
-              <td class="col-expand"></td><td class="col-region">${childRegion ? `<span class="badge-region">${childRegion}</span>` : '—'}</td>
+              <td class="col-expand"></td><td class="col-region">${childRegion ? `<span class="badge-region">${escapeHtml(childRegion)}</span>` : '—'}</td>
               <td class="col-time">${renderTimestampCell(c.timestamp)}</td>
               <td class="mono col-hash" data-filter-field="hash" data-filter-value="${escapeHtml(c.hash || '')}">${truncate(c.hash || '', 8)}</td>
               <td class="col-size" data-filter-field="size" data-filter-value="${size || ''}">${size}B</td>
@@ -2764,7 +2764,7 @@
     const _flatHashStripe = _hashStripeStyle(p.hash);
     const _flatStyle = _flatHashStripe + _chanStyle;
     return `<tr data-id="${p.id}" data-hash="${p.hash || ''}" data-action="select-hash" data-value="${p.hash || p.id}" data-entry-idx="${entryIdx}" tabindex="0" role="row" class="${selectedId === p.id ? 'selected' : ''}"${_flatStyle ? ' style="' + _flatStyle + '"' : ''}>
-        <td class="col-expand"></td><td class="col-region">${region ? `<span class="badge-region">${region}</span>` : '—'}</td>
+        <td class="col-expand"></td><td class="col-region">${region ? `<span class="badge-region">${escapeHtml(region)}</span>` : '—'}</td>
         <td class="col-time">${renderTimestampCell(p.timestamp)}</td>
         <td class="mono col-hash" data-filter-field="hash" data-filter-value="${escapeHtml(p.hash || '')}">${truncate(p.hash || String(p.id), 8)}</td>
         <td class="col-size" data-filter-field="size" data-filter-value="${size || ''}">${size}B</td>

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 += '<button class="region-pill' + (active ? ' region-pill-active' : '') +
-        '" data-region="' + code + '" role="checkbox" aria-checked="' + !!active + '">' + label + '</button>';
+        '" data-region="' + escapeHtml(code) + '" role="checkbox" aria-checked="' + !!active + '">' + escapeHtml(label) + '</button>';
     });
     html += '</div>';
     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 += '<label class="region-dropdown-item"><input type="checkbox" data-region="' + code + '"' +
-        (active ? ' checked' : '') + '> ' + label + '</label>';
+      html += '<label class="region-dropdown-item"><input type="checkbox" data-region="' + escapeHtml(code) + '"' +
+        (active ? ' checked' : '') + '> ' + escapeHtml(label) + '</label>';
     });
     html += '</div></div>';
     container.innerHTML = html;

scripts/check-xss-sinks.sh

@@ -13,6 +13,26 @@
 # 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 flags an unescaped node-controlled field inside
+# a QUOTED ATTRIBUTE on a sink line, in both the template form
+# (`el.innerHTML = \`<span title="${o.observer_name}">\``) and the concat
+# form (`el.innerHTML = '<span title="' + o.name + '">'`), 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 '<td>' + o.iata + '</td>'; }   // 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 <path>       Scan a single file. Exit 1 if any flagged sink
 #                          interpolates a node-controlled identifier
@@ -195,16 +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 = '<span title="' + o.name + '">'
+# 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)
+    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)