From 8adba3157a3c244d7688178581c979d85aba3215 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 16:40:45 +0000 Subject: [PATCH] fix(api): rate-limit authenticated document-admin routes (audit H2, partial) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_011Zbpyexer9cLgxhrB61RCU --- src/app/api/documents/[id]/labels/route.ts | 34 +++++++ .../api/documents/[id]/table-facts/route.ts | 23 +++++ src/app/api/documents/bulk/route.ts | 12 +++ src/lib/api-rate-limit.ts | 6 +- tests/document-admin-rate-limit.test.ts | 92 +++++++++++++++++++ tests/document-mutation-routes.test.ts | 19 ++++ 6 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 tests/document-admin-rate-limit.test.ts diff --git a/src/app/api/documents/[id]/labels/route.ts b/src/app/api/documents/[id]/labels/route.ts index 6cd574841..6cba3a086 100644 --- a/src/app/api/documents/[id]/labels/route.ts +++ b/src/app/api/documents/[id]/labels/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { z } from "zod"; +import { consumeApiRateLimit, rateLimitJsonResponse } from "@/lib/api-rate-limit"; import { isDemoMode } from "@/lib/env"; import { jsonError, PublicApiError } from "@/lib/http"; import { normalizeDocumentLabelForStorage } from "@/lib/document-tags"; @@ -110,6 +111,17 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + await requireOwnedDocument(supabase, id, user.id); const { data: existing, error: existingError } = await supabase @@ -167,6 +179,17 @@ export async function PATCH(request: Request, { params }: { params: Promise<{ id const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + await requireOwnedDocument(supabase, id, user.id); if ("action" in parsed) { @@ -265,6 +288,17 @@ export async function DELETE(request: Request, { params }: { params: Promise<{ i const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + await requireOwnedDocument(supabase, id, user.id); const { data: existing, error: existingError } = await supabase diff --git a/src/app/api/documents/[id]/table-facts/route.ts b/src/app/api/documents/[id]/table-facts/route.ts index 18653deb5..8481cb753 100644 --- a/src/app/api/documents/[id]/table-facts/route.ts +++ b/src/app/api/documents/[id]/table-facts/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { z } from "zod"; +import { consumeApiRateLimit, rateLimitJsonResponse } from "@/lib/api-rate-limit"; import { isDemoMode } from "@/lib/env"; import { jsonError, PublicApiError } from "@/lib/http"; import { invalidateRagCachesForOwner } from "@/lib/rag"; @@ -44,6 +45,17 @@ export async function GET(request: Request, { params }: { params: Promise<{ id: const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + const document = await loadOwnedDocument({ supabase, documentId: id, ownerId: user.id }); if (!document) { return NextResponse.json({ error: "Document not found." }, { status: 404 }); @@ -78,6 +90,17 @@ export async function PATCH(request: Request, { params }: { params: Promise<{ id const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + const document = await loadOwnedDocument({ supabase, documentId: id, ownerId: user.id }); if (!document) { return NextResponse.json({ error: "Document not found." }, { status: 404 }); diff --git a/src/app/api/documents/bulk/route.ts b/src/app/api/documents/bulk/route.ts index 0d04d269e..1223d7087 100644 --- a/src/app/api/documents/bulk/route.ts +++ b/src/app/api/documents/bulk/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { z } from "zod"; +import { consumeApiRateLimit, rateLimitJsonResponse } from "@/lib/api-rate-limit"; import { normalizeDocumentLabelForStorage } from "@/lib/document-tags"; import { isDemoMode } from "@/lib/env"; import { jsonError, PublicApiError } from "@/lib/http"; @@ -129,6 +130,17 @@ export async function POST(request: Request) { const supabase = createAdminClient(); const user = await requireAuthenticatedUser(request, supabase); + + const rateLimit = await consumeApiRateLimit({ + supabase, + ownerId: user.id, + bucket: "document_admin", + allowInMemoryFallbackOnUnavailable: true, + }); + if (rateLimit.limited) { + return rateLimitJsonResponse("Too many document administration requests. Retry shortly.", rateLimit); + } + const ids = Array.from(new Set(parsed.documentIds)); const { data: documents, error: documentsError } = await supabase diff --git a/src/lib/api-rate-limit.ts b/src/lib/api-rate-limit.ts index fb8b5a68a..ec80860f3 100644 --- a/src/lib/api-rate-limit.ts +++ b/src/lib/api-rate-limit.ts @@ -39,7 +39,8 @@ export type ApiRateLimitBucket = | "bulk_reindex" | "source_review" | "answer_feedback" - | "registry"; + | "registry" + | "document_admin"; export type ApiRateLimitResult = { limited: boolean; @@ -60,6 +61,9 @@ const apiRateLimitDefaults = { source_review: { limit: 30, windowSeconds: 60 }, answer_feedback: { limit: 30, windowSeconds: 60 }, registry: { limit: 120, windowSeconds: 60 }, + // Authenticated owner document-admin writes (bulk metadata, label edits, table-fact review). + // Generous for interactive single-owner admin use, bounded against an abusive/compromised client. + document_admin: { limit: 60, windowSeconds: 60 }, } as const satisfies Record; const anonymousApiRateLimitDefaults: Partial> = { diff --git a/tests/document-admin-rate-limit.test.ts b/tests/document-admin-rate-limit.test.ts new file mode 100644 index 000000000..fea56be86 --- /dev/null +++ b/tests/document-admin-rate-limit.test.ts @@ -0,0 +1,92 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +// Proves the `document_admin` bucket enforces a 429 on the authenticated admin +// write/read routes when the durable limiter reports the bucket exhausted. The +// routes pass `allowInMemoryFallbackOnUnavailable: true`, so a valid `limited: true` +// row is honoured directly (the fallback only engages when the limiter errors). + +const userId = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; +const documentId = "11111111-1111-4111-8111-111111111111"; + +function rateLimitedClient() { + return { + from: vi.fn(() => { + throw new Error("no table access should occur once the request is rate limited"); + }), + rpc: vi.fn(async (name: string) => { + if (name === "consume_api_rate_limit") { + return { + data: [ + { + limited: true, + limit_value: 60, + remaining: 0, + retry_after_seconds: 42, + reset_at: new Date(Date.now() + 42_000).toISOString(), + }, + ], + error: null, + }; + } + return { data: [], error: null }; + }), + }; +} + +function mockRuntime(client: ReturnType) { + vi.resetModules(); + vi.doMock("@/lib/env", () => ({ + env: { + MAX_UPLOAD_MB: 150, + SUPABASE_DOCUMENT_BUCKET: "clinical-documents", + SUPABASE_IMAGE_BUCKET: "clinical-images", + RAG_SEARCH_CACHE_TTL_MS: 0, + RAG_SEARCH_CACHE_SIZE: 0, + RAG_ANSWER_CACHE_TTL_MS: 0, + RAG_ANSWER_CACHE_SIZE: 0, + RAG_AWAIT_QUERY_LOGS: false, + WORKER_STALE_AFTER_MINUTES: 10, + WORKER_MAX_ATTEMPTS: 3, + }, + isDemoMode: () => false, + isLocalNoAuthMode: () => false, + requireOpenAIEnv: () => undefined, + requireServerEnv: () => undefined, + })); + vi.doMock("@/lib/supabase/admin", () => ({ createAdminClient: () => client })); + vi.doMock("@/lib/supabase/auth", () => ({ + AuthenticationError: class AuthenticationError extends Error {}, + requireAuthenticatedUser: vi.fn(async () => ({ id: userId })), + unauthorizedResponse: () => new Response(JSON.stringify({ error: "Authentication required." }), { status: 401 }), + })); + vi.doMock("@/lib/rag", () => ({ + invalidateRagCachesForOwner: vi.fn(), + invalidateRagCachesForDocumentMutation: vi.fn(), + })); +} + +afterEach(() => { + vi.restoreAllMocks(); + vi.resetModules(); +}); + +describe("document-admin rate limiting", () => { + it("returns 429 + Retry-After when the admin bucket is exhausted, before any table access", async () => { + const client = rateLimitedClient(); + mockRuntime(client); + const { GET } = await import("../src/app/api/documents/[id]/table-facts/route"); + + const response = await GET( + new Request(`http://localhost/api/documents/${documentId}/table-facts`, { + headers: { authorization: "Bearer valid-token" }, + }), + { params: Promise.resolve({ id: documentId }) }, + ); + const body = (await response.json()) as Record; + + expect(response.status).toBe(429); + expect(response.headers.get("retry-after")).toBe("42"); + expect(body).toMatchObject({ code: "rate_limited" }); + expect(client.from).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/document-mutation-routes.test.ts b/tests/document-mutation-routes.test.ts index d3af3afd6..5d94c7753 100644 --- a/tests/document-mutation-routes.test.ts +++ b/tests/document-mutation-routes.test.ts @@ -96,6 +96,25 @@ function createSupabaseMock(resolveCall: QueryResolver) { calls.push(call); return new QueryBuilder(call, resolveCall); }), + rpc: vi.fn(async (name: string) => { + // The document-admin routes consult the rate limiter before touching tables. + // Return a not-limited row so these functional tests exercise the happy path. + if (name === "consume_api_rate_limit" || name === "consume_api_subject_rate_limit") { + return { + data: [ + { + limited: false, + limit_value: 60, + remaining: 59, + retry_after_seconds: 60, + reset_at: new Date(Date.now() + 60_000).toISOString(), + }, + ], + error: null, + }; + } + return { data: [], error: null }; + }), }, }; }