mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-12 09:41:11 +00:00
fix(browser): align browser.proxy profile mutation guards (#60489)
* fix(browser): block proxy profile mutations * docs(changelog): add browser proxy guard entry --------- Co-authored-by: Devin Robison <drobison@nvidia.com> Co-authored-by: Devin Robison <drobison00@users.noreply.github.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 })) {
|
||||
|
||||
@@ -98,6 +98,39 @@ type PendingNodeAction = {
|
||||
|
||||
const pendingNodeActionsById = new Map<string, PendingNodeAction[]>();
|
||||
|
||||
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);
|
||||
|
||||
@@ -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<void>((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<string, unknown> | null = null;
|
||||
|
||||
Reference in New Issue
Block a user