From f9859fab1c7540b8356bd1c1a728f70c5f800413 Mon Sep 17 00:00:00 2001 From: Erhnysr Date: Mon, 31 Aug 2026 10:23:40 +0300 Subject: [PATCH] fix(spend-control): count in-flight reservations against hourly/daily caps under a live clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check() captures `now` once, then getSpendingInWindow re-read the clock via a second this.now() to decide whether in-flight reservations (pendingTotal) count. On a real advancing clock (Date.now()) the second read can land a millisecond after the captured now, flipping `to >= this.now()` false and silently dropping the pending total from the hourly/daily window — so two concurrent payments could both clear the same remaining budget and overspend the operator's cap. The session window was unaffected (it reads sessionSpent + pendingTotal directly). Thread the caller's single `now` into getSpendingInWindow(from, to, now) and gate on `to >= now`. This removes the racing second read while keeping the guard meaningful: a genuinely historical window (to < now) still excludes live holds. Every existing spend-control test injects a frozen clock (now: () => clock), so its two reads always matched and the sub-ms window was masked — which is why the suite was green. Add a live-clock (advancing) test covering the hourly and daily windows; it fails on the pre-fix code and passes after. --- src/spend-control.test.ts | 46 +++++++++++++++++++++++++++++++++++++++ src/spend-control.ts | 32 +++++++++++++++++---------- 2 files changed, 67 insertions(+), 11 deletions(-) diff --git a/src/spend-control.test.ts b/src/spend-control.test.ts index cc6aee69..aa44b71a 100644 --- a/src/spend-control.test.ts +++ b/src/spend-control.test.ts @@ -780,6 +780,52 @@ describe("x402 onBeforePaymentCreation spend policy", () => { }); }); +describe("in-flight reservations under a live (non-frozen) clock", () => { + // Every other test in this file injects a FROZEN clock (`now: () => clock`, see + // createControl above): both clock reads inside a single check() return the same + // value, so a reservation is always counted and the race below cannot surface. + // Production uses Date.now(), which advances — when a millisecond ticks between + // the `now` check() captures at its top and the second `this.now()` the window + // helper used to read, the hourly/daily window silently dropped the pending + // total. This live clock (each read 1ms later) models that sub-ms advance so the + // concurrent-overspend path is actually exercised. + const liveClock = (startMs = 1_000_000_000_000) => { + let t = startMs; + return () => (t += 1); + }; + + it("counts an in-flight reservation against the hourly window while the clock advances mid-check", () => { + const control = new SpendControl({ + storage: new InMemorySpendControlStorage(), + now: liveClock(), + }); + control.setLimit("hourly", 1.0); + + // Payment A is signed-in-flight: it cleared check() and reserved its cost but + // has not settled yet. + control.reserve(0.8); + + // Payment B arrives concurrently. A's live $0.80 hold plus B's $0.80 is $1.60, + // over the $1.00/hr cap — B must be refused, or the two together overspend. + const result = control.check(0.8, {}); + expect(result.allowed).toBe(false); + expect(result.blockedBy).toBe("hourly"); + }); + + it("counts an in-flight reservation against the daily window while the clock advances mid-check", () => { + const control = new SpendControl({ + storage: new InMemorySpendControlStorage(), + now: liveClock(), + }); + control.setLimit("daily", 1.0); + control.reserve(0.8); + + const result = control.check(0.8, {}); + expect(result.allowed).toBe(false); + expect(result.blockedBy).toBe("daily"); + }); +}); + describe("formatDuration", () => { it("formats seconds", () => { expect(formatDuration(30)).toBe("30s"); diff --git a/src/spend-control.ts b/src/spend-control.ts index 064736a8..9fbe7f98 100644 --- a/src/spend-control.ts +++ b/src/spend-control.ts @@ -461,7 +461,7 @@ export class SpendControl { } if (this.limits.hourly !== undefined) { - const hourlySpent = this.getSpendingInWindow(now - HOUR_MS, now); + const hourlySpent = this.getSpendingInWindow(now - HOUR_MS, now, now); const remaining = this.limits.hourly - hourlySpent; if (estimatedCost > remaining) { const oldestInWindow = this.history.find((r) => r.timestamp >= now - HOUR_MS); @@ -479,7 +479,7 @@ export class SpendControl { } if (this.limits.daily !== undefined) { - const dailySpent = this.getSpendingInWindow(now - DAY_MS, now); + const dailySpent = this.getSpendingInWindow(now - DAY_MS, now, now); const remaining = this.limits.daily - dailySpent; if (estimatedCost > remaining) { const oldestInWindow = this.history.find((r) => r.timestamp >= now - DAY_MS); @@ -600,23 +600,33 @@ export class SpendControl { } } - private getSpendingInWindow(from: number, to: number): number { + // `now` is the single clock reading the caller already took to build the + // window; it must be passed in, not re-read here. In-flight reservations are + // "now" holds, so they count only against a window that reaches the present + // (`to >= now`). The bug this guards against: reading the clock a SECOND time + // inside this method (the old `to >= this.now()`) could land a millisecond + // after the caller's `now`, flip the guard false, and silently drop the + // pending total from the hourly/daily check — letting two concurrent payments + // both clear the same remaining budget. Threading the caller's `now` keeps the + // guard meaningful (a genuinely historical window with `to < now` still + // excludes live holds) without a second, racing read. Every existing test + // injects a frozen clock (`now: () => clock`), so the two reads always matched + // and the sub-ms window went uncaught; see the live-clock test in + // spend-control.test.ts. + private getSpendingInWindow(from: number, to: number, now: number): number { const recorded = this.history .filter((r) => r.timestamp >= from && r.timestamp <= to) .reduce((sum, r) => sum + r.amount, 0); - // In-flight reservations count against every window they could land in. - // Both the hourly and daily windows end at `now`, so a live hold belongs - // to each of them. - return recorded + (to >= this.now() ? this.pendingTotal() : 0); + return recorded + (to >= now ? this.pendingTotal() : 0); } getSpending(window: "hourly" | "daily" | "session"): number { const now = this.now(); switch (window) { case "hourly": - return this.getSpendingInWindow(now - HOUR_MS, now); + return this.getSpendingInWindow(now - HOUR_MS, now, now); case "daily": - return this.getSpendingInWindow(now - DAY_MS, now); + return this.getSpendingInWindow(now - DAY_MS, now, now); case "session": return this.sessionSpent + this.pendingTotal(); } @@ -630,8 +640,8 @@ export class SpendControl { getStatus(): SpendingStatus { const now = this.now(); - const hourlySpent = this.getSpendingInWindow(now - HOUR_MS, now); - const dailySpent = this.getSpendingInWindow(now - DAY_MS, now); + const hourlySpent = this.getSpendingInWindow(now - HOUR_MS, now, now); + const dailySpent = this.getSpendingInWindow(now - DAY_MS, now, now); return { limits: cloneLimits(this.limits),