diff --git a/apps/cloud/src/engine/execution-gate.ts b/apps/cloud/src/engine/execution-gate.ts index 60c102663..5cea9a345 100644 --- a/apps/cloud/src/engine/execution-gate.ts +++ b/apps/cloud/src/engine/execution-gate.ts @@ -5,9 +5,14 @@ // Usage is tracked to Autumn after every execution (execution-usage.ts), but // nothing ever CHECKED the balance before running, so a free-tier org could run // unbounded executions past its quota. This gate consults -// `AutumnService.checkExecutionBalance` before `execute` / `executeWithPause`. -// `resume` is never gated: a paused execution already consumed its quota slot -// when it started, and blocking resume would strand approved work forever. +// `AutumnService.checkExecutionBalance` before `execute` / `executeWithPause`, +// and before `resume` too: a paused execution consumed its quota slot when it +// started, but one execution can pause many times, and each continuation is +// fresh work — ungating resume let a single billable execution carry unbounded +// resumed computation past both this gate and the rate limiter. A blocked +// resume answers with the same descriptive error result as a blocked execute +// and leaves the paused execution intact, so approved work can complete once +// the org is back under its limit. // // FAIL OPEN is a hard requirement: any Autumn error, timeout, or missing // customer/feature allows the execution (logged + Sentry, mirroring @@ -68,12 +73,13 @@ export type GateDecision = | { readonly blocked: true; readonly error: { readonly message: string } }; /** - * Wrap an engine so `decide` runs before `execute` / `executeWithPause`. A - * blocked decision short-circuits to a descriptive `ExecuteResult.error` - * (which `formatExecuteResult` renders as a clean `isError` MCP tool result) - * WITHOUT invoking the inner engine — so a blocked execution is neither run - * nor usage-tracked. `resume` and all read-only members pass through - * untouched: paused executions must always be able to complete. + * Wrap an engine so `decide` runs before `execute` / `executeWithPause` / + * `resume`. A blocked decision short-circuits to a descriptive + * `ExecuteResult.error` (which `formatExecuteResult` renders as a clean + * `isError` MCP tool result) WITHOUT invoking the inner engine — so a blocked + * execution is neither run nor usage-tracked, and a blocked resume leaves the + * paused execution untouched and retryable. The read-only members pass through + * untouched. */ export const withPreExecutionGate = ( engine: ExecutionEngine, @@ -98,8 +104,17 @@ export const withPreExecutionGate = ( }) : engine.executeWithPause(code, options), ), - // resume is never gated: paused executions must be able to complete. - resume: (executionId, response) => engine.resume(executionId, response), + resume: (executionId, response) => + Effect.flatMap( + decide, + (decision): Effect.Effect => + decision.blocked + ? Effect.succeed({ + status: "completed", + result: { result: null, error: decision.error.message }, + }) + : engine.resume(executionId, response), + ), // Optional member, so it must be forwarded explicitly — a decorator that // rebuilds the object literal drops it, and the host then reads every settled // execution as "never existed". diff --git a/apps/cloud/src/engine/execution-rate-limit.ts b/apps/cloud/src/engine/execution-rate-limit.ts index 906bee252..f266829f1 100644 --- a/apps/cloud/src/engine/execution-rate-limit.ts +++ b/apps/cloud/src/engine/execution-rate-limit.ts @@ -3,7 +3,8 @@ // // The balance gate (execution-gate.ts) depends on Autumn and fails open, so a // billing outage plus runaway automation could still run unbounded executions. -// This limiter counts `execute` calls per organization in a fixed hourly +// This limiter counts `execute` calls (and `resume` continuations — each one +// is fresh work, see execution-gate.ts) per organization in a fixed hourly // window, backed by a minimal counter Durable Object (cross-session state: // each MCP session lives in its own DO instance, so an in-memory counter // would be per-session and trivially bypassed by opening more sessions). diff --git a/apps/cloud/src/engine/execution-resume-gate.node.test.ts b/apps/cloud/src/engine/execution-resume-gate.node.test.ts new file mode 100644 index 000000000..1f9c431ab --- /dev/null +++ b/apps/cloud/src/engine/execution-resume-gate.node.test.ts @@ -0,0 +1,120 @@ +// --------------------------------------------------------------------------- +// Resume is metered work — the regression guard for the un-metered-resume +// bypass. +// +// One execution can pause MANY times (every elicitation is a pause point, and +// each pause mints a new execution id), and each `resume` continues the +// sandbox from where it stopped — fresh arbitrary code and tool calls. Both +// pre-execution guards used to pass `resume` through untouched ("a paused +// execution already consumed its quota slot"), so a single billable +// `executeWithPause` that paused in a loop drove an unbounded chain of +// continuations past the balance gate and the rate limiter: one quota slot, +// zero further checks, unlimited resumed work between the pauses. +// +// These tests pin the fix: a blocked decision refuses the resume with the same +// descriptive error a blocked execute returns, WITHOUT invoking the inner +// engine — nothing runs un-gated, the paused execution and its unconsumed +// approval decision stay intact, and the work completes once the org is back +// under its limit. +// --------------------------------------------------------------------------- + +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import type { ExecutionEngine } from "@executor-js/execution"; + +import { makeExecutionLimitGate } from "./execution-gate"; +import { makeExecutionRateLimiter } from "./execution-rate-limit"; +import { + EXECUTION_LIMIT_BLOCKED_MESSAGE, + RATE_LIMIT_BLOCKED_MESSAGE, +} from "./execution-limit-messages"; + +const ORG = "org_free_tier"; + +/** + * The exploit shape: one execute that pauses, then a chain of resumes — each + * one a fresh unit of continued work that used to run with no gate in front + * of it. + */ +const multiPauseEngine = (work: Array): ExecutionEngine => ({ + execute: () => { + work.push("execute"); + return Effect.succeed({ result: "ran" }); + }, + executeWithPause: () => { + work.push("execute"); + return Effect.succeed({ + status: "paused", + execution: { id: "exec_1", elicitationContext: {} }, + } as never); + }, + resume: () => { + work.push("resume"); + return Effect.succeed({ status: "completed", result: { result: "ran" } }); + }, + getPausedExecution: () => Effect.succeed({ id: "exec_1", elicitationContext: {} } as never), + pausedExecutionCount: () => Effect.succeed(1), + hasPausedExecutions: () => Effect.succeed(true), + getDescription: Effect.succeed("stub"), + // The stub forks nothing, so there is no sandbox fiber to end. + shutdown: Effect.void, +}); + +describe("metered resume — every continuation is gated work", () => { + it("the balance gate refuses a resumed continuation when the quota is spent", async () => { + const work: Array = []; + const gate = makeExecutionLimitGate(() => Effect.succeed({ allowed: false })); + const engine = gate.decorate(ORG, multiPauseEngine(work)); + + const refused = await Effect.runPromise(engine.resume("exec_1", { action: "accept" })); + expect(refused).toMatchObject({ + status: "completed", + result: { result: null, error: EXECUTION_LIMIT_BLOCKED_MESSAGE }, + }); + expect(work, "a blocked resume runs nothing").toEqual([]); + + // The refusal happened BEFORE the engine: the pause is still live and the + // human's approval decision was never consumed by the refused attempt. + expect(await Effect.runPromise(engine.getPausedExecution("exec_1"))).toMatchObject({ + id: "exec_1", + }); + }); + + it("each resume counts against the rate limiter's hourly cap", async () => { + const work: Array = []; + let count = 0; + const limiter = makeExecutionRateLimiter(() => Effect.succeed(++count), { limit: 10 }); + const engine = limiter.decorate(ORG, multiPauseEngine(work)); + + // The billable execute spends count 1; nine continuations spend 2-10 and + // all run; the eleventh increment (count 11) is over the cap and refuses. + await Effect.runPromise(engine.executeWithPause("code")); + for (let i = 0; i < 9; i += 1) + expect(await Effect.runPromise(engine.resume("exec_1", { action: "accept" }))).toMatchObject({ + status: "completed", + result: { result: "ran" }, + }); + expect(work, "execute plus nine allowed resumes reached the engine").toEqual([ + "execute", + ...Array.from({ length: 9 }, () => "resume"), + ]); + + const blocked = await Effect.runPromise(engine.resume("exec_1", { action: "accept" })); + expect(blocked).toMatchObject({ + status: "completed", + result: { result: null, error: RATE_LIMIT_BLOCKED_MESSAGE }, + }); + expect(work.length, "the blocked resume ran nothing").toBe(10); + }); + + it("an allowed balance reaches the engine — gating, not stranding", async () => { + const work: Array = []; + const gate = makeExecutionLimitGate(() => Effect.succeed({ allowed: true })); + const engine = gate.decorate(ORG, multiPauseEngine(work)); + + const resumed = await Effect.runPromise(engine.resume("exec_1", { action: "accept" })); + expect(resumed).toMatchObject({ status: "completed", result: { result: "ran" } }); + expect(work).toEqual(["resume"]); + }); +});