Skip to content

Commit 865b8db

Browse files
steipetevincentkoc
steipete
and
vincentkoc
committed
fix(gateway): advertise exec approval node commands
Co-authored-by: Vincent Koc <[email protected]>
1 parent 2db5bd3 commit 865b8db

5 files changed

Lines changed: 233 additions & 0 deletions

File tree

‎src/gateway/node-command-policy.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
isNodeCommandAllowed,
1919
normalizeDeclaredNodeCommands,
2020
resolveNodeCommandAllowlist,
21+
resolveNodePairingCommandAllowlist,
2122
} from "./node-command-policy.js";
2223

2324
describe("gateway/node-command-policy", () => {
@@ -175,12 +176,34 @@ describe("gateway/node-command-policy", () => {
175176
expect(allowlist.has("system.run")).toBe(false);
176177
expect(allowlist.has("system.run.prepare")).toBe(false);
177178
expect(allowlist.has("system.which")).toBe(false);
179+
expect(allowlist.has("system.execApprovals.get")).toBe(false);
180+
expect(allowlist.has("system.execApprovals.set")).toBe(false);
178181
expect(allowlist.has("browser.proxy")).toBe(false);
179182
expect(allowlist.has("screen.snapshot")).toBe(false);
180183
expect(allowlist.has("system.notify")).toBe(true);
181184
}
182185
});
183186

187+
it("allows exec approval commands only through desktop node pairing approval", () => {
188+
const cfg = {} as OpenClawConfig;
189+
const desktopNode = { platform: "windows", deviceFamily: "Windows" };
190+
191+
const pairingAllowlist = resolveNodePairingCommandAllowlist(cfg, desktopNode);
192+
expect(pairingAllowlist.has("system.execApprovals.get")).toBe(true);
193+
expect(pairingAllowlist.has("system.execApprovals.set")).toBe(true);
194+
195+
const unapprovedRuntimeAllowlist = resolveNodeCommandAllowlist(cfg, desktopNode);
196+
expect(unapprovedRuntimeAllowlist.has("system.execApprovals.get")).toBe(false);
197+
expect(unapprovedRuntimeAllowlist.has("system.execApprovals.set")).toBe(false);
198+
199+
const approvedRuntimeAllowlist = resolveNodeCommandAllowlist(cfg, {
200+
...desktopNode,
201+
approvedCommands: ["system.execApprovals.get", "system.execApprovals.set"],
202+
});
203+
expect(approvedRuntimeAllowlist.has("system.execApprovals.get")).toBe(true);
204+
expect(approvedRuntimeAllowlist.has("system.execApprovals.set")).toBe(true);
205+
});
206+
184207
it("keeps defaults for first-party native platform labels with matching families", () => {
185208
const cfg = {} as OpenClawConfig;
186209

‎src/gateway/node-command-policy.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { normalizeUniqueStringEntries } from "@openclaw/normalization-core/strin
55
import type { OpenClawConfig } from "../config/types.openclaw.js";
66
import {
77
NODE_BROWSER_PROXY_COMMAND,
8+
NODE_EXEC_APPROVALS_COMMANDS,
89
NODE_SYSTEM_NOTIFY_COMMAND,
910
NODE_SYSTEM_RUN_COMMANDS,
1011
} from "../infra/node-commands.js";
@@ -54,11 +55,13 @@ const IOS_SYSTEM_COMMANDS = [NODE_SYSTEM_NOTIFY_COMMAND];
5455

5556
const SYSTEM_COMMANDS = [
5657
...NODE_SYSTEM_RUN_COMMANDS,
58+
...NODE_EXEC_APPROVALS_COMMANDS,
5759
NODE_SYSTEM_NOTIFY_COMMAND,
5860
NODE_BROWSER_PROXY_COMMAND,
5961
];
6062
const DESKTOP_HOST_COMMANDS = new Set<string>([
6163
...NODE_SYSTEM_RUN_COMMANDS,
64+
...NODE_EXEC_APPROVALS_COMMANDS,
6265
NODE_BROWSER_PROXY_COMMAND,
6366
...SCREEN_COMMANDS,
6467
]);

‎src/gateway/node-connect-reconcile.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,55 @@ describe("reconcileNodePairingOnConnect", () => {
118118
);
119119
});
120120

121+
it("reapproves and then preserves Windows exec approval commands", async () => {
122+
const commands = [
123+
"system.run.prepare",
124+
"system.run",
125+
"system.which",
126+
"system.execApprovals.get",
127+
"system.execApprovals.set",
128+
];
129+
const previouslyApprovedCommands = ["system.run.prepare", "system.run", "system.which"];
130+
const connectParams = makeNodeConnectParams({
131+
client: {
132+
id: GATEWAY_CLIENT_IDS.NODE_HOST,
133+
version: "test",
134+
platform: "windows",
135+
deviceFamily: "Windows",
136+
mode: GATEWAY_CLIENT_MODES.NODE,
137+
},
138+
caps: ["system"],
139+
commands,
140+
});
141+
const requestPairing = makePendingPairingRequest("req-windows");
142+
143+
const upgrade = await reconcileNodePairingOnConnect({
144+
cfg: {} as never,
145+
connectParams,
146+
pairedNode: makePairedNode({ caps: ["system"], commands: previouslyApprovedCommands }),
147+
requestPairing,
148+
});
149+
150+
expect(upgrade.declaredCommands).toEqual(commands);
151+
expect(upgrade.effectiveCommands).toEqual(previouslyApprovedCommands);
152+
expect(requestPairing).toHaveBeenCalledWith(
153+
expect.objectContaining({
154+
commands,
155+
}),
156+
);
157+
158+
const approvedPairingRequest = vi.fn();
159+
const approvedReconnect = await reconcileNodePairingOnConnect({
160+
cfg: {} as never,
161+
connectParams,
162+
pairedNode: makePairedNode({ caps: ["system"], commands }),
163+
requestPairing: approvedPairingRequest,
164+
});
165+
166+
expect(approvedReconnect.effectiveCommands).toEqual(commands);
167+
expect(approvedPairingRequest).not.toHaveBeenCalled();
168+
});
169+
121170
it.each([
122171
["conflicts with device family", { deviceFamily: "iPhone" }],
123172
["omits device family", {}],

‎src/gateway/server-methods/exec-approvals.test.ts‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,138 @@ describe("exec approvals gateway methods", () => {
7979
}),
8080
);
8181
});
82+
83+
it.each([
84+
{
85+
method: "exec.approvals.node.get" as const,
86+
command: "system.execApprovals.get",
87+
params: { nodeId: "node-1" },
88+
commands: [],
89+
config: {},
90+
},
91+
{
92+
method: "exec.approvals.node.set" as const,
93+
command: "system.execApprovals.set",
94+
params: {
95+
nodeId: "node-1",
96+
file: { version: 1, agents: {} },
97+
baseHash: "base-hash",
98+
},
99+
commands: ["system.execApprovals.set"],
100+
config: { gateway: { nodes: { denyCommands: ["system.execApprovals.set"] } } },
101+
},
102+
])("blocks $method outside the effective command policy", async (testCase) => {
103+
const invoke = vi.fn();
104+
const respond = vi.fn();
105+
106+
await execApprovalsHandlers[testCase.method]({
107+
req: {
108+
type: "req",
109+
id: "req-node-blocked",
110+
method: testCase.method,
111+
params: testCase.params,
112+
},
113+
params: testCase.params,
114+
client: null,
115+
isWebchatConnect: () => false,
116+
respond,
117+
context: {
118+
getRuntimeConfig: () => testCase.config,
119+
nodeRegistry: {
120+
get: () => ({
121+
nodeId: "node-1",
122+
connId: "conn-1",
123+
platform: "windows",
124+
deviceFamily: "Windows",
125+
declaredCommands: [testCase.command],
126+
commands: testCase.commands,
127+
}),
128+
invoke,
129+
},
130+
} as never,
131+
});
132+
133+
expect(invoke).not.toHaveBeenCalled();
134+
expect(respond).toHaveBeenCalledWith(
135+
false,
136+
undefined,
137+
expect.objectContaining({
138+
code: "INVALID_REQUEST",
139+
details: expect.objectContaining({ command: testCase.command }),
140+
}),
141+
);
142+
});
143+
144+
it("relays approved exec-approval commands", async () => {
145+
const command = "system.execApprovals.get";
146+
const invoke = vi.fn().mockResolvedValue({ ok: true, payload: { exists: true } });
147+
const respond = vi.fn();
148+
149+
await execApprovalsHandlers["exec.approvals.node.get"]({
150+
req: {
151+
type: "req",
152+
id: "req-node-allowed",
153+
method: "exec.approvals.node.get",
154+
params: { nodeId: "node-1" },
155+
},
156+
params: { nodeId: "node-1" },
157+
client: null,
158+
isWebchatConnect: () => false,
159+
respond,
160+
context: {
161+
getRuntimeConfig: () => ({}),
162+
nodeRegistry: {
163+
get: () => ({
164+
nodeId: "node-1",
165+
connId: "conn-1",
166+
platform: "windows",
167+
deviceFamily: "Windows",
168+
declaredCommands: [command],
169+
commands: [command],
170+
}),
171+
invoke,
172+
},
173+
} as never,
174+
});
175+
176+
expect(invoke).toHaveBeenCalledWith({ nodeId: "node-1", command, params: {} });
177+
expect(respond).toHaveBeenCalledWith(true, { exists: true }, undefined);
178+
});
179+
180+
it("preserves unavailable details for unknown nodes", async () => {
181+
const invoke = vi.fn().mockResolvedValue({
182+
ok: false,
183+
error: { code: "NOT_CONNECTED", message: "node not connected" },
184+
});
185+
const respond = vi.fn();
186+
187+
await execApprovalsHandlers["exec.approvals.node.get"]({
188+
req: {
189+
type: "req",
190+
id: "req-node-missing",
191+
method: "exec.approvals.node.get",
192+
params: { nodeId: "missing-node" },
193+
},
194+
params: { nodeId: "missing-node" },
195+
client: null,
196+
isWebchatConnect: () => false,
197+
respond,
198+
context: {
199+
getRuntimeConfig: () => ({}),
200+
nodeRegistry: { get: () => undefined, invoke },
201+
} as never,
202+
});
203+
204+
expect(invoke).toHaveBeenCalled();
205+
expect(respond).toHaveBeenCalledWith(
206+
false,
207+
undefined,
208+
expect.objectContaining({
209+
code: "UNAVAILABLE",
210+
details: {
211+
nodeError: { code: "NOT_CONNECTED", message: "node not connected" },
212+
},
213+
}),
214+
);
215+
});
82216
});

