Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/app/api/documents/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ function projectPublicFields<T extends Record<string, unknown>>(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()
Expand Down
6 changes: 4 additions & 2 deletions src/lib/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 7 additions & 6 deletions src/lib/supabase/auth.ts
Original file line number Diff line number Diff line change
@@ -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<typeof createAdminClient>;

Expand Down Expand Up @@ -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 });
}

/**
Expand Down
44 changes: 44 additions & 0 deletions tests/api-validation-contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
117 changes: 116 additions & 1 deletion tests/http-error-response.test.ts
Original file line number Diff line number Diff line change
@@ -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 () => {
Expand Down Expand Up @@ -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();
});
});