diff --git a/electron/main.ts b/electron/main.ts index c695943..dfdd167 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -49,6 +49,7 @@ import { describeImageWithOllama } from "./vision"; import { LocalJobManager } from "./jobs"; import { closeMcpConnections, disconnectMcpServer, testMcpServer } from "./mcp"; import { discoverModelCatalog } from "./model-catalog"; +import { registerExternalNavigation } from "./navigation"; import { mapWorkspaceMcpSecrets } from "./workspace-secrets"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -979,15 +980,10 @@ function createWindow(): void { }); mainWindow.once("ready-to-show", () => mainWindow?.show()); - mainWindow.webContents.setWindowOpenHandler(({ url }) => { - if (url.startsWith("https://")) void shell.openExternal(url); - return { action: "deny" }; - }); - mainWindow.webContents.on("will-navigate", (event, url) => { - const developmentUrl = process.env.VITE_DEV_SERVER_URL; - if (!developmentUrl || !url.startsWith(developmentUrl)) - event.preventDefault(); - }); + registerExternalNavigation( + mainWindow.webContents, + process.env.VITE_DEV_SERVER_URL, + ); if (process.env.VITE_DEV_SERVER_URL) { void mainWindow.loadURL(process.env.VITE_DEV_SERVER_URL); diff --git a/electron/navigation.test.ts b/electron/navigation.test.ts new file mode 100644 index 0000000..86680e2 --- /dev/null +++ b/electron/navigation.test.ts @@ -0,0 +1,105 @@ +import { EventEmitter } from "node:events"; +import { shell, type WebContents } from "electron"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { registerExternalNavigation } from "./navigation"; + +vi.mock("electron", () => ({ + shell: { openExternal: vi.fn().mockResolvedValue(undefined) }, +})); + +function setup(developmentUrl?: string) { + const contents = Object.assign(new EventEmitter(), { + setWindowOpenHandler: vi.fn(), + }); + registerExternalNavigation(contents as unknown as WebContents, developmentUrl); + return { + navigate(url: string) { + const event = { preventDefault: vi.fn() }; + contents.emit("will-navigate", event, url); + return event.preventDefault; + }, + openWindow(url: string) { + return contents.setWindowOpenHandler.mock.calls[0][0]({ url }); + }, + }; +} + +afterEach(() => { + vi.clearAllMocks(); + vi.restoreAllMocks(); +}); + +describe("external navigation", () => { + it.each([undefined, "http://localhost:5173/"])( + "opens Hugging Face links externally with development URL %s", + (developmentUrl) => { + const url = "https://huggingface.co/organization/model"; + expect(setup(developmentUrl).navigate(url)).toHaveBeenCalledOnce(); + expect(shell.openExternal).toHaveBeenCalledExactlyOnceWith(url); + }, + ); + + it("opens HTTPS popup links in the browser, not an Electron window", () => { + const url = "https://huggingface.co/organization/model"; + expect(setup().openWindow(url)).toEqual({ action: "deny" }); + expect(shell.openExternal).toHaveBeenCalledExactlyOnceWith(url); + }); + + it.each([ + "http://huggingface.co/organization/model", + "file:///tmp/model.html", + "javascript:alert(1)", + "data:text/html,model", + "hf://organization/model", + "not a URL", + ])("blocks unsafe or unsupported links: %s", (url) => { + for (const developmentUrl of [undefined, "http://localhost:5173/"]) { + const navigation = setup(developmentUrl); + expect(navigation.navigate(url)).toHaveBeenCalledOnce(); + expect(navigation.openWindow(url)).toEqual({ action: "deny" }); + } + expect(shell.openExternal).not.toHaveBeenCalled(); + }); + + it.each(["username", "password"] as const)( + "rejects links with an embedded %s", + (field) => { + const url = new URL("https://huggingface.co/organization/model"); + url[field] = "fixture"; + const navigation = setup(); + expect(navigation.navigate(url.toString())).toHaveBeenCalledOnce(); + expect(navigation.openWindow(url.toString())).toEqual({ action: "deny" }); + expect(shell.openExternal).not.toHaveBeenCalled(); + }, + ); + + it("allows navigation within the development server origin", () => { + const navigation = setup("http://localhost:5173/"); + expect( + navigation.navigate("http://localhost:5173/models"), + ).not.toHaveBeenCalled(); + expect(shell.openExternal).not.toHaveBeenCalled(); + }); + + it("does not trust URLs that merely start with the development URL", () => { + const navigation = setup("http://localhost:5173"); + expect(navigation.navigate("http://localhost:51730/")).toHaveBeenCalledOnce(); + const spoofedUrl = new URL("http://other.test/"); + spoofedUrl.username = "localhost"; + spoofedUrl.password = "5173"; + expect(navigation.navigate(spoofedUrl.toString())).toHaveBeenCalledOnce(); + expect(shell.openExternal).not.toHaveBeenCalled(); + }); + + it("handles browser-launch failures without an unhandled rejection", async () => { + const error = new Error("No default browser"); + vi.mocked(shell.openExternal).mockRejectedValueOnce(error); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + expect( + setup().navigate("https://huggingface.co/organization/model"), + ).toHaveBeenCalledOnce(); + await vi.waitFor(() => + expect(warn).toHaveBeenCalledWith("Could not open external link:", error), + ); + }); +}); diff --git a/electron/navigation.ts b/electron/navigation.ts new file mode 100644 index 0000000..3ac3b87 --- /dev/null +++ b/electron/navigation.ts @@ -0,0 +1,36 @@ +import { shell, type WebContents } from "electron"; + +function parseUrl(value: string): URL | undefined { + try { + return new URL(value); + } catch { + return undefined; + } +} + +function openExternalLink(value: string): void { + const url = parseUrl(value); + if (!url || url.protocol !== "https:" || url.username || url.password) return; + void shell.openExternal(url.toString()).catch((error) => { + console.warn("Could not open external link:", error); + }); +} + +export function registerExternalNavigation( + webContents: WebContents, + developmentUrl?: string, +): void { + const developmentOrigin = developmentUrl + ? parseUrl(developmentUrl)?.origin + : undefined; + + webContents.setWindowOpenHandler(({ url }) => { + openExternalLink(url); + return { action: "deny" }; + }); + webContents.on("will-navigate", (event, url) => { + if (developmentOrigin && parseUrl(url)?.origin === developmentOrigin) return; + event.preventDefault(); + openExternalLink(url); + }); +} diff --git a/src/features/models/ModelsView.test.tsx b/src/features/models/ModelsView.test.tsx index 1fbd5c9..6082248 100644 --- a/src/features/models/ModelsView.test.tsx +++ b/src/features/models/ModelsView.test.tsx @@ -58,6 +58,35 @@ afterEach(() => { }); describe("ModelsView", () => { + it("opens Hugging Face pages through the catalog API and reports failures", async () => { + vi.spyOn(forgeApi.catalog, "list").mockResolvedValue(catalog); + const open = vi + .spyOn(forgeApi.catalog, "open") + .mockRejectedValueOnce(new Error("No default browser")) + .mockResolvedValue(undefined); + render( + , + ); + + fireEvent.click(await screen.findByRole("button", { name: "View" })); + expect(open).toHaveBeenCalledWith(catalog.items[0].url); + expect(await screen.findByRole("alert")).toHaveTextContent( + "Could not open the model page in your browser.", + ); + + fireEvent.click(screen.getByRole("button", { name: "View" })); + await waitFor(() => expect(screen.queryByRole("alert")).toBeNull()); + expect(open).toHaveBeenCalledTimes(2); + }); + it("discovers and searches NSFW pipelines only after consent", async () => { const listCatalog = vi .spyOn(forgeApi.catalog, "list") diff --git a/src/features/models/ModelsView.tsx b/src/features/models/ModelsView.tsx index 2f65c2a..fb3cd6c 100644 --- a/src/features/models/ModelsView.tsx +++ b/src/features/models/ModelsView.tsx @@ -90,6 +90,7 @@ export function ModelsView({ const [catalog, setCatalog] = useState(null); const [catalogLoading, setCatalogLoading] = useState(true); const [catalogError, setCatalogError] = useState(""); + const [catalogOpenError, setCatalogOpenError] = useState(""); const [customModel, setCustomModel] = useState(""); const [pull, setPull] = useState(null); const installedNames = new Set(models.map((model) => model.name)); @@ -160,6 +161,17 @@ export function ModelsView({ void loadCatalog(true); } + async function openModelPage(url: string) { + setCatalogOpenError(""); + try { + await forgeApi.catalog.open(url); + } catch { + setCatalogOpenError( + "Could not open the model page in your browser. Check that a default browser is configured and try again.", + ); + } + } + return (
@@ -266,6 +278,11 @@ export function ModelsView({ {catalogError && (

{catalogError}

)} + {catalogOpenError && ( +

+ {catalogOpenError} +

+ )} {catalog?.warnings.length ? (

{catalog.warnings.join(" ")}

) : null} @@ -348,7 +365,7 @@ export function ModelsView({ ) : (