From bee15d4fa27056ac71a9ca22f2b1f57c78dfee55 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 24 May 2026 01:17:58 +0100 Subject: [PATCH] fix(browser): validate inputs and redact remote URLs --- .../register.element.ts | 17 ++++- .../register.navigation.ts | 21 +++++- .../src/cli/browser-cli-manage.test.ts | 70 +++++++++++++++++++ .../browser/src/cli/browser-cli-manage.ts | 2 +- 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/extensions/browser/src/cli/browser-cli-actions-input/register.element.ts b/extensions/browser/src/cli/browser-cli-actions-input/register.element.ts index 87103cd6c27..76d3bb03433 100644 --- a/extensions/browser/src/cli/browser-cli-actions-input/register.element.ts +++ b/extensions/browser/src/cli/browser-cli-actions-input/register.element.ts @@ -13,6 +13,16 @@ export function registerBrowserElementCommands( browser: Command, parentOpts: (cmd: Command) => BrowserParentOpts, ) { + const parseRequiredNumber = (value: string, label: string): number | undefined => { + const parsed = Number(value); + if (!Number.isFinite(parsed)) { + defaultRuntime.error(danger(`Invalid ${label}: must be a finite number`)); + defaultRuntime.exit(1); + return undefined; + } + return parsed; + }; + const runElementAction = async (params: { cmd: Command; body: Record; @@ -85,8 +95,11 @@ export function registerBrowserElementCommands( .option("--button ", "Mouse button to use") .option("--delay-ms ", "Delay between mouse down/up", (v: string) => Number(v)) .action(async (xRaw: string, yRaw: string, opts, cmd) => { - const x = Number(xRaw); - const y = Number(yRaw); + const x = parseRequiredNumber(xRaw, "x"); + const y = parseRequiredNumber(yRaw, "y"); + if (x === undefined || y === undefined) { + return; + } await runElementAction({ cmd, body: { diff --git a/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts b/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts index b86e89d7135..f85003f4331 100644 --- a/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts +++ b/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts @@ -9,6 +9,16 @@ export function registerBrowserNavigationCommands( browser: Command, parentOpts: (cmd: Command) => BrowserParentOpts, ) { + const parseRequiredNumber = (value: unknown, label: string): number | undefined => { + const parsed = Number(value); + if (!Number.isFinite(parsed)) { + defaultRuntime.error(danger(`Invalid ${label}: must be a finite number`)); + defaultRuntime.exit(1); + return undefined; + } + return parsed; + }; + browser .command("navigate") .description("Navigate the current tab to a URL") @@ -48,16 +58,21 @@ export function registerBrowserNavigationCommands( .argument("", "Viewport height", (v: string) => Number(v)) .option("--target-id ", "CDP target id (or unique prefix)") .action(async (width: number, height: number, opts, cmd) => { + const normalizedWidth = parseRequiredNumber(width, "width"); + const normalizedHeight = parseRequiredNumber(height, "height"); + if (normalizedWidth === undefined || normalizedHeight === undefined) { + return; + } const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); try { await runBrowserResizeWithOutput({ parent, profile, - width, - height, + width: normalizedWidth, + height: normalizedHeight, targetId: opts.targetId, timeoutMs: 20000, - successMessage: `resized to ${width}x${height}`, + successMessage: `resized to ${normalizedWidth}x${normalizedHeight}`, }); } catch (err) { defaultRuntime.error(danger(String(err))); diff --git a/extensions/browser/src/cli/browser-cli-manage.test.ts b/extensions/browser/src/cli/browser-cli-manage.test.ts index e3c2f6a1460..7007926c7f4 100644 --- a/extensions/browser/src/cli/browser-cli-manage.test.ts +++ b/extensions/browser/src/cli/browser-cli-manage.test.ts @@ -125,6 +125,39 @@ describe("browser manage output", () => { expect(output).not.toContain("port: 0"); }); + it("redacts remote cdpUrl details in browser profiles output", async () => { + getBrowserManageCallBrowserRequestMock().mockImplementation(async (_opts: unknown, req) => + req.path === "/profiles" + ? { + profiles: [ + { + name: "remote", + driver: "openclaw", + transport: "cdp", + running: true, + tabCount: 1, + isDefault: false, + isRemote: true, + cdpPort: null, + cdpUrl: + "https://alice:supersecretpasswordvalue1234@example.com/chrome?token=supersecrettokenvalue1234567890", + color: "#00AA00", + }, + ], + } + : {}, + ); + + const program = createBrowserManageProgram(); + await program.parseAsync(["browser", "profiles"], { from: "user" }); + + const output = lastRuntimeLog(); + expect(output).toContain("cdpUrl: https://example.com/chrome?token=supers…7890"); + expect(output).not.toContain("alice"); + expect(output).not.toContain("supersecretpasswordvalue1234"); + expect(output).not.toContain("supersecrettokenvalue1234567890"); + }); + it("shows chrome-mcp transport after creating an existing-session profile", async () => { getBrowserManageCallBrowserRequestMock().mockImplementation(async (_opts: unknown, req) => req.path === "/profiles/create" @@ -153,6 +186,43 @@ describe("browser manage output", () => { expect(output).not.toContain("port: 0"); }); + it("redacts remote cdpUrl details after creating a remote profile", async () => { + getBrowserManageCallBrowserRequestMock().mockImplementation(async (_opts: unknown, req) => + req.path === "/profiles/create" + ? { + ok: true, + profile: "remote", + transport: "cdp", + cdpPort: null, + cdpUrl: + "https://alice:supersecretpasswordvalue1234@example.com/chrome?token=supersecrettokenvalue1234567890", + userDataDir: null, + color: "#00AA00", + isRemote: true, + } + : {}, + ); + + const program = createBrowserManageProgram(); + await program.parseAsync( + [ + "browser", + "create-profile", + "--name", + "remote", + "--cdp-url", + "https://alice:supersecretpasswordvalue1234@example.com/chrome?token=supersecrettokenvalue1234567890", + ], + { from: "user" }, + ); + + const output = lastRuntimeLog(); + expect(output).toContain("cdpUrl: https://example.com/chrome?token=supers…7890"); + expect(output).not.toContain("alice"); + expect(output).not.toContain("supersecretpasswordvalue1234"); + expect(output).not.toContain("supersecrettokenvalue1234567890"); + }); + it("redacts sensitive remote cdpUrl details in status output", async () => { getBrowserManageCallBrowserRequestMock().mockImplementation(async (_opts: unknown, req) => req.path === "/" diff --git a/extensions/browser/src/cli/browser-cli-manage.ts b/extensions/browser/src/cli/browser-cli-manage.ts index 3284a658830..5cf08f1d88a 100644 --- a/extensions/browser/src/cli/browser-cli-manage.ts +++ b/extensions/browser/src/cli/browser-cli-manage.ts @@ -279,7 +279,7 @@ function formatBrowserConnectionSummary(params: { : "transport: chrome-mcp"; } if (params.isRemote) { - return `cdpUrl: ${params.cdpUrl ?? "(unset)"}`; + return `cdpUrl: ${params.cdpUrl ? redactCdpUrl(params.cdpUrl) : "(unset)"}`; } return `port: ${params.cdpPort ?? "(unset)"}`; }