fix: replace innerHTML with safer DOM methods

[cass-clearly]

What changed

Audited all innerHTML usage across the client-side codebase (sidebar.js, ui.js, index.js, toast.js) and replaced unsafe patterns with safer DOM methods:

• User-supplied content (comment author names, quoted text, empty state messages): replaced innerHTML with textContent or createElement/appendChild
• Clearing elements: replaced innerHTML = "" with .replaceChildren()
• Markdown-rendered content (comment bodies): added DOMPurify as a defense-in-depth sanitizer via a centralized sanitizeHtml() wrapper
• Static HTML templates (form layouts, SVG icons): left as innerHTML since they contain no user data
• Toast messages: replaced innerHTML with textContent
• Built bundle (serve/feedback-layer.js): regenerated with the same changes

Added jsdom as a dev dependency and created sanitize.test.mjs with 7 tests covering script stripping, event handler removal, safe tag preservation, javascript: URL blocking, and edge cases.

Why

Closes #188. Using innerHTML with user-supplied content is an XSS vector. Even though renderMarkdown() escapes HTML first, adding DOMPurify provides defense-in-depth, and replacing innerHTML with textContent where possible eliminates the risk entirely.

How to verify

1. Run the full test suite:

   npm run check
   

   All 235 tests pass (137 server + 98 client), lint clean, 100% coverage on sanitize.js.

2. Verify the new sanitize tests specifically:

   npm run test:client
   

   Look for the sanitizeHtml test suite (7 tests).

3. Test in browser: open a document, create comments with markdown (**bold**, *italic*, `code`), verify rendering works. Try pasting <script>alert(1)</script> in a comment body — it should render as escaped text.

Manual testing checklist

• Existing tests pass (npm test)
• Server starts without errors (npm run start)
• Tested in browser (annotations, sidebar, highlights work)
• No console errors in browser DevTools

[cass-clearly]

Review: fix: replace innerHTML with safer DOM methods (#188)

Verdict: APPROVE (posting as comment because GitHub blocks self-approval)

Test Results

• ✅ All 98 client tests pass, 99.68% coverage (including 7 new sanitize tests)
• ✅ All 137 server tests pass, 98.63% coverage
• ✅ Build succeeds, bundle regenerated correctly

What's Good

1. Comprehensive audit. Every innerHTML usage across sidebar.js, ui.js, toast.js has been addressed — user-supplied content now uses textContent or createElement, clearing uses .replaceChildren().

2. Defense-in-depth with DOMPurify. Markdown-rendered content gets sanitized via a clean sanitizeHtml() wrapper. Even if renderMarkdown() escapes incorrectly or is bypassed, DOMPurify provides a second layer.

3. Correct DOMPurify integration. Default config is secure: strips <script>, event handlers, and javascript: URLs. The test suite verifies all of these cases.

4. Static templates left alone. Form layouts and SVG icons still use innerHTML because they contain no user data — correct call, avoids over-engineering.

5. Test quality. The 7 new tests cover the important cases: script removal, event handler stripping, safe tag preservation, URL sanitization, empty/null inputs.

6. PR description accurate. Every claim matches the implementation.

What Changed

• sidebar.js: 15 replacements (user names, quotes, empty states, list clearing)
• ui.js: 2 replacements (quote display)
• toast.js: 2 replacements (message display)
• sanitize.js: New DOMPurify wrapper
• sanitize.test.mjs: 7 tests with JSDOM
• Bundle regenerated

No Issues Found

No security holes, no regressions, no over-engineering. This is textbook defense-in-depth hardening. Ready to merge after Chris reviews.

[cass-clearly]

Review: fix: replace innerHTML with safer DOM methods

Verdict: Approve (posted as comment because GitHub blocks self-approval)

What's Good

• DOMPurify is the right choice for XSS sanitization — battle-tested, widely used, explicitly designed for this use case
• The sanitizeHtml() wrapper in utils/sanitize.js provides a clean abstraction — future code can call one function rather than scattering DOMPurify.sanitize() calls
• Switching from innerHTML to textContent/createElement for non-HTML content is the right defense-in-depth approach
• jsdom added as a dev dependency for testing in Node (where DOMPurify needs a DOM environment)
• Tests updated to cover sanitization behavior

Observations (non-blocking)

1. Bundle size: DOMPurify adds ~22KB minified. The feedback-layer bundle was already optimized (#73). Not a blocker, but worth noting in the PR for visibility.
2. Large package-lock.json diff: The jsdom dev dependency pulls in a large transitive tree. All dev-only (won't affect the built bundle size), but the diff is noisy.
3. ui.js still uses document.querySelector(...).innerHTML for reading docHtml in the HF modal — this is reading content (not writing), so it's not an XSS vector. Correct to leave it.

Solid security fix. The DOMPurify + textContent approach is the right layered defense. Approve.