Skip to content

fix(ui): sanitize repository markdown and lock down what the page may load - #12

Merged
fiddur merged 3 commits into
developfrom
sanitize-markdown
Aug 21, 2026
Merged

fix(ui): sanitize repository markdown and lock down what the page may load#12
fiddur merged 3 commits into
developfrom
sanitize-markdown

Conversation

@fiddur

@fiddur fiddur commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Audit finding P1-5. markdown-preview.tsx ran rehype-raw with no sanitizer, so a markdown file
in a diff was rendered as live HTML. React strips onerror and neutralises javascript: URLs, but
these survived as live elements: script, iframe, object, embed, link, style, base.

The iframe alone completed a chain without needing script execution: it loaded attacker JS in the
attacker's own origin, which could then read http://localhost:5391/api/diff — authorised by the
ACAO: * that #5 removed — and post a private diff out. Reviewing a PR that touches a README.md
was sufficient.

  • 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
  • tests/**/*.test.tsx added to the UI vitest include, which previously matched .ts only and silently skipped this file

script-src still needs 'unsafe-inline' for the three inline scripts in the built index.html, so
the 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-src would remove that caveat; it needs the
hashes computed from index.html at server start.

Verified on a live instance:

GET /diff                      → Content-Security-Policy: default-src 'self'; … connect-src 'self'
GET /api/tree/raw/package.json → application/octet-stream; Content-Disposition: attachment
GET /api/tree/raw/logo.png     → image/png

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

fiddur and others added 2 commits August 21, 2026 11:03
… 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
@fiddur
fiddur changed the base branch from path-containment to develop August 21, 2026 13:34
@fiddur
fiddur marked this pull request as ready for review August 21, 2026 13:34
@fiddur
fiddur merged commit 88d8c09 into develop Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant