From fb6bb085a51cbc7ab9312d8b62d9bf249bb83ac5 Mon Sep 17 00:00:00 2001
From: Kpa-clawbot <kpaclawbot@outlook.com>
Date: Thu, 11 Jun 2026 07:34:51 -0700
Subject: [PATCH] fix(analytics): render Channels group-header sprites as HTML,
 not escaped text (#1657) (#1658)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes #1657

## Bug

On `/analytics` → **Channels** tab, the "Channel Activity" table's
group-header rows ("My Channels", "Network", "Encrypted") rendered
literal HTML source text:

```
<SVG CLASS="PH-ICON" ARIA-HIDDEN="TRUE"><USE HREF="/ICONS/PHOSPHOR-SPRITE.SVG#PH-KEY"/></SVG> My Channels
```

instead of the actual Phosphor sprites. Per-row encrypted/lock icons
rendered fine — the bug was isolated to the group-header render path.

## Root cause

`public/analytics.js` `channelTbodyHtml` builds each group-section
header by wrapping the section label in `esc()`:

```js
esc(sections[si].label) + ' <span class="text-muted">(' + rows.length + ')</span>'
```

But the labels (`sections[].label`) are hardcoded sprite-bearing
strings:

```js
{ key: 'mine', label: '<svg class="ph-icon" aria-hidden="true"><use href="…#ph-key"/></svg> My Channels' },
```

`esc()` HTML-encoded the `<` / `>` so the browser displayed the source
text rather than rendering the sprite. Affects all 3 groups (and any
future group with a sprite).

## Fix

Drop the `esc()` wrap on the hardcoded label (single line change, same
pattern as M3 commit 4ca73ced for mobile channel avatars). The
`(<count>)` suffix is numeric and was always safe.

## Tests

New `test-issue-1657-analytics-channels-group-sprites-e2e.js` (mobile
375 viewport, matching the bug report):

- (1) at least one group-header row renders
- (2) every header row contains a real `<svg.ph-icon>` child
- (3) per-group sprite refs resolve (My Channels → `#ph-key`, Network →
`#ph-radio`, Encrypted → `#ph-lock`)
- (4) the Channel Activity table's `innerText` contains no literal
`<svg` substring (escape-leak gate)

Wired into the CI E2E lane (`.github/workflows/deploy.yml`) immediately
after the M4 icons E2E.

## TDD evidence

- Red commit: `8f8781c1` — test + CI wiring only, no production change.
Test asserts behavior that did not exist on master → CI fails on this
commit.
- Green commit: `8385fa54` — 1-line fix to `public/analytics.js`. Test
passes.

## Anti-tautology proof

Hot-patched staging with the `analytics.js` from the red commit
(pre-fix), reloaded `/analytics?tab=channels` at 375 viewport, and the
in-browser DOM probe returned:

```
headers[0].text = "<svg class=\"ph-icon\" aria-hidden=\"true\"><use href=\"/icons/phosphor-sprite.svg#ph…"
headers[0].svgs = 0           // (2) would fail
headers[1].svgs = 0           // (2) would fail
literalSvg     = true         // (4) would fail
```

Restored the fixed file; same probe returned `svgs=1`, correct `uses[]`
refs, `literalSvg=false`.

## Staging verification

Hot-patched `corescope-staging-go:/app/public/analytics.js` (no restart
needed — static file). Mobile dark @ 375 viewport shows Network → radio
sprite and Encrypted → lock sprite rendering correctly. (My Channels
group not present because the e2e fixture has no `mine`-tagged channels
— expected; the test skips that assertion when the row is absent.)

## Scope discipline

Touched only:
- `public/analytics.js` (1-line `esc()` removal + comment)
- `test-issue-1657-…-e2e.js` (new)
- `.github/workflows/deploy.yml` (1-line E2E wire)

No broadening, no helper renames, no related-but-different
escape-removal opportunism.

---------

Co-authored-by: openclaw-bot <bot@openclaw.local>
---
 .github/workflows/deploy.yml                  |   1 +
 public/analytics.js                           |   6 +-
 ...57-analytics-channels-group-sprites-e2e.js | 148 ++++++++++++++++++
 3 files changed, 154 insertions(+), 1 deletion(-)
 create mode 100644 test-issue-1657-analytics-channels-group-sprites-e2e.js

diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml
index 272751a5..e9e3db2d 100644
--- a/.github/workflows/deploy.yml
+++ b/.github/workflows/deploy.yml
@@ -403,6 +403,7 @@ jobs:
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1648-m2-icons-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1648-m3-icons-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1648-m4-icons-e2e.js 2>&1 | tee -a e2e-output.txt
+          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1657-analytics-channels-group-sprites-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1224-channels-mobile-ux-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1367-channels-chat-app-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1236-map-mobile-e2e.js 2>&1 | tee -a e2e-output.txt
diff --git a/public/analytics.js b/public/analytics.js
index 6769da3a..6ee8487a 100644
--- a/public/analytics.js
+++ b/public/analytics.js
@@ -1033,7 +1033,11 @@
         if (!rows.length) continue;
         parts.push(
           '<tr class="ch-section-row"><td colspan="6" class="ch-section-header">' +
-          esc(sections[si].label) + ' <span class="text-muted">(' + rows.length + ')</span>' +
+          // sections[].label is a hardcoded sprite-bearing string (no user
+          // input) — must be inserted raw so the <svg> renders. esc() here
+          // HTML-escaped the angle brackets and surfaced literal "<svg…>"
+          // text in the table (#1657).
+          sections[si].label + ' <span class="text-muted">(' + rows.length + ')</span>' +
           '</td></tr>'
         );
         for (var ri = 0; ri < rows.length; ri++) parts.push(channelRowHtml(rows[ri]));
diff --git a/test-issue-1657-analytics-channels-group-sprites-e2e.js b/test-issue-1657-analytics-channels-group-sprites-e2e.js
new file mode 100644
index 00000000..f60cef38
--- /dev/null
+++ b/test-issue-1657-analytics-channels-group-sprites-e2e.js
@@ -0,0 +1,148 @@
+#!/usr/bin/env node
+/* Issue #1657 — Analytics → Channels: group-header rows must render real
+ * <svg> Phosphor sprites (My Channels / Network / Encrypted), NOT the
+ * HTML-escaped literal "<svg class=\"ph-icon\"…>" source text.
+ *
+ * Bug: public/analytics.js passed the hardcoded group-section label (which
+ * contains the sprite markup) through esc(), HTML-encoding the angle
+ * brackets so the browser displayed the source instead of rendering it.
+ *
+ * This test asserts (in real Chromium against a running server):
+ *   (1) The Channel Activity table has at least one group-header row.
+ *   (2) Each rendered group-header row contains a real <svg.ph-icon>
+ *       element (NOT escaped text).
+ *   (3) For each expected group (key/radio/lock), if the label text is
+ *       present, an actual <use href="…#ph-{key|radio|lock}"> resolves
+ *       inside the same row.
+ *   (4) The Channel Activity table's innerText contains zero literal
+ *       "<svg" substrings (case-insensitive) — i.e. no escape leak.
+ *
+ * CHROMIUM_REQUIRE=1 makes Chromium-launch failure a HARD FAIL.
+ */
+'use strict';
+
+const { chromium } = require('playwright');
+const assert = require('assert');
+
+const BASE = process.env.BASE_URL || 'http://localhost:13581';
+
+let passes = 0, failures = 0;
+function pass(msg) { console.log(`  ✓ ${msg}`); passes++; }
+function fail(msg) { console.error(`  ✗ ${msg}`); failures++; }
+
+async function main() {
+  const requireChromium = process.env.CHROMIUM_REQUIRE === '1';
+  let browser;
+  try {
+    browser = await chromium.launch({ headless: true });
+  } catch (err) {
+    if (requireChromium) {
+      console.error(`HARD FAIL — Chromium unavailable: ${err.message}`);
+      process.exit(1);
+    }
+    console.warn(`SKIP — Chromium unavailable: ${err.message}`);
+    process.exit(0);
+  }
+
+  // Mobile viewport (375 wide) matches the staging screenshot that surfaced the bug.
+  const ctx = await browser.newContext({ viewport: { width: 375, height: 800 } });
+  const page = await ctx.newPage();
+
+  await page.goto(`${BASE}/#/analytics?tab=channels`, { waitUntil: 'domcontentloaded' });
+
+  // Wait for the channels table tbody to populate.
+  try {
+    await page.waitForFunction(() => {
+      const tb = document.getElementById('channelsTbody');
+      return tb && tb.querySelectorAll('tr').length > 0;
+    }, null, { timeout: 12000 });
+  } catch {
+    fail('channelsTbody never populated within 12s');
+    await browser.close();
+    console.log(`\ntest-issue-1657: ${passes} passed, ${failures} failed`);
+    process.exit(failures ? 1 : 0);
+  }
+
+  // Allow grouped render (decorate + buildHashKeyMap promise) to settle.
+  await page.waitForTimeout(800);
+
+  const result = await page.evaluate(() => {
+    const tb = document.getElementById('channelsTbody');
+    if (!tb) return { error: 'tbody missing' };
+    const headerRows = Array.from(tb.querySelectorAll('tr.ch-section-row'));
+    const perHeader = headerRows.map((tr) => {
+      const cell = tr.querySelector('td.ch-section-header') || tr.querySelector('td');
+      const text = (cell && cell.textContent) || '';
+      const svgs = cell ? cell.querySelectorAll('svg.ph-icon').length : 0;
+      const uses = cell
+        ? Array.from(cell.querySelectorAll('svg.ph-icon use'))
+            .map((u) => (u.getAttribute('href') || u.getAttribute('xlink:href') || ''))
+        : [];
+      return { text: text.trim(), svgs, uses };
+    });
+    const tableEl = document.getElementById('channelsTable') || tb.parentElement;
+    const innerHTML = tableEl ? tableEl.innerHTML : '';
+    // innerText reflects what the user sees; literal "<svg" in innerText
+    // means the markup got HTML-escaped before being inserted.
+    const innerText = tableEl ? (tableEl.innerText || tableEl.textContent || '') : '';
+    return { headerRows: perHeader, hasLiteralSvgInText: /<svg/i.test(innerText), htmlLen: innerHTML.length };
+  });
+
+  if (result.error) {
+    fail(result.error);
+    await browser.close();
+    process.exit(1);
+  }
+
+  // (1) at least one header row
+  if (result.headerRows.length === 0) {
+    fail('(1) no group-header rows rendered (expected at least one of My Channels / Network / Encrypted)');
+  } else {
+    pass(`(1) ${result.headerRows.length} group-header row(s) present`);
+  }
+
+  // (2) every header row has a real SVG element
+  let allHaveSvg = true;
+  for (const h of result.headerRows) {
+    if (h.svgs < 1) {
+      fail(`(2) group-header row "${h.text.slice(0, 40)}" has 0 svg.ph-icon children (escape leak)`);
+      allHaveSvg = false;
+    }
+  }
+  if (allHaveSvg && result.headerRows.length) pass(`(2) all ${result.headerRows.length} group-header rows contain a real <svg.ph-icon>`);
+
+  // (3) per expected group, verify the sprite ref
+  const expected = [
+    { match: /My Channels/i, ref: /#ph-key$/, name: 'My Channels → #ph-key' },
+    { match: /Network/i,     ref: /#ph-radio$/, name: 'Network → #ph-radio' },
+    { match: /Encrypted/i,   ref: /#ph-lock$/, name: 'Encrypted → #ph-lock' },
+  ];
+  for (const e of expected) {
+    const row = result.headerRows.find((h) => e.match.test(h.text));
+    if (!row) {
+      // Not all groups are guaranteed to have rows (depends on fixture data).
+      // Skip silently — group only renders if it has channels.
+      continue;
+    }
+    const ok = row.uses.some((u) => e.ref.test(u));
+    if (!ok) fail(`(3) ${e.name}: expected ${e.ref} but uses=${JSON.stringify(row.uses)}`);
+    else pass(`(3) ${e.name} rendered as real sprite`);
+  }
+
+  // (4) no literal "<svg" in the table's user-visible text
+  if (result.hasLiteralSvgInText) {
+    fail('(4) Channel Activity table innerText contains literal "<svg" — sprite markup was HTML-escaped');
+  } else {
+    pass('(4) Channel Activity table innerText is free of literal "<svg" leaks');
+  }
+
+  await browser.close();
+  console.log(`\ntest-issue-1657: ${passes} passed, ${failures} failed`);
+  assert.strictEqual(failures, 0, `${failures} assertion(s) failed`);
+  process.exit(0);
+}
+
+main().catch((err) => {
+  console.error('test-issue-1657: FAIL —', err);
+  process.exit(1);
+});