Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,14 @@ If you need to know how something works — a flag, a field, a timing, a behavio
### Theming
All colors MUST use CSS variables. Never hardcode `#hex` values outside of `:root` definitions. The customizer controls colors via `THEME_CSS_MAP` in customize.js. If you add a new color, add it as a CSS variable and map it in the customizer.

### XSS preflight gate (CI)
`scripts/check-xss-sinks.sh --diff` runs on every PR and flags HTML sinks (`innerHTML`, `bindPopup`, URL/`on*` `setAttribute`, …) that interpolate a node-controlled field. In CI a flagged line passes only via:
- an escape helper on the same line (`escapeHtml` / `escapeAttr` / `esc`), or
- building the DOM with `createElement` + `textContent` / non-URL `setAttribute` instead of an HTML string — **the preferred fix for false positives**, or
- `PREFLIGHT-XSS-OPTOUT: <file>:<line> reason="…"` in the PR body **plus** the `xss-optout` label.

Existing or same-PR tests do **not** count in CI: the workflow never sets `PREFLIGHT_TEST_FILES`, so that route only applies to local/skill-side runs that set it.

### Shared Helpers (roles.js)
- `getNodeStatus(role, lastSeenMs)` → 'active' | 'stale'
- `getHealthThresholds(role)` → `{ staleMs, degradedMs, silentMs }`
Expand Down
22 changes: 21 additions & 1 deletion scripts/check-xss-sinks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@
# and apply the same rules to added lines only.
# BASE defaults to origin/master.
#
# CI scope: the PR-only `--diff` step in .github/workflows/deploy.yml sets
# PREFLIGHT_PR_BODY and PREFLIGHT_PR_LABELS but NEVER PREFLIGHT_TEST_FILES.
# The same-PR DOM-grep test route therefore only applies to local and
# skill-side runs that set the variable themselves. In CI a flagged sink
# passes only via an escape helper on the line, DOM construction
# (createElement + textContent / non-URL setAttribute) instead of an HTML
# string, or the PR-body opt-out + `xss-optout` label. This is deliberate:
# the sham-test check (basename + marker grep) is too weak to trust in CI.
# The fix hint mirrors this and only mentions the test route when
# PREFLIGHT_TEST_FILES is non-empty.
#
# The canonical pr-preflight gate (skill-side) consumes the same allowlist
# format documented inline below.
#
Expand Down Expand Up @@ -325,7 +336,16 @@ def emit_finding(file, lineno, token, sink):
print(f"ℹ️ {file}:{lineno}: flagged token '{token}' in {sink} — author opt-out in PR body (xss-optout label + ≥40ch reason)")
return False
print(f"❌ {file}:{lineno}: flagged: {token} (sink: {sink})")
print(f" fix: wrap with escapeHtml(...) / escapeAttr(...) — or add a DOM-grep test in test*.js asserting the payload renders inert — or add 'PREFLIGHT-XSS-OPTOUT: {file}:{lineno} reason=\"...(≥40 chars)...\"' to the PR body AND apply the xss-optout label.")
# Only offer the same-PR test route when this run can honor it. CI's
# --diff step never sets PREFLIGHT_TEST_FILES, so there it is dead.
test_route = (" — or add a DOM-grep test in test*.js (listed in PREFLIGHT_TEST_FILES)"
" asserting the payload renders inert") if TEST_FILES else ""
print(f" fix: wrap with escapeHtml(...) / escapeAttr(...) / esc(...)"
f" — or build the DOM with createElement + textContent (setAttribute for non-URL, non-on* attributes) instead of an HTML string"
f"{test_route}"
f" — or add 'PREFLIGHT-XSS-OPTOUT: {file}:{lineno} reason=\"...(≥40 chars)...\"' to the PR body AND apply the xss-optout label.")
if not TEST_FILES:
print(" note: PREFLIGHT_TEST_FILES is not set, so existing or same-PR tests do not count as coverage (CI never sets it).")
return True

def scan_lines(lines_with_no):
Expand Down
57 changes: 57 additions & 0 deletions test-preflight-xss-gate.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,63 @@ for (const c of cases) {
}
}

// Fix-hint wording. CI's --diff step never sets PREFLIGHT_TEST_FILES, so
// the same-PR DOM-grep test route is dead there. The hint must only offer
// that route when the variable is set and non-empty — otherwise authors
// (and reviewers) assume an existing test covers the sink and CI fails.
function runGate(file, testFiles) {
const env = Object.assign({}, process.env);
if (testFiles === undefined) delete env.PREFLIGHT_TEST_FILES;
else env.PREFLIGHT_TEST_FILES = testFiles;
return spawnSync('bash', [SCRIPT, '--file', path.join(FIXTURE_DIR, file)], {
env,
encoding: 'utf8',
});
}

const hintCases = [
{ label: 'PREFLIGHT_TEST_FILES unset → no DOM-grep test hint',
testFiles: undefined, domTestHint: false },
{ label: 'PREFLIGHT_TEST_FILES empty → no DOM-grep test hint',
testFiles: '', domTestHint: false },
// test-good-2.js does not name bad-1-template-literal.js, so the sink is
// still flagged — but the route exists in this run, so the hint stays.
{ label: 'PREFLIGHT_TEST_FILES set (non-covering) → DOM-grep test hint',
testFiles: path.join(FIXTURE_DIR, 'test-good-2.js'), domTestHint: true },
];

for (const h of hintCases) {
const res = runGate('bad-1-template-literal.js', h.testFiles);
const out = res.stdout || '';
const problems = [];
if (res.status !== 1) problems.push(`expected exit 1, got ${res.status}`);
if (out.includes('DOM-grep test') !== h.domTestHint) {
problems.push(`DOM-grep test hint ${h.domTestHint ? 'missing' : 'present'}`);
}
for (const needle of ['escapeHtml', 'textContent', 'PREFLIGHT-XSS-OPTOUT', 'xss-optout label']) {
if (!out.includes(needle)) problems.push(`hint missing "${needle}"`);
}
if (problems.length) {
console.error(`FAIL: hint — ${h.label}: ${problems.join('; ')}`);
console.error(' stdout:', out.trim());
failed++;
} else {
console.log(`PASS: hint — ${h.label}`);
}
}

// Every bad fixture run without test files must omit the dead route.
for (const c of cases.filter(c => c.expect === 1 && !c.tests)) {
const res = runGate(c.file, undefined);
if (res.status !== 1 || (res.stdout || '').includes('DOM-grep test')) {
console.error(`FAIL: hint — ${c.file} without PREFLIGHT_TEST_FILES mentions DOM-grep test (exit ${res.status})`);
console.error(' stdout:', (res.stdout || '').trim());
failed++;
} else {
console.log(`PASS: hint — ${c.file} omits DOM-grep test route in CI mode`);
}
}

if (failed > 0) {
console.error(`\n${failed} assertion(s) failed.`);
process.exit(1);
Expand Down
Loading