Files
meshcore-analyzer/test-preflight-xss-gate.js
T
e4a21fc9ab feat(preflight): hard-fail gate on unescaped node-controlled HTML sinks (#1543)
## 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>
2026-06-03 22:07:49 +00:00

102 lines
4.3 KiB
JavaScript

#!/usr/bin/env node
// test-preflight-xss-gate.js — exercises scripts/check-xss-sinks.sh against
// the testdata/preflight-xss fixtures. Asserts the bad fixtures HARD-FAIL
// (exit 1) and the good fixtures pass (exit 0).
//
// This is the repo-side validation of the canonical pr-preflight gate
// documented at ~/.openclaw/skills/pr-preflight/scripts/check-xss-sinks.sh.
// The skill-side script enforces the gate at PR-creation time; this test
// guards against regressions in the local mirror at scripts/check-xss-sinks.sh.
//
// Each fixture line is a behavioral assertion:
// bad-* MUST fail — proves the gate catches the unescaped sink class.
// good-* MUST pass — proves the gate doesn't false-positive on escaped
// or test-covered sinks.
//
// Exit 1 on any assertion failure.
'use strict';
const { spawnSync } = require('child_process');
const path = require('path');
const fs = require('fs');
const SCRIPT = path.resolve(__dirname, 'scripts/check-xss-sinks.sh');
const FIXTURE_DIR = path.resolve(__dirname, 'testdata/preflight-xss');
if (!fs.existsSync(SCRIPT)) {
console.error(`FAIL: ${SCRIPT} missing`);
process.exit(1);
}
if (!fs.existsSync(FIXTURE_DIR)) {
console.error(`FAIL: ${FIXTURE_DIR} missing`);
process.exit(1);
}
// Each case may specify which test file(s) to expose via PREFLIGHT_TEST_FILES.
const cases = [
// bad fixtures: gate MUST flag (exit 1)
{ file: 'bad-1-template-literal.js', expect: 1, label: 'innerHTML template literal with ${name}' },
{ file: 'bad-2-setAttribute-href.js', expect: 1, label: "setAttribute('href', `…${hash}…`)" },
{ file: 'bad-3-bindPopup.js', expect: 1, label: 'Leaflet bindPopup(`…${observer}…`)' },
{ file: 'bad-4-bare-ident.js', expect: 1, label: 'innerHTML = name (bare ident, no quote/backtick)' },
{ file: 'bad-5-string-concat.js', expect: 1, label: 'innerHTML = name + "<b>" (concat)' },
{ file: 'bad-6-bindPopup-concat.js', expect: 1, label: 'bindPopup("Name: " + observer) (concat)' },
{ file: 'bad-7-outerHTML.js', expect: 1, label: 'outerHTML sink' },
{ file: 'bad-8-document-write.js', expect: 1, label: 'document.write template literal' },
{ file: 'bad-9-shamtest-fixture.js', expect: 1, label: 'sham test (markers only in comments)',
tests: ['sham-test-fixture-9.js'] },
{ file: 'bad-10-line-level-escape-bypass.js', expect: 1,
label: '${escapeHtml(role)} ${name} — one wrapped, one raw' },
{ file: 'bad-11-comment-rubber-stamp.js', expect: 1,
label: 'escapeHtml mentioned only in // comment' },
{ file: 'bad-12-setattr-concat.js', expect: 1,
label: 'setAttribute("href", "javascript:" + payload) (concat, no $)' },
// 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',
tests: ['test-good-2.js'] },
{ file: 'good-3-catch-block-error.js', expect: 0,
label: 'catch (e) { ${e.message} } — exception property' },
{ file: 'good-3b-chained-cause.js', expect: 0,
label: '${error.cause.message} / ${parseError.message} chained error props' },
{ file: 'good-4-tested.js', expect: 0,
label: 'unescaped sink, REAL test (markers in executable code)',
tests: ['test-good-4.js'] },
];
let failed = 0;
for (const c of cases) {
const target = path.join(FIXTURE_DIR, c.file);
if (!fs.existsSync(target)) {
console.error(`FAIL: fixture missing: ${target}`);
failed++;
continue;
}
const tests = (c.tests || []).map(t => path.join(FIXTURE_DIR, t));
const env = Object.assign({}, process.env);
if (tests.length > 0) {
env.PREFLIGHT_TEST_FILES = tests.join(':');
} else {
delete env.PREFLIGHT_TEST_FILES;
}
const res = spawnSync('bash', [SCRIPT, '--file', target], {
env,
encoding: 'utf8',
});
const got = res.status;
if (got !== c.expect) {
console.error(`FAIL: ${c.file} (${c.label}) — expected exit ${c.expect}, got ${got}`);
if (res.stdout) console.error(' stdout:', res.stdout.trim());
if (res.stderr) console.error(' stderr:', res.stderr.trim());
failed++;
} else {
console.log(`PASS: ${c.file}${c.label} (exit ${got})`);
}
}
if (failed > 0) {
console.error(`\n${failed} assertion(s) failed.`);
process.exit(1);
}
console.log('\nAll preflight-xss-gate assertions passed.');