fix(api): enforce the stored per-key daily quota (ratePerDay) #175

[beebah30]

Overview

This PR makes an API key's stored ratePerDay daily quota actually enforced. The value was already persisted on ApiKey (column default 10000), settable through POST /admin/keys and scripts/issue-api-key.ts, returned in the admin response, and loaded onto req.apiKey — but nothing ever read it. Only ratePerMin was checked, so a key issued with --per-day 100 was still free to spend its full 60 requests per minute: roughly 86,400 requests a day against an allowance of 100.

The daily counter lives in the Redis instance the API already uses, is keyed by the UTC day so it resets for free at midnight, and is incremented before the request is allowed through, so an over-quota key cannot buy itself a reset by hammering the endpoint. When Redis is unreachable the check fails open and says so loudly, rather than answering 429 to every authenticated request while a cache is down.

Related Issue

Closes #175

Changes

🕒 Daily Quota Enforcement

• [ADD] src/api/dailyQuota.ts

  • consumeDailyQuota(keyId, limit, now) increments lens:apikey:quota:day:<YYYY-MM-DD>:<keyId> in a single MULTI and returns { allowed, remaining, retryAfterSeconds, unavailable }.
  • Each increment refreshes a TTL that always lands on the midnight ending that UTC day, so a finished day's counter is reclaimed exactly when the day is over — no sweep job, and no stale keys.
  • registerDailyQuota(app) — a Fastify onRequest hook that reuses the key apiKeyAuth already resolved (no second DB/Redis lookup) and skips public routes and keyless requests, which have no per-key allowance to spend.
  • now is injectable, so the day boundary is tested deterministically rather than with faked timers.
  • A non-finite stored limit falls back to the ratePerDay column default, so one bad value degrades to the normal default instead of blocking every request.

• [MODIFY] src/index.ts

  • Registers the hook after the API-key auth hook (so req.apiKey is populated) and after @fastify/rate-limit (so the per-minute window is still evaluated first and its behaviour is unchanged).

• [MODIFY] src/metrics.ts

  • api_key_daily_quota_events_total{outcome} — a closed set of exceeded and unavailable. Deliberately not labelled by key id: that value is unbounded, and one series per key would grow without limit.

• [ADD] src/__tests__/dailyQuota.test.ts

  • 17 tests in three groups — day-boundary helpers, consumeDailyQuota units, and plugin-level integration: under-limit passes, over-limit 429s, the UTC day reset, per-key isolation, restart survival, the per-minute limit still tripping first, public routes skipped, and both fail-open paths (thrown error and a null transaction reply).

• [MODIFY] Documentation — docs/api-key-quotas.md (new), .env.example, openapi.yaml / openapi.json

  • Documents both quotas side by side, the UTC-day/Redis design, the fail-open decision and its residual risk, and adds a TooManyRequests description that says which window tripped and what Retry-After means for each.
  • Regenerating openapi.json also picked up pre-existing drift: the /prices/history path was already in openapi.yaml but had never been committed to the JSON. It is mechanical output of the repo's own npm run openapi:gen.

• [ADD] .changeset/enforce-daily-api-key-quota.md

Verification Results

npx tsc --noEmit
✅ exit 0

npm run build
✅ exit 0

npm test
✅ 417 passed | 1 skipped (418)
✅ src/__tests__/dailyQuota.test.ts — 17/17 passed

Acceptance Criteria	Status
A key's ratePerDay is enforced, not merely stored	✅ The onRequest hook refuses the first request past the limit
Over-quota requests get a clear, actionable 429	✅ Same body as the per-minute limiter, with retryAfter (and the Retry-After header) pointing at the next UTC midnight
The daily window resets once per UTC day	✅ The date is part of the counter key, so midnight is simply a different key — no sweep job needed
The enforced count survives an API restart	✅ The counter is in Redis, not process memory
Existing ratePerMin behaviour is unchanged	✅ Registered after @fastify/rate-limit; the suite asserts the per-minute 429 is untouched
Public and unauthenticated requests are unaffected	✅ Skipped by the hook — there is no per-key allowance to spend
A quota that stops being enforced is not silent	✅ Error log plus api_key_daily_quota_events_total{outcome="unavailable"}

