From 6d86b71a3cb31d9b8289f966c4242d855e3ae64a Mon Sep 17 00:00:00 2001 From: Antiheld86 <4201585+Antiheld86@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:21:04 +0000 Subject: [PATCH] Address review: move the query-shape case, document the mint race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two notes from review, neither a behaviour change. `counts only live tokens carrying this scope` mints successfully and then inspects the `where` handed to the count — it was sitting in the refusal block and belongs beside the other what-was-the-helper-given cases. Moved verbatim. `still mints on the last free slot` stays where it is, next to the 409 it is the other half of: the pair is what pins `>=` against `>`, and reading either alone tells you less than reading both together. The ceiling is count-then-create and so not atomic — two mints racing at nine both pass and the account lands eleven. Now said out loud, along with the two things a reader cannot see from this file: the overshoot is bounded because the rate limit above is a single atomic upsert, and the house pattern for making it exact is an advisory lock, named with call sites. Left unserialised on purpose — this catches a runaway loop rather than metering a quota, and a lock on a credential mint buys nothing for what it costs. --- .../measurements/__tests__/route.test.ts | 38 +++++++++---------- src/app/api/tokens/measurements/route.ts | 16 ++++++++ 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/app/api/tokens/measurements/__tests__/route.test.ts b/src/app/api/tokens/measurements/__tests__/route.test.ts index b4836c24a..21ebe8bfd 100644 --- a/src/app/api/tokens/measurements/__tests__/route.test.ts +++ b/src/app/api/tokens/measurements/__tests__/route.test.ts @@ -121,6 +121,25 @@ describe("what it mints", () => { }), ); }); + + it("counts only live tokens carrying this scope", async () => { + // The whole shape of the query, asserted rather than assumed. Counting + // every row would let the `["*"]` tokens a login mints consume the budget + // and refuse a mint for a reason the person cannot see; counting revoked + // or expired rows would mean revoking never frees a slot. + await POST(req({ name: "Scale" }) as never); + + const where = vi.mocked(prisma.apiToken.count).mock.calls[0][0]?.where; + expect(where).toMatchObject({ + userId: USER.id, + revoked: false, + permissions: { has: MEASUREMENTS_WRITE_SCOPE }, + }); + expect(where?.OR).toEqual([ + { expiresAt: null }, + { expiresAt: { gt: expect.any(Date) } }, + ]); + }); }); describe("when it refuses", () => { @@ -173,25 +192,6 @@ describe("when it refuses", () => { expect(res.status).toBe(201); }); - it("counts only live tokens carrying this scope", async () => { - // The whole shape of the query, asserted rather than assumed. Counting - // every row would let the `["*"]` tokens a login mints consume the budget - // and refuse a mint for a reason the person cannot see; counting revoked - // or expired rows would mean revoking never frees a slot. - await POST(req({ name: "Scale" }) as never); - - const where = vi.mocked(prisma.apiToken.count).mock.calls[0][0]?.where; - expect(where).toMatchObject({ - userId: USER.id, - revoked: false, - permissions: { has: MEASUREMENTS_WRITE_SCOPE }, - }); - expect(where?.OR).toEqual([ - { expiresAt: null }, - { expiresAt: { gt: expect.any(Date) } }, - ]); - }); - it("checks the ceiling only after the body validates", async () => { // A malformed request should hear about the malformation, not the // ceiling — and should not spend a query establishing it. diff --git a/src/app/api/tokens/measurements/route.ts b/src/app/api/tokens/measurements/route.ts index e36a5c8e0..d1c30410b 100644 --- a/src/app/api/tokens/measurements/route.ts +++ b/src/app/api/tokens/measurements/route.ts @@ -131,6 +131,22 @@ export const POST = apiHandler(async (request: NextRequest) => { // Placed after validation and before the create: a request that is going to // be refused should not cost a credential, and one that is malformed should // hear about the malformation rather than the ceiling. + // + // Count-then-create is NOT atomic, and deliberately so. Two mints racing at + // nine both read nine, both pass, and the account lands eleven. Nothing here + // prevents that; what bounds it is the rate limit above, which is a single + // `INSERT … ON CONFLICT DO UPDATE … RETURNING` (`lib/rate-limit.ts`) and so + // admits at most `MINT_RATE_LIMIT_MAX` mints per user per window. The race + // can overshoot the ceiling; it cannot run away from it. + // + // Left unserialised because of what this limit is for: catching a runaway + // automation, not metering a quota anybody is billed against. The holder can + // revoke and mint freely, so eleven tokens instead of ten costs nothing, + // while a lock on a credential mint is a contention point on a write that + // must not hang. If it ever does need to be exact, the house pattern is + // `pg_advisory_xact_lock(hashtextextended(, 0))` inside the + // transaction — see `lib/managed-profiles/lifecycle.ts:47` or + // `lib/integrations/oauth-refresh.ts:89`. const live = await prisma.apiToken.count({ where: { userId: user.id,