fix(api): rate-limit authenticated document-admin routes (audit H2, partial)#727
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDocument administration routes now use a shared ChangesDocument administration rate limiting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AuthenticatedRoute
participant consumeApiRateLimit
participant SupabaseRPC
participant rateLimitJsonResponse
AuthenticatedRoute->>consumeApiRateLimit: Check document_admin bucket
consumeApiRateLimit->>SupabaseRPC: Call consume_api_rate_limit
SupabaseRPC-->>consumeApiRateLimit: Return limited status and retry data
consumeApiRateLimit-->>AuthenticatedRoute: Return rate-limit result
AuthenticatedRoute->>rateLimitJsonResponse: Build 429 response when limited
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…artial) Wave-H finding H2 (S6): the authenticated document-admin routes had no rate-limit bucket, so a compromised or abusive authenticated client could hammer bulk metadata edits, label writes, and table-fact review with no per-owner ceiling. Adds a `document_admin` bucket (60/min per owner) and consults it — before any table access — in the document-admin routes: - `documents/bulk` (POST) - `documents/[id]/labels` (POST/PATCH/DELETE) - `documents/[id]/table-facts` (GET/PATCH) Exhaustion returns the shared 429 + `Retry-After` envelope via `rateLimitJsonResponse`. The bucket uses `allowInMemoryFallbackOnUnavailable: true` (matching the read/registry buckets), so a durable-limiter outage degrades these authenticated routes to per-instance limiting rather than failing them closed. Tests: new tests/document-admin-rate-limit.test.ts proves a 429 + Retry-After when the durable limiter reports the bucket exhausted (and no table access occurs). Added a not-limited `rpc` stub to tests/document-mutation-routes.test.ts so its functional cases exercise the happy path through the new limiter check. Verification: the 6 affected route suites pass (159), api-rate-limit-fallback passes (11); tsc/eslint/prettier clean on all changed files. (Repo-wide verify:cheap typecheck fails only on pre-existing missing optional test dev deps, unrelated.) Scope note: H2 also lists the ingestion-quality and eval-cases routes; those are authenticated read/eval dashboards and are deferred to a follow-up (separate bucket naming + their own test-mock updates), recorded in docs/audit-remediation-plan-2026-07-14.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zbpyexer9cLgxhrB61RCU
0bf5239 to
8adba31
Compare
…mit buckets) Resolves the expected conflict in src/lib/api-rate-limit.ts after #731 (the H2 remainder) merged first: #731 added the `ingestion_admin` bucket and this PR adds `document_admin`. Keep-both resolution — both entries retained in the ApiRateLimitBucket union and apiRateLimitDefaults. No behaviour change; the two buckets are independent. Verified: document-admin-rate-limit + api-rate-limit-fallback + document-mutation-routes pass (19); tsc/eslint/prettier clean on api-rate-limit.ts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zbpyexer9cLgxhrB61RCU
Summary
Wave-H finding H2 (S6) — the authenticated document-admin routes had no rate-limit bucket, so a compromised or abusive authenticated client could hammer bulk metadata edits, label writes, and table-fact review with no per-owner ceiling.
document_adminbucket (60/min per owner) insrc/lib/api-rate-limit.ts.documents/bulk(POST),documents/[id]/labels(POST/PATCH/DELETE), anddocuments/[id]/table-facts(GET/PATCH).Retry-Afterenvelope viarateLimitJsonResponse.allowInMemoryFallbackOnUnavailable: true(matching the read/registry buckets), so a durable-limiter outage degrades these authenticated routes to per-instance limiting rather than failing them closed.Scope note: H2's ingestion-quality and eval-cases routes (authenticated read/eval dashboards) are deferred to a follow-up, recorded in
docs/audit-remediation-plan-2026-07-14.md.Verification
tests/document-admin-rate-limit.test.ts— 429 +Retry-Afterwhen the durable limiter reports the bucket exhausted, and no table access occurs.tsc,eslint,prettierclean on every changed file; rebased cleanly onto latestmain.npm run verify:pr-localnot run: the repo-wide typecheck fails only on pre-existing missing optional test dev deps (@testing-library/*,@axe-core/playwright) absent in this container and unrelated to this change.npm run verify:ui— N/A; no UI, routing, or styling code changed (onlysrc/app/api/**+src/lib).Risk and rollout
document_adminbucket is application config only, so reverting fully removes the limiter with no cleanup or backfill.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Summary by CodeRabbit
New Features
Tests