fix(widgets): bound the unauthenticated telemetry store, and stop it writing rows

The diag widget runs in a null-origin sandboxed iframe, so it cannot carry a session and
its telemetry POST must stay unauthenticated. But the handler stored into a plain Map
keyed on a value taken from the request body, with no cap, no TTL and no eviction — an
unauthenticated caller could add entries until the process died. On this product a dead
server is a fleet-wide reconnect, so a bound here is a fleet-safety control.

Two changes:

- lib/bounded-snapshot-store.js: a "latest snapshot per key" store with a global entry cap
  and a TTL, evicting least-recently-WRITTEN. The cap is GLOBAL rather than per-IP on
  purpose — signage sites egress through one NAT address, so a per-IP limit punishes a
  whole venue for one noisy panel and does nothing about a distributed writer. Same
  reasoning the OTA download guard already documents ("NEVER per-IP (SNAT)"). A live panel
  rewrites its key every 2.5s, so only entries the dashboard already treats as stale
  (>15s) are ever eligible for eviction.

- The POST now answers 204 instead of res.json({ok:true}). The reporting widget ignores
  the response (fetch(...).catch()), and services/activity.js activityLogger wraps
  res.json — so this also stops an anonymous caller from writing one activity_log row, and
  running two synchronous statements, per report.

Read contract unchanged: a live key returns its object, an unknown OR expired key returns
null — the shape frontend/js/views/device-detail.js already handles ("no report yet"), and
it treats anything older than 15s as stale regardless, so the 60s TTL is 4x looser than
what the UI honours. No client change; no rate limiter added.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

server/lib/bounded-snapshot-store.js

@@ -0,0 +1,72 @@
+'use strict';
+
+// A bounded "latest snapshot per key" store for diagnostic data that arrives from
+// UNAUTHENTICATED callers.
+//
+// The widget-telemetry store was a plain Map keyed on a value the caller supplies, with no
+// cap, no TTL and no eviction: an unauthenticated caller could add entries until the
+// process ran out of memory, and on this product a dead server means every screen in the
+// fleet reconnects at once. A bound is therefore a fleet-safety control, not tidiness.
+//
+// Deliberately NOT rate-limited per IP. This codebase already learned that lesson for the
+// OTA download guard ("NEVER per-IP (SNAT)"): signage sites egress through one NAT
+// address, so a per-IP cap punishes a whole venue for one noisy panel and does nothing
+// against a distributed writer. The GLOBAL entry cap is the honest bound — it makes the
+// worst case a fixed amount of memory regardless of who is writing or from where.
+//
+// Eviction is least-recently-WRITTEN. Every live reporter rewrites its own key on each
+// report, so under a flood the only entries eligible for eviction are ones already older
+// than any consumer would treat as live.
+
+function createStore({ max = 500, ttlMs = 60_000 } = {}) {
+  // Map preserves insertion order, and re-setting a key does NOT refresh that order, so
+  // delete-then-set is what makes the iteration order a true recency order.
+  const m = new Map();
+  let sweepTimer = null;
+
+  function set(key, value) {
+    if (m.has(key)) m.delete(key);
+    m.set(key, value);
+    // Evict the least-recently-written entries until we are back inside the cap.
+    while (m.size > max) {
+      const oldest = m.keys().next();
+      if (oldest.done) break;
+      m.delete(oldest.value);
+    }
+    return value;
+  }
+
+  // Returns null for a missing OR expired entry. Expiry is enforced on READ as well as by
+  // the sweep, so a stale value can never be served just because the sweep hasn't run.
+  function get(key, now = Date.now()) {
+    const v = m.get(key);
+    if (!v) return null;
+    const at = typeof v.receivedAt === 'number' ? v.receivedAt : 0;
+    if (now - at > ttlMs) { m.delete(key); return null; }
+    return v;
+  }
+
+  function sweep(now = Date.now()) {
+    let dropped = 0;
+    for (const [k, v] of m) {
+      const at = typeof v.receivedAt === 'number' ? v.receivedAt : 0;
+      if (now - at > ttlMs) { m.delete(k); dropped++; }
+    }
+    return dropped;
+  }
+
+  // unref() so the interval never holds the process open (same discipline as
+  // lib/log-coalescer.js and the other sweeps).
+  function startSweep(intervalMs = ttlMs) {
+    if (sweepTimer) return sweepTimer;
+    sweepTimer = setInterval(() => { try { sweep(); } catch (_) { /* never throw from a timer */ } }, intervalMs);
+    if (sweepTimer.unref) sweepTimer.unref();
+    return sweepTimer;
+  }
+
+  function stopSweep() { if (sweepTimer) { clearInterval(sweepTimer); sweepTimer = null; } }
+
+  return { set, get, sweep, startSweep, stopSweep, size: () => m.size, max, ttlMs };
+}
+
+module.exports = { createStore };