‎src/gateway/server-methods/exec-approvals.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
type ExecApprovalsFile,
1818
type ExecApprovalsSnapshot,
1919
} from "../../infra/exec-approvals.js";
20+
import { isNodeCommandAllowed, resolveNodeCommandAllowlist } from "../node-command-policy.js";
2021
import { resolveBaseHashParam } from "./base-hash.js";
2122
import {
2223
respondUnavailableOnNodeInvokeError,
@@ -112,6 +113,29 @@ async function respondWithExecApprovalsNodePayload<TParams extends { nodeId: str
112113
params.respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "nodeId required"));
113114
return;
114115
}
116+
const nodeSession = params.context.nodeRegistry.get(nodeId);
117+
if (nodeSession) {
118+
const allowed = isNodeCommandAllowed({
119+
command: params.command,
120+
declaredCommands: nodeSession.commands,
121+
allowlist: resolveNodeCommandAllowlist(params.context.getRuntimeConfig(), {
122+
...nodeSession,
123+
approvedCommands: nodeSession.commands,
124+
}),
125+
});
126+
if (!allowed.ok) {
127+
params.respond(
128+
false,
129+
undefined,
130+
errorShape(
131+
ErrorCodes.INVALID_REQUEST,
132+
`node command not allowed: ${params.command} (${allowed.reason})`,
133+
{ details: { command: params.command, reason: allowed.reason } },
134+
),
135+
);
136+
return;
137+
}
138+
}
115139
await respondUnavailableOnThrow(params.respond, async () => {
116140
const res = await params.context.nodeRegistry.invoke({
117141
nodeId,

0 commit comments

Comments
 (0)