LLM/moltbot
1
0
Fork 0
mirror of https://github.com/moltbot/moltbot.git synced 2026-04-23 22:55:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(security): simplify system.run approval model

Browse Source
This commit is contained in:
Peter Steinberger
2026-03-11 01:42:47 +00:00
parent 5716e52417
commit 68c674d37c
32 changed files with 332 additions and 207 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/node-host/invoke-system-run-plan.test.ts
View File

@@ -101,7 +101,7 @@ describe("hardenApprovedExecutionPaths", () => {
mode: "build-plan",
argv: ["env", "sh", "-c", "echo SAFE"],
expectedArgv: () => ["env", "sh", "-c", "echo SAFE"],
expectedCmdText: "echo SAFE",
expectedCmdText: 'env sh -c "echo SAFE"',
expectedCommandPreview: "echo SAFE",
},
{
@@ -144,7 +144,7 @@ describe("hardenApprovedExecutionPaths", () => {
mode: "build-plan",
argv: ["./env", "sh", "-c", "echo SAFE"],
expectedArgv: () => ["./env", "sh", "-c", "echo SAFE"],
expectedCmdText: "echo SAFE",
expectedCmdText: './env sh -c "echo SAFE"',
checkRawCommandMatchesArgv: true,
expectedCommandPreview: "echo SAFE",
},
@@ -175,10 +175,10 @@ describe("hardenApprovedExecutionPaths", () => {
}
expect(prepared.plan.argv).toEqual(testCase.expectedArgv({ pathToken }));
if (testCase.expectedCmdText) {
expect(prepared.cmdText).toBe(testCase.expectedCmdText);
expect(prepared.plan.commandText).toBe(testCase.expectedCmdText);
}
if (testCase.checkRawCommandMatchesArgv) {
expect(prepared.plan.rawCommand).toBe(formatExecCommand(prepared.plan.argv));
expect(prepared.plan.commandText).toBe(formatExecCommand(prepared.plan.argv));
}
if ("expectedCommandPreview" in testCase) {
expect(prepared.plan.commandPreview ?? null).toBe(testCase.expectedCommandPreview);

15
src/node-host/invoke-system-run-plan.ts
View File

@@ -17,7 +17,7 @@ import {
POSIX_INLINE_COMMAND_FLAGS,
resolveInlineCommandMatch,
} from "../infra/shell-inline-command.js";
import { formatExecCommand, resolveSystemRunCommand } from "../infra/system-run-command.js";
import { formatExecCommand, resolveSystemRunCommandRequest } from "../infra/system-run-command.js";
export type ApprovedCwdSnapshot = {
cwd: string;
@@ -630,8 +630,8 @@ export function buildSystemRunApprovalPlan(params: {
cwd?: unknown;
agentId?: unknown;
sessionKey?: unknown;
}): { ok: true; plan: SystemRunApprovalPlan; cmdText: string } | { ok: false; message: string } {
const command = resolveSystemRunCommand({
}): { ok: true; plan: SystemRunApprovalPlan } | { ok: false; message: string } {
const command = resolveSystemRunCommandRequest({
command: params.command,
rawCommand: params.rawCommand,
});
@@ -644,15 +644,15 @@ export function buildSystemRunApprovalPlan(params: {
const hardening = hardenApprovedExecutionPaths({
approvedByAsk: true,
argv: command.argv,
shellCommand: command.shellCommand,
shellCommand: command.shellPayload,
cwd: normalizeString(params.cwd) ?? undefined,
});
if (!hardening.ok) {
return { ok: false, message: hardening.message };
}
const rawCommand = formatExecCommand(hardening.argv) || null;
const commandText = formatExecCommand(hardening.argv);
const commandPreview =
command.previewText?.trim() && command.previewText.trim() !== rawCommand
command.previewText?.trim() && command.previewText.trim() !== commandText
? command.previewText.trim()
: null;
const mutableFileOperand = resolveMutableFileOperandSnapshotSync({
@@ -667,12 +667,11 @@ export function buildSystemRunApprovalPlan(params: {
plan: {
argv: hardening.argv,
cwd: hardening.cwd ?? null,
rawCommand,
commandText,
commandPreview,
agentId: normalizeString(params.agentId),
sessionKey: normalizeString(params.sessionKey),
mutableFileOperand: mutableFileOperand.snapshot ?? undefined,
},
cmdText: commandPreview ?? rawCommand ?? formatExecCommand(hardening.argv),
};
}

14
src/node-host/invoke-system-run.test.ts
View File

@@ -432,7 +432,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
expectInvokeOk(sendInvokeResult, { payloadContains: "app-ok" });
});
it("forwards canonical cmdText to mac app exec host for positional-argv shell wrappers", async () => {
it("forwards canonical command text to mac app exec host for positional-argv shell wrappers", async () => {
const { runViaMacAppExecHost } = await runSystemInvoke({
preferMacAppExecHost: true,
command: ["/bin/sh", "-lc", '$0 "$1"', "/usr/bin/touch", "/tmp/marker"],
@@ -505,7 +505,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
approvals: expect.anything(),
request: expect.objectContaining({
command: ["env", "sh", "-c", "echo SAFE"],
rawCommand: "echo SAFE",
rawCommand: 'env sh -c "echo SAFE"',
cwd: canonicalCwd,
}),
});
@@ -593,7 +593,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
const { runCommand, sendInvokeResult } = await runSystemInvoke({
preferMacAppExecHost: false,
command: prepared.plan.argv,
rawCommand: prepared.plan.rawCommand,
rawCommand: prepared.plan.commandText,
approved: true,
security: "full",
ask: "off",
@@ -789,7 +789,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
const { runCommand, sendInvokeResult } = await runSystemInvoke({
preferMacAppExecHost: false,
command: prepared.plan.argv,
rawCommand: prepared.plan.rawCommand,
rawCommand: prepared.plan.commandText,
systemRunPlan: prepared.plan,
cwd: prepared.plan.cwd ?? tmp,
approved: true,
@@ -827,7 +827,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
const { runCommand, sendInvokeResult } = await runSystemInvoke({
preferMacAppExecHost: false,
command: prepared.plan.argv,
rawCommand: prepared.plan.rawCommand,
rawCommand: prepared.plan.commandText,
systemRunPlan: prepared.plan,
cwd: prepared.plan.cwd ?? tmp,
approved: true,
@@ -866,7 +866,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
const { runCommand, sendInvokeResult } = await runSystemInvoke({
preferMacAppExecHost: false,
command: prepared.plan.argv,
rawCommand: prepared.plan.rawCommand,
rawCommand: prepared.plan.commandText,
systemRunPlan: prepared.plan,
cwd: prepared.plan.cwd ?? tmp,
approved: true,
@@ -908,7 +908,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
const { runCommand, sendInvokeResult } = await runSystemInvoke({
preferMacAppExecHost: false,
command: prepared.plan.argv,
rawCommand: prepared.plan.rawCommand,
rawCommand: prepared.plan.commandText,
systemRunPlan: prepared.plan,
cwd: prepared.plan.cwd ?? tmp,
approved: true,

48
src/node-host/invoke-system-run.ts
View File

@@ -16,7 +16,7 @@ import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../in
import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
import { sanitizeSystemRunEnvOverrides } from "../infra/host-env-security.js";
import { normalizeSystemRunApprovalPlan } from "../infra/system-run-approval-binding.js";
import { resolveSystemRunCommand } from "../infra/system-run-command.js";
import { resolveSystemRunCommandRequest } from "../infra/system-run-command.js";
import { logWarn } from "../logger.js";
import { evaluateSystemRunPolicy, resolveExecApprovalDecision } from "./exec-policy.js";
import {
@@ -56,7 +56,7 @@ type SystemRunDeniedReason =
type SystemRunExecutionContext = {
sessionKey: string;
runId: string;
cmdText: string;
commandText: string;
suppressNotifyOnExit: boolean;
};
@@ -64,8 +64,9 @@ type ResolvedExecApprovals = ReturnType<typeof resolveExecApprovals>;
type SystemRunParsePhase = {
argv: string[];
shellCommand: string | null;
cmdText: string;
shellPayload: string | null;
commandText: string;
commandPreview: string | null;
approvalPlan: import("../infra/exec-approvals.js").SystemRunApprovalPlan | null;
agentId: string | undefined;
sessionKey: string;
@@ -167,7 +168,7 @@ async function sendSystemRunDenied(
sessionKey: execution.sessionKey,
runId: execution.runId,
host: "node",
command: execution.cmdText,
command: execution.commandText,
reason: params.reason,
suppressNotifyOnExit: execution.suppressNotifyOnExit,
}),
@@ -184,7 +185,7 @@ export { buildSystemRunApprovalPlan } from "./invoke-system-run-plan.js";
async function parseSystemRunPhase(
opts: HandleSystemRunInvokeOptions,
): Promise<SystemRunParsePhase | null> {
const command = resolveSystemRunCommand({
const command = resolveSystemRunCommandRequest({
command: opts.params.command,
rawCommand: opts.params.rawCommand,
});
@@ -203,8 +204,8 @@ async function parseSystemRunPhase(
return null;
}
const shellCommand = command.shellCommand;
const cmdText = command.cmdText;
const shellPayload = command.shellPayload;
const commandText = command.commandText;
const approvalPlan =
opts.params.systemRunPlan === undefined
? null
@@ -222,17 +223,18 @@ async function parseSystemRunPhase(
const suppressNotifyOnExit = opts.params.suppressNotifyOnExit === true;
const envOverrides = sanitizeSystemRunEnvOverrides({
overrides: opts.params.env ?? undefined,
shellWrapper: shellCommand !== null,
shellWrapper: shellPayload !== null,
});
return {
argv: command.argv,
shellCommand,
cmdText,
shellPayload,
commandText,
commandPreview: command.previewText,
approvalPlan,
agentId,
sessionKey,
runId,
execution: { sessionKey, runId, cmdText, suppressNotifyOnExit },
execution: { sessionKey, runId, commandText, suppressNotifyOnExit },
approvalDecision: resolveExecApprovalDecision(opts.params.approvalDecision),
envOverrides,
env: opts.sanitizeEnv(envOverrides),
@@ -270,7 +272,7 @@ async function evaluateSystemRunPolicyPhase(
});
const bins = autoAllowSkills ? await opts.skillBins.current() : [];
let { analysisOk, allowlistMatches, allowlistSatisfied, segments } = evaluateSystemRunAllowlist({
shellCommand: parsed.shellCommand,
shellCommand: parsed.shellPayload,
argv: parsed.argv,
approvals,
security,
@@ -283,7 +285,7 @@ async function evaluateSystemRunPolicyPhase(
autoAllowSkills,
});
const isWindows = process.platform === "win32";
const cmdInvocation = parsed.shellCommand
const cmdInvocation = parsed.shellPayload
? opts.isCmdExeInvocation(segments[0]?.argv ?? [])
: opts.isCmdExeInvocation(parsed.argv);
const policy = evaluateSystemRunPolicy({
@@ -295,7 +297,7 @@ async function evaluateSystemRunPolicyPhase(
approved: parsed.approved,
isWindows,
cmdInvocation,
shellWrapperInvocation: parsed.shellCommand !== null,
shellWrapperInvocation: parsed.shellPayload !== null,
});
analysisOk = policy.analysisOk;
allowlistSatisfied = policy.allowlistSatisfied;
@@ -308,7 +310,7 @@ async function evaluateSystemRunPolicyPhase(
}
// Fail closed if policy/runtime drift re-allows unapproved shell wrappers.
if (security === "allowlist" && parsed.shellCommand && !policy.approvedByAsk) {
if (security === "allowlist" && parsed.shellPayload && !policy.approvedByAsk) {
await sendSystemRunDenied(opts, parsed.execution, {
reason: "approval-required",
message: "SYSTEM_RUN_DENIED: approval required",
@@ -319,7 +321,7 @@ async function evaluateSystemRunPolicyPhase(
const hardenedPaths = hardenApprovedExecutionPaths({
approvedByAsk: policy.approvedByAsk,
argv: parsed.argv,
shellCommand: parsed.shellCommand,
shellCommand: parsed.shellPayload,
cwd: parsed.cwd,
});
if (!hardenedPaths.ok) {
@@ -340,7 +342,7 @@ async function evaluateSystemRunPolicyPhase(
const plannedAllowlistArgv = resolvePlannedAllowlistArgv({
security,
shellCommand: parsed.shellCommand,
shellCommand: parsed.shellPayload,
policy,
segments,
});
@@ -405,7 +407,7 @@ async function executeSystemRunPhase(
command: phase.plannedAllowlistArgv ?? phase.argv,
// Forward canonical display text so companion approval/prompt surfaces bind to
// the exact command context already validated on the node-host.
rawCommand: phase.cmdText || null,
rawCommand: phase.commandText || null,
cwd: phase.cwd ?? null,
env: phase.envOverrides ?? null,
timeoutMs: phase.timeoutMs ?? null,
@@ -437,7 +439,7 @@ async function executeSystemRunPhase(
await opts.sendExecFinishedEvent({
sessionKey: phase.sessionKey,
runId: phase.runId,
cmdText: phase.cmdText,
commandText: phase.commandText,
result,
suppressNotifyOnExit: phase.suppressNotifyOnExit,
});
@@ -476,7 +478,7 @@ async function executeSystemRunPhase(
phase.approvals.file,
phase.agentId,
match,
phase.cmdText,
phase.commandText,
phase.segments[0]?.resolution?.resolvedPath,
);
}
@@ -496,7 +498,7 @@ async function executeSystemRunPhase(
security: phase.security,
isWindows: phase.isWindows,
policy: phase.policy,
shellCommand: phase.shellCommand,
shellCommand: phase.shellPayload,
segments: phase.segments,
});
@@ -505,7 +507,7 @@ async function executeSystemRunPhase(
await opts.sendExecFinishedEvent({
sessionKey: phase.sessionKey,
runId: phase.runId,
cmdText: phase.cmdText,
commandText: phase.commandText,
result,
suppressNotifyOnExit: phase.suppressNotifyOnExit,
});

2
src/node-host/invoke-types.ts
View File

@@ -51,7 +51,7 @@ export type ExecFinishedResult = {
export type ExecFinishedEventParams = {
sessionKey: string;
runId: string;
cmdText: string;
commandText: string;
result: ExecFinishedResult;
suppressNotifyOnExit?: boolean;
};

7
src/node-host/invoke.ts
View File

@@ -350,7 +350,7 @@ async function sendExecFinishedEvent(
sessionKey: params.sessionKey,
runId: params.runId,
host: "node",
command: params.cmdText,
command: params.commandText,
exitCode: params.result.exitCode ?? undefined,
timedOut: params.result.timedOut,
success: params.result.success,
@@ -505,7 +505,6 @@ export async function handleInvoke(
return;
}
await sendJsonPayloadResult(client, frame, {
cmdText: prepared.cmdText,
plan: prepared.plan,
});
} catch (err) {
@@ -549,8 +548,8 @@ export async function handleInvoke(
sendInvokeResult: async (result) => {
await sendInvokeResult(client, frame, result);
},
sendExecFinishedEvent: async ({ sessionKey, runId, cmdText, result }) => {
await sendExecFinishedEvent({ client, sessionKey, runId, cmdText, result });
sendExecFinishedEvent: async ({ sessionKey, runId, commandText, result }) => {
await sendExecFinishedEvent({ client, sessionKey, runId, commandText, result });
},
preferMacAppExecHost,
});