From 4e690e09c746408b5e27617a20cb3fdc5190dbda Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 26 Feb 2026 22:01:16 +0100 Subject: [PATCH] refactor(gateway): centralize system.run approval context and errors --- .../node-invoke-system-run-approval-errors.ts | 29 ++++ .../node-invoke-system-run-approval.ts | 156 ++++++++---------- src/gateway/server-methods/exec-approval.ts | 38 ++--- src/infra/system-run-approval-context.ts | 123 ++++++++++++++ 4 files changed, 239 insertions(+), 107 deletions(-) create mode 100644 src/gateway/node-invoke-system-run-approval-errors.ts create mode 100644 src/infra/system-run-approval-context.ts diff --git a/src/gateway/node-invoke-system-run-approval-errors.ts b/src/gateway/node-invoke-system-run-approval-errors.ts new file mode 100644 index 00000000000..9c50a5004b1 --- /dev/null +++ b/src/gateway/node-invoke-system-run-approval-errors.ts @@ -0,0 +1,29 @@ +export type SystemRunApprovalGuardError = { + ok: false; + message: string; + details: Record; +}; + +export function systemRunApprovalGuardError(params: { + code: string; + message: string; + details?: Record; +}): SystemRunApprovalGuardError { + const details = params.details ? { ...params.details } : {}; + return { + ok: false, + message: params.message, + details: { + code: params.code, + ...details, + }, + }; +} + +export function systemRunApprovalRequired(runId: string): SystemRunApprovalGuardError { + return systemRunApprovalGuardError({ + code: "APPROVAL_REQUIRED", + message: "approval required", + details: { runId }, + }); +} diff --git a/src/gateway/node-invoke-system-run-approval.ts b/src/gateway/node-invoke-system-run-approval.ts index a9d572c9763..efee11572b1 100644 --- a/src/gateway/node-invoke-system-run-approval.ts +++ b/src/gateway/node-invoke-system-run-approval.ts @@ -1,6 +1,10 @@ -import { normalizeSystemRunApprovalPlanV2 } from "../infra/system-run-approval-binding.js"; +import { resolveSystemRunApprovalRuntimeContext } from "../infra/system-run-approval-context.js"; import { resolveSystemRunCommand } from "../infra/system-run-command.js"; import type { ExecApprovalRecord } from "./exec-approval-manager.js"; +import { + systemRunApprovalGuardError, + systemRunApprovalRequired, +} from "./node-invoke-system-run-approval-errors.js"; import { evaluateSystemRunApprovalMatch, toSystemRunApprovalMismatchError, @@ -125,62 +129,60 @@ export function sanitizeSystemRunParamsForForwarding(opts: { const runId = normalizeString(p.runId); if (!runId) { - return { - ok: false, + return systemRunApprovalGuardError({ + code: "MISSING_RUN_ID", message: "approval override requires params.runId", - details: { code: "MISSING_RUN_ID" }, - }; + }); } const manager = opts.execApprovalManager; if (!manager) { - return { - ok: false, + return systemRunApprovalGuardError({ + code: "APPROVALS_UNAVAILABLE", message: "exec approvals unavailable", - details: { code: "APPROVALS_UNAVAILABLE" }, - }; + }); } const snapshot = manager.getSnapshot(runId); if (!snapshot) { - return { - ok: false, + return systemRunApprovalGuardError({ + code: "UNKNOWN_APPROVAL_ID", message: "unknown or expired approval id", - details: { code: "UNKNOWN_APPROVAL_ID", runId }, - }; + details: { runId }, + }); } const nowMs = typeof opts.nowMs === "number" ? opts.nowMs : Date.now(); if (nowMs > snapshot.expiresAtMs) { - return { - ok: false, + return systemRunApprovalGuardError({ + code: "APPROVAL_EXPIRED", message: "approval expired", - details: { code: "APPROVAL_EXPIRED", runId }, - }; + details: { runId }, + }); } const targetNodeId = normalizeString(opts.nodeId); if (!targetNodeId) { - return { - ok: false, + return systemRunApprovalGuardError({ + code: "MISSING_NODE_ID", message: "node.invoke requires nodeId", - details: { code: "MISSING_NODE_ID", runId }, - }; + details: { runId }, + }); } const approvalNodeId = normalizeString(snapshot.request.nodeId); if (!approvalNodeId) { - return { - ok: false, + return systemRunApprovalGuardError({ + code: "APPROVAL_NODE_BINDING_MISSING", message: "approval id missing node binding", - details: { code: "APPROVAL_NODE_BINDING_MISSING", runId }, - }; + details: { runId }, + }); } if (approvalNodeId !== targetNodeId) { - return { - ok: false, + return systemRunApprovalGuardError({ + code: "APPROVAL_NODE_MISMATCH", message: "approval id not valid for this node", - details: { code: "APPROVAL_NODE_MISMATCH", runId }, - }; + details: { runId }, + }); } // Prefer binding by device identity (stable across reconnects / per-call clients like callGateway()). @@ -189,79 +191,69 @@ export function sanitizeSystemRunParamsForForwarding(opts: { const clientDeviceId = opts.client?.connect?.device?.id ?? null; if (snapshotDeviceId) { if (snapshotDeviceId !== clientDeviceId) { - return { - ok: false, + return systemRunApprovalGuardError({ + code: "APPROVAL_DEVICE_MISMATCH", message: "approval id not valid for this device", - details: { code: "APPROVAL_DEVICE_MISMATCH", runId }, - }; + details: { runId }, + }); } } else if ( snapshot.requestedByConnId && snapshot.requestedByConnId !== (opts.client?.connId ?? null) ) { - return { - ok: false, + return systemRunApprovalGuardError({ + code: "APPROVAL_CLIENT_MISMATCH", message: "approval id not valid for this client", - details: { code: "APPROVAL_CLIENT_MISMATCH", runId }, - }; + details: { runId }, + }); } - const planV2 = normalizeSystemRunApprovalPlanV2(snapshot.request.systemRunPlanV2 ?? null); - let approvalArgv: string[]; - let approvalCwd: string | null; - let approvalAgentId: string | null; - let approvalSessionKey: string | null; - if (planV2) { - approvalArgv = [...planV2.argv]; - approvalCwd = planV2.cwd; - approvalAgentId = planV2.agentId; - approvalSessionKey = planV2.sessionKey; - next.command = [...planV2.argv]; - if (planV2.rawCommand) { - next.rawCommand = planV2.rawCommand; + const runtimeContext = resolveSystemRunApprovalRuntimeContext({ + planV2: snapshot.request.systemRunPlanV2 ?? null, + command: p.command, + rawCommand: p.rawCommand, + cwd: p.cwd, + agentId: p.agentId, + sessionKey: p.sessionKey, + }); + if (!runtimeContext.ok) { + return { + ok: false, + message: runtimeContext.message, + details: runtimeContext.details, + }; + } + if (runtimeContext.planV2) { + next.command = [...runtimeContext.planV2.argv]; + if (runtimeContext.rawCommand) { + next.rawCommand = runtimeContext.rawCommand; } else { delete next.rawCommand; } - if (planV2.cwd) { - next.cwd = planV2.cwd; + if (runtimeContext.cwd) { + next.cwd = runtimeContext.cwd; } else { delete next.cwd; } - if (planV2.agentId) { - next.agentId = planV2.agentId; + if (runtimeContext.agentId) { + next.agentId = runtimeContext.agentId; } else { delete next.agentId; } - if (planV2.sessionKey) { - next.sessionKey = planV2.sessionKey; + if (runtimeContext.sessionKey) { + next.sessionKey = runtimeContext.sessionKey; } else { delete next.sessionKey; } - } else { - const cmdTextResolution = resolveSystemRunCommand({ - command: p.command, - rawCommand: p.rawCommand, - }); - if (!cmdTextResolution.ok) { - return { - ok: false, - message: cmdTextResolution.message, - details: cmdTextResolution.details, - }; - } - approvalArgv = cmdTextResolution.argv; - approvalCwd = normalizeString(p.cwd) ?? null; - approvalAgentId = normalizeString(p.agentId) ?? null; - approvalSessionKey = normalizeString(p.sessionKey) ?? null; } const approvalMatch = evaluateSystemRunApprovalMatch({ - argv: approvalArgv, + argv: runtimeContext.argv, request: snapshot.request, binding: { - cwd: approvalCwd, - agentId: approvalAgentId, - sessionKey: approvalSessionKey, + cwd: runtimeContext.cwd, + agentId: runtimeContext.agentId, + sessionKey: runtimeContext.sessionKey, env: p.env, }, }); @@ -272,11 +264,7 @@ export function sanitizeSystemRunParamsForForwarding(opts: { // Normal path: enforce the decision recorded by the gateway. if (snapshot.decision === "allow-once") { if (typeof manager.consumeAllowOnce !== "function" || !manager.consumeAllowOnce(runId)) { - return { - ok: false, - message: "approval required", - details: { code: "APPROVAL_REQUIRED", runId }, - }; + return systemRunApprovalRequired(runId); } next.approved = true; next.approvalDecision = "allow-once"; @@ -306,9 +294,5 @@ export function sanitizeSystemRunParamsForForwarding(opts: { return { ok: true, params: next }; } - return { - ok: false, - message: "approval required", - details: { code: "APPROVAL_REQUIRED", runId }, - }; + return systemRunApprovalRequired(runId); } diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 707686539c8..2d362efa214 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -3,11 +3,8 @@ import { DEFAULT_EXEC_APPROVAL_TIMEOUT_MS, type ExecApprovalDecision, } from "../../infra/exec-approvals.js"; -import { - buildSystemRunApprovalBindingV1, - normalizeSystemRunApprovalPlanV2, -} from "../../infra/system-run-approval-binding.js"; -import { formatExecCommand } from "../../infra/system-run-command.js"; +import { buildSystemRunApprovalBindingV1 } 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 { ErrorCodes, @@ -72,21 +69,20 @@ export function createExecApprovalHandlers( const explicitId = typeof p.id === "string" && p.id.trim().length > 0 ? p.id.trim() : null; const host = typeof p.host === "string" ? p.host.trim() : ""; const nodeId = typeof p.nodeId === "string" ? p.nodeId.trim() : ""; - const commandArgv = Array.isArray(p.commandArgv) - ? p.commandArgv.map((entry) => String(entry)) - : undefined; - const systemRunPlanV2 = - host === "node" ? normalizeSystemRunApprovalPlanV2(p.systemRunPlanV2) : null; - const effectiveCommandArgv = systemRunPlanV2?.argv ?? commandArgv; - const effectiveCwd = systemRunPlanV2?.cwd ?? p.cwd; - const effectiveAgentId = systemRunPlanV2?.agentId ?? p.agentId; - const effectiveSessionKey = systemRunPlanV2?.sessionKey ?? p.sessionKey; - const effectiveCommandText = (() => { - if (!systemRunPlanV2) { - return p.command; - } - return systemRunPlanV2.rawCommand ?? formatExecCommand(systemRunPlanV2.argv); - })(); + const approvalContext = resolveSystemRunApprovalRequestContext({ + host, + command: p.command, + commandArgv: p.commandArgv, + systemRunPlanV2: p.systemRunPlanV2, + cwd: p.cwd, + agentId: p.agentId, + sessionKey: p.sessionKey, + }); + const effectiveCommandArgv = approvalContext.commandArgv; + const effectiveCwd = approvalContext.cwd; + const effectiveAgentId = approvalContext.agentId; + const effectiveSessionKey = approvalContext.sessionKey; + const effectiveCommandText = approvalContext.commandText; if (host === "node" && !nodeId) { respond( false, @@ -129,7 +125,7 @@ export function createExecApprovalHandlers( commandArgv: effectiveCommandArgv, envKeys: systemRunBindingV1?.envKeys?.length ? systemRunBindingV1.envKeys : undefined, systemRunBindingV1: systemRunBindingV1?.binding ?? null, - systemRunPlanV2: systemRunPlanV2, + systemRunPlanV2: approvalContext.planV2, cwd: effectiveCwd ?? null, nodeId: host === "node" ? nodeId : null, host: host || null, diff --git a/src/infra/system-run-approval-context.ts b/src/infra/system-run-approval-context.ts new file mode 100644 index 00000000000..25cbee1fcfc --- /dev/null +++ b/src/infra/system-run-approval-context.ts @@ -0,0 +1,123 @@ +import type { SystemRunApprovalPlanV2 } from "./exec-approvals.js"; +import { normalizeSystemRunApprovalPlanV2 } from "./system-run-approval-binding.js"; +import { formatExecCommand, resolveSystemRunCommand } from "./system-run-command.js"; + +type PreparedRunPayload = { + cmdText: string; + plan: SystemRunApprovalPlanV2; +}; + +type SystemRunApprovalRequestContext = { + planV2: SystemRunApprovalPlanV2 | null; + commandArgv: string[] | undefined; + commandText: string; + cwd: string | null; + agentId: string | null; + sessionKey: string | null; +}; + +type SystemRunApprovalRuntimeContext = + | { + ok: true; + planV2: SystemRunApprovalPlanV2 | null; + argv: string[]; + cwd: string | null; + agentId: string | null; + sessionKey: string | null; + rawCommand: string | null; + } + | { + ok: false; + message: string; + details?: Record; + }; + +function normalizeString(value: unknown): string | null { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + return trimmed ? trimmed : null; +} + +function normalizeStringArray(value: unknown): string[] { + return Array.isArray(value) ? value.map((entry) => String(entry)) : []; +} + +function normalizeCommandText(value: unknown): string { + return typeof value === "string" ? value : ""; +} + +export function parsePreparedSystemRunPayload(payload: unknown): PreparedRunPayload | null { + if (!payload || typeof payload !== "object" || Array.isArray(payload)) { + return null; + } + const raw = payload as { cmdText?: unknown; plan?: unknown }; + const cmdText = normalizeString(raw.cmdText); + const plan = normalizeSystemRunApprovalPlanV2(raw.plan); + if (!cmdText || !plan) { + return null; + } + return { cmdText, plan }; +} + +export function resolveSystemRunApprovalRequestContext(params: { + host?: unknown; + command?: unknown; + commandArgv?: unknown; + systemRunPlanV2?: unknown; + cwd?: unknown; + agentId?: unknown; + sessionKey?: unknown; +}): SystemRunApprovalRequestContext { + const host = normalizeString(params.host) ?? ""; + const planV2 = host === "node" ? normalizeSystemRunApprovalPlanV2(params.systemRunPlanV2) : 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), + }; +} + +export function resolveSystemRunApprovalRuntimeContext(params: { + planV2?: unknown; + command?: unknown; + rawCommand?: unknown; + cwd?: unknown; + agentId?: unknown; + sessionKey?: unknown; +}): SystemRunApprovalRuntimeContext { + const normalizedPlan = normalizeSystemRunApprovalPlanV2(params.planV2 ?? null); + if (normalizedPlan) { + return { + ok: true, + planV2: normalizedPlan, + argv: [...normalizedPlan.argv], + cwd: normalizedPlan.cwd, + agentId: normalizedPlan.agentId, + sessionKey: normalizedPlan.sessionKey, + rawCommand: normalizedPlan.rawCommand, + }; + } + const command = resolveSystemRunCommand({ + command: params.command, + rawCommand: params.rawCommand, + }); + if (!command.ok) { + return { ok: false, message: command.message, details: command.details }; + } + return { + ok: true, + planV2: null, + argv: command.argv, + cwd: normalizeString(params.cwd), + agentId: normalizeString(params.agentId), + sessionKey: normalizeString(params.sessionKey), + rawCommand: normalizeString(params.rawCommand), + }; +}