Repository navigation
fix(media): keep script-capable sources out of links and frames - #148
Open
pratikbuilds wants to merge 2 commits into
Open
pratikbuilds wants to merge 2 commits into
pratikbuilds wants to merge 2 commits into
Conversation
Addresses findings 2, 3 and 4 of corbitsdev#142. 2. The Download link reused the media-element allowlist, which lets a data:image/svg+xml through: safe in an <img>, but as a target="_blank" href it opens an SVG document that can run script. The link now uses a "link" sink (the old "frame" sink, renamed now that no frame uses it) that takes only http:, https: and blob:. A data: image still shows; it is just not offered for download. 3. PDFs rendered in an unsandboxed <iframe>, so a URL labelled as a PDF that serves HTML ran script, same-origin for blob: and same-origin http(s), and could navigate the top page. A short spike, run in Playwright's new-headless Chromium and in installed Chrome stable, showed Chrome refuses its PDF viewer in any sandboxed frame. It shows the "blocked" page for sandbox="", "allow-scripts", "allow-same-origin" and even "allow-scripts allow-same-origin", which would also void the sandbox for a same-origin URL. <object>/<embed> were not tried: per the HTML spec they follow the response's Content-Type, so they would not refuse HTML either. Proving the bytes are a PDF would mean fetching the URL, which needs CORS and goes against the package's no-fetching rule. So a PDF now gets the "No preview for this file." state with its Download link, which in Chrome opens the native viewer in a new tab. 4. When the open item fell off a shortened list, the index read as closed but stayed stored, so the viewer reopened when the list grew back. An effect keyed on falling out of range now commits the close: uncontrolled state clears, and a controlled caller gets one onIndexChange(null) even if it keeps passing the stale index.
Contributor
|
Triage: Fail @pratikbuilds, please address the following:
Triage will request changes. |
corbits-triage
Bot
added
Triage: Waiting for CI
Triage: Not Ready
and removed
Triage: Waiting for CI
labels
![corbits-triage[bot]](https://avatars.githubusercontent.com/u/275831056?s=60&v=4){kind=small}
|
All contributors have signed the CLA. |
cursor
Bot
force-pushed
the
claude/i142-media-viewer-af93aa
branch
from
October 30, 2026 12:10
e46130d to
ddce89f
Compare
This pull request now only keeps script-capable sources out of links and frames.
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.
Summary
Keeps script-capable sources out of links and frames in
MediaViewer, and closes a viewer whose item left the list (findings 2, 3 and 4).data:image/svg+xmlthrough. That is safe in an<img>, but as atarget="_blank"href it opens an SVG document that can run script. The link now uses a"link"sink (the internal"frame"sink, renamed) that accepts onlyhttp:,https:andblob:. Adata:image still shows; it just gets no Download link.<iframe>, so a URL labelled as a PDF that serves HTML ran script. Chrome refuses its PDF viewer in any sandboxed frame, and checking the bytes would mean fetching the URL, so a PDF now shows "No preview for this file." with its Download link. In Chrome, Download opens the native PDF viewer in a new tab.onIndexChange(null).Behaviour change: PDFs no longer preview inline. The README row and the component docs say so.
Verification
bun run checkmedia-src.test.ts,media-viewer.test.tsxanduse-media-viewer.test.tsCloses #142