fix(frontend): preserve pdf link targets in desktop viewer (Stirling-Tools#7235)

Same PR as Stirling-Tools#6396, just with the conflicts resolved and some fixes on
top. Original commit by @saul1310 is preserved as-is; everything else is
a follow-up commit.

Refs Stirling-Tools#6272

## Conflicts

Stirling-Tools#6396 was written before the frontend was restructured, so all four
files it touched moved (`frontend/src/**` -> `frontend/editor/src/**`)
and `LinkLayer.tsx` had drifted. Cherry-picked with rename detection and
re-resolved against current `main`.

## Fixes on top

- **Reuse the existing platform seam instead of adding a second one.**
`main` already has `@app/platform/*` seams with per-flavour
implementations; Stirling-Tools#6396 added a parallel `@app/utils/openExternalUrl`
core+desktop pair that re-implemented the Tauri shell call already in
`desktop/platform/openExternal.ts`. Split into a pure sanitiser
(`@app/utils/externalUrl`) and a platform seam
(`@app/platform/openExternalTab`), with the desktop impl delegating to
the existing `openExternal`.
- **Kept PDF links off the `openExternal` seam.** That seam is for
leave-and-return redirects (Stripe) and its saas impl is
`window.location.assign` - routing PDF links through it would navigate
the whole app away from the user's document. `openExternalTab` always
opens alongside the app; desktop shadows it to escape the webview.
- **Fixed the same defect in two sibling call sites** that Stirling-Tools#6396 didn't
cover: `BookmarkSidebar` (bookmark URI / LaunchAppOrOpenFile actions)
and `useAnnotationMenuHandlers` (annotation menu "go to link"). Both
called `window.open` on an unsanitised PDF-supplied URI, so on desktop
they trapped the link in the webview exactly like the viewer did.
- **Dropped the unguarded fallback.** The old code fell back to
`window.open(uri)` when `new URL()` threw, so an unparseable URI
bypassed the allowlist entirely. It is now blocked.
- Empty/whitespace URIs are blocked rather than silently resolving to
the app's own page via the base URL.
- Tests: sanitiser cases (casing, leading whitespace, `data:`,
`vbscript:`, unparseable), a core seam test asserting
new-tab-not-navigate, and a desktop seam regression test asserting the
URL goes to the OS rather than `window.open`.
- **`openExternalTab` now re-validates its own input.** Every caller
sanitises first, so nothing reached it unvalidated - but it is the sink
that hands a URL to `window.open` (executes `javascript:` in our origin)
or to an OS handler on desktop, and its safety shouldn't depend on
callers remembering. Both impls fail closed, with tests that call them
directly with `javascript:`/`data:`/`file:`/`ftp:`.

## Unrelated fix included (flagged deliberately)

The last commit fixes `frontend/editor/vitest.config.ts`: `testTimeout:
10000` was set on the root `test` block, but tests all run under
`projects`, which do not inherit it - so the whole suite has silently
been running at vitest's 5s default.

This is not cosmetic. It made `task check` fail intermittently on
unrelated portal specs (`demoData`, `ConnectionModal`); the ConsignO
test takes 2966ms with only the portal project running, i.e. 59% of a
budget it was never meant to have, so any CPU contention tips it over.
Proven with an identical 6.5s probe test: times out at 5000ms on the old
config, passes at 6512ms on the fixed one.

Happy to split this into its own PR if preferred - it is here because
the gate could not be trusted without it.

## Validation

Typecheck passes for all 7 build flavours (core, proprietary, saas,
desktop, cloud, prototypes, portal); ESLint, Prettier, dpdm and the full
1662-test vitest suite pass.

Driven live against the dev server + backend with a PDF carrying five
URI annotations (https, `javascript:`, mailto, `file:`, relative). 14/14
behavioural checks pass on this branch; 5 of them fail on `main`:

| check | main | this PR |
| --- | --- | --- |
| safe https link exposes real href (copy-link) | `href="#"` |
`https://example.com/safe-link?a=1` |
| link opens in new tab / tabnabbing-proof | no `target`/`rel` |
`_blank` + `noopener noreferrer` |
| mailto link exposes real href | `href="#"` | `mailto:test@example.com`
|
| relative URI resolved against app origin | `href="#"` | resolved |
| `javascript:` / `file:` never reach href | blocked | blocked |
| clicking blocked link doesn't execute or navigate | ok | ok |
| clicking safe link opens new tab at source URL | - | ok, app not
navigated away |

---

## Checklist

### General

- [x] I have read the [Contribution
Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md)
- [x] I have read the [Stirling-PDF Developer
Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md)
(if applicable)
- [x] I have performed a self-review of my own code
- [x] My changes generate no new warnings

### Testing (if applicable)

- [x] I have run `task check` to verify linters, typechecks, and tests
pass
- [x] I have tested my changes locally

---------

Co-authored-by: Saul <saulifshin.cs@gmail.com>

Author: Frooodle

frontend/editor/src/core/components/viewer/BookmarkSidebar.tsx

@@ -17,6 +17,8 @@ import { useFileContext } from "@app/contexts/FileContext";
 import { isStirlingFile, type FileId } from "@app/types/fileContext";
 import { createStirlingFilesAndStubs } from "@app/services/fileStubHelpers";
 import apiClient from "@app/services/apiClient";
+import { openExternalTab } from "@app/platform/openExternalTab";
+import { getExternalHref } from "@app/utils/externalUrl";
 import { PdfBookmarkObject, PdfActionType } from "@embedpdf/models";
 import { useTranslation } from "react-i18next";
 import BookmarksIcon from "@mui/icons-material/BookmarksRounded";
@@ -74,6 +76,17 @@ const resolvePageNumber = (bookmark: PdfBookmarkObject): number | null => {
   return null;
 };
 
+// Bookmark targets are PDF-supplied, so sanitise before opening. Local paths
+// from LaunchAppOrOpenFile fail the allowlist - a browser blocks them anyway.
+const openBookmarkTarget = (rawUrl: string): void => {
+  const href = getExternalHref(rawUrl);
+  if (!href) {
+    console.warn("[BookmarkSidebar] Blocked unsafe URL:", rawUrl);
+    return;
+  }
+  void openExternalTab(href);
+};
+
 export const BookmarkSidebar = ({
   visible,
   thumbnailVisible,
@@ -515,12 +528,12 @@ export const BookmarkSidebar = ({
       const action = target.action;
       if (action.type === PdfActionType.URI && action.uri) {
         event.preventDefault();
-        window.open(action.uri, "_blank", "noopener");
+        openBookmarkTarget(action.uri);
         return;
       }
       if (action.type === PdfActionType.LaunchAppOrOpenFile && action.path) {
         event.preventDefault();
-        window.open(action.path, "_blank", "noopener");
+        openBookmarkTarget(action.path);
         return;
       }
     }

frontend/editor/src/core/components/viewer/LinkLayer.tsx

@@ -19,6 +19,9 @@ import {
 import { Z_INDEX_VIEWER_FLOATING_MENU } from "@app/styles/zIndex";
 import { Button } from "@app/ui/Button";
 import { ActionIcon } from "@app/ui/ActionIcon";
+import { openExternalTab } from "@app/platform/openExternalTab";
+import { getExternalHref } from "@app/utils/externalUrl";
+
 // ---------------------------------------------------------------------------
 // Inline SVG icons (thin-stroke, modern)
 // ---------------------------------------------------------------------------
@@ -401,19 +404,11 @@ export const LinkLayer: React.FC<LinkLayerProps> = ({
             behavior: "smooth",
           });
         } else if (action.type === PdfActionType.URI) {
-          const uri = action.uri;
-          try {
-            const url = new URL(uri, window.location.href);
-            if (["http:", "https:", "mailto:"].includes(url.protocol)) {
-              window.open(uri, "_blank", "noopener,noreferrer");
-            } else {
-              console.warn(
-                "[LinkLayer] Blocked unsafe URL protocol:",
-                url.protocol,
-              );
-            }
-          } catch {
-            window.open(uri, "_blank", "noopener,noreferrer");
+          const href = getExternalHref(action.uri);
+          if (href) {
+            void openExternalTab(href);
+          } else {
+            console.warn("[LinkLayer] Blocked unsafe URL:", action.uri);
           }
         }
       }
@@ -513,6 +508,11 @@ export const LinkLayer: React.FC<LinkLayerProps> = ({
           const top = annotationLink.rect.origin.y * scale;
           const width = annotationLink.rect.size.width * scale;
           const height = annotationLink.rect.size.height * scale;
+          const externalHref =
+            annotationLink.target?.type === "action" &&
+            annotationLink.target.action.type === PdfActionType.URI
+              ? getExternalHref(annotationLink.target.action.uri)
+              : null;
 
           return (
             <a
@@ -524,7 +524,9 @@ export const LinkLayer: React.FC<LinkLayerProps> = ({
                   linkElementRefs.current.delete(annotationLink.id);
                 }
               }}
-              href="#"
+              href={externalHref ?? "#"}
+              target={externalHref ? "_blank" : undefined}
+              rel={externalHref ? "noopener noreferrer" : undefined}
               onClick={(e) => {
                 e.preventDefault();
                 e.stopPropagation();

frontend/editor/src/core/components/viewer/useAnnotationMenuHandlers.ts

@@ -14,6 +14,8 @@ import type {
   AnnotationPatch,
 } from "@app/components/viewer/viewerTypes";
 import type { ScrollActions } from "@app/contexts/viewer/viewerActions";
+import { openExternalTab } from "@app/platform/openExternalTab";
+import { getExternalHref } from "@app/utils/externalUrl";
 
 export type AnnotationType =
   | "textMarkup"
@@ -370,7 +372,15 @@ export function useAnnotationMenuHandlers({
   const onGoToLink = useCallback(() => {
     if (!firstLinkTarget) return;
     if (firstLinkTarget.type === "uri") {
-      window.open(firstLinkTarget.uri, "_blank", "noopener,noreferrer");
+      const href = getExternalHref(firstLinkTarget.uri);
+      if (href) {
+        void openExternalTab(href);
+      } else {
+        console.warn(
+          "[useAnnotationMenuHandlers] Blocked unsafe URL:",
+          firstLinkTarget.uri,
+        );
+      }
     } else {
       scrollActions.scrollToPage(firstLinkTarget.pageIndex + 1);
     }

frontend/editor/src/core/platform/openExternalTab.test.ts

@@ -0,0 +1,45 @@
+import { afterEach, describe, expect, test, vi } from "vitest";
+import { openExternalTab } from "@app/platform/openExternalTab";
+import { expectConsole } from "@app/tests/failOnConsole";
+
+describe("openExternalTab (core/web)", () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  // Opening in a new tab is the point of this seam: @app/platform/openExternal
+  // navigates the current tab on saas, which would tear the user out of the PDF.
+  test("opens alongside the app rather than navigating it away", async () => {
+    const openSpy = vi
+      .spyOn(window, "open")
+      .mockImplementation(() => null as Window | null);
+    const originalHref = window.location.href;
+
+    await openExternalTab("https://example.com/");
+
+    expect(openSpy).toHaveBeenCalledWith(
+      "https://example.com/",
+      "_blank",
+      "noopener,noreferrer",
+    );
+    expect(window.location.href).toBe(originalHref);
+  });
+
+  // The seam is the sink, so it must not depend on callers having sanitised:
+  // window.open on a javascript: URL would execute it in our own origin.
+  test.each([
+    "javascript:alert(1)",
+    "  javascript:alert(1)",
+    "data:text/html,<script>alert(1)</script>",
+    "file:///etc/passwd",
+  ])("refuses to open %s even if a caller skips sanitising", async (url) => {
+    const openSpy = vi
+      .spyOn(window, "open")
+      .mockImplementation(() => null as Window | null);
+    expectConsole.warn(/Refused to open unsafe URL/);
+
+    await openExternalTab(url);
+
+    expect(openSpy).not.toHaveBeenCalled();
+  });
+});

frontend/editor/src/core/platform/openExternalTab.ts

@@ -0,0 +1,27 @@
+/**
+ * core/web implementation of the @app/platform/openExternalTab seam.
+ *
+ * Distinct from @app/platform/openExternal: that seam is for "leave and return"
+ * redirects (Stripe checkout), so its saas impl navigates the CURRENT tab. A PDF
+ * link must never do that — it would tear the user out of their document — so
+ * this seam always opens alongside the app. Desktop shadows it to escape the
+ * Tauri webview; saas/proprietary fall through to this window.open.
+ *
+ * Callers are expected to sanitise, but this is the sink that actually hands the
+ * URL to the browser, so it re-checks rather than trusting them: window.open on
+ * a `javascript:` URL executes it in our own origin.
+ */
+import { getExternalHref } from "@app/utils/externalUrl";
+
+export type OpenExternalTab = (url: string) => Promise<void>;
+
+export const openExternalTab: OpenExternalTab = async (
+  url: string,
+): Promise<void> => {
+  const safeHref = getExternalHref(url);
+  if (!safeHref) {
+    console.warn("[openExternalTab] Refused to open unsafe URL:", url);
+    return;
+  }
+  window.open(safeHref, "_blank", "noopener,noreferrer");
+};

frontend/editor/src/core/utils/externalUrl.test.ts

@@ -0,0 +1,45 @@
+import { describe, expect, test } from "vitest";
+import { getExternalHref, toSafeExternalUrl } from "@app/utils/externalUrl";
+
+describe("externalUrl", () => {
+  test("accepts http/https/mailto URLs", () => {
+    expect(toSafeExternalUrl("https://example.com/test")?.href).toBe(
+      "https://example.com/test",
+    );
+    expect(toSafeExternalUrl("http://example.com/test")?.href).toBe(
+      "http://example.com/test",
+    );
+    expect(toSafeExternalUrl("mailto:test@example.com")?.href).toBe(
+      "mailto:test@example.com",
+    );
+  });
+
+  test("rejects unsafe protocols", () => {
+    expect(toSafeExternalUrl("javascript:alert(1)")).toBeNull();
+    expect(toSafeExternalUrl("file:///etc/passwd")).toBeNull();
+    expect(toSafeExternalUrl("ftp://example.com")).toBeNull();
+    expect(
+      toSafeExternalUrl("data:text/html,<script>alert(1)</script>"),
+    ).toBeNull();
+    expect(toSafeExternalUrl("vbscript:msgbox(1)")).toBeNull();
+  });
+
+  test("rejects unparseable input instead of opening it blind", () => {
+    expect(toSafeExternalUrl("")).toBeNull();
+    expect(toSafeExternalUrl("http://[")).toBeNull();
+  });
+
+  test("is not fooled by casing or leading whitespace", () => {
+    expect(toSafeExternalUrl("JavaScript:alert(1)")).toBeNull();
+    expect(toSafeExternalUrl("  javascript:alert(1)")).toBeNull();
+    expect(toSafeExternalUrl("HTTPS://example.com")?.protocol).toBe("https:");
+  });
+
+  test("normalizes relative URLs against current origin", () => {
+    expect(getExternalHref("/docs/help")?.endsWith("/docs/help")).toBe(true);
+  });
+
+  test("getExternalHref returns null for blocked URLs", () => {
+    expect(getExternalHref("javascript:alert(1)")).toBeNull();
+  });
+});

frontend/editor/src/core/utils/externalUrl.ts

@@ -0,0 +1,32 @@
+/**
+ * Sanitisation for URLs that come out of a PDF (link annotations, bookmark
+ * actions). PDF-supplied URIs are untrusted input, so everything that opens one
+ * funnels through here first and drops anything outside the allowlist.
+ *
+ * Pure helpers only — opening the URL is a platform concern and lives behind
+ * the @app/platform/openExternalTab seam.
+ */
+const ALLOWED_EXTERNAL_PROTOCOLS = new Set(["http:", "https:", "mailto:"]);
+
+function getExternalUrlBase(): string | undefined {
+  // Relative URIs resolve against the app's own location, matching how the
+  // viewer has always treated them. No DOM => absolute URLs only.
+  return typeof window !== "undefined" ? window.location?.href : undefined;
+}
+
+/** Parses `rawUrl` and returns it only if its protocol is on the allowlist. */
+export function toSafeExternalUrl(rawUrl: string): URL | null {
+  // An empty URI would otherwise resolve to the app's own page via the base.
+  if (!rawUrl?.trim()) return null;
+  try {
+    const parsed = new URL(rawUrl, getExternalUrlBase());
+    return ALLOWED_EXTERNAL_PROTOCOLS.has(parsed.protocol) ? parsed : null;
+  } catch {
+    return null;
+  }
+}
+
+/** Normalised href for a safe external URL, or null if it is not safe to open. */
+export function getExternalHref(rawUrl: string): string | null {
+  return toSafeExternalUrl(rawUrl)?.href ?? null;
+}

frontend/editor/src/desktop/platform/openExternalTab.test.ts

@@ -0,0 +1,43 @@
+import { afterEach, describe, expect, test, vi } from "vitest";
+
+const shellOpenMock = vi.fn();
+
+vi.mock("@tauri-apps/plugin-shell", () => ({
+  open: (url: string) => shellOpenMock(url),
+}));
+
+import { openExternalTab } from "@app/platform/openExternalTab";
+import { expectConsole } from "@app/tests/failOnConsole";
+
+describe("openExternalTab (desktop/Tauri)", () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+    shellOpenMock.mockReset();
+  });
+
+  // Regression for #6272: window.open traps the link inside the Tauri webview,
+  // so a PDF link opened a blank in-app window instead of the user's browser.
+  test("hands the URL to the OS instead of the webview", async () => {
+    const openSpy = vi
+      .spyOn(window, "open")
+      .mockImplementation(() => null as Window | null);
+
+    await openExternalTab("https://example.com/");
+
+    expect(shellOpenMock).toHaveBeenCalledWith("https://example.com/");
+    expect(openSpy).not.toHaveBeenCalled();
+  });
+
+  // Worse than the web case: an unvalidated scheme here reaches an OS handler
+  // rather than staying inside a browser.
+  test.each(["javascript:alert(1)", "file:///etc/passwd", "ftp://example.com"])(
+    "refuses to hand %s to the OS",
+    async (url) => {
+      expectConsole.warn(/Refused to open unsafe URL/);
+
+      await openExternalTab(url);
+
+      expect(shellOpenMock).not.toHaveBeenCalled();
+    },
+  );
+});

frontend/editor/src/desktop/platform/openExternalTab.ts

@@ -0,0 +1,24 @@
+/**
+ * desktop (Tauri) implementation of the @app/platform/openExternalTab seam.
+ *
+ * window.open would trap the URL inside our own webview, so hand it to the OS.
+ * Delegates to the openExternal seam rather than calling the Tauri shell plugin
+ * again — on desktop "new tab" and "system browser" are the same action.
+ *
+ * Re-checks the URL for the same reason the core impl does, and more so: here it
+ * reaches an OS handler, so an unvalidated scheme is not confined to a browser.
+ */
+import { openExternal } from "@app/platform/openExternal";
+import type { OpenExternalTab } from "@core/platform/openExternalTab";
+import { getExternalHref } from "@core/utils/externalUrl";
+
+export const openExternalTab: OpenExternalTab = async (
+  url: string,
+): Promise<void> => {
+  const safeHref = getExternalHref(url);
+  if (!safeHref) {
+    console.warn("[openExternalTab] Refused to open unsafe URL:", url);
+    return;
+  }
+  await openExternal(safeHref);
+};