From e5836283abf81b0b40a26ddd698ce9a2d7012976 Mon Sep 17 00:00:00 2001 From: Shakker Date: Tue, 24 Feb 2026 13:27:04 +0000 Subject: [PATCH] ui: centralize safe external URL opening --- package.json | 3 +- scripts/check-no-raw-window-open.mjs | 87 +++++++++++++++++++ ui/src/ui/chat/grouped-render.ts | 12 +-- ui/src/ui/chat/image-open.test.ts | 48 ---------- ui/src/ui/chat/image-open.ts | 39 --------- ui/src/ui/open-external-url.test.ts | 56 ++++++++++++ ui/src/ui/open-external-url.ts | 68 +++++++++++++++ .../ui/views/chat-image-open.browser.test.ts | 53 +++++++++++ 8 files changed, 268 insertions(+), 98 deletions(-) create mode 100644 scripts/check-no-raw-window-open.mjs delete mode 100644 ui/src/ui/chat/image-open.test.ts delete mode 100644 ui/src/ui/chat/image-open.ts create mode 100644 ui/src/ui/open-external-url.test.ts create mode 100644 ui/src/ui/open-external-url.ts create mode 100644 ui/src/ui/views/chat-image-open.browser.test.ts diff --git a/package.json b/package.json index c1b68821083..92e04fb7def 100644 --- a/package.json +++ b/package.json @@ -87,12 +87,13 @@ "ios:gen": "bash -lc './scripts/ios-configure-signing.sh && cd apps/ios && xcodegen generate'", "ios:open": "bash -lc './scripts/ios-configure-signing.sh && cd apps/ios && xcodegen generate && open OpenClaw.xcodeproj'", "ios:run": "bash -lc './scripts/ios-configure-signing.sh && cd apps/ios && xcodegen generate && xcodebuild -project OpenClaw.xcodeproj -scheme OpenClaw -destination \"${IOS_DEST:-platform=iOS Simulator,name=iPhone 17}\" -configuration Debug build && xcrun simctl boot \"${IOS_SIM:-iPhone 17}\" || true && xcrun simctl launch booted ai.openclaw.ios'", - "lint": "oxlint --type-aware", + "lint": "oxlint --type-aware && pnpm lint:ui:no-raw-window-open", "lint:all": "pnpm lint && pnpm lint:swift", "lint:docs": "pnpm dlx markdownlint-cli2", "lint:docs:fix": "pnpm dlx markdownlint-cli2 --fix", "lint:fix": "oxlint --type-aware --fix && pnpm format", "lint:swift": "swiftlint lint --config .swiftlint.yml && (cd apps/ios && swiftlint lint --config .swiftlint.yml)", + "lint:ui:no-raw-window-open": "node scripts/check-no-raw-window-open.mjs", "mac:open": "open dist/OpenClaw.app", "mac:package": "bash scripts/package-mac-app.sh", "mac:restart": "bash scripts/restart-mac.sh", diff --git a/scripts/check-no-raw-window-open.mjs b/scripts/check-no-raw-window-open.mjs new file mode 100644 index 00000000000..55334549ba1 --- /dev/null +++ b/scripts/check-no-raw-window-open.mjs @@ -0,0 +1,87 @@ +#!/usr/bin/env node + +import { promises as fs } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const uiSourceDir = path.join(repoRoot, "ui", "src", "ui"); +const allowedCallsites = new Set([path.join(uiSourceDir, "open-external-url.ts")]); + +function isTestFile(filePath) { + return ( + filePath.endsWith(".test.ts") || + filePath.endsWith(".browser.test.ts") || + filePath.endsWith(".node.test.ts") + ); +} + +async function collectTypeScriptFiles(dir) { + const entries = await fs.readdir(dir, { withFileTypes: true }); + const out = []; + for (const entry of entries) { + const entryPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + out.push(...(await collectTypeScriptFiles(entryPath))); + continue; + } + if (!entry.isFile()) { + continue; + } + if (!entryPath.endsWith(".ts")) { + continue; + } + if (isTestFile(entryPath)) { + continue; + } + out.push(entryPath); + } + return out; +} + +function lineNumberAt(content, index) { + let lines = 1; + for (let i = 0; i < index; i++) { + if (content.charCodeAt(i) === 10) { + lines++; + } + } + return lines; +} + +async function main() { + const files = await collectTypeScriptFiles(uiSourceDir); + const violations = []; + const rawWindowOpenRe = /\bwindow\s*\.\s*open\s*\(/g; + + for (const filePath of files) { + if (allowedCallsites.has(filePath)) { + continue; + } + + const content = await fs.readFile(filePath, "utf8"); + let match = rawWindowOpenRe.exec(content); + while (match) { + const line = lineNumberAt(content, match.index); + const relPath = path.relative(repoRoot, filePath); + violations.push(`${relPath}:${line}`); + match = rawWindowOpenRe.exec(content); + } + } + + if (violations.length === 0) { + return; + } + + console.error("Found raw window.open usage outside safe helper:"); + for (const violation of violations) { + console.error(`- ${violation}`); + } + console.error("Use openExternalUrlSafe(...) from ui/src/ui/open-external-url.ts instead."); + process.exit(1); +} + +main().catch((error) => { + console.error(error); + process.exit(1); +}); diff --git a/ui/src/ui/chat/grouped-render.ts b/ui/src/ui/chat/grouped-render.ts index e8993f0c29d..df4689b0fa4 100644 --- a/ui/src/ui/chat/grouped-render.ts +++ b/ui/src/ui/chat/grouped-render.ts @@ -2,10 +2,10 @@ import { html, nothing } from "lit"; import { unsafeHTML } from "lit/directives/unsafe-html.js"; import type { AssistantIdentity } from "../assistant-identity.ts"; import { toSanitizedMarkdownHtml } from "../markdown.ts"; +import { openExternalUrlSafe } from "../open-external-url.ts"; import { detectTextDirection } from "../text-direction.ts"; import type { MessageGroup } from "../types/chat-types.ts"; import { renderCopyAsMarkdownButton } from "./copy-as-markdown.ts"; -import { resolveSafeImageOpenUrl } from "./image-open.ts"; import { extractTextCached, extractThinkingCached, @@ -202,15 +202,7 @@ function renderMessageImages(images: ImageBlock[]) { } const openImage = (url: string) => { - const safeUrl = resolveSafeImageOpenUrl(url, window.location.href); - if (!safeUrl) { - return; - } - - const opened = window.open(safeUrl, "_blank", "noopener,noreferrer"); - if (opened) { - opened.opener = null; - } + openExternalUrlSafe(url, { allowDataImage: true }); }; return html` diff --git a/ui/src/ui/chat/image-open.test.ts b/ui/src/ui/chat/image-open.test.ts deleted file mode 100644 index 180e909bb09..00000000000 --- a/ui/src/ui/chat/image-open.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { resolveSafeImageOpenUrl } from "./image-open.ts"; - -describe("resolveSafeImageOpenUrl", () => { - const baseHref = "https://openclaw.ai/chat"; - - it("allows absolute https URLs", () => { - expect(resolveSafeImageOpenUrl("https://example.com/a.png?x=1#y", baseHref)).toBe( - "https://example.com/a.png?x=1#y", - ); - }); - - it("allows relative URLs resolved against the current origin", () => { - expect(resolveSafeImageOpenUrl("/assets/pic.png", baseHref)).toBe( - "https://openclaw.ai/assets/pic.png", - ); - }); - - it("allows blob URLs", () => { - expect(resolveSafeImageOpenUrl("blob:https://openclaw.ai/abc-123", baseHref)).toBe( - "blob:https://openclaw.ai/abc-123", - ); - }); - - it("allows data image URLs", () => { - expect(resolveSafeImageOpenUrl("data:image/png;base64,iVBORw0KGgo=", baseHref)).toBe( - "data:image/png;base64,iVBORw0KGgo=", - ); - }); - - it("rejects non-image data URLs", () => { - expect( - resolveSafeImageOpenUrl("data:text/html,", baseHref), - ).toBeNull(); - }); - - it("rejects javascript URLs", () => { - expect(resolveSafeImageOpenUrl("javascript:alert(1)", baseHref)).toBeNull(); - }); - - it("rejects file URLs", () => { - expect(resolveSafeImageOpenUrl("file:///tmp/x.png", baseHref)).toBeNull(); - }); - - it("rejects empty values", () => { - expect(resolveSafeImageOpenUrl(" ", baseHref)).toBeNull(); - }); -}); diff --git a/ui/src/ui/chat/image-open.ts b/ui/src/ui/chat/image-open.ts deleted file mode 100644 index fd096c67184..00000000000 --- a/ui/src/ui/chat/image-open.ts +++ /dev/null @@ -1,39 +0,0 @@ -const DATA_URL_PREFIX = "data:"; -const ALLOWED_OPEN_PROTOCOLS = new Set(["http:", "https:", "blob:"]); - -function isAllowedDataImageUrl(url: string): boolean { - if (!url.toLowerCase().startsWith(DATA_URL_PREFIX)) { - return false; - } - - const commaIndex = url.indexOf(","); - if (commaIndex < DATA_URL_PREFIX.length) { - return false; - } - - const metadata = url.slice(DATA_URL_PREFIX.length, commaIndex); - const mimeType = metadata.split(";")[0]?.trim().toLowerCase() ?? ""; - return mimeType.startsWith("image/"); -} - -export function resolveSafeImageOpenUrl(rawUrl: string, baseHref: string): string | null { - const candidate = rawUrl.trim(); - if (!candidate) { - return null; - } - - if (isAllowedDataImageUrl(candidate)) { - return candidate; - } - - if (candidate.toLowerCase().startsWith(DATA_URL_PREFIX)) { - return null; - } - - try { - const parsed = new URL(candidate, baseHref); - return ALLOWED_OPEN_PROTOCOLS.has(parsed.protocol.toLowerCase()) ? parsed.toString() : null; - } catch { - return null; - } -} diff --git a/ui/src/ui/open-external-url.test.ts b/ui/src/ui/open-external-url.test.ts new file mode 100644 index 00000000000..8972516ed57 --- /dev/null +++ b/ui/src/ui/open-external-url.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "vitest"; +import { resolveSafeExternalUrl } from "./open-external-url.ts"; + +describe("resolveSafeExternalUrl", () => { + const baseHref = "https://openclaw.ai/chat"; + + it("allows absolute https URLs", () => { + expect(resolveSafeExternalUrl("https://example.com/a.png?x=1#y", baseHref)).toBe( + "https://example.com/a.png?x=1#y", + ); + }); + + it("allows relative URLs resolved against the current origin", () => { + expect(resolveSafeExternalUrl("/assets/pic.png", baseHref)).toBe( + "https://openclaw.ai/assets/pic.png", + ); + }); + + it("allows blob URLs", () => { + expect(resolveSafeExternalUrl("blob:https://openclaw.ai/abc-123", baseHref)).toBe( + "blob:https://openclaw.ai/abc-123", + ); + }); + + it("allows data image URLs when enabled", () => { + expect( + resolveSafeExternalUrl("data:image/png;base64,iVBORw0KGgo=", baseHref, { + allowDataImage: true, + }), + ).toBe("data:image/png;base64,iVBORw0KGgo="); + }); + + it("rejects non-image data URLs", () => { + expect( + resolveSafeExternalUrl("data:text/html,", baseHref, { + allowDataImage: true, + }), + ).toBeNull(); + }); + + it("rejects data image URLs unless explicitly enabled", () => { + expect(resolveSafeExternalUrl("data:image/png;base64,iVBORw0KGgo=", baseHref)).toBeNull(); + }); + + it("rejects javascript URLs", () => { + expect(resolveSafeExternalUrl("javascript:alert(1)", baseHref)).toBeNull(); + }); + + it("rejects file URLs", () => { + expect(resolveSafeExternalUrl("file:///tmp/x.png", baseHref)).toBeNull(); + }); + + it("rejects empty values", () => { + expect(resolveSafeExternalUrl(" ", baseHref)).toBeNull(); + }); +}); diff --git a/ui/src/ui/open-external-url.ts b/ui/src/ui/open-external-url.ts new file mode 100644 index 00000000000..321e69a71fc --- /dev/null +++ b/ui/src/ui/open-external-url.ts @@ -0,0 +1,68 @@ +const DATA_URL_PREFIX = "data:"; +const ALLOWED_EXTERNAL_PROTOCOLS = new Set(["http:", "https:", "blob:"]); + +function isAllowedDataImageUrl(url: string): boolean { + if (!url.toLowerCase().startsWith(DATA_URL_PREFIX)) { + return false; + } + + const commaIndex = url.indexOf(","); + if (commaIndex < DATA_URL_PREFIX.length) { + return false; + } + + const metadata = url.slice(DATA_URL_PREFIX.length, commaIndex); + const mimeType = metadata.split(";")[0]?.trim().toLowerCase() ?? ""; + return mimeType.startsWith("image/"); +} + +export type ResolveSafeExternalUrlOptions = { + allowDataImage?: boolean; +}; + +export function resolveSafeExternalUrl( + rawUrl: string, + baseHref: string, + opts: ResolveSafeExternalUrlOptions = {}, +): string | null { + const candidate = rawUrl.trim(); + if (!candidate) { + return null; + } + + if (opts.allowDataImage === true && isAllowedDataImageUrl(candidate)) { + return candidate; + } + + if (candidate.toLowerCase().startsWith(DATA_URL_PREFIX)) { + return null; + } + + try { + const parsed = new URL(candidate, baseHref); + return ALLOWED_EXTERNAL_PROTOCOLS.has(parsed.protocol.toLowerCase()) ? parsed.toString() : null; + } catch { + return null; + } +} + +export type OpenExternalUrlSafeOptions = ResolveSafeExternalUrlOptions & { + baseHref?: string; +}; + +export function openExternalUrlSafe( + rawUrl: string, + opts: OpenExternalUrlSafeOptions = {}, +): WindowProxy | null { + const baseHref = opts.baseHref ?? window.location.href; + const safeUrl = resolveSafeExternalUrl(rawUrl, baseHref, opts); + if (!safeUrl) { + return null; + } + + const opened = window.open(safeUrl, "_blank", "noopener,noreferrer"); + if (opened) { + opened.opener = null; + } + return opened; +} diff --git a/ui/src/ui/views/chat-image-open.browser.test.ts b/ui/src/ui/views/chat-image-open.browser.test.ts new file mode 100644 index 00000000000..60e6df26554 --- /dev/null +++ b/ui/src/ui/views/chat-image-open.browser.test.ts @@ -0,0 +1,53 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { mountApp, registerAppMountHooks } from "../test-helpers/app-mount.ts"; + +registerAppMountHooks(); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function renderAssistantImage(url: string) { + return { + role: "assistant", + content: [{ type: "image_url", image_url: { url } }], + timestamp: Date.now(), + }; +} + +describe("chat image open safety", () => { + it("opens safe image URLs in a hardened new tab", async () => { + const app = mountApp("/chat"); + await app.updateComplete; + + const openSpy = vi.spyOn(window, "open").mockReturnValue(null); + app.chatMessages = [renderAssistantImage("https://example.com/cat.png")]; + await app.updateComplete; + + const image = app.querySelector(".chat-message-image"); + expect(image).not.toBeNull(); + image?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + + expect(openSpy).toHaveBeenCalledTimes(1); + expect(openSpy).toHaveBeenCalledWith( + "https://example.com/cat.png", + "_blank", + "noopener,noreferrer", + ); + }); + + it("does not open unsafe image URLs", async () => { + const app = mountApp("/chat"); + await app.updateComplete; + + const openSpy = vi.spyOn(window, "open").mockReturnValue(null); + app.chatMessages = [renderAssistantImage("javascript:alert(1)")]; + await app.updateComplete; + + const image = app.querySelector(".chat-message-image"); + expect(image).not.toBeNull(); + image?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + + expect(openSpy).not.toHaveBeenCalled(); + }); +});