mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-08 06:54:24 +00:00
fix(exec): bind env-prefixed shell wrappers to full approval text
This commit is contained in:
@@ -82,4 +82,47 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
|
||||
expect(params.approved).toBe(true);
|
||||
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 {
|
||||
return scanWrapperInvocation(argv, {
|
||||
separators: new Set(["--"]),
|
||||
@@ -345,6 +400,49 @@ export function unwrapDispatchWrappersForResolution(
|
||||
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 {
|
||||
return extractInlineCommandByFlags(argv, POSIX_INLINE_COMMAND_FLAGS, { allowCombinedC: true });
|
||||
}
|
||||
|
||||
@@ -106,6 +106,27 @@ describe("system run command helpers", () => {
|
||||
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", () => {
|
||||
expectRawCommandMismatch({
|
||||
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.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 =
|
||||
| {
|
||||
@@ -54,8 +57,14 @@ export function validateSystemRunCommandConsistency(params: {
|
||||
typeof params.rawCommand === "string" && params.rawCommand.trim().length > 0
|
||||
? params.rawCommand.trim()
|
||||
: null;
|
||||
const shellCommand = extractShellWrapperCommand(params.argv).command;
|
||||
const inferred = shellCommand !== null ? shellCommand.trim() : formatExecCommand(params.argv);
|
||||
const shellWrapperResolution = extractShellWrapperCommand(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) {
|
||||
return {
|
||||
@@ -72,10 +81,15 @@ export function validateSystemRunCommandConsistency(params: {
|
||||
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 !== null ? (raw ?? shellCommand) : null,
|
||||
cmdText: raw ?? shellCommand ?? inferred,
|
||||
// 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,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user