Skip to content

Follow-up: complete XSS hardening beyond presence title trust #12

Description

@LynnColeArt

Summary

Hi team 👋 — this issue is a friendly follow-up to PR #11.

PR #11 hardened the real-time presence channel so the "Currently Being Consulted" panel no longer trusts untrusted payload text and now resolves titles from server-owned data.

Objective

Complete broader XSS hardening (and close remaining injection-adjacent gaps) across user-controlled content paths, while keeping the current UI behavior intact.

Concrete implementation plan

1) Inventory and triage all user-input sinks (quick pass)

  • Enumerate every request/response path that accepts user input (query params, bodies, route params, WS messages).
  • Map each path to the DOM sink:
    • Raw HTML injection points (dangerouslySetInnerHTML, HTML parsing/rewrite output)
    • Text-inserting contexts ({value}, setState + render)
    • Attribute contexts (links, URLs, data attributes)
  • Mark each as Low / Medium / High risk and prioritize fixes.

2) Comments path: input validation + output encoding

Goal: comments body/content stays text-safe and cannot break out of React render context.

Files likely involved:

  • src/worker/comments.ts
  • src/client/Comments.tsx

Checklist:

  • Add/confirm server-side max-length checks for comment body (and any other user-provided fields).
  • Add normalization/trimming + reject obviously dangerous control patterns before DB write:
    • <, >, on*=/javascript: in server-side validation (at minimum log + hard reject, or escape strategy for legacy compatibility).
  • Ensure duplicate/empty content is rejected consistently with current API semantics.
  • Confirm React rendering path keeps comment body as text (no HTML parsing).
  • Add regression tests or manual check with payloads such as <img src=x onerror=alert(1)> and javascript: links.

3) Sidebar/presence and title surfaces

Goal: preserve PR #11 trust boundary and lock down any remaining title/title-like paths.

  • Confirm all presence clients send only { t: "r", s }.
  • Confirm top payload titles are only sourced from trusted metadata/fallback.
  • Add a tiny assertion/validation in src/worker/presence.ts that rejects absurdly long/empty slugs post-slugify.
  • Keep ti out of any inbound message contract definitions/docs.

4) Link/title metadata hardening for rendered HTML

Files:

  • src/worker/sanitize.ts

Checklist:

  • Add regression cases for javascript:, data:, vbscript:, protocol-relative, and uppercase variants in hrefs.
  • Ensure attribute escaping also handles ' consistently where relevant.
  • Verify disallowed tags are removed even when malformed/unbalanced/obfuscated.
  • Add focused unit-style checks for sanitizer output with crafted payloads.

5) Route/query handling sanity checks

Files:

  • src/client/App.tsx
  • src/worker/index.ts (search/page handlers)
  • any route handlers that reflect slug/q directly

Checklist:

  • Verify slugify/query normalization path is applied before any cache/db/file-system/identifier usage.
  • Confirm no user-controlled field is rendered as untrusted HTML in headers, title, or DOM attributes.
  • Confirm search query handling does not expose reflected HTML in error/path contexts.

6) Transport-layer hardening

  • Add/verify Content-Security-Policy and related safe-response headers (as practical for this app/Cloudflare setup).
  • Ensure X-Content-Type-Options: nosniff and frame protection headers are set consistently.
  • Confirm browser-side strict noopener links where external targets are used.

7) Verification pass (acceptance checklist)

  • Run a manual payload sweep on the top 10 highest-risk routes.
  • Confirm no script execution in browser for: comments, article HTML render, title displays, and presence panel.
  • Add at least 1 regression case per bullet above in the issue if not already covered by tests.
  • Update issue with pass/fail status per bullet.

Acceptance criteria

  • No user-supplied string can directly control DOM HTML insertion outside approved sanitize/render paths.
  • "Currently Being Consulted" remains stable and trusted (as in PR Harden presence titles using server-owned metadata #11).
  • No newly introduced regressions in existing user flows (comments, article read, search, sidebar).

Notes

If this helps, I can split the above into milestone-sized PRs (ex: #A sanitizer hardening, #B comment validation, #C headers/CSP). That usually keeps review faster and safer.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions