From ddce89f12d3f15fb42e050baa54e0696c4483736 Mon Sep 17 00:00:00 2001
From: 0xPratik <sariapratik@gmail.com>
Date: Thu, 29 Oct 2026 18:03:28 +0545
Subject: [PATCH 1/2] fix(media): keep script-capable sources out of links and
 frames

Addresses findings 2, 3 and 4 of #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.
---
 README.md                          |  2 +-
 src/hooks/use-media-viewer.test.ts | 32 ++++++++++++++++++++++++++++++
 src/hooks/use-media-viewer.ts      | 13 +++++++++---
 src/lib/media-src.test.ts          | 12 ++++++-----
 src/lib/media-src.ts               |  8 ++++----
 src/ui/media-viewer.test.tsx       | 16 +++++++++++++--
 src/ui/media-viewer.tsx            | 25 +++++++----------------
 7 files changed, 75 insertions(+), 33 deletions(-)

diff --git a/README.md b/README.md
index 07529823..d918cac5 100644
--- a/README.md
+++ b/README.md
@@ -200,7 +200,7 @@ Everything imports from `@corbits/react-ui`, except rows marked "optional peer",
 | `ToolApproval`             | Approve, approve always or deny a tool call waiting on a person                                                     |
 | `GeneratedImages`          | Images an agent made, with placeholders while generating                                                            |
 | `Sources`                  | Numbered citations for an answer                                                                                    |
-| `MediaViewer`              | Pictures, video, audio, PDFs and text full size; `useMediaViewer`                                                   |
+| `MediaViewer`              | Pictures, video, audio and text full size; `useMediaViewer`                                                         |
 | `Attachments`              | Files as a stack, tiles or a list; `useAttachments`                                                                 |
 | `AttachmentDropZone`       | Area that takes dropped files and folders; `useFileDrop`                                                            |
 | `PromptInput`              | The message box: `PromptInputTextarea`, `PromptInputSubmit`, `PromptInputSuggestions`; `usePromptInput`             |
diff --git a/src/hooks/use-media-viewer.test.ts b/src/hooks/use-media-viewer.test.ts
index 3dfad00f..65e3d15c 100644
--- a/src/hooks/use-media-viewer.test.ts
+++ b/src/hooks/use-media-viewer.test.ts
@@ -65,6 +65,38 @@ describe("useMediaViewer", () => {
     hook.unmount();
   });
 
