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.

Author: pratikbuilds

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`             |

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({

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) => {

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);
   });
 });

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;

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",

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"