From 3f7e6eebc2f5f8b925a7cd8d84e7b21a1db7b9e6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 3 May 2026 13:13:03 +0100 Subject: [PATCH] refactor: unify command analysis for exec approvals --- src/agents/bash-tools.exec-host-gateway.ts | 14 +- .../bash-tools.exec-host-node-phases.ts | 11 +- src/agents/bash-tools.exec.ts | 32 +- src/gateway/server-methods/exec-approval.ts | 49 +++ .../server-methods/server-methods.test.ts | 37 ++ src/infra/approval-view-model.ts | 1 + src/infra/approval-view-model.types.ts | 2 + src/infra/command-analysis/explain.test.ts | 29 ++ src/infra/command-analysis/explain.ts | 92 +++++ src/infra/command-analysis/index.ts | 3 + src/infra/command-analysis/policy.ts | 68 ++++ src/infra/command-analysis/risks.test.ts | 95 +++++ src/infra/command-analysis/risks.ts | 366 ++++++++++++++++++ src/infra/command-explainer/extract.ts | 115 ++---- src/infra/exec-approval-forwarder.test.ts | 30 +- src/infra/exec-approval-forwarder.ts | 19 +- src/infra/exec-approvals-allowlist.ts | 11 +- src/infra/exec-approvals.ts | 2 + src/infra/exec-inline-eval.ts | 2 +- src/node-host/invoke-system-run.ts | 13 +- 20 files changed, 844 insertions(+), 147 deletions(-) create mode 100644 src/infra/command-analysis/explain.test.ts create mode 100644 src/infra/command-analysis/explain.ts create mode 100644 src/infra/command-analysis/index.ts create mode 100644 src/infra/command-analysis/policy.ts create mode 100644 src/infra/command-analysis/risks.test.ts create mode 100644 src/infra/command-analysis/risks.ts diff --git a/src/agents/bash-tools.exec-host-gateway.ts b/src/agents/bash-tools.exec-host-gateway.ts index 1c4315788ed..e2690010ed0 100644 --- a/src/agents/bash-tools.exec-host-gateway.ts +++ b/src/agents/bash-tools.exec-host-gateway.ts @@ -1,4 +1,5 @@ import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import { detectPolicyInlineEval } from "../infra/command-analysis/policy.js"; import { addDurableCommandApproval, type ExecAsk, @@ -12,10 +13,7 @@ import { resolveApprovalAuditCandidatePath, requiresExecApproval, } from "../infra/exec-approvals.js"; -import { - describeInterpreterInlineEval, - detectInterpreterInlineEvalArgv, -} from "../infra/exec-inline-eval.js"; +import { describeInterpreterInlineEval } from "../infra/exec-inline-eval.js"; import type { SafeBinProfile } from "../infra/exec-safe-bin-policy.js"; import { markBackgrounded, tail } from "./bash-process-registry.js"; import { @@ -292,13 +290,7 @@ export async function processGatewayAllowlist( commandText: params.command, }); const inlineEvalHit = - params.strictInlineEval === true - ? (allowlistEval.segments - .map((segment) => - detectInterpreterInlineEvalArgv(segment.resolution?.effectiveArgv ?? segment.argv), - ) - .find((entry) => entry !== null) ?? null) - : null; + params.strictInlineEval === true ? detectPolicyInlineEval(allowlistEval.segments) : null; if (inlineEvalHit) { params.warnings.push( `Warning: strict inline-eval mode requires explicit approval for ${describeInterpreterInlineEval( diff --git a/src/agents/bash-tools.exec-host-node-phases.ts b/src/agents/bash-tools.exec-host-node-phases.ts index 163bc753eef..c44ea7f7a7b 100644 --- a/src/agents/bash-tools.exec-host-node-phases.ts +++ b/src/agents/bash-tools.exec-host-node-phases.ts @@ -1,5 +1,6 @@ import crypto from "node:crypto"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import { detectPolicyInlineEval } from "../infra/command-analysis/policy.js"; import { type ExecApprovalsFile, type ExecAsk, @@ -11,7 +12,7 @@ import { } from "../infra/exec-approvals.js"; import { describeInterpreterInlineEval, - detectInterpreterInlineEvalArgv, + type InterpreterInlineEvalHit, } from "../infra/exec-inline-eval.js"; import { buildNodeShellCommand } from "../infra/node-shell.js"; import { parsePreparedSystemRunPayload } from "../infra/system-run-approval-context.js"; @@ -46,7 +47,7 @@ type NodeApprovalAnalysis = { analysisOk: boolean; allowlistSatisfied: boolean; durableApprovalSatisfied: boolean; - inlineEvalHit: ReturnType; + inlineEvalHit: InterpreterInlineEvalHit | null; }; export function shouldSkipNodeApprovalPrepare(params: { @@ -293,11 +294,7 @@ export async function analyzeNodeApprovalRequirement(params: { let durableApprovalSatisfied = false; const inlineEvalHit = params.request.strictInlineEval === true - ? (baseAllowlistEval.segments - .map((segment) => - detectInterpreterInlineEvalArgv(segment.resolution?.effectiveArgv ?? segment.argv), - ) - .find((entry) => entry !== null) ?? null) + ? detectPolicyInlineEval(baseAllowlistEval.segments) : null; if (inlineEvalHit) { params.request.warnings.push( diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 134b82e1e06..112ca29be6d 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -16,6 +16,7 @@ import { getShellPathFromLoginShell, resolveShellEnvFallbackTimeoutMs, } from "../infra/shell-env.js"; +import { extractShellWrapperInlineCommand } from "../infra/shell-wrapper-resolution.js"; import { logInfo } from "../logger.js"; import { parseAgentSessionKey, resolveAgentIdFromSessionKey } from "../routing/session-key.js"; import { createLazyImportLoader } from "../shared/lazy-promise.js"; @@ -1141,7 +1142,6 @@ function parseOpenClawChannelsLoginShellCommand(raw: string): boolean { function rejectUnsafeControlShellCommand(command: string): void { const isEnvAssignmentToken = (token: string): boolean => /^[A-Za-z_][A-Za-z0-9_]*=.*$/u.test(token); - const shellWrappers = new Set(["bash", "dash", "fish", "ksh", "sh", "zsh"]); const commandStandaloneOptions = new Set(["-p", "-v", "-V"]); const envOptionsWithValues = new Set([ "-C", @@ -1310,35 +1310,19 @@ function rejectUnsafeControlShellCommand(command: string): void { } return remaining; }; - const extractShellWrapperPayload = (argv: string[]): string[] => { - const [commandName, ...rest] = argv; - if (!commandName || !shellWrappers.has(path.basename(commandName))) { - return []; - } - for (let i = 0; i < rest.length; i += 1) { - const token = rest[i]; - if (!token) { - continue; - } - if (token === "-c" || token === "-lc" || token === "-ic" || token === "-xc") { - return rest[i + 1] ? [rest[i + 1]] : []; - } - if (/^-[^-]*c[^-]*$/u.test(token)) { - return rest[i + 1] ? [rest[i + 1]] : []; - } - } - return []; - }; const buildCandidates = (argv: string[]): string[] => { const envSplitCandidates = extractEnvSplitStringPayload(argv).flatMap((payload) => { const innerArgv = splitShellArgs(payload); return innerArgv ? buildCandidates(innerArgv) : [payload]; }); const stripped = stripApprovalCommandPrefixes(argv); - const shellWrapperCandidates = extractShellWrapperPayload(stripped).flatMap((payload) => { - const innerArgv = splitShellArgs(payload); - return innerArgv ? buildCandidates(innerArgv) : [payload]; - }); + const shellWrapperPayload = extractShellWrapperInlineCommand(stripped); + const shellWrapperCandidates = shellWrapperPayload + ? (() => { + const innerArgv = splitShellArgs(shellWrapperPayload); + return innerArgv ? buildCandidates(innerArgv) : [shellWrapperPayload]; + })() + : []; return [ ...(stripped.length > 0 ? [stripped.join(" ")] : []), ...envSplitCandidates, diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 6ea70fb61bd..e438bd8724c 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -1,3 +1,8 @@ +import { + summarizeCommandSegmentsForDisplay, + type CommandExplanationSummary, +} from "../../infra/command-analysis/explain.js"; +import { analyzeCommandForPolicy } from "../../infra/command-analysis/policy.js"; import { resolveExecApprovalCommandDisplay, sanitizeExecApprovalDisplayText, @@ -42,6 +47,43 @@ const APPROVAL_ALLOW_ALWAYS_UNAVAILABLE_DETAILS = { } as const; const RESERVED_PLUGIN_APPROVAL_ID_PREFIX = "plugin:"; +function sanitizeCommandAnalysisSummary( + summary: CommandExplanationSummary, +): CommandExplanationSummary { + return { + commandCount: summary.commandCount, + nestedCommandCount: summary.nestedCommandCount, + riskKinds: summary.riskKinds.map((kind) => sanitizeExecApprovalWarningText(kind)), + warningLines: summary.warningLines.map((line) => sanitizeExecApprovalWarningText(line)), + }; +} + +function resolveExecApprovalCommandAnalysis(params: { + host: string; + commandText: string; + commandArgv?: string[]; + cwd?: string | null; +}): CommandExplanationSummary | null { + const analysis = + Array.isArray(params.commandArgv) && params.commandArgv.length > 0 + ? analyzeCommandForPolicy({ + source: "argv", + argv: params.commandArgv, + cwd: params.cwd ?? undefined, + }) + : params.host === "node" + ? null + : analyzeCommandForPolicy({ + source: "shell", + command: params.commandText, + cwd: params.cwd ?? undefined, + }); + if (!analysis?.ok) { + return null; + } + return sanitizeCommandAnalysisSummary(summarizeCommandSegmentsForDisplay(analysis.segments)); +} + type ExecApprovalIosPushDelivery = { handleRequested?: (request: ExecApprovalRequest) => Promise; handleResolved?: (resolved: ExecApprovalResolved) => Promise; @@ -207,6 +249,12 @@ export function createExecApprovalHandlers( } const envBinding = buildSystemRunApprovalEnvBinding(p.env); const warningText = normalizeOptionalString(p.warningText); + const commandAnalysis = resolveExecApprovalCommandAnalysis({ + host, + commandText: effectiveCommandText, + commandArgv: effectiveCommandArgv, + cwd: effectiveCwd, + }); const systemRunBinding = host === "node" ? buildSystemRunApprovalBinding({ @@ -241,6 +289,7 @@ export function createExecApprovalHandlers( security: p.security ?? null, ask: p.ask ?? null, warningText: warningText ? sanitizeExecApprovalWarningText(warningText) : null, + commandAnalysis, allowedDecisions: resolveExecApprovalAllowedDecisions({ ask: p.ask ?? null }), agentId: effectiveAgentId ?? null, resolvedPath: p.resolvedPath ?? null, diff --git a/src/gateway/server-methods/server-methods.test.ts b/src/gateway/server-methods/server-methods.test.ts index 6066b165daa..e10262996d7 100644 --- a/src/gateway/server-methods/server-methods.test.ts +++ b/src/gateway/server-methods/server-methods.test.ts @@ -897,6 +897,43 @@ describe("exec approval handlers", () => { await requestPromise; }); + it("attaches shared command analysis to gateway exec approval requests", async () => { + const { handlers, broadcasts, respond, context } = createExecApprovalFixture(); + + const requestPromise = requestExecApproval({ + handlers, + respond, + context, + params: { + twoPhase: true, + host: "gateway", + command: "python3 -c 'print(1)'", + commandArgv: ["python3", "-c", "print(1)"], + systemRunPlan: undefined, + nodeId: undefined, + }, + }); + + const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested"); + const request = requested?.payload as { id?: string; request?: { commandAnalysis?: unknown } }; + expect(request.request?.commandAnalysis).toEqual( + expect.objectContaining({ + commandCount: 1, + riskKinds: expect.arrayContaining(["inline-eval"]), + warningLines: expect.arrayContaining(["Contains inline-eval: python3 -c"]), + }), + ); + + const resolveRespond = vi.fn(); + await resolveExecApproval({ + handlers, + id: request.id ?? "", + respond: resolveRespond, + context, + }); + await requestPromise; + }); + it("lists pending exec approvals", async () => { const { handlers, respond, context } = createExecApprovalFixture(); const requestPromise = requestExecApproval({ diff --git a/src/infra/approval-view-model.ts b/src/infra/approval-view-model.ts index f6fd65bac7d..4121e729ff6 100644 --- a/src/infra/approval-view-model.ts +++ b/src/infra/approval-view-model.ts @@ -69,6 +69,7 @@ function buildExecViewBase( ask: request.request.ask ?? null, agentId: request.request.agentId ?? null, warningText: request.request.warningText ?? null, + commandAnalysis: request.request.commandAnalysis ?? null, commandText, commandPreview, cwd: request.request.cwd ?? null, diff --git a/src/infra/approval-view-model.types.ts b/src/infra/approval-view-model.types.ts index ae3c52da1a8..c5fc2526ff1 100644 --- a/src/infra/approval-view-model.types.ts +++ b/src/infra/approval-view-model.types.ts @@ -1,5 +1,6 @@ import type { InteractiveReplyButton } from "../interactive/payload.js"; import type { ChannelApprovalKind } from "./approval-types.js"; +import type { CommandExplanationSummary } from "./command-analysis/explain.js"; import type { ExecApprovalDecision, ExecApprovalRequest, @@ -35,6 +36,7 @@ export type ExecApprovalViewBase = ApprovalViewBase & { ask?: string | null; agentId?: string | null; warningText?: string | null; + commandAnalysis?: CommandExplanationSummary | null; commandText: string; commandPreview?: string | null; cwd?: string | null; diff --git a/src/infra/command-analysis/explain.test.ts b/src/infra/command-analysis/explain.test.ts new file mode 100644 index 00000000000..2eff34f8dd8 --- /dev/null +++ b/src/infra/command-analysis/explain.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { explainShellCommand } from "../command-explainer/index.js"; +import { summarizeCommandExplanation, summarizeCommandSegmentsForDisplay } from "./explain.js"; + +describe("command-analysis explanation summary", () => { + it("summarizes commands and risk kinds", async () => { + const explanation = await explainShellCommand(`bash -lc 'python3 -c "print(1)"'`); + const summary = summarizeCommandExplanation(explanation); + + expect(summary.commandCount).toBe(1); + expect(summary.riskKinds).toContain("shell-wrapper"); + expect(summary.riskKinds).toContain("inline-eval"); + expect(summary.warningLines.some((line) => line.includes("inline-eval"))).toBe(true); + }); + + it("summarizes policy command segments without async parsing", () => { + const summary = summarizeCommandSegmentsForDisplay([ + { + raw: "sudo python3 -c 'print(1)'", + argv: ["sudo", "python3", "-c", "print(1)"], + resolution: null, + }, + ]); + + expect(summary.commandCount).toBe(1); + expect(summary.riskKinds).toEqual(["inline-eval"]); + expect(summary.warningLines).toEqual(["Contains inline-eval: python3 -c"]); + }); +}); diff --git a/src/infra/command-analysis/explain.ts b/src/infra/command-analysis/explain.ts new file mode 100644 index 00000000000..8bdb00a8479 --- /dev/null +++ b/src/infra/command-analysis/explain.ts @@ -0,0 +1,92 @@ +import { explainShellCommand } from "../command-explainer/extract.js"; +import type { CommandExplanation, CommandRisk } from "../command-explainer/types.js"; +import type { ExecCommandSegment } from "../exec-approvals-analysis.js"; +import { detectCommandCarrierArgv, detectInlineEvalInSegments } from "./risks.js"; + +export type CommandExplanationSummary = { + commandCount: number; + nestedCommandCount: number; + riskKinds: string[]; + warningLines: string[]; +}; + +function riskLabel(risk: CommandRisk): string { + switch (risk.kind) { + case "inline-eval": + return `${risk.command} ${risk.flag}`; + case "shell-wrapper": + return `${risk.executable} ${risk.flag}`; + case "command-carrier": + return risk.flag ? `${risk.command} ${risk.flag}` : risk.command; + case "dynamic-argument": + return `${risk.command} dynamic argument`; + case "source": + return risk.command; + case "function-definition": + return risk.name; + default: + return risk.kind; + } +} + +export function summarizeCommandExplanation( + explanation: CommandExplanation, +): CommandExplanationSummary { + const riskKinds = [...new Set(explanation.risks.map((risk) => risk.kind))]; + const warningLines = explanation.risks.map((risk) => { + const label = riskLabel(risk); + return label === risk.kind ? `Contains ${risk.kind}` : `Contains ${risk.kind}: ${label}`; + }); + return { + commandCount: explanation.topLevelCommands.length, + nestedCommandCount: explanation.nestedCommands.length, + riskKinds, + warningLines: [...new Set(warningLines)], + }; +} + +function uniqueStrings(values: string[]): string[] { + return [...new Set(values)]; +} + +export function summarizeCommandSegmentsForDisplay( + segments: readonly ExecCommandSegment[], +): CommandExplanationSummary { + const riskKinds: string[] = []; + const warningLines: string[] = []; + const inlineEval = detectInlineEvalInSegments(segments); + if (inlineEval) { + riskKinds.push("inline-eval"); + warningLines.push( + `Contains inline-eval: ${inlineEval.normalizedExecutable} ${inlineEval.flag}`, + ); + } + for (const segment of segments) { + const effectiveArgv = segment.resolution?.effectiveArgv ?? segment.argv; + for (const hit of detectCommandCarrierArgv(effectiveArgv)) { + riskKinds.push("command-carrier"); + warningLines.push( + hit.flag + ? `Contains command-carrier: ${hit.command} ${hit.flag}` + : `Contains command-carrier: ${hit.command}`, + ); + } + } + return { + commandCount: segments.length, + nestedCommandCount: 0, + riskKinds: uniqueStrings(riskKinds), + warningLines: uniqueStrings(warningLines), + }; +} + +export async function explainCommandForDisplay( + command: string, +): Promise<{ explanation: CommandExplanation; summary: CommandExplanationSummary } | null> { + try { + const explanation = await explainShellCommand(command); + return { explanation, summary: summarizeCommandExplanation(explanation) }; + } catch { + return null; + } +} diff --git a/src/infra/command-analysis/index.ts b/src/infra/command-analysis/index.ts new file mode 100644 index 00000000000..aa3e084d957 --- /dev/null +++ b/src/infra/command-analysis/index.ts @@ -0,0 +1,3 @@ +export * from "./explain.js"; +export * from "./policy.js"; +export * from "./risks.js"; diff --git a/src/infra/command-analysis/policy.ts b/src/infra/command-analysis/policy.ts new file mode 100644 index 00000000000..f102a2203a7 --- /dev/null +++ b/src/infra/command-analysis/policy.ts @@ -0,0 +1,68 @@ +import { + analyzeArgvCommand, + analyzeShellCommand, + type ExecCommandAnalysis, + type ExecCommandSegment, +} from "../exec-approvals-analysis.js"; +import { detectInlineEvalInSegments } from "./risks.js"; + +export type CommandPolicyAnalysis = + | { + ok: true; + source: "argv" | "shell"; + analysis: ExecCommandAnalysis; + segments: ExecCommandSegment[]; + } + | { + ok: false; + source: "argv" | "shell"; + reason?: string; + analysis: ExecCommandAnalysis; + segments: []; + }; + +export function analyzeCommandForPolicy( + params: + | { + source: "shell"; + command: string; + cwd?: string; + env?: NodeJS.ProcessEnv; + platform?: string | null; + } + | { + source: "argv"; + argv: string[]; + cwd?: string; + env?: NodeJS.ProcessEnv; + }, +): CommandPolicyAnalysis { + const analysis = + params.source === "shell" + ? analyzeShellCommand({ + command: params.command, + cwd: params.cwd, + env: params.env, + platform: params.platform, + }) + : analyzeArgvCommand({ argv: params.argv, cwd: params.cwd, env: params.env }); + if (!analysis.ok) { + return { + ok: false, + source: params.source, + reason: analysis.reason, + analysis, + segments: [], + }; + } + return { + ok: true, + source: params.source, + analysis, + segments: analysis.segments, + }; +} + +export function detectPolicyInlineEval(segments: readonly ExecCommandSegment[]) { + return detectInlineEvalInSegments(segments); +} diff --git a/src/infra/command-analysis/risks.test.ts b/src/infra/command-analysis/risks.test.ts new file mode 100644 index 00000000000..eea46e967f4 --- /dev/null +++ b/src/infra/command-analysis/risks.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; +import { + detectCarriedShellBuiltinArgv, + detectCommandCarrierArgv, + detectEnvSplitStringFlag, + detectInlineEvalArgv, + detectInlineEvalInSegments, + detectShellWrapperThroughCarrierArgv, +} from "./risks.js"; + +describe("command-analysis risks", () => { + it("detects inline eval through transparent carriers", () => { + expect(detectInlineEvalArgv(["python3", "-c", "print(1)"])?.flag).toBe("-c"); + expect(detectInlineEvalArgv(["sudo", "python3", "-c", "print(1)"])?.flag).toBe("-c"); + expect(detectInlineEvalArgv(["sudo", "-u", "root", "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"); + expect(detectInlineEvalArgv(["python3", "script.py"])).toBeNull(); + }); + + 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(["env", "-S", 'echo python3 -c "print(1)"'])).toBeNull(); + expect(detectInlineEvalArgv(["command", "-v", "python3", "-c", "print(1)"])).toBeNull(); + }); + + it("detects command carriers", () => { + expect(detectCommandCarrierArgv(["find", ".", "-exec", "rm", "{}", ";"])).toEqual([ + { command: "find", flag: "-exec" }, + ]); + expect(detectCommandCarrierArgv(["xargs", "-I{}", "sh", "-c", "id"])).toEqual([ + { command: "xargs" }, + ]); + expect(detectCommandCarrierArgv(["env", "-S", "sh -c id"])).toEqual([ + { command: "env", flag: "-S" }, + ]); + }); + + it("detects env split-string flag forms", () => { + expect(detectEnvSplitStringFlag(["env", "-S", "sh -c id"])).toBe("-S"); + expect(detectEnvSplitStringFlag(["env", "-Ssh -c id"])).toBe("-S"); + expect(detectEnvSplitStringFlag(["env", "--split-string=sh -c id"])).toBe("--split-string"); + expect(detectEnvSplitStringFlag(["env", "sh", "-c", "id"])).toBeNull(); + }); + + it("detects shell wrappers carried through prefix commands", () => { + const hit = detectShellWrapperThroughCarrierArgv( + ["sudo", "bash", "-lc", "id"], + (argv, startIndex) => argv[startIndex] === "-lc", + ); + expect(hit).toBe("sudo"); + expect( + detectShellWrapperThroughCarrierArgv( + ["sudo", "echo", "bash", "-lc", "id"], + (argv, startIndex) => argv[startIndex] === "-lc", + ), + ).toBeNull(); + }); + + it("detects carried eval and source builtins", () => { + expect(detectCarriedShellBuiltinArgv(["builtin", "eval", "echo hi"])).toEqual({ + kind: "eval", + }); + expect(detectCarriedShellBuiltinArgv(["command", "source", "./env.sh"])).toEqual({ + kind: "source", + command: "source", + }); + expect(detectCarriedShellBuiltinArgv(["command", "echo", "eval"])).toBeNull(); + }); + + it("checks both effective and original argv for segment inline eval", () => { + const hit = detectInlineEvalInSegments([ + { + raw: "sudo python3 -c 'print(1)'", + argv: ["sudo", "python3", "-c", "print(1)"], + resolution: { + execution: { + rawExecutable: "sudo", + executableName: "sudo", + }, + policy: { + rawExecutable: "sudo", + executableName: "sudo", + }, + effectiveArgv: ["sudo", "python3", "-c", "print(1)"], + }, + }, + ]); + expect(hit?.normalizedExecutable).toBe("python3"); + }); +}); diff --git a/src/infra/command-analysis/risks.ts b/src/infra/command-analysis/risks.ts new file mode 100644 index 00000000000..6f48c57e37e --- /dev/null +++ b/src/infra/command-analysis/risks.ts @@ -0,0 +1,366 @@ +import { splitShellArgs } from "../../utils/shell-argv.js"; +import { unwrapKnownDispatchWrapperInvocation } from "../dispatch-wrapper-resolution.js"; +import type { ExecCommandSegment } from "../exec-approvals-analysis.js"; +import { + detectInterpreterInlineEvalArgv, + type InterpreterInlineEvalHit, +} from "../exec-inline-eval.js"; +import { normalizeExecutableToken } from "../exec-wrapper-resolution.js"; +import { isShellWrapperExecutable } from "../shell-wrapper-resolution.js"; + +export const COMMAND_CARRIER_EXECUTABLES = new Set(["sudo", "doas", "env", "command", "builtin"]); + +export const SOURCE_EXECUTABLES = new Set([".", "source"]); + +export type CommandCarrierHit = { + command: string; + flag?: string; +}; + +export type CarriedShellBuiltinHit = { kind: "eval" } | { kind: "source"; command: string }; + +const MAX_INLINE_EVAL_CARRIER_DEPTH = 4; + +const COMMAND_EXECUTING_OPTIONS = new Set(["-p"]); +const COMMAND_QUERY_OPTIONS = new Set(["-v", "-V"]); +const ENV_OPTIONS_WITH_VALUE = new Set([ + "-C", + "-S", + "-u", + "--argv0", + "--block-signal", + "--chdir", + "--default-signal", + "--ignore-signal", + "--split-string", + "--unset", +]); +const ENV_STANDALONE_OPTIONS = new Set(["-0", "-i", "--ignore-environment", "--null"]); +const SUDO_OPTIONS_WITH_VALUE = new Set([ + "-C", + "-D", + "-g", + "-h", + "-p", + "-R", + "-T", + "-U", + "-u", + "--chdir", + "--close-from", + "--group", + "--host", + "--other-user", + "--prompt", + "--role", + "--type", + "--user", +]); +const SUDO_STANDALONE_OPTIONS = new Set([ + "-A", + "-b", + "-E", + "-H", + "-n", + "-P", + "-S", + "--askpass", + "--background", + "--login", + "--non-interactive", + "--preserve-env", + "--reset-home", + "--stdin", +]); +const SUDO_NON_EXEC_OPTIONS = new Set([ + "-K", + "-k", + "-l", + "-V", + "-v", + "-e", + "--edit", + "--help", + "--list", + "--remove-timestamp", + "--reset-timestamp", + "--validate", + "--version", +]); +const DOAS_OPTIONS_WITH_VALUE = new Set(["-a", "-C", "-u"]); +const DOAS_STANDALONE_OPTIONS = new Set(["-L", "-n", "-s"]); + +function isEnvAssignmentToken(token: string): boolean { + return /^[A-Za-z_][A-Za-z0-9_]*=.*$/u.test(token); +} + +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; +} + +function resolveEnvSplitPayload(payload: string, depth: number): string[] | null { + const innerArgv = splitShellArgs(payload); + if (!innerArgv || innerArgv.length === 0) { + return null; + } + return resolveEnvCarriedArgv(["env", ...innerArgv], depth + 1) ?? innerArgv; +} + +function resolveEnvCarriedArgv(argv: string[], depth = 0): string[] | null { + if (depth > MAX_INLINE_EVAL_CARRIER_DEPTH || normalizeExecutableToken(argv[0] ?? "") !== "env") { + return null; + } + for (let index = 1; index < argv.length; index += 1) { + const token = argv[index] ?? ""; + if (!token) { + return null; + } + if (isEnvAssignmentToken(token)) { + continue; + } + if (token === "--") { + return argv.slice(index + 1); + } + if (token === "-S" || token === "--split-string") { + const payload = argv[index + 1]; + return typeof payload === "string" ? resolveEnvSplitPayload(payload, depth) : null; + } + if (token.startsWith("--split-string=")) { + return resolveEnvSplitPayload(token.slice("--split-string=".length), depth); + } + if (token.startsWith("-S") && token.length > 2) { + return resolveEnvSplitPayload(token.slice(2), depth); + } + if (token.startsWith("-")) { + const normalized = optionName(token); + if (ENV_STANDALONE_OPTIONS.has(normalized)) { + continue; + } + if (ENV_OPTIONS_WITH_VALUE.has(normalized)) { + if (!token.includes("=") && !hasInlineShortOptionValue(token)) { + index += 1; + } + continue; + } + return null; + } + return argv.slice(index); + } + return null; +} + +function resolveCommandBuiltinCarriedArgv(argv: string[]): string[] | null { + const executable = normalizeExecutableToken(argv[0] ?? ""); + if (executable !== "command" && executable !== "builtin") { + return null; + } + for (let index = 1; index < argv.length; index += 1) { + const token = argv[index] ?? ""; + if (token === "--") { + return argv.slice(index + 1); + } + if (!token.startsWith("-")) { + return argv.slice(index); + } + const normalized = optionName(token); + if (COMMAND_QUERY_OPTIONS.has(normalized)) { + return null; + } + if (!COMMAND_EXECUTING_OPTIONS.has(normalized)) { + return null; + } + } + return null; +} + +function resolveSudoLikeCarriedArgv(argv: string[]): string[] | null { + const executable = normalizeExecutableToken(argv[0] ?? ""); + const standaloneOptions = + executable === "sudo" + ? SUDO_STANDALONE_OPTIONS + : executable === "doas" + ? DOAS_STANDALONE_OPTIONS + : null; + const optionsWithValue = + executable === "sudo" + ? SUDO_OPTIONS_WITH_VALUE + : executable === "doas" + ? DOAS_OPTIONS_WITH_VALUE + : null; + if (!standaloneOptions || !optionsWithValue) { + return null; + } + for (let index = 1; index < argv.length; index += 1) { + const token = argv[index] ?? ""; + if (token === "--") { + return argv.slice(index + 1); + } + if (!token.startsWith("-")) { + return argv.slice(index); + } + const normalized = optionName(token); + if (executable === "sudo" && SUDO_NON_EXEC_OPTIONS.has(normalized)) { + return null; + } + if (standaloneOptions.has(normalized)) { + continue; + } + if (optionsWithValue.has(normalized)) { + if (!token.includes("=") && !hasInlineShortOptionValue(token)) { + index += 1; + } + continue; + } + return null; + } + return null; +} + +function resolveCarrierCommandArgv(argv: string[], depth = 0): string[] | null { + if (depth > MAX_INLINE_EVAL_CARRIER_DEPTH) { + return null; + } + const executable = normalizeExecutableToken(argv[0] ?? ""); + switch (executable) { + case "env": + return resolveEnvCarriedArgv(argv, depth); + case "command": + case "builtin": + return resolveCommandBuiltinCarriedArgv(argv); + case "sudo": + case "doas": + return resolveSudoLikeCarriedArgv(argv); + default: + return null; + } +} + +export function detectCarrierInlineEvalArgv( + argv: string[], + depth = 0, +): InterpreterInlineEvalHit | null { + if (depth > MAX_INLINE_EVAL_CARRIER_DEPTH) { + return null; + } + const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(argv); + if (dispatchUnwrap.kind === "unwrapped") { + return detectInlineEvalArgv(dispatchUnwrap.argv, depth + 1); + } + + const executable = normalizeExecutableToken(argv[0] ?? ""); + if (!COMMAND_CARRIER_EXECUTABLES.has(executable)) { + return null; + } + const carriedArgv = resolveCarrierCommandArgv(argv, depth); + return carriedArgv ? detectInlineEvalArgv(carriedArgv, depth + 1) : null; +} + +export function detectInlineEvalArgv( + argv: string[] | undefined | null, + depth = 0, +): InterpreterInlineEvalHit | null { + if (!Array.isArray(argv)) { + return null; + } + return detectInterpreterInlineEvalArgv(argv) ?? detectCarrierInlineEvalArgv(argv, depth); +} + +export function detectInlineEvalInSegments( + segments: readonly ExecCommandSegment[], +): InterpreterInlineEvalHit | null { + for (const segment of segments) { + const effective = segment.resolution?.effectiveArgv ?? segment.argv; + const hit = detectInlineEvalArgv(effective) ?? detectInlineEvalArgv(segment.argv); + if (hit) { + return hit; + } + } + return null; +} + +export function detectCommandCarrierArgv(argv: string[]): CommandCarrierHit[] { + const executable = argv[0]; + if (!executable) { + return []; + } + const normalizedExecutable = normalizeExecutableToken(executable); + const hits: CommandCarrierHit[] = []; + if (normalizedExecutable === "find") { + const flag = argv.find((arg) => ["-exec", "-execdir", "-ok", "-okdir"].includes(arg)); + if (flag) { + hits.push({ command: executable, flag }); + } + } + if (normalizedExecutable === "xargs") { + hits.push({ command: normalizedExecutable }); + } + const splitStringFlag = detectEnvSplitStringFlag(argv); + if (splitStringFlag) { + hits.push({ command: normalizedExecutable, flag: splitStringFlag }); + } + return hits; +} + +export function detectEnvSplitStringFlag(argv: string[]): string | null { + if (normalizeExecutableToken(argv[0] ?? "") !== "env") { + return null; + } + for (const arg of argv.slice(1)) { + const token = arg.trim(); + if (token === "-S" || token === "--split-string") { + return token; + } + if (token.startsWith("--split-string=") || (token.startsWith("-S") && token.length > 2)) { + return token.startsWith("--") ? "--split-string" : "-S"; + } + } + return null; +} + +export function detectShellWrapperThroughCarrierArgv( + argv: string[], + shellCommandFlag: (argv: string[], startIndex: number) => unknown, +): string | null { + const executable = normalizeExecutableToken(argv[0] ?? ""); + if (!COMMAND_CARRIER_EXECUTABLES.has(executable)) { + return null; + } + const carriedArgv = resolveCarrierCommandArgv(argv); + if (!carriedArgv) { + return null; + } + if (isShellWrapperExecutable(carriedArgv[0] ?? "") && shellCommandFlag(carriedArgv, 1)) { + return executable; + } + return detectShellWrapperThroughCarrierArgv(carriedArgv, shellCommandFlag) ? executable : null; +} + +export function detectCarriedShellBuiltinArgv(argv: string[]): CarriedShellBuiltinHit | null { + const executable = normalizeExecutableToken(argv[0] ?? ""); + if (!COMMAND_CARRIER_EXECUTABLES.has(executable)) { + return null; + } + const carriedArgv = resolveCarrierCommandArgv(argv); + if (!carriedArgv) { + return null; + } + const nestedCarrierHit = detectCarriedShellBuiltinArgv(carriedArgv); + if (nestedCarrierHit) { + return nestedCarrierHit; + } + const carriedCommand = carriedArgv[0]; + const normalizedCarriedCommand = carriedCommand + ? normalizeExecutableToken(carriedCommand) + : undefined; + if (normalizedCarriedCommand === "eval") { + return { kind: "eval" }; + } + if (normalizedCarriedCommand && SOURCE_EXECUTABLES.has(normalizedCarriedCommand)) { + return { kind: "source", command: normalizedCarriedCommand }; + } + return null; +} diff --git a/src/infra/command-explainer/extract.ts b/src/infra/command-explainer/extract.ts index 4d659065358..043b1c9200a 100644 --- a/src/infra/command-explainer/extract.ts +++ b/src/infra/command-explainer/extract.ts @@ -1,6 +1,12 @@ import type { Node as TreeSitterNode } from "web-tree-sitter"; -import { unwrapKnownDispatchWrapperInvocation } from "../dispatch-wrapper-resolution.js"; -import { detectInterpreterInlineEvalArgv } from "../exec-inline-eval.js"; +import { + detectCarriedShellBuiltinArgv, + detectCommandCarrierArgv, + detectInlineEvalArgv, + detectShellWrapperThroughCarrierArgv, + SOURCE_EXECUTABLES, +} from "../command-analysis/risks.js"; +import type { InterpreterInlineEvalHit } from "../exec-inline-eval.js"; import { normalizeExecutableToken } from "../exec-wrapper-resolution.js"; import { extractShellWrapperCommand, @@ -54,8 +60,6 @@ type WalkState = { const MAX_WRAPPER_PAYLOAD_DEPTH = 2; const PARSEABLE_SHELL_WRAPPERS = new Set(POSIX_SHELL_WRAPPERS); -const SHELL_CARRIER_EXECUTABLES = new Set(["sudo", "doas", "env", "command", "builtin"]); -const SOURCE_EXECUTABLES = new Set([".", "source"]); type SpanBase = { startIndex: number; @@ -891,42 +895,7 @@ function shellWrapperPayloadForParsing( return { command: shellWrapper.command, spanBase }; } -type InlineEvalHit = NonNullable>; - -function detectCarrierInlineEvalArgv(argv: string[]): InlineEvalHit | null { - const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(argv); - if (dispatchUnwrap.kind === "unwrapped") { - return detectInterpreterInlineEvalArgv(dispatchUnwrap.argv); - } - - const executable = normalizeExecutableToken(argv[0] ?? ""); - if (!SHELL_CARRIER_EXECUTABLES.has(executable)) { - return null; - } - for (let index = 1; index < argv.length; index += 1) { - const hit = detectInterpreterInlineEvalArgv(argv.slice(index)); - if (hit) { - return hit; - } - } - return null; -} - -function envSplitStringFlag(argv: string[]): string | null { - if (normalizeExecutableToken(argv[0] ?? "") !== "env") { - return null; - } - for (const arg of argv.slice(1)) { - const token = arg.trim(); - if (token === "-S" || token === "--split-string") { - return token; - } - if (token.startsWith("--split-string=") || (token.startsWith("-S") && token.length > 2)) { - return token.startsWith("--") ? "--split-string" : "-S"; - } - } - return null; -} +type InlineEvalHit = InterpreterInlineEvalHit; function recordInlineEvalRisk( inlineEval: InlineEvalHit, @@ -972,7 +941,7 @@ function recordCommandRisks( } const normalizedExecutable = normalizeExecutableToken(executable); recordDynamicArgumentRisks(normalizedExecutable, dynamicArguments, output); - const inlineEval = detectInterpreterInlineEvalArgv(argv) ?? detectCarrierInlineEvalArgv(argv); + const inlineEval = detectInlineEvalArgv(argv); if (inlineEval) { recordInlineEvalRisk(inlineEval, text, span, output); } @@ -1001,21 +970,11 @@ function recordCommandRisks( } } - if (normalizedExecutable === "find") { - const flag = argv.find((arg) => ["-exec", "-execdir", "-ok", "-okdir"].includes(arg)); - if (flag) { - output.risks.push({ kind: "command-carrier", command: executable, flag, text, span }); - } - } - if (normalizedExecutable === "xargs") { - output.risks.push({ kind: "command-carrier", command: normalizedExecutable, text, span }); - } - const splitStringFlag = envSplitStringFlag(argv); - if (splitStringFlag) { + for (const carrier of detectCommandCarrierArgv(argv)) { output.risks.push({ kind: "command-carrier", - command: normalizedExecutable, - flag: splitStringFlag, + command: carrier.command, + flag: carrier.flag, text, span, }); @@ -1029,34 +988,28 @@ function recordCommandRisks( if (normalizedExecutable === "alias") { output.risks.push({ kind: "alias", text, span }); } - if (!shellWrapper.isWrapper && SHELL_CARRIER_EXECUTABLES.has(normalizedExecutable)) { - const shellIndex = argv.findIndex((arg) => isShellWrapperExecutable(arg)); - if (shellIndex >= 0 && shellCommandFlag(argv, shellIndex + 1)) { - output.risks.push({ - kind: "shell-wrapper-through-carrier", - command: normalizedExecutable, - text, - span, - }); - } - - const carriedCommand = argv.slice(1).find((arg) => { - const normalized = normalizeExecutableToken(arg); - return normalized === "eval" || SOURCE_EXECUTABLES.has(normalized); + const carrierShellWrapper = !shellWrapper.isWrapper + ? detectShellWrapperThroughCarrierArgv(argv, shellCommandFlag) + : null; + if (carrierShellWrapper) { + output.risks.push({ + kind: "shell-wrapper-through-carrier", + command: carrierShellWrapper, + text, + span, + }); + } + + const carriedShellBuiltin = detectCarriedShellBuiltinArgv(argv); + if (carriedShellBuiltin?.kind === "eval") { + output.risks.push({ kind: "eval", text, span }); + } else if (carriedShellBuiltin?.kind === "source") { + output.risks.push({ + kind: "source", + command: carriedShellBuiltin.command, + text, + span, }); - const normalizedCarriedCommand = carriedCommand - ? normalizeExecutableToken(carriedCommand) - : undefined; - if (normalizedCarriedCommand === "eval") { - output.risks.push({ kind: "eval", text, span }); - } else if (normalizedCarriedCommand && SOURCE_EXECUTABLES.has(normalizedCarriedCommand)) { - output.risks.push({ - kind: "source", - command: normalizedCarriedCommand, - text, - span, - }); - } } } diff --git a/src/infra/exec-approval-forwarder.test.ts b/src/infra/exec-approval-forwarder.test.ts index 55979176969..2e37d1a51de 100644 --- a/src/infra/exec-approval-forwarder.test.ts +++ b/src/infra/exec-approval-forwarder.test.ts @@ -4,7 +4,10 @@ import type { ChannelPlugin } from "../channels/plugins/types.js"; import type { OpenClawConfig } from "../config/config.js"; import { setActivePluginRegistry } from "../plugins/runtime.js"; import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js"; -import { createExecApprovalForwarder } from "./exec-approval-forwarder.js"; +import { + buildExecApprovalRequestMessage, + createExecApprovalForwarder, +} from "./exec-approval-forwarder.js"; const baseRequest = { id: "req-1", @@ -30,8 +33,9 @@ afterEach(() => { const emptyRegistry = createTestRegistry([]); async function flushPendingDelivery(): Promise { - await Promise.resolve(); - await Promise.resolve(); + for (let index = 0; index < 10; index += 1) { + await Promise.resolve(); + } } function isDiscordExecApprovalClientEnabledForTest(params: { @@ -570,6 +574,26 @@ describe("exec approval forwarder", () => { expect(text).toContain("Reply with: /approve allow-once|allow-always|deny"); }); + it("includes command analysis warnings in fallback delivery text", async () => { + const text = buildExecApprovalRequestMessage( + { + ...baseRequest, + request: { + ...baseRequest.request, + commandAnalysis: { + commandCount: 1, + nestedCommandCount: 0, + riskKinds: ["inline-eval"], + warningLines: ["Contains inline-eval: python3 -c"], + }, + }, + }, + 1000, + ); + expect(text).toContain("Command analysis:"); + expect(text).toContain("- Contains inline-eval: python3 -c"); + }); + it("omits allow-always from forwarded fallback text when ask=always", async () => { vi.useFakeTimers(); const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG }); diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index e439684ed05..f5bd68252f1 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -24,7 +24,10 @@ import { type DeliverableMessageChannel, } from "../utils/message-channel.js"; import { matchesApprovalRequestFilters } from "./approval-request-filters.js"; -import { resolveExecApprovalCommandDisplay } from "./exec-approval-command-display.js"; +import { + resolveExecApprovalCommandDisplay, + sanitizeExecApprovalWarningText, +} from "./exec-approval-command-display.js"; import { formatExecApprovalExpiresIn } from "./exec-approval-reply.js"; import { resolveExecApprovalRequestAllowedDecisions, @@ -226,7 +229,7 @@ function formatApprovalCommand(command: string): { inline: boolean; text: string return { inline: false, text: `${fence}\n${command}\n${fence}` }; } -function buildRequestMessage(request: ExecApprovalRequest, nowMs: number) { +export function buildExecApprovalRequestMessage(request: ExecApprovalRequest, nowMs: number) { const allowedDecisions = resolveExecApprovalRequestAllowedDecisions(request.request); const decisionText = allowedDecisions.join("|"); const lines: string[] = ["🔒 Exec approval required", `ID: ${request.id}`]; @@ -234,6 +237,16 @@ function buildRequestMessage(request: ExecApprovalRequest, nowMs: number) { if (warningText) { lines.push("", warningText); } + const analysisWarningLines = request.request.commandAnalysis?.warningLines + .map((line) => sanitizeExecApprovalWarningText(line).trim()) + .filter(Boolean) + .slice(0, 5); + if (analysisWarningLines && analysisWarningLines.length > 0) { + lines.push("", "Command analysis:"); + for (const line of analysisWarningLines) { + lines.push(`- ${line}`); + } + } const command = formatApprovalCommand( resolveExecApprovalCommandDisplay(request.request).commandText, ); @@ -407,7 +420,7 @@ function buildExecPendingPayload(params: { buildApprovalPendingReplyPayload({ approvalId: params.request.id, approvalSlug: params.request.id.slice(0, 8), - text: buildRequestMessage(params.request, params.nowMs), + text: buildExecApprovalRequestMessage(params.request, params.nowMs), agentId: params.request.request.agentId ?? null, allowedDecisions: resolveExecApprovalRequestAllowedDecisions(params.request.request), sessionKey: params.request.request.sessionKey ?? null, diff --git a/src/infra/exec-approvals-allowlist.ts b/src/infra/exec-approvals-allowlist.ts index 1410adc1016..c5b15639e9d 100644 --- a/src/infra/exec-approvals-allowlist.ts +++ b/src/infra/exec-approvals-allowlist.ts @@ -4,6 +4,7 @@ import { normalizeOptionalLowercaseString, normalizeOptionalString, } from "../shared/string-coerce.js"; +import { detectInlineEvalArgv } from "./command-analysis/risks.js"; import { isDispatchWrapperExecutable } from "./dispatch-wrapper-resolution.js"; import { analyzeShellCommand, @@ -22,10 +23,7 @@ import { type ShellChainOperator, } from "./exec-approvals-analysis.js"; import type { ExecAllowlistEntry } from "./exec-approvals.types.js"; -import { - detectInterpreterInlineEvalArgv, - isInterpreterLikeAllowlistPattern, -} from "./exec-inline-eval.js"; +import { isInterpreterLikeAllowlistPattern } from "./exec-inline-eval.js"; import { DEFAULT_SAFE_BINS, SAFE_BIN_PROFILES, @@ -964,10 +962,7 @@ function collectAllowAlwaysPatterns(params: { } if (isInterpreterLikeAllowlistPattern(candidatePath)) { const effectiveArgv = segment.resolution?.effectiveArgv ?? segment.argv; - if ( - params.strictInlineEval !== true || - detectInterpreterInlineEvalArgv(effectiveArgv) !== null - ) { + if (params.strictInlineEval !== true || detectInlineEvalArgv(effectiveArgv) !== null) { return; } } diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index e74d406acf4..04c0f207738 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -8,6 +8,7 @@ import { normalizeOptionalString, readStringValue, } from "../shared/string-coerce.js"; +import type { CommandExplanationSummary } from "./command-analysis/explain.js"; import { resolveAllowAlwaysPatternEntries } from "./exec-approvals-allowlist.js"; import type { ExecCommandSegment } from "./exec-approvals-analysis.js"; import type { ExecAllowlistEntry } from "./exec-approvals.types.js"; @@ -121,6 +122,7 @@ export type ExecApprovalRequestPayload = { security?: string | null; ask?: string | null; warningText?: string | null; + commandAnalysis?: CommandExplanationSummary | null; allowedDecisions?: readonly ExecApprovalDecision[]; agentId?: string | null; resolvedPath?: string | null; diff --git a/src/infra/exec-inline-eval.ts b/src/infra/exec-inline-eval.ts index 68fd8776fe2..db7ee1907d3 100644 --- a/src/infra/exec-inline-eval.ts +++ b/src/infra/exec-inline-eval.ts @@ -1,7 +1,7 @@ import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; import { normalizeExecutableToken } from "./exec-wrapper-resolution.js"; -type InterpreterInlineEvalHit = { +export type InterpreterInlineEvalHit = { executable: string; normalizedExecutable: string; flag: string; diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index b673eda1ef4..f5dbdee8023 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -1,6 +1,7 @@ import crypto from "node:crypto"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { GatewayClient } from "../gateway/client.js"; +import { detectPolicyInlineEval } from "../infra/command-analysis/policy.js"; import { addDurableCommandApproval, hasDurableExecApproval, @@ -16,7 +17,7 @@ import { import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js"; import { describeInterpreterInlineEval, - detectInterpreterInlineEvalArgv, + type InterpreterInlineEvalHit, } from "../infra/exec-inline-eval.js"; import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; import { @@ -106,7 +107,7 @@ type SystemRunPolicyPhase = SystemRunParsePhase & { policy: ReturnType; durableApprovalSatisfied: boolean; strictInlineEval: boolean; - inlineEvalHit: ReturnType; + inlineEvalHit: InterpreterInlineEvalHit | null; allowlistMatches: ExecAllowlistEntry[]; analysisOk: boolean; allowlistSatisfied: boolean; @@ -404,13 +405,7 @@ async function evaluateSystemRunPolicyPhase( }); const strictInlineEval = agentExec?.strictInlineEval === true || cfg.tools?.exec?.strictInlineEval === true; - const inlineEvalHit = strictInlineEval - ? (segments - .map((segment) => - detectInterpreterInlineEvalArgv(segment.resolution?.effectiveArgv ?? segment.argv), - ) - .find((entry) => entry !== null) ?? null) - : null; + const inlineEvalHit = strictInlineEval ? detectPolicyInlineEval(segments) : null; const isWindows = process.platform === "win32"; // Detect Windows wrapper transport from the same shell-wrapper view used to // derive the inner payload. That keeps `cmd.exe /c` approval-gated even when