diff --git a/src/gateway/exec-approval-manager.expiry.test.ts b/src/gateway/exec-approval-manager.expiry.test.ts index f53dd8559545..94f10c0b0c33 100644 --- a/src/gateway/exec-approval-manager.expiry.test.ts +++ b/src/gateway/exec-approval-manager.expiry.test.ts @@ -81,4 +81,28 @@ describe("ExecApprovalManager timeout expiry publication", () => { { recordId: record.id, status: "expired", requestCommand: "echo expired" }, ]); }); + + it("rejects ask-fallback replay of a run-aborted cancellation", async () => { + installTimerMocks(); + const manager = new ExecApprovalManager(); + const record = manager.create({ command: "echo ok" }, 60_000, "approval-cancelled"); + const decisionPromise = manager.register(record, 60_000); + + // Dispatch fencing / run abort ends decision-less like a timeout, but its + // authority closed deliberately — replay must not re-admit through it. + const denied = manager.forceDenyDetailed( + "approval-cancelled", + "run-aborted", + { kind: "system", id: "worker-dispatch" }, + "cancelled", + ); + expect(denied.outcome).toBe("denied"); + await expect(decisionPromise).resolves.toBeNull(); + + expect(manager.getSnapshot("approval-cancelled")).toMatchObject({ + status: "cancelled", + terminalReason: "run-aborted", + }); + expect(manager.consumeAskFallback("approval-cancelled")).toBe(false); + }); }); diff --git a/src/gateway/exec-approval-manager.ts b/src/gateway/exec-approval-manager.ts index 79f757dd48c8..4bf6286b21fa 100644 --- a/src/gateway/exec-approval-manager.ts +++ b/src/gateway/exec-approval-manager.ts @@ -160,7 +160,6 @@ type ExecApprovalDurableLookup = type PendingEntry = { record: ExecApprovalRecord; resolve: (decision: ExecApprovalDecision | null) => void; - reject: (err: Error) => void; timer: ReturnType | null; cleanupTimer: ReturnType | null; handoffRetainCount: number; @@ -357,16 +356,13 @@ export class ExecApprovalManager { } let resolvePromise: (decision: ExecApprovalDecision | null) => void; - let rejectPromise: (err: Error) => void; - const promise = new Promise((resolve, reject) => { + const promise = new Promise((resolve) => { resolvePromise = resolve; - rejectPromise = reject; }); // Create entry first so we can capture it in the closure (not re-fetch from map) const entry: PendingEntry = { record, resolve: resolvePromise!, - reject: rejectPromise!, timer: null, cleanupTimer: null, handoffRetainCount: 0, @@ -1020,7 +1016,11 @@ export class ExecApprovalManager { record.resolvedAtMs === undefined || record.decision !== undefined || record.consumedDecision !== undefined || - record.askFallbackConsumed === true + record.askFallbackConsumed === true || + // Only unanswered approvals (timeout or no delivery route) are + // re-admissible. Cancelled/fenced records also end decision-less, but + // their authority closed deliberately — never replay through them. + (record.status !== "expired" && record.terminalReason !== "no-route") ) { return false; } @@ -1256,9 +1256,5 @@ export class ExecApprovalManager { } return { kind: "none" }; } - - lookupPendingId(input: string): ExecApprovalIdLookupResult { - return this.lookupApprovalId(input); - } } /* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ diff --git a/src/gateway/server-core-runtime.ts b/src/gateway/server-core-runtime.ts index 979d1dc1d9bc..e15949baae99 100644 --- a/src/gateway/server-core-runtime.ts +++ b/src/gateway/server-core-runtime.ts @@ -372,14 +372,16 @@ export async function startGatewayCoreRuntime(input: { for (const sessionKey of keys) { revokeAttachGrantsForSession(sessionKey); } - for (const record of execApprovalManager.listPendingRecords()) { - if (approvalRequestTargetsSession(record.request, keys, sessionId)) { - execApprovalManager.expire(record.id, "worker-dispatch"); - } - } - for (const record of pluginApprovalManager.listPendingRecords()) { - if (approvalRequestTargetsSession(record.request, keys, sessionId)) { - pluginApprovalManager.expire(record.id, "worker-dispatch"); + // Dispatch fencing closes approval authority deliberately: record it as a + // run-aborted cancellation, not a timeout, so ask-fallback replay cannot + // re-admit through the fenced record (consumeAskFallback admits only + // expired/no-route terminals). + const fenceResolver = { kind: "system", id: "worker-dispatch" } as const; + for (const manager of [execApprovalManager, pluginApprovalManager]) { + for (const record of manager.listPendingRecords()) { + if (approvalRequestTargetsSession(record.request, keys, sessionId)) { + manager.forceDenyDetailed(record.id, "run-aborted", fenceResolver, "cancelled"); + } } } }; diff --git a/src/gateway/server-methods/server-methods.test.ts b/src/gateway/server-methods/server-methods.test.ts index 7934e341cd6e..7f8b5f331273 100644 --- a/src/gateway/server-methods/server-methods.test.ts +++ b/src/gateway/server-methods/server-methods.test.ts @@ -3567,8 +3567,8 @@ describe("exec approval handlers", () => { const pendingRecord = manager.create({ command: "echo new", host: "gateway" }, 2_000, "abcdef"); void manager.register(pendingRecord, 2_000); - expect(manager.lookupPendingId("abc")).toEqual({ kind: "none" }); - expect(manager.lookupPendingId("abcdef")).toEqual({ kind: "exact", id: "abcdef" }); + expect(manager.lookupApprovalId("abc")).toEqual({ kind: "none" }); + expect(manager.lookupApprovalId("abcdef")).toEqual({ kind: "exact", id: "abcdef" }); }); it("stores versioned system.run binding and sorted env keys on approval request", async () => {