From 30aa7e0d4d225650b58c2fba79e71bffa5627b08 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Apr 2026 03:23:25 +0100 Subject: [PATCH] fix: harden Windows browser open --- CHANGELOG.md | 1 + src/commands/onboard-helpers.test.ts | 31 +++++++++++++++++++----- src/infra/browser-open.ts | 35 ++++++++++++++++++++++++---- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8c3397b7f0..87eadff6c0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Dashboard/Windows: open Control UI and OAuth URLs through the system URL handler without `cmd.exe` parsing or PATH-based `rundll32` lookup, and reject non-HTTP browser-open inputs. Fixes #71098. Thanks @Sanjays2402. - Providers/OpenAI: separate API-key and Codex sign-in onboarding groups, and avoid replaying stale OpenAI Responses reasoning blocks after a model route switch. - Browser/config: expand `~` in `browser.executablePath` before Chromium launch, so home-relative custom browser paths no longer fail with `ENOENT`. Fixes #67264. Thanks @Quratulain-bilal. - Telegram/streaming: hide tool-progress status updates by default while keeping explicit `streaming.preview.toolProgress` opt-in support for edited preview messages. Fixes #71320. Thanks @neeravmakwana. diff --git a/src/commands/onboard-helpers.test.ts b/src/commands/onboard-helpers.test.ts index d4c658adf24..b66c48b2a85 100644 --- a/src/commands/onboard-helpers.test.ts +++ b/src/commands/onboard-helpers.test.ts @@ -1,4 +1,5 @@ import os from "node:os"; +import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { normalizeGatewayTokenInput, @@ -39,17 +40,19 @@ vi.mock("../gateway/probe.js", () => ({ })); afterEach(() => { + vi.clearAllMocks(); vi.restoreAllMocks(); vi.unstubAllEnvs(); }); describe("openUrl", () => { - it("passes OAuth URLs to explorer.exe on win32 without cmd parsing", async () => { + it("passes OAuth URLs to Windows FileProtocolHandler without cmd parsing", async () => { vi.stubEnv("VITEST", ""); vi.stubEnv("NODE_ENV", ""); + vi.stubEnv("SystemRoot", "C:\\Windows"); const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); - vi.stubEnv("VITEST", ""); vi.stubEnv("NODE_ENV", "development"); + const rundll32 = path.win32.join("C:\\Windows", "System32", "rundll32.exe"); const url = "https://accounts.google.com/o/oauth2/v2/auth?client_id=abc&response_type=code&redirect_uri=http%3A%2F%2Flocalhost"; @@ -59,20 +62,36 @@ describe("openUrl", () => { expect(mocks.runCommandWithTimeout).toHaveBeenCalledTimes(1); const [argv, options] = mocks.runCommandWithTimeout.mock.calls[0] ?? []; - expect(argv).toEqual(["explorer.exe", url]); + expect(argv).toEqual([rundll32, "url.dll,FileProtocolHandler", url]); expect(options).toMatchObject({ timeoutMs: 5_000 }); expect(options?.windowsVerbatimArguments).toBeUndefined(); platformSpy.mockRestore(); }); + + it("does not pass non-http URLs to the OS browser handler", async () => { + vi.stubEnv("VITEST", ""); + vi.stubEnv("NODE_ENV", "development"); + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + + const ok = await openUrl("file://C:/Users/test/secrets.txt"); + + expect(ok).toBe(false); + expect(mocks.runCommandWithTimeout).not.toHaveBeenCalled(); + + platformSpy.mockRestore(); + }); }); describe("resolveBrowserOpenCommand", () => { - it("uses explorer.exe on win32", async () => { + it("uses trusted rundll32 on win32", async () => { + vi.stubEnv("SystemRoot", "C:\\Windows"); const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const rundll32 = path.win32.join("C:\\Windows", "System32", "rundll32.exe"); + const resolved = await resolveBrowserOpenCommand(); - expect(resolved.argv).toEqual(["explorer.exe"]); - expect(resolved.command).toBe("explorer.exe"); + expect(resolved.argv).toEqual([rundll32, "url.dll,FileProtocolHandler"]); + expect(resolved.command).toBe(rundll32); platformSpy.mockRestore(); }); }); diff --git a/src/infra/browser-open.ts b/src/infra/browser-open.ts index 3a4ea559f9d..bf9151f1d2a 100644 --- a/src/infra/browser-open.ts +++ b/src/infra/browser-open.ts @@ -1,3 +1,4 @@ +import path from "node:path"; import { runCommandWithTimeout } from "../process/exec.js"; import { detectBinary } from "./detect-binary.js"; import { isWSL } from "./wsl.js"; @@ -21,6 +22,23 @@ function shouldSkipBrowserOpenInTests(): boolean { return process.env.NODE_ENV === "test"; } +function resolveWindowsRundll32Path(): string { + const systemRoot = process.env.SystemRoot?.trim() || process.env.windir?.trim() || "C:\\Windows"; + return path.win32.join(systemRoot, "System32", "rundll32.exe"); +} + +function normalizeBrowserOpenUrl(raw: string): string | null { + try { + const parsed = new URL(raw); + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { + return null; + } + return parsed.toString(); + } catch { + return null; + } +} + export async function resolveBrowserOpenCommand(): Promise { const platform = process.platform; const hasDisplay = Boolean(process.env.DISPLAY || process.env.WAYLAND_DISPLAY); @@ -34,9 +52,10 @@ export async function resolveBrowserOpenCommand(): Promise { } if (platform === "win32") { + const rundll32 = resolveWindowsRundll32Path(); return { - argv: ["explorer.exe"], - command: "explorer.exe", + argv: [rundll32, "url.dll,FileProtocolHandler"], + command: rundll32, }; } @@ -80,12 +99,16 @@ export async function openUrl(url: string): Promise { if (shouldSkipBrowserOpenInTests()) { return false; } + const normalizedUrl = normalizeBrowserOpenUrl(url); + if (!normalizedUrl) { + return false; + } const resolved = await resolveBrowserOpenCommand(); if (!resolved.argv) { return false; } const command = [...resolved.argv]; - command.push(url); + command.push(normalizedUrl); try { await runCommandWithTimeout(command, { timeoutMs: 5_000 }); return true; @@ -98,6 +121,10 @@ export async function openUrlInBackground(url: string): Promise { if (shouldSkipBrowserOpenInTests()) { return false; } + const normalizedUrl = normalizeBrowserOpenUrl(url); + if (!normalizedUrl) { + return false; + } if (process.platform !== "darwin") { return false; } @@ -106,7 +133,7 @@ export async function openUrlInBackground(url: string): Promise { return false; } try { - await runCommandWithTimeout(["open", "-g", url], { timeoutMs: 5_000 }); + await runCommandWithTimeout(["open", "-g", normalizedUrl], { timeoutMs: 5_000 }); return true; } catch { return false;