Skip to content

Commit bd0e10a

Browse files
committed
refactor: route inline eval through command analysis
1 parent 99176e1 commit bd0e10a

15 files changed

Lines changed: 94 additions & 66 deletions

src/agents/bash-tools.exec-host-gateway.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ vi.mock("./bash-process-registry.js", () => ({
129129
tail: vi.fn((value) => value),
130130
}));
131131

132-
vi.mock("../infra/exec-inline-eval.js", () => ({
132+
vi.mock("../infra/command-analysis/inline-eval.js", () => ({
133133
describeInterpreterInlineEval: vi.fn(() => "python -c"),
134134
detectInterpreterInlineEvalArgv: detectInterpreterInlineEvalArgvMock,
135135
}));

src/agents/bash-tools.exec-host-gateway.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
2+
import { describeInterpreterInlineEval } from "../infra/command-analysis/inline-eval.js";
23
import { detectPolicyInlineEval } from "../infra/command-analysis/policy.js";
34
import {
45
addDurableCommandApproval,
@@ -13,7 +14,6 @@ import {
1314
resolveApprovalAuditCandidatePath,
1415
requiresExecApproval,
1516
} from "../infra/exec-approvals.js";
16-
import { describeInterpreterInlineEval } from "../infra/exec-inline-eval.js";
1717
import type { SafeBinProfile } from "../infra/exec-safe-bin-policy.js";
1818
import { markBackgrounded, tail } from "./bash-process-registry.js";
1919
import {

src/agents/bash-tools.exec-host-node-phases.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import crypto from "node:crypto";
22
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
3+
import {
4+
describeInterpreterInlineEval,
5+
type InterpreterInlineEvalHit,
6+
} from "../infra/command-analysis/inline-eval.js";
37
import { detectPolicyInlineEval } from "../infra/command-analysis/policy.js";
48
import {
59
type ExecApprovalsFile,
@@ -10,10 +14,6 @@ import {
1014
hasDurableExecApproval,
1115
resolveExecApprovalsFromFile,
1216
} from "../infra/exec-approvals.js";
13-
import {
14-
describeInterpreterInlineEval,
15-
type InterpreterInlineEvalHit,
16-
} from "../infra/exec-inline-eval.js";
1717
import { buildNodeShellCommand } from "../infra/node-shell.js";
1818
import { parsePreparedSystemRunPayload } from "../infra/system-run-approval-context.js";
1919
import { formatExecCommand, resolveSystemRunCommandRequest } from "../infra/system-run-command.js";

src/agents/bash-tools.exec-host-node.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ vi.mock("../infra/exec-approvals.js", () => ({
9292
})),
9393
}));
9494

95-
vi.mock("../infra/exec-inline-eval.js", () => ({
95+
vi.mock("../infra/command-analysis/inline-eval.js", () => ({
9696
describeInterpreterInlineEval: vi.fn(() => "inline-eval"),
9797
detectInterpreterInlineEvalArgv: detectInterpreterInlineEvalArgvMock,
9898
}));

src/gateway/server-methods/exec-approval.ts

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import {
2-
summarizeCommandSegmentsForDisplay,
3-
type CommandExplanationSummary,
4-
} from "../../infra/command-analysis/explain.js";
5-
import { analyzeCommandForPolicy } from "../../infra/command-analysis/policy.js";
1+
import { resolveCommandAnalysisSummaryForDisplay } from "../../infra/command-analysis/explain.js";
62
import {
73
resolveExecApprovalCommandDisplay,
84
sanitizeExecApprovalDisplayText,
@@ -47,43 +43,6 @@ const APPROVAL_ALLOW_ALWAYS_UNAVAILABLE_DETAILS = {
4743
} as const;
4844
const RESERVED_PLUGIN_APPROVAL_ID_PREFIX = "plugin:";
4945

50-
function sanitizeCommandAnalysisSummary(
51-
summary: CommandExplanationSummary,
52-
): CommandExplanationSummary {
53-
return {
54-
commandCount: summary.commandCount,
55-
nestedCommandCount: summary.nestedCommandCount,
56-
riskKinds: summary.riskKinds.map((kind) => sanitizeExecApprovalWarningText(kind)),
57-
warningLines: summary.warningLines.map((line) => sanitizeExecApprovalWarningText(line)),
58-
};
59-
}
60-
61-
function resolveExecApprovalCommandAnalysis(params: {
62-
host: string;
63-
commandText: string;
64-
commandArgv?: string[];
65-
cwd?: string | null;
66-
}): CommandExplanationSummary | null {
67-
const analysis =
68-
Array.isArray(params.commandArgv) && params.commandArgv.length > 0
69-
? analyzeCommandForPolicy({
70-
source: "argv",
71-
argv: params.commandArgv,
72-
cwd: params.cwd ?? undefined,
73-
})
74-
: params.host === "node"
75-
? null
76-
: analyzeCommandForPolicy({
77-
source: "shell",
78-
command: params.commandText,
79-
cwd: params.cwd ?? undefined,
80-
});
81-
if (!analysis?.ok) {
82-
return null;
83-
}
84-
return sanitizeCommandAnalysisSummary(summarizeCommandSegmentsForDisplay(analysis.segments));
85-
}
86-
8746
type ExecApprovalIosPushDelivery = {
8847
handleRequested?: (request: ExecApprovalRequest) => Promise<boolean>;
8948
handleResolved?: (resolved: ExecApprovalResolved) => Promise<void>;
@@ -249,11 +208,12 @@ export function createExecApprovalHandlers(
249208
}
250209
const envBinding = buildSystemRunApprovalEnvBinding(p.env);
251210
const warningText = normalizeOptionalString(p.warningText);
252-
const commandAnalysis = resolveExecApprovalCommandAnalysis({
211+
const commandAnalysis = resolveCommandAnalysisSummaryForDisplay({
253212
host,
254213
commandText: effectiveCommandText,
255214
commandArgv: effectiveCommandArgv,
256215
cwd: effectiveCwd,
216+
sanitizeText: sanitizeExecApprovalWarningText,
257217
});
258218
const systemRunBinding =
259219
host === "node"

src/infra/command-analysis/explain.test.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { describe, expect, it } from "vitest";
22
import { explainShellCommand } from "../command-explainer/index.js";
3-
import { summarizeCommandExplanation, summarizeCommandSegmentsForDisplay } from "./explain.js";
3+
import {
4+
resolveCommandAnalysisSummaryForDisplay,
5+
summarizeCommandExplanation,
6+
summarizeCommandSegmentsForDisplay,
7+
} from "./explain.js";
48

59
describe("command-analysis explanation summary", () => {
610
it("summarizes commands and risk kinds", async () => {
@@ -26,4 +30,33 @@ describe("command-analysis explanation summary", () => {
2630
expect(summary.riskKinds).toEqual(["inline-eval"]);
2731
expect(summary.warningLines).toEqual(["Contains inline-eval: python3 -c"]);
2832
});
33+
34+
it("resolves display summaries from argv or shell commands", () => {
35+
expect(
36+
resolveCommandAnalysisSummaryForDisplay({
37+
host: "gateway",
38+
commandText: "echo ok",
39+
commandArgv: ["python3", "-c", "print(1)"],
40+
}),
41+
).toEqual(
42+
expect.objectContaining({
43+
commandCount: 1,
44+
riskKinds: ["inline-eval"],
45+
warningLines: ["Contains inline-eval: python3 -c"],
46+
}),
47+
);
48+
expect(
49+
resolveCommandAnalysisSummaryForDisplay({
50+
host: "node",
51+
commandText: "python3 -c 'print(1)'",
52+
}),
53+
).toBeNull();
54+
expect(
55+
resolveCommandAnalysisSummaryForDisplay({
56+
host: "gateway",
57+
commandText: "python3 -c 'print(1)'",
58+
sanitizeText: (value) => value.replaceAll("python3", "python"),
59+
})?.warningLines,
60+
).toEqual(["Contains inline-eval: python -c"]);
61+
});
2962
});

src/infra/command-analysis/explain.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { explainShellCommand } from "../command-explainer/extract.js";
22
import type { CommandExplanation, CommandRisk } from "../command-explainer/types.js";
33
import type { ExecCommandSegment } from "../exec-approvals-analysis.js";
4+
import { analyzeCommandForPolicy } from "./policy.js";
45
import { detectCommandCarrierArgv, detectInlineEvalInSegments } from "./risks.js";
56

67
export type CommandExplanationSummary = {
@@ -80,6 +81,43 @@ export function summarizeCommandSegmentsForDisplay(
8081
};
8182
}
8283

84+
export function resolveCommandAnalysisSummaryForDisplay(params: {
85+
host?: string | null;
86+
commandText: string;
87+
commandArgv?: string[];
88+
cwd?: string | null;
89+
sanitizeText?: (value: string) => string;
90+
}): CommandExplanationSummary | null {
91+
const analysis =
92+
Array.isArray(params.commandArgv) && params.commandArgv.length > 0
93+
? analyzeCommandForPolicy({
94+
source: "argv",
95+
argv: params.commandArgv,
96+
cwd: params.cwd ?? undefined,
97+
})
98+
: params.host === "node"
99+
? null
100+
: analyzeCommandForPolicy({
101+
source: "shell",
102+
command: params.commandText,
103+
cwd: params.cwd ?? undefined,
104+
});
105+
if (!analysis?.ok) {
106+
return null;
107+
}
108+
const summary = summarizeCommandSegmentsForDisplay(analysis.segments);
109+
const sanitizeText = params.sanitizeText;
110+
if (!sanitizeText) {
111+
return summary;
112+
}
113+
return {
114+
commandCount: summary.commandCount,
115+
nestedCommandCount: summary.nestedCommandCount,
116+
riskKinds: summary.riskKinds.map((kind) => sanitizeText(kind)),
117+
warningLines: summary.warningLines.map((line) => sanitizeText(line)),
118+
};
119+
}
120+
83121
export async function explainCommandForDisplay(
84122
command: string,
85123
): Promise<{ explanation: CommandExplanation; summary: CommandExplanationSummary } | null> {

src/infra/exec-inline-eval.test.ts renamed to src/infra/command-analysis/inline-eval.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
describeInterpreterInlineEval,
44
detectInterpreterInlineEvalArgv,
55
isInterpreterLikeAllowlistPattern,
6-
} from "./exec-inline-eval.js";
6+
} from "./inline-eval.js";
77

88
describe("exec inline eval detection", () => {
99
it.each([
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
2-
import { normalizeExecutableToken } from "./exec-wrapper-resolution.js";
1+
import { normalizeLowercaseStringOrEmpty } from "../../shared/string-coerce.js";
2+
import { normalizeExecutableToken } from "../exec-wrapper-resolution.js";
33

44
export type InterpreterInlineEvalHit = {
55
executable: string;

src/infra/command-analysis/risks.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
import { splitShellArgs } from "../../utils/shell-argv.js";
22
import { unwrapKnownDispatchWrapperInvocation } from "../dispatch-wrapper-resolution.js";
33
import type { ExecCommandSegment } from "../exec-approvals-analysis.js";
4-
import {
5-
detectInterpreterInlineEvalArgv,
6-
type InterpreterInlineEvalHit,
7-
} from "../exec-inline-eval.js";
84
import { normalizeExecutableToken } from "../exec-wrapper-resolution.js";
95
import {
106
extractShellWrapperInlineCommand,
117
isShellWrapperExecutable,
128
} from "../shell-wrapper-resolution.js";
9+
import { detectInterpreterInlineEvalArgv, type InterpreterInlineEvalHit } from "./inline-eval.js";
1310

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

0 commit comments

Comments
 (0)