fix(ui): sanitize repository markdown and lock down what the page may load - #12
Merged
Conversation
… load markdown-preview ran rehype-raw with no sanitizer, so a markdown file in the diff was rendered as live HTML. React strips onerror and javascript: URLs, but script, iframe, object, embed, link and style elements survived - and an iframe pointing at an attacker origin was enough to load their JS and read the API from there. Markdown now passes through rehype-sanitize with GitHub's default schema, which keeps class="language-…" on code so the highlighter still works. A Content-Security-Policy on every response closes the network sinks for the whole UI, including remote images in comment bodies pulled from GitHub, which sanitizing markdown does not cover. script-src still needs 'unsafe-inline' for the inline scripts in the built index.html; the sanitizer is what keeps injected script out of the DOM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
tsc could not name the inferred type without reaching into node_modules/hast-util-sanitize (TS2883). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
The CSP blocked WebAssembly.instantiate, so shiki never initialised and the diff lost all syntax highlighting. 'wasm-unsafe-eval' permits WASM compilation without permitting eval. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audit finding P1-5.
markdown-preview.tsxranrehype-rawwith no sanitizer, so a markdown filein a diff was rendered as live HTML. React strips
onerrorand neutralisesjavascript:URLs, butthese survived as live elements:
script,iframe,object,embed,link,style,base.The
iframealone completed a chain without needing script execution: it loaded attacker JS in theattacker's own origin, which could then read
http://localhost:5391/api/diff— authorised by theACAO: *that #5 removed — and post a private diff out. Reviewing a PR that touches aREADME.mdwas sufficient.
rehype-sanitizewith GitHub's default schema, which keepsclass="language-…"oncodeso the highlighter still worksContent-Security-Policyon every response closes the network sinks for the whole UI, including remote images in comment bodies pulled from GitHub — which sanitizing markdown does not covertests/**/*.test.tsxadded to the UI vitest include, which previously matched.tsonly and silently skipped this filescript-srcstill needs'unsafe-inline'for the three inline scripts in the builtindex.html, sothe sanitizer is what keeps injected script out of the DOM and the CSP is what stops anything that
does run from reaching the network. Hash-based
script-srcwould remove that caveat; it needs thehashes computed from
index.htmlat server start.Verified on a live instance:
Note the trade-off: remote images in a README no longer render, badges included. That is the point —
they were beacons that fired on preview.
Stacked on #11.
🤖 Generated with Claude Code
https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs