Skip to content

Commit 1ceea79

Browse files
LaPhilosophieLaPhilosophie
committed
fix(gateway): require admin scope for browser proxy invoke
1 parent 1350efc commit 1ceea79

15 files changed

Lines changed: 499 additions & 14 deletions

‎extensions/browser/src/browser-tool.test.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,7 @@ function nodeInvokeCall(callIndex: number): {
416416
body?: Record<string, unknown>;
417417
};
418418
};
419+
extra?: { scopes?: string[] };
419420
} {
420421
const toolName = mockCallArg<string>(gatewayMocks.callGatewayTool, callIndex, 0);
421422
const options = mockCallArg<{ timeoutMs?: number }>(gatewayMocks.callGatewayTool, callIndex, 1);
@@ -431,8 +432,13 @@ function nodeInvokeCall(callIndex: number): {
431432
body?: Record<string, unknown>;
432433
};
433434
}>(gatewayMocks.callGatewayTool, callIndex, 2);
435+
const extra = mockCallArg<{ scopes?: string[] } | undefined>(
436+
gatewayMocks.callGatewayTool,
437+
callIndex,
438+
3,
439+
);
434440
expect(toolName).toBe("node.invoke");
435-
return { options, request };
441+
return { options, request, extra };
436442
}
437443

438444
function lastNodeInvokeCall(): ReturnType<typeof nodeInvokeCall> {
@@ -828,8 +834,9 @@ describe("browser tool snapshot maxChars", () => {
828834
const tool = createBrowserTool();
829835
await tool.execute?.("call-1", { action: "status", target: "node" });
830836

831-
const { options, request } = lastNodeInvokeCall();
837+
const { options, request, extra } = lastNodeInvokeCall();
832838
expect(options.timeoutMs).toBe(25_000);
839+
expect(extra?.scopes).toEqual(["operator.admin"]);
833840
expect(request.nodeId).toBe("node-1");
834841
expect(request.command).toBe("browser.proxy");
835842
expect(request.params?.timeoutMs).toBe(20_000);

‎extensions/browser/src/browser-tool.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@ async function callBrowserProxy(params: {
316316
},
317317
idempotencyKey: crypto.randomUUID(),
318318
},
319+
{ scopes: ["operator.admin"] },
319320
);
320321
const parsed = unwrapBrowserProxyPayload(payload);
321322
if (!parsed || typeof parsed !== "object" || !("result" in parsed)) {

‎extensions/google-meet/src/transports/chrome-browser-proxy.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ describe("Google Meet Chrome browser proxy", () => {
3535
timeoutMs: 100,
3636
},
3737
timeoutMs: 5_100,
38+
scopes: ["operator.admin"],
3839
});
3940
});
4041

‎extensions/google-meet/src/transports/chrome-browser-proxy.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ export async function callBrowserProxyOnNode(params: {
191191
timeoutMs: params.timeoutMs,
192192
},
193193
timeoutMs: addTimerTimeoutGraceMs(params.timeoutMs) ?? 1,
194+
scopes: ["operator.admin"],
194195
});
195196
return parseBrowserProxyResult(raw);
196197
}

‎src/gateway/server-methods/nodes.invoke-wake.test.ts‎

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ async function invokeNode(params: {
302302
error?: { code?: string; message?: string } | null;
303303
}>;
304304
};
305+
client?: unknown;
305306
requestParams?: Partial<Record<string, unknown>>;
306307
}) {
307308
const respond = vi.fn();
@@ -318,13 +319,32 @@ async function invokeNode(params: {
318319
logGateway,
319320
getRuntimeConfig: () => mocks.getRuntimeConfig(),
320321
} as never,
321-
client: null,
322+
client: (params.client ?? null) as never,
322323
req: { type: "req", id: "req-node-invoke", method: "node.invoke" },
323324
isWebchatConnect: () => false,
324325
});
325326
return respond;
326327
}
327328