[beebah30]

Note for reviewers: this PR supersedes #182 (same author), which I've closed. #182 targeted the same issue (#175) and the same files (src/api/dailyQuota.ts, src/__tests__/dailyQuota.test.ts, src/index.ts), so the two would have conflicted if both stayed open — only one can land.

The substantive difference is the Redis-outage behaviour. #182 returned 503 (fail closed); this PR fails open with an error log and the api_key_daily_quota_events_total{outcome="unavailable"} counter, on the basis that Redis is a cache here and refusing every keyed request during a cache outage would turn it into a full API outage — including for keys far inside their allowance. The residual risk (a key can exceed its quota during an outage) is documented in docs/api-key-quotas.md and alertable via that counter.

If you'd rather have the stricter trust boundary, I'm happy to switch to fail-closed — it's a small change to the unavailable branches in src/api/dailyQuota.ts plus flipping the two fail-open tests. The design rationale is isolated in that one module.

[Miracle656]

This is a strong piece of work and it closes #175 properly. I went looking specifically for the two ways a quota like this usually goes wrong, and it holds up on both:

• No reset race. The counter is a MULTI of INCR + EXPIRE and the increment happens before the decision (src/api/dailyQuota.ts:128), so a key already over its allowance spends a slot on every rejected attempt rather than being able to hammer itself back under. INCR is atomic, so two concurrent requests get distinct values.
• Nothing defaults to unlimited. normalizeLimit (src/api/dailyQuota.ts:92-95) sends a missing or non-finite ratePerDay to the column default of 10,000 — not Infinity, and not NaN, which would have made count <= NaN false and blocked the key entirely. An explicit 0 means zero requests, and a negative clamps to 0. A missing API key is refused one layer up by the auth hook, and if (!apiKey) return here only ever sees public routes or REQUIRE_API_KEY=false.

Plumbing is right too: registered after auth and after @fastify/rate-limit (src/index.ts:104, 116, 136), day-keyed so the reset is free and a restart can't grant a fresh allowance, TTL never extends past the boundary, and api_key_daily_quota_events_total keeps outcome as a closed set with an explicit note that the key id is deliberately not a label — exactly right, that one would have been unbounded.

One thing needs to change.

src/api/dailyQuota.ts:166-169 — the fail-open path logs once per request

req.log.error(...) is called unconditionally for every request that finds Redis unreachable. I measured it against your own test harness — 50 requests through the plugin with exec rejecting:

REQUESTS=50  QUOTA_ERROR_LOG_LINES=50
=> at 10 req/s that is 864,000 lines/day

That's one line per request for the entire duration of an outage, and config.redis.url defaults to redis://localhost:6379 (src/config.ts:240), so any deployment without Redis configured is in this state permanently, not just during an incident.

The reason to care is 40 lines away in src/redis.ts:22-46. That file went out of its way to back off both the reconnect loop and the error events, with the comment:

roughly 170k lines a day, which buries every real message in the log and is how a dead cache stayed invisible for weeks.

This reintroduces that at about five times the volume, and it defeats its own goal: the point of the fail-open branch is to be visible, and a message that repeats a million times a day is as invisible as one that never prints.

The metric is already the right channel for per-request volume — keep api_key_daily_quota_events_total{outcome="unavailable"} incrementing on every request, that's what counters are for. Just throttle the log the same way src/redis.ts:33-46 already does: print the first one, then at most one a minute carrying a count of what was suppressed. An operator still learns the quota has stopped being enforced within a minute, and the alert you recommend in docs/api-key-quotas.md (…{outcome="unavailable"} > 0) is unaffected.

docs/api-key-quotas.md, .env.example and the changeset all currently say "every affected request logs an error", so they'll want the same edit to stay accurate — that's why I'm sending it back rather than patching it myself.

One merge note

openapi.json is generated from openapi.yaml (scripts/generate-openapi.ts). I regenerated it on main in 1c7e200 while merging #184, and your branch regenerates it too, so you'll likely want a rebase and a fresh npm run openapi:gen before this goes in. Your regeneration also picked up a pre-existing /prices/history drift, which is a good catch and worth keeping.

Everything else is ready to go — this is one change, not a rework.