+  test("an item falling off a shortened list closes it for good", () => {
+    let items: readonly string[] = ITEMS;
+    const hook = renderHook(() =>
+      useMediaViewer<string>({ items, defaultIndex: 2 }),
+    );
+    items = ["a"];
+    hook.rerender();
+    expect(hook.result.current.index).toBe(null);
+    items = ITEMS;
+    hook.rerender();
+    expect(hook.result.current.index).toBe(null);
+    hook.unmount();
+  });
+
+  test("controlled, an item falling off the list reports one close", () => {
+    let items: readonly string[] = ITEMS;
+    const changes: (number | null)[] = [];
+    const hook = renderHook(() =>
+      useMediaViewer<string>({
+        items,
+        index: 2,
+        onIndexChange: (index) => changes.push(index),
+      }),
+    );
+    items = ["a"];
+    hook.rerender();
+    hook.rerender();
+    expect(hook.result.current.index).toBe(null);
+    expect(changes).toEqual([null]);
+    hook.unmount();
+  });
+
   test("controlled, it reports changes and follows the value it is given", () => {
     const changes: (number | null)[] = [];
     const hook = viewer({
diff --git a/src/hooks/use-media-viewer.ts b/src/hooks/use-media-viewer.ts
index d5258c1d..b6a085e9 100644
--- a/src/hooks/use-media-viewer.ts
+++ b/src/hooks/use-media-viewer.ts
@@ -1,4 +1,4 @@
-import { useCallback, useRef } from "react";
+import { useCallback, useEffect, useRef } from "react";
 import type { RefObject } from "react";
 
 import { useControllableState } from "./use-controllable-state.js";
@@ -37,9 +37,9 @@ export type MediaViewerState<T> = {
 
 /**
  * Which item of a set is open in a viewer. Stepping stops at either end
- * rather than wrapping, and an index outside `items` reads as closed, so a
+ * rather than wrapping, and an index outside `items` closes the viewer, so a
  * list that shrinks under an open viewer closes it rather than showing
- * nothing.
+ * nothing, and growing back does not reopen it.
  */
 export function useMediaViewer<T>({
   items,
@@ -62,6 +62,13 @@ export function useMediaViewer<T>({
     rawIndex < count
       ? rawIndex
       : null;
+  const outOfRange = rawIndex !== null && index === null;
+
+  // Commits the close an out-of-range index already reads as. Keyed on
+  // falling out alone, so a controlled caller hears it once.
+  useEffect(() => {
+    if (outOfRange) setIndex(null);
+  }, [outOfRange]);
 
   const open = useCallback(
     (next: number, element?: HTMLElement | null) => {
diff --git a/src/lib/media-src.test.ts b/src/lib/media-src.test.ts
index c0f68d56..4f40a90b 100644
--- a/src/lib/media-src.test.ts
+++ b/src/lib/media-src.test.ts
@@ -10,7 +10,7 @@ describe("safeMediaSrc", () => {
       "blob:https://example.com/0f6c1b52",
     ]) {
       expect(safeMediaSrc(src, "media")).toBe(src);
-      expect(safeMediaSrc(src, "frame")).toBe(src);
+      expect(safeMediaSrc(src, "link")).toBe(src);
     }
   });
 
@@ -25,16 +25,18 @@ describe("safeMediaSrc", () => {
     );
   });
 
-  test("a frame never takes a data: URL", () => {
-    expect(safeMediaSrc("data:image/png;base64,AAAA", "frame")).toBe(undefined);
-    expect(safeMediaSrc("data:application/pdf;base64,AAAA", "frame")).toBe(
+  test("a link never takes a data: URL", () => {
+    expect(safeMediaSrc("data:image/svg+xml;base64,PHN2Zy8+", "link")).toBe(
+      undefined,
+    );
+    expect(safeMediaSrc("data:application/pdf;base64,AAAA", "link")).toBe(
       undefined,
     );
   });
 
   test("script and relative URLs are refused", () => {
     expect(safeMediaSrc("javascript:alert(1)", "media")).toBe(undefined);
-    expect(safeMediaSrc("JavaScript:alert(1)", "frame")).toBe(undefined);
+    expect(safeMediaSrc("JavaScript:alert(1)", "link")).toBe(undefined);
     expect(safeMediaSrc("/files/a.png", "media")).toBe(undefined);
   });
 });
diff --git a/src/lib/media-src.ts b/src/lib/media-src.ts
index 82670f9b..96ebab17 100644
--- a/src/lib/media-src.ts
+++ b/src/lib/media-src.ts
@@ -1,7 +1,7 @@
 import { isSafeUrl } from "./url.js";
 
 /** Where a media sink may load from. */
-export type MediaSink = "media" | "frame";
+export type MediaSink = "media" | "link";
 
 const NETWORK_PROTOCOLS: readonly string[] = ["http:", "https:", "blob:"];
 
@@ -12,9 +12,9 @@ const DATA_MEDIA_TYPE = /^data:(image|video|audio)\/[a-z0-9.+-]+[;,]/i;
  * `src` back if a media sink may load it, otherwise `undefined`.
  *
  * Image, video and audio elements (`"media"`) also accept a `data:` URL of
- * their own kind, since they never run what they load. A frame (`"frame"`)
- * takes only `http:`, `https:` and `blob:`: a `data:` document in a frame
- * runs script.
+ * their own kind, since they never run what they load. A link (`"link"`)
+ * takes only `http:`, `https:` and `blob:`: it opens what it points at as a
+ * document, and a `data:` document, an SVG image among them, runs script.
  */
 export function safeMediaSrc(src: string, sink: MediaSink): string | undefined {
   if (sink === "media" && DATA_MEDIA_TYPE.test(src)) return src;
diff --git a/src/ui/media-viewer.test.tsx b/src/ui/media-viewer.test.tsx
index c13fd607..c809e34e 100644
--- a/src/ui/media-viewer.test.tsx
+++ b/src/ui/media-viewer.test.tsx
@@ -237,12 +237,14 @@ describe("MediaViewer", () => {
       download: true,
     },
     {
+      // Chrome shows no PDF in a sandboxed frame, and an unsandboxed one
+      // runs whatever the URL serves, so a PDF is a download only.
       item: {
         name: "report.pdf",
         mediaType: "application/pdf",
         src: "https://media.example.com/report.pdf",
       },
-      slot: "media-viewer-page",
+      slot: "media-viewer-empty",
       download: true,
     },
     {
@@ -256,7 +258,7 @@ describe("MediaViewer", () => {
       download: true,
     },
     {
-      // A data: document would run script in the frame.
+      // A data: document would run script when opened.
       item: {
         name: "inline.pdf",
         mediaType: "application/pdf",
@@ -265,6 +267,16 @@ describe("MediaViewer", () => {
       slot: "media-viewer-empty",
       download: false,
     },
+    {
+      // Fine in an <img>, but as a link an SVG document runs script.
+      item: {
+        name: "inline.svg",
+        mediaType: "image/svg+xml",
+        src: "data:image/svg+xml;base64,PHN2Zy8+",
+      },
+      slot: "media-viewer-image",
+      download: false,
+    },
     {
       item: {
         name: "script.pdf",
diff --git a/src/ui/media-viewer.tsx b/src/ui/media-viewer.tsx
index 341a46ae..e78163bb 100644
--- a/src/ui/media-viewer.tsx
+++ b/src/ui/media-viewer.tsx
@@ -16,8 +16,9 @@ export type MediaViewerItem = {
   readonly mediaType: string;
   /**
    * Where to load it from: `http:`, `https:`, `blob:`, or a `data:` image,
-   * video or audio. Leave it out for an item that cannot be loaded; the
-   * viewer then says there is no preview and offers no download.
+   * video or audio (shown, but not offered for download). Leave it out for
+   * an item that cannot be loaded; the viewer then says there is no preview
+   * and offers no download.
    */
   readonly src?: string;
   /** What an image shows. Defaults to `name`. */
@@ -40,15 +41,15 @@ const CONTROL =
   "grid size-9 shrink-0 place-items-center rounded-full bg-popover text-popover-foreground shadow-sm transition-colors hover:bg-muted [&_svg]:size-4";
 
 /**
- * Pictures, video, audio, PDFs and text full size, over everything, on the
+ * Pictures, video, audio and text full size, over everything, on the
  * native modal `<dialog>`: it holds focus and closes on Escape, and a click
  * outside the media closes it too. The page behind it does not scroll. With
  * more than one item, the arrow keys and the Previous and Next buttons step
  * through them, stopping at either end. Download opens a new tab where the
  * browser will not download the file. On close, focus goes back to the
  * element given to `viewer.open`, or else to whatever had it when the viewer
- * opened. Escape pressed inside a PDF goes to the browser's PDF viewer, not
- * the dialog; Tab leaves the PDF and reaches Close.
+ * opened. A PDF has no preview, only Download: Chrome will not show one in
+ * a sandboxed frame, and an unsandboxed frame runs whatever its URL serves.
  *
  * Renders nothing while `viewer` is closed. A `ref` gets the `<dialog>`
  * while it is open.
@@ -142,7 +143,7 @@ function MediaViewerDialog({
   const count = viewer.items.length;
   const many = count > 1;
   const position = many ? `${index + 1} of ${count}` : null;
-  const download = safeItemSrc(item, "media");
+  const download = safeItemSrc(item, "link");
 
   return (
     <dialog
@@ -344,18 +345,6 @@ function MediaViewerContent({ item }: { readonly item: MediaViewerItem }) {
       />
     );
   }
-  const page = safeItemSrc(item, "frame");
-  if (kind === "pdf" && page !== undefined) {
-    return (
-      <iframe
-        key={page}
-        src={page}
-        title={item.name}
-        data-slot="media-viewer-page"
-        className={PAGE_FRAME}
-      />
-    );
-  }
   return (
     <p
       data-slot="media-viewer-empty"

From 0f4a97675ed0dcb596dd4ee47bdc8355bbe2ebd9 Mon Sep 17 00:00:00 2001
From: 0xPratik <sariapratik@gmail.com>
Date: Fri, 30 Oct 2026 12:25:09 +0000
Subject: [PATCH 2/2] fix(media): leave the shrinking-list close for its own
 change

This pull request now only keeps script-capable sources out of links and frames.
---
 src/hooks/use-media-viewer.test.ts | 32 ------------------------------
 src/hooks/use-media-viewer.ts      | 13 +++---------
 2 files changed, 3 insertions(+), 42 deletions(-)

diff --git a/src/hooks/use-media-viewer.test.ts b/src/hooks/use-media-viewer.test.ts
index 65e3d15c..3dfad00f 100644
--- a/src/hooks/use-media-viewer.test.ts
+++ b/src/hooks/use-media-viewer.test.ts
@@ -65,38 +65,6 @@ describe("useMediaViewer", () => {
     hook.unmount();
   });
 
-  test("an item falling off a shortened list closes it for good", () => {
-    let items: readonly string[] = ITEMS;
-    const hook = renderHook(() =>
-      useMediaViewer<string>({ items, defaultIndex: 2 }),
-    );
-    items = ["a"];
-    hook.rerender();
-    expect(hook.result.current.index).toBe(null);
-    items = ITEMS;
-    hook.rerender();
-    expect(hook.result.current.index).toBe(null);
-    hook.unmount();
-  });
-
-  test("controlled, an item falling off the list reports one close", () => {
-    let items: readonly string[] = ITEMS;
-    const changes: (number | null)[] = [];
-    const hook = renderHook(() =>
-      useMediaViewer<string>({
-        items,
-        index: 2,
-        onIndexChange: (index) => changes.push(index),
-      }),
-    );
-    items = ["a"];
-    hook.rerender();
-    hook.rerender();
-    expect(hook.result.current.index).toBe(null);
-    expect(changes).toEqual([null]);
-    hook.unmount();
-  });
-
   test("controlled, it reports changes and follows the value it is given", () => {
     const changes: (number | null)[] = [];
     const hook = viewer({
diff --git a/src/hooks/use-media-viewer.ts b/src/hooks/use-media-viewer.ts
index b6a085e9..d5258c1d 100644
--- a/src/hooks/use-media-viewer.ts
+++ b/src/hooks/use-media-viewer.ts
@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useRef } from "react";
+import { useCallback, useRef } from "react";
 import type { RefObject } from "react";
 
 import { useControllableState } from "./use-controllable-state.js";
@@ -37,9 +37,9 @@ export type MediaViewerState<T> = {
 
 /**
  * Which item of a set is open in a viewer. Stepping stops at either end
- * rather than wrapping, and an index outside `items` closes the viewer, so a
+ * rather than wrapping, and an index outside `items` reads as closed, so a
  * list that shrinks under an open viewer closes it rather than showing
- * nothing, and growing back does not reopen it.
+ * nothing.
  */
 export function useMediaViewer<T>({
   items,
@@ -62,13 +62,6 @@ export function useMediaViewer<T>({
     rawIndex < count
       ? rawIndex
       : null;
-  const outOfRange = rawIndex !== null && index === null;
-
-  // Commits the close an out-of-range index already reads as. Keyed on
-  // falling out alone, so a controlled caller hears it once.
-  useEffect(() => {
-    if (outOfRange) setIndex(null);
-  }, [outOfRange]);
 
   const open = useCallback(
     (next: number, element?: HTMLElement | null) => {