From 6007941f04df1edcca679dd6c95949744fdbd4df Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 11:49:13 +0100 Subject: [PATCH] fix(security): harden and refactor system.run command resolution --- .../node-invoke-system-run-approval.test.ts | 85 ++++ .../node-invoke-system-run-approval.ts | 78 ++-- src/infra/system-run-command.test.ts | 44 ++ src/infra/system-run-command.ts | 74 ++- src/node-host/invoke-system-run.ts | 422 ++++++++++++++++++ src/node-host/invoke.ts | 344 +------------- 6 files changed, 679 insertions(+), 368 deletions(-) create mode 100644 src/gateway/node-invoke-system-run-approval.test.ts create mode 100644 src/node-host/invoke-system-run.ts diff --git a/src/gateway/node-invoke-system-run-approval.test.ts b/src/gateway/node-invoke-system-run-approval.test.ts new file mode 100644 index 00000000000..e0bc8fdd4f4 --- /dev/null +++ b/src/gateway/node-invoke-system-run-approval.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, test } from "vitest"; +import type { ExecApprovalRecord } from "./exec-approval-manager.js"; +import { sanitizeSystemRunParamsForForwarding } from "./node-invoke-system-run-approval.js"; + +describe("sanitizeSystemRunParamsForForwarding", () => { + const now = Date.now(); + const client = { + connId: "conn-1", + connect: { + scopes: ["operator.write", "operator.approvals"], + device: { id: "dev-1" }, + client: { id: "cli-1" }, + }, + }; + + function makeRecord(command: string): ExecApprovalRecord { + return { + id: "approval-1", + request: { + host: "node", + command, + cwd: null, + agentId: null, + sessionKey: null, + }, + createdAtMs: now - 1_000, + expiresAtMs: now + 60_000, + requestedByConnId: "conn-1", + requestedByDeviceId: "dev-1", + requestedByClientId: "cli-1", + resolvedAtMs: now - 500, + decision: "allow-once", + resolvedBy: "operator", + }; + } + + function manager(record: ReturnType) { + return { + getSnapshot: () => record, + }; + } + + test("rejects cmd.exe /c trailing-arg mismatch against rawCommand", () => { + const result = sanitizeSystemRunParamsForForwarding({ + rawParams: { + command: ["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"], + rawCommand: "echo", + runId: "approval-1", + approved: true, + approvalDecision: "allow-once", + }, + client, + execApprovalManager: manager(makeRecord("echo")), + nowMs: now, + }); + expect(result.ok).toBe(false); + if (result.ok) { + throw new Error("unreachable"); + } + expect(result.message).toContain("rawCommand does not match command"); + expect(result.details?.code).toBe("RAW_COMMAND_MISMATCH"); + }); + + test("accepts matching cmd.exe /c command text for approval binding", () => { + const result = sanitizeSystemRunParamsForForwarding({ + rawParams: { + command: ["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"], + rawCommand: "echo SAFE&&whoami", + runId: "approval-1", + approved: true, + approvalDecision: "allow-once", + }, + client, + execApprovalManager: manager(makeRecord("echo SAFE&&whoami")), + nowMs: now, + }); + expect(result.ok).toBe(true); + if (!result.ok) { + throw new Error("unreachable"); + } + const params = result.params as Record; + expect(params.approved).toBe(true); + expect(params.approvalDecision).toBe("allow-once"); + }); +}); diff --git a/src/gateway/node-invoke-system-run-approval.ts b/src/gateway/node-invoke-system-run-approval.ts index 66865953d46..5684f4221f5 100644 --- a/src/gateway/node-invoke-system-run-approval.ts +++ b/src/gateway/node-invoke-system-run-approval.ts @@ -1,9 +1,5 @@ -import { - formatExecCommand, - validateSystemRunCommandConsistency, -} from "../infra/system-run-command.js"; -import type { ExecApprovalManager, ExecApprovalRecord } from "./exec-approval-manager.js"; -import type { GatewayClient } from "./server-methods/types.js"; +import { resolveSystemRunCommand } from "../infra/system-run-command.js"; +import type { ExecApprovalRecord } from "./exec-approval-manager.js"; type SystemRunParamsLike = { command?: unknown; @@ -19,6 +15,18 @@ type SystemRunParamsLike = { runId?: unknown; }; +type ApprovalLookup = { + getSnapshot: (recordId: string) => ExecApprovalRecord | null; +}; + +type ApprovalClient = { + connId?: string | null; + connect?: { + scopes?: unknown; + device?: { id?: string | null } | null; + } | null; +}; + function asRecord(value: unknown): Record | null { if (!value || typeof value !== "object" || Array.isArray(value)) { return null; @@ -39,31 +47,20 @@ function normalizeApprovalDecision(value: unknown): "allow-once" | "allow-always return s === "allow-once" || s === "allow-always" ? s : null; } -function clientHasApprovals(client: GatewayClient | null): boolean { +function clientHasApprovals(client: ApprovalClient | null): boolean { const scopes = Array.isArray(client?.connect?.scopes) ? client?.connect?.scopes : []; return scopes.includes("operator.admin") || scopes.includes("operator.approvals"); } -function getCmdText(params: SystemRunParamsLike): string { - const raw = normalizeString(params.rawCommand); - if (raw) { - return raw; - } - if (Array.isArray(params.command)) { - const parts = params.command.map((v) => String(v)); - if (parts.length > 0) { - return formatExecCommand(parts); - } - } - return ""; -} - -function approvalMatchesRequest(params: SystemRunParamsLike, record: ExecApprovalRecord): boolean { +function approvalMatchesRequest( + cmdText: string, + params: SystemRunParamsLike, + record: ExecApprovalRecord, +): boolean { if (record.request.host !== "node") { return false; } - const cmdText = getCmdText(params); if (!cmdText || record.request.command !== cmdText) { return false; } @@ -118,8 +115,8 @@ function pickSystemRunParams(raw: Record): Record String(v)) : []; - const raw = normalizeString(p.rawCommand); - if (raw) { - if (!Array.isArray(p.command) || argv.length === 0) { - return { - ok: false, - message: "rawCommand requires params.command", - details: { code: "MISSING_COMMAND" }, - }; - } - const validation = validateSystemRunCommandConsistency({ argv, rawCommand: raw }); - if (!validation.ok) { - return { - ok: false, - message: validation.message, - details: validation.details ?? { code: "RAW_COMMAND_MISMATCH" }, - }; - } + const cmdTextResolution = resolveSystemRunCommand({ + command: p.command, + rawCommand: p.rawCommand, + }); + if (!cmdTextResolution.ok) { + return { + ok: false, + message: cmdTextResolution.message, + details: cmdTextResolution.details, + }; } + const cmdText = cmdTextResolution.cmdText; const approved = p.approved === true; const requestedDecision = normalizeApprovalDecision(p.approvalDecision); @@ -221,7 +211,7 @@ export function sanitizeSystemRunParamsForForwarding(opts: { }; } - if (!approvalMatchesRequest(p, snapshot)) { + if (!approvalMatchesRequest(cmdText, p, snapshot)) { return { ok: false, message: "approval id does not match request", diff --git a/src/infra/system-run-command.test.ts b/src/infra/system-run-command.test.ts index 28fa16cec20..b375c07913d 100644 --- a/src/infra/system-run-command.test.ts +++ b/src/infra/system-run-command.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "vitest"; import { extractShellCommandFromArgv, formatExecCommand, + resolveSystemRunCommand, validateSystemRunCommandConsistency, } from "./system-run-command.js"; @@ -18,6 +19,12 @@ describe("system run command helpers", () => { expect(extractShellCommandFromArgv(["cmd.exe", "/d", "/s", "/c", "echo hi"])).toBe("echo hi"); }); + test("extractShellCommandFromArgv includes trailing cmd.exe args after /c", () => { + expect(extractShellCommandFromArgv(["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"])).toBe( + "echo SAFE&&whoami", + ); + }); + test("validateSystemRunCommandConsistency accepts rawCommand matching direct argv", () => { const res = validateSystemRunCommandConsistency({ argv: ["echo", "hi"], @@ -51,4 +58,41 @@ describe("system run command helpers", () => { }); expect(res.ok).toBe(true); }); + + test("validateSystemRunCommandConsistency rejects cmd.exe /c trailing-arg smuggling", () => { + const res = validateSystemRunCommandConsistency({ + argv: ["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"], + rawCommand: "echo", + }); + expect(res.ok).toBe(false); + if (res.ok) { + throw new Error("unreachable"); + } + expect(res.message).toContain("rawCommand does not match command"); + expect(res.details?.code).toBe("RAW_COMMAND_MISMATCH"); + }); + + test("resolveSystemRunCommand requires command when rawCommand is present", () => { + const res = resolveSystemRunCommand({ rawCommand: "echo hi" }); + expect(res.ok).toBe(false); + if (res.ok) { + throw new Error("unreachable"); + } + expect(res.message).toContain("rawCommand requires params.command"); + expect(res.details?.code).toBe("MISSING_COMMAND"); + }); + + test("resolveSystemRunCommand returns normalized argv and cmdText", () => { + const res = resolveSystemRunCommand({ + command: ["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"], + rawCommand: "echo SAFE&&whoami", + }); + expect(res.ok).toBe(true); + if (!res.ok) { + throw new Error("unreachable"); + } + expect(res.argv).toEqual(["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"]); + expect(res.shellCommand).toBe("echo SAFE&&whoami"); + expect(res.cmdText).toBe("echo SAFE&&whoami"); + }); }); diff --git a/src/infra/system-run-command.ts b/src/infra/system-run-command.ts index 5ba6e669cba..4d61c2e2464 100644 --- a/src/infra/system-run-command.ts +++ b/src/infra/system-run-command.ts @@ -12,6 +12,20 @@ export type SystemRunCommandValidation = details?: Record; }; +export type ResolvedSystemRunCommand = + | { + ok: true; + argv: string[]; + rawCommand: string | null; + shellCommand: string | null; + cmdText: string; + } + | { + ok: false; + message: string; + details?: Record; + }; + function basenameLower(token: string): string { const win = path.win32.basename(token); const posix = path.posix.basename(token); @@ -65,8 +79,12 @@ export function extractShellCommandFromArgv(argv: string[]): string | null { if (idx === -1) { return null; } - const cmd = argv[idx + 1]; - return typeof cmd === "string" ? cmd : null; + const tail = argv.slice(idx + 1).map((item) => String(item)); + if (tail.length === 0) { + return null; + } + const cmd = tail.join(" ").trim(); + return cmd.length > 0 ? cmd : null; } return null; @@ -81,7 +99,7 @@ export function validateSystemRunCommandConsistency(params: { ? params.rawCommand.trim() : null; const shellCommand = extractShellCommandFromArgv(params.argv); - const inferred = shellCommand ? shellCommand.trim() : formatExecCommand(params.argv); + const inferred = shellCommand !== null ? shellCommand.trim() : formatExecCommand(params.argv); if (raw && raw !== inferred) { return { @@ -100,7 +118,55 @@ export function validateSystemRunCommandConsistency(params: { // Only treat this as a shell command when argv is a recognized shell wrapper. // For direct argv execution, rawCommand is purely display/approval text and // must match the formatted argv. - shellCommand: shellCommand ? (raw ?? shellCommand) : null, + shellCommand: shellCommand !== null ? (raw ?? shellCommand) : null, cmdText: raw ?? shellCommand ?? inferred, }; } + +export function resolveSystemRunCommand(params: { + command?: unknown; + rawCommand?: unknown; +}): ResolvedSystemRunCommand { + const raw = + typeof params.rawCommand === "string" && params.rawCommand.trim().length > 0 + ? params.rawCommand.trim() + : null; + const command = Array.isArray(params.command) ? params.command : []; + if (command.length === 0) { + if (raw) { + return { + ok: false, + message: "rawCommand requires params.command", + details: { code: "MISSING_COMMAND" }, + }; + } + return { + ok: true, + argv: [], + rawCommand: null, + shellCommand: null, + cmdText: "", + }; + } + + const argv = command.map((v) => String(v)); + const validation = validateSystemRunCommandConsistency({ + argv, + rawCommand: raw, + }); + if (!validation.ok) { + return { + ok: false, + message: validation.message, + details: validation.details ?? { code: "RAW_COMMAND_MISMATCH" }, + }; + } + + return { + ok: true, + argv, + rawCommand: raw, + shellCommand: validation.shellCommand, + cmdText: validation.cmdText, + }; +} diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts new file mode 100644 index 00000000000..fc1a2fee3ea --- /dev/null +++ b/src/node-host/invoke-system-run.ts @@ -0,0 +1,422 @@ +import crypto from "node:crypto"; +import { resolveAgentConfig } from "../agents/agent-scope.js"; +import { loadConfig } from "../config/config.js"; +import type { GatewayClient } from "../gateway/client.js"; +import { + addAllowlistEntry, + analyzeArgvCommand, + evaluateExecAllowlist, + evaluateShellAllowlist, + recordAllowlistUse, + requiresExecApproval, + resolveExecApprovals, + resolveSafeBins, + type ExecAllowlistEntry, + type ExecAsk, + type ExecCommandSegment, + type ExecSecurity, +} from "../infra/exec-approvals.js"; +import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js"; +import { getTrustedSafeBinDirs } from "../infra/exec-safe-bin-trust.js"; +import { resolveSystemRunCommand } from "../infra/system-run-command.js"; + +type SystemRunParams = { + command: string[]; + rawCommand?: string | null; + cwd?: string | null; + env?: Record; + timeoutMs?: number | null; + needsScreenRecording?: boolean | null; + agentId?: string | null; + sessionKey?: string | null; + approved?: boolean | null; + approvalDecision?: string | null; + runId?: string | null; +}; + +type RunResult = { + exitCode?: number; + timedOut: boolean; + success: boolean; + stdout: string; + stderr: string; + error?: string | null; + truncated: boolean; +}; + +type ExecEventPayload = { + sessionKey: string; + runId: string; + host: string; + command?: string; + exitCode?: number; + timedOut?: boolean; + success?: boolean; + output?: string; + reason?: string; +}; + +export type SkillBinsProvider = { + current(force?: boolean): Promise>; +}; + +type SystemRunInvokeResult = { + ok: boolean; + payloadJSON?: string | null; + error?: { code?: string; message?: string } | null; +}; + +export async function handleSystemRunInvoke(opts: { + client: GatewayClient; + params: SystemRunParams; + skillBins: SkillBinsProvider; + execHostEnforced: boolean; + execHostFallbackAllowed: boolean; + resolveExecSecurity: (value?: string) => ExecSecurity; + resolveExecAsk: (value?: string) => ExecAsk; + isCmdExeInvocation: (argv: string[]) => boolean; + sanitizeEnv: (overrides?: Record | null) => Record | undefined; + runCommand: ( + argv: string[], + cwd: string | undefined, + env: Record | undefined, + timeoutMs: number | undefined, + ) => Promise; + runViaMacAppExecHost: (params: { + approvals: ReturnType; + request: ExecHostRequest; + }) => Promise; + sendNodeEvent: (client: GatewayClient, event: string, payload: unknown) => Promise; + buildExecEventPayload: (payload: ExecEventPayload) => ExecEventPayload; + sendInvokeResult: (result: SystemRunInvokeResult) => Promise; + sendExecFinishedEvent: (params: { + sessionKey: string; + runId: string; + cmdText: string; + result: { + stdout?: string; + stderr?: string; + error?: string | null; + exitCode?: number | null; + timedOut?: boolean; + success?: boolean; + }; + }) => Promise; +}): Promise { + const command = resolveSystemRunCommand({ + command: opts.params.command, + rawCommand: opts.params.rawCommand, + }); + if (!command.ok) { + await opts.sendInvokeResult({ + ok: false, + error: { code: "INVALID_REQUEST", message: command.message }, + }); + return; + } + if (command.argv.length === 0) { + await opts.sendInvokeResult({ + ok: false, + error: { code: "INVALID_REQUEST", message: "command required" }, + }); + return; + } + + const argv = command.argv; + const rawCommand = command.rawCommand ?? ""; + const shellCommand = command.shellCommand; + const cmdText = command.cmdText; + const agentId = opts.params.agentId?.trim() || undefined; + const cfg = loadConfig(); + const agentExec = agentId ? resolveAgentConfig(cfg, agentId)?.tools?.exec : undefined; + const configuredSecurity = opts.resolveExecSecurity( + agentExec?.security ?? cfg.tools?.exec?.security, + ); + const configuredAsk = opts.resolveExecAsk(agentExec?.ask ?? cfg.tools?.exec?.ask); + const approvals = resolveExecApprovals(agentId, { + security: configuredSecurity, + ask: configuredAsk, + }); + const security = approvals.agent.security; + const ask = approvals.agent.ask; + const autoAllowSkills = approvals.agent.autoAllowSkills; + const sessionKey = opts.params.sessionKey?.trim() || "node"; + const runId = opts.params.runId?.trim() || crypto.randomUUID(); + const env = opts.sanitizeEnv(opts.params.env ?? undefined); + const safeBins = resolveSafeBins(agentExec?.safeBins ?? cfg.tools?.exec?.safeBins); + const trustedSafeBinDirs = getTrustedSafeBinDirs(); + 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, + 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, + 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; + } + const isWindows = process.platform === "win32"; + const cmdInvocation = shellCommand + ? opts.isCmdExeInvocation(segments[0]?.argv ?? []) + : opts.isCmdExeInvocation(argv); + if (security === "allowlist" && isWindows && cmdInvocation) { + analysisOk = false; + allowlistSatisfied = false; + } + + const useMacAppExec = process.platform === "darwin"; + if (useMacAppExec) { + const approvalDecision = + opts.params.approvalDecision === "allow-once" || + opts.params.approvalDecision === "allow-always" + ? opts.params.approvalDecision + : null; + const execRequest: ExecHostRequest = { + command: argv, + rawCommand: rawCommand || shellCommand || null, + cwd: opts.params.cwd ?? null, + env: opts.params.env ?? null, + timeoutMs: opts.params.timeoutMs ?? null, + needsScreenRecording: opts.params.needsScreenRecording ?? null, + agentId: agentId ?? null, + sessionKey: sessionKey ?? null, + approvalDecision, + }; + 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", + }, + }); + 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 }, + }); + return; + } else { + const result: ExecHostRunResult = response.payload; + await opts.sendExecFinishedEvent({ sessionKey, runId, cmdText, result }); + await opts.sendInvokeResult({ + ok: true, + payloadJSON: JSON.stringify(result), + }); + return; + } + } + + if (security === "deny") { + await opts.sendNodeEvent( + opts.client, + "exec.denied", + opts.buildExecEventPayload({ + sessionKey, + runId, + host: "node", + command: cmdText, + reason: "security=deny", + }), + ); + await opts.sendInvokeResult({ + ok: false, + error: { code: "UNAVAILABLE", message: "SYSTEM_RUN_DISABLED: security=deny" }, + }); + return; + } + + const requiresAsk = requiresExecApproval({ + ask, + security, + analysisOk, + allowlistSatisfied, + }); + + const approvalDecision = + opts.params.approvalDecision === "allow-once" || opts.params.approvalDecision === "allow-always" + ? opts.params.approvalDecision + : null; + const approvedByAsk = approvalDecision !== null || opts.params.approved === true; + if (requiresAsk && !approvedByAsk) { + await opts.sendNodeEvent( + opts.client, + "exec.denied", + opts.buildExecEventPayload({ + sessionKey, + runId, + host: "node", + command: cmdText, + reason: "approval-required", + }), + ); + await opts.sendInvokeResult({ + ok: false, + error: { code: "UNAVAILABLE", message: "SYSTEM_RUN_DENIED: approval required" }, + }); + return; + } + if (approvalDecision === "allow-always" && security === "allowlist") { + if (analysisOk) { + for (const segment of segments) { + const pattern = segment.resolution?.resolvedPath ?? ""; + if (pattern) { + addAllowlistEntry(approvals.file, agentId, pattern); + } + } + } + } + + if (security === "allowlist" && (!analysisOk || !allowlistSatisfied) && !approvedByAsk) { + await opts.sendNodeEvent( + opts.client, + "exec.denied", + opts.buildExecEventPayload({ + sessionKey, + runId, + host: "node", + command: cmdText, + reason: "allowlist-miss", + }), + ); + await opts.sendInvokeResult({ + ok: false, + error: { code: "UNAVAILABLE", message: "SYSTEM_RUN_DENIED: allowlist miss" }, + }); + return; + } + + if (allowlistMatches.length > 0) { + const seen = new Set(); + for (const match of allowlistMatches) { + if (!match?.pattern || seen.has(match.pattern)) { + continue; + } + seen.add(match.pattern); + recordAllowlistUse( + approvals.file, + agentId, + match, + cmdText, + segments[0]?.resolution?.resolvedPath, + ); + } + } + + 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" }, + }); + return; + } + + let execArgv = argv; + if ( + security === "allowlist" && + isWindows && + !approvedByAsk && + shellCommand && + analysisOk && + allowlistSatisfied && + segments.length === 1 && + segments[0]?.argv.length > 0 + ) { + execArgv = segments[0].argv; + } + + const result = await opts.runCommand( + execArgv, + opts.params.cwd?.trim() || undefined, + 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}`; + } + } + await opts.sendExecFinishedEvent({ sessionKey, runId, cmdText, result }); + + await opts.sendInvokeResult({ + ok: true, + payloadJSON: JSON.stringify({ + exitCode: result.exitCode, + timedOut: result.timedOut, + success: result.success, + stdout: result.stdout, + stderr: result.stderr, + error: result.error ?? null, + }), + }); +} diff --git a/src/node-host/invoke.ts b/src/node-host/invoke.ts index d2c95b7e0b1..5b9ae837084 100644 --- a/src/node-host/invoke.ts +++ b/src/node-host/invoke.ts @@ -1,40 +1,26 @@ import { spawn } from "node:child_process"; -import crypto from "node:crypto"; import fs from "node:fs"; import path from "node:path"; -import { resolveAgentConfig } from "../agents/agent-scope.js"; -import { loadConfig } from "../config/config.js"; import { GatewayClient } from "../gateway/client.js"; import { - addAllowlistEntry, - analyzeArgvCommand, - evaluateExecAllowlist, - evaluateShellAllowlist, - requiresExecApproval, - normalizeExecApprovals, - mergeExecApprovalsSocketDefaults, - recordAllowlistUse, - resolveExecApprovals, - resolveSafeBins, ensureExecApprovals, + mergeExecApprovalsSocketDefaults, + normalizeExecApprovals, readExecApprovalsSnapshot, saveExecApprovals, type ExecAsk, type ExecApprovalsFile, - type ExecAllowlistEntry, - type ExecCommandSegment, + type ExecApprovalsResolved, type ExecSecurity, } from "../infra/exec-approvals.js"; import { requestExecHostViaSocket, type ExecHostRequest, type ExecHostResponse, - type ExecHostRunResult, } from "../infra/exec-host.js"; -import { getTrustedSafeBinDirs } from "../infra/exec-safe-bin-trust.js"; import { sanitizeHostExecEnv } from "../infra/host-env-security.js"; -import { validateSystemRunCommandConsistency } from "../infra/system-run-command.js"; import { runBrowserProxyCommand } from "./invoke-browser.js"; +import { handleSystemRunInvoke } from "./invoke-system-run.js"; const OUTPUT_CAP = 200_000; const OUTPUT_EVENT_TAIL = 20_000; @@ -336,7 +322,7 @@ async function sendExecFinishedEvent(params: { } async function runViaMacAppExecHost(params: { - approvals: ReturnType; + approvals: ExecApprovalsResolved; request: ExecHostRequest; }): Promise { const { approvals, request } = params; @@ -483,308 +469,26 @@ export async function handleInvoke( return; } - const argv = params.command.map((item) => String(item)); - const rawCommand = typeof params.rawCommand === "string" ? params.rawCommand.trim() : ""; - const consistency = validateSystemRunCommandConsistency({ - argv, - rawCommand: rawCommand || null, - }); - if (!consistency.ok) { - await sendErrorResult(client, frame, "INVALID_REQUEST", consistency.message); - return; - } - - const shellCommand = consistency.shellCommand; - const cmdText = consistency.cmdText; - const agentId = params.agentId?.trim() || undefined; - const cfg = loadConfig(); - const agentExec = agentId ? resolveAgentConfig(cfg, agentId)?.tools?.exec : undefined; - const configuredSecurity = resolveExecSecurity(agentExec?.security ?? cfg.tools?.exec?.security); - const configuredAsk = resolveExecAsk(agentExec?.ask ?? cfg.tools?.exec?.ask); - const approvals = resolveExecApprovals(agentId, { - security: configuredSecurity, - ask: configuredAsk, - }); - const security = approvals.agent.security; - const ask = approvals.agent.ask; - const autoAllowSkills = approvals.agent.autoAllowSkills; - const sessionKey = params.sessionKey?.trim() || "node"; - const runId = params.runId?.trim() || crypto.randomUUID(); - const env = sanitizeEnv(params.env ?? undefined); - const safeBins = resolveSafeBins(agentExec?.safeBins ?? cfg.tools?.exec?.safeBins); - const trustedSafeBinDirs = getTrustedSafeBinDirs(); - const bins = autoAllowSkills ? await 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, - cwd: 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: params.cwd ?? undefined, env }); - const allowlistEval = evaluateExecAllowlist({ - analysis, - allowlist: approvals.allowlist, - safeBins, - cwd: params.cwd ?? undefined, - trustedSafeBinDirs, - skillBins: bins, - autoAllowSkills, - }); - analysisOk = analysis.ok; - allowlistMatches = allowlistEval.allowlistMatches; - allowlistSatisfied = - security === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false; - segments = analysis.segments; - } - const isWindows = process.platform === "win32"; - const cmdInvocation = shellCommand - ? isCmdExeInvocation(segments[0]?.argv ?? []) - : isCmdExeInvocation(argv); - if (security === "allowlist" && isWindows && cmdInvocation) { - analysisOk = false; - allowlistSatisfied = false; - } - - const useMacAppExec = process.platform === "darwin"; - if (useMacAppExec) { - const approvalDecision = - params.approvalDecision === "allow-once" || params.approvalDecision === "allow-always" - ? params.approvalDecision - : null; - const execRequest: ExecHostRequest = { - command: argv, - rawCommand: rawCommand || shellCommand || null, - cwd: params.cwd ?? null, - env: params.env ?? null, - timeoutMs: params.timeoutMs ?? null, - needsScreenRecording: params.needsScreenRecording ?? null, - agentId: agentId ?? null, - sessionKey: sessionKey ?? null, - approvalDecision, - }; - const response = await runViaMacAppExecHost({ approvals, request: execRequest }); - if (!response) { - if (execHostEnforced || !execHostFallbackAllowed) { - await sendNodeEvent( - client, - "exec.denied", - buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: "companion-unavailable", - }), - ); - await sendInvokeResult(client, frame, { - ok: false, - error: { - code: "UNAVAILABLE", - message: "COMPANION_APP_UNAVAILABLE: macOS app exec host unreachable", - }, - }); - return; - } - } else if (!response.ok) { - const reason = response.error.reason ?? "approval-required"; - await sendNodeEvent( - client, - "exec.denied", - buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason, - }), - ); - await sendInvokeResult(client, frame, { - ok: false, - error: { code: "UNAVAILABLE", message: response.error.message }, - }); - return; - } else { - const result: ExecHostRunResult = response.payload; + await handleSystemRunInvoke({ + client, + params, + skillBins, + execHostEnforced, + execHostFallbackAllowed, + resolveExecSecurity, + resolveExecAsk, + isCmdExeInvocation, + sanitizeEnv, + runCommand, + runViaMacAppExecHost, + sendNodeEvent, + buildExecEventPayload, + sendInvokeResult: async (result) => { + await sendInvokeResult(client, frame, result); + }, + sendExecFinishedEvent: async ({ sessionKey, runId, cmdText, result }) => { await sendExecFinishedEvent({ client, sessionKey, runId, cmdText, result }); - await sendInvokeResult(client, frame, { - ok: true, - payloadJSON: JSON.stringify(result), - }); - return; - } - } - - if (security === "deny") { - await sendNodeEvent( - client, - "exec.denied", - buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: "security=deny", - }), - ); - await sendInvokeResult(client, frame, { - ok: false, - error: { code: "UNAVAILABLE", message: "SYSTEM_RUN_DISABLED: security=deny" }, - }); - return; - } - - const requiresAsk = requiresExecApproval({ - ask, - security, - analysisOk, - allowlistSatisfied, - }); - - const approvalDecision = - params.approvalDecision === "allow-once" || params.approvalDecision === "allow-always" - ? params.approvalDecision - : null; - const approvedByAsk = approvalDecision !== null || params.approved === true; - if (requiresAsk && !approvedByAsk) { - await sendNodeEvent( - client, - "exec.denied", - buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: "approval-required", - }), - ); - await sendInvokeResult(client, frame, { - ok: false, - error: { code: "UNAVAILABLE", message: "SYSTEM_RUN_DENIED: approval required" }, - }); - return; - } - if (approvalDecision === "allow-always" && security === "allowlist") { - if (analysisOk) { - for (const segment of segments) { - const pattern = segment.resolution?.resolvedPath ?? ""; - if (pattern) { - addAllowlistEntry(approvals.file, agentId, pattern); - } - } - } - } - - if (security === "allowlist" && (!analysisOk || !allowlistSatisfied) && !approvedByAsk) { - await sendNodeEvent( - client, - "exec.denied", - buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: "allowlist-miss", - }), - ); - await sendInvokeResult(client, frame, { - ok: false, - error: { code: "UNAVAILABLE", message: "SYSTEM_RUN_DENIED: allowlist miss" }, - }); - return; - } - - if (allowlistMatches.length > 0) { - const seen = new Set(); - for (const match of allowlistMatches) { - if (!match?.pattern || seen.has(match.pattern)) { - continue; - } - seen.add(match.pattern); - recordAllowlistUse( - approvals.file, - agentId, - match, - cmdText, - segments[0]?.resolution?.resolvedPath, - ); - } - } - - if (params.needsScreenRecording === true) { - await sendNodeEvent( - client, - "exec.denied", - buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: "permission:screenRecording", - }), - ); - await sendInvokeResult(client, frame, { - ok: false, - error: { code: "UNAVAILABLE", message: "PERMISSION_MISSING: screenRecording" }, - }); - return; - } - - let execArgv = argv; - if ( - security === "allowlist" && - isWindows && - !approvedByAsk && - shellCommand && - analysisOk && - allowlistSatisfied && - segments.length === 1 && - segments[0]?.argv.length > 0 - ) { - execArgv = segments[0].argv; - } - - const result = await runCommand( - execArgv, - params.cwd?.trim() || undefined, - env, - 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}`; - } - } - await sendExecFinishedEvent({ client, sessionKey, runId, cmdText, result }); - - await sendInvokeResult(client, frame, { - ok: true, - payloadJSON: JSON.stringify({ - exitCode: result.exitCode, - timedOut: result.timedOut, - success: result.success, - stdout: result.stdout, - stderr: result.stderr, - error: result.error ?? null, - }), + }, }); }