feat(security): frame-ancestors on the viewer + noreferrer on external links#210
Merged
Conversation
…l links Harden the trusted viewer origin (shared with the authenticated API and the comment→agent channel) against the two OSS-side gaps flagged alongside the hosted iframe-CSRF work: - Clickjacking: the viewer HTML now sends `Content-Security-Policy: frame-ancestors 'self'` via the single configuredViewerHtml chokepoint (covers `/`, `/session/:id`, and the standalone `/s/:id` post view). The sandboxed `/s/:id?part=N` surface documents are meant to be framed and keep their own `sandbox` CSP, so they're untouched. - Referrer leak: external links the viewer opens — the openLink bridge's `window.open`, release-notes markdown links, and the image/trace/footer anchors — now carry `noreferrer` alongside `noopener`, so the current URL (which can hold the `?key=` deploy token) never rides an outbound Referer. Tests: api.test asserts frame-ancestors on the viewer routes (and its absence of the sandbox CSP); a new notes.test pins noreferrer on rendered external links. Changeset: patch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BLz2z8mwodKkzr8x1NapM
…noopener on Card links Address sonnet review nits on the iframe-CSRF hardening: - api.test now asserts surface docs (/s/:id?part=N) never carry frame-ancestors (the invariant that keeps them framable), and that the nested post-permalink alias (/session/:id/p/:id) is frame-ancestors protected like its siblings. - Card.tsx's two bare target="_blank" links (open-in-tab, open-as-PNG) get rel="noopener" for reverse-tabnabbing hygiene. They're same-origin, so no noreferrer (keeps the useful same-origin Referer) — the external links keep noopener noreferrer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BLz2z8mwodKkzr8x1NapM
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
2 participants
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.
What
The self-hosted OSS-side counterpart to the hosted iframe-CSRF hardening (sideshow-cloud #172/#173). Closes two gaps in the trusted viewer origin — which shares its origin with the authenticated API and the comment→agent channel.
1. Clickjacking —
frame-ancestors 'self'on the viewer HTML.The viewer HTML now sends
Content-Security-Policy: frame-ancestors 'self', set once in theconfiguredViewerHtmlchokepoint so it covers every viewer-HTML route (/,/session/:id, and the standalone/s/:idpost view). Without it, a cross-origin page could frame the authenticated viewer and clickjack actions or the prompt-injection channel.2. Referrer leak —
noreferreron external-link opens.Self-hosted auth puts the deploy token in the URL (
?key=<token>). Anything that sends aRefererfrom the viewer can leak it. External links the viewer opens now carrynoreferreralongside the existingnoopener:viewer/src/App.tsx— theopenLinkbridge'swindow.open(...)(arbitrary external destination — the primary vector) and the footer link.viewer/src/notes.ts— release-notes markdown links.viewer/src/ImageSurface.tsx,viewer/src/TraceSurface.tsx— the asset open/download anchors (same-origin today, normalized for consistency).Deliberately not covered
The sandboxed
/s/:id?part=Nsurface documents are meant to be framed by the viewer and are neutralized by their ownsandboxCSP response header. They don't pass throughconfiguredViewerHtml, so they never receiveframe-ancestors— verified by test. A bareframe-ancestorsdirective doesn't restrict script/style-src, so the singlefile viewer's inline scripts/styles are unaffected.Testing
test/api.test.ts: the viewer shell (/s/:id),/, and/session/:idnow assertframe-ancestors 'self'and the absence of the sandbox CSP; the existing?part=Nsurface-doc assertions (sandbox present) are untouched.test/notes.test.ts(new): pinsrel="noopener noreferrer"on rendered external links.npm test(422),npm run typecheck,npm run lint,npm run format:check,npm run security:audit;npm run build:viewercompiles the JSX edits cleanly.patch).Behavior change to note
Cross-origin iframe embedding of the self-hosted viewer HTML is now refused. The sanctioned embed path is the embeddable engine (
mountViewer, a JS/shadow-root mount), not iframing the viewer document, so this shouldn't affect real embedders — flagging it explicitly in case any self-hoster relies on iframing the full viewer.🤖 Generated with Claude Code
https://claude.ai/code/session_017BLz2z8mwodKkzr8x1NapM
Generated by Claude Code