diff --git a/src/app/api/documents/route.ts b/src/app/api/documents/route.ts index cb3350718..5524e7b7b 100644 --- a/src/app/api/documents/route.ts +++ b/src/app/api/documents/route.ts @@ -111,7 +111,9 @@ function projectPublicFields>(row: T, columns: const documentListQuerySchema = z.object({ limit: queryInteger({ fallback: 100, min: 1, max: 200 }), - offset: queryInteger({ fallback: 0, min: 0, max: 1_000_000 }), + // Cap the offset at 10k (matching the jobs list) so a deep-offset request cannot force + // PostgREST to skip through a million rows as a slow-query/DoS lever. `queryInteger` clamps. + offset: queryInteger({ fallback: 0, min: 0, max: 10_000 }), q: z.string().optional().default("").transform(safeSearchTerm), status: z .string() diff --git a/src/lib/http.ts b/src/lib/http.ts index 82ee6613c..7daeaa6b8 100644 --- a/src/lib/http.ts +++ b/src/lib/http.ts @@ -55,12 +55,14 @@ function logSafeError(error: unknown, status: number) { }); } -export function jsonError(error: unknown, status = 500) { +export function jsonError(error: unknown, status = 500, options?: { log?: boolean }) { const responseStatus = error instanceof PublicApiError ? error.status : status; const message = publicErrorMessage(error, responseStatus); const code = publicErrorCode(error, responseStatus); const requestId = error instanceof PublicApiError ? error.details?.requestId : undefined; - logSafeError(error, responseStatus); + // Expected client-auth failures (e.g. an unauthenticated request) can opt out of the + // error-level log so a routine 401 does not read as a server fault in the logs. + if (options?.log ?? true) logSafeError(error, responseStatus); return NextResponse.json( { error: message, diff --git a/src/lib/supabase/auth.ts b/src/lib/supabase/auth.ts index 49b6bee29..d27d31a9f 100644 --- a/src/lib/supabase/auth.ts +++ b/src/lib/supabase/auth.ts @@ -1,7 +1,7 @@ import { createServerClient, parseCookieHeader } from "@supabase/ssr"; -import { NextResponse } from "next/server"; -import { createAdminClient } from "@/lib/supabase/admin"; import { env } from "@/lib/env"; +import { jsonError } from "@/lib/http"; +import { createAdminClient } from "@/lib/supabase/admin"; type AdminClient = ReturnType; @@ -78,10 +78,11 @@ export class AuthenticationError extends Error { export function unauthorizedResponse(error?: AuthenticationError) { void error; - return NextResponse.json( - { error: "Authentication required." }, - { status: 401, headers: { "Cache-Control": "private, no-store" } }, - ); + // Route through the shared error envelope so 401s carry a stable `code`/`message` + // like every other API failure (and inherit its `Cache-Control: private, no-store`). + // `log: false` keeps a routine unauthenticated request from being recorded as a + // server-side error. + return jsonError(new AuthenticationError(), 401, { log: false }); } /** diff --git a/tests/api-validation-contract.test.ts b/tests/api-validation-contract.test.ts index 41743422c..e5251971f 100644 --- a/tests/api-validation-contract.test.ts +++ b/tests/api-validation-contract.test.ts @@ -288,6 +288,50 @@ describe("API validation contracts", () => { expect(client.calls[1].range).toEqual({ from: 0, to: 99 }); }); + it("caps the document list offset at 10k so a deep-offset request cannot force a full scan", async () => { + const client = createSupabaseMock(() => ok([], 0)); + mockRuntime(client); + const { GET } = await import("../src/app/api/documents/route"); + + const response = await GET(authenticatedRequest("/api/documents?limit=200&offset=5000000&includeMeta=false")); + const body = await payload(response); + + expect(response.status).toBe(200); + expect(body.pagination).toMatchObject({ limit: 200, offset: 10_000 }); + expect(client.calls[0].range).toEqual({ from: 10_000, to: 10_199 }); + }); + + it("passes through a document list offset at or under the 10k cap unchanged", async () => { + const client = createSupabaseMock(() => ok([], 0)); + mockRuntime(client); + const { GET } = await import("../src/app/api/documents/route"); + + const underCapResponse = await GET(authenticatedRequest("/api/documents?limit=50&offset=9999&includeMeta=false")); + const underCapBody = await payload(underCapResponse); + const atCapResponse = await GET(authenticatedRequest("/api/documents?limit=50&offset=10000&includeMeta=false")); + const atCapBody = await payload(atCapResponse); + + expect(underCapResponse.status).toBe(200); + expect(underCapBody.pagination).toMatchObject({ limit: 50, offset: 9999 }); + expect(client.calls[0].range).toEqual({ from: 9999, to: 10_048 }); + expect(atCapResponse.status).toBe(200); + expect(atCapBody.pagination).toMatchObject({ limit: 50, offset: 10_000 }); + expect(client.calls[1].range).toEqual({ from: 10_000, to: 10_049 }); + }); + + it("clamps a document list offset that is just past the 10k cap down to exactly 10k", async () => { + const client = createSupabaseMock(() => ok([], 0)); + mockRuntime(client); + const { GET } = await import("../src/app/api/documents/route"); + + const response = await GET(authenticatedRequest("/api/documents?limit=50&offset=10001&includeMeta=false")); + const body = await payload(response); + + expect(response.status).toBe(200); + expect(body.pagination).toMatchObject({ limit: 50, offset: 10_000 }); + expect(client.calls[0].range).toEqual({ from: 10_000, to: 10_049 }); + }); + it("treats empty document-detail chunk as absent and clamps page/chunk windows", async () => { const client = createSupabaseMock((call) => { if (call.table === "documents" && call.maybeSingle) { diff --git a/tests/http-error-response.test.ts b/tests/http-error-response.test.ts index 5c544d600..61da5cc9c 100644 --- a/tests/http-error-response.test.ts +++ b/tests/http-error-response.test.ts @@ -1,5 +1,7 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { jsonError, PublicApiError } from "../src/lib/http"; +import { logger } from "../src/lib/logger"; +import { AuthenticationError, unauthorizedResponse } from "../src/lib/supabase/auth"; describe("jsonError public payload", () => { it("keeps public error payloads stable without exposing stack or internal causes", async () => { @@ -84,3 +86,116 @@ describe("jsonError public payload", () => { expect(emptyBody.code).toBe("request_failed"); }); }); + +describe("jsonError logging opt-out", () => { + afterEach(() => vi.restoreAllMocks()); + + it("logs by default but stays silent when log is disabled, without changing the payload", async () => { + const errorSpy = vi.spyOn(logger, "error").mockImplementation(() => {}); + + const logged = await jsonError(new PublicApiError("nope", 500, { code: "boom" })).json(); + expect(errorSpy).toHaveBeenCalledTimes(1); + + errorSpy.mockClear(); + const silent = await jsonError(new PublicApiError("nope", 500, { code: "boom" }), 500, { log: false }).json(); + expect(errorSpy).not.toHaveBeenCalled(); + expect(silent).toEqual(logged); + }); + + it("still logs when an options object is passed without an explicit log key", async () => { + const errorSpy = vi.spyOn(logger, "error").mockImplementation(() => {}); + + jsonError(new PublicApiError("nope", 500, { code: "boom" }), 500, {}); + + expect(errorSpy).toHaveBeenCalledTimes(1); + }); + + it("logs when log is explicitly enabled", async () => { + const errorSpy = vi.spyOn(logger, "error").mockImplementation(() => {}); + + jsonError(new PublicApiError("nope", 500, { code: "boom" }), 500, { log: true }); + + expect(errorSpy).toHaveBeenCalledTimes(1); + }); + + it("suppresses logging for a plain (non-PublicApiError) error when log is disabled", async () => { + const errorSpy = vi.spyOn(logger, "error").mockImplementation(() => {}); + + const response = jsonError(new Error("boom"), 503, { log: false }); + const body = await response.json(); + + expect(errorSpy).not.toHaveBeenCalled(); + expect(response.status).toBe(503); + expect(body).toEqual({ + error: "Request failed.", + message: "Request failed.", + code: "internal_error", + }); + }); + + it("does not affect the response status, headers, or requestId when logging is disabled", async () => { + vi.spyOn(logger, "error").mockImplementation(() => {}); + + const error = new PublicApiError("Search failed safely.", 503, { + code: "search_unavailable", + requestId: "req_456", + }); + const response = jsonError(error, 500, { log: false }); + const body = await response.json(); + + expect(response.status).toBe(503); + expect(response.headers.get("Cache-Control")).toBe("private, no-store"); + expect(body).toEqual({ + error: "Search failed safely.", + message: "Search failed safely.", + code: "search_unavailable", + requestId: "req_456", + }); + }); +}); + +describe("unauthorizedResponse envelope", () => { + afterEach(() => vi.restoreAllMocks()); + + it("returns the shared error envelope with a stable authentication_required code", async () => { + const response = unauthorizedResponse(); + const body = await response.json(); + + expect(response.status).toBe(401); + expect(body).toEqual({ + error: "Authentication required.", + message: "Authentication required.", + code: "authentication_required", + }); + }); + + it("does not record a routine unauthenticated request as a server error", () => { + const errorSpy = vi.spyOn(logger, "error").mockImplementation(() => {}); + unauthorizedResponse(); + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it("ignores a caller-supplied AuthenticationError and always returns the generic message", async () => { + const response = unauthorizedResponse(new AuthenticationError("Session expired for user 12345")); + const body = await response.json(); + + expect(response.status).toBe(401); + expect(body).toEqual({ + error: "Authentication required.", + message: "Authentication required.", + code: "authentication_required", + }); + }); + + it("sets a private, no-store Cache-Control header so a 401 is never cached or shared", () => { + const response = unauthorizedResponse(); + + expect(response.headers.get("Cache-Control")).toBe("private, no-store"); + }); + + it("does not log even when a caller-supplied AuthenticationError is provided", () => { + const errorSpy = vi.spyOn(logger, "error").mockImplementation(() => {}); + unauthorizedResponse(new AuthenticationError("some internal detail")); + expect(errorSpy).not.toHaveBeenCalled(); + }); +});