Closes#1562. Follow-up to #1551 and #1552.
## Problem
On CDN-fronted deployments (e.g. meshcore.meshat.se), the observers page
header rendered totals computed entirely client-side from a
possibly-stale `/api/observers` response. Operators saw e.g. `0 Online /
43 Stale / 37 Offline` while a cache-busted request returned `44 Online
/ 0 Stale / 36 Offline` — the aggregate row was the first thing they
looked at to assess mesh health, so wrong numbers meant wrong actions.
#1551 added `Cache-Control: no-store` on `/api/*` responses, but the
client also has its own in-memory cache (`api(path, { ttl })`), and
there was no UI signal at all that the rendered counts could be stale.
## Fix scope (Option 3 + light Option 2)
Per the issue's three options, this PR implements **Option 3**
(timestamp label) and a light **Option 2** (manual-refresh button
bypasses client cache). Option 1 (a new server-side
`/api/observers/summary` endpoint) is **deferred** as a follow-up — it's
the most correct fix, but a bigger lift than what's needed to stop
operators from acting on silently-wrong numbers.
## Changes
- **`public/observers.js`**
- New `window.ObserversSummary` pure helper exposing
`computeCounts(observers)` and `renderHeader(counts, fetchedAt)`. Pure
functions = easy to unit test.
- Track `_fetchedAt` (ms) on each successful `loadObservers()` response.
- `render()` delegates header HTML to
`ObserversSummary.renderHeader(counts, fetchedAt)`. Existing aggregate
display (`Online / Stale / Offline / Total`) is preserved exactly — the
only visible additions are the "Last updated: Xs ago" label and a
warning class when the timestamp is >60s old.
- Manual refresh button now passes `{ bust: true }` to `api()` so the
operator can force a fresh fetch when they suspect staleness.
- **`public/style.css`**
- New `.obs-updated` and `.obs-updated-stale` rules using existing
`--text-muted` / `--warning` CSS variables (no new colors).
- **`test-issue-1562-observers-summary.js`** +
**`.github/workflows/deploy.yml`**
- Unit tests for `computeCounts` (mixed ages → 1/1/1 + total),
`renderHeader` (label presence + stale-warning class), plus DOM-grep
checks that observers.js still tracks `_fetchedAt` and bypasses the
cache on manual refresh.
## TDD
Red commit asserts `ObserversSummary` doesn't exist / no `_fetchedAt`
tracking / no `obs-updated-stale` CSS → fails. Green commit adds the
implementation → passes.
## What this PR does NOT touch
- **Observer health thresholds** — owned by #1552, untouched here.
- **`healthStatus()` per-row classification** — untouched. The same
function still gates per-row colors AND aggregate counts; the fix is
about freshness visibility, not classification logic.
- **No new server endpoint** — Option 1 deferred. Will file a follow-up
if anyone wants that tracked.
---------
Co-authored-by: openclaw-bot <bot@openclaw.local>
Co-authored-by: mc-bot <bot@meshcore.local>
# fix(ingestor): write resolved_path on new observations (full restore —
closes#1547 + #1560)
Fixes#1547. Closes#1560.
## Root cause
PR #1289 (the "ingestor owns the neighbor graph; server is read-only"
refactor, ~2026-05-21) moved the neighbor graph + schema writes to the
ingestor, and as a side-effect removed the server-side writer that
populated `observations.resolved_path` AND the context-aware
`pm.resolveWithContext` that disambiguated 1-byte prefix collisions.
Result: every observation inserted after the deploy has `resolved_path =
NULL` (3.1M/6.3M NULL on staging; 100% NULL on fresh deploys; symptom on
Cascadia: hops fail to resolve because the small-mesh client-side
fallback breaks on prefix collisions).
## Full restore
This PR resolves both single-byte and multi-byte prefix paths.
Single-byte disambiguation uses NeighborGraph adjacency and ADVERT
`from_pubkey` anchoring, ported from pre-#1289 `pm.resolveWithContext`
logic (last good at cmd/server/store.go @ commit 450236d5) and the #1144
/ #1352 fixes.
New file `cmd/ingestor/path_resolver.go`:
- `NeighborGraph` + `neighborGraphHolder` — in-memory adjacency
snapshot, atomic-published.
- `loadNeighborGraph(db)` — one-shot SELECT from `neighbor_edges`.
- `resolveHopWithContext(hop, anchor, graph, idx, exclude) *string` —
single-hop, tier-1 disambiguator.
- `resolvePathWithContext(hops, fromPubkey, graph, idx) []*string` —
walks the path, anchoring hop 0 on `from_pubkey` (ADVERTs) and each
subsequent hop on the previous resolved hop, excluding already-resolved
pubkeys.
- `Store.RefreshNeighborGraph()` — called on warm-up and every 60s tick
in the neighbor-edges builder alongside `RefreshPrefixIndex`.
Existing file `cmd/ingestor/resolved_path.go` (PR #1547 base) is
untouched: `resolvePath` + `marshalResolvedPath` + the all-nil →
empty-string clobber-guard contract are preserved verbatim.
`cmd/ingestor/db.go` — `InsertTransmission` now calls
`resolvePathWithContext` instead of the naive `resolvePath`.
## Algorithm (per hop)
1. Look up candidate pubkeys by prefix-match (existing `prefixIndex`).
2. `len==0 → nil`; `len==1 → that pubkey`.
3. `len>1` → filter by `NeighborGraph` adjacency to the anchor. Anchor
is `from_pubkey` for hop 0 on ADVERTs, the previous resolved hop
otherwise. Exactly 1 surviving candidate → use it; else nil.
4. Previously resolved hops (and the originator) are excluded from
downstream candidate pools — a packet does not revisit a node.
Tier-2/3/4 from pre-#1289 (geo proximity, GPS preference,
observation-count fallback) are intentionally NOT ported — those were
noisy in practice and belong in a separate enhancement, not in this
regression restore.
## Out of scope
- The ~3.1M existing NULL rows from the regression window. Filed as a
follow-up backfill task — too risky to bundle here (touches a 6M-row
table).
- The dead-flag bug #1546 — separate concern.
## TDD red → green
- Red commit `80b0f476` — adds five new context-resolver tests; stub
`resolvePathWithContext` falls back to naive `resolvePath`. CI run
26946935615 → **failure** with assertion errors on the three collision
tests (`TestResolveHopWithContext_OneByteCollision_AdjacencyResolves`,
`TestResolvePathWithContext_TwoHopChainAnchoredOnFromNode`,
`TestResolvePathWithContext_AdvertAnchoring`); the two regression tests
(multi-byte still works + all-nil contract) stayed green.
- Green commit `7b4950ce` — real algorithm + InsertTransmission wiring +
RefreshNeighborGraph in the builder tick. All five new tests pass;
original four `resolved_path` tests stay green.
## Verification
- `go test -race ./cmd/ingestor/...` for the 11 affected tests — pass.
- `bash ~/.openclaw/skills/pr-preflight/scripts/run-all.sh
origin/master` — exit 0 (all gates clean).
- PII grep on body + diff: clean.
Tested with: existing `TestInsertTransmissionWritesResolvedPath` +
`TestInsertTransmissionDoesNotClobberResolvedPathOnAllNil` (PR #1547
base) plus the new collision-resolution suite:
- `TestResolveHopWithContext_OneByteCollision_AdjacencyResolves` —
3-of-5 nodes share `0x5c`, chain A↔B↔C↔D↔E; anchored on A, hop `5c` → B.
- `TestResolvePathWithContext_TwoHopChainAnchoredOnFromNode` — path
`[5c, 5c]` from_node A → `[B, C]`.
- `TestResolveHopWithContext_NoAdjacencyContext_ReturnsNil` — 3
ambiguous candidates, no anchor / non-adjacent anchor → nil.
- `TestResolvePathWithContext_AdvertAnchoring` — ADVERT,
`from_pubkey=A`, path `[5c]` → only-adjacent neighbor B.
- `TestResolvePathWithContext_RegressionMultiByteStillWorks` —
unique-prefix path with no graph context still resolves.
- `TestResolvePathWithContext_AllNilContractPreserved` — unresolvable
path → `marshalResolvedPath==""` (clobber-guard from PR #1548
untouched).
## Browser-validated
N/A — backend-only change. Frontend already handles populated
`resolved_path` via `getResolvedPath` in `cmd/server/db.go` and
`public/packets.js`.
## Round-1 fixes addressed
- **MUST-FIX #1 (data-loss clobber on all-nil resolution):** when every
hop fails to resolve, `marshalResolvedPath` returns `""` instead of
`"[null,null,...]"`, so `nilIfEmpty` → SQL NULL and the
`COALESCE(excluded.resolved_path, resolved_path)` UPSERT preserves any
previously stored good value on re-ingest. Regression test asserts:
insert a transmission, observe `resolved_path` populated, wipe the
prefix index, re-ingest the same packet, assert the existing
`resolved_path` is unchanged.
---------
Co-authored-by: corescope-bot <bot@corescope>
Co-authored-by: openclaw-bot <bot@openclaw>
Co-authored-by: openclaw-bot <bot@openclaw.local>
List of changes too long to describe, so I'll hit high level.
- Config now supports the json map tiles that were suggested by
@Kpa-clawbot.
- Leaflet map layer button appears in the top right of live.js and
map.js (because all the work was already done on live.js... Added bonus)
- Allows users to enter creds for OSM and Stamen to get enterprise
related perks, in the config file
- Added a default light map under customizer. Still suggest removing
them all together and relying on the config
- You can enable OSM and Stamen in the config without a license, but at
your own risk!!!
- Config comment explains where to register and the providers for osm,
as well as the general limits per X interval
- Updated tests (28) to address the changes made to the maps
### TDD Exemption
**Reason**: Net-new UI surfaces (per `AGENTS.md`)
This PR introduces a net-new UI surface (the multi-provider map tile
selector). Under the `AGENTS.md` exemption for net-new UI surfaces, the
absence of an initial failing (red) commit is permitted, as the UI was
built first. However, the underlying public APIs are fully covered.
The following tests serve as the first assertions for these new APIs:
- `window.MC_createLayerControl`: Asserted in `MC_createLayerControl
handles Auto mode and explicit layers correctly`
- `window.MC_setDarkTileProvider` & `window.MC_getDarkTileProvider`:
Asserted in `MC_setDarkTileProvider persists to localStorage...`
- `window.MC_setLightTileProvider` & `window.MC_getLightTileProvider`:
Asserted in `MC_setLightTileProvider persists to localStorage...`
- `window.MC_initTileRegistry`: Asserted in `MC_initTileRegistry(true)
dispatches mc-tile-provider-changed`
- `applyTileFilter`: Asserted in `applyTileFilter sets invert CSS for
inverted dark provider...`
- Cross-tab synchronization: Asserted in `Cross-tab storage event
re-dispatches mc-tile-provider-changed`
Closes#1561. Follow-up to #1551.
## Why
#1551 added `Cache-Control: no-store` to all `/api/*` responses. That's
sufficient for CDNs that honour origin headers (Varnish, nginx). It is
**not** sufficient for Cloudflare zones where Cache Rules / Page Rules
override origin Cache-Control.
Field evidence from the meshat.se diagnosis (2026-06-04): observers
behind Cloudflare were returning `cf-cache-status: HIT` with `age` up to
~6 hours despite the origin emitting `no-store`. The CDN was caching per
zone policy and ignoring the upstream directive — exactly the failure
mode #1551 cannot reach. The application has no way to inject CDN rules;
the only durable fix is operator-side.
This PR makes that operator step discoverable and verifiable.
## What
### Server-side detection (log-only)
`cmd/server/cdn_detection.go` adds a middleware wired into the `/api/*`
chain after `noStoreAPIMiddleware`. On the **first** request bearing any
CDN-typical header (`CF-Connecting-IP`, `CF-Ray`, `X-Forwarded-For`,
`X-Real-IP`, `Fastly-Client-IP`, `True-Client-IP`) it logs:
```
[security] WARNING: detected request via CDN (CF-Ray header present).
Ensure /api/* is bypassed in your CDN config — see docs/deployment-behind-cdn.md.
Cached API responses cause observer-flap and incorrect dashboards.
```
`sync.Once` guarantees the warning fires at most once per process boot.
The middleware never blocks, never modifies the response, never adds
headers. Detection is observational only — operators who run behind a
CDN without bypass have a real bug; the warning is appropriate.
### Operator documentation
`docs/deployment.md` gains a new **"Behind a CDN (Cloudflare, Fastly)"**
section covering:
1. Curl verification command + healthy vs unhealthy output examples
2. Cloudflare Cache Rule creation (URI Path starts-with `/api/` → Bypass
cache)
3. Legacy Page Rules equivalent
4. Fastly note
5. Re-verification
6. Meaning of the startup log warning
7. Why we can't fix this server-side
`docs/deployment-behind-cdn.md` is the canonical path the log message
references — it's a short TL;DR that links back to the full section.
### Healthcheck script
`scripts/check-cdn-bypass.sh` — POSIX sh, no dependencies beyond curl +
grep + awk. Operators run:
```sh
scripts/check-cdn-bypass.sh https://your-domain.example.com
```
Exits `0` with `OK: no CDN caching detected ...` or `1` with a precise
diagnostic naming the offending header (`cf-cache-status: HIT` or stale
`age`).
## TDD
- **Red commit `e90ccaba`** (`test(security): RED ...`) —
`cmd/server/cdn_detection_test.go` (4 Go tests + 6 subtests for each
header) and `scripts/test-check-cdn-bypass.sh` (3 shell harness cases).
Middleware stub returns `next` unchanged so tests compile and fail on
assertions, not build errors.
- **Green commit `5e6a60b5`** (`feat(security): GREEN ...`) — real
middleware, wiring in `routes.go`, healthcheck script, doc.
## Deliverables
| File | Status | Purpose |
|------|--------|---------|
| `cmd/server/cdn_detection.go` | new | middleware + sync.Once warning |
| `cmd/server/cdn_detection_test.go` | new | 4 Go tests (1 stand-alone +
1 silence + 1 once + 1 table-driven over 6 headers) |
| `cmd/server/routes.go` | modified | `r.Use(cdnDetectionMiddleware)`
after no-store |
| `docs/deployment.md` | modified | TOC entry + "Behind a CDN" section |
| `docs/deployment-behind-cdn.md` | new | canonical path referenced by
log message + script output |
| `scripts/check-cdn-bypass.sh` | new | operator-runnable healthcheck |
| `scripts/test-check-cdn-bypass.sh` | new | shell harness with fake
curl |
## What this PR explicitly does NOT do
- Does not block requests based on CDN detection (log-only).
- Does not enforce CDN bypass (impossible — operator-controlled).
- Does not spoof, strip or modify CDN headers.
- Does not add CSP / HSTS / other security headers (out of scope).
- Warning is not configurable — operators behind a CDN without bypass
have a real bug, surfacing it is correct.
## Verification
- `go test ./...` in `cmd/server/` — full suite green.
- `sh scripts/test-check-cdn-bypass.sh` — 3/3 pass.
- Preflight checklist — all 11 gates clean (PII, branch scope, red
commit, CSS vars, CSS self-fallback, LIKE-on-JSON, sync migration,
async-migration annotation, XSS sinks, img/SVG ratio, themed-img/SVG,
fixture coverage).
---------
Co-authored-by: openclaw-bot <bot@openclaw.local>
Co-authored-by: clawbot <bot@clawbot.invalid>
Closes#1552.
## What
Make observer `Online` / `Stale` / `Offline` thresholds
operator-configurable via `config.json`'s existing `healthThresholds`
block — and **raise the defaults** from 10 min / 60 min to **60 min /
1440 min (1 h / 24 h)** so they match the node thresholds and stop
producing flap out of the box.
⚠️ **This is a default behavior change.** Operators who want the old
aggressive 10-min Online threshold must opt in via:
```json
"healthThresholds": { "observerOnlineMinutes": 10 }
```
## Why
Per #1552: the `600000` / `3600000` constants in `public/observers.js`
were not tunable, *and* 10 min is wrong as a default. Wide-geo,
low-traffic meshes legitimately see observers go quiet for >10 min
between reports, and operators behind a CDN (#1551) get cached
`last_seen` values that can push the observer 15+ min behind reality —
guaranteeing flap at the 10-min threshold. The meshat.se operator (43
observers, v3.8.3) reports exactly this pattern.
Defaults raised from 10 / 60 minutes to 60 / 1440 minutes (1 h / 24 h)
to match the node thresholds for consistency and eliminate flap on
low-traffic / CDN-fronted instances. Operators wanting the old 10-min
Online behavior can set `observerOnlineMinutes: 10` in config.
## Changes
Backend (`cmd/server/config.go`):
- `HealthThresholds` gains `ObserverOnlineMinutes` /
`ObserverStaleMinutes` (int).
- `GetHealthThresholds()` defaults to **60 / 1440** when zero/absent.
- `ToClientMs()` emits `observerOnlineMs` / `observerStaleMs`, picked up
by the existing `/api/config-public` → `roles.js`
`Object.assign(HEALTH_THRESHOLDS, …)` pipeline.
`config.example.json`: new `observerOnlineMinutes` /
`observerStaleMinutes` keys (60 / 1440) + `_comment_observerThresholds`
explaining the rationale and opt-out.
Frontend:
- `public/observers.js` `healthStatus()` — reads from
`window.HEALTH_THRESHOLDS.observerOnlineMs / observerStaleMs`, falls
back to **3600000 / 86400000** (matching the new Go defaults for the
pre-`/api/config-public` window).
- `public/observer-detail.js` — same refactor (was previously hardcoded
`600000` + misusing `nodeDegradedMs` for the Stale boundary).
## Backward compat
- API shape: unchanged — only adds two optional keys.
- Config: unchanged keys / no renames.
- Default behavior: **changed** — operators relying on the implicit
10/60 must opt in (one config line).
## TDD
- RED 1 (`ee19058f`): assertions on the new fields + `ToClientMs` keys +
`healthStatus` reading from `window.HEALTH_THRESHOLDS`. CI:
[failure](https://github.com/Kpa-clawbot/CoreScope/actions/runs/26945264822).
- GREEN 1 (`30cfbf7a`): configurability landed (defaults still old
10/60). CI:
[success](https://github.com/Kpa-clawbot/CoreScope/actions/runs/26945220598).
- RED 2 (`2649cf35`): pin new 60/1440 defaults — empty-config Go path +
JS `healthStatus` with no `HEALTH_THRESHOLDS`. CI must fail.
- GREEN 2 (`5ef85bca`): bump Go defaults to 60/1440, JS fallbacks to
3600000/86400000, `config.example.json` updated. CI must pass.
## Preflight
Clean (exit 0). `cross-stack` ack in commit messages — single feature
spans Go + JSON + JS readers.
## Not in scope
- Customizer UI for editing the thresholds (config-only per issue).
- Node/infra thresholds (unchanged).
- The deeper observer-flap root cause (#1551 cache-control is a separate
PR in flight).
---------
Co-authored-by: corescope-bot <bot@corescope>
Co-authored-by: mc-bot <bot@meshcore.local>
Closes#1551.
## Problem
`/api/*` Go responses emit no `Cache-Control` header. CDNs (Cloudflare,
nginx, Varnish) default to caching `application/json` for **15 min – 4
h** when no directive is set. Observed against a public
Cloudflare-fronted CoreScope instance (`meshcore.meshat.se`):
- 17 consecutive polls of `/api/observers` over ~10 min returned
byte-identical responses
- Response headers showed `cf-cache-status: HIT`, `age: 878` (~15 min)
- Cache-busting query param → `cf-cache-status: MISS` with fresh
`last_seen` values
This causes WebSocket pushes to diverge from REST GETs (WS fresh, REST
stale) and produces false-positive stale/online flips for observers near
the 10-min threshold.
## Fix
New `noStoreAPIMiddleware` in `cmd/server/routes.go` wired into the
gorilla/mux chain alongside the existing `backfillStatusMiddleware`.
Sets `Cache-Control: no-store` on every response whose request path
starts with `/api/`.
## Design choice: `no-store` vs `private, max-age=0`
Chose `no-store`. CoreScope's REST endpoints are fresh-on-every-request
by contract (WS pushes diff against REST GETs), so any intermediary
cache is wrong. `no-store` forbids **any** cache (CDN, browser,
intermediary). `private, max-age=0` still permits short browser caches
and some intermediaries — no benefit here.
## Scope discipline
- `/api/` prefix only.
- Static assets (`/`, `/app.js`, `/style.css`, …) keep their existing
`no-cache, no-store, must-revalidate` headers from `spaHandler` in
`main.go`. Hashed assets stay CDN-cacheable by design.
- The middleware runs for **all** registered routes including the
websocket upgrade HTTP request, since `/ws` is served through the same
mux.
## TDD
- **Red** `1beb5432`: `cmd/server/cache_control_api_test.go` asserts
`Cache-Control: no-store` on `/api/stats`, `/api/observers`,
`/api/packets`, `/api/nodes`, and asserts the middleware does NOT leak
onto `/` or `/app.js`. Fails on assertion (no Cache-Control header
emitted) — not a compile error.
- **Green** `13be675f`: middleware + wiring. All assertions pass; full
`cmd/server` suite stays green.
## Files
- `cmd/server/routes.go` — middleware definition +
`r.Use(noStoreAPIMiddleware)`
- `cmd/server/cache_control_api_test.go` — 6 sub-tests across 2
top-level tests
## Preflight
`bash ~/.openclaw/skills/pr-preflight/scripts/run-all.sh origin/master`
→ clean (exit 0).
---------
Co-authored-by: corescope-bot <bot@corescope>
## Summary
Closes the "XSS regression in newly-added sink" class. Follow-up to
#1537 (10 stored-XSS sinks in node names) and the post-#1537 audit
(TRACE-1, OBS-1, ANL-1 — 3 additional HIGH XSS in files #1537 didn't
touch).
After those fixes land, the project still has **zero automated catch for
the next one**. Every future PR can re-introduce the same class freely.
This PR closes that gap with a hard-fail pr-preflight gate that runs at
PR-creation time and in CI.
## What the gate does
A NEW or MODIFIED line in the PR diff under `public/**/*.{js,html}` is
flagged when it matches any of these sink patterns:
| Pattern | What it catches |
|---|---|
| `.innerHTML = \`…\`` / `'…'` | template-literal or string-concat HTML
injection |
| `insertAdjacentHTML(…, \`…\`)` | DOM-adjacent injection |
| `.bindPopup(\`…\`)` / `.bindTooltip(\`…\`)` | Leaflet popup/tooltip
injection (the OBS-1 class) |
| `.setAttribute('on<event>', …)` | inline event-handler injection |
| `.setAttribute('href'\|'src'\|'action'\|'formaction', <interp>)` |
`javascript:` URI class |
For each flagged line, the gate then walks the dynamic substring
(`${…}`, post-`+`, or `setAttribute` value arg) and only fires if it
interpolates an identifier from the node-controlled allowlist (`name`,
`observer`, `sender`, `pubkey`, `body`, `hash`, …). This keeps the regex
off static CSS classes like `text-center`.
A flagged line is accepted (no fail) when ANY of:
- **(a)** wrapped in `escapeHtml(` / `escapeAttr(` / `safeEsc(` / local
`esc(` — the audited helpers
- **(b)** a same-PR `test*.js` file DOM-greps the audit payload (`'
onfocus=` or `onerror=alert`) AND references the sink file's basename
- **(c)** the PR body carries `PREFLIGHT-XSS-OPTOUT: <file>:<line>
reason="…"` — explicit author opt-out logged for reviewer attention
Otherwise: **HARD FAIL** with `file:line: flagged: <token>` plus a
suggested fix.
## Split
- **Skill directory** (local, no PR):
- `~/.openclaw/skills/pr-preflight/scripts/check-xss-sinks.sh` —
canonical gate
- `~/.openclaw/skills/pr-preflight/data/xss-node-controlled-fields.txt`
— allowlist (27 identifiers, easy to extend without a repo PR)
- wired into `~/.openclaw/skills/pr-preflight/scripts/run-all.sh`
- **This PR** (in repo):
- `testdata/preflight-xss/` — fixtures (`bad-1..bad-3`,
`good-1..good-2`, `test-good-2.js`)
- `scripts/check-xss-sinks.sh` — local mirror of the canonical gate, so
CI can exercise the gate without depending on the skill dir
- `test-preflight-xss-gate.js` — Node test wrapper that asserts bad
fixtures fail (exit 1) and good fixtures pass (exit 0)
- `public/app.js` — `escapeHtml` docstring marked CANONICAL with links
to the enforcing gate
- `.github/workflows/deploy.yml` — invoke `node
test-preflight-xss-gate.js` alongside the existing
`test-xss-escape-sinks.js`
## TDD red → green
| | Commit | Test result |
|---|---|---|
| **Red** | `test(preflight-xss): RED — fixtures + assertion wrapper for
XSS sink gate` | `test-preflight-xss-gate.js` exits 1 — bad fixtures
unexpectedly pass because `scripts/check-xss-sinks.sh` is a no-op stub.
Genuine assertion failure (not a build error). |
| **Green** | `feat(preflight): GREEN — implement XSS-sink check +
escapeHtml docstring` | stub replaced with real check; all 5 fixtures
behave as expected. |
The red commit ships a working stub script so the test runs to
completion and fails on an **assertion**, not on a missing-file error.
## Coverage proof — would the gate have caught the originals?
- **PR #1537 (10 sinks):** synthetic file from the deleted lines of
#1537 → gate flags `n.name` in `innerHTML \`tpl\`` and two
`bindPopup(\`…${n.name}\`)` lines. Yes, the gate would have caught these
the moment they hit a PR diff.
- **Post-#1537 audit:**
- **TRACE-1** (`traces.js` `${e.message}` / `${urlHash}` in innerHTML):
yes — the `hash`/`urlHash` tokens are allowlisted and the innerHTML
template-literal pattern matches.
- **OBS-1** (`observer-detail.js` URL fragment + MQTT fields into
innerHTML / bindPopup): yes — the `observer`, `text`, `hash` tokens are
allowlisted and both sink patterns match.
- **ANL-1** (`analytics.js` attribute-mutation roundtrip): yes for
`setAttribute('on*', …)` and `setAttribute('href', \`…${interp}…\`)`
patterns. (Note: pure innerHTML lines with only `${e.message}` are not
node-controlled and are intentionally not flagged.)
## Allowlist (initial 27 identifiers)
```
adv_name name observer observer_name sender from_node channel channel_name
model firmware client_version radio iata
hopNames nodeLabel obsName n.name o.name obs.name
public_key pubkey area_key region_name
text body message preview
hash urlHash
```
Extend in
`~/.openclaw/skills/pr-preflight/data/xss-node-controlled-fields.txt`
whenever a new node-controlled field surfaces in an audit — no repo PR
required.
## Hard rules respected
- No build step, no ESLint plugin, no AST analysis — grep + heuristics +
opt-out escape valves
- Hard fail (exit 1), not warning-only (exit 2)
- PII preflight grep on every commit + this PR body
- Same split as the sibling migration-gate PR
## Three-axis merge-readiness
- **Mergeable:** yes — branch is clean off `origin/master`, no conflicts
- **CI:** will report on push; red commit expected to fail, green commit
expected to pass
- **Threads:** none open yet (new PR)
---------
Co-authored-by: meshcore-bot <bot@local>
Co-authored-by: mc-bot <bot@meshcore.local>
Co-authored-by: corescope-bot <bot@corescope>
Follow-up to merged #1540. Self-review of #1540 found 3 additional
`log.Printf` sites interpolating MQTT-controlled strings without
`sanitizeLogString` — fixing here for completeness.
## Sites fixed
| File:line | Format | MQTT-controlled fields | Attacker scenario |
|---|---|---|---|
| `cmd/ingestor/main.go:531` | `status: %s (%s)` | `name`, `iata` |
Hostile node sends status with `name="evil\r\n[security] forged-line"` —
appears as a fake log line in operator dashboards / journalctl. |
| `cmd/ingestor/main.go:854` | `channel message: ch%s from %s` |
`channelIdx`, `sender` | Attacker spoofs `sender="evil\r\n[security]
backdoor-installed"` on any channel message — same forged-line outcome.
|
| `cmd/ingestor/main.go:940` | `direct message from %s` | `sender` | DM
injection via crafted sender field, same outcome. |
All three now route through `sanitizeLogString` from
`cmd/ingestor/sanitize_log.go` (added by #1540) which replaces
CR/LF/control bytes with `?`.
## TDD
Red commit (`8b3ad398`) adds 3 testable format helpers
(`formatStatusLog`, `formatChannelMessageLog`, `formatDirectMessageLog`)
plus tests pinning CR/LF stripping. Helpers return raw `fmt.Sprintf`
output, so tests fail on assertion (not build).
Green commit applies `sanitizeLogString` inside the helpers and swaps
the 3 call sites in `main.go` to use them.
Tests red-on-revert (verified locally).
## Scope
Strictly the 3 sites above. No other refactors. No changes to
`sanitizeLogString` itself.
---------
Co-authored-by: clawbot <clawbot@users.noreply.github.com>
🚧 Draft — red commit only. Tests added are expected to FAIL; fix lands
in next commit.
Follow-up to #1537 — security sweep found 3 additional stored XSS sinks
of the same class.
Once the green commit lands and CI is green, this body will be replaced.
---------
Co-authored-by: CoreScope Bot <bot@meshcore>
Co-authored-by: Kpa-clawbot <bot@kpa-clawbot.local>
Co-authored-by: openclaw-bot <bot@openclaw.local>
Kpa-clawbot