mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 14:16:12 +00:00
Address review: move the query-shape case, document the mint race
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.
This commit is contained in:
1 parent
d9d4a5703a
commit
6d86b71a3c
2 files changed
+35
-19
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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(<key>, 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,
|
||||
|
||||
Reference in new issue
Block a user