XSS preflight gate: only suggest the DOM-grep test route when it can apply

[dborup]

Problem

scripts/check-xss-sinks.sh always printed this fix hint: "or add a DOM-grep test in test*.js asserting the payload renders inert". CI's PR-only --diff step in .github/workflows/deploy.yml sets only PREFLIGHT_PR_BODY and PREFLIGHT_PR_LABELS. It never sets PREFLIGHT_TEST_FILES, so the test route can't work in CI.

In dborup/CoreScope#99, the hint led an agent and a reviewer to believe an existing test covered a flagged sink. It didn't, and CI failed. The fix there was to build the dialog with the DOM API.

Why the gate is not weakened

This PR changes only the wording of the hint and the comments. Detection, the allowlist, the opt-out rules and the workflow are unchanged. The test route stays out of CI on purpose, because the sham-test check (basename plus a marker grep) is too weak to trust there. In CI a flagged line still passes only through one of these:

• an escape helper on the line
• DOM construction instead of an HTML string
• the PR-body opt-out plus the xss-optout label

Changes

• scripts/check-xss-sinks.sh

  • The hint mentions the DOM-grep test route only when PREFLIGHT_TEST_FILES is non-empty.
  • Otherwise it suggests one of: escapeHtml / escapeAttr / esc, createElement + textContent (or setAttribute for non-URL, non-on* attributes), or opt-out + label. It also adds a note that tests do not count in this run.
  • The header comment now describes what CI covers.

• AGENTS.md: a short "XSS preflight gate (CI)" note under Frontend Conventions. It says what CI accepts and that DOM construction is the preferred fix for false positives.

• test-preflight-xss-gate.js: new assertions on the hint wording:

  • with PREFLIGHT_TEST_FILES unset, empty and set
  • for every bad fixture in CI mode

  The existing exit-code table is unchanged.

Hint before and after (CI mode, bad-1-template-literal.js)

Before:

fix: wrap with escapeHtml(...) / escapeAttr(...) — or add a DOM-grep test in test*.js asserting the payload renders inert — or add 'PREFLIGHT-XSS-OPTOUT: …' to the PR body AND apply the xss-optout label.

After:

fix: wrap with escapeHtml(...) / escapeAttr(...) / esc(...) — or build the DOM with createElement + textContent (setAttribute for non-URL, non-on* attributes) instead of an HTML string — or add 'PREFLIGHT-XSS-OPTOUT: …' to the PR body AND apply the xss-optout label.
note: PREFLIGHT_TEST_FILES is not set, so existing or same-PR tests do not count as coverage (CI never sets it).

Tests

• node test-preflight-xss-gate.js: 14 new assertions failed before the script change and pass after it. All 31 checks pass, including the 17 existing fixture exit-code cases.
• node test-xss-escape-sinks.js: 34 passed.
• CI's "Run JS unit tests" block run locally: all 73 scripts pass.
• check-xss-sinks.sh --diff origin/master on this branch: exit 0, because no public/ files changed.
• Scratch repro of feat(channels): shared hashtag channel proposals, with admin revoke #99: a commit adds dlg.innerHTML = helper(id, name) to public/ next to an existing test file that contains onerror=alert. In CI mode the old and new scripts both fail with exit 1; only the new script gives the correct hint. With PREFLIGHT_TEST_FILES set locally, the sink is accepted, which is exactly the behaviour CI does not have.

Perf

The only change is an extra string check when a finding is printed. The scanning path is not touched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Rd9Nh9Z2A9UgMupvLHCKUr

---

Generated by Claude Code