From 155118751ff99169f0d5ed335b2c075db09b5547 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 01:12:47 +0000 Subject: [PATCH] refactor!: remove versioned system-run approval contract --- CHANGELOG.md | 7 +- .../OpenClawProtocol/GatewayModels.swift | 8 +- .../OpenClawProtocol/GatewayModels.swift | 8 +- docs/gateway/protocol.md | 1 + docs/tools/exec-approvals.md | 4 + .../bash-tools.exec-approval-request.ts | 9 +- src/agents/bash-tools.exec-host-node.ts | 47 ++++-- .../bash-tools.exec.approval-id.test.ts | 55 +++++- src/agents/openclaw-tools.camera.test.ts | 85 +++++++++- src/agents/tools/nodes-tool.ts | 43 +++-- src/cli/nodes-cli.coverage.test.ts | 7 +- src/cli/nodes-cli/register.invoke.ts | 2 +- ...e-invoke-system-run-approval-match.test.ts | 12 +- .../node-invoke-system-run-approval-match.ts | 14 +- .../node-invoke-system-run-approval.test.ts | 19 +-- .../node-invoke-system-run-approval.ts | 6 +- src/gateway/protocol/schema/exec-approvals.ts | 3 +- src/gateway/server-methods/exec-approval.ts | 24 ++- .../server-methods/server-methods.test.ts | 58 +++++-- ...server.node-invoke-approval-bypass.test.ts | 7 + ...stem-run-approval-binding.contract.test.ts | 18 +- .../system-run-approval-binding.test.ts | 18 +- src/infra/exec-approvals-analysis.ts | 5 +- src/infra/exec-approvals.ts | 10 +- src/infra/exec-command-resolution.ts | 16 ++ src/infra/system-run-approval-binding.ts | 28 +--- src/infra/system-run-approval-context.ts | 36 ++-- src/node-host/invoke-system-run-allowlist.ts | 3 +- src/node-host/invoke-system-run-plan.ts | 47 ++---- src/node-host/invoke-system-run.test.ts | 156 ++++++++++++++++-- src/node-host/invoke-system-run.ts | 3 +- src/node-host/invoke.ts | 4 +- .../system-run-approval-binding-contract.json | 16 +- 33 files changed, 564 insertions(+), 215 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05e9a49bd88..6fa98109a4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,11 @@ Docs: https://docs.openclaw.ai - OpenAI/WebSocket warm-up: add optional OpenAI Responses WebSocket warm-up (`response.create` with `generate:false`), enable it by default for `openai/*`, and expose `params.openaiWsWarmup` for per-model enable/disable control. - Agents/Subagents runtime events: replace ad-hoc subagent completion system-message handoff with typed internal completion events (`task_completion`) that are rendered consistently across direct and queued announce paths, with gateway/CLI plumbing for structured `internalEvents`. +### Breaking + +- **BREAKING:** Node exec approval payloads now require `systemRunPlan`. `host=node` approval requests without that plan are rejected. +- **BREAKING:** Node `system.run` execution now pins path-token commands to the canonical executable path (`realpath`) in both allowlist and approval execution flows. Integrations/tests that asserted token-form argv (for example `tr`) must now accept canonical paths (for example `/usr/bin/tr`). + ### Fixes - Security/Sandbox media reads: eliminate sandbox media TOCTOU symlink-retarget escapes by enforcing root-scoped boundary-safe reads at attachment/image load time and consolidating shared safe-read helpers across sandbox media callsites. This ships in the next npm release. Thanks @tdjackey for reporting. @@ -257,7 +262,7 @@ Docs: https://docs.openclaw.ai - Models/OpenAI Codex config schema parity: accept `openai-codex-responses` in the config model API schema and TypeScript `ModelApi` union, with regression coverage for config validation. Landed from contributor PR #27501 by @AytuncYildizli. Thanks @AytuncYildizli. - Agents/Models config: preserve agent-level provider `apiKey` and `baseUrl` during merge-mode `models.json` updates when agent values are present. (#27293) thanks @Sid-Qin. - Azure OpenAI Responses: force `store=true` for `azure-openai-responses` direct responses API calls to avoid multi-turn 400 failures. Landed from contributor PR #27499 by @polarbear-Yang. (#27497) -- Security/Node exec approvals: require structured `commandArgv` approvals for `host=node`, enforce versioned `systemRunBindingV1` matching for argv/cwd/session/agent/env context with fail-closed behavior on missing/mismatched bindings, and add `GIT_EXTERNAL_DIFF` to blocked host env keys. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting. +- Security/Node exec approvals: require structured `commandArgv` approvals for `host=node`, enforce `systemRunBinding` matching for argv/cwd/session/agent/env context with fail-closed behavior on missing/mismatched bindings, and add `GIT_EXTERNAL_DIFF` to blocked host env keys. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting. - Security/Command authorization: enforce sender authorization for natural-language abort triggers (`stop`-like text) and `/models` listings, preventing unauthorized session aborts and model-auth metadata disclosure. This ships in the next npm release (`2026.2.27`). Thanks @tdjackey for reporting. - Security/Plugin channel HTTP auth: normalize protected `/api/channels` path checks against canonicalized request paths (case + percent-decoding + slash normalization), resolve encoded dot-segment traversal variants, and fail closed on malformed `%`-encoded channel prefixes so alternate-path variants cannot bypass gateway auth. This ships in the next npm release (`2026.2.26`). Thanks @zpbrent for reporting. - Security/Gateway node pairing: pin paired-device `platform`/`deviceFamily` metadata across reconnects and bind those fields into device-auth signatures, so reconnect metadata spoofing cannot expand node command allowlists without explicit repair pairing. This ships in the next npm release (`2026.2.26`). Thanks @76embiid21 for reporting. diff --git a/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift b/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift index 28382e89d84..7aa2933479b 100644 --- a/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift @@ -2822,7 +2822,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { public let id: String? public let command: String public let commandargv: [String]? - public let systemrunplanv2: [String: AnyCodable]? + public let systemrunplan: [String: AnyCodable]? public let env: [String: AnyCodable]? public let cwd: AnyCodable? public let nodeid: AnyCodable? @@ -2843,7 +2843,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { id: String?, command: String, commandargv: [String]?, - systemrunplanv2: [String: AnyCodable]?, + systemrunplan: [String: AnyCodable]?, env: [String: AnyCodable]?, cwd: AnyCodable?, nodeid: AnyCodable?, @@ -2863,7 +2863,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { self.id = id self.command = command self.commandargv = commandargv - self.systemrunplanv2 = systemrunplanv2 + self.systemrunplan = systemrunplan self.env = env self.cwd = cwd self.nodeid = nodeid @@ -2885,7 +2885,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { case id case command case commandargv = "commandArgv" - case systemrunplanv2 = "systemRunPlanV2" + case systemrunplan = "systemRunPlan" case env case cwd case nodeid = "nodeId" diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index 28382e89d84..7aa2933479b 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -2822,7 +2822,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { public let id: String? public let command: String public let commandargv: [String]? - public let systemrunplanv2: [String: AnyCodable]? + public let systemrunplan: [String: AnyCodable]? public let env: [String: AnyCodable]? public let cwd: AnyCodable? public let nodeid: AnyCodable? @@ -2843,7 +2843,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { id: String?, command: String, commandargv: [String]?, - systemrunplanv2: [String: AnyCodable]?, + systemrunplan: [String: AnyCodable]?, env: [String: AnyCodable]?, cwd: AnyCodable?, nodeid: AnyCodable?, @@ -2863,7 +2863,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { self.id = id self.command = command self.commandargv = commandargv - self.systemrunplanv2 = systemrunplanv2 + self.systemrunplan = systemrunplan self.env = env self.cwd = cwd self.nodeid = nodeid @@ -2885,7 +2885,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { case id case command case commandargv = "commandArgv" - case systemrunplanv2 = "systemRunPlanV2" + case systemrunplan = "systemRunPlan" case env case cwd case nodeid = "nodeId" diff --git a/docs/gateway/protocol.md b/docs/gateway/protocol.md index f62a88f17ee..fe0ddb3f052 100644 --- a/docs/gateway/protocol.md +++ b/docs/gateway/protocol.md @@ -182,6 +182,7 @@ The Gateway treats these as **claims** and enforces server-side allowlists. - When an exec request needs approval, the gateway broadcasts `exec.approval.requested`. - Operator clients resolve by calling `exec.approval.resolve` (requires `operator.approvals` scope). +- For `host=node`, `exec.approval.request` must include `systemRunPlan` (canonical `argv`/`cwd`/`rawCommand`/session metadata). Requests missing `systemRunPlan` are rejected. ## Versioning diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 619f5cdb38e..45141e6d735 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -252,6 +252,10 @@ When a prompt is required, the gateway broadcasts `exec.approval.requested` to o The Control UI and macOS app resolve it via `exec.approval.resolve`, then the gateway forwards the approved request to the node host. +For `host=node`, approval requests include a canonical `systemRunPlan` payload. The gateway uses +that plan as the authoritative command/cwd/session context when forwarding approved `system.run` +requests. + When approvals are required, the exec tool returns immediately with an approval id. Use that id to correlate later system events (`Exec finished` / `Exec denied`). If no decision arrives before the timeout, the request is treated as an approval timeout and surfaced as a denial reason. diff --git a/src/agents/bash-tools.exec-approval-request.ts b/src/agents/bash-tools.exec-approval-request.ts index 842fcc1dcf4..0b0c0228c6e 100644 --- a/src/agents/bash-tools.exec-approval-request.ts +++ b/src/agents/bash-tools.exec-approval-request.ts @@ -1,4 +1,4 @@ -import type { ExecAsk, ExecSecurity } from "../infra/exec-approvals.js"; +import type { ExecAsk, ExecSecurity, SystemRunApprovalPlan } from "../infra/exec-approvals.js"; import { DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS, DEFAULT_APPROVAL_TIMEOUT_MS, @@ -9,6 +9,7 @@ export type RequestExecApprovalDecisionParams = { id: string; command: string; commandArgv?: string[]; + systemRunPlan?: SystemRunApprovalPlan; env?: Record; cwd: string; nodeId?: string; @@ -28,6 +29,7 @@ type ExecApprovalRequestToolParams = { id: string; command: string; commandArgv?: string[]; + systemRunPlan?: SystemRunApprovalPlan; env?: Record; cwd: string; nodeId?: string; @@ -52,6 +54,7 @@ function buildExecApprovalRequestToolParams( id: params.id, command: params.command, commandArgv: params.commandArgv, + systemRunPlan: params.systemRunPlan, env: params.env, cwd: params.cwd, nodeId: params.nodeId, @@ -156,6 +159,7 @@ export async function requestExecApprovalDecisionForHost(params: { approvalId: string; command: string; commandArgv?: string[]; + systemRunPlan?: SystemRunApprovalPlan; env?: Record; workdir: string; host: "gateway" | "node"; @@ -174,6 +178,7 @@ export async function requestExecApprovalDecisionForHost(params: { id: params.approvalId, command: params.command, commandArgv: params.commandArgv, + systemRunPlan: params.systemRunPlan, env: params.env, cwd: params.workdir, nodeId: params.nodeId, @@ -194,6 +199,7 @@ export async function registerExecApprovalRequestForHost(params: { approvalId: string; command: string; commandArgv?: string[]; + systemRunPlan?: SystemRunApprovalPlan; env?: Record; workdir: string; host: "gateway" | "node"; @@ -212,6 +218,7 @@ export async function registerExecApprovalRequestForHost(params: { id: params.approvalId, command: params.command, commandArgv: params.commandArgv, + systemRunPlan: params.systemRunPlan, env: params.env, cwd: params.workdir, nodeId: params.nodeId, diff --git a/src/agents/bash-tools.exec-host-node.ts b/src/agents/bash-tools.exec-host-node.ts index 1c210ef7b88..f72b6e289ed 100644 --- a/src/agents/bash-tools.exec-host-node.ts +++ b/src/agents/bash-tools.exec-host-node.ts @@ -13,6 +13,7 @@ import { } from "../infra/exec-approvals.js"; import { detectCommandObfuscation } from "../infra/exec-obfuscation-detect.js"; import { buildNodeShellCommand } from "../infra/node-shell.js"; +import { parsePreparedSystemRunPayload } from "../infra/system-run-approval-context.js"; import { logInfo } from "../logger.js"; import { registerExecApprovalRequestForHost, @@ -95,6 +96,31 @@ export async function executeNodeHostCommand( ); } const argv = buildNodeShellCommand(params.command, nodeInfo?.platform); + const prepareRaw = await callGatewayTool<{ payload?: unknown }>( + "node.invoke", + { timeoutMs: 15_000 }, + { + nodeId, + command: "system.run.prepare", + params: { + command: argv, + rawCommand: params.command, + cwd: params.workdir, + agentId: params.agentId, + sessionKey: params.sessionKey, + }, + idempotencyKey: crypto.randomUUID(), + }, + ); + const prepared = parsePreparedSystemRunPayload(prepareRaw?.payload); + if (!prepared) { + throw new Error("invalid system.run.prepare response"); + } + const runArgv = prepared.plan.argv; + const runRawCommand = prepared.plan.rawCommand ?? prepared.cmdText; + const runCwd = prepared.plan.cwd ?? params.workdir; + const runAgentId = prepared.plan.agentId ?? params.agentId; + const runSessionKey = prepared.plan.sessionKey ?? params.sessionKey; const nodeEnv = params.requestedEnv ? { ...params.requestedEnv } : undefined; const baseAllowlistEval = evaluateShellAllowlist({ @@ -170,13 +196,13 @@ export async function executeNodeHostCommand( nodeId, command: "system.run", params: { - command: argv, - rawCommand: params.command, - cwd: params.workdir, + command: runArgv, + rawCommand: runRawCommand, + cwd: runCwd, env: nodeEnv, timeoutMs: typeof params.timeoutSec === "number" ? params.timeoutSec * 1000 : undefined, - agentId: params.agentId, - sessionKey: params.sessionKey, + agentId: runAgentId, + sessionKey: runSessionKey, approved: approvedByAsk, approvalDecision: approvalDecision ?? undefined, runId: runId ?? undefined, @@ -197,16 +223,17 @@ export async function executeNodeHostCommand( // Register first so the returned approval ID is actionable immediately. const registration = await registerExecApprovalRequestForHost({ approvalId, - command: params.command, - commandArgv: argv, + command: prepared.cmdText, + commandArgv: prepared.plan.argv, + systemRunPlan: prepared.plan, env: nodeEnv, - workdir: params.workdir, + workdir: runCwd, host: "node", nodeId, security: hostSecurity, ask: hostAsk, - agentId: params.agentId, - sessionKey: params.sessionKey, + agentId: runAgentId, + sessionKey: runSessionKey, turnSourceChannel: params.turnSourceChannel, turnSourceTo: params.turnSourceTo, turnSourceAccountId: params.turnSourceAccountId, diff --git a/src/agents/bash-tools.exec.approval-id.test.ts b/src/agents/bash-tools.exec.approval-id.test.ts index fc04efc0a63..d99e3d6fcbb 100644 --- a/src/agents/bash-tools.exec.approval-id.test.ts +++ b/src/agents/bash-tools.exec.approval-id.test.ts @@ -27,6 +27,33 @@ let callGatewayTool: typeof import("./tools/gateway.js").callGatewayTool; let createExecTool: typeof import("./bash-tools.exec.js").createExecTool; let detectCommandObfuscation: typeof import("../infra/exec-obfuscation-detect.js").detectCommandObfuscation; +function buildPreparedSystemRunPayload(rawInvokeParams: unknown) { + const invoke = (rawInvokeParams ?? {}) as { + params?: { + command?: unknown; + rawCommand?: unknown; + cwd?: unknown; + agentId?: unknown; + sessionKey?: unknown; + }; + }; + const params = invoke.params ?? {}; + const argv = Array.isArray(params.command) ? params.command.map(String) : []; + const rawCommand = typeof params.rawCommand === "string" ? params.rawCommand : null; + return { + payload: { + cmdText: rawCommand ?? argv.join(" "), + plan: { + argv, + cwd: typeof params.cwd === "string" ? params.cwd : null, + rawCommand, + agentId: typeof params.agentId === "string" ? params.agentId : null, + sessionKey: typeof params.sessionKey === "string" ? params.sessionKey : null, + }, + }, + }; +} + describe("exec approvals", () => { let previousHome: string | undefined; let previousUserProfile: string | undefined; @@ -71,8 +98,14 @@ describe("exec approvals", () => { return { decision: "allow-once" }; } if (method === "node.invoke") { - invokeParams = params; - return { ok: true }; + const invoke = params as { command?: string }; + if (invoke.command === "system.run.prepare") { + return buildPreparedSystemRunPayload(params); + } + if (invoke.command === "system.run") { + invokeParams = params; + return { payload: { success: true, stdout: "ok" } }; + } } return { ok: true }; }); @@ -116,12 +149,16 @@ describe("exec approvals", () => { }; const calls: string[] = []; - vi.mocked(callGatewayTool).mockImplementation(async (method) => { + vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { calls.push(method); if (method === "exec.approvals.node.get") { return { file: approvalsFile }; } if (method === "node.invoke") { + const invoke = params as { command?: string }; + if (invoke.command === "system.run.prepare") { + return buildPreparedSystemRunPayload(params); + } return { payload: { success: true, stdout: "ok" } }; } // exec.approval.request should NOT be called when allowlist is satisfied @@ -266,7 +303,8 @@ describe("exec approvals", () => { }); const calls: string[] = []; - vi.mocked(callGatewayTool).mockImplementation(async (method) => { + const nodeInvokeCommands: string[] = []; + vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { calls.push(method); if (method === "exec.approval.request") { return { status: "accepted", id: "approval-id" }; @@ -275,6 +313,13 @@ describe("exec approvals", () => { return {}; } if (method === "node.invoke") { + const invoke = params as { command?: string }; + if (invoke.command) { + nodeInvokeCommands.push(invoke.command); + } + if (invoke.command === "system.run.prepare") { + return buildPreparedSystemRunPayload(params); + } return { payload: { success: true, stdout: "should-not-run" } }; } return { ok: true }; @@ -289,7 +334,7 @@ describe("exec approvals", () => { const result = await tool.execute("call5", { command: "echo hi | sh" }); expect(result.details.status).toBe("approval-pending"); - await expect.poll(() => calls.filter((call) => call === "node.invoke").length).toBe(0); + await expect.poll(() => nodeInvokeCommands.includes("system.run")).toBe(false); }); it("denies gateway obfuscated command when approval request times out", async () => { diff --git a/src/agents/openclaw-tools.camera.test.ts b/src/agents/openclaw-tools.camera.test.ts index 171788e5350..7e3132b3152 100644 --- a/src/agents/openclaw-tools.camera.test.ts +++ b/src/agents/openclaw-tools.camera.test.ts @@ -356,6 +356,21 @@ describe("nodes run", () => { return mockNodeList(["system.run"]); } if (method === "node.invoke") { + const command = (params as { command?: string } | undefined)?.command; + if (command === "system.run.prepare") { + return { + payload: { + cmdText: "echo hi", + plan: { + argv: ["echo", "hi"], + cwd: "/tmp", + rawCommand: "echo hi", + agentId: null, + sessionKey: null, + }, + }, + }; + } expect(params).toMatchObject({ nodeId: NODE_ID, command: "system.run", @@ -391,6 +406,21 @@ describe("nodes run", () => { return mockNodeList(["system.run"]); } if (method === "node.invoke") { + const command = (params as { command?: string } | undefined)?.command; + if (command === "system.run.prepare") { + return { + payload: { + cmdText: "echo hi", + plan: { + argv: ["echo", "hi"], + cwd: null, + rawCommand: "echo hi", + agentId: null, + sessionKey: null, + }, + }, + }; + } invokeCalls += 1; if (invokeCalls === 1) { throw new Error("SYSTEM_RUN_DENIED: approval required"); @@ -411,6 +441,10 @@ describe("nodes run", () => { expect(params).toMatchObject({ id: expect.any(String), command: "echo hi", + commandArgv: ["echo", "hi"], + systemRunPlan: expect.objectContaining({ + argv: ["echo", "hi"], + }), nodeId: NODE_ID, host: "node", timeoutMs: 120_000, @@ -429,11 +463,26 @@ describe("nodes run", () => { }); it("fails with user denied when approval decision is deny", async () => { - callGateway.mockImplementation(async ({ method }) => { + callGateway.mockImplementation(async ({ method, params }) => { if (method === "node.list") { return mockNodeList(["system.run"]); } if (method === "node.invoke") { + const command = (params as { command?: string } | undefined)?.command; + if (command === "system.run.prepare") { + return { + payload: { + cmdText: "echo hi", + plan: { + argv: ["echo", "hi"], + cwd: null, + rawCommand: "echo hi", + agentId: null, + sessionKey: null, + }, + }, + }; + } throw new Error("SYSTEM_RUN_DENIED: approval required"); } if (method === "exec.approval.request") { @@ -446,11 +495,26 @@ describe("nodes run", () => { }); it("fails closed for timeout and invalid approval decisions", async () => { - callGateway.mockImplementation(async ({ method }) => { + callGateway.mockImplementation(async ({ method, params }) => { if (method === "node.list") { return mockNodeList(["system.run"]); } if (method === "node.invoke") { + const command = (params as { command?: string } | undefined)?.command; + if (command === "system.run.prepare") { + return { + payload: { + cmdText: "echo hi", + plan: { + argv: ["echo", "hi"], + cwd: null, + rawCommand: "echo hi", + agentId: null, + sessionKey: null, + }, + }, + }; + } throw new Error("SYSTEM_RUN_DENIED: approval required"); } if (method === "exec.approval.request") { @@ -460,11 +524,26 @@ describe("nodes run", () => { }); await expect(executeNodes(BASE_RUN_INPUT)).rejects.toThrow("exec denied: approval timed out"); - callGateway.mockImplementation(async ({ method }) => { + callGateway.mockImplementation(async ({ method, params }) => { if (method === "node.list") { return mockNodeList(["system.run"]); } if (method === "node.invoke") { + const command = (params as { command?: string } | undefined)?.command; + if (command === "system.run.prepare") { + return { + payload: { + cmdText: "echo hi", + plan: { + argv: ["echo", "hi"], + cwd: null, + rawCommand: "echo hi", + agentId: null, + sessionKey: null, + }, + }, + }; + } throw new Error("SYSTEM_RUN_DENIED: approval required"); } if (method === "exec.approval.request") { diff --git a/src/agents/tools/nodes-tool.ts b/src/agents/tools/nodes-tool.ts index f10e05ab82e..2ec726a123c 100644 --- a/src/agents/tools/nodes-tool.ts +++ b/src/agents/tools/nodes-tool.ts @@ -18,6 +18,7 @@ import { } from "../../cli/nodes-screen.js"; import { parseDurationMs } from "../../cli/parse-duration.js"; import type { OpenClawConfig } from "../../config/config.js"; +import { parsePreparedSystemRunPayload } from "../../infra/system-run-approval-context.js"; import { formatExecCommand } from "../../infra/system-run-command.js"; import { imageMimeFromFormat } from "../../media/mime.js"; import type { GatewayMessageChannel } from "../../utils/message-channel.js"; @@ -530,14 +531,36 @@ export function createNodesTool(options?: { typeof params.needsScreenRecording === "boolean" ? params.needsScreenRecording : undefined; + const prepareRaw = await callGatewayTool<{ payload?: unknown }>( + "node.invoke", + gatewayOpts, + { + nodeId, + command: "system.run.prepare", + params: { + command, + rawCommand: formatExecCommand(command), + cwd, + agentId, + sessionKey, + }, + timeoutMs: invokeTimeoutMs, + idempotencyKey: crypto.randomUUID(), + }, + ); + const prepared = parsePreparedSystemRunPayload(prepareRaw?.payload); + if (!prepared) { + throw new Error("invalid system.run.prepare response"); + } const runParams = { - command, - cwd, + command: prepared.plan.argv, + rawCommand: prepared.plan.rawCommand ?? prepared.cmdText, + cwd: prepared.plan.cwd ?? cwd, env, timeoutMs: commandTimeoutMs, needsScreenRecording, - agentId, - sessionKey, + agentId: prepared.plan.agentId ?? agentId, + sessionKey: prepared.plan.sessionKey ?? sessionKey, }; // First attempt without approval flags. @@ -560,20 +583,20 @@ export function createNodesTool(options?: { // Node requires approval – create a pending approval request on // the gateway and wait for the user to approve/deny via the UI. const APPROVAL_TIMEOUT_MS = 120_000; - const cmdText = formatExecCommand(command); const approvalId = crypto.randomUUID(); const approvalResult = await callGatewayTool( "exec.approval.request", { ...gatewayOpts, timeoutMs: APPROVAL_TIMEOUT_MS + 5_000 }, { id: approvalId, - command: cmdText, - commandArgv: command, - cwd, + command: prepared.cmdText, + commandArgv: prepared.plan.argv, + systemRunPlan: prepared.plan, + cwd: prepared.plan.cwd ?? cwd, nodeId, host: "node", - agentId, - sessionKey, + agentId: prepared.plan.agentId ?? agentId, + sessionKey: prepared.plan.sessionKey ?? sessionKey, turnSourceChannel, turnSourceTo, turnSourceAccountId, diff --git a/src/cli/nodes-cli.coverage.test.ts b/src/cli/nodes-cli.coverage.test.ts index 2670586662a..f66373a52bc 100644 --- a/src/cli/nodes-cli.coverage.test.ts +++ b/src/cli/nodes-cli.coverage.test.ts @@ -47,7 +47,6 @@ const callGateway = vi.fn(async (opts: NodeInvokeCall) => { payload: { cmdText: rawCommand ?? argv.join(" "), plan: { - version: 2, argv, cwd: typeof params.cwd === "string" ? params.cwd : null, rawCommand, @@ -185,8 +184,7 @@ describe("nodes-cli coverage", () => { expect(invoke?.params?.timeoutMs).toBe(5000); const approval = getApprovalRequestCall(); expect(approval?.params?.["commandArgv"]).toEqual(["echo", "hi"]); - expect(approval?.params?.["systemRunPlanV2"]).toEqual({ - version: 2, + expect(approval?.params?.["systemRunPlan"]).toEqual({ argv: ["echo", "hi"], cwd: "/tmp", rawCommand: null, @@ -220,8 +218,7 @@ describe("nodes-cli coverage", () => { }); const approval = getApprovalRequestCall(); expect(approval?.params?.["commandArgv"]).toEqual(["/bin/sh", "-lc", "echo hi"]); - expect(approval?.params?.["systemRunPlanV2"]).toEqual({ - version: 2, + expect(approval?.params?.["systemRunPlan"]).toEqual({ argv: ["/bin/sh", "-lc", "echo hi"], cwd: null, rawCommand: "echo hi", diff --git a/src/cli/nodes-cli/register.invoke.ts b/src/cli/nodes-cli/register.invoke.ts index e38f329f208..d23d35c9f21 100644 --- a/src/cli/nodes-cli/register.invoke.ts +++ b/src/cli/nodes-cli/register.invoke.ts @@ -228,7 +228,7 @@ async function maybeRequestNodesRunApproval(params: { id: approvalId, command: params.preparedCmdText, commandArgv: params.approvalPlan.argv, - systemRunPlanV2: params.approvalPlan, + systemRunPlan: params.approvalPlan, cwd: params.approvalPlan.cwd, nodeId: params.nodeId, host: "node", diff --git a/src/gateway/node-invoke-system-run-approval-match.test.ts b/src/gateway/node-invoke-system-run-approval-match.test.ts index 9ba85d5350d..4f6d5d84c52 100644 --- a/src/gateway/node-invoke-system-run-approval-match.test.ts +++ b/src/gateway/node-invoke-system-run-approval-match.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "vitest"; -import { buildSystemRunApprovalBindingV1 } from "../infra/system-run-approval-binding.js"; +import { buildSystemRunApprovalBinding } from "../infra/system-run-approval-binding.js"; import { evaluateSystemRunApprovalMatch } from "./node-invoke-system-run-approval-match.js"; describe("evaluateSystemRunApprovalMatch", () => { @@ -29,7 +29,7 @@ describe("evaluateSystemRunApprovalMatch", () => { request: { host: "node", command: "echo SAFE", - systemRunBindingV1: buildSystemRunApprovalBindingV1({ + systemRunBinding: buildSystemRunApprovalBinding({ argv: ["echo", "SAFE"], cwd: null, agentId: null, @@ -51,7 +51,7 @@ describe("evaluateSystemRunApprovalMatch", () => { request: { host: "node", command: "echo SAFE", - systemRunBindingV1: buildSystemRunApprovalBindingV1({ + systemRunBinding: buildSystemRunApprovalBinding({ argv: ["echo SAFE"], cwd: null, agentId: null, @@ -77,7 +77,7 @@ describe("evaluateSystemRunApprovalMatch", () => { request: { host: "node", command: "git diff", - systemRunBindingV1: buildSystemRunApprovalBindingV1({ + systemRunBinding: buildSystemRunApprovalBinding({ argv: ["git", "diff"], cwd: null, agentId: null, @@ -104,7 +104,7 @@ describe("evaluateSystemRunApprovalMatch", () => { request: { host: "node", command: "git diff", - systemRunBindingV1: buildSystemRunApprovalBindingV1({ + systemRunBinding: buildSystemRunApprovalBinding({ argv: ["git", "diff"], cwd: null, agentId: null, @@ -149,7 +149,7 @@ describe("evaluateSystemRunApprovalMatch", () => { host: "node", command: "echo STALE", commandArgv: ["echo STALE"], - systemRunBindingV1: buildSystemRunApprovalBindingV1({ + systemRunBinding: buildSystemRunApprovalBinding({ argv: ["echo", "SAFE"], cwd: null, agentId: null, diff --git a/src/gateway/node-invoke-system-run-approval-match.ts b/src/gateway/node-invoke-system-run-approval-match.ts index c67231f760c..e52ee0e56ea 100644 --- a/src/gateway/node-invoke-system-run-approval-match.ts +++ b/src/gateway/node-invoke-system-run-approval-match.ts @@ -1,8 +1,8 @@ import type { ExecApprovalRequestPayload } from "../infra/exec-approvals.js"; import { - buildSystemRunApprovalBindingV1, - missingSystemRunApprovalBindingV1, - matchSystemRunApprovalBindingV1, + buildSystemRunApprovalBinding, + missingSystemRunApprovalBinding, + matchSystemRunApprovalBinding, type SystemRunApprovalMatchResult, } from "../infra/system-run-approval-binding.js"; @@ -33,7 +33,7 @@ export function evaluateSystemRunApprovalMatch(params: { return requestMismatch(); } - const actualBinding = buildSystemRunApprovalBindingV1({ + const actualBinding = buildSystemRunApprovalBinding({ argv: params.argv, cwd: params.binding.cwd, agentId: params.binding.agentId, @@ -41,13 +41,13 @@ export function evaluateSystemRunApprovalMatch(params: { env: params.binding.env, }); - const expectedBinding = params.request.systemRunBindingV1; + const expectedBinding = params.request.systemRunBinding; if (!expectedBinding) { - return missingSystemRunApprovalBindingV1({ + return missingSystemRunApprovalBinding({ actualEnvKeys: actualBinding.envKeys, }); } - return matchSystemRunApprovalBindingV1({ + return matchSystemRunApprovalBinding({ expected: expectedBinding, actual: actualBinding.binding, actualEnvKeys: actualBinding.envKeys, diff --git a/src/gateway/node-invoke-system-run-approval.test.ts b/src/gateway/node-invoke-system-run-approval.test.ts index 50798323a3b..dfffe562170 100644 --- a/src/gateway/node-invoke-system-run-approval.test.ts +++ b/src/gateway/node-invoke-system-run-approval.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "vitest"; import { - buildSystemRunApprovalBindingV1, + buildSystemRunApprovalBinding, buildSystemRunApprovalEnvBinding, } from "../infra/system-run-approval-binding.js"; import { ExecApprovalManager, type ExecApprovalRecord } from "./exec-approval-manager.js"; @@ -30,7 +30,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { nodeId: "node-1", command, commandArgv, - systemRunBindingV1: buildSystemRunApprovalBindingV1({ + systemRunBinding: buildSystemRunApprovalBinding({ argv: effectiveBindingArgv, cwd: null, agentId: null, @@ -229,17 +229,16 @@ describe("sanitizeSystemRunParamsForForwarding", () => { expectAllowOnceForwardingResult(result); }); - test("uses systemRunPlanV2 for forwarded command context and ignores caller tampering", () => { + test("uses systemRunPlan for forwarded command context and ignores caller tampering", () => { const record = makeRecord("echo SAFE", ["echo", "SAFE"]); - record.request.systemRunPlanV2 = { - version: 2, + record.request.systemRunPlan = { argv: ["/usr/bin/echo", "SAFE"], cwd: "/real/cwd", rawCommand: "/usr/bin/echo SAFE", agentId: "main", sessionKey: "agent:main:main", }; - record.request.systemRunBindingV1 = buildSystemRunApprovalBindingV1({ + record.request.systemRunBinding = buildSystemRunApprovalBinding({ argv: ["/usr/bin/echo", "SAFE"], cwd: "/real/cwd", agentId: "main", @@ -297,8 +296,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { test("rejects env hash mismatch", () => { const record = makeRecord("git diff", ["git", "diff"]); - record.request.systemRunBindingV1 = { - version: 1, + record.request.systemRunBinding = { argv: ["git", "diff"], cwd: null, agentId: null, @@ -329,8 +327,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { test("accepts matching env hash with reordered keys", () => { const record = makeRecord("git diff", ["git", "diff"]); const binding = buildSystemRunApprovalEnvBinding({ SAFE_A: "1", SAFE_B: "2" }); - record.request.systemRunBindingV1 = { - version: 1, + record.request.systemRunBinding = { argv: ["git", "diff"], cwd: null, agentId: null, @@ -363,7 +360,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { nodeId: "node-1", command: "echo SAFE", commandArgv: ["echo", "SAFE"], - systemRunBindingV1: buildSystemRunApprovalBindingV1({ + systemRunBinding: buildSystemRunApprovalBinding({ argv: ["echo", "SAFE"], cwd: null, agentId: null, diff --git a/src/gateway/node-invoke-system-run-approval.ts b/src/gateway/node-invoke-system-run-approval.ts index efee11572b1..cf182559b9d 100644 --- a/src/gateway/node-invoke-system-run-approval.ts +++ b/src/gateway/node-invoke-system-run-approval.ts @@ -209,7 +209,7 @@ export function sanitizeSystemRunParamsForForwarding(opts: { } const runtimeContext = resolveSystemRunApprovalRuntimeContext({ - planV2: snapshot.request.systemRunPlanV2 ?? null, + plan: snapshot.request.systemRunPlan ?? null, command: p.command, rawCommand: p.rawCommand, cwd: p.cwd, @@ -223,8 +223,8 @@ export function sanitizeSystemRunParamsForForwarding(opts: { details: runtimeContext.details, }; } - if (runtimeContext.planV2) { - next.command = [...runtimeContext.planV2.argv]; + if (runtimeContext.plan) { + next.command = [...runtimeContext.plan.argv]; if (runtimeContext.rawCommand) { next.rawCommand = runtimeContext.rawCommand; } else { diff --git a/src/gateway/protocol/schema/exec-approvals.ts b/src/gateway/protocol/schema/exec-approvals.ts index 0358cde48fe..d7773c6b418 100644 --- a/src/gateway/protocol/schema/exec-approvals.ts +++ b/src/gateway/protocol/schema/exec-approvals.ts @@ -90,10 +90,9 @@ export const ExecApprovalRequestParamsSchema = Type.Object( id: Type.Optional(NonEmptyString), command: NonEmptyString, commandArgv: Type.Optional(Type.Array(Type.String())), - systemRunPlanV2: Type.Optional( + systemRunPlan: Type.Optional( Type.Object( { - version: Type.Literal(2), argv: Type.Array(Type.String()), cwd: Type.Union([Type.String(), Type.Null()]), rawCommand: Type.Union([Type.String(), Type.Null()]), diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 2d362efa214..8d040e25e37 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -3,7 +3,7 @@ import { DEFAULT_EXEC_APPROVAL_TIMEOUT_MS, type ExecApprovalDecision, } from "../../infra/exec-approvals.js"; -import { buildSystemRunApprovalBindingV1 } from "../../infra/system-run-approval-binding.js"; +import { buildSystemRunApprovalBinding } from "../../infra/system-run-approval-binding.js"; import { resolveSystemRunApprovalRequestContext } from "../../infra/system-run-approval-context.js"; import type { ExecApprovalManager } from "../exec-approval-manager.js"; import { @@ -48,7 +48,7 @@ export function createExecApprovalHandlers( commandArgv?: string[]; env?: Record; cwd?: string; - systemRunPlanV2?: unknown; + systemRunPlan?: unknown; nodeId?: string; host?: string; security?: string; @@ -73,7 +73,7 @@ export function createExecApprovalHandlers( host, command: p.command, commandArgv: p.commandArgv, - systemRunPlanV2: p.systemRunPlanV2, + systemRunPlan: p.systemRunPlan, cwd: p.cwd, agentId: p.agentId, sessionKey: p.sessionKey, @@ -91,6 +91,14 @@ export function createExecApprovalHandlers( ); return; } + if (host === "node" && !approvalContext.plan) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "systemRunPlan is required for host=node"), + ); + return; + } if ( host === "node" && (!Array.isArray(effectiveCommandArgv) || effectiveCommandArgv.length === 0) @@ -102,9 +110,9 @@ export function createExecApprovalHandlers( ); return; } - const systemRunBindingV1 = + const systemRunBinding = host === "node" - ? buildSystemRunApprovalBindingV1({ + ? buildSystemRunApprovalBinding({ argv: effectiveCommandArgv, cwd: effectiveCwd, agentId: effectiveAgentId, @@ -123,9 +131,9 @@ export function createExecApprovalHandlers( const request = { command: effectiveCommandText, commandArgv: effectiveCommandArgv, - envKeys: systemRunBindingV1?.envKeys?.length ? systemRunBindingV1.envKeys : undefined, - systemRunBindingV1: systemRunBindingV1?.binding ?? null, - systemRunPlanV2: approvalContext.planV2, + envKeys: systemRunBinding?.envKeys?.length ? systemRunBinding.envKeys : undefined, + systemRunBinding: systemRunBinding?.binding ?? null, + systemRunPlan: approvalContext.plan, cwd: effectiveCwd ?? null, nodeId: host === "node" ? nodeId : null, host: host || null, diff --git a/src/gateway/server-methods/server-methods.test.ts b/src/gateway/server-methods/server-methods.test.ts index c6db927093a..c3c049cfe4b 100644 --- a/src/gateway/server-methods/server-methods.test.ts +++ b/src/gateway/server-methods/server-methods.test.ts @@ -6,7 +6,7 @@ import { fileURLToPath } from "node:url"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { emitAgentEvent } from "../../infra/agent-events.js"; import { formatZonedTimestamp } from "../../infra/format-time/format-datetime.js"; -import { buildSystemRunApprovalBindingV1 } from "../../infra/system-run-approval-binding.js"; +import { buildSystemRunApprovalBinding } from "../../infra/system-run-approval-binding.js"; import { resetLogger, setLoggerOverride } from "../../logging.js"; import { ExecApprovalManager } from "../exec-approval-manager.js"; import { validateExecApprovalRequestParams } from "../protocol/index.js"; @@ -249,6 +249,13 @@ describe("exec approval handlers", () => { const defaultExecApprovalRequestParams = { command: "echo ok", commandArgv: ["echo", "ok"], + systemRunPlan: { + argv: ["/usr/bin/echo", "ok"], + cwd: "/tmp", + rawCommand: "/usr/bin/echo ok", + agentId: "main", + sessionKey: "agent:main:main", + }, cwd: "/tmp", nodeId: "node-1", host: "node", @@ -278,6 +285,37 @@ describe("exec approval handlers", () => { ...defaultExecApprovalRequestParams, ...params.params, } as unknown as ExecApprovalRequestArgs["params"]; + const hasExplicitPlan = !!params.params && Object.hasOwn(params.params, "systemRunPlan"); + if ( + !hasExplicitPlan && + (requestParams as { host?: string }).host === "node" && + Array.isArray((requestParams as { commandArgv?: unknown }).commandArgv) + ) { + const commandArgv = (requestParams as { commandArgv: unknown[] }).commandArgv.map((entry) => + String(entry), + ); + const cwdValue = + typeof (requestParams as { cwd?: unknown }).cwd === "string" + ? ((requestParams as { cwd: string }).cwd ?? null) + : null; + const commandText = + typeof (requestParams as { command?: unknown }).command === "string" + ? ((requestParams as { command: string }).command ?? null) + : null; + requestParams.systemRunPlan = { + argv: commandArgv, + cwd: cwdValue, + rawCommand: commandText, + agentId: + typeof (requestParams as { agentId?: unknown }).agentId === "string" + ? ((requestParams as { agentId: string }).agentId ?? null) + : null, + sessionKey: + typeof (requestParams as { sessionKey?: unknown }).sessionKey === "string" + ? ((requestParams as { sessionKey: string }).sessionKey ?? null) + : null, + }; + } return params.handlers["exec.approval.request"]({ params: requestParams, respond: params.respond as unknown as ExecApprovalRequestArgs["respond"], @@ -385,21 +423,21 @@ describe("exec approval handlers", () => { ); }); - it("rejects host=node approval requests without commandArgv", async () => { + it("rejects host=node approval requests without systemRunPlan", async () => { const { handlers, respond, context } = createExecApprovalFixture(); await requestExecApproval({ handlers, respond, context, params: { - commandArgv: undefined, + systemRunPlan: undefined, }, }); expect(respond).toHaveBeenCalledWith( false, undefined, expect.objectContaining({ - message: "commandArgv is required for host=node", + message: "systemRunPlan is required for host=node", }), ); }); @@ -462,8 +500,8 @@ describe("exec approval handlers", () => { expect(requested).toBeTruthy(); const request = (requested?.payload as { request?: Record })?.request ?? {}; expect(request["envKeys"]).toEqual(["A_VAR", "Z_VAR"]); - expect(request["systemRunBindingV1"]).toEqual( - buildSystemRunApprovalBindingV1({ + expect(request["systemRunBinding"]).toEqual( + buildSystemRunApprovalBinding({ argv: ["echo", "ok"], cwd: "/tmp", env: { A_VAR: "a", Z_VAR: "z" }, @@ -471,7 +509,7 @@ describe("exec approval handlers", () => { ); }); - it("prefers systemRunPlanV2 canonical command/cwd when present", async () => { + it("prefers systemRunPlan canonical command/cwd when present", async () => { const { handlers, broadcasts, respond, context } = createExecApprovalFixture(); await requestExecApproval({ handlers, @@ -481,8 +519,7 @@ describe("exec approval handlers", () => { command: "echo stale", commandArgv: ["echo", "stale"], cwd: "/tmp/link/sub", - systemRunPlanV2: { - version: 2, + systemRunPlan: { argv: ["/usr/bin/echo", "ok"], cwd: "/real/cwd", rawCommand: "/usr/bin/echo ok", @@ -499,8 +536,7 @@ describe("exec approval handlers", () => { expect(request["cwd"]).toBe("/real/cwd"); expect(request["agentId"]).toBe("main"); expect(request["sessionKey"]).toBe("agent:main:main"); - expect(request["systemRunPlanV2"]).toEqual({ - version: 2, + expect(request["systemRunPlan"]).toEqual({ argv: ["/usr/bin/echo", "ok"], cwd: "/real/cwd", rawCommand: "/usr/bin/echo ok", diff --git a/src/gateway/server.node-invoke-approval-bypass.test.ts b/src/gateway/server.node-invoke-approval-bypass.test.ts index 0e01a9619b9..9df14115c83 100644 --- a/src/gateway/server.node-invoke-approval-bypass.test.ts +++ b/src/gateway/server.node-invoke-approval-bypass.test.ts @@ -80,6 +80,13 @@ async function requestAllowOnceApproval( id: approvalId, command, commandArgv, + systemRunPlan: { + argv: commandArgv, + cwd: null, + rawCommand: command, + agentId: null, + sessionKey: null, + }, nodeId, cwd: null, host: "node", diff --git a/src/gateway/system-run-approval-binding.contract.test.ts b/src/gateway/system-run-approval-binding.contract.test.ts index 48976c3bdc5..5d78a140631 100644 --- a/src/gateway/system-run-approval-binding.contract.test.ts +++ b/src/gateway/system-run-approval-binding.contract.test.ts @@ -3,7 +3,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { describe, expect, test } from "vitest"; import type { ExecApprovalRequestPayload } from "../infra/exec-approvals.js"; -import { buildSystemRunApprovalBindingV1 } from "../infra/system-run-approval-binding.js"; +import { buildSystemRunApprovalBinding } from "../infra/system-run-approval-binding.js"; import { evaluateSystemRunApprovalMatch } from "./node-invoke-system-run-approval-match.js"; type FixtureCase = { @@ -15,7 +15,7 @@ type FixtureCase = { cwd?: string | null; agentId?: string | null; sessionKey?: string | null; - bindingV1?: { + binding?: { argv: string[]; cwd?: string | null; agentId?: string | null; @@ -57,13 +57,13 @@ function buildRequestPayload(entry: FixtureCase): ExecApprovalRequestPayload { agentId: entry.request.agentId ?? null, sessionKey: entry.request.sessionKey ?? null, }; - if (entry.request.bindingV1) { - payload.systemRunBindingV1 = buildSystemRunApprovalBindingV1({ - argv: entry.request.bindingV1.argv, - cwd: entry.request.bindingV1.cwd, - agentId: entry.request.bindingV1.agentId, - sessionKey: entry.request.bindingV1.sessionKey, - env: entry.request.bindingV1.env, + if (entry.request.binding) { + payload.systemRunBinding = buildSystemRunApprovalBinding({ + argv: entry.request.binding.argv, + cwd: entry.request.binding.cwd, + agentId: entry.request.binding.agentId, + sessionKey: entry.request.binding.sessionKey, + env: entry.request.binding.env, }).binding; } return payload; diff --git a/src/gateway/system-run-approval-binding.test.ts b/src/gateway/system-run-approval-binding.test.ts index 383b2895ffd..4c6205e6409 100644 --- a/src/gateway/system-run-approval-binding.test.ts +++ b/src/gateway/system-run-approval-binding.test.ts @@ -1,8 +1,8 @@ import { describe, expect, test } from "vitest"; import { - buildSystemRunApprovalBindingV1, + buildSystemRunApprovalBinding, buildSystemRunApprovalEnvBinding, - matchSystemRunApprovalBindingV1, + matchSystemRunApprovalBinding, matchSystemRunApprovalEnvHash, toSystemRunApprovalMismatchError, } from "../infra/system-run-approval-binding.js"; @@ -48,16 +48,16 @@ describe("matchSystemRunApprovalEnvHash", () => { }); }); -describe("matchSystemRunApprovalBindingV1", () => { +describe("matchSystemRunApprovalBinding", () => { test("accepts matching binding with reordered env keys", () => { - const expected = buildSystemRunApprovalBindingV1({ + const expected = buildSystemRunApprovalBinding({ argv: ["git", "diff"], cwd: null, agentId: null, sessionKey: null, env: { SAFE_A: "1", SAFE_B: "2" }, }); - const actual = buildSystemRunApprovalBindingV1({ + const actual = buildSystemRunApprovalBinding({ argv: ["git", "diff"], cwd: null, agentId: null, @@ -65,7 +65,7 @@ describe("matchSystemRunApprovalBindingV1", () => { env: { SAFE_B: "2", SAFE_A: "1" }, }); expect( - matchSystemRunApprovalBindingV1({ + matchSystemRunApprovalBinding({ expected: expected.binding, actual: actual.binding, actualEnvKeys: actual.envKeys, @@ -74,21 +74,21 @@ describe("matchSystemRunApprovalBindingV1", () => { }); test("rejects env mismatch", () => { - const expected = buildSystemRunApprovalBindingV1({ + const expected = buildSystemRunApprovalBinding({ argv: ["git", "diff"], cwd: null, agentId: null, sessionKey: null, env: { SAFE: "1" }, }); - const actual = buildSystemRunApprovalBindingV1({ + const actual = buildSystemRunApprovalBinding({ argv: ["git", "diff"], cwd: null, agentId: null, sessionKey: null, env: { SAFE: "2" }, }); - const result = matchSystemRunApprovalBindingV1({ + const result = matchSystemRunApprovalBinding({ expected: expected.binding, actual: actual.binding, actualEnvKeys: actual.envKeys, diff --git a/src/infra/exec-approvals-analysis.ts b/src/infra/exec-approvals-analysis.ts index 8d2fe38c973..e28e0e5c673 100644 --- a/src/infra/exec-approvals-analysis.ts +++ b/src/infra/exec-approvals-analysis.ts @@ -626,7 +626,7 @@ function renderQuotedArgv(argv: string[]): string { return argv.map((token) => shellEscapeSingleArg(token)).join(" "); } -function resolvePlannedSegmentArgv(segment: ExecCommandSegment): string[] | null { +export function resolvePlannedSegmentArgv(segment: ExecCommandSegment): string[] | null { if (segment.resolution?.policyBlocked === true) { return null; } @@ -638,7 +638,8 @@ function resolvePlannedSegmentArgv(segment: ExecCommandSegment): string[] | null return null; } const argv = [...baseArgv]; - const resolvedExecutable = segment.resolution?.resolvedPath?.trim() ?? ""; + const resolvedExecutable = + segment.resolution?.resolvedRealPath?.trim() ?? segment.resolution?.resolvedPath?.trim() ?? ""; if (resolvedExecutable) { argv[0] = resolvedExecutable; } diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index b48a65e02ca..c99eaeef189 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -11,8 +11,7 @@ export type ExecHost = "sandbox" | "gateway" | "node"; export type ExecSecurity = "deny" | "allowlist" | "full"; export type ExecAsk = "off" | "on-miss" | "always"; -export type SystemRunApprovalBindingV1 = { - version: 1; +export type SystemRunApprovalBinding = { argv: string[]; cwd: string | null; agentId: string | null; @@ -20,8 +19,7 @@ export type SystemRunApprovalBindingV1 = { envHash: string | null; }; -export type SystemRunApprovalPlanV2 = { - version: 2; +export type SystemRunApprovalPlan = { argv: string[]; cwd: string | null; rawCommand: string | null; @@ -34,8 +32,8 @@ export type ExecApprovalRequestPayload = { commandArgv?: string[]; // Optional UI-safe env key preview for approval prompts. envKeys?: string[]; - systemRunBindingV1?: SystemRunApprovalBindingV1 | null; - systemRunPlanV2?: SystemRunApprovalPlanV2 | null; + systemRunBinding?: SystemRunApprovalBinding | null; + systemRunPlan?: SystemRunApprovalPlan | null; cwd?: string | null; nodeId?: string | null; host?: string | null; diff --git a/src/infra/exec-command-resolution.ts b/src/infra/exec-command-resolution.ts index d102a1030f1..d69edbf113f 100644 --- a/src/infra/exec-command-resolution.ts +++ b/src/infra/exec-command-resolution.ts @@ -9,6 +9,7 @@ export const DEFAULT_SAFE_BINS = ["jq", "cut", "uniq", "head", "tail", "tr", "wc export type CommandResolution = { rawExecutable: string; resolvedPath?: string; + resolvedRealPath?: string; executableName: string; effectiveArgv?: string[]; wrapperChain?: string[]; @@ -86,6 +87,17 @@ function resolveExecutablePath(rawExecutable: string, cwd?: string, env?: NodeJS return undefined; } +function tryResolveRealpath(filePath: string | undefined): string | undefined { + if (!filePath) { + return undefined; + } + try { + return fs.realpathSync(filePath); + } catch { + return undefined; + } +} + export function resolveCommandResolution( command: string, cwd?: string, @@ -96,10 +108,12 @@ export function resolveCommandResolution( return null; } const resolvedPath = resolveExecutablePath(rawExecutable, cwd, env); + const resolvedRealPath = tryResolveRealpath(resolvedPath); const executableName = resolvedPath ? path.basename(resolvedPath) : rawExecutable; return { rawExecutable, resolvedPath, + resolvedRealPath, executableName, effectiveArgv: [rawExecutable], wrapperChain: [], @@ -119,10 +133,12 @@ export function resolveCommandResolutionFromArgv( return null; } const resolvedPath = resolveExecutablePath(rawExecutable, cwd, env); + const resolvedRealPath = tryResolveRealpath(resolvedPath); const executableName = resolvedPath ? path.basename(resolvedPath) : rawExecutable; return { rawExecutable, resolvedPath, + resolvedRealPath, executableName, effectiveArgv, wrapperChain: plan.wrappers, diff --git a/src/infra/system-run-approval-binding.ts b/src/infra/system-run-approval-binding.ts index a760f4948ef..936ba9b0ec3 100644 --- a/src/infra/system-run-approval-binding.ts +++ b/src/infra/system-run-approval-binding.ts @@ -1,5 +1,5 @@ import crypto from "node:crypto"; -import type { SystemRunApprovalBindingV1, SystemRunApprovalPlanV2 } from "./exec-approvals.js"; +import type { SystemRunApprovalBinding, SystemRunApprovalPlan } from "./exec-approvals.js"; import { normalizeEnvVarKey } from "./host-env-security.js"; type NormalizedSystemRunEnvEntry = [key: string, value: string]; @@ -16,20 +16,16 @@ function normalizeStringArray(value: unknown): string[] { return Array.isArray(value) ? value.map((entry) => String(entry)) : []; } -export function normalizeSystemRunApprovalPlanV2(value: unknown): SystemRunApprovalPlanV2 | null { +export function normalizeSystemRunApprovalPlan(value: unknown): SystemRunApprovalPlan | null { if (!value || typeof value !== "object" || Array.isArray(value)) { return null; } const candidate = value as Record; - if (candidate.version !== 2) { - return null; - } const argv = normalizeStringArray(candidate.argv); if (argv.length === 0) { return null; } return { - version: 2, argv, cwd: normalizeString(candidate.cwd), rawCommand: normalizeString(candidate.rawCommand), @@ -75,17 +71,16 @@ export function buildSystemRunApprovalEnvBinding(env: unknown): { }; } -export function buildSystemRunApprovalBindingV1(params: { +export function buildSystemRunApprovalBinding(params: { argv: unknown; cwd?: unknown; agentId?: unknown; sessionKey?: unknown; env?: unknown; -}): { binding: SystemRunApprovalBindingV1; envKeys: string[] } { +}): { binding: SystemRunApprovalBinding; envKeys: string[] } { const envBinding = buildSystemRunApprovalEnvBinding(params.env); return { binding: { - version: 1, argv: normalizeStringArray(params.argv), cwd: normalizeString(params.cwd), agentId: normalizeString(params.agentId), @@ -161,17 +156,11 @@ export function matchSystemRunApprovalEnvHash(params: { return { ok: true }; } -export function matchSystemRunApprovalBindingV1(params: { - expected: SystemRunApprovalBindingV1; - actual: SystemRunApprovalBindingV1; +export function matchSystemRunApprovalBinding(params: { + expected: SystemRunApprovalBinding; + actual: SystemRunApprovalBinding; actualEnvKeys: string[]; }): SystemRunApprovalMatchResult { - if (params.expected.version !== 1 || params.actual.version !== 1) { - return requestMismatch({ - expectedVersion: params.expected.version, - actualVersion: params.actual.version, - }); - } if (!argvMatches(params.expected.argv, params.actual.argv)) { return requestMismatch(); } @@ -191,11 +180,10 @@ export function matchSystemRunApprovalBindingV1(params: { }); } -export function missingSystemRunApprovalBindingV1(params: { +export function missingSystemRunApprovalBinding(params: { actualEnvKeys: string[]; }): SystemRunApprovalMatchResult { return requestMismatch({ - requiredBindingVersion: 1, envKeys: params.actualEnvKeys, }); } diff --git a/src/infra/system-run-approval-context.ts b/src/infra/system-run-approval-context.ts index 25cbee1fcfc..9d01206b8b1 100644 --- a/src/infra/system-run-approval-context.ts +++ b/src/infra/system-run-approval-context.ts @@ -1,14 +1,14 @@ -import type { SystemRunApprovalPlanV2 } from "./exec-approvals.js"; -import { normalizeSystemRunApprovalPlanV2 } from "./system-run-approval-binding.js"; +import type { SystemRunApprovalPlan } from "./exec-approvals.js"; +import { normalizeSystemRunApprovalPlan } from "./system-run-approval-binding.js"; import { formatExecCommand, resolveSystemRunCommand } from "./system-run-command.js"; type PreparedRunPayload = { cmdText: string; - plan: SystemRunApprovalPlanV2; + plan: SystemRunApprovalPlan; }; type SystemRunApprovalRequestContext = { - planV2: SystemRunApprovalPlanV2 | null; + plan: SystemRunApprovalPlan | null; commandArgv: string[] | undefined; commandText: string; cwd: string | null; @@ -19,7 +19,7 @@ type SystemRunApprovalRequestContext = { type SystemRunApprovalRuntimeContext = | { ok: true; - planV2: SystemRunApprovalPlanV2 | null; + plan: SystemRunApprovalPlan | null; argv: string[]; cwd: string | null; agentId: string | null; @@ -54,7 +54,7 @@ export function parsePreparedSystemRunPayload(payload: unknown): PreparedRunPayl } const raw = payload as { cmdText?: unknown; plan?: unknown }; const cmdText = normalizeString(raw.cmdText); - const plan = normalizeSystemRunApprovalPlanV2(raw.plan); + const plan = normalizeSystemRunApprovalPlan(raw.plan); if (!cmdText || !plan) { return null; } @@ -65,38 +65,38 @@ export function resolveSystemRunApprovalRequestContext(params: { host?: unknown; command?: unknown; commandArgv?: unknown; - systemRunPlanV2?: unknown; + systemRunPlan?: unknown; cwd?: unknown; agentId?: unknown; sessionKey?: unknown; }): SystemRunApprovalRequestContext { const host = normalizeString(params.host) ?? ""; - const planV2 = host === "node" ? normalizeSystemRunApprovalPlanV2(params.systemRunPlanV2) : null; + const plan = host === "node" ? normalizeSystemRunApprovalPlan(params.systemRunPlan) : null; const fallbackArgv = normalizeStringArray(params.commandArgv); const fallbackCommand = normalizeCommandText(params.command); return { - planV2, - commandArgv: planV2?.argv ?? (fallbackArgv.length > 0 ? fallbackArgv : undefined), - commandText: planV2 ? (planV2.rawCommand ?? formatExecCommand(planV2.argv)) : fallbackCommand, - cwd: planV2?.cwd ?? normalizeString(params.cwd), - agentId: planV2?.agentId ?? normalizeString(params.agentId), - sessionKey: planV2?.sessionKey ?? normalizeString(params.sessionKey), + plan, + commandArgv: plan?.argv ?? (fallbackArgv.length > 0 ? fallbackArgv : undefined), + commandText: plan ? (plan.rawCommand ?? formatExecCommand(plan.argv)) : fallbackCommand, + cwd: plan?.cwd ?? normalizeString(params.cwd), + agentId: plan?.agentId ?? normalizeString(params.agentId), + sessionKey: plan?.sessionKey ?? normalizeString(params.sessionKey), }; } export function resolveSystemRunApprovalRuntimeContext(params: { - planV2?: unknown; + plan?: unknown; command?: unknown; rawCommand?: unknown; cwd?: unknown; agentId?: unknown; sessionKey?: unknown; }): SystemRunApprovalRuntimeContext { - const normalizedPlan = normalizeSystemRunApprovalPlanV2(params.planV2 ?? null); + const normalizedPlan = normalizeSystemRunApprovalPlan(params.plan ?? null); if (normalizedPlan) { return { ok: true, - planV2: normalizedPlan, + plan: normalizedPlan, argv: [...normalizedPlan.argv], cwd: normalizedPlan.cwd, agentId: normalizedPlan.agentId, @@ -113,7 +113,7 @@ export function resolveSystemRunApprovalRuntimeContext(params: { } return { ok: true, - planV2: null, + plan: null, argv: command.argv, cwd: normalizeString(params.cwd), agentId: normalizeString(params.agentId), diff --git a/src/node-host/invoke-system-run-allowlist.ts b/src/node-host/invoke-system-run-allowlist.ts index b9edc7a5e23..d4817453a70 100644 --- a/src/node-host/invoke-system-run-allowlist.ts +++ b/src/node-host/invoke-system-run-allowlist.ts @@ -2,6 +2,7 @@ import { analyzeArgvCommand, evaluateExecAllowlist, evaluateShellAllowlist, + resolvePlannedSegmentArgv, resolveExecApprovals, type ExecAllowlistEntry, type ExecCommandSegment, @@ -95,7 +96,7 @@ export function resolvePlannedAllowlistArgv(params: { ) { return undefined; } - const plannedAllowlistArgv = params.segments[0]?.resolution?.effectiveArgv; + const plannedAllowlistArgv = resolvePlannedSegmentArgv(params.segments[0]); return plannedAllowlistArgv && plannedAllowlistArgv.length > 0 ? plannedAllowlistArgv : null; } diff --git a/src/node-host/invoke-system-run-plan.ts b/src/node-host/invoke-system-run-plan.ts index 27af0f8bbf3..cbcb4484ca8 100644 --- a/src/node-host/invoke-system-run-plan.ts +++ b/src/node-host/invoke-system-run-plan.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; import path from "node:path"; -import type { SystemRunApprovalPlanV2 } from "../infra/exec-approvals.js"; +import type { SystemRunApprovalPlan } from "../infra/exec-approvals.js"; +import { resolveCommandResolutionFromArgv } from "../infra/exec-command-resolution.js"; import { sameFileIdentity } from "../infra/file-identity.js"; import { resolveSystemRunCommand } from "../infra/system-run-command.js"; @@ -12,22 +13,6 @@ function normalizeString(value: unknown): string | null { return trimmed ? trimmed : null; } -function isPathLikeExecutableToken(value: string): boolean { - if (!value) { - return false; - } - if (value.startsWith(".") || value.startsWith("/") || value.startsWith("\\")) { - return true; - } - if (value.includes("/") || value.includes("\\")) { - return true; - } - if (process.platform === "win32" && /^[a-zA-Z]:[\\/]/.test(value)) { - return true; - } - return false; -} - function pathComponentsFromRootSync(targetPath: string): string[] { const absolute = path.resolve(targetPath); const parts: string[] = []; @@ -71,7 +56,6 @@ function hasMutableSymlinkPathComponentSync(targetPath: string): boolean { export function hardenApprovedExecutionPaths(params: { approvedByAsk: boolean; argv: string[]; - shellCommand: string | null; cwd: string | undefined; }): { ok: true; argv: string[]; cwd: string | undefined } | { ok: false; message: string } { if (!params.approvedByAsk) { @@ -127,38 +111,31 @@ export function hardenApprovedExecutionPaths(params: { hardenedCwd = cwdReal; } - if (params.shellCommand !== null || params.argv.length === 0) { + if (params.argv.length === 0) { return { ok: true, argv: params.argv, cwd: hardenedCwd }; } - const argv = [...params.argv]; - const rawExecutable = argv[0] ?? ""; - if (!isPathLikeExecutableToken(rawExecutable)) { - return { ok: true, argv, cwd: hardenedCwd }; - } - - const base = hardenedCwd ?? process.cwd(); - const candidate = path.isAbsolute(rawExecutable) - ? rawExecutable - : path.resolve(base, rawExecutable); - try { - argv[0] = fs.realpathSync(candidate); - } catch { + const resolution = resolveCommandResolutionFromArgv(params.argv, hardenedCwd); + const pinnedExecutable = resolution?.resolvedRealPath ?? resolution?.resolvedPath; + if (!pinnedExecutable) { return { ok: false, message: "SYSTEM_RUN_DENIED: approval requires a stable executable path", }; } + + const argv = [...params.argv]; + argv[0] = pinnedExecutable; return { ok: true, argv, cwd: hardenedCwd }; } -export function buildSystemRunApprovalPlanV2(params: { +export function buildSystemRunApprovalPlan(params: { command?: unknown; rawCommand?: unknown; cwd?: unknown; agentId?: unknown; sessionKey?: unknown; -}): { ok: true; plan: SystemRunApprovalPlanV2; cmdText: string } | { ok: false; message: string } { +}): { ok: true; plan: SystemRunApprovalPlan; cmdText: string } | { ok: false; message: string } { const command = resolveSystemRunCommand({ command: params.command, rawCommand: params.rawCommand, @@ -172,7 +149,6 @@ export function buildSystemRunApprovalPlanV2(params: { const hardening = hardenApprovedExecutionPaths({ approvedByAsk: true, argv: command.argv, - shellCommand: command.shellCommand, cwd: normalizeString(params.cwd) ?? undefined, }); if (!hardening.ok) { @@ -181,7 +157,6 @@ export function buildSystemRunApprovalPlanV2(params: { return { ok: true, plan: { - version: 2, argv: hardening.argv, cwd: hardening.cwd ?? null, rawCommand: command.cmdText.trim() || null, diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 1ad04cc4b38..d1e7557e6c4 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -54,15 +54,22 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { ask?: "off" | "on-miss" | "always"; approved?: boolean; }) { - const runCommand = vi.fn(async () => ({ - success: true, - stdout: "local-ok", - stderr: "", - timedOut: false, - truncated: false, - exitCode: 0, - error: null, - })); + const runCommand = vi.fn( + async ( + _command: string[], + _cwd?: string, + _env?: Record, + _timeoutMs?: number, + ) => ({ + success: true, + stdout: "local-ok", + stderr: "", + timedOut: false, + truncated: false, + exitCode: 0, + error: null, + }), + ); const runViaMacAppExecHost = vi.fn(async () => params.runViaResponse ?? null); const sendInvokeResult = vi.fn(async () => {}); const sendExecFinishedEvent = vi.fn(async () => {}); @@ -192,7 +199,10 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { return; } - expect(runCommand).toHaveBeenCalledWith(["tr", "a", "b"], undefined, undefined, undefined); + const runArgs = vi.mocked(runCommand).mock.calls[0]?.[0] as string[] | undefined; + expect(runArgs).toBeDefined(); + expect(runArgs?.[0]).toMatch(/(^|[/\\])tr$/); + expect(runArgs?.slice(1)).toEqual(["a", "b"]); expect(sendInvokeResult).toHaveBeenCalledWith( expect.objectContaining({ ok: true, @@ -217,6 +227,132 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { ); }); + it.runIf(process.platform !== "win32")( + "pins PATH-token executable to canonical path for approval-based runs", + async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-path-pin-")); + const binDir = path.join(tmp, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const link = path.join(binDir, "poccmd"); + fs.symlinkSync("/bin/echo", link); + const expected = fs.realpathSync(link); + const oldPath = process.env.PATH; + process.env.PATH = `${binDir}${path.delimiter}${oldPath ?? ""}`; + try { + const { runCommand, sendInvokeResult } = await runSystemInvoke({ + preferMacAppExecHost: false, + command: ["poccmd", "-n", "SAFE"], + approved: true, + security: "full", + ask: "off", + }); + expect(runCommand).toHaveBeenCalledWith( + [expected, "-n", "SAFE"], + undefined, + undefined, + undefined, + ); + expect(sendInvokeResult).toHaveBeenCalledWith( + expect.objectContaining({ + ok: true, + }), + ); + } finally { + if (oldPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = oldPath; + } + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform !== "win32")( + "pins PATH-token executable to canonical path for allowlist runs", + async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-allowlist-path-pin-")); + const binDir = path.join(tmp, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const link = path.join(binDir, "poccmd"); + fs.symlinkSync("/bin/echo", link); + const expected = fs.realpathSync(link); + const oldPath = process.env.PATH; + process.env.PATH = `${binDir}${path.delimiter}${oldPath ?? ""}`; + const runCommand = vi.fn(async () => ({ + success: true, + stdout: "local-ok", + stderr: "", + timedOut: false, + truncated: false, + exitCode: 0, + error: null, + })); + const sendInvokeResult = vi.fn(async () => {}); + const sendNodeEvent = vi.fn(async () => {}); + try { + await withTempApprovalsHome({ + approvals: { + version: 1, + defaults: { + security: "allowlist", + ask: "off", + askFallback: "deny", + }, + agents: { + main: { + allowlist: [{ pattern: link }], + }, + }, + }, + run: async () => { + await handleSystemRunInvoke({ + client: {} as never, + params: { + command: ["poccmd", "-n", "SAFE"], + sessionKey: "agent:main:main", + }, + skillBins: { + current: async () => [], + }, + execHostEnforced: false, + execHostFallbackAllowed: true, + resolveExecSecurity: () => "allowlist", + resolveExecAsk: () => "off", + isCmdExeInvocation: () => false, + sanitizeEnv: () => undefined, + runCommand, + runViaMacAppExecHost: vi.fn(async () => null), + sendNodeEvent, + buildExecEventPayload: (payload) => payload, + sendInvokeResult, + sendExecFinishedEvent: vi.fn(async () => {}), + preferMacAppExecHost: false, + }); + }, + }); + expect(runCommand).toHaveBeenCalledWith( + [expected, "-n", "SAFE"], + undefined, + undefined, + undefined, + ); + expect(sendInvokeResult).toHaveBeenCalledWith( + expect.objectContaining({ + ok: true, + }), + ); + } finally { + if (oldPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = oldPath; + } + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + ); + it.runIf(process.platform !== "win32")( "denies approval-based execution when cwd is a symlink", async () => { diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index ab325321fe2..f8bf21f651e 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -174,7 +174,7 @@ async function sendSystemRunDenied( } export { formatSystemRunAllowlistMissMessage } from "./exec-policy.js"; -export { buildSystemRunApprovalPlanV2 } from "./invoke-system-run-plan.js"; +export { buildSystemRunApprovalPlan } from "./invoke-system-run-plan.js"; async function parseSystemRunPhase( opts: HandleSystemRunInvokeOptions, @@ -300,7 +300,6 @@ async function evaluateSystemRunPolicyPhase( const hardenedPaths = hardenApprovedExecutionPaths({ approvedByAsk: policy.approvedByAsk, argv: parsed.argv, - shellCommand: parsed.shellCommand, cwd: parsed.cwd, }); if (!hardenedPaths.ok) { diff --git a/src/node-host/invoke.ts b/src/node-host/invoke.ts index 7d7b21ad474..11baa45e780 100644 --- a/src/node-host/invoke.ts +++ b/src/node-host/invoke.ts @@ -20,7 +20,7 @@ import { } from "../infra/exec-host.js"; import { sanitizeHostExecEnv } from "../infra/host-env-security.js"; import { runBrowserProxyCommand } from "./invoke-browser.js"; -import { buildSystemRunApprovalPlanV2, handleSystemRunInvoke } from "./invoke-system-run.js"; +import { buildSystemRunApprovalPlan, handleSystemRunInvoke } from "./invoke-system-run.js"; import type { ExecEventPayload, RunResult, @@ -429,7 +429,7 @@ export async function handleInvoke( agentId?: unknown; sessionKey?: unknown; }>(frame.paramsJSON); - const prepared = buildSystemRunApprovalPlanV2(params); + const prepared = buildSystemRunApprovalPlan(params); if (!prepared.ok) { await sendErrorResult(client, frame, "INVALID_REQUEST", prepared.message); return; diff --git a/test/fixtures/system-run-approval-binding-contract.json b/test/fixtures/system-run-approval-binding-contract.json index 2a5a5ad55c2..6d96c388e66 100644 --- a/test/fixtures/system-run-approval-binding-contract.json +++ b/test/fixtures/system-run-approval-binding-contract.json @@ -1,11 +1,11 @@ { "cases": [ { - "name": "v1 matches when env key order changes", + "name": "binding matches when env key order changes", "request": { "host": "node", "command": "git diff", - "bindingV1": { + "binding": { "argv": ["git", "diff"], "cwd": null, "agentId": null, @@ -25,11 +25,11 @@ "expected": { "ok": true } }, { - "name": "v1 rejects env mismatch", + "name": "binding rejects env mismatch", "request": { "host": "node", "command": "git diff", - "bindingV1": { + "binding": { "argv": ["git", "diff"], "cwd": null, "agentId": null, @@ -49,11 +49,11 @@ "expected": { "ok": false, "code": "APPROVAL_ENV_MISMATCH" } }, { - "name": "v1 rejects unbound env overrides", + "name": "binding rejects unbound env overrides", "request": { "host": "node", "command": "git diff", - "bindingV1": { + "binding": { "argv": ["git", "diff"], "cwd": null, "agentId": null, @@ -89,12 +89,12 @@ "expected": { "ok": false, "code": "APPROVAL_REQUEST_MISMATCH" } }, { - "name": "v1 stays authoritative when legacy command text diverges", + "name": "binding stays authoritative when legacy command text diverges", "request": { "host": "node", "command": "echo STALE", "commandArgv": ["echo", "STALE"], - "bindingV1": { + "binding": { "argv": ["echo", "SAFE"], "cwd": null, "agentId": null,