From b573231cd127f9095eaccbbb7bd69bb044d2050f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 22 Jan 2026 06:42:36 +0000 Subject: [PATCH] fix: prevent exec approval resolve race --- .../server-methods/exec-approval.test.ts | 51 +++++++++++++++++++ src/gateway/server-methods/exec-approval.ts | 3 +- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/gateway/server-methods/exec-approval.test.ts b/src/gateway/server-methods/exec-approval.test.ts index 8142b281f..0b1da93f3 100644 --- a/src/gateway/server-methods/exec-approval.test.ts +++ b/src/gateway/server-methods/exec-approval.test.ts @@ -105,6 +105,57 @@ describe("exec approval handlers", () => { expect(broadcasts.some((entry) => entry.event === "exec.approval.resolved")).toBe(true); }); + it("accepts resolve during broadcast", async () => { + const manager = new ExecApprovalManager(); + const handlers = createExecApprovalHandlers(manager); + const respond = vi.fn(); + const resolveRespond = vi.fn(); + + const resolveContext = { + broadcast: () => {}, + }; + + const context = { + broadcast: (event: string, payload: unknown) => { + if (event !== "exec.approval.requested") return; + const id = (payload as { id?: string })?.id ?? ""; + void handlers["exec.approval.resolve"]({ + params: { id, decision: "allow-once" }, + respond: resolveRespond, + context: resolveContext as unknown as Parameters< + (typeof handlers)["exec.approval.resolve"] + >[0]["context"], + client: { connect: { client: { id: "cli", displayName: "CLI" } } }, + req: { id: "req-2", type: "req", method: "exec.approval.resolve" }, + isWebchatConnect: noop, + }); + }, + }; + + await handlers["exec.approval.request"]({ + params: { + command: "echo ok", + cwd: "/tmp", + host: "node", + timeoutMs: 2000, + }, + respond, + context: context as unknown as Parameters< + (typeof handlers)["exec.approval.request"] + >[0]["context"], + client: null, + req: { id: "req-1", type: "req", method: "exec.approval.request" }, + isWebchatConnect: noop, + }); + + expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined); + expect(respond).toHaveBeenCalledWith( + true, + expect.objectContaining({ decision: "allow-once" }), + undefined, + ); + }); + it("accepts explicit approval ids", async () => { const manager = new ExecApprovalManager(); const handlers = createExecApprovalHandlers(manager); diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 8fb13cea0..48663b96c 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -58,6 +58,7 @@ export function createExecApprovalHandlers(manager: ExecApprovalManager): Gatewa sessionKey: p.sessionKey ?? null, }; const record = manager.create(request, timeoutMs, explicitId); + const decisionPromise = manager.waitForDecision(record, timeoutMs); context.broadcast( "exec.approval.requested", { @@ -68,7 +69,7 @@ export function createExecApprovalHandlers(manager: ExecApprovalManager): Gatewa }, { dropIfSlow: true }, ); - const decision = await manager.waitForDecision(record, timeoutMs); + const decision = await decisionPromise; respond( true, {