mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-25 06:23:32 +00:00
fix(security): bind system.run approvals to exact argv text
This commit is contained in:
@@ -53,6 +53,7 @@ export type SystemRunApprovalPlan = {
|
||||
argv: string[];
|
||||
cwd: string | null;
|
||||
rawCommand: string | null;
|
||||
commandPreview?: string | null;
|
||||
agentId: string | null;
|
||||
sessionKey: string | null;
|
||||
mutableFileOperand?: SystemRunApprovalFileOperand | null;
|
||||
@@ -60,6 +61,7 @@ export type SystemRunApprovalPlan = {
|
||||
|
||||
export type ExecApprovalRequestPayload = {
|
||||
command: string;
|
||||
commandPreview?: string | null;
|
||||
commandArgv?: string[];
|
||||
// Optional UI-safe env key preview for approval prompts.
|
||||
envKeys?: string[];
|
||||
|
||||
@@ -54,6 +54,7 @@ export function normalizeSystemRunApprovalPlan(value: unknown): SystemRunApprova
|
||||
argv,
|
||||
cwd: normalizeNonEmptyString(candidate.cwd),
|
||||
rawCommand: normalizeNonEmptyString(candidate.rawCommand),
|
||||
commandPreview: normalizeNonEmptyString(candidate.commandPreview),
|
||||
agentId: normalizeNonEmptyString(candidate.agentId),
|
||||
sessionKey: normalizeNonEmptyString(candidate.sessionKey),
|
||||
mutableFileOperand: mutableFileOperand ?? undefined,
|
||||
|
||||
40
src/infra/system-run-approval-context.test.ts
Normal file
40
src/infra/system-run-approval-context.test.ts
Normal file
@@ -0,0 +1,40 @@
|
||||
import { describe, expect, test } from "vitest";
|
||||
import { resolveSystemRunApprovalRequestContext } from "./system-run-approval-context.js";
|
||||
|
||||
describe("resolveSystemRunApprovalRequestContext", () => {
|
||||
test("uses full approval text and separate preview for node system.run plans", () => {
|
||||
const context = resolveSystemRunApprovalRequestContext({
|
||||
host: "node",
|
||||
command: "jq --version",
|
||||
systemRunPlan: {
|
||||
argv: ["./env", "sh", "-c", "jq --version"],
|
||||
cwd: "/tmp",
|
||||
rawCommand: './env sh -c "jq --version"',
|
||||
commandPreview: "jq --version",
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
},
|
||||
});
|
||||
|
||||
expect(context.commandText).toBe('./env sh -c "jq --version"');
|
||||
expect(context.commandPreview).toBe("jq --version");
|
||||
expect(context.commandArgv).toEqual(["./env", "sh", "-c", "jq --version"]);
|
||||
});
|
||||
|
||||
test("derives preview from fallback command for older node plans", () => {
|
||||
const context = resolveSystemRunApprovalRequestContext({
|
||||
host: "node",
|
||||
command: "jq --version",
|
||||
systemRunPlan: {
|
||||
argv: ["./env", "sh", "-c", "jq --version"],
|
||||
cwd: "/tmp",
|
||||
rawCommand: './env sh -c "jq --version"',
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
},
|
||||
});
|
||||
|
||||
expect(context.commandText).toBe('./env sh -c "jq --version"');
|
||||
expect(context.commandPreview).toBe("jq --version");
|
||||
});
|
||||
});
|
||||
@@ -12,6 +12,7 @@ type SystemRunApprovalRequestContext = {
|
||||
plan: SystemRunApprovalPlan | null;
|
||||
commandArgv: string[] | undefined;
|
||||
commandText: string;
|
||||
commandPreview: string | null;
|
||||
cwd: string | null;
|
||||
agentId: string | null;
|
||||
sessionKey: string | null;
|
||||
@@ -37,6 +38,17 @@ function normalizeCommandText(value: unknown): string {
|
||||
return typeof value === "string" ? value : "";
|
||||
}
|
||||
|
||||
function normalizeCommandPreview(
|
||||
value: string | null | undefined,
|
||||
authoritative: string,
|
||||
): string | null {
|
||||
const preview = normalizeNonEmptyString(value);
|
||||
if (!preview || preview === authoritative) {
|
||||
return null;
|
||||
}
|
||||
return preview;
|
||||
}
|
||||
|
||||
export function parsePreparedSystemRunPayload(payload: unknown): PreparedRunPayload | null {
|
||||
if (!payload || typeof payload !== "object" || Array.isArray(payload)) {
|
||||
return null;
|
||||
@@ -63,10 +75,14 @@ export function resolveSystemRunApprovalRequestContext(params: {
|
||||
const plan = host === "node" ? normalizeSystemRunApprovalPlan(params.systemRunPlan) : null;
|
||||
const fallbackArgv = normalizeStringArray(params.commandArgv);
|
||||
const fallbackCommand = normalizeCommandText(params.command);
|
||||
const commandText = plan ? (plan.rawCommand ?? formatExecCommand(plan.argv)) : fallbackCommand;
|
||||
return {
|
||||
plan,
|
||||
commandArgv: plan?.argv ?? (fallbackArgv.length > 0 ? fallbackArgv : undefined),
|
||||
commandText: plan ? (plan.rawCommand ?? formatExecCommand(plan.argv)) : fallbackCommand,
|
||||
commandText,
|
||||
commandPreview: plan
|
||||
? normalizeCommandPreview(plan.commandPreview ?? fallbackCommand, commandText)
|
||||
: null,
|
||||
cwd: plan?.cwd ?? normalizeNonEmptyString(params.cwd),
|
||||
agentId: plan?.agentId ?? normalizeNonEmptyString(params.agentId),
|
||||
sessionKey: plan?.sessionKey ?? normalizeNonEmptyString(params.sessionKey),
|
||||
|
||||
@@ -106,6 +106,10 @@ describe("system run command helpers", () => {
|
||||
rawCommand: "echo hi",
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
if (!res.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
expect(res.previewText).toBe("echo hi");
|
||||
});
|
||||
|
||||
test("validateSystemRunCommandConsistency rejects shell-only rawCommand for positional-argv carrier wrappers", () => {
|
||||
@@ -121,6 +125,10 @@ describe("system run command helpers", () => {
|
||||
rawCommand: "echo hi",
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
if (!res.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
expect(res.previewText).toBe("echo hi");
|
||||
});
|
||||
|
||||
test("validateSystemRunCommandConsistency rejects shell-only rawCommand for env assignment prelude", () => {
|
||||
@@ -142,6 +150,7 @@ describe("system run command helpers", () => {
|
||||
}
|
||||
expect(res.shellCommand).toBe("echo hi");
|
||||
expect(res.cmdText).toBe(raw);
|
||||
expect(res.previewText).toBe(null);
|
||||
});
|
||||
|
||||
test("validateSystemRunCommandConsistency rejects cmd.exe /c trailing-arg smuggling", () => {
|
||||
@@ -180,6 +189,7 @@ describe("system run command helpers", () => {
|
||||
expect(res.argv).toEqual(["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"]);
|
||||
expect(res.shellCommand).toBe("echo SAFE&&whoami");
|
||||
expect(res.cmdText).toBe("echo SAFE&&whoami");
|
||||
expect(res.previewText).toBe("echo SAFE&&whoami");
|
||||
});
|
||||
|
||||
test("resolveSystemRunCommand binds cmdText to full argv for shell-wrapper positional-argv carriers", () => {
|
||||
@@ -192,6 +202,7 @@ describe("system run command helpers", () => {
|
||||
}
|
||||
expect(res.shellCommand).toBe('$0 "$1"');
|
||||
expect(res.cmdText).toBe('/bin/sh -lc "$0 \\"$1\\"" /usr/bin/touch /tmp/marker');
|
||||
expect(res.previewText).toBe(null);
|
||||
});
|
||||
|
||||
test("resolveSystemRunCommand binds cmdText to full argv when env prelude modifies shell wrapper", () => {
|
||||
@@ -204,5 +215,32 @@ describe("system run command helpers", () => {
|
||||
}
|
||||
expect(res.shellCommand).toBe("echo hi");
|
||||
expect(res.cmdText).toBe('/usr/bin/env BASH_ENV=/tmp/payload.sh bash -lc "echo hi"');
|
||||
expect(res.previewText).toBe(null);
|
||||
});
|
||||
|
||||
test("resolveSystemRunCommand keeps wrapper preview separate from approval text", () => {
|
||||
const res = resolveSystemRunCommand({
|
||||
command: ["./env", "sh", "-c", "jq --version"],
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
if (!res.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
expect(res.cmdText).toBe("jq --version");
|
||||
expect(res.previewText).toBe("jq --version");
|
||||
});
|
||||
|
||||
test("resolveSystemRunCommand accepts canonical full argv text for wrapper approvals", () => {
|
||||
const res = resolveSystemRunCommand({
|
||||
command: ["./env", "sh", "-c", "jq --version"],
|
||||
rawCommand: './env sh -c "jq --version"',
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
if (!res.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
expect(res.cmdText).toBe('./env sh -c "jq --version"');
|
||||
expect(res.previewText).toBe("jq --version");
|
||||
expect(res.shellCommand).toBe("jq --version");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,6 +16,7 @@ export type SystemRunCommandValidation =
|
||||
ok: true;
|
||||
shellCommand: string | null;
|
||||
cmdText: string;
|
||||
previewText: string | null;
|
||||
}
|
||||
| {
|
||||
ok: false;
|
||||
@@ -30,6 +31,7 @@ export type ResolvedSystemRunCommand =
|
||||
rawCommand: string | null;
|
||||
shellCommand: string | null;
|
||||
cmdText: string;
|
||||
previewText: string | null;
|
||||
}
|
||||
| {
|
||||
ok: false;
|
||||
@@ -112,35 +114,35 @@ export function validateSystemRunCommandConsistency(params: {
|
||||
const envManipulationBeforeShellWrapper =
|
||||
shellWrapperResolution.isWrapper && hasEnvManipulationBeforeShellWrapper(params.argv);
|
||||
const mustBindDisplayToFullArgv = envManipulationBeforeShellWrapper || shellWrapperPositionalArgv;
|
||||
const inferred =
|
||||
shellCommand !== null && !mustBindDisplayToFullArgv
|
||||
? shellCommand.trim()
|
||||
: formatExecCommand(params.argv);
|
||||
const formattedArgv = formatExecCommand(params.argv);
|
||||
const legacyShellText =
|
||||
shellCommand !== null && !mustBindDisplayToFullArgv ? shellCommand.trim() : null;
|
||||
const previewText = legacyShellText;
|
||||
const cmdText = raw ?? legacyShellText ?? formattedArgv;
|
||||
|
||||
if (raw && raw !== inferred) {
|
||||
return {
|
||||
ok: false,
|
||||
message: "INVALID_REQUEST: rawCommand does not match command",
|
||||
details: {
|
||||
code: "RAW_COMMAND_MISMATCH",
|
||||
rawCommand: raw,
|
||||
inferred,
|
||||
},
|
||||
};
|
||||
if (raw) {
|
||||
const matchesCanonicalArgv = raw === formattedArgv;
|
||||
const matchesLegacyShellText = legacyShellText !== null && raw === legacyShellText;
|
||||
if (!matchesCanonicalArgv && !matchesLegacyShellText) {
|
||||
return {
|
||||
ok: false,
|
||||
message: "INVALID_REQUEST: rawCommand does not match command",
|
||||
details: {
|
||||
code: "RAW_COMMAND_MISMATCH",
|
||||
rawCommand: raw,
|
||||
inferred: legacyShellText ?? formattedArgv,
|
||||
formattedArgv,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
ok: true,
|
||||
// Only treat this as a shell command when argv is a recognized shell wrapper.
|
||||
// For direct argv execution and shell wrappers with env prelude modifiers,
|
||||
// rawCommand is purely display/approval text and must match the formatted argv.
|
||||
shellCommand:
|
||||
shellCommand !== null
|
||||
? envManipulationBeforeShellWrapper
|
||||
? shellCommand
|
||||
: (raw ?? shellCommand)
|
||||
: null,
|
||||
cmdText: raw ?? inferred,
|
||||
shellCommand: shellCommand !== null ? shellCommand : null,
|
||||
cmdText,
|
||||
previewText,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -167,6 +169,7 @@ export function resolveSystemRunCommand(params: {
|
||||
rawCommand: null,
|
||||
shellCommand: null,
|
||||
cmdText: "",
|
||||
previewText: null,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -189,5 +192,6 @@ export function resolveSystemRunCommand(params: {
|
||||
rawCommand: raw,
|
||||
shellCommand: validation.shellCommand,
|
||||
cmdText: validation.cmdText,
|
||||
previewText: validation.previewText,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user