Skip to content

Commit 4d7cc6b

Browse files
jacobtomlinsonjacobtomlinson
authored
gateway: restrict node pairing approvals (#55951)
* gateway: restrict node pairing approvals * gateway: tighten node pairing scope checks * gateway: harden node pairing reconnects * agents: request elevated node pairing scopes * agents: fix node pairing approval preflight scopes
1 parent 68ceaf7 commit 4d7cc6b

11 files changed

Lines changed: 581 additions & 13 deletions

File tree

‎src/agents/tools/gateway.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,22 @@ describe("gateway tool defaults", () => {
174174
);
175175
});
176176

177+
it("allows explicit scope overrides for dynamic callers", async () => {
178+
callGatewayMock.mockResolvedValueOnce({ ok: true });
179+
await callGatewayTool(
180+
"node.pair.approve",
181+
{},
182+
{ requestId: "req-1" },
183+
{ scopes: ["operator.admin"] },
184+
);
185+
expect(callGatewayMock).toHaveBeenCalledWith(
186+
expect.objectContaining({
187+
method: "node.pair.approve",
188+
scopes: ["operator.admin"],
189+
}),
190+
);
191+
});
192+
177193
it("default-denies unknown methods by sending no scopes", async () => {
178194
callGatewayMock.mockResolvedValueOnce({ ok: true });
179195
await callGatewayTool("nonexistent.method", {}, {});

‎src/agents/tools/gateway.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { loadConfig, resolveGatewayPort } from "../../config/config.js";
22
import { callGateway } from "../../gateway/call.js";
33
import { resolveGatewayCredentialsFromConfig, trimToUndefined } from "../../gateway/credentials.js";
4-
import { resolveLeastPrivilegeOperatorScopesForMethod } from "../../gateway/method-scopes.js";
4+
import {
5+
resolveLeastPrivilegeOperatorScopesForMethod,
6+
type OperatorScope,
7+
} from "../../gateway/method-scopes.js";
58
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../../utils/message-channel.js";
69
import { readStringParam } from "./common.js";
710

@@ -141,10 +144,12 @@ export async function callGatewayTool<T = Record<string, unknown>>(
141144
method: string,
142145
opts: GatewayCallOptions,
143146
params?: unknown,
144-
extra?: { expectFinal?: boolean },
147+
extra?: { expectFinal?: boolean; scopes?: OperatorScope[] },
145148
) {
146149
const gateway = resolveGatewayOptions(opts);
147-
const scopes = resolveLeastPrivilegeOperatorScopesForMethod(method);
150+
const scopes = Array.isArray(extra?.scopes)
151+
? extra.scopes
152+
: resolveLeastPrivilegeOperatorScopesForMethod(method);
148153
return await callGateway<T>({
149154
url: gateway.url,
150155
token: gateway.token,

‎src/agents/tools/nodes-tool.test.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,4 +273,116 @@ describe("createNodesTool screen_record duration guardrails", () => {
273273
});
274274
expect(JSON.stringify(result?.content ?? [])).not.toContain("MEDIA:");
275275
});
276+
277+
it("uses operator.admin to approve exec-capable node pair requests", async () => {
278+
gatewayMocks.callGatewayTool.mockImplementation(async (method, _opts, params, extra) => {
279+
if (method === "node.pair.list") {
280+
return {
281+
pending: [
282+
{
283+
requestId: "req-1",
284+
commands: ["system.run"],
285+
},
286+
],
287+
};
288+
}
289+
if (method === "node.pair.approve") {
290+
return { ok: true, method, params, extra };
291+
}
292+
throw new Error(`unexpected method: ${String(method)}`);
293+
});
294+
const tool = createNodesTool();
295+
296+
await tool.execute("call-1", {
297+
action: "approve",
298+
requestId: "req-1",
299+
});
300+
301+
expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith(
302+
1,
303+
"node.pair.list",
304+
{},
305+
{},
306+
{ scopes: ["operator.pairing", "operator.write"] },
307+
);
308+
expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith(
309+
2,
310+
"node.pair.approve",
311+
{},
312+
{ requestId: "req-1" },
313+
{ scopes: ["operator.admin"] },
314+
);
315+
});
316+
317+
it("uses operator.write to approve non-exec node pair requests", async () => {
318+
gatewayMocks.callGatewayTool.mockImplementation(async (method, _opts, params, extra) => {
319+
if (method === "node.pair.list") {
320+
return {
321+
pending: [
322+
{
323+
requestId: "req-1",
324+
commands: ["canvas.snapshot"],
325+
},
326+
],
327+
};
328+
}
329+
if (method === "node.pair.approve") {
330+
return { ok: true, method, params, extra };
331+
}
332+
throw new Error(`unexpected method: ${String(method)}`);
333+
});
334+
const tool = createNodesTool();
335+
336+
await tool.execute("call-1", {
337+
action: "approve",
338+
requestId: "req-1",
339+
});
340+
341+
expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith(
342+
1,
343+
"node.pair.list",
344+
{},
345+
{},
346+
{ scopes: ["operator.pairing", "operator.write"] },
347+
);
348+
expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith(
349+
2,
350+
"node.pair.approve",
351+
{},
352+
{ requestId: "req-1" },
353+
{ scopes: ["operator.write"] },
354+
);
355+
});
356+
357+
it("uses operator.write for commandless node pair requests", async () => {
358+
gatewayMocks.callGatewayTool.mockImplementation(async (method, _opts, params, extra) => {
359+
if (method === "node.pair.list") {
360+
return {
361+
pending: [
362+
{
363+
requestId: "req-1",
364+
},
365+
],
366+
};
367+
}
368+
if (method === "node.pair.approve") {
369+
return { ok: true, method, params, extra };
370+
}
371+
throw new Error(`unexpected method: ${String(method)}`);
372+
});
373+
const tool = createNodesTool();
374+
375+
await tool.execute("call-1", {
376+
action: "approve",
377+
requestId: "req-1",
378+
});
379+
380+
expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith(
381+
2,
382+
"node.pair.approve",
383+
{},
384+
{ requestId: "req-1" },
385+
{ scopes: ["operator.write"] },
386+
);
387+
});
276388
});

‎src/agents/tools/nodes-tool.ts‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import {
1717
} from "../../cli/nodes-screen.js";
1818
import { parseDurationMs } from "../../cli/parse-duration.js";
1919
import type { OpenClawConfig } from "../../config/config.js";
20+
import type { OperatorScope } from "../../gateway/method-scopes.js";
21+
import { NODE_SYSTEM_RUN_COMMANDS } from "../../infra/node-commands.js";
2022
import { parsePreparedSystemRunPayload } from "../../infra/system-run-approval-context.js";
2123
import { imageMimeFromFormat } from "../../media/mime.js";
2224
import type { GatewayMessageChannel } from "../../utils/message-channel.js";
@@ -72,6 +74,33 @@ const NODE_READ_ACTION_COMMANDS = {
7274
} as const;
7375
type GatewayCallOptions = ReturnType<typeof readGatewayCallOptions>;
7476

77+
function resolveApproveScopes(commands: unknown): OperatorScope[] {
78+
const normalized = Array.isArray(commands)
79+
? commands.filter((value): value is string => typeof value === "string")
80+
: [];
81+
if (
82+
normalized.some((command) => NODE_SYSTEM_RUN_COMMANDS.some((allowed) => allowed === command))
83+
) {
84+
return ["operator.admin"];
85+
}
86+
if (normalized.length > 0) {
87+
return ["operator.write"];
88+
}
89+
return ["operator.write"];
90+
}
91+
92+
async function resolveNodePairApproveScopes(
93+
gatewayOpts: GatewayCallOptions,
94+
requestId: string,
95+
): Promise<OperatorScope[]> {
96+
const pairing = await callGatewayTool<{
97+
pending?: Array<{ requestId?: string; commands?: unknown }>;
98+
}>("node.pair.list", gatewayOpts, {}, { scopes: ["operator.pairing", "operator.write"] });
99+
const pending = Array.isArray(pairing?.pending) ? pairing.pending : [];
100+
const match = pending.find((entry) => entry?.requestId === requestId);
101+
return resolveApproveScopes(match?.commands);
102+
}
103+
75104
async function invokeNodeCommandPayload(params: {
76105
gatewayOpts: GatewayCallOptions;
77106
node: string;
@@ -199,10 +228,16 @@ export function createNodesTool(options?: {
199228
const requestId = readStringParam(params, "requestId", {
200229
required: true,
201230
});
231+
const scopes = await resolveNodePairApproveScopes(gatewayOpts, requestId);
202232
return jsonResult(
203-
await callGatewayTool("node.pair.approve", gatewayOpts, {
204-
requestId,
205-
}),
233+
await callGatewayTool(
234+
"node.pair.approve",
235+
gatewayOpts,
236+
{
237+
requestId,
238+
},
239+
{ scopes },
240+
),
206241
);
207242
}
208243
case "reject": {

‎src/gateway/method-scopes.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ describe("method scope resolution", () => {
2222
["sessions.abort", ["operator.write"]],
2323
["sessions.messages.subscribe", ["operator.read"]],
2424
["sessions.messages.unsubscribe", ["operator.read"]],
25+
["node.pair.approve", ["operator.write"]],
2526
["poll", ["operator.write"]],
2627
["config.patch", ["operator.admin"]],
2728
["wizard.start", ["operator.admin"]],
@@ -66,6 +67,10 @@ describe("operator scope authorization", () => {
6667
allowed: false,
6768
missingScope: "operator.write",
6869
});
70+
expect(authorizeOperatorScopesForMethod("node.pair.approve", ["operator.pairing"])).toEqual({
71+
allowed: false,
72+
missingScope: "operator.write",
73+
});
6974
});
7075

7176
it("requires approvals scope for approval methods", () => {

‎src/gateway/method-scopes.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ const METHOD_SCOPE_GROUPS: Record<OperatorScope, readonly string[]> = {
4343
[PAIRING_SCOPE]: [
4444
"node.pair.request",
4545
"node.pair.list",
46-
"node.pair.approve",
4746
"node.pair.reject",
4847
"node.pair.verify",
4948
"device.pair.list",
@@ -111,6 +110,7 @@ const METHOD_SCOPE_GROUPS: Record<OperatorScope, readonly string[]> = {
111110
"tts.setProvider",
112111
"voicewake.set",
113112
"node.invoke",
113+
"node.pair.approve",
114114
"chat.send",
115115
"chat.abort",
116116
"sessions.create",

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ export const nodeHandlers: GatewayRequestHandlers = {
539539
respond(true, list, undefined);
540540
});
541541
},
542-
"node.pair.approve": async ({ params, respond, context }) => {
542+
"node.pair.approve": async ({ params, respond, context, client }) => {
543543
if (!validateNodePairApproveParams(params)) {
544544
respondInvalidParams({
545545
respond,
@@ -549,17 +549,32 @@ export const nodeHandlers: GatewayRequestHandlers = {
549549
return;
550550
}
551551
const { requestId } = params as { requestId: string };
552+
// Intentionally fail closed for RPC callers without an explicit scoped session.
553+
const callerScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : [];
552554
await respondUnavailableOnThrow(respond, async () => {
553-
const approved = await approveNodePairing(requestId);
555+
const approved = await approveNodePairing(requestId, { callerScopes });
554556
if (!approved) {
555557
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown requestId"));
556558
return;
557559
}
560+
if ("status" in approved && approved.status === "forbidden") {
561+
respond(
562+
false,
563+
undefined,
564+
errorShape(ErrorCodes.INVALID_REQUEST, `missing scope: ${approved.missingScope}`),
565+
);
566+
return;
567+
}
568+
if (!("node" in approved)) {
569+
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown requestId"));
570+
return;
571+
}
572+
const approvedNode = approved.node;
558573
context.broadcast(
559574
"node.pair.resolved",
560575
{
561576
requestId,
562-
nodeId: approved.node.nodeId,
577+
nodeId: approvedNode.nodeId,
563578
decision: "approved",
564579
ts: Date.now(),
565580
},

0 commit comments

Comments
 (0)