mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-10 18:44:57 +00:00
refactor!: remove versioned system-run approval contract
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import type { ExecAsk, ExecSecurity } from "../infra/exec-approvals.js";
|
||||
import type { ExecAsk, ExecSecurity, SystemRunApprovalPlan } from "../infra/exec-approvals.js";
|
||||
import {
|
||||
DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS,
|
||||
DEFAULT_APPROVAL_TIMEOUT_MS,
|
||||
@@ -9,6 +9,7 @@ export type RequestExecApprovalDecisionParams = {
|
||||
id: string;
|
||||
command: string;
|
||||
commandArgv?: string[];
|
||||
systemRunPlan?: SystemRunApprovalPlan;
|
||||
env?: Record<string, string>;
|
||||
cwd: string;
|
||||
nodeId?: string;
|
||||
@@ -28,6 +29,7 @@ type ExecApprovalRequestToolParams = {
|
||||
id: string;
|
||||
command: string;
|
||||
commandArgv?: string[];
|
||||
systemRunPlan?: SystemRunApprovalPlan;
|
||||
env?: Record<string, string>;
|
||||
cwd: string;
|
||||
nodeId?: string;
|
||||
@@ -52,6 +54,7 @@ function buildExecApprovalRequestToolParams(
|
||||
id: params.id,
|
||||
command: params.command,
|
||||
commandArgv: params.commandArgv,
|
||||
systemRunPlan: params.systemRunPlan,
|
||||
env: params.env,
|
||||
cwd: params.cwd,
|
||||
nodeId: params.nodeId,
|
||||
@@ -156,6 +159,7 @@ export async function requestExecApprovalDecisionForHost(params: {
|
||||
approvalId: string;
|
||||
command: string;
|
||||
commandArgv?: string[];
|
||||
systemRunPlan?: SystemRunApprovalPlan;
|
||||
env?: Record<string, string>;
|
||||
workdir: string;
|
||||
host: "gateway" | "node";
|
||||
@@ -174,6 +178,7 @@ export async function requestExecApprovalDecisionForHost(params: {
|
||||
id: params.approvalId,
|
||||
command: params.command,
|
||||
commandArgv: params.commandArgv,
|
||||
systemRunPlan: params.systemRunPlan,
|
||||
env: params.env,
|
||||
cwd: params.workdir,
|
||||
nodeId: params.nodeId,
|
||||
@@ -194,6 +199,7 @@ export async function registerExecApprovalRequestForHost(params: {
|
||||
approvalId: string;
|
||||
command: string;
|
||||
commandArgv?: string[];
|
||||
systemRunPlan?: SystemRunApprovalPlan;
|
||||
env?: Record<string, string>;
|
||||
workdir: string;
|
||||
host: "gateway" | "node";
|
||||
@@ -212,6 +218,7 @@ export async function registerExecApprovalRequestForHost(params: {
|
||||
id: params.approvalId,
|
||||
command: params.command,
|
||||
commandArgv: params.commandArgv,
|
||||
systemRunPlan: params.systemRunPlan,
|
||||
env: params.env,
|
||||
cwd: params.workdir,
|
||||
nodeId: params.nodeId,
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
} from "../infra/exec-approvals.js";
|
||||
import { detectCommandObfuscation } from "../infra/exec-obfuscation-detect.js";
|
||||
import { buildNodeShellCommand } from "../infra/node-shell.js";
|
||||
import { parsePreparedSystemRunPayload } from "../infra/system-run-approval-context.js";
|
||||
import { logInfo } from "../logger.js";
|
||||
import {
|
||||
registerExecApprovalRequestForHost,
|
||||
@@ -95,6 +96,31 @@ export async function executeNodeHostCommand(
|
||||
);
|
||||
}
|
||||
const argv = buildNodeShellCommand(params.command, nodeInfo?.platform);
|
||||
const prepareRaw = await callGatewayTool<{ payload?: unknown }>(
|
||||
"node.invoke",
|
||||
{ timeoutMs: 15_000 },
|
||||
{
|
||||
nodeId,
|
||||
command: "system.run.prepare",
|
||||
params: {
|
||||
command: argv,
|
||||
rawCommand: params.command,
|
||||
cwd: params.workdir,
|
||||
agentId: params.agentId,
|
||||
sessionKey: params.sessionKey,
|
||||
},
|
||||
idempotencyKey: crypto.randomUUID(),
|
||||
},
|
||||
);
|
||||
const prepared = parsePreparedSystemRunPayload(prepareRaw?.payload);
|
||||
if (!prepared) {
|
||||
throw new Error("invalid system.run.prepare response");
|
||||
}
|
||||
const runArgv = prepared.plan.argv;
|
||||
const runRawCommand = prepared.plan.rawCommand ?? prepared.cmdText;
|
||||
const runCwd = prepared.plan.cwd ?? params.workdir;
|
||||
const runAgentId = prepared.plan.agentId ?? params.agentId;
|
||||
const runSessionKey = prepared.plan.sessionKey ?? params.sessionKey;
|
||||
|
||||
const nodeEnv = params.requestedEnv ? { ...params.requestedEnv } : undefined;
|
||||
const baseAllowlistEval = evaluateShellAllowlist({
|
||||
@@ -170,13 +196,13 @@ export async function executeNodeHostCommand(
|
||||
nodeId,
|
||||
command: "system.run",
|
||||
params: {
|
||||
command: argv,
|
||||
rawCommand: params.command,
|
||||
cwd: params.workdir,
|
||||
command: runArgv,
|
||||
rawCommand: runRawCommand,
|
||||
cwd: runCwd,
|
||||
env: nodeEnv,
|
||||
timeoutMs: typeof params.timeoutSec === "number" ? params.timeoutSec * 1000 : undefined,
|
||||
agentId: params.agentId,
|
||||
sessionKey: params.sessionKey,
|
||||
agentId: runAgentId,
|
||||
sessionKey: runSessionKey,
|
||||
approved: approvedByAsk,
|
||||
approvalDecision: approvalDecision ?? undefined,
|
||||
runId: runId ?? undefined,
|
||||
@@ -197,16 +223,17 @@ export async function executeNodeHostCommand(
|
||||
// Register first so the returned approval ID is actionable immediately.
|
||||
const registration = await registerExecApprovalRequestForHost({
|
||||
approvalId,
|
||||
command: params.command,
|
||||
commandArgv: argv,
|
||||
command: prepared.cmdText,
|
||||
commandArgv: prepared.plan.argv,
|
||||
systemRunPlan: prepared.plan,
|
||||
env: nodeEnv,
|
||||
workdir: params.workdir,
|
||||
workdir: runCwd,
|
||||
host: "node",
|
||||
nodeId,
|
||||
security: hostSecurity,
|
||||
ask: hostAsk,
|
||||
agentId: params.agentId,
|
||||
sessionKey: params.sessionKey,
|
||||
agentId: runAgentId,
|
||||
sessionKey: runSessionKey,
|
||||
turnSourceChannel: params.turnSourceChannel,
|
||||
turnSourceTo: params.turnSourceTo,
|
||||
turnSourceAccountId: params.turnSourceAccountId,
|
||||
|
||||
@@ -27,6 +27,33 @@ let callGatewayTool: typeof import("./tools/gateway.js").callGatewayTool;
|
||||
let createExecTool: typeof import("./bash-tools.exec.js").createExecTool;
|
||||
let detectCommandObfuscation: typeof import("../infra/exec-obfuscation-detect.js").detectCommandObfuscation;
|
||||
|
||||
function buildPreparedSystemRunPayload(rawInvokeParams: unknown) {
|
||||
const invoke = (rawInvokeParams ?? {}) as {
|
||||
params?: {
|
||||
command?: unknown;
|
||||
rawCommand?: unknown;
|
||||
cwd?: unknown;
|
||||
agentId?: unknown;
|
||||
sessionKey?: unknown;
|
||||
};
|
||||
};
|
||||
const params = invoke.params ?? {};
|
||||
const argv = Array.isArray(params.command) ? params.command.map(String) : [];
|
||||
const rawCommand = typeof params.rawCommand === "string" ? params.rawCommand : null;
|
||||
return {
|
||||
payload: {
|
||||
cmdText: rawCommand ?? argv.join(" "),
|
||||
plan: {
|
||||
argv,
|
||||
cwd: typeof params.cwd === "string" ? params.cwd : null,
|
||||
rawCommand,
|
||||
agentId: typeof params.agentId === "string" ? params.agentId : null,
|
||||
sessionKey: typeof params.sessionKey === "string" ? params.sessionKey : null,
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("exec approvals", () => {
|
||||
let previousHome: string | undefined;
|
||||
let previousUserProfile: string | undefined;
|
||||
@@ -71,8 +98,14 @@ describe("exec approvals", () => {
|
||||
return { decision: "allow-once" };
|
||||
}
|
||||
if (method === "node.invoke") {
|
||||
invokeParams = params;
|
||||
return { ok: true };
|
||||
const invoke = params as { command?: string };
|
||||
if (invoke.command === "system.run.prepare") {
|
||||
return buildPreparedSystemRunPayload(params);
|
||||
}
|
||||
if (invoke.command === "system.run") {
|
||||
invokeParams = params;
|
||||
return { payload: { success: true, stdout: "ok" } };
|
||||
}
|
||||
}
|
||||
return { ok: true };
|
||||
});
|
||||
@@ -116,12 +149,16 @@ describe("exec approvals", () => {
|
||||
};
|
||||
|
||||
const calls: string[] = [];
|
||||
vi.mocked(callGatewayTool).mockImplementation(async (method) => {
|
||||
vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => {
|
||||
calls.push(method);
|
||||
if (method === "exec.approvals.node.get") {
|
||||
return { file: approvalsFile };
|
||||
}
|
||||
if (method === "node.invoke") {
|
||||
const invoke = params as { command?: string };
|
||||
if (invoke.command === "system.run.prepare") {
|
||||
return buildPreparedSystemRunPayload(params);
|
||||
}
|
||||
return { payload: { success: true, stdout: "ok" } };
|
||||
}
|
||||
// exec.approval.request should NOT be called when allowlist is satisfied
|
||||
@@ -266,7 +303,8 @@ describe("exec approvals", () => {
|
||||
});
|
||||
|
||||
const calls: string[] = [];
|
||||
vi.mocked(callGatewayTool).mockImplementation(async (method) => {
|
||||
const nodeInvokeCommands: string[] = [];
|
||||
vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => {
|
||||
calls.push(method);
|
||||
if (method === "exec.approval.request") {
|
||||
return { status: "accepted", id: "approval-id" };
|
||||
@@ -275,6 +313,13 @@ describe("exec approvals", () => {
|
||||
return {};
|
||||
}
|
||||
if (method === "node.invoke") {
|
||||
const invoke = params as { command?: string };
|
||||
if (invoke.command) {
|
||||
nodeInvokeCommands.push(invoke.command);
|
||||
}
|
||||
if (invoke.command === "system.run.prepare") {
|
||||
return buildPreparedSystemRunPayload(params);
|
||||
}
|
||||
return { payload: { success: true, stdout: "should-not-run" } };
|
||||
}
|
||||
return { ok: true };
|
||||
@@ -289,7 +334,7 @@ describe("exec approvals", () => {
|
||||
|
||||
const result = await tool.execute("call5", { command: "echo hi | sh" });
|
||||
expect(result.details.status).toBe("approval-pending");
|
||||
await expect.poll(() => calls.filter((call) => call === "node.invoke").length).toBe(0);
|
||||
await expect.poll(() => nodeInvokeCommands.includes("system.run")).toBe(false);
|
||||
});
|
||||
|
||||
it("denies gateway obfuscated command when approval request times out", async () => {
|
||||
|
||||
@@ -356,6 +356,21 @@ describe("nodes run", () => {
|
||||
return mockNodeList(["system.run"]);
|
||||
}
|
||||
if (method === "node.invoke") {
|
||||
const command = (params as { command?: string } | undefined)?.command;
|
||||
if (command === "system.run.prepare") {
|
||||
return {
|
||||
payload: {
|
||||
cmdText: "echo hi",
|
||||
plan: {
|
||||
argv: ["echo", "hi"],
|
||||
cwd: "/tmp",
|
||||
rawCommand: "echo hi",
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
expect(params).toMatchObject({
|
||||
nodeId: NODE_ID,
|
||||
command: "system.run",
|
||||
@@ -391,6 +406,21 @@ describe("nodes run", () => {
|
||||
return mockNodeList(["system.run"]);
|
||||
}
|
||||
if (method === "node.invoke") {
|
||||
const command = (params as { command?: string } | undefined)?.command;
|
||||
if (command === "system.run.prepare") {
|
||||
return {
|
||||
payload: {
|
||||
cmdText: "echo hi",
|
||||
plan: {
|
||||
argv: ["echo", "hi"],
|
||||
cwd: null,
|
||||
rawCommand: "echo hi",
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
invokeCalls += 1;
|
||||
if (invokeCalls === 1) {
|
||||
throw new Error("SYSTEM_RUN_DENIED: approval required");
|
||||
@@ -411,6 +441,10 @@ describe("nodes run", () => {
|
||||
expect(params).toMatchObject({
|
||||
id: expect.any(String),
|
||||
command: "echo hi",
|
||||
commandArgv: ["echo", "hi"],
|
||||
systemRunPlan: expect.objectContaining({
|
||||
argv: ["echo", "hi"],
|
||||
}),
|
||||
nodeId: NODE_ID,
|
||||
host: "node",
|
||||
timeoutMs: 120_000,
|
||||
@@ -429,11 +463,26 @@ describe("nodes run", () => {
|
||||
});
|
||||
|
||||
it("fails with user denied when approval decision is deny", async () => {
|
||||
callGateway.mockImplementation(async ({ method }) => {
|
||||
callGateway.mockImplementation(async ({ method, params }) => {
|
||||
if (method === "node.list") {
|
||||
return mockNodeList(["system.run"]);
|
||||
}
|
||||
if (method === "node.invoke") {
|
||||
const command = (params as { command?: string } | undefined)?.command;
|
||||
if (command === "system.run.prepare") {
|
||||
return {
|
||||
payload: {
|
||||
cmdText: "echo hi",
|
||||
plan: {
|
||||
argv: ["echo", "hi"],
|
||||
cwd: null,
|
||||
rawCommand: "echo hi",
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
throw new Error("SYSTEM_RUN_DENIED: approval required");
|
||||
}
|
||||
if (method === "exec.approval.request") {
|
||||
@@ -446,11 +495,26 @@ describe("nodes run", () => {
|
||||
});
|
||||
|
||||
it("fails closed for timeout and invalid approval decisions", async () => {
|
||||
callGateway.mockImplementation(async ({ method }) => {
|
||||
callGateway.mockImplementation(async ({ method, params }) => {
|
||||
if (method === "node.list") {
|
||||
return mockNodeList(["system.run"]);
|
||||
}
|
||||
if (method === "node.invoke") {
|
||||
const command = (params as { command?: string } | undefined)?.command;
|
||||
if (command === "system.run.prepare") {
|
||||
return {
|
||||
payload: {
|
||||
cmdText: "echo hi",
|
||||
plan: {
|
||||
argv: ["echo", "hi"],
|
||||
cwd: null,
|
||||
rawCommand: "echo hi",
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
throw new Error("SYSTEM_RUN_DENIED: approval required");
|
||||
}
|
||||
if (method === "exec.approval.request") {
|
||||
@@ -460,11 +524,26 @@ describe("nodes run", () => {
|
||||
});
|
||||
await expect(executeNodes(BASE_RUN_INPUT)).rejects.toThrow("exec denied: approval timed out");
|
||||
|
||||
callGateway.mockImplementation(async ({ method }) => {
|
||||
callGateway.mockImplementation(async ({ method, params }) => {
|
||||
if (method === "node.list") {
|
||||
return mockNodeList(["system.run"]);
|
||||
}
|
||||
if (method === "node.invoke") {
|
||||
const command = (params as { command?: string } | undefined)?.command;
|
||||
if (command === "system.run.prepare") {
|
||||
return {
|
||||
payload: {
|
||||
cmdText: "echo hi",
|
||||
plan: {
|
||||
argv: ["echo", "hi"],
|
||||
cwd: null,
|
||||
rawCommand: "echo hi",
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
throw new Error("SYSTEM_RUN_DENIED: approval required");
|
||||
}
|
||||
if (method === "exec.approval.request") {
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
} from "../../cli/nodes-screen.js";
|
||||
import { parseDurationMs } from "../../cli/parse-duration.js";
|
||||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
import { parsePreparedSystemRunPayload } from "../../infra/system-run-approval-context.js";
|
||||
import { formatExecCommand } from "../../infra/system-run-command.js";
|
||||
import { imageMimeFromFormat } from "../../media/mime.js";
|
||||
import type { GatewayMessageChannel } from "../../utils/message-channel.js";
|
||||
@@ -530,14 +531,36 @@ export function createNodesTool(options?: {
|
||||
typeof params.needsScreenRecording === "boolean"
|
||||
? params.needsScreenRecording
|
||||
: undefined;
|
||||
const prepareRaw = await callGatewayTool<{ payload?: unknown }>(
|
||||
"node.invoke",
|
||||
gatewayOpts,
|
||||
{
|
||||
nodeId,
|
||||
command: "system.run.prepare",
|
||||
params: {
|
||||
command,
|
||||
rawCommand: formatExecCommand(command),
|
||||
cwd,
|
||||
agentId,
|
||||
sessionKey,
|
||||
},
|
||||
timeoutMs: invokeTimeoutMs,
|
||||
idempotencyKey: crypto.randomUUID(),
|
||||
},
|
||||
);
|
||||
const prepared = parsePreparedSystemRunPayload(prepareRaw?.payload);
|
||||
if (!prepared) {
|
||||
throw new Error("invalid system.run.prepare response");
|
||||
}
|
||||
const runParams = {
|
||||
command,
|
||||
cwd,
|
||||
command: prepared.plan.argv,
|
||||
rawCommand: prepared.plan.rawCommand ?? prepared.cmdText,
|
||||
cwd: prepared.plan.cwd ?? cwd,
|
||||
env,
|
||||
timeoutMs: commandTimeoutMs,
|
||||
needsScreenRecording,
|
||||
agentId,
|
||||
sessionKey,
|
||||
agentId: prepared.plan.agentId ?? agentId,
|
||||
sessionKey: prepared.plan.sessionKey ?? sessionKey,
|
||||
};
|
||||
|
||||
// First attempt without approval flags.
|
||||
@@ -560,20 +583,20 @@ export function createNodesTool(options?: {
|
||||
// Node requires approval – create a pending approval request on
|
||||
// the gateway and wait for the user to approve/deny via the UI.
|
||||
const APPROVAL_TIMEOUT_MS = 120_000;
|
||||
const cmdText = formatExecCommand(command);
|
||||
const approvalId = crypto.randomUUID();
|
||||
const approvalResult = await callGatewayTool(
|
||||
"exec.approval.request",
|
||||
{ ...gatewayOpts, timeoutMs: APPROVAL_TIMEOUT_MS + 5_000 },
|
||||
{
|
||||
id: approvalId,
|
||||
command: cmdText,
|
||||
commandArgv: command,
|
||||
cwd,
|
||||
command: prepared.cmdText,
|
||||
commandArgv: prepared.plan.argv,
|
||||
systemRunPlan: prepared.plan,
|
||||
cwd: prepared.plan.cwd ?? cwd,
|
||||
nodeId,
|
||||
host: "node",
|
||||
agentId,
|
||||
sessionKey,
|
||||
agentId: prepared.plan.agentId ?? agentId,
|
||||
sessionKey: prepared.plan.sessionKey ?? sessionKey,
|
||||
turnSourceChannel,
|
||||
turnSourceTo,
|
||||
turnSourceAccountId,
|
||||
|
||||
Reference in New Issue
Block a user