diff --git a/src/agents/bash-tools.exec-approval-request.ts b/src/agents/bash-tools.exec-approval-request.ts index 50815704cee0..47fd477bc241 100644 --- a/src/agents/bash-tools.exec-approval-request.ts +++ b/src/agents/bash-tools.exec-approval-request.ts @@ -228,6 +228,7 @@ type HostExecApprovalParams = { turnSourceAccountId?: string; turnSourceThreadId?: string | number; approvalReviewerDeviceIds?: string[]; + trigger?: string; requireDeliveryRoute?: boolean; suppressDelivery?: boolean; }; @@ -338,7 +339,10 @@ async function buildHostApprovalDecisionParams( runId: params.runId, toolCallId: params.toolCallId, requireDeliveryRoute: params.requireDeliveryRoute, - suppressDelivery: params.suppressDelivery, + // Cron has no live reviewer. Register for audit/fallback resolution without + // exposing an operator-visible request that the scheduled run cannot answer. + suppressDelivery: + params.suppressDelivery === true || params.trigger === "cron" ? true : undefined, approvalReviewerDeviceIds: params.approvalReviewerDeviceIds, ...buildExecApprovalTurnSourceContext(params), }; diff --git a/src/agents/bash-tools.exec-host-gateway.ts b/src/agents/bash-tools.exec-host-gateway.ts index 81473f2b8d13..55a3f9542d3c 100644 --- a/src/agents/bash-tools.exec-host-gateway.ts +++ b/src/agents/bash-tools.exec-host-gateway.ts @@ -984,6 +984,7 @@ export async function processGatewayAllowlist( sessionId: params.sessionId, runId: params.runId, toolCallId: params.toolCallId, + trigger: params.trigger, approvalReviewerDeviceIds: params.approvalReviewerDeviceId ? [params.approvalReviewerDeviceId] : undefined, diff --git a/src/agents/bash-tools.exec-host-node.ts b/src/agents/bash-tools.exec-host-node.ts index aab4b3adb0c5..3d1cf95ce9c6 100644 --- a/src/agents/bash-tools.exec-host-node.ts +++ b/src/agents/bash-tools.exec-host-node.ts @@ -218,6 +218,7 @@ export async function executeNodeHostCommand( workdir: prepared.cwd, host: "node", nodeId: target.nodeId, + trigger: params.trigger, toolCallId: params.toolCallId, security: hostSecurity, ask: hostAsk, diff --git a/src/agents/bash-tools.exec.approval-id.test.ts b/src/agents/bash-tools.exec.approval-id.test.ts index ff0486f01a1a..608e53368ac4 100644 --- a/src/agents/bash-tools.exec.approval-id.test.ts +++ b/src/agents/bash-tools.exec.approval-id.test.ts @@ -386,6 +386,16 @@ function mockNoApprovalRouteRegistration() { const requireRecord = createRequireRecord("record", "expected-label"); +function requireExecApprovalRequestCall() { + const requestCall = vi + .mocked(callGatewayTool) + .mock.calls.find(([method]) => method === "exec.approval.request"); + return { + params: requireRecord(requestCall?.[2], "approval request params"), + options: requireRecord(requestCall?.[3], "approval request options"), + }; +} + function expectRecordFields( record: Record | undefined, expected: Record, @@ -1562,6 +1572,7 @@ describe("exec approvals", () => { resolveRegistration?.({ status: "accepted", id: "approval-id" }); const result = await executePromise; expect(result.details.status).toBe("approval-pending"); + expect(requireExecApprovalRequestCall().params.suppressDelivery).toBeUndefined(); expect(calls[0]).toBe("exec.approval.request"); expect(calls).toContain("exec.approval.waitDecision"); }); @@ -1609,12 +1620,9 @@ describe("exec approvals", () => { expect(result.details.status).toBe("completed"); expect(getResultText(result)).toContain("cron-ok"); - const approvalRequestCall = vi - .mocked(callGatewayTool) - .mock.calls.find(([method]) => method === "exec.approval.request"); - expect(requireRecord(approvalRequestCall?.[3], "approval request options").expectFinal).toBe( - false, - ); + const approvalRequest = requireExecApprovalRequestCall(); + expect(approvalRequest.options.expectFinal).toBe(false); + expect(approvalRequest.params.suppressDelivery).toBe(true); expect( vi .mocked(callGatewayTool) @@ -1682,6 +1690,7 @@ describe("exec approvals", () => { expect(result.details.status).toBe("completed"); expect(getResultText(result)).toContain("cron-node-ok"); + expect(requireExecApprovalRequestCall().params.suppressDelivery).toBe(true); const systemRun = requireRecord(systemRunInvoke, "system.run invoke"); expect(systemRun.command).toBe("system.run"); const params = requireRecord(systemRun.params, "system.run params"); @@ -1713,6 +1722,54 @@ describe("exec approvals", () => { command: "echo cron-denied", }), ).rejects.toThrow("Automation runs cannot wait for interactive exec approval"); + expect(requireExecApprovalRequestCall().params.suppressDelivery).toBe(true); + }); + + it("denies node cron no-route approvals when askFallback is deny", async () => { + await writeExecApprovalsConfig({ + version: 1, + defaults: { security: "full", ask: "always", askFallback: "deny" }, + agents: {}, + }); + vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { + if (method === "exec.approval.request") { + return { id: "approval-id", decision: null }; + } + if (method === "exec.approval.waitDecision") { + return { decision: null }; + } + if (method === "node.invoke") { + const invoke = requireRecord(params, "node invoke"); + if (invoke.command === "system.run.prepare") { + return buildPreparedSystemRunPayload(params); + } + } + return { ok: true }; + }); + + const tool = createExecTool({ + host: "node", + ask: "always", + security: "full", + trigger: "cron", + approvalRunningNoticeMs: 0, + }); + + await expect( + tool.execute("call-cron-node-denied", { + command: "echo cron-node-denied", + }), + ).rejects.toThrow("Automation runs cannot wait for interactive exec approval"); + expect(requireExecApprovalRequestCall().params.suppressDelivery).toBe(true); + expect( + vi + .mocked(callGatewayTool) + .mock.calls.some( + ([method, _opts, params]) => + method === "node.invoke" && + requireRecord(params, "node invoke").command === "system.run", + ), + ).toBe(false); }); }); /* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ diff --git a/src/gateway/server-methods/approval-shared.test.ts b/src/gateway/server-methods/approval-shared.test.ts index 32dc5a26a73d..7e25750d15b2 100644 --- a/src/gateway/server-methods/approval-shared.test.ts +++ b/src/gateway/server-methods/approval-shared.test.ts @@ -861,6 +861,57 @@ describe("handlePendingApprovalRequest", () => { ); }); + it("expires suppressed requests instead of retaining a hidden turn-source route", async () => { + const manager = new ExecApprovalManager(); + const record = manager.create( + { + command: "echo cron", + turnSourceChannel: "discord", + turnSourceAccountId: "default", + }, + 60_000, + "approval-suppressed-turn-source", + ); + const decisionPromise = manager.register(record, 60_000); + const respond = vi.fn(); + const broadcast = vi.fn(); + const deliverRequest = vi.fn(() => true); + + await handlePendingApprovalRequest({ + manager, + record, + decisionPromise, + respond, + context: { + broadcast, + hasExecApprovalClients: () => true, + } as unknown as GatewayRequestContext, + requestEventName: "exec.approval.requested", + requestEvent: { + id: record.id, + request: record.request, + createdAtMs: record.createdAtMs, + expiresAtMs: record.expiresAtMs, + }, + twoPhase: true, + suppressDelivery: true, + deliverRequest, + }); + + expect(broadcast).not.toHaveBeenCalled(); + expect(deliverRequest).not.toHaveBeenCalled(); + expect(hasApprovalTurnSourceRouteMock).not.toHaveBeenCalled(); + expect(manager.getSnapshot(record.id)).toMatchObject({ + resolvedBy: "no-approval-route", + terminalReason: "no-route", + }); + expect(respond).toHaveBeenCalledWith( + true, + expect.objectContaining({ id: record.id, decision: null }), + undefined, + ); + }); + it("does not target no-device browser UI approvals to unrelated approval-scoped clients", async () => { hasApprovalTurnSourceRouteMock.mockReturnValueOnce(false); const manager = new ExecApprovalManager(); diff --git a/src/gateway/server-methods/approval-shared.ts b/src/gateway/server-methods/approval-shared.ts index 7fc08da65338..fd15c724d5ec 100644 --- a/src/gateway/server-methods/approval-shared.ts +++ b/src/gateway/server-methods/approval-shared.ts @@ -347,6 +347,7 @@ export async function handlePendingApprovalRequest< // A turn-source route can approve without an active approval client, so keep // the record alive when the originating channel/account can still receive it. const hasTurnSourceRoute = + !suppressDelivery && !hasApprovalClients && !delivered && hasApprovalTurnSourceRoute({