LLM/moltbot
1
0
Fork 0
mirror of https://github.com/moltbot/moltbot.git synced 2026-03-07 22:44:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(gateway): bind node exec approvals to nodeId

Browse Source
This commit is contained in:
Peter Steinberger
2026-02-24 03:05:36 +00:00
parent 9530c01085
commit 4a3f8438e5
18 changed files with 231 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -11,6 +11,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- Security/Shell env fallback: remove trusted-prefix shell-path fallback and only trust login shells explicitly registered in `/etc/shells`, defaulting to `/bin/sh` when `SHELL` is not registered. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Exec approvals: bind `host=node` approvals to explicit `nodeId`, reject cross-node replay of approved `system.run` requests, and include the target node in approval prompts. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Voice Call: harden Twilio webhook replay handling by preserving provider event IDs through normalization, adding bounded replay dedupe, and enforcing per-call turn-token matching for call-state transitions. This ships in the next npm release. Thanks @jiseoung for reporting.
- Security/Export session HTML: escape raw HTML markdown tokens in the exported session viewer, harden tree/header metadata rendering against HTML injection, and sanitize image data-URL MIME types in export output to prevent stored XSS when opening exported HTML files. This ships in the next npm release. Thanks @allsmog for reporting.
- Security/iOS deep links: require local confirmation (or trusted key) before forwarding `openclaw://agent` requests from iOS to gateway `agent.request`, and strip unkeyed delivery-routing fields to reduce exfiltration risk. This ships in the next npm release. Thanks @GCXWLP for reporting.

3
src/agents/bash-tools.exec-approval-request.test.ts
View File

