Harden CI review workflow and markdown viewers

[chrisnestrud]

Summary

• harden the privileged Claude review workflow by gating fork PR execution, pinning third-party actions by SHA, and making maintainer label approval operational
• harden the desktop markdown viewer by stripping remote-resource HTML, locking the embedded WebView to an explicit internal document URL, and adding backend-free security regressions
• harden the web markdown viewer by centralizing sanitizer policy, expanding the read-only denylist, adding config-aware Node security tests, and moving the bootstrap loader out of inline HTML so CSP can be enforced

Test Plan

• python3 -m pytest clients/desktop/tests/test_markdown_viewer_security.py -v
• node --test clients/web/tests/markdown_viewer_security.test.mjs
• rg --pcre2 -n "<script(?! src=)" clients/web/index.html

[zarvox32]

Reviewed and tested. Both new suites pass locally on Windows:

• clients/desktop/tests/test_markdown_viewer_security.py — 7 passed
• clients/web/tests/markdown_viewer_security.test.mjs — 2 passed
• rg "<script(?! src=)" clients/web/index.html — no matches

Likely issue worth fixing before merge

Desktop external links silently dead-end. _on_webview_navigating vetoes any URL whose base differs from playpalace://document/viewer. Anchors (#fragment) still work, but clicking an https:// or mailto: link in a rendered document now does nothing — no webbrowser.open fallback, no message, no speech. Per CLAUDE.md's "silence is a bug" rule this is a regression. Suggest: in the veto branch, call webbrowser.open(url) for schemes that pass the same allowlist as _ALLOWED_URL_SCHEMES.

Minor

• CSP connect-src 'self' ws: wss: — scheme-only sources allow WebSocket to any host. Probably wider than intended; tightening to a configured origin would close a small exfil vector.
• Test fakes for nh3 / markdown are regex-based and much weaker than real nh3. The tests verify wiring (does the dialog call sanitize/veto?) but not policy (_ALLOWED_TAGS, _ALLOWED_ATTRIBUTES, _ALLOWED_URL_SCHEMES). One additional test that imports the real library — skipped if not installed — would catch policy regressions.
• MARKDOWN_DOCUMENT_BASE_URL is a sentinel custom scheme, not a registered handler. Worth a one-line comment so a future reader doesn't wonder why nothing handles playpalace://.
• link_rel="noopener noreferrer" in sanitize_markdown is dead weight on desktop since external nav is vetoed anyway, but it's correct defense-in-depth.

Sections better suited to a human reviewer than an AI

1. The two pinned action SHAs in .github/workflows/claude-code-review.yml (actions/checkout@34e1148..., anthropics/claude-code-action@51ea8ea..., currently 5 commits behind the v1 tag tip). Both resolve to real commits, but confirming each is a trusted, audited release — not a malicious or fork-introduced commit — is a judgment call requiring repo authority. SHA-pinning is right; which SHA is the human part.
2. The safe-to-run-claude-review label policy — criteria for applying it, whether a documented checklist is needed, rollback plan if it's applied carelessly. Organizational, not code-correctness.
3. CSP suitability for production — whether style-src 'unsafe-inline', connect-src ws:, and img-src data: match the actual deployment posture. I can validate syntax; not threat model.
4. The silent-veto UX trade-off flagged above — if "no external navigation, period" is the intended posture, accessibility users may still disagree about silence vs. message. Human call.
5. Completeness of FORBID_TAGS in clients/web/ui/markdown_viewer.js. DOMPurify has secure defaults; the explicit denylist is belt-and-suspenders but denylists drift. An allowlist would be more durable but a bigger change.

Recommendation: approve with the silent-veto link fix addressed. Remaining items are nits or human-judgment calls.