mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-03 10:50:20 +00:00
fix(node-host): enforce system.run rawCommand/argv consistency
This commit is contained in:
@@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Security/Node Host: enforce `system.run` rawCommand/argv consistency to prevent allowlist/approval bypass. Thanks @christos-eth.
|
||||
- Security/Agents: scope CLI process cleanup to owned child PIDs to avoid killing unrelated processes on shared hosts. Thanks @aether-ai-agent.
|
||||
- Security/Agents (macOS): prevent shell injection when writing Claude CLI keychain credentials. (#15924) Thanks @aether-ai-agent.
|
||||
- Security: fix Chutes manual OAuth login state validation (thanks @aether-ai-agent). (#16058)
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
import type { ExecApprovalManager, ExecApprovalRecord } from "./exec-approval-manager.js";
|
||||
import type { GatewayClient } from "./server-methods/types.js";
|
||||
import {
|
||||
formatExecCommand,
|
||||
validateSystemRunCommandConsistency,
|
||||
} from "../infra/system-run-command.js";
|
||||
|
||||
type SystemRunParamsLike = {
|
||||
command?: unknown;
|
||||
@@ -48,7 +52,7 @@ function getCmdText(params: SystemRunParamsLike): string {
|
||||
if (Array.isArray(params.command)) {
|
||||
const parts = params.command.map((v) => String(v));
|
||||
if (parts.length > 0) {
|
||||
return parts.join(" ");
|
||||
return formatExecCommand(parts);
|
||||
}
|
||||
}
|
||||
return "";
|
||||
@@ -126,6 +130,26 @@ export function sanitizeSystemRunParamsForForwarding(opts: {
|
||||
}
|
||||
|
||||
const p = obj as SystemRunParamsLike;
|
||||
const argv = Array.isArray(p.command) ? p.command.map((v) => String(v)) : [];
|
||||
const raw = normalizeString(p.rawCommand);
|
||||
if (raw) {
|
||||
if (!Array.isArray(p.command) || argv.length === 0) {
|
||||
return {
|
||||
ok: false,
|
||||
message: "rawCommand requires params.command",
|
||||
details: { code: "MISSING_COMMAND" },
|
||||
};
|
||||
}
|
||||
const validation = validateSystemRunCommandConsistency({ argv, rawCommand: raw });
|
||||
if (!validation.ok) {
|
||||
return {
|
||||
ok: false,
|
||||
message: validation.message,
|
||||
details: validation.details ?? { code: "RAW_COMMAND_MISMATCH" },
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
const approved = p.approved === true;
|
||||
const requestedDecision = normalizeApprovalDecision(p.approvalDecision);
|
||||
const wantsApprovalOverride = approved || requestedDecision !== null;
|
||||
|
||||
@@ -124,6 +124,41 @@ describe("node.invoke approval bypass", () => {
|
||||
return client;
|
||||
};
|
||||
|
||||
test("rejects rawCommand/command mismatch before forwarding to node", async () => {
|
||||
let sawInvoke = false;
|
||||
const node = await connectLinuxNode(() => {
|
||||
sawInvoke = true;
|
||||
});
|
||||
const ws = await connectOperator(["operator.write"]);
|
||||
|
||||
const nodes = await rpcReq<{ nodes?: Array<{ nodeId: string; connected?: boolean }> }>(
|
||||
ws,
|
||||
"node.list",
|
||||
{},
|
||||
);
|
||||
expect(nodes.ok).toBe(true);
|
||||
const nodeId = nodes.payload?.nodes?.find((n) => n.connected)?.nodeId ?? "";
|
||||
expect(nodeId).toBeTruthy();
|
||||
|
||||
const res = await rpcReq(ws, "node.invoke", {
|
||||
nodeId,
|
||||
command: "system.run",
|
||||
params: {
|
||||
command: ["uname", "-a"],
|
||||
rawCommand: "echo hi",
|
||||
},
|
||||
idempotencyKey: crypto.randomUUID(),
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.error?.message ?? "").toContain("rawCommand does not match command");
|
||||
|
||||
await sleep(50);
|
||||
expect(sawInvoke).toBe(false);
|
||||
|
||||
ws.close();
|
||||
node.stop();
|
||||
});
|
||||
|
||||
test("rejects injecting approved/approvalDecision without approval id", async () => {
|
||||
let sawInvoke = false;
|
||||
const node = await connectLinuxNode(() => {
|
||||
|
||||
54
src/infra/system-run-command.test.ts
Normal file
54
src/infra/system-run-command.test.ts
Normal file
@@ -0,0 +1,54 @@
|
||||
import { describe, expect, test } from "vitest";
|
||||
import {
|
||||
extractShellCommandFromArgv,
|
||||
formatExecCommand,
|
||||
validateSystemRunCommandConsistency,
|
||||
} from "./system-run-command.js";
|
||||
|
||||
describe("system run command helpers", () => {
|
||||
test("formatExecCommand quotes args with spaces", () => {
|
||||
expect(formatExecCommand(["echo", "hi there"])).toBe('echo "hi there"');
|
||||
});
|
||||
|
||||
test("extractShellCommandFromArgv extracts sh -lc command", () => {
|
||||
expect(extractShellCommandFromArgv(["/bin/sh", "-lc", "echo hi"])).toBe("echo hi");
|
||||
});
|
||||
|
||||
test("extractShellCommandFromArgv extracts cmd.exe /c command", () => {
|
||||
expect(extractShellCommandFromArgv(["cmd.exe", "/d", "/s", "/c", "echo hi"])).toBe("echo hi");
|
||||
});
|
||||
|
||||
test("validateSystemRunCommandConsistency accepts rawCommand matching direct argv", () => {
|
||||
const res = validateSystemRunCommandConsistency({
|
||||
argv: ["echo", "hi"],
|
||||
rawCommand: "echo hi",
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
if (!res.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
expect(res.shellCommand).toBe(null);
|
||||
expect(res.cmdText).toBe("echo hi");
|
||||
});
|
||||
|
||||
test("validateSystemRunCommandConsistency rejects mismatched rawCommand vs direct argv", () => {
|
||||
const res = validateSystemRunCommandConsistency({
|
||||
argv: ["uname", "-a"],
|
||||
rawCommand: "echo hi",
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
if (res.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
expect(res.message).toContain("rawCommand does not match command");
|
||||
expect(res.details?.code).toBe("RAW_COMMAND_MISMATCH");
|
||||
});
|
||||
|
||||
test("validateSystemRunCommandConsistency accepts rawCommand matching sh wrapper argv", () => {
|
||||
const res = validateSystemRunCommandConsistency({
|
||||
argv: ["/bin/sh", "-lc", "echo hi"],
|
||||
rawCommand: "echo hi",
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
});
|
||||
});
|
||||
106
src/infra/system-run-command.ts
Normal file
106
src/infra/system-run-command.ts
Normal file
@@ -0,0 +1,106 @@
|
||||
import path from "node:path";
|
||||
|
||||
export type SystemRunCommandValidation =
|
||||
| {
|
||||
ok: true;
|
||||
shellCommand: string | null;
|
||||
cmdText: string;
|
||||
}
|
||||
| {
|
||||
ok: false;
|
||||
message: string;
|
||||
details?: Record<string, unknown>;
|
||||
};
|
||||
|
||||
function basenameLower(token: string): string {
|
||||
const win = path.win32.basename(token);
|
||||
const posix = path.posix.basename(token);
|
||||
const base = win.length < posix.length ? win : posix;
|
||||
return base.trim().toLowerCase();
|
||||
}
|
||||
|
||||
export function formatExecCommand(argv: string[]): string {
|
||||
return argv
|
||||
.map((arg) => {
|
||||
const trimmed = arg.trim();
|
||||
if (!trimmed) {
|
||||
return '""';
|
||||
}
|
||||
const needsQuotes = /\s|"/.test(trimmed);
|
||||
if (!needsQuotes) {
|
||||
return trimmed;
|
||||
}
|
||||
return `"${trimmed.replace(/"/g, '\\"')}"`;
|
||||
})
|
||||
.join(" ");
|
||||
}
|
||||
|
||||
export function extractShellCommandFromArgv(argv: string[]): string | null {
|
||||
const token0 = argv[0]?.trim();
|
||||
if (!token0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const base0 = basenameLower(token0);
|
||||
|
||||
// POSIX-style shells: sh -lc "<cmd>"
|
||||
if (
|
||||
base0 === "sh" ||
|
||||
base0 === "bash" ||
|
||||
base0 === "zsh" ||
|
||||
base0 === "dash" ||
|
||||
base0 === "ksh"
|
||||
) {
|
||||
const flag = argv[1]?.trim();
|
||||
if (flag !== "-lc" && flag !== "-c") {
|
||||
return null;
|
||||
}
|
||||
const cmd = argv[2];
|
||||
return typeof cmd === "string" ? cmd : null;
|
||||
}
|
||||
|
||||
// Windows cmd.exe: cmd.exe /d /s /c "<cmd>"
|
||||
if (base0 === "cmd.exe" || base0 === "cmd") {
|
||||
const idx = argv.findIndex((item) => String(item).trim().toLowerCase() === "/c");
|
||||
if (idx === -1) {
|
||||
return null;
|
||||
}
|
||||
const cmd = argv[idx + 1];
|
||||
return typeof cmd === "string" ? cmd : null;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
export function validateSystemRunCommandConsistency(params: {
|
||||
argv: string[];
|
||||
rawCommand?: string | null;
|
||||
}): SystemRunCommandValidation {
|
||||
const raw =
|
||||
typeof params.rawCommand === "string" && params.rawCommand.trim().length > 0
|
||||
? params.rawCommand.trim()
|
||||
: null;
|
||||
const shellCommand = extractShellCommandFromArgv(params.argv);
|
||||
const inferred = shellCommand ? shellCommand.trim() : formatExecCommand(params.argv);
|
||||
|
||||
if (raw && raw !== inferred) {
|
||||
return {
|
||||
ok: false,
|
||||
message: "INVALID_REQUEST: rawCommand does not match command",
|
||||
details: {
|
||||
code: "RAW_COMMAND_MISMATCH",
|
||||
rawCommand: raw,
|
||||
inferred,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
ok: true,
|
||||
// Only treat this as a shell command when argv is a recognized shell wrapper.
|
||||
// For direct argv execution, rawCommand is purely display/approval text and
|
||||
// must match the formatted argv.
|
||||
shellCommand: shellCommand ? (raw ?? shellCommand) : null,
|
||||
cmdText: raw ?? shellCommand ?? inferred,
|
||||
};
|
||||
}
|
||||
@@ -31,6 +31,7 @@ import {
|
||||
type ExecHostResponse,
|
||||
type ExecHostRunResult,
|
||||
} from "../infra/exec-host.js";
|
||||
import { validateSystemRunCommandConsistency } from "../infra/system-run-command.js";
|
||||
import { runBrowserProxyCommand } from "./invoke-browser.js";
|
||||
|
||||
const OUTPUT_CAP = 200_000;
|
||||
@@ -174,22 +175,6 @@ function sanitizeEnv(
|
||||
return merged;
|
||||
}
|
||||
|
||||
function formatCommand(argv: string[]): string {
|
||||
return argv
|
||||
.map((arg) => {
|
||||
const trimmed = arg.trim();
|
||||
if (!trimmed) {
|
||||
return '""';
|
||||
}
|
||||
const needsQuotes = /\s|"/.test(trimmed);
|
||||
if (!needsQuotes) {
|
||||
return trimmed;
|
||||
}
|
||||
return `"${trimmed.replace(/"/g, '\\"')}"`;
|
||||
})
|
||||
.join(" ");
|
||||
}
|
||||
|
||||
function truncateOutput(raw: string, maxChars: number): { text: string; truncated: boolean } {
|
||||
if (raw.length <= maxChars) {
|
||||
return { text: raw, truncated: false };
|
||||
@@ -514,7 +499,20 @@ export async function handleInvoke(
|
||||
|
||||
const argv = params.command.map((item) => String(item));
|
||||
const rawCommand = typeof params.rawCommand === "string" ? params.rawCommand.trim() : "";
|
||||
const cmdText = rawCommand || formatCommand(argv);
|
||||
const consistency = validateSystemRunCommandConsistency({
|
||||
argv,
|
||||
rawCommand: rawCommand || null,
|
||||
});
|
||||
if (!consistency.ok) {
|
||||
await sendInvokeResult(client, frame, {
|
||||
ok: false,
|
||||
error: { code: "INVALID_REQUEST", message: consistency.message },
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const shellCommand = consistency.shellCommand;
|
||||
const cmdText = consistency.cmdText;
|
||||
const agentId = params.agentId?.trim() || undefined;
|
||||
const cfg = loadConfig();
|
||||
const agentExec = agentId ? resolveAgentConfig(cfg, agentId)?.tools?.exec : undefined;
|
||||
@@ -536,9 +534,9 @@ export async function handleInvoke(
|
||||
let allowlistMatches: ExecAllowlistEntry[] = [];
|
||||
let allowlistSatisfied = false;
|
||||
let segments: ExecCommandSegment[] = [];
|
||||
if (rawCommand) {
|
||||
if (shellCommand) {
|
||||
const allowlistEval = evaluateShellAllowlist({
|
||||
command: rawCommand,
|
||||
command: shellCommand,
|
||||
allowlist: approvals.allowlist,
|
||||
safeBins,
|
||||
cwd: params.cwd ?? undefined,
|
||||
@@ -569,7 +567,7 @@ export async function handleInvoke(
|
||||
segments = analysis.segments;
|
||||
}
|
||||
const isWindows = process.platform === "win32";
|
||||
const cmdInvocation = rawCommand
|
||||
const cmdInvocation = shellCommand
|
||||
? isCmdExeInvocation(segments[0]?.argv ?? [])
|
||||
: isCmdExeInvocation(argv);
|
||||
if (security === "allowlist" && isWindows && cmdInvocation) {
|
||||
@@ -585,7 +583,7 @@ export async function handleInvoke(
|
||||
: null;
|
||||
const execRequest: ExecHostRequest = {
|
||||
command: argv,
|
||||
rawCommand: rawCommand || null,
|
||||
rawCommand: rawCommand || shellCommand || null,
|
||||
cwd: params.cwd ?? null,
|
||||
env: params.env ?? null,
|
||||
timeoutMs: params.timeoutMs ?? null,
|
||||
@@ -780,7 +778,7 @@ export async function handleInvoke(
|
||||
security === "allowlist" &&
|
||||
isWindows &&
|
||||
!approvedByAsk &&
|
||||
rawCommand &&
|
||||
shellCommand &&
|
||||
analysisOk &&
|
||||
allowlistSatisfied &&
|
||||
segments.length === 1 &&
|
||||
|
||||
Reference in New Issue
Block a user