@@ -44,6 +44,7 @@ describe("requestExecApprovalDecision", () => {
id: "approval-id",
command: "echo hi",
cwd: "/tmp",
nodeId: undefined,
host: "gateway",
security: "allowlist",
ask: "always",
@@ -62,6 +63,7 @@ describe("requestExecApprovalDecision", () => {
id: "approval-id",
command: "echo hi",
cwd: "/tmp",
nodeId: "node-1",
host: "node",
security: "allowlist",
ask: "on-miss",
@@ -74,6 +76,7 @@ describe("requestExecApprovalDecision", () => {
id: "approval-id-2",
command: "echo hi",
cwd: "/tmp",
nodeId: "node-1",
host: "node",
security: "allowlist",
ask: "on-miss",

4
src/agents/bash-tools.exec-approval-request.ts
View File

@@ -9,6 +9,7 @@ export type RequestExecApprovalDecisionParams = {
id: string;
command: string;
cwd: string;
nodeId?: string;
host: "gateway" | "node";
security: ExecSecurity;
ask: ExecAsk;
@@ -27,6 +28,7 @@ export async function requestExecApprovalDecision(
id: params.id,
command: params.command,
cwd: params.cwd,
nodeId: params.nodeId,
host: params.host,
security: params.security,
ask: params.ask,
@@ -48,6 +50,7 @@ export async function requestExecApprovalDecisionForHost(params: {
command: string;
workdir: string;
host: "gateway" | "node";
nodeId?: string;
security: ExecSecurity;
ask: ExecAsk;
agentId?: string;
@@ -58,6 +61,7 @@ export async function requestExecApprovalDecisionForHost(params: {
id: params.approvalId,
command: params.command,
cwd: params.workdir,
nodeId: params.nodeId,
host: params.host,
security: params.security,
ask: params.ask,

1
src/agents/bash-tools.exec-host-node.ts
View File

@@ -193,6 +193,7 @@ export async function executeNodeHostCommand(
command: params.command,
workdir: params.workdir,
host: "node",
nodeId,
security: hostSecurity,
ask: hostAsk,
agentId: params.agentId,

1
src/agents/openclaw-tools.camera.test.ts
View File

@@ -165,6 +165,7 @@ describe("nodes run", () => {
expect(params).toMatchObject({
id: expect.any(String),
command: "echo hi",
nodeId: NODE_ID,
host: "node",
timeoutMs: 120_000,
});

1
src/agents/tools/nodes-tool.ts
View File

@@ -482,6 +482,7 @@ export function createNodesTool(options?: {
id: approvalId,
command: cmdText,
cwd,
nodeId,
host: "node",
agentId,
sessionKey,

1
src/cli/nodes-cli/register.invoke.ts
View File

@@ -253,6 +253,7 @@ export function registerNodesInvokeCommands(nodes: Command) {
id: approvalId,
command: rawCommand ?? argv.join(" "),
cwd: opts.cwd,
nodeId,
host: "node",
security: hostSecurity,
ask: hostAsk,

1
src/gateway/exec-approval-manager.ts
View File

@@ -7,6 +7,7 @@ const RESOLVED_ENTRY_GRACE_MS = 15_000;
export type ExecApprovalRequestPayload = {
command: string;
cwd?: string | null;
nodeId?: string | null;
host?: string | null;
security?: string | null;
ask?: string | null;

2
src/gateway/node-invoke-sanitize.ts
View File

@@ -3,6 +3,7 @@ import { sanitizeSystemRunParamsForForwarding } from "./node-invoke-system-run-a
import type { GatewayClient } from "./server-methods/types.js";
export function sanitizeNodeInvokeParamsForForwarding(opts: {
nodeId: string;
command: string;
rawParams: unknown;
client: GatewayClient | null;
@@ -12,6 +13,7 @@ export function sanitizeNodeInvokeParamsForForwarding(opts: {
| { ok: false; message: string; details?: Record<string, unknown> } {
if (opts.command === "system.run") {
return sanitizeSystemRunParamsForForwarding({
nodeId: opts.nodeId,
rawParams: opts.rawParams,
client: opts.client,
execApprovalManager: opts.execApprovalManager,

49
src/gateway/node-invoke-system-run-approval.test.ts
View File

@@ -18,6 +18,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
id: "approval-1",
request: {
host: "node",
nodeId: "node-1",
command,
cwd: null,
agentId: null,
@@ -61,6 +62,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
approved: true,
approvalDecision: "allow-once",
},
nodeId: "node-1",
client,
execApprovalManager: manager(makeRecord("echo")),
nowMs: now,
@@ -82,6 +84,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
approved: true,
approvalDecision: "allow-once",
},
nodeId: "node-1",
client,
execApprovalManager: manager(makeRecord("echo SAFE&&whoami")),
nowMs: now,
@@ -97,6 +100,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
approved: true,
approvalDecision: "allow-once",
},
nodeId: "node-1",
client,
execApprovalManager: manager(makeRecord("echo SAFE")),
nowMs: now,
@@ -117,6 +121,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
approved: true,
approvalDecision: "allow-once",
},
nodeId: "node-1",
client,
execApprovalManager: manager(
makeRecord('/usr/bin/env BASH_ENV=/tmp/payload.sh bash -lc "echo SAFE"'),
@@ -125,4 +130,48 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
});
expectAllowOnceForwardingResult(result);
});
test("rejects approval ids that do not bind a nodeId", () => {
const record = makeRecord("echo SAFE");
record.request.nodeId = null;
const result = sanitizeSystemRunParamsForForwarding({
rawParams: {
command: ["echo", "SAFE"],
runId: "approval-1",
approved: true,
approvalDecision: "allow-once",
},
nodeId: "node-1",
client,
execApprovalManager: manager(record),
nowMs: now,
});
expect(result.ok).toBe(false);
if (result.ok) {
throw new Error("unreachable");
}
expect(result.message).toContain("missing node binding");
expect(result.details?.code).toBe("APPROVAL_NODE_BINDING_MISSING");
});
test("rejects approval ids replayed against a different nodeId", () => {
const result = sanitizeSystemRunParamsForForwarding({
rawParams: {
command: ["echo", "SAFE"],
runId: "approval-1",
approved: true,
approvalDecision: "allow-once",
},
nodeId: "node-2",
client,
execApprovalManager: manager(makeRecord("echo SAFE")),
nowMs: now,
});
expect(result.ok).toBe(false);
if (result.ok) {
throw new Error("unreachable");
}
expect(result.message).toContain("not valid for this node");
expect(result.details?.code).toBe("APPROVAL_NODE_MISMATCH");
});
});

25
src/gateway/node-invoke-system-run-approval.ts
View File

@@ -114,6 +114,7 @@ function pickSystemRunParams(raw: Record<string, unknown>): Record<string, unkno
* bypassing node-host approvals by injecting control fields into `node.invoke`.
*/
export function sanitizeSystemRunParamsForForwarding(opts: {
nodeId?: string | null;
rawParams: unknown;
client: ApprovalClient | null;
execApprovalManager?: ApprovalLookup;
@@ -188,6 +189,30 @@ export function sanitizeSystemRunParamsForForwarding(opts: {
};
}
const targetNodeId = normalizeString(opts.nodeId);
if (!targetNodeId) {
return {
ok: false,
message: "node.invoke requires nodeId",
details: { code: "MISSING_NODE_ID", runId },
};
}
const approvalNodeId = normalizeString(snapshot.request.nodeId);
if (!approvalNodeId) {
return {
ok: false,
message: "approval id missing node binding",
details: { code: "APPROVAL_NODE_BINDING_MISSING", runId },
};
}
if (approvalNodeId !== targetNodeId) {
return {
ok: false,
message: "approval id not valid for this node",
details: { code: "APPROVAL_NODE_MISMATCH", runId },
};
}
// Prefer binding by device identity (stable across reconnects / per-call clients like callGateway()).
// Fallback to connId only when device identity is not available.
const snapshotDeviceId = snapshot.requestedByDeviceId ?? null;

1
src/gateway/protocol/schema/exec-approvals.ts
View File

@@ -90,6 +90,7 @@ export const ExecApprovalRequestParamsSchema = Type.Object(
id: Type.Optional(NonEmptyString),
command: NonEmptyString,
cwd: Type.Optional(Type.Union([Type.String(), Type.Null()])),
nodeId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
host: Type.Optional(Type.Union([Type.String(), Type.Null()])),
security: Type.Optional(Type.Union([Type.String(), Type.Null()])),
ask: Type.Optional(Type.Union([Type.String(), Type.Null()])),

14
src/gateway/server-methods/exec-approval.ts
View File

@@ -44,6 +44,7 @@ export function createExecApprovalHandlers(
id?: string;
command: string;
cwd?: string;
nodeId?: string;
host?: string;
security?: string;
ask?: string;
@@ -57,6 +58,16 @@ export function createExecApprovalHandlers(
const timeoutMs =
typeof p.timeoutMs === "number" ? p.timeoutMs : DEFAULT_EXEC_APPROVAL_TIMEOUT_MS;
const explicitId = typeof p.id === "string" && p.id.trim().length > 0 ? p.id.trim() : null;
const host = typeof p.host === "string" ? p.host.trim() : "";
const nodeId = typeof p.nodeId === "string" ? p.nodeId.trim() : "";
if (host === "node" && !nodeId) {
respond(
false,
undefined,
errorShape(ErrorCodes.INVALID_REQUEST, "nodeId is required for host=node"),
);
return;
}
if (explicitId && manager.getSnapshot(explicitId)) {
respond(
false,
@@ -68,7 +79,8 @@ export function createExecApprovalHandlers(
const request = {
command: p.command,
cwd: p.cwd ?? null,
host: p.host ?? null,
nodeId: host === "node" ? nodeId : null,
host: host || null,
security: p.security ?? null,
ask: p.ask ?? null,
agentId: p.agentId ?? null,

1
src/gateway/server-methods/nodes.ts
View File

@@ -698,6 +698,7 @@ export const nodeHandlers: GatewayRequestHandlers = {
return;
}
const forwardedParams = sanitizeNodeInvokeParamsForForwarding({
nodeId,
command,
rawParams: p.params,
client,

24
src/gateway/server-methods/server-methods.test.ts
View File

@@ -248,6 +248,7 @@ describe("exec approval handlers", () => {
const defaultExecApprovalRequestParams = {
command: "echo ok",
cwd: "/tmp",
nodeId: "node-1",
host: "node",
timeoutMs: 2000,
} as const;
@@ -323,6 +324,7 @@ describe("exec approval handlers", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
nodeId: "node-1",
host: "node",
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
@@ -332,6 +334,7 @@ describe("exec approval handlers", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
nodeId: "node-1",
host: "node",
resolvedPath: "/usr/bin/echo",
};
@@ -342,6 +345,7 @@ describe("exec approval handlers", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
nodeId: "node-1",
host: "node",
resolvedPath: undefined,
};
@@ -352,6 +356,7 @@ describe("exec approval handlers", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
nodeId: "node-1",
host: "node",
resolvedPath: null,
};
@@ -359,6 +364,25 @@ describe("exec approval handlers", () => {
});
});
it("rejects host=node approval requests without nodeId", async () => {
const { handlers, respond, context } = createExecApprovalFixture();
await requestExecApproval({
handlers,
respond,
context,
params: {
nodeId: undefined,
},
});
expect(respond).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({
message: "nodeId is required for host=node",
}),
);
});
it("broadcasts request + resolve", async () => {
const { handlers, broadcasts, respond, context } = createExecApprovalFixture();

103
src/gateway/server.node-invoke-approval-bypass.test.ts
View File

@@ -3,6 +3,7 @@ import { afterAll, beforeAll, describe, expect, test } from "vitest";
import { WebSocket } from "ws";
import {
deriveDeviceIdFromPublicKey,
type DeviceIdentity,
publicKeyRawBase64UrlFromPem,
signDevicePayload,
} from "../infra/device-identity.js";
@@ -23,6 +24,22 @@ installGatewayTestHooks({ scope: "suite" });
const NODE_CONNECT_TIMEOUT_MS = 3_000;
const CONNECT_REQ_TIMEOUT_MS = 2_000;
function createDeviceIdentity(): DeviceIdentity {
const { publicKey, privateKey } = crypto.generateKeyPairSync("ed25519");
const publicKeyPem = publicKey.export({ type: "spki", format: "pem" }).toString();
const privateKeyPem = privateKey.export({ type: "pkcs8", format: "pem" }).toString();
const publicKeyRaw = publicKeyRawBase64UrlFromPem(publicKeyPem);
const deviceId = deriveDeviceIdFromPublicKey(publicKeyRaw);
if (!deviceId) {
throw new Error("failed to create test device identity");
}
return {
deviceId,
publicKeyPem,
privateKeyPem,
};
}
async function expectNoForwardedInvoke(hasInvoke: () => boolean): Promise<void> {
// Yield a couple of macrotasks so any accidental async forwarding would fire.
await new Promise<void>((resolve) => setImmediate(resolve));
@@ -42,11 +59,26 @@ async function getConnectedNodeId(ws: WebSocket): Promise<string> {
return nodeId;
}
async function requestAllowOnceApproval(ws: WebSocket, command: string): Promise<string> {
async function getConnectedNodeIds(ws: WebSocket): Promise<string[]> {
const nodes = await rpcReq<{ nodes?: Array<{ nodeId: string; connected?: boolean }> }>(
ws,
"node.list",
{},
);
expect(nodes.ok).toBe(true);
return (nodes.payload?.nodes ?? []).filter((n) => n.connected).map((n) => n.nodeId);
}
async function requestAllowOnceApproval(
ws: WebSocket,
command: string,
nodeId: string,
): Promise<string> {
const approvalId = crypto.randomUUID();
const requestP = rpcReq(ws, "exec.approval.request", {
id: approvalId,
command,
nodeId,
cwd: null,
host: "node",
timeoutMs: 30_000,
@@ -161,7 +193,10 @@ describe("node.invoke approval bypass", () => {
});
};
const connectLinuxNode = async (onInvoke: (payload: unknown) => void) => {
const connectLinuxNode = async (
onInvoke: (payload: unknown) => void,
deviceIdentity?: DeviceIdentity,
) => {
let readyResolve: (() => void) | null = null;
const ready = new Promise<void>((resolve) => {
readyResolve = resolve;
@@ -180,6 +215,7 @@ describe("node.invoke approval bypass", () => {
mode: GATEWAY_CLIENT_MODES.NODE,
scopes: [],
commands: ["system.run"],
deviceIdentity,
onHelloOk: () => readyResolve?.(),
onEvent: (evt) => {
if (evt.event !== "node.invoke.request") {
@@ -295,7 +331,7 @@ describe("node.invoke approval bypass", () => {
try {
const nodeId = await getConnectedNodeId(wsApprover);
const approvalId = await requestAllowOnceApproval(wsApprover, "echo hi");
const approvalId = await requestAllowOnceApproval(wsApprover, "echo hi", nodeId);
// Separate caller connection simulates per-call clients.
const invoke = await rpcReq(wsCaller, "node.invoke", {
nodeId,
@@ -316,7 +352,7 @@ describe("node.invoke approval bypass", () => {
expect(lastInvokeParams?.["approvalDecision"]).toBe("allow-once");
expect(lastInvokeParams?.["injected"]).toBeUndefined();
const replayApprovalId = await requestAllowOnceApproval(wsApprover, "echo hi");
const replayApprovalId = await requestAllowOnceApproval(wsApprover, "echo hi", nodeId);
const invokeCountBeforeReplay = invokeCount;
const replay = await rpcReq(wsOtherDevice, "node.invoke", {
nodeId,
@@ -340,4 +376,63 @@ describe("node.invoke approval bypass", () => {
node.stop();
}
});
test("blocks cross-node replay on same device", async () => {
const invokeCounts = new Map<string, number>();
const onInvoke = (payload: unknown) => {
const obj = payload as { nodeId?: unknown };
const nodeId = typeof obj?.nodeId === "string" ? obj.nodeId : "";
if (!nodeId) {
return;
}
invokeCounts.set(nodeId, (invokeCounts.get(nodeId) ?? 0) + 1);
};
const nodeA = await connectLinuxNode(onInvoke, createDeviceIdentity());
const nodeB = await connectLinuxNode(onInvoke, createDeviceIdentity());
const wsApprover = await connectOperator(["operator.write", "operator.approvals"]);
const wsCaller = await connectOperator(["operator.write"]);
try {
await expect
.poll(async () => (await getConnectedNodeIds(wsApprover)).length, {
timeout: 3_000,
interval: 50,
})
.toBeGreaterThanOrEqual(2);
const connectedNodeIds = await getConnectedNodeIds(wsApprover);
const approvedNodeId = connectedNodeIds[0] ?? "";
const replayNodeId = connectedNodeIds.find((id) => id !== approvedNodeId) ?? "";
expect(approvedNodeId).toBeTruthy();
expect(replayNodeId).toBeTruthy();
const approvalId = await requestAllowOnceApproval(wsApprover, "echo hi", approvedNodeId);
const beforeReplayApprovedNode = invokeCounts.get(approvedNodeId) ?? 0;
const beforeReplayOtherNode = invokeCounts.get(replayNodeId) ?? 0;
const replay = await rpcReq(wsCaller, "node.invoke", {
nodeId: replayNodeId,
command: "system.run",
params: {
command: ["echo", "hi"],
rawCommand: "echo hi",
runId: approvalId,
approved: true,
approvalDecision: "allow-once",
},
idempotencyKey: crypto.randomUUID(),
});
expect(replay.ok).toBe(false);
expect(replay.error?.message ?? "").toContain("not valid for this node");
await expectNoForwardedInvoke(
() =>
(invokeCounts.get(approvedNodeId) ?? 0) > beforeReplayApprovedNode ||
(invokeCounts.get(replayNodeId) ?? 0) > beforeReplayOtherNode,
);
} finally {
wsApprover.close();
wsCaller.close();
nodeA.stop();
nodeB.stop();
}
});
});

3
src/infra/exec-approval-forwarder.ts
View File

@@ -168,6 +168,9 @@ function buildRequestMessage(request: ExecApprovalRequest, nowMs: number) {
if (request.request.cwd) {
lines.push(`CWD: ${request.request.cwd}`);
}
if (request.request.nodeId) {
lines.push(`Node: ${request.request.nodeId}`);
}
if (request.request.host) {
lines.push(`Host: ${request.request.host}`);
}

1
src/infra/exec-approvals.ts
View File

@@ -16,6 +16,7 @@ export type ExecApprovalRequest = {
request: {
command: string;
cwd?: string | null;
nodeId?: string | null;
host?: string | null;
security?: string | null;
ask?: string | null;