fix(gateway): consume allow-once approvals to prevent replay

(cherry picked from commit 6adacd447c)
This commit is contained in:
Brian Mendonca
2026-02-23 14:43:38 -07:00
committed by Peter Steinberger
parent ffc22778f3
commit 3f5e7f8156
3 changed files with 91 additions and 3 deletions

View File

@@ -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.

View File

@@ -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<typeof makeRecord>) {
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");

View File

@@ -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 };
}