refactor(viewer): parse the openLink url instead of regex-matching its scheme

Address review feedback: rather than regex-checking the raw string and then
handing that same raw string to window.open (validate one representation, act
on another — a parser-differential smell), parse it once with the URL
constructor, validate `protocol`, and open the normalized `href` from the same
parse. What we check and what opens are now byte-identical, and a malformed
string is rejected outright. The negative test now also covers data: and an
unparseable string.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: benvinegar

e2e/isolation.spec.ts

@@ -94,12 +94,19 @@ test("a surface send is not delivered to the agent as user feedback", async ({
 const openLinkMsg = (url: string) =>
   `<script>parent.postMessage({__sideshow:true,type:"open-link",url:${JSON.stringify(url)}},"*")</script>`;
 
-test("openLink ignores a non-http(s) scheme — no prompt, no open", async ({ page, server }) => {
-  await publish(server.url, {
-    html: openLinkMsg("javascript:alert(1)"),
-    title: "bad",
-    agent: "e2e",
-  });
+test("openLink ignores non-http(s) and malformed urls — no prompt, no open", async ({
+  page,
+  server,
+}) => {
+  // A scheme that parses (javascript:), another (data:), and a string that
+  // fails to parse at all — all must be refused before the confirm.
+  const bad = ["javascript:alert(1)", "data:text/html,<b>x</b>", "::: not a url :::"];
+  const fire = `<script>${bad
+    .map(
+      (u) => `parent.postMessage({__sideshow:true,type:"open-link",url:${JSON.stringify(u)}},"*");`,
+    )
+    .join("")}</script>`;
+  await publish(server.url, { html: fire, title: "bad", agent: "e2e" });
 
   let dialogs = 0;
   page.on("dialog", (d) => {
@@ -111,7 +118,7 @@ test("openLink ignores a non-http(s) scheme — no prompt, no open", async ({ pa
   await expect(page.locator(".card:not(#whatsNew) iframe").first()).toBeVisible();
   await page.waitForTimeout(500);
 
-  // The scheme check returns before the confirm, so no dialog is ever raised.
+  // Each is rejected before the confirm, so no dialog is ever raised.
   expect(dialogs).toBe(0);
 });
 

viewer/src/App.tsx

@@ -323,14 +323,22 @@ async function onBridgeMessage(ev: MessageEvent) {
     });
     toast("Added to this surface’s thread");
   } else if (d.type === "open-link" && isOwnFrame(ev.source)) {
-    // Only ever open real external links. The in-frame click handler already
-    // forwards just http(s) hrefs, but a surface can call openLink() directly
-    // (or post this message raw) with any scheme — javascript:, data:, file: —
-    // so re-check here, host-side, where it can't be bypassed. (noopener already
-    // severs those from the board, but there's no reason to open them at all.)
-    const url = String(d.url);
-    if (!/^https?:\/\//i.test(url)) return;
-    if (confirm(`Open external link?\n\n${url}`)) window.open(url, "_blank", "noopener");
+    // Only ever open real external links. The in-frame click handler forwards
+    // just http(s) hrefs, but a surface can call openLink() directly (or post
+    // this message raw) with any scheme — javascript:, data:, file: — so
+    // re-check host-side, where it can't be bypassed. Parse once and act on the
+    // parsed result: validate `protocol` and open the normalized `href` from the
+    // same parse, so there's no gap between what we check and what window.open
+    // re-parses (and a malformed string is rejected outright).
+    let link: URL;
+    try {
+      link = new URL(String(d.url));
+    } catch {
+      return;
+    }
+    if (link.protocol !== "http:" && link.protocol !== "https:") return;
+    if (confirm(`Open external link?\n\n${link.href}`))
+      window.open(link.href, "_blank", "noopener");
   } else if (d.type === "copy" && isOwnFrame(ev.source)) {
     void navigator.clipboard?.writeText(String(d.text)).catch(() => {});
   }