From 3f5e7f815668e8764ce3f2e46eaa51f370045c84 Mon Sep 17 00:00:00 2001 From: Brian Mendonca Date: Mon, 23 Feb 2026 14:43:38 -0700 Subject: [PATCH] fix(gateway): consume allow-once approvals to prevent replay (cherry picked from commit 6adacd447c61b7b743d49e8fabab37fb0b2694c5) --- src/gateway/exec-approval-manager.ts | 15 +++++ .../node-invoke-system-run-approval.test.ts | 61 ++++++++++++++++++- .../node-invoke-system-run-approval.ts | 18 +++++- 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/gateway/exec-approval-manager.ts b/src/gateway/exec-approval-manager.ts index a065be1916a..5e582d42a03 100644 --- a/src/gateway/exec-approval-manager.ts +++ b/src/gateway/exec-approval-manager.ts @@ -154,6 +154,21 @@ export class ExecApprovalManager { return entry?.record ?? null; } + consumeAllowOnce(recordId: string): boolean { + const entry = this.pending.get(recordId); + if (!entry) { + return false; + } + const record = entry.record; + if (record.decision !== "allow-once") { + return false; + } + // One-time approvals must be consumed atomically so the same runId + // cannot be replayed during the resolved-entry grace window. + record.decision = undefined; + return true; + } + /** * Wait for decision on an already-registered approval. * Returns the decision promise if the ID is pending, null otherwise. diff --git a/src/gateway/node-invoke-system-run-approval.test.ts b/src/gateway/node-invoke-system-run-approval.test.ts index ddae856048b..653f0d47852 100644 --- a/src/gateway/node-invoke-system-run-approval.test.ts +++ b/src/gateway/node-invoke-system-run-approval.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "vitest"; -import type { ExecApprovalRecord } from "./exec-approval-manager.js"; +import { ExecApprovalManager, type ExecApprovalRecord } from "./exec-approval-manager.js"; import { sanitizeSystemRunParamsForForwarding } from "./node-invoke-system-run-approval.js"; describe("sanitizeSystemRunParamsForForwarding", () => { @@ -36,8 +36,17 @@ describe("sanitizeSystemRunParamsForForwarding", () => { } function manager(record: ReturnType) { + let consumed = false; return { getSnapshot: () => record, + consumeAllowOnce: () => { + if (consumed || record.decision !== "allow-once") { + return false; + } + consumed = true; + record.decision = undefined; + return true; + }, }; } @@ -130,6 +139,56 @@ describe("sanitizeSystemRunParamsForForwarding", () => { }); expectAllowOnceForwardingResult(result); }); + test("consumes allow-once approvals and blocks same runId replay", async () => { + const approvalManager = new ExecApprovalManager(); + const runId = "approval-replay-1"; + const record = approvalManager.create( + { + host: "node", + command: "echo SAFE", + cwd: null, + agentId: null, + sessionKey: null, + }, + 60_000, + runId, + ); + record.requestedByConnId = "conn-1"; + record.requestedByDeviceId = "dev-1"; + record.requestedByClientId = "cli-1"; + + const decisionPromise = approvalManager.register(record, 60_000); + approvalManager.resolve(runId, "allow-once", "operator"); + await expect(decisionPromise).resolves.toBe("allow-once"); + + const params = { + command: ["echo", "SAFE"], + rawCommand: "echo SAFE", + runId, + approved: true, + approvalDecision: "allow-once", + }; + + const first = sanitizeSystemRunParamsForForwarding({ + rawParams: params, + client, + execApprovalManager: approvalManager, + nowMs: now, + }); + expectAllowOnceForwardingResult(first); + + const second = sanitizeSystemRunParamsForForwarding({ + rawParams: params, + client, + execApprovalManager: approvalManager, + nowMs: now, + }); + expect(second.ok).toBe(false); + if (second.ok) { + throw new Error("unreachable"); + } + expect(second.details?.code).toBe("APPROVAL_REQUIRED"); + }); test("rejects approval ids that do not bind a nodeId", () => { const record = makeRecord("echo SAFE"); diff --git a/src/gateway/node-invoke-system-run-approval.ts b/src/gateway/node-invoke-system-run-approval.ts index 5bf31db8fb5..d5600adf032 100644 --- a/src/gateway/node-invoke-system-run-approval.ts +++ b/src/gateway/node-invoke-system-run-approval.ts @@ -17,6 +17,7 @@ type SystemRunParamsLike = { type ApprovalLookup = { getSnapshot: (recordId: string) => ExecApprovalRecord | null; + consumeAllowOnce?: (recordId: string) => boolean; }; type ApprovalClient = { @@ -245,9 +246,22 @@ export function sanitizeSystemRunParamsForForwarding(opts: { } // Normal path: enforce the decision recorded by the gateway. - if (snapshot.decision === "allow-once" || snapshot.decision === "allow-always") { + if (snapshot.decision === "allow-once") { + if (typeof manager.consumeAllowOnce !== "function" || !manager.consumeAllowOnce(runId)) { + return { + ok: false, + message: "approval required", + details: { code: "APPROVAL_REQUIRED", runId }, + }; + } next.approved = true; - next.approvalDecision = snapshot.decision; + next.approvalDecision = "allow-once"; + return { ok: true, params: next }; + } + + if (snapshot.decision === "allow-always") { + next.approved = true; + next.approvalDecision = "allow-always"; return { ok: true, params: next }; }