fix: harden sudo command carrier parsing

This commit is contained in:
Peter Steinberger
2026-05-04 05:24:47 +01:00
parent 7d26fb32a7
commit cf1991d27d
4 changed files with 65 additions and 8 deletions

View File

@@ -346,7 +346,12 @@ describe("exec host env validation", () => {
"sudo /approve abc123 allow-once",
"sudo -E /approve abc123 allow-once",
"sudo -EH /approve abc123 allow-once",
"sudo -k /approve abc123 allow-once",
"sudo --reset-timestamp /approve abc123 allow-once",
"sudo --command-timeout=1 /approve abc123 allow-once",
"sudo OPENCLAW_APPROVE=1 /approve abc123 allow-once",
"sudo -uroot bash -lc '/approve abc123 allow-once'",
"sudo -u root OPENCLAW_APPROVE=1 bash -lc '/approve abc123 allow-once'",
"sudo -EH bash -lc '/approve abc123 allow-once'",
"doas -uroot bash -lc '/approve abc123 deny'",
"env env env env env env /approve abc123 allow-once",

View File

@@ -20,6 +20,26 @@ describe("command-analysis risks", () => {
expect(detectInlineEvalArgv(["sudo", "-EH", "python3", "-c", "print(1)"])?.flag).toBe("-c");
expect(detectInlineEvalArgv(["sudo", "-i", "python3", "-c", "print(1)"])?.flag).toBe("-c");
expect(detectInlineEvalArgv(["sudo", "-s", "python3", "-c", "print(1)"])?.flag).toBe("-c");
expect(detectInlineEvalArgv(["sudo", "-k", "python3", "-c", "print(1)"])?.flag).toBe("-c");
expect(
detectInlineEvalArgv(["sudo", "--reset-timestamp", "python3", "-c", "print(1)"])?.flag,
).toBe("-c");
expect(
detectInlineEvalArgv(["sudo", "--command-timeout=1", "python3", "-c", "print(1)"])?.flag,
).toBe("-c");
expect(detectInlineEvalArgv(["sudo", "--chroot=/", "python3", "-c", "print(1)"])?.flag).toBe(
"-c",
);
expect(
detectInlineEvalArgv(["sudo", "PYTHONPATH=/tmp", "python3", "-c", "print(1)"])?.flag,
).toBe("-c");
expect(
detectInlineEvalArgv(["sudo", "-u", "root", "PYTHONPATH=/tmp", "python3", "-c", "print(1)"])
?.flag,
).toBe("-c");
expect(
detectInlineEvalArgv(["sudo", "--", "PYTHONPATH=/tmp", "python3", "-c", "print(1)"])?.flag,
).toBe("-c");
expect(detectInlineEvalArgv(["sudo", "--shell", "python3", "-c", "print(1)"])?.flag).toBe("-c");
expect(detectInlineEvalArgv(["sudo", "-Eu", "root", "python3", "-c", "print(1)"])?.flag).toBe(
"-c",
@@ -42,6 +62,9 @@ describe("command-analysis risks", () => {
it("keeps carrier inline eval detection command-boundary aware", () => {
expect(detectInlineEvalArgv(["command", "echo", "python3", "-c", "print(1)"])).toBeNull();
expect(detectInlineEvalArgv(["sudo", "echo", "python3", "-c", "print(1)"])).toBeNull();
expect(
detectInlineEvalArgv(["sudo", "FOO=bar", "echo", "python3", "-c", "print(1)"]),
).toBeNull();
expect(detectInlineEvalArgv(["env", "-S", 'echo python3 -c "print(1)"'])).toBeNull();
expect(detectInlineEvalArgv(["command", "-v", "python3", "-c", "print(1)"])).toBeNull();
});
@@ -115,6 +138,18 @@ describe("command-analysis risks", () => {
expect(buildCommandPayloadCandidates(["sudo", "-s", "/approve", "abc"])).toEqual([
"/approve abc",
]);
expect(buildCommandPayloadCandidates(["sudo", "-k", "/approve", "abc"])).toEqual([
"/approve abc",
]);
expect(buildCommandPayloadCandidates(["sudo", "--reset-timestamp", "/approve", "abc"])).toEqual(
["/approve abc"],
);
expect(
buildCommandPayloadCandidates(["sudo", "--command-timeout=1", "/approve", "abc"]),
).toEqual(["/approve abc"]);
expect(buildCommandPayloadCandidates(["sudo", "OPENCLAW_ENV=1", "/approve", "abc"])).toEqual([
"/approve abc",
]);
expect(buildCommandPayloadCandidates(["sudo", "--shell", "/approve", "abc"])).toEqual([
"/approve abc",
]);

View File

@@ -74,22 +74,23 @@ function detectCarrierInlineEvalArgvInternal(
argv: string[],
seenArgv: Set<string>,
): InterpreterInlineEvalHit | null {
const key = commandArgvKey(argv);
const executableArgv = stripLeadingEnvAssignments(argv);
const key = commandArgvKey(executableArgv);
if (seenArgv.has(key)) {
return null;
}
seenArgv.add(key);
const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(argv);
const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(executableArgv);
if (dispatchUnwrap.kind === "unwrapped") {
return detectInlineEvalArgvInternal(dispatchUnwrap.argv, seenArgv);
}
const executable = normalizeExecutableToken(argv[0] ?? "");
const executable = normalizeExecutableToken(executableArgv[0] ?? "");
if (!COMMAND_CARRIER_EXECUTABLES.has(executable)) {
return null;
}
const carriedArgv = resolveCarrierCommandArgv(argv);
const carriedArgv = resolveCarrierCommandArgv(executableArgv);
if (!carriedArgv) {
return null;
}

View File

@@ -35,7 +35,9 @@ const SUDO_OPTIONS_WITH_VALUE = new Set([
"-U",
"-u",
"--chdir",
"--chroot",
"--close-from",
"--command-timeout",
"--group",
"--host",
"--other-user",
@@ -51,6 +53,7 @@ const SUDO_STANDALONE_OPTIONS = new Set([
"-E",
"-H",
"-i",
"-k",
"-N",
"-n",
"-P",
@@ -65,13 +68,13 @@ const SUDO_STANDALONE_OPTIONS = new Set([
"--preserve-env",
"--preserve-groups",
"--reset-home",
"--reset-timestamp",
"--set-home",
"--shell",
"--stdin",
]);
const SUDO_NON_EXEC_OPTIONS = new Set([
"-K",
"-k",
"-l",
"-V",
"-v",
@@ -80,7 +83,6 @@ const SUDO_NON_EXEC_OPTIONS = new Set([
"--help",
"--list",
"--remove-timestamp",
"--reset-timestamp",
"--validate",
"--version",
]);
@@ -169,6 +171,20 @@ function knownCarrierOptionConsumesNextValue(
return consumesNextValue;
}
function stripSudoEnvAssignmentsFromCommandArgv(
executable: string,
argv: string[],
): string[] | null {
if (executable !== "sudo") {
return argv.length > 0 ? argv : null;
}
let index = 0;
while (index < argv.length && isEnvAssignmentToken(argv[index] ?? "")) {
index += 1;
}
return index < argv.length ? argv.slice(index) : null;
}
function findParsedCarrierOption(
options: readonly ParsedCarrierOption[],
names: ReadonlySet<string>,
@@ -317,10 +333,10 @@ function resolveSudoLikeCarriedArgv(argv: string[]): string[] | null {
for (let index = 1; index < argv.length; index += 1) {
const token = argv[index] ?? "";
if (token === "--") {
return argv.slice(index + 1);
return stripSudoEnvAssignmentsFromCommandArgv(executable, argv.slice(index + 1));
}
if (!token.startsWith("-")) {
return argv.slice(index);
return stripSudoEnvAssignmentsFromCommandArgv(executable, argv.slice(index));
}
const option = parseCarrierOptionToken(
token,