server/routes/widgets.js

@@ -224,7 +224,19 @@ router.get('/:id/data.json', (req, res) => {
 
 // Latest frame-rate telemetry per widget, reported by the diag-smoothness widget running on a device.
 // In-memory (diagnostic, not persisted) — a device page reads the snapshot for the widget it plays.
-const widgetTelemetry = new Map();
+//
+// BOUNDED, because the writer is unauthenticated (the widget runs in a null-origin sandboxed
+// iframe and cannot carry a session) and the key comes from the request body. An uncapped map
+// keyed on caller-supplied values is a remote memory-exhaustion path, and on this product a dead
+// server means the whole fleet reconnects at once.
+//
+// The cap is GLOBAL rather than per-IP on purpose: signage sites egress through one NAT address,
+// so a per-IP limit would punish an entire venue for one noisy panel while doing nothing about a
+// distributed writer. Same reasoning as lib/ota-download-guard ("NEVER per-IP (SNAT)"). Eviction
+// is least-recently-written, and a live panel rewrites its key every 2.5s, so only entries the
+// dashboard would already call stale (>15s) are ever eligible.
+const widgetTelemetry = require('../lib/bounded-snapshot-store').createStore({ max: 500, ttlMs: 60_000 });
+widgetTelemetry.startSweep();
 // Public POST from the widget: it runs in a null-origin sandboxed iframe, so this must be no-auth +
 // CORS-open. The widget sends text/plain (a "simple" request → no CORS preflight); we JSON.parse it.
 router.post('/:id/telemetry', express.text({ type: '*/*', limit: '16kb' }), (req, res) => {
@@ -236,7 +248,11 @@ router.post('/:id/telemetry', express.text({ type: '*/*', limit: '16kb' }), (req
   // fall back to a widget-scoped key for players that don't pass a device id yet.
   const key = (t.device && String(t.device).slice(0, 64)) || ('w:' + req.params.id);
   widgetTelemetry.set(key, t);
-  res.json({ ok: true });
+  // 204, not res.json(): this is fire-and-forget diagnostic data and the reporting widget ignores
+  // the response entirely (routes/widgets.js renderDiagSmoothness -> fetch(...).catch()). It also
+  // keeps services/activity.js activityLogger — which wraps res.json — from writing an activity_log
+  // row per unauthenticated report, i.e. from letting an anonymous caller grow a DB table.
+  res.status(204).end();
 });
 // Public GET so the dashboard device page can display the snapshot. ?device=<id> reads that panel's
 // report; without it (or if that panel hasn't reported) falls back to the widget-scoped snapshot.
@@ -247,8 +263,10 @@ router.get('/:id/telemetry', (req, res) => {
   // Device-scoped request returns ONLY that device's report — NO widget-wide fallback, or one
   // reporting panel's data would show on every other device's page (incl. offline ones). A request
   // with no device id gets the widget-scoped snapshot (raw/debug view only).
-  const rec = dev ? (widgetTelemetry.get(dev) || null) : (widgetTelemetry.get('w:' + req.params.id) || null);
-  res.json(rec);
+  // get() returns null for a missing OR expired entry, so a stale snapshot is never served
+  // as live even between sweeps.
+  const rec = dev ? widgetTelemetry.get(dev) : widgetTelemetry.get('w:' + req.params.id);
+  res.json(rec || null);
 });
 
 // Preview unsaved widget from config (used by editor Preview button)

server/test/widget-telemetry-bounded.test.js

@@ -0,0 +1,144 @@
+'use strict';
+
+// The widget telemetry endpoint accepts writes from UNAUTHENTICATED callers (the diag
+// widget runs in a null-origin sandboxed iframe, so it cannot carry a session). Two
+// invariants keep that from being a resource-exhaustion path — on this product a dead
+// server is a fleet-wide reconnect, so these are fleet-safety properties:
+//
+//   1. The in-memory store is BOUNDED — a fixed entry cap and a TTL, so an unauthenticated
+//      writer cannot grow it without limit no matter how many distinct keys it invents.
+//   2. An unauthenticated report writes NO durable row — it must not be able to grow a
+//      database table either.
+//
+// And the consumer contract is unchanged: a live key returns its object, an unknown or
+// expired key returns null, which frontend/js/views/device-detail.js already handles
+// (it renders "no report yet" and treats anything older than 15s as stale anyway).
+
+const { test, before, after } = require('node:test');
+const assert = require('node:assert/strict');
+const { spawn } = require('node:child_process');
+const path = require('node:path');
+const os = require('node:os');
+const fs = require('node:fs');
+const crypto = require('node:crypto');
+const { createStore } = require('../lib/bounded-snapshot-store');
+
+// ---------------------------------------------------------------------------
+// 1. The bound itself (unit)
+// ---------------------------------------------------------------------------
+test('the store never exceeds its cap, however many distinct keys arrive', () => {
+  const s = createStore({ max: 50, ttlMs: 60_000 });
+  for (let i = 0; i < 5000; i++) s.set('attacker-key-' + i, { receivedAt: Date.now() });
+  assert.equal(s.size(), 50, '5000 distinct keys must not produce 5000 entries');
+});
+
+test('eviction is least-recently-written, so a live reporter is never dropped', () => {
+  const s = createStore({ max: 10, ttlMs: 60_000 });
+  s.set('live-panel', { receivedAt: Date.now() });
+  for (let i = 0; i < 100; i++) {
+    s.set('noise-' + i, { receivedAt: Date.now() });
+    s.set('live-panel', { receivedAt: Date.now() }); // the panel keeps reporting
+  }
+  assert.ok(s.get('live-panel'), 'a key that keeps being written survives a flood');
+  assert.equal(s.size(), 10);
+});
+
+test('entries expire, on read as well as by sweep', () => {
+  const s = createStore({ max: 100, ttlMs: 1000 });
+  const t0 = 1_000_000;
+  s.set('k', { receivedAt: t0 });
+  assert.ok(s.get('k', t0 + 500), 'fresh entry is returned');
+  assert.equal(s.get('k', t0 + 5000), null, 'expired entry reads as null, not stale data');
+
+  s.set('a', { receivedAt: t0 });
+  s.set('b', { receivedAt: t0 });
+  assert.equal(s.sweep(t0 + 5000), 2, 'sweep drops expired entries');
+  assert.equal(s.size(), 0);
+});
+
+test('a missing key reads as null — the shape the dashboard already handles', () => {
+  const s = createStore();
+  assert.equal(s.get('never-seen'), null);
+});
+
+test('the sweep timer does not hold the process open', () => {
+  const s = createStore({ ttlMs: 50 });
+  const t = s.startSweep(10);
+  assert.equal(typeof t.unref, 'function');
+  s.stopSweep();
+});
+
+// ---------------------------------------------------------------------------
+// 2. End to end: no durable row, and the HTTP contract is unchanged
+// ---------------------------------------------------------------------------
+const { freePort } = require('./helpers/free-port');
+let PORT, BASE, proc;
+const DATA_DIR = path.join(os.tmpdir(), 'st-telemetry-test-' + crypto.randomBytes(4).toString('hex'));
+const LOG = path.join(os.tmpdir(), 'st-telemetry-' + crypto.randomBytes(4).toString('hex') + '.log');
+
+before(async () => {
+  PORT = await freePort();
+  BASE = `http://127.0.0.1:${PORT}`;
+  const logFd = fs.openSync(LOG, 'w');
+  proc = spawn('node', ['server.js'], {
+    cwd: path.join(__dirname, '..'),
+    env: { ...process.env, DATA_DIR, SELF_HOSTED: 'true', PORT: String(PORT), NODE_ENV: 'test' },
+    stdio: ['ignore', logFd, logFd],
+  });
+  for (let i = 0; i < 80; i++) {
+    try { const r = await fetch(BASE + '/api/status'); if (r.ok) return; } catch { /* not yet */ }
+    await new Promise(r => setTimeout(r, 250));
+  }
+  throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000));
+});
+after(() => { try { proc.kill('SIGKILL'); } catch { /* ignore */ } });
+
+const postTelemetry = (widgetId, body) => fetch(`${BASE}/api/widgets/${widgetId}/telemetry`, {
+  method: 'POST', headers: { 'Content-Type': 'text/plain' }, body: JSON.stringify(body),
+});
+
+test('an unauthenticated report writes no durable row', async () => {
+  const wid = 'w-' + crypto.randomBytes(4).toString('hex');
+  for (let i = 0; i < 25; i++) {
+    const res = await postTelemetry(wid, { device: 'dev-' + i, fps: 60 });
+    assert.ok(res.status < 400, 'the widget must keep being able to report');
+  }
+  const Database = require('better-sqlite3');
+  const db = new Database(path.join(DATA_DIR, 'db', 'remote_display.db'), { readonly: true });
+  const n = db.prepare("SELECT COUNT(*) n FROM activity_log WHERE action LIKE '%telemetry%'").get().n;
+  db.close();
+  assert.equal(n, 0, 'an unauthenticated caller must not be able to grow activity_log');
+});
+
+test('the SERVER store is bounded — a flood of distinct keys evicts older ones', async () => {
+  // The unit tests above specify the store in isolation; this one proves it is actually
+  // WIRED IN, by observing eviction through the HTTP surface. On an unbounded store the
+  // first key survives forever and this fails.
+  const wid = 'w-' + crypto.randomBytes(4).toString('hex');
+  const first = 'dev-first-' + crypto.randomBytes(4).toString('hex');
+  await postTelemetry(wid, { device: first, fps: 1 });
+  assert.ok((await (await fetch(`${BASE}/api/widgets/${wid}/telemetry?device=${first}`)).json()),
+    'the first report is readable before the flood');
+
+  for (let i = 0; i < 700; i++) await postTelemetry(wid, { device: `flood-${i}`, fps: 60 });
+
+  const after = await (await fetch(`${BASE}/api/widgets/${wid}/telemetry?device=${first}`)).json();
+  assert.equal(after, null, 'an unbounded store would still be holding the first key');
+});
+
+test('the read contract is unchanged: live key -> object, unknown key -> null', async () => {
+  const wid = 'w-' + crypto.randomBytes(4).toString('hex');
+  const dev = 'dev-' + crypto.randomBytes(4).toString('hex');
+  await postTelemetry(wid, { device: dev, fps: 59, verdict: 'SMOOTH' });
+
+  const live = await fetch(`${BASE}/api/widgets/${wid}/telemetry?device=${dev}`);
+  assert.equal(live.status, 200);
+  const body = await live.json();
+  assert.equal(body.fps, 59, 'the reporting panel\'s snapshot comes back');
+  assert.equal(body.verdict, 'SMOOTH');
+  assert.equal(typeof body.receivedAt, 'number', 'receivedAt drives the dashboard staleness check');
+
+  const unknown = await fetch(`${BASE}/api/widgets/${wid}/telemetry?device=nobody`);
+  assert.equal(unknown.status, 200);
+  assert.equal(await unknown.json(), null, 'unknown device reads as null, the shape the UI handles');
+});