329+
function createOperatorClient(params?: { scopes?: string[]; pluginRuntimeOwnerId?: string }) {
330+
return {
331+
connect: {
332+
role: "operator" as const,
333+
scopes: params?.scopes ?? ["operator.write"],
334+
client: {
335+
id: "operator-test",
336+
mode: "backend" as const,
337+
name: "operator-test",
338+
platform: "node",
339+
version: "test",
340+
},
341+
},
342+
internal: params?.pluginRuntimeOwnerId
343+
? { pluginRuntimeOwnerId: params.pluginRuntimeOwnerId }
344+
: {},
345+
};
346+
}
347+
328348
function createNodeClient(nodeId: string, commands?: string[]) {
329349
return {
330350
connect: {
@@ -505,6 +525,72 @@ describe("node.invoke APNs wake path", () => {
505525
vi.useRealTimers();
506526
});
507527

528+
it("rejects browser.proxy for plugin runtime owners without admin scope", async () => {
529+
const nodeRegistry = {
530+
get: vi.fn(() => ({
531+
nodeId: "browser-node",
532+
commands: ["browser.proxy"],
533+
})),
534+
invoke: vi.fn().mockResolvedValue({
535+
ok: true,
536+
payloadJSON: '{"ok":true}',
537+
}),
538+
};
539+
540+
const respond = await invokeNode({
541+
nodeRegistry,
542+
client: createOperatorClient({
543+
scopes: ["operator.write"],
544+
pluginRuntimeOwnerId: "third-party",
545+
}),
546+
requestParams: {
547+
nodeId: "browser-node",
548+
command: "browser.proxy",
549+
params: { method: "GET", path: "/profiles" },
550+
},
551+
});
552+
553+
const call = firstRespondCall(respond);
554+
expect(call[0]).toBe(false);
555+
expect(call[2]?.message).toContain("missing scope: operator.admin");
556+
expect(nodeRegistry.invoke).not.toHaveBeenCalled();
557+
});
558+
559+
it("allows browser.proxy for admin-scoped plugin runtime callers", async () => {
560+
const nodeRegistry = {
561+
get: vi.fn(() => ({
562+
nodeId: "browser-node",
563+
commands: ["browser.proxy"],
564+
})),
565+
invoke: vi.fn().mockResolvedValue({
566+
ok: true,
567+
payloadJSON: '{"ok":true}',
568+
}),
569+
};
570+
571+
const respond = await invokeNode({
572+
nodeRegistry,
573+
client: createOperatorClient({
574+
scopes: ["operator.admin"],
575+
pluginRuntimeOwnerId: "google-meet",
576+
}),
577+
requestParams: {
578+
nodeId: "browser-node",
579+
command: "browser.proxy",
580+
params: { method: "GET", path: "/profiles" },
581+
},
582+
});
583+
584+
const call = firstRespondCall(respond);
585+
expect(call[0]).toBe(true);
586+
expect(nodeRegistry.invoke).toHaveBeenCalledTimes(1);
587+
expectRecordFields(mockArg(nodeRegistry.invoke, 0, 0), "node invoke payload", {
588+
nodeId: "browser-node",
589+
command: "browser.proxy",
590+
params: { method: "GET", path: "/profiles" },
591+
});
592+
});
593+
508594
it("keeps the existing not-connected response when wake path is unavailable", async () => {
509595
mocks.loadApnsRegistration.mockResolvedValue(null);
510596

‎src/gateway/server-methods/nodes.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ const TALK_PTT_COMMANDS = new Set([
9595
"talk.ptt.cancel",
9696
"talk.ptt.once",
9797
]);
98+
const BROWSER_PROXY_REQUIRED_SCOPE = "operator.admin";
9899
const talkPttEventSeqBySessionId = new Map<string, number>();
99100

100101
type NodeWakeNudgeAttempt = {
@@ -150,6 +151,11 @@ function isForbiddenBrowserProxyMutation(params: unknown): boolean {
150151
return Boolean(method && path && isPersistentBrowserProxyMutation(method, path));
151152
}
152153

154+
function clientHasOperatorAdminScope(client: GatewayClient | null): boolean {
155+
const scopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : [];
156+
return scopes.includes(BROWSER_PROXY_REQUIRED_SCOPE);
157+
}
158+
153159
function normalizePluginSurfaceRefreshParams(params: unknown): { surface: string } | undefined {
154160
if (!params || typeof params !== "object") {
155161
return undefined;
@@ -1102,7 +1108,14 @@ export const nodeHandlers: GatewayRequestHandlers = {
11021108
);
11031109
return;
11041110
}
1105-
1111+
if (command === "browser.proxy" && !clientHasOperatorAdminScope(client)) {
1112+
respond(
1113+
false,
1114+
undefined,
1115+
errorShape(ErrorCodes.INVALID_REQUEST, `missing scope: ${BROWSER_PROXY_REQUIRED_SCOPE}`),
1116+
);
1117+
return;
1118+
}
11061119
await respondUnavailableOnThrow(respond, async () => {
11071120
const cfg = context.getRuntimeConfig();
11081121
let nodeSession = context.nodeRegistry.get(nodeId);

‎src/gateway/server-plugins.test.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { afterEach, beforeAll, beforeEach, describe, expect, test, vi } from "vitest";
2+
import { createPluginRecord } from "../plugins/loader-records.js";
23
import type { PluginLookUpTable } from "../plugins/plugin-lookup-table.js";
34
import type { PluginRegistry } from "../plugins/registry.js";
45
import type { PluginRuntimeGatewayRequestScope } from "../plugins/runtime/gateway-request-scope.js";
@@ -115,6 +116,30 @@ const createRegistry = (diagnostics: PluginDiagnostic[]): PluginRegistry => ({
115116
diagnostics,
116117
});
117118

119+
function addLoadedPlugin(
120+
registry: PluginRegistry,
121+
params: {
122+
id: string;
123+
origin?: PluginRegistry["plugins"][number]["origin"];
124+
trustedOfficialInstall?: boolean;
125+
},
126+
): PluginRegistry {
127+
registry.plugins.push(
128+
createPluginRecord({
129+
id: params.id,
130+
name: params.id,
131+
source: `/tmp/${params.id}/index.js`,
132+
origin: params.origin ?? "bundled",
133+
enabled: true,
134+
configSchema: false,
135+
...(params.trustedOfficialInstall !== undefined
136+
? { trustedOfficialInstall: params.trustedOfficialInstall }
137+
: {}),
138+
}),
139+
);
140+
return registry;
141+
}
142+
118143
function createLookUpTableForTest(params: {
119144
manifestRegistry?: PluginLookUpTable["manifestRegistry"];
120145
pluginIds?: readonly string[];
@@ -795,6 +820,67 @@ describe("loadGatewayPlugins", () => {
795820
expect(result.nodes).toEqual([{ nodeId: "connected", connected: true }]);
796821
});
797822

823+
test("lets trusted official plugin runtime request admin scope for browser proxy", async () => {
824+
loadOpenClawPlugins.mockReturnValue(addLoadedPlugin(createRegistry([]), { id: "google-meet" }));
825+
loadGatewayStartupPluginsForTest();
826+
serverPluginsModule.setFallbackGatewayContext(createTestContext("nodes-invoke-browser-proxy"));
827+
828+
const runtime = runtimeModule.createPluginRuntime({
829+
allowGatewaySubagentBinding: true,
830+
});
831+
await gatewayRequestScopeModule.withPluginRuntimePluginScope(
832+
{ pluginId: "google-meet", pluginOrigin: "bundled" },
833+
() =>
834+
runtime.nodes.invoke({
835+
nodeId: "node-1",
836+
command: "browser.proxy",
837+
params: { method: "GET", path: "/profiles" },
838+
scopes: ["operator.admin"],
839+
}),
840+
);
841+
842+
expect(getLastDispatchedParams()).toMatchObject({
843+
nodeId: "node-1",
844+
command: "browser.proxy",
845+
params: { method: "GET", path: "/profiles" },
846+
});
847+
expect(getLastDispatchedClientScopes()).toEqual(["operator.admin"]);
848+
expect(getLastDispatchedClientInternal().pluginRuntimeOwnerId).toBe("google-meet");
849+
});
850+
851+
test("does not let arbitrary plugin nodes runtime mint admin scope for browser proxy", async () => {
852+
loadOpenClawPlugins.mockReturnValue(
853+
addLoadedPlugin(createRegistry([]), { id: "third-party", origin: "global" }),
854+
);
855+
loadGatewayStartupPluginsForTest();
856+
serverPluginsModule.setFallbackGatewayContext(
857+
createTestContext("nodes-invoke-browser-proxy-no-elevate"),
858+
);
859+
860+
const runtime = runtimeModule.createPluginRuntime({
861+
allowGatewaySubagentBinding: true,
862+
});
863+
await gatewayRequestScopeModule.withPluginRuntimePluginScope(
864+
{ pluginId: "third-party", pluginOrigin: "global" },
865+
() =>
866+
runtime.nodes.invoke({
867+
nodeId: "node-1",
868+
command: "browser.proxy",
869+
params: { method: "GET", path: "/profiles" },
870+
scopes: ["operator.admin"],
871+
}),
872+
);
873+
874+
expect(getLastDispatchedParams()).toMatchObject({
875+
nodeId: "node-1",
876+
command: "browser.proxy",
877+
params: { method: "GET", path: "/profiles" },
878+
});
879+
expect(getLastDispatchedClientScopes()).toEqual(["operator.write"]);
880+
expect(getLastDispatchedClientScopes()).not.toContain("operator.admin");
881+
expect(getLastDispatchedClientInternal().pluginRuntimeOwnerId).toBe("third-party");
882+
});
883+
798884
test("forwards provider and model overrides when the request scope is authorized", async () => {
799885
const serverPlugins = serverPluginsModule;
800886
const runtime = await createSubagentRuntime(serverPlugins);

0 commit comments

Comments
 (0)