diff --git a/src/agents/bash-tools.exec.path.test.ts b/src/agents/bash-tools.exec.path.test.ts index cd20428ab95..ed44778525f 100644 --- a/src/agents/bash-tools.exec.path.test.ts +++ b/src/agents/bash-tools.exec.path.test.ts @@ -342,6 +342,8 @@ describe("exec host env validation", () => { "exec -a openclaw /approve abc123 deny", "sudo /approve abc123 allow-once", "sudo -E /approve abc123 allow-once", + "sudo -uroot bash -lc '/approve abc123 allow-once'", + "doas -uroot bash -lc '/approve abc123 deny'", "bash -lc '/approve abc123 deny'", "bash -c 'sudo /approve abc123 allow-once'", "sh -c '/approve abc123 allow-once'", diff --git a/src/infra/command-analysis/risks.test.ts b/src/infra/command-analysis/risks.test.ts index ae107e992d5..e312728f805 100644 --- a/src/infra/command-analysis/risks.test.ts +++ b/src/infra/command-analysis/risks.test.ts @@ -16,6 +16,8 @@ describe("command-analysis risks", () => { expect(detectInlineEvalArgv(["sudo", "-u", "root", "python3", "-c", "print(1)"])?.flag).toBe( "-c", ); + expect(detectInlineEvalArgv(["sudo", "-uroot", "python3", "-c", "print(1)"])?.flag).toBe("-c"); + expect(detectInlineEvalArgv(["doas", "-uroot", "python3", "-c", "print(1)"])?.flag).toBe("-c"); expect(detectInlineEvalArgv(["env", "sudo", "python3", "-c", "print(1)"])?.flag).toBe("-c"); expect(detectInlineEvalArgv(["command", "node", "--eval", "1"])?.flag).toBe("--eval"); expect(detectInlineEvalArgv(["env", "-S", 'python3 -c "print(1)"'])?.flag).toBe("-c"); @@ -54,6 +56,12 @@ describe("command-analysis risks", () => { (argv, startIndex) => argv[startIndex] === "-lc", ); expect(hit).toBe("sudo"); + expect( + detectShellWrapperThroughCarrierArgv( + ["sudo", "-uroot", "bash", "-lc", "id"], + (argv, startIndex) => argv[startIndex] === "-lc", + ), + ).toBe("sudo"); expect( detectShellWrapperThroughCarrierArgv( ["sudo", "echo", "bash", "-lc", "id"], @@ -77,6 +85,12 @@ describe("command-analysis risks", () => { expect(buildCommandPayloadCandidates(["FOO=1", "sudo", "-E", "/approve", "abc"])).toEqual([ "/approve abc", ]); + expect( + buildCommandPayloadCandidates(["sudo", "-uroot", "bash", "-lc", "/approve req allow-once"]), + ).toEqual(["bash -lc /approve req allow-once", "/approve req allow-once"]); + expect( + buildCommandPayloadCandidates(["doas", "-uroot", "bash", "-lc", "/approve req allow-once"]), + ).toEqual(["bash -lc /approve req allow-once", "/approve req allow-once"]); expect(buildCommandPayloadCandidates(["env", "-S", "bash -lc '/approve abc deny'"])).toEqual([ "bash -lc /approve abc deny", "/approve abc deny", diff --git a/src/infra/command-analysis/risks.ts b/src/infra/command-analysis/risks.ts index 3751b05e60d..34284eea36b 100644 --- a/src/infra/command-analysis/risks.ts +++ b/src/infra/command-analysis/risks.ts @@ -100,8 +100,37 @@ function optionName(token: string): string { return token.split("=", 1)[0] ?? token; } -function hasInlineShortOptionValue(token: string): boolean { - return /^-[A-Za-z].+/u.test(token) && token.length > 2; +type ParsedCarrierOption = { + name: string; + hasInlineValue: boolean; +}; + +function parseCarrierOptionToken( + token: string, + optionsWithValue: ReadonlySet, +): ParsedCarrierOption { + if (token.startsWith("--")) { + return { name: optionName(token), hasInlineValue: token.includes("=") }; + } + const shortName = token.slice(0, 2); + if (/^-[A-Za-z]/u.test(shortName) && token.length > 2 && optionsWithValue.has(shortName)) { + return { name: shortName, hasInlineValue: true }; + } + return { name: token, hasInlineValue: false }; +} + +function knownCarrierOptionConsumesNextValue( + option: ParsedCarrierOption, + standaloneOptions: ReadonlySet, + optionsWithValue: ReadonlySet, +): boolean | null { + if (standaloneOptions.has(option.name)) { + return false; + } + if (optionsWithValue.has(option.name)) { + return !option.hasInlineValue; + } + return null; } function resolveEnvSplitPayload(payload: string, depth: number): string[] | null { @@ -138,17 +167,18 @@ function resolveEnvCarriedArgv(argv: string[], depth = 0): string[] | null { return resolveEnvSplitPayload(token.slice(2), depth); } if (token.startsWith("-")) { - const normalized = optionName(token); - if (ENV_STANDALONE_OPTIONS.has(normalized)) { - continue; + const consumeNextValue = knownCarrierOptionConsumesNextValue( + parseCarrierOptionToken(token, ENV_OPTIONS_WITH_VALUE), + ENV_STANDALONE_OPTIONS, + ENV_OPTIONS_WITH_VALUE, + ); + if (consumeNextValue === null) { + return null; } - if (ENV_OPTIONS_WITH_VALUE.has(normalized)) { - if (!token.includes("=") && !hasInlineShortOptionValue(token)) { - index += 1; - } - continue; + if (consumeNextValue) { + index += 1; } - return null; + continue; } return argv.slice(index); } @@ -204,20 +234,22 @@ function resolveSudoLikeCarriedArgv(argv: string[]): string[] | null { if (!token.startsWith("-")) { return argv.slice(index); } - const normalized = optionName(token); - if (executable === "sudo" && SUDO_NON_EXEC_OPTIONS.has(normalized)) { + const option = parseCarrierOptionToken(token, optionsWithValue); + if (executable === "sudo" && SUDO_NON_EXEC_OPTIONS.has(option.name)) { return null; } - if (standaloneOptions.has(normalized)) { - continue; + const consumeNextValue = knownCarrierOptionConsumesNextValue( + option, + standaloneOptions, + optionsWithValue, + ); + if (consumeNextValue === null) { + return null; } - if (optionsWithValue.has(normalized)) { - if (!token.includes("=") && !hasInlineShortOptionValue(token)) { - index += 1; - } - continue; + if (consumeNextValue) { + index += 1; } - return null; + continue; } return null; } @@ -259,17 +291,18 @@ function resolveExecCarriedArgv(argv: string[]): string[] | null { if (!token.startsWith("-")) { return argv.slice(index); } - const normalized = optionName(token); - if (EXEC_STANDALONE_OPTIONS.has(normalized)) { - continue; + const consumeNextValue = knownCarrierOptionConsumesNextValue( + parseCarrierOptionToken(token, EXEC_OPTIONS_WITH_VALUE), + EXEC_STANDALONE_OPTIONS, + EXEC_OPTIONS_WITH_VALUE, + ); + if (consumeNextValue === null) { + return null; } - if (EXEC_OPTIONS_WITH_VALUE.has(normalized)) { - if (!token.includes("=") && !hasInlineShortOptionValue(token)) { - index += 1; - } - continue; + if (consumeNextValue) { + index += 1; } - return null; + continue; } return null; }