Skip to content

Commit 99176e1

Browse files
steipetesteipete
committed
refactor: share command payload extraction
1 parent 3f7e6ee commit 99176e1

3 files changed

Lines changed: 100 additions & 195 deletions

File tree

‎src/agents/bash-tools.exec.ts‎

Lines changed: 3 additions & 193 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import path from "node:path";
22
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
3+
import { buildCommandPayloadCandidates } from "../infra/command-analysis/risks.js";
34
import { analyzeShellCommand } from "../infra/exec-approvals-analysis.js";
45
import {
56
type ExecAsk,
@@ -16,7 +17,6 @@ import {
1617
getShellPathFromLoginShell,
1718
resolveShellEnvFallbackTimeoutMs,
1819
} from "../infra/shell-env.js";
19-
import { extractShellWrapperInlineCommand } from "../infra/shell-wrapper-resolution.js";
2020
import { logInfo } from "../logger.js";
2121
import { parseAgentSessionKey, resolveAgentIdFromSessionKey } from "../routing/session-key.js";
2222
import { createLazyImportLoader } from "../shared/lazy-promise.js";
@@ -1140,207 +1140,17 @@ function parseOpenClawChannelsLoginShellCommand(raw: string): boolean {
11401140
}
11411141

11421142
function rejectUnsafeControlShellCommand(command: string): void {
1143-
const isEnvAssignmentToken = (token: string): boolean =>
1144-
/^[A-Za-z_][A-Za-z0-9_]*=.*$/u.test(token);
1145-
const commandStandaloneOptions = new Set(["-p", "-v", "-V"]);
1146-
const envOptionsWithValues = new Set([
1147-
"-C",
1148-
"-S",
1149-
"-u",
1150-
"--argv0",
1151-
"--block-signal",
1152-
"--chdir",
1153-
"--default-signal",
1154-
"--ignore-signal",
1155-
"--split-string",
1156-
"--unset",
1157-
]);
1158-
const execOptionsWithValues = new Set(["-a"]);
1159-
const execStandaloneOptions = new Set(["-c", "-l"]);
1160-
const sudoOptionsWithValues = new Set([
1161-
"-C",
1162-
"-D",
1163-
"-g",
1164-
"-p",
1165-
"-R",
1166-
"-T",
1167-
"-U",
1168-
"-u",
1169-
"--chdir",
1170-
"--close-from",
1171-
"--group",
1172-
"--host",
1173-
"--other-user",
1174-
"--prompt",
1175-
"--role",
1176-
"--type",
1177-
"--user",
1178-
]);
1179-
const sudoStandaloneOptions = new Set(["-A", "-E", "--askpass", "--preserve-env"]);
1180-
const extractEnvSplitStringPayload = (argv: string[]): string[] => {
1181-
const remaining = [...argv];
1182-
while (remaining[0] && isEnvAssignmentToken(remaining[0])) {
1183-
remaining.shift();
1184-
}
1185-
if (remaining[0] !== "env") {
1186-
return [];
1187-
}
1188-
remaining.shift();
1189-
const payloads: string[] = [];
1190-
while (remaining.length > 0) {
1191-
while (remaining[0] && isEnvAssignmentToken(remaining[0])) {
1192-
remaining.shift();
1193-
}
1194-
const token: string | undefined = remaining[0];
1195-
if (!token) {
1196-
break;
1197-
}
1198-
if (token === "--") {
1199-
remaining.shift();
1200-
continue;
1201-
}
1202-
if (!token.startsWith("-") || token === "-") {
1203-
break;
1204-
}
1205-
const option = remaining.shift()!;
1206-
const normalized = option.split("=", 1)[0];
1207-
if (normalized === "-S" || normalized === "--split-string") {
1208-
const value = option.includes("=")
1209-
? option.slice(option.indexOf("=") + 1)
1210-
: remaining.shift();
1211-
if (value?.trim()) {
1212-
payloads.push(value);
1213-
}
1214-
continue;
1215-
}
1216-
if (envOptionsWithValues.has(normalized) && !option.includes("=") && remaining[0]) {
1217-
remaining.shift();
1218-
}
1219-
}
1220-
return payloads;
1221-
};
1222-
const stripApprovalCommandPrefixes = (argv: string[]): string[] => {
1223-
const remaining = [...argv];
1224-
while (remaining.length > 0) {
1225-
while (remaining[0] && isEnvAssignmentToken(remaining[0])) {
1226-
remaining.shift();
1227-
}
1228-
1229-
const token = remaining[0];
1230-
if (!token) {
1231-
break;
1232-
}
1233-
if (token === "--") {
1234-
remaining.shift();
1235-
continue;
1236-
}
1237-
if (token === "env") {
1238-
remaining.shift();
1239-
while (remaining.length > 0) {
1240-
while (remaining[0] && isEnvAssignmentToken(remaining[0])) {
1241-
remaining.shift();
1242-
}
1243-
const envToken = remaining[0];
1244-
if (!envToken) {
1245-
break;
1246-
}
1247-
if (envToken === "--") {
1248-
remaining.shift();
1249-
continue;
1250-
}
1251-
if (!envToken.startsWith("-") || envToken === "-") {
1252-
break;
1253-
}
1254-
const option = remaining.shift()!;
1255-
const normalized = option.split("=", 1)[0];
1256-
if (envOptionsWithValues.has(normalized) && !option.includes("=") && remaining[0]) {
1257-
remaining.shift();
1258-
}
1259-
}
1260-
continue;
1261-
}
1262-
if (token === "command" || token === "builtin") {
1263-
remaining.shift();
1264-
while (remaining[0]?.startsWith("-")) {
1265-
const option = remaining.shift()!;
1266-
if (option === "--") {
1267-
break;
1268-
}
1269-
if (!commandStandaloneOptions.has(option.split("=", 1)[0])) {
1270-
continue;
1271-
}
1272-
}
1273-
continue;
1274-
}
1275-
if (token === "exec") {
1276-
remaining.shift();
1277-
while (remaining[0]?.startsWith("-")) {
1278-
const option = remaining.shift()!;
1279-
if (option === "--") {
1280-
break;
1281-
}
1282-
const normalized = option.split("=", 1)[0];
1283-
if (execStandaloneOptions.has(normalized)) {
1284-
continue;
1285-
}
1286-
if (execOptionsWithValues.has(normalized) && !option.includes("=") && remaining[0]) {
1287-
remaining.shift();
1288-
}
1289-
}
1290-
continue;
1291-
}
1292-
if (token === "sudo") {
1293-
remaining.shift();
1294-
while (remaining[0]?.startsWith("-")) {
1295-
const option = remaining.shift()!;
1296-
if (option === "--") {
1297-
break;
1298-
}
1299-
const normalized = option.split("=", 1)[0];
1300-
if (sudoStandaloneOptions.has(normalized)) {
1301-
continue;
1302-
}
1303-
if (sudoOptionsWithValues.has(normalized) && !option.includes("=") && remaining[0]) {
1304-
remaining.shift();
1305-
}
1306-
}
1307-
continue;
1308-
}
1309-
break;
1310-
}
1311-
return remaining;
1312-
};
1313-
const buildCandidates = (argv: string[]): string[] => {
1314-
const envSplitCandidates = extractEnvSplitStringPayload(argv).flatMap((payload) => {
1315-
const innerArgv = splitShellArgs(payload);
1316-
return innerArgv ? buildCandidates(innerArgv) : [payload];
1317-
});
1318-
const stripped = stripApprovalCommandPrefixes(argv);
1319-
const shellWrapperPayload = extractShellWrapperInlineCommand(stripped);
1320-
const shellWrapperCandidates = shellWrapperPayload
1321-
? (() => {
1322-
const innerArgv = splitShellArgs(shellWrapperPayload);
1323-
return innerArgv ? buildCandidates(innerArgv) : [shellWrapperPayload];
1324-
})()
1325-
: [];
1326-
return [
1327-
...(stripped.length > 0 ? [stripped.join(" ")] : []),
1328-
...envSplitCandidates,
1329-
...shellWrapperCandidates,
1330-
];
1331-
};
1332-
13331143
const rawCommand = command.trim();
13341144
const analysis = analyzeShellCommand({ command: rawCommand });
13351145
const candidates = analysis.ok
1336-
? analysis.segments.flatMap((segment) => buildCandidates(segment.argv))
1146+
? analysis.segments.flatMap((segment) => buildCommandPayloadCandidates(segment.argv))
13371147
: rawCommand
13381148
.split(/\r?\n/)
13391149
.map((line) => line.trim())
13401150
.filter(Boolean)
13411151
.flatMap((line) => {
13421152
const argv = splitShellArgs(line);
1343-
return argv ? buildCandidates(argv) : [line];
1153+
return argv ? buildCommandPayloadCandidates(argv) : [line];
13441154
});
13451155
for (const candidate of candidates) {
13461156
if (parseExecApprovalShellCommand(candidate)) {

‎src/infra/command-analysis/risks.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, it } from "vitest";
22
import {
3+
buildCommandPayloadCandidates,
34
detectCarriedShellBuiltinArgv,
45
detectCommandCarrierArgv,
56
detectEnvSplitStringFlag,
@@ -72,6 +73,22 @@ describe("command-analysis risks", () => {
7273
expect(detectCarriedShellBuiltinArgv(["command", "echo", "eval"])).toBeNull();
7374
});
7475

76+
it("builds executable payload candidates through carriers and shell wrappers", () => {
77+
expect(buildCommandPayloadCandidates(["FOO=1", "sudo", "-E", "/approve", "abc"])).toEqual([
78+
"/approve abc",
79+
]);
80+
expect(buildCommandPayloadCandidates(["env", "-S", "bash -lc '/approve abc deny'"])).toEqual([
81+
"bash -lc /approve abc deny",
82+
"/approve abc deny",
83+
]);
84+
expect(buildCommandPayloadCandidates(["exec", "-a", "openclaw", "/approve", "abc"])).toEqual([
85+
"/approve abc",
86+
]);
87+
expect(buildCommandPayloadCandidates(["command", "-v", "/approve"])).toEqual([
88+
"command -v /approve",
89+
]);
90+
});
91+
7592
it("checks both effective and original argv for segment inline eval", () => {
7693
const hit = detectInlineEvalInSegments([
7794
{

‎src/infra/command-analysis/risks.ts‎

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import {
66
type InterpreterInlineEvalHit,
77
} from "../exec-inline-eval.js";
88
import { normalizeExecutableToken } from "../exec-wrapper-resolution.js";
9-
import { isShellWrapperExecutable } from "../shell-wrapper-resolution.js";
9+
import {
10+
extractShellWrapperInlineCommand,
11+
isShellWrapperExecutable,
12+
} from "../shell-wrapper-resolution.js";
1013

1114
export const COMMAND_CARRIER_EXECUTABLES = new Set(["sudo", "doas", "env", "command", "builtin"]);
1215

@@ -89,6 +92,8 @@ const SUDO_NON_EXEC_OPTIONS = new Set([
8992
]);
9093
const DOAS_OPTIONS_WITH_VALUE = new Set(["-a", "-C", "-u"]);
9194
const DOAS_STANDALONE_OPTIONS = new Set(["-L", "-n", "-s"]);
95+
const EXEC_OPTIONS_WITH_VALUE = new Set(["-a"]);
96+
const EXEC_STANDALONE_OPTIONS = new Set(["-c", "-l"]);
9297

9398
function isEnvAssignmentToken(token: string): boolean {
9499
return /^[A-Za-z_][A-Za-z0-9_]*=.*$/u.test(token);
@@ -220,7 +225,11 @@ function resolveSudoLikeCarriedArgv(argv: string[]): string[] | null {
220225
return null;
221226
}
222227

223-
function resolveCarrierCommandArgv(argv: string[], depth = 0): string[] | null {
228+
export function resolveCarrierCommandArgv(
229+
argv: string[],
230+
depth = 0,
231+
options?: { includeExec?: boolean },
232+
): string[] | null {
224233
if (depth > MAX_INLINE_EVAL_CARRIER_DEPTH) {
225234
return null;
226235
}
@@ -234,11 +243,80 @@ function resolveCarrierCommandArgv(argv: string[], depth = 0): string[] | null {
234243
case "sudo":
235244
case "doas":
236245
return resolveSudoLikeCarriedArgv(argv);
246+
case "exec":
247+
return options?.includeExec ? resolveExecCarriedArgv(argv) : null;
237248
default:
238249
return null;
239250
}
240251
}
241252

253+
function resolveExecCarriedArgv(argv: string[]): string[] | null {
254+
if (normalizeExecutableToken(argv[0] ?? "") !== "exec") {
255+
return null;
256+
}
257+
for (let index = 1; index < argv.length; index += 1) {
258+
const token = argv[index] ?? "";
259+
if (token === "--") {
260+
return argv.slice(index + 1);
261+
}
262+
if (!token.startsWith("-")) {
263+
return argv.slice(index);
264+
}
265+
const normalized = optionName(token);
266+
if (EXEC_STANDALONE_OPTIONS.has(normalized)) {
267+
continue;
268+
}
269+
if (EXEC_OPTIONS_WITH_VALUE.has(normalized)) {
270+
if (!token.includes("=") && !hasInlineShortOptionValue(token)) {
271+
index += 1;
272+
}
273+
continue;
274+
}
275+
return null;
276+
}
277+
return null;
278+
}
279+
280+
export function buildCommandPayloadCandidates(argv: string[], depth = 0): string[] {
281+
if (depth > MAX_INLINE_EVAL_CARRIER_DEPTH) {
282+
return argv.length > 0 ? [argv.join(" ")] : [];
283+
}
284+
const assignmentStrippedArgv = stripLeadingEnvAssignments(argv);
285+
const carriedArgv = resolveCarrierCommandArgv(assignmentStrippedArgv, depth, {
286+
includeExec: true,
287+
});
288+
const executableArgv = carriedArgv ?? assignmentStrippedArgv;
289+
const carriedCandidates = carriedArgv
290+
? buildCommandPayloadCandidates(carriedArgv, depth + 1)
291+
: [];
292+
const shellWrapperPayload = extractShellWrapperInlineCommand(executableArgv);
293+
const shellWrapperCandidates = shellWrapperPayload
294+
? (() => {
295+
const innerArgv = splitShellArgs(shellWrapperPayload);
296+
return innerArgv
297+
? buildCommandPayloadCandidates(innerArgv, depth + 1)
298+
: [shellWrapperPayload];
299+
})()
300+
: [];
301+
return uniqueCommandPayloadCandidates([
302+
...(executableArgv.length > 0 ? [executableArgv.join(" ")] : []),
303+
...carriedCandidates,
304+
...shellWrapperCandidates,
305+
]);
306+
}
307+
308+
function stripLeadingEnvAssignments(argv: string[]): string[] {
309+
let index = 0;
310+
while (index < argv.length && isEnvAssignmentToken(argv[index] ?? "")) {
311+
index += 1;
312+
}
313+
return index > 0 ? argv.slice(index) : argv;
314+
}
315+
316+
function uniqueCommandPayloadCandidates(candidates: string[]): string[] {
317+
return [...new Set(candidates.filter((candidate) => candidate.trim().length > 0))];
318+
}
319+
242320
export function detectCarrierInlineEvalArgv(
243321
argv: string[],
244322
depth = 0,

0 commit comments

Comments
 (0)