From 1b3a1246d00c35de419ca81ba301332f12a11ef2 Mon Sep 17 00:00:00 2001 From: Devin Robison Date: Wed, 25 Mar 2026 12:36:59 -0700 Subject: [PATCH] Block reset-profile on lower-privilege browser request surfaces (#54618) * Block reset-profile on lower-privilege browser request surfaces * add missing tests * Fix tests * Test fix --- src/browser/request-policy.test.ts | 25 +++++++++++++++ src/browser/request-policy.ts | 5 ++- .../browser.profile-from-body.test.ts | 31 ++++++++++++++++++- src/gateway/server-methods/browser.ts | 2 +- src/node-host/invoke-browser.test.ts | 25 +++++++++++++-- src/node-host/invoke-browser.ts | 2 +- 6 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 src/browser/request-policy.test.ts diff --git a/src/browser/request-policy.test.ts b/src/browser/request-policy.test.ts new file mode 100644 index 00000000000..0920ad416cb --- /dev/null +++ b/src/browser/request-policy.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from "vitest"; +import { isPersistentBrowserProfileMutation } from "./request-policy.js"; + +describe("isPersistentBrowserProfileMutation", () => { + it.each([ + ["POST", "/profiles/create"], + ["POST", "profiles/create"], + ["POST", "/reset-profile"], + ["POST", "reset-profile"], + ["DELETE", "/profiles/poc"], + ])("treats %s %s as a persistent profile mutation", (method, path) => { + expect(isPersistentBrowserProfileMutation(method, path)).toBe(true); + }); + + it.each([ + ["GET", "/profiles"], + ["GET", "/profiles/poc"], + ["GET", "/status"], + ["POST", "/stop"], + ["DELETE", "/profiles"], + ["DELETE", "/profiles/poc/tabs"], + ])("allows non-mutating browser routes for %s %s", (method, path) => { + expect(isPersistentBrowserProfileMutation(method, path)).toBe(false); + }); +}); diff --git a/src/browser/request-policy.ts b/src/browser/request-policy.ts index 40df346c68c..6288c6b11a6 100644 --- a/src/browser/request-policy.ts +++ b/src/browser/request-policy.ts @@ -18,7 +18,10 @@ export function normalizeBrowserRequestPath(value: string): string { export function isPersistentBrowserProfileMutation(method: string, path: string): boolean { const normalizedPath = normalizeBrowserRequestPath(path); - if (method === "POST" && normalizedPath === "/profiles/create") { + if ( + method === "POST" && + (normalizedPath === "/profiles/create" || normalizedPath === "/reset-profile") + ) { return true; } return method === "DELETE" && /^\/profiles\/[^/]+$/.test(normalizedPath); diff --git a/src/gateway/server-methods/browser.profile-from-body.test.ts b/src/gateway/server-methods/browser.profile-from-body.test.ts index 3b2caf8dbdc..5f709d1aed7 100644 --- a/src/gateway/server-methods/browser.profile-from-body.test.ts +++ b/src/gateway/server-methods/browser.profile-from-body.test.ts @@ -122,6 +122,16 @@ describe("browser.request profile selection", () => { path: "profiles/poc", body: undefined, }, + { + method: "POST", + path: "/reset-profile", + body: { profile: "poc", name: "poc" }, + }, + { + method: "POST", + path: "reset-profile", + body: { profile: "poc", name: "poc" }, + }, ])("blocks persistent profile mutations for $method $path", async ({ method, path, body }) => { const { respond, nodeRegistry } = await runBrowserRequest({ method, @@ -134,8 +144,27 @@ describe("browser.request profile selection", () => { false, undefined, expect.objectContaining({ - message: "browser.request cannot create or delete persistent browser profiles", + message: "browser.request cannot mutate persistent browser profiles", }), ); }); + + it("allows non-mutating profile reads", async () => { + const { respond, nodeRegistry } = await runBrowserRequest({ + method: "GET", + path: "/profiles", + }); + + expect(nodeRegistry.invoke).toHaveBeenCalledWith( + expect.objectContaining({ + command: "browser.proxy", + params: expect.objectContaining({ + method: "GET", + path: "/profiles", + }), + }), + ); + const call = respond.mock.calls[0] as RespondCall | undefined; + expect(call?.[0]).toBe(true); + }); }); diff --git a/src/gateway/server-methods/browser.ts b/src/gateway/server-methods/browser.ts index 72d88a4f4cf..654ae6297b9 100644 --- a/src/gateway/server-methods/browser.ts +++ b/src/gateway/server-methods/browser.ts @@ -158,7 +158,7 @@ export const browserHandlers: GatewayRequestHandlers = { undefined, errorShape( ErrorCodes.INVALID_REQUEST, - "browser.request cannot create or delete persistent browser profiles", + "browser.request cannot mutate persistent browser profiles", ), ); return; diff --git a/src/node-host/invoke-browser.test.ts b/src/node-host/invoke-browser.test.ts index 8dcd2ac817d..f92889468b4 100644 --- a/src/node-host/invoke-browser.test.ts +++ b/src/node-host/invoke-browser.test.ts @@ -238,7 +238,7 @@ describe("runBrowserProxyCommand", () => { }), ), ).rejects.toThrow( - "INVALID_REQUEST: browser.proxy cannot create or delete persistent browser profiles when allowProfiles is configured", + "INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles when allowProfiles is configured", ); expect(dispatcherMocks.dispatch).not.toHaveBeenCalled(); }); @@ -258,7 +258,28 @@ describe("runBrowserProxyCommand", () => { }), ), ).rejects.toThrow( - "INVALID_REQUEST: browser.proxy cannot create or delete persistent browser profiles when allowProfiles is configured", + "INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles when allowProfiles is configured", + ); + expect(dispatcherMocks.dispatch).not.toHaveBeenCalled(); + }); + + it("rejects persistent profile reset when allowProfiles is configured", async () => { + configMocks.loadConfig.mockReturnValue({ + browser: {}, + nodeHost: { browserProxy: { enabled: true, allowProfiles: ["openclaw"] } }, + }); + + await expect( + runBrowserProxyCommand( + JSON.stringify({ + method: "POST", + path: "/reset-profile", + body: { profile: "openclaw", name: "openclaw" }, + timeoutMs: 50, + }), + ), + ).rejects.toThrow( + "INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles when allowProfiles is configured", ); expect(dispatcherMocks.dispatch).not.toHaveBeenCalled(); }); diff --git a/src/node-host/invoke-browser.ts b/src/node-host/invoke-browser.ts index d352d2d8ea1..dc41220f61c 100644 --- a/src/node-host/invoke-browser.ts +++ b/src/node-host/invoke-browser.ts @@ -239,7 +239,7 @@ export async function runBrowserProxyCommand(paramsJSON?: string | null): Promis if (allowedProfiles.length > 0) { if (isPersistentBrowserProfileMutation(method, path)) { throw new Error( - "INVALID_REQUEST: browser.proxy cannot create or delete persistent browser profiles when allowProfiles is configured", + "INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles when allowProfiles is configured", ); } if (path !== "/profiles") {