mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-11 20:13:43 +00:00
fix(exec): bind env-prefixed shell wrappers to full approval text
(cherry picked from commit 1edf957988)
This commit is contained in:
committed by
Peter Steinberger
parent
216d99e585
commit
bd8b9af9a7
@@ -82,4 +82,47 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
|
|||||||
expect(params.approved).toBe(true);
|
expect(params.approved).toBe(true);
|
||||||
expect(params.approvalDecision).toBe("allow-once");
|
expect(params.approvalDecision).toBe("allow-once");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("rejects env-assignment shell wrapper when approval command omits env prelude", () => {
|
||||||
|
const result = sanitizeSystemRunParamsForForwarding({
|
||||||
|
rawParams: {
|
||||||
|
command: ["/usr/bin/env", "BASH_ENV=/tmp/payload.sh", "bash", "-lc", "echo SAFE"],
|
||||||
|
runId: "approval-1",
|
||||||
|
approved: true,
|
||||||
|
approvalDecision: "allow-once",
|
||||||
|
},
|
||||||
|
client,
|
||||||
|
execApprovalManager: manager(makeRecord("echo SAFE")),
|
||||||
|
nowMs: now,
|
||||||
|
});
|
||||||
|
expect(result.ok).toBe(false);
|
||||||
|
if (result.ok) {
|
||||||
|
throw new Error("unreachable");
|
||||||
|
}
|
||||||
|
expect(result.message).toContain("approval id does not match request");
|
||||||
|
expect(result.details?.code).toBe("APPROVAL_REQUEST_MISMATCH");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("accepts env-assignment shell wrapper only when approval command matches full argv text", () => {
|
||||||
|
const result = sanitizeSystemRunParamsForForwarding({
|
||||||
|
rawParams: {
|
||||||
|
command: ["/usr/bin/env", "BASH_ENV=/tmp/payload.sh", "bash", "-lc", "echo SAFE"],
|
||||||
|
runId: "approval-1",
|
||||||
|
approved: true,
|
||||||
|
approvalDecision: "allow-once",
|
||||||
|
},
|
||||||
|
client,
|
||||||
|
execApprovalManager: manager(
|
||||||
|
makeRecord('/usr/bin/env BASH_ENV=/tmp/payload.sh bash -lc "echo SAFE"'),
|
||||||
|
),
|
||||||
|
nowMs: now,
|
||||||
|
});
|
||||||
|
expect(result.ok).toBe(true);
|
||||||
|
if (!result.ok) {
|
||||||
|
throw new Error("unreachable");
|
||||||
|
}
|
||||||
|
const params = result.params as Record<string, unknown>;
|
||||||
|
expect(params.approved).toBe(true);
|
||||||
|
expect(params.approvalDecision).toBe("allow-once");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -209,6 +209,61 @@ export function unwrapEnvInvocation(argv: string[]): string[] | null {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function envInvocationUsesModifiers(argv: string[]): boolean {
|
||||||
|
let idx = 1;
|
||||||
|
let expectsOptionValue = false;
|
||||||
|
while (idx < argv.length) {
|
||||||
|
const token = argv[idx]?.trim() ?? "";
|
||||||
|
if (!token) {
|
||||||
|
idx += 1;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (expectsOptionValue) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (token === "--" || token === "-") {
|
||||||
|
idx += 1;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if (isEnvAssignment(token)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (!token.startsWith("-") || token === "-") {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
const lower = token.toLowerCase();
|
||||||
|
const [flag] = lower.split("=", 2);
|
||||||
|
if (ENV_FLAG_OPTIONS.has(flag)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (ENV_OPTIONS_WITH_VALUE.has(flag)) {
|
||||||
|
if (lower.includes("=") || lower !== flag) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
expectsOptionValue = true;
|
||||||
|
idx += 1;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (
|
||||||
|
lower.startsWith("-u") ||
|
||||||
|
lower.startsWith("-c") ||
|
||||||
|
lower.startsWith("-s") ||
|
||||||
|
lower.startsWith("--unset=") ||
|
||||||
|
lower.startsWith("--chdir=") ||
|
||||||
|
lower.startsWith("--split-string=") ||
|
||||||
|
lower.startsWith("--default-signal=") ||
|
||||||
|
lower.startsWith("--ignore-signal=") ||
|
||||||
|
lower.startsWith("--block-signal=")
|
||||||
|
) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
// Unknown env flags are treated conservatively as modifiers.
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
function unwrapNiceInvocation(argv: string[]): string[] | null {
|
function unwrapNiceInvocation(argv: string[]): string[] | null {
|
||||||
return scanWrapperInvocation(argv, {
|
return scanWrapperInvocation(argv, {
|
||||||
separators: new Set(["--"]),
|
separators: new Set(["--"]),
|
||||||
@@ -345,6 +400,49 @@ export function unwrapDispatchWrappersForResolution(
|
|||||||
return current;
|
return current;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function hasEnvManipulationBeforeShellWrapperInternal(
|
||||||
|
argv: string[],
|
||||||
|
depth: number,
|
||||||
|
envManipulationSeen: boolean,
|
||||||
|
): boolean {
|
||||||
|
if (depth >= MAX_DISPATCH_WRAPPER_DEPTH) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
const token0 = argv[0]?.trim();
|
||||||
|
if (!token0) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(argv);
|
||||||
|
if (dispatchUnwrap.kind === "blocked") {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (dispatchUnwrap.kind === "unwrapped") {
|
||||||
|
const nextEnvManipulationSeen =
|
||||||
|
envManipulationSeen || (dispatchUnwrap.wrapper === "env" && envInvocationUsesModifiers(argv));
|
||||||
|
return hasEnvManipulationBeforeShellWrapperInternal(
|
||||||
|
dispatchUnwrap.argv,
|
||||||
|
depth + 1,
|
||||||
|
nextEnvManipulationSeen,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
const wrapper = findShellWrapperSpec(normalizeExecutableToken(token0));
|
||||||
|
if (!wrapper) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const payload = extractShellWrapperPayload(argv, wrapper);
|
||||||
|
if (!payload) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return envManipulationSeen;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function hasEnvManipulationBeforeShellWrapper(argv: string[]): boolean {
|
||||||
|
return hasEnvManipulationBeforeShellWrapperInternal(argv, 0, false);
|
||||||
|
}
|
||||||
|
|
||||||
function extractPosixShellInlineCommand(argv: string[]): string | null {
|
function extractPosixShellInlineCommand(argv: string[]): string | null {
|
||||||
return extractInlineCommandByFlags(argv, POSIX_INLINE_COMMAND_FLAGS, { allowCombinedC: true });
|
return extractInlineCommandByFlags(argv, POSIX_INLINE_COMMAND_FLAGS, { allowCombinedC: true });
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -106,6 +106,27 @@ describe("system run command helpers", () => {
|
|||||||
expect(res.ok).toBe(true);
|
expect(res.ok).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("validateSystemRunCommandConsistency rejects shell-only rawCommand for env assignment prelude", () => {
|
||||||
|
expectRawCommandMismatch({
|
||||||
|
argv: ["/usr/bin/env", "BASH_ENV=/tmp/payload.sh", "bash", "-lc", "echo hi"],
|
||||||
|
rawCommand: "echo hi",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test("validateSystemRunCommandConsistency accepts full rawCommand for env assignment prelude", () => {
|
||||||
|
const raw = '/usr/bin/env BASH_ENV=/tmp/payload.sh bash -lc "echo hi"';
|
||||||
|
const res = validateSystemRunCommandConsistency({
|
||||||
|
argv: ["/usr/bin/env", "BASH_ENV=/tmp/payload.sh", "bash", "-lc", "echo hi"],
|
||||||
|
rawCommand: raw,
|
||||||
|
});
|
||||||
|
expect(res.ok).toBe(true);
|
||||||
|
if (!res.ok) {
|
||||||
|
throw new Error("unreachable");
|
||||||
|
}
|
||||||
|
expect(res.shellCommand).toBe("echo hi");
|
||||||
|
expect(res.cmdText).toBe(raw);
|
||||||
|
});
|
||||||
|
|
||||||
test("validateSystemRunCommandConsistency rejects cmd.exe /c trailing-arg smuggling", () => {
|
test("validateSystemRunCommandConsistency rejects cmd.exe /c trailing-arg smuggling", () => {
|
||||||
expectRawCommandMismatch({
|
expectRawCommandMismatch({
|
||||||
argv: ["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"],
|
argv: ["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"],
|
||||||
@@ -143,4 +164,16 @@ describe("system run command helpers", () => {
|
|||||||
expect(res.shellCommand).toBe("echo SAFE&&whoami");
|
expect(res.shellCommand).toBe("echo SAFE&&whoami");
|
||||||
expect(res.cmdText).toBe("echo SAFE&&whoami");
|
expect(res.cmdText).toBe("echo SAFE&&whoami");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("resolveSystemRunCommand binds cmdText to full argv when env prelude modifies shell wrapper", () => {
|
||||||
|
const res = resolveSystemRunCommand({
|
||||||
|
command: ["/usr/bin/env", "BASH_ENV=/tmp/payload.sh", "bash", "-lc", "echo hi"],
|
||||||
|
});
|
||||||
|
expect(res.ok).toBe(true);
|
||||||
|
if (!res.ok) {
|
||||||
|
throw new Error("unreachable");
|
||||||
|
}
|
||||||
|
expect(res.shellCommand).toBe("echo hi");
|
||||||
|
expect(res.cmdText).toBe('/usr/bin/env BASH_ENV=/tmp/payload.sh bash -lc "echo hi"');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,4 +1,7 @@
|
|||||||
import { extractShellWrapperCommand } from "./exec-wrapper-resolution.js";
|
import {
|
||||||
|
extractShellWrapperCommand,
|
||||||
|
hasEnvManipulationBeforeShellWrapper,
|
||||||
|
} from "./exec-wrapper-resolution.js";
|
||||||
|
|
||||||
export type SystemRunCommandValidation =
|
export type SystemRunCommandValidation =
|
||||||
| {
|
| {
|
||||||
@@ -54,8 +57,14 @@ export function validateSystemRunCommandConsistency(params: {
|
|||||||
typeof params.rawCommand === "string" && params.rawCommand.trim().length > 0
|
typeof params.rawCommand === "string" && params.rawCommand.trim().length > 0
|
||||||
? params.rawCommand.trim()
|
? params.rawCommand.trim()
|
||||||
: null;
|
: null;
|
||||||
const shellCommand = extractShellWrapperCommand(params.argv).command;
|
const shellWrapperResolution = extractShellWrapperCommand(params.argv);
|
||||||
const inferred = shellCommand !== null ? shellCommand.trim() : formatExecCommand(params.argv);
|
const shellCommand = shellWrapperResolution.command;
|
||||||
|
const envManipulationBeforeShellWrapper =
|
||||||
|
shellWrapperResolution.isWrapper && hasEnvManipulationBeforeShellWrapper(params.argv);
|
||||||
|
const inferred =
|
||||||
|
shellCommand !== null && !envManipulationBeforeShellWrapper
|
||||||
|
? shellCommand.trim()
|
||||||
|
: formatExecCommand(params.argv);
|
||||||
|
|
||||||
if (raw && raw !== inferred) {
|
if (raw && raw !== inferred) {
|
||||||
return {
|
return {
|
||||||
@@ -72,10 +81,15 @@ export function validateSystemRunCommandConsistency(params: {
|
|||||||
return {
|
return {
|
||||||
ok: true,
|
ok: true,
|
||||||
// Only treat this as a shell command when argv is a recognized shell wrapper.
|
// 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
|
// For direct argv execution and shell wrappers with env prelude modifiers,
|
||||||
// must match the formatted argv.
|
// rawCommand is purely display/approval text and must match the formatted argv.
|
||||||
shellCommand: shellCommand !== null ? (raw ?? shellCommand) : null,
|
shellCommand:
|
||||||
cmdText: raw ?? shellCommand ?? inferred,
|
shellCommand !== null
|
||||||
|
? envManipulationBeforeShellWrapper
|
||||||
|
? shellCommand
|
||||||
|
: (raw ?? shellCommand)
|
||||||
|
: null,
|
||||||
|
cmdText: raw ?? inferred,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user