diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 16c5a46ea5d..dace3aeffeb 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import type { ExecHostResponse } from "../infra/exec-host.js"; import { handleSystemRunInvoke, formatSystemRunAllowlistMissMessage } from "./invoke-system-run.js"; @@ -147,4 +150,67 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { }), ); }); + + it("denies ./sh wrapper spoof in allowlist on-miss mode before execution", async () => { + const marker = path.join(os.tmpdir(), `openclaw-wrapper-spoof-${process.pid}-${Date.now()}`); + const runCommand = vi.fn(async () => { + fs.writeFileSync(marker, "executed"); + return { + success: true, + stdout: "local-ok", + stderr: "", + timedOut: false, + truncated: false, + exitCode: 0, + error: null, + }; + }); + const sendInvokeResult = vi.fn(async () => {}); + const sendNodeEvent = vi.fn(async () => {}); + + await handleSystemRunInvoke({ + client: {} as never, + params: { + command: ["./sh", "-lc", "/bin/echo approved-only"], + sessionKey: "agent:main:main", + }, + skillBins: { + current: async () => new Set(), + }, + execHostEnforced: false, + execHostFallbackAllowed: true, + resolveExecSecurity: () => "allowlist", + resolveExecAsk: () => "on-miss", + isCmdExeInvocation: () => false, + sanitizeEnv: () => undefined, + runCommand, + runViaMacAppExecHost: vi.fn(async () => null), + sendNodeEvent, + buildExecEventPayload: (payload) => payload, + sendInvokeResult, + sendExecFinishedEvent: vi.fn(async () => {}), + preferMacAppExecHost: false, + }); + + expect(runCommand).not.toHaveBeenCalled(); + expect(fs.existsSync(marker)).toBe(false); + expect(sendNodeEvent).toHaveBeenCalledWith( + expect.anything(), + "exec.denied", + expect.objectContaining({ reason: "approval-required" }), + ); + expect(sendInvokeResult).toHaveBeenCalledWith( + expect.objectContaining({ + ok: false, + error: expect.objectContaining({ + message: "SYSTEM_RUN_DENIED: approval required", + }), + }), + ); + try { + fs.unlinkSync(marker); + } catch { + // no-op + } + }); }); diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 8ce09c8ec68..aeef1522fcc 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -32,9 +32,43 @@ type SystemRunInvokeResult = { payloadJSON?: string | null; error?: { code?: string; message?: string } | null; }; -export { formatSystemRunAllowlistMissMessage } from "./exec-policy.js"; -export async function handleSystemRunInvoke(opts: { +type SystemRunDeniedReason = + | "security=deny" + | "approval-required" + | "allowlist-miss" + | "execution-plan-miss" + | "companion-unavailable" + | "permission:screenRecording"; + +type SystemRunExecutionContext = { + sessionKey: string; + runId: string; + cmdText: string; +}; + +type SystemRunAllowlistAnalysis = { + analysisOk: boolean; + allowlistMatches: ExecAllowlistEntry[]; + allowlistSatisfied: boolean; + segments: ExecCommandSegment[]; +}; + +function normalizeDeniedReason(reason: string | null | undefined): SystemRunDeniedReason { + switch (reason) { + case "security=deny": + case "approval-required": + case "allowlist-miss": + case "execution-plan-miss": + case "companion-unavailable": + case "permission:screenRecording": + return reason; + default: + return "approval-required"; + } +} + +export type HandleSystemRunInvokeOptions = { client: GatewayClient; params: SystemRunParams; skillBins: SkillBinsProvider; @@ -71,7 +105,161 @@ export async function handleSystemRunInvoke(opts: { }; }) => Promise; preferMacAppExecHost: boolean; -}): Promise { +}; + +async function sendSystemRunDenied( + opts: Pick< + HandleSystemRunInvokeOptions, + "client" | "sendNodeEvent" | "buildExecEventPayload" | "sendInvokeResult" + >, + execution: SystemRunExecutionContext, + params: { + reason: SystemRunDeniedReason; + message: string; + }, +) { + await opts.sendNodeEvent( + opts.client, + "exec.denied", + opts.buildExecEventPayload({ + sessionKey: execution.sessionKey, + runId: execution.runId, + host: "node", + command: execution.cmdText, + reason: params.reason, + }), + ); + await opts.sendInvokeResult({ + ok: false, + error: { code: "UNAVAILABLE", message: params.message }, + }); +} + +function evaluateSystemRunAllowlist(params: { + shellCommand: string | null; + argv: string[]; + approvals: ReturnType; + security: ExecSecurity; + safeBins: ReturnType["safeBins"]; + safeBinProfiles: ReturnType["safeBinProfiles"]; + trustedSafeBinDirs: ReturnType["trustedSafeBinDirs"]; + cwd: string | undefined; + env: Record | undefined; + skillBins: Set; + autoAllowSkills: boolean; +}): SystemRunAllowlistAnalysis { + if (params.shellCommand) { + const allowlistEval = evaluateShellAllowlist({ + command: params.shellCommand, + allowlist: params.approvals.allowlist, + safeBins: params.safeBins, + safeBinProfiles: params.safeBinProfiles, + cwd: params.cwd, + env: params.env, + trustedSafeBinDirs: params.trustedSafeBinDirs, + skillBins: params.skillBins, + autoAllowSkills: params.autoAllowSkills, + platform: process.platform, + }); + return { + analysisOk: allowlistEval.analysisOk, + allowlistMatches: allowlistEval.allowlistMatches, + allowlistSatisfied: + params.security === "allowlist" && allowlistEval.analysisOk + ? allowlistEval.allowlistSatisfied + : false, + segments: allowlistEval.segments, + }; + } + + const analysis = analyzeArgvCommand({ argv: params.argv, cwd: params.cwd, env: params.env }); + const allowlistEval = evaluateExecAllowlist({ + analysis, + allowlist: params.approvals.allowlist, + safeBins: params.safeBins, + safeBinProfiles: params.safeBinProfiles, + cwd: params.cwd, + trustedSafeBinDirs: params.trustedSafeBinDirs, + skillBins: params.skillBins, + autoAllowSkills: params.autoAllowSkills, + }); + return { + analysisOk: analysis.ok, + allowlistMatches: allowlistEval.allowlistMatches, + allowlistSatisfied: + params.security === "allowlist" && analysis.ok ? allowlistEval.allowlistSatisfied : false, + segments: analysis.segments, + }; +} + +function resolvePlannedAllowlistArgv(params: { + security: ExecSecurity; + shellCommand: string | null; + policy: { + approvedByAsk: boolean; + analysisOk: boolean; + allowlistSatisfied: boolean; + }; + segments: ExecCommandSegment[]; +}): string[] | undefined | null { + if ( + params.security !== "allowlist" || + params.policy.approvedByAsk || + params.shellCommand || + !params.policy.analysisOk || + !params.policy.allowlistSatisfied || + params.segments.length !== 1 + ) { + return undefined; + } + const plannedAllowlistArgv = params.segments[0]?.resolution?.effectiveArgv; + return plannedAllowlistArgv && plannedAllowlistArgv.length > 0 ? plannedAllowlistArgv : null; +} + +function resolveSystemRunExecArgv(params: { + plannedAllowlistArgv: string[] | undefined; + argv: string[]; + security: ExecSecurity; + isWindows: boolean; + policy: { + approvedByAsk: boolean; + analysisOk: boolean; + allowlistSatisfied: boolean; + }; + shellCommand: string | null; + segments: ExecCommandSegment[]; +}): string[] { + let execArgv = params.plannedAllowlistArgv ?? params.argv; + if ( + params.security === "allowlist" && + params.isWindows && + !params.policy.approvedByAsk && + params.shellCommand && + params.policy.analysisOk && + params.policy.allowlistSatisfied && + params.segments.length === 1 && + params.segments[0]?.argv.length > 0 + ) { + execArgv = params.segments[0].argv; + } + return execArgv; +} + +function applyOutputTruncation(result: RunResult) { + if (!result.truncated) { + return; + } + const suffix = "... (truncated)"; + if (result.stderr.trim().length > 0) { + result.stderr = `${result.stderr}\n${suffix}`; + } else { + result.stdout = `${result.stdout}\n${suffix}`; + } +} + +export { formatSystemRunAllowlistMissMessage } from "./exec-policy.js"; + +export async function handleSystemRunInvoke(opts: HandleSystemRunInvokeOptions): Promise { const command = resolveSystemRunCommand({ command: opts.params.command, rawCommand: opts.params.rawCommand, @@ -111,6 +299,7 @@ export async function handleSystemRunInvoke(opts: { const autoAllowSkills = approvals.agent.autoAllowSkills; const sessionKey = opts.params.sessionKey?.trim() || "node"; const runId = opts.params.runId?.trim() || crypto.randomUUID(); + const execution: SystemRunExecutionContext = { sessionKey, runId, cmdText }; const approvalDecision = resolveExecApprovalDecision(opts.params.approvalDecision); const envOverrides = sanitizeSystemRunEnvOverrides({ overrides: opts.params.env ?? undefined, @@ -122,46 +311,19 @@ export async function handleSystemRunInvoke(opts: { local: agentExec, }); const bins = autoAllowSkills ? await opts.skillBins.current() : new Set(); - let analysisOk = false; - let allowlistMatches: ExecAllowlistEntry[] = []; - let allowlistSatisfied = false; - let segments: ExecCommandSegment[] = []; - if (shellCommand) { - const allowlistEval = evaluateShellAllowlist({ - command: shellCommand, - allowlist: approvals.allowlist, - safeBins, - safeBinProfiles, - cwd: opts.params.cwd ?? undefined, - env, - trustedSafeBinDirs, - skillBins: bins, - autoAllowSkills, - platform: process.platform, - }); - analysisOk = allowlistEval.analysisOk; - allowlistMatches = allowlistEval.allowlistMatches; - allowlistSatisfied = - security === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false; - segments = allowlistEval.segments; - } else { - const analysis = analyzeArgvCommand({ argv, cwd: opts.params.cwd ?? undefined, env }); - const allowlistEval = evaluateExecAllowlist({ - analysis, - allowlist: approvals.allowlist, - safeBins, - safeBinProfiles, - cwd: opts.params.cwd ?? undefined, - trustedSafeBinDirs, - skillBins: bins, - autoAllowSkills, - }); - analysisOk = analysis.ok; - allowlistMatches = allowlistEval.allowlistMatches; - allowlistSatisfied = - security === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false; - segments = analysis.segments; - } + let { analysisOk, allowlistMatches, allowlistSatisfied, segments } = evaluateSystemRunAllowlist({ + shellCommand, + argv, + approvals, + security, + safeBins, + safeBinProfiles, + trustedSafeBinDirs, + cwd: opts.params.cwd ?? undefined, + env, + skillBins: bins, + autoAllowSkills, + }); const isWindows = process.platform === "win32"; const cmdInvocation = shellCommand ? opts.isCmdExeInvocation(segments[0]?.argv ?? []) @@ -180,52 +342,34 @@ export async function handleSystemRunInvoke(opts: { analysisOk = policy.analysisOk; allowlistSatisfied = policy.allowlistSatisfied; if (!policy.allowed) { - await opts.sendNodeEvent( - opts.client, - "exec.denied", - opts.buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: policy.eventReason, - }), - ); - await opts.sendInvokeResult({ - ok: false, - error: { code: "UNAVAILABLE", message: policy.errorMessage }, + await sendSystemRunDenied(opts, execution, { + reason: policy.eventReason, + message: policy.errorMessage, }); return; } - let plannedAllowlistArgv: string[] | undefined; - if ( - security === "allowlist" && - !policy.approvedByAsk && - !shellCommand && - policy.analysisOk && - policy.allowlistSatisfied && - segments.length === 1 - ) { - plannedAllowlistArgv = segments[0]?.resolution?.effectiveArgv; - if (!plannedAllowlistArgv || plannedAllowlistArgv.length === 0) { - await opts.sendNodeEvent( - opts.client, - "exec.denied", - opts.buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: "execution-plan-miss", - }), - ); - await opts.sendInvokeResult({ - ok: false, - error: { code: "UNAVAILABLE", message: "SYSTEM_RUN_DENIED: execution plan mismatch" }, - }); - return; - } + // Fail closed if policy/runtime drift re-allows unapproved shell wrappers. + if (security === "allowlist" && shellCommand && !policy.approvedByAsk) { + await sendSystemRunDenied(opts, execution, { + reason: "approval-required", + message: "SYSTEM_RUN_DENIED: approval required", + }); + return; + } + + const plannedAllowlistArgv = resolvePlannedAllowlistArgv({ + security, + shellCommand, + policy, + segments, + }); + if (plannedAllowlistArgv === null) { + await sendSystemRunDenied(opts, execution, { + reason: "execution-plan-miss", + message: "SYSTEM_RUN_DENIED: execution plan mismatch", + }); + return; } const useMacAppExec = opts.preferMacAppExecHost; @@ -244,42 +388,16 @@ export async function handleSystemRunInvoke(opts: { const response = await opts.runViaMacAppExecHost({ approvals, request: execRequest }); if (!response) { if (opts.execHostEnforced || !opts.execHostFallbackAllowed) { - await opts.sendNodeEvent( - opts.client, - "exec.denied", - opts.buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: "companion-unavailable", - }), - ); - await opts.sendInvokeResult({ - ok: false, - error: { - code: "UNAVAILABLE", - message: "COMPANION_APP_UNAVAILABLE: macOS app exec host unreachable", - }, + await sendSystemRunDenied(opts, execution, { + reason: "companion-unavailable", + message: "COMPANION_APP_UNAVAILABLE: macOS app exec host unreachable", }); return; } } else if (!response.ok) { - const reason = response.error.reason ?? "approval-required"; - await opts.sendNodeEvent( - opts.client, - "exec.denied", - opts.buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason, - }), - ); - await opts.sendInvokeResult({ - ok: false, - error: { code: "UNAVAILABLE", message: response.error.message }, + await sendSystemRunDenied(opts, execution, { + reason: normalizeDeniedReason(response.error.reason), + message: response.error.message, }); return; } else { @@ -327,37 +445,22 @@ export async function handleSystemRunInvoke(opts: { } if (opts.params.needsScreenRecording === true) { - await opts.sendNodeEvent( - opts.client, - "exec.denied", - opts.buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: "permission:screenRecording", - }), - ); - await opts.sendInvokeResult({ - ok: false, - error: { code: "UNAVAILABLE", message: "PERMISSION_MISSING: screenRecording" }, + await sendSystemRunDenied(opts, execution, { + reason: "permission:screenRecording", + message: "PERMISSION_MISSING: screenRecording", }); return; } - let execArgv = plannedAllowlistArgv ?? argv; - if ( - security === "allowlist" && - isWindows && - !policy.approvedByAsk && - shellCommand && - policy.analysisOk && - policy.allowlistSatisfied && - segments.length === 1 && - segments[0]?.argv.length > 0 - ) { - execArgv = segments[0].argv; - } + const execArgv = resolveSystemRunExecArgv({ + plannedAllowlistArgv: plannedAllowlistArgv ?? undefined, + argv, + security, + isWindows, + policy, + shellCommand, + segments, + }); const result = await opts.runCommand( execArgv, @@ -365,14 +468,7 @@ export async function handleSystemRunInvoke(opts: { env, opts.params.timeoutMs ?? undefined, ); - if (result.truncated) { - const suffix = "... (truncated)"; - if (result.stderr.trim().length > 0) { - result.stderr = `${result.stderr}\n${suffix}`; - } else { - result.stdout = `${result.stdout}\n${suffix}`; - } - } + applyOutputTruncation(result); await opts.sendExecFinishedEvent({ sessionKey, runId, cmdText, result }); await opts.sendInvokeResult({