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
This commit is contained in:
Devin Robison
2026-03-25 12:36:59 -07:00
committed by GitHub
parent 4797bbc5b9
commit 1b3a1246d0
6 changed files with 84 additions and 6 deletions

View File

@@ -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);
});
});

View File

@@ -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);

View File

@@ -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);
});
});

View File

@@ -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;

View File

@@ -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();
});

View File

@@ -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") {