From d3c9828207027602f166cb2dbaf124553fcf3755 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Sun, 16 Aug 2026 14:57:29 -0700 Subject: [PATCH] fix: metering gui maths (#3592) --- .../services/metering/MeteringService.test.ts | 21 ++++++++++++++++++ .../services/metering/MeteringService.ts | 22 ++++++++++++++----- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/backend/services/metering/MeteringService.test.ts b/src/backend/services/metering/MeteringService.test.ts index 78fa36fe8..e3b81e653 100644 --- a/src/backend/services/metering/MeteringService.test.ts +++ b/src/backend/services/metering/MeteringService.test.ts @@ -1432,6 +1432,27 @@ describe('MeteringService', () => { expect(allowed.remaining).toBe(0); }); + it('never trusts a stored allowanceUsed past the month total', async () => { + const sub = await target.getActorSubscription(actor); + const month = `${new Date().getUTCFullYear()}-${String(new Date().getUTCMonth() + 1).padStart(2, '0')}`; + + // A corrupt record: allowanceUsed grew past the total (a raced + // or repeated write). The split is bookkeeping over the total, + // so the total is the most the allowance can have been charged. + await server.stores.meteringBuffer.incr({ + key: `${METRICS_PREFIX}:actor:${actor.user.uuid}:${month}`, + pathAndAmountMap: { + total: 1_000_000, + allowanceUsed: sub.monthUsageAllowance + 99_000_000, + }, + }); + + const allowed = await target.getAllowedUsage(actor); + expect(allowed.remaining).toBe( + sub.monthUsageAllowance - 1_000_000, + ); + }); + it('folds the legacy baseline in exactly once under concurrent increments', async () => { const sub = await target.getActorSubscription(actor); const month = `${new Date().getUTCFullYear()}-${String(new Date().getUTCMonth() + 1).padStart(2, '0')}`; diff --git a/src/backend/services/metering/MeteringService.ts b/src/backend/services/metering/MeteringService.ts index 1948cdf7d..ce6113464 100644 --- a/src/backend/services/metering/MeteringService.ts +++ b/src/backend/services/metering/MeteringService.ts @@ -1144,15 +1144,23 @@ export class MeteringService extends PuterService { * pre-split reading — everything counted against the allowance, capped at * it — which is also what keeps balances unchanged across the deploy that * introduced the field. + * + * Allowance-charged spend is a subset of spend, so the stored value is + * never trusted past the month's total: the split is bookkeeping over the + * total, and a stored value exceeding it is corrupt (a write raced or + * repeated). Clamping here makes such a record cost the user at most their + * real spend rather than however large the corrupt value grew. */ private static allowanceUsedFrom( usage: UsageByType | null | undefined, monthUsageAllowance: number, ): number { if (!usage) return 0; - return ( + const total = usage.total || 0; + return Math.min( usage.allowanceUsed ?? - Math.min(usage.total || 0, Math.max(0, monthUsageAllowance || 0)) + Math.min(total, Math.max(0, monthUsageAllowance || 0)), + total, ); } @@ -1774,11 +1782,13 @@ export class MeteringService extends PuterService { 0, (usageRecord.total || 0) - incrementCost, ); - // Same fallback as `allowanceUsedFrom`, measured before this - // increment's own cost. - const usedBefore = + // Same fallback and corrupt-value clamp as `allowanceUsedFrom`, + // measured before this increment's own cost. + const usedBefore = Math.min( usageRecord.allowanceUsed ?? - Math.min(totalBefore, Math.max(0, monthUsageAllowance || 0)); + Math.min(totalBefore, Math.max(0, monthUsageAllowance || 0)), + totalBefore, + ); let baseline = 0; if (usageRecord.allowanceUsed === undefined && usedBefore > 0) {