diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f7c5a236a5..d1a3ec9702e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ Docs: https://docs.openclaw.ai - Plugins/provider hooks: stop recursive provider snapshot loads from overflowing the stack during plugin initialization, while still preserving cached nested provider-hook results. (#61922, #61938, #61946, #61951) - Exec/runtime events: mark background `notifyOnExit` summaries and ACP parent-stream relays as untrusted system events so lower-trust runtime output no longer re-enters later turns as trusted `System:` text. - Hooks/wake: queue direct and mapped wake-hook payloads as untrusted system events so external wake content no longer enters the main session as trusted input. (#62003) +- Browser/node invoke: block persistent browser profile create, reset, and delete mutations through `browser.proxy` on both gateway-forwarded `node.invoke` and the node-host proxy path, even when no profile allowlist is configured. (#60489) - Slack/thread mentions: add `channels.slack.thread.requireExplicitMention` so Slack channels that already require mentions can also require explicit `@bot` mentions inside bot-participated threads. (#58276) Thanks @praktika-engineer. - UI/light mode: target both root and nested WebKit scrollbar thumbs in the light theme so page-level and container scrollbars stay visible on light backgrounds. (#61753) Thanks @chziyue. - Matrix/onboarding: add an invite auto-join setup step with explicit off warnings and strict stable-target validation so new Matrix accounts stop silently ignoring invited rooms and fresh DM-style invites unless operators opt in. (#62168) Thanks @gumadeiras. diff --git a/extensions/browser/src/node-host/invoke-browser.test.ts b/extensions/browser/src/node-host/invoke-browser.test.ts index f131558e028..85c6503cfba 100644 --- a/extensions/browser/src/node-host/invoke-browser.test.ts +++ b/extensions/browser/src/node-host/invoke-browser.test.ts @@ -316,9 +316,7 @@ describe("runBrowserProxyCommand", () => { timeoutMs: 50, }), ), - ).rejects.toThrow( - "INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles when allowProfiles is configured", - ); + ).rejects.toThrow("INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles"); expect(dispatcherMocks.dispatch).not.toHaveBeenCalled(); }); @@ -336,9 +334,7 @@ describe("runBrowserProxyCommand", () => { timeoutMs: 50, }), ), - ).rejects.toThrow( - "INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles when allowProfiles is configured", - ); + ).rejects.toThrow("INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles"); expect(dispatcherMocks.dispatch).not.toHaveBeenCalled(); }); @@ -357,9 +353,7 @@ describe("runBrowserProxyCommand", () => { timeoutMs: 50, }), ), - ).rejects.toThrow( - "INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles when allowProfiles is configured", - ); + ).rejects.toThrow("INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles"); expect(dispatcherMocks.dispatch).not.toHaveBeenCalled(); }); @@ -390,27 +384,17 @@ describe("runBrowserProxyCommand", () => { ); }); - it("preserves legacy proxy behavior when allowProfiles is empty", async () => { - dispatcherMocks.dispatch.mockResolvedValue({ - status: 200, - body: { ok: true }, - }); - - await runBrowserProxyCommand( - JSON.stringify({ - method: "POST", - path: "/profiles/create", - body: { name: "poc", cdpUrl: "http://127.0.0.1:9222" }, - timeoutMs: 50, - }), - ); - - expect(dispatcherMocks.dispatch).toHaveBeenCalledWith( - expect.objectContaining({ - method: "POST", - path: "/profiles/create", - body: { name: "poc", cdpUrl: "http://127.0.0.1:9222" }, - }), - ); + it("rejects persistent profile creation when allowProfiles is empty", async () => { + await expect( + runBrowserProxyCommand( + JSON.stringify({ + method: "POST", + path: "/profiles/create", + body: { name: "poc", cdpUrl: "http://127.0.0.1:9222" }, + timeoutMs: 50, + }), + ), + ).rejects.toThrow("INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles"); + expect(dispatcherMocks.dispatch).not.toHaveBeenCalled(); }); }); diff --git a/extensions/browser/src/node-host/invoke-browser.ts b/extensions/browser/src/node-host/invoke-browser.ts index 6c054679f35..b403fc03c50 100644 --- a/extensions/browser/src/node-host/invoke-browser.ts +++ b/extensions/browser/src/node-host/invoke-browser.ts @@ -240,12 +240,10 @@ export async function runBrowserProxyCommand(paramsJSON?: string | null): Promis profile: params.profile, }) ?? ""; const allowedProfiles = proxyConfig.allowProfiles; + if (isPersistentBrowserProfileMutation(method, path)) { + throw new Error("INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles"); + } if (allowedProfiles.length > 0) { - if (isPersistentBrowserProfileMutation(method, path)) { - throw new Error( - "INVALID_REQUEST: browser.proxy cannot mutate persistent browser profiles when allowProfiles is configured", - ); - } if (path !== "/profiles") { const profileToCheck = requestedProfile || resolved.defaultProfile; if (!isProfileAllowed({ allowProfiles: allowedProfiles, profile: profileToCheck })) { diff --git a/src/gateway/server-methods/nodes.ts b/src/gateway/server-methods/nodes.ts index 3658d96a45f..ce972ad8acc 100644 --- a/src/gateway/server-methods/nodes.ts +++ b/src/gateway/server-methods/nodes.ts @@ -98,6 +98,39 @@ type PendingNodeAction = { const pendingNodeActionsById = new Map(); +function normalizeBrowserProxyPath(value: string): string { + const trimmed = value.trim(); + if (!trimmed) { + return trimmed; + } + const withLeadingSlash = trimmed.startsWith("/") ? trimmed : `/${trimmed}`; + if (withLeadingSlash.length <= 1) { + return withLeadingSlash; + } + return withLeadingSlash.replace(/\/+$/, ""); +} + +function isPersistentBrowserProxyMutation(method: string, path: string): boolean { + const normalizedPath = normalizeBrowserProxyPath(path); + if ( + method === "POST" && + (normalizedPath === "/profiles/create" || normalizedPath === "/reset-profile") + ) { + return true; + } + return method === "DELETE" && /^\/profiles\/[^/]+$/.test(normalizedPath); +} + +function isForbiddenBrowserProxyMutation(params: unknown): boolean { + if (!params || typeof params !== "object") { + return false; + } + const candidate = params as { method?: unknown; path?: unknown }; + const method = typeof candidate.method === "string" ? candidate.method.trim().toUpperCase() : ""; + const path = typeof candidate.path === "string" ? candidate.path.trim() : ""; + return Boolean(method && path && isPersistentBrowserProxyMutation(method, path)); +} + async function resolveDirectNodePushConfig() { const auth = await resolveApnsAuthConfigFromEnv(process.env); return auth.ok @@ -848,6 +881,18 @@ export const nodeHandlers: GatewayRequestHandlers = { ); return; } + if (command === "browser.proxy" && isForbiddenBrowserProxyMutation(p.params)) { + respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + "node.invoke cannot mutate persistent browser profiles via browser.proxy", + { details: { command } }, + ), + ); + return; + } await respondUnavailableOnThrow(respond, async () => { let nodeSession = context.nodeRegistry.get(nodeId); diff --git a/src/gateway/server.node-invoke-approval-bypass.test.ts b/src/gateway/server.node-invoke-approval-bypass.test.ts index 1f8392caae3..d7a572774fb 100644 --- a/src/gateway/server.node-invoke-approval-bypass.test.ts +++ b/src/gateway/server.node-invoke-approval-bypass.test.ts @@ -207,6 +207,7 @@ describe("node.invoke approval bypass", () => { const connectLinuxNode = async ( onInvoke: (payload: unknown) => void, deviceIdentity?: DeviceIdentity, + commands: string[] = ["system.run"], ) => { let readyResolve: (() => void) | null = null; const ready = new Promise((resolve) => { @@ -226,7 +227,7 @@ describe("node.invoke approval bypass", () => { platform: "linux", mode: GATEWAY_CLIENT_MODES.NODE, scopes: [], - commands: ["system.run"], + commands, deviceIdentity: resolvedDeviceIdentity, onHelloOk: () => readyResolve?.(), onEvent: (evt) => { @@ -322,6 +323,39 @@ describe("node.invoke approval bypass", () => { } }); + test("rejects browser.proxy persistent profile mutations before forwarding", async () => { + let sawInvoke = false; + const node = await connectLinuxNode( + () => { + sawInvoke = true; + }, + undefined, + ["browser.proxy"], + ); + const ws = await connectOperator(["operator.write"]); + try { + const nodeId = await getConnectedNodeId(ws); + const res = await rpcReq(ws, "node.invoke", { + nodeId, + command: "browser.proxy", + params: { + method: "POST", + path: "/profiles/create", + body: { name: "poc", cdpUrl: "http://127.0.0.1:9222" }, + }, + idempotencyKey: crypto.randomUUID(), + }); + expect(res.ok).toBe(false); + expect(res.error?.message ?? "").toContain( + "node.invoke cannot mutate persistent browser profiles via browser.proxy", + ); + await expectNoForwardedInvoke(() => sawInvoke); + } finally { + ws.close(); + node.stop(); + } + }); + test("binds approvals to decision/device and blocks cross-device replay", async () => { let invokeCount = 0; let lastInvokeParams: Record | null = null;