From 7c0a96c69b01436cf0edbbccd1484f3924c80a48 Mon Sep 17 00:00:00 2001 From: Paco Cartones <253313177+pacocartones@users.noreply.github.com> Date: Thu, 6 Aug 2026 05:09:43 +0200 Subject: [PATCH] fix(batches): validate the list endpoint's ?limit query param (#9073) Validated in local merge-train T5 (base49+contributors+pacocartones) --- .../9073-batches-list-limit-validation.md | 1 + src/app/api/v1/batches/parseListLimit.ts | 30 ++++++++++++ src/app/api/v1/batches/route.ts | 10 +++- .../unit/batch-list-limit-validation.test.ts | 46 +++++++++++++++++++ 4 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fixes/9073-batches-list-limit-validation.md create mode 100644 src/app/api/v1/batches/parseListLimit.ts create mode 100644 tests/unit/batch-list-limit-validation.test.ts diff --git a/changelog.d/fixes/9073-batches-list-limit-validation.md b/changelog.d/fixes/9073-batches-list-limit-validation.md new file mode 100644 index 0000000000..18f5538f82 --- /dev/null +++ b/changelog.d/fixes/9073-batches-list-limit-validation.md @@ -0,0 +1 @@ +- **fix(batches):** `GET /v1/batches` now validates the `limit` query param instead of passing `Number.parseInt(limit)` straight to the SQLite `LIMIT` bind. Previously `?limit=abc` threw an unhandled `datatype mismatch` (→ HTTP 500), `?limit=-1`/`0` returned an incoherent `has_more:true` empty page with `last_id:null`, and a large `?limit` read the entire `batches` table into memory. It now returns a `400` for any non-integer or out-of-range value (1–100, default 20), matching the `POST` handler's Zod validation and the OpenAI Batches contract ([#9073](https://github.com/diegosouzapw/OmniRoute/pull/9073)) diff --git a/src/app/api/v1/batches/parseListLimit.ts b/src/app/api/v1/batches/parseListLimit.ts new file mode 100644 index 0000000000..101d6352ab --- /dev/null +++ b/src/app/api/v1/batches/parseListLimit.ts @@ -0,0 +1,30 @@ +// Validates the `limit` query param for `GET /v1/batches`. The list endpoint previously +// passed `Number.parseInt(limit)` straight to the SQLite `LIMIT ?` bind with no validation, +// so `?limit=abc` threw an unhandled "datatype mismatch" (→ 500), `?limit=-1|0` produced an +// incoherent `has_more:true` empty page, and a large `?limit` read the whole table. +// +// This mirrors the OpenAI Batches list contract (integer, 1–100, default 20) and the repo's +// own query-param bounds (see `max_results` in src/shared/validation/schemas/apiV1.ts: +// `z.coerce.number().int().min(1).max(100)`). Kept dependency-free so it is unit-testable +// in isolation; swap in the Zod schema if the maintainer prefers. + +export const DEFAULT_BATCH_LIST_LIMIT = 20; +export const MAX_BATCH_LIST_LIMIT = 100; + +export type ParsedListLimit = + | { ok: true; limit: number } + | { ok: false; message: string }; + +export function parseBatchListLimit(raw: string | null): ParsedListLimit { + if (raw === null || raw === "") { + return { ok: true, limit: DEFAULT_BATCH_LIST_LIMIT }; + } + const n = Number(raw); + if (!Number.isInteger(n) || n < 1 || n > MAX_BATCH_LIST_LIMIT) { + return { + ok: false, + message: `'limit' must be an integer between 1 and ${MAX_BATCH_LIST_LIMIT}`, + }; + } + return { ok: true, limit: n }; +} diff --git a/src/app/api/v1/batches/route.ts b/src/app/api/v1/batches/route.ts index cc4a2bcd14..9b2be67f6c 100644 --- a/src/app/api/v1/batches/route.ts +++ b/src/app/api/v1/batches/route.ts @@ -4,6 +4,7 @@ import { v1BatchCreateSchema } from "@/shared/validation/schemas"; import { NextResponse } from "next/server"; import { getApiKeyRequestScope } from "@/app/api/v1/_helpers/apiKeyScope"; import { formatBatchResponse } from "./formatBatchResponse"; +import { parseBatchListLimit } from "./parseListLimit"; export async function OPTIONS() { return handleCorsOptions(); @@ -69,7 +70,14 @@ export async function GET(request: Request) { const apiKeyId = scope.apiKeyId; const url = new URL(request.url); - const limit = Number.parseInt(url.searchParams.get("limit") || "20"); + const parsedLimit = parseBatchListLimit(url.searchParams.get("limit")); + if (!parsedLimit.ok) { + return NextResponse.json( + { error: { message: parsedLimit.message, type: "invalid_request_error" } }, + { status: 400, headers: CORS_HEADERS } + ); + } + const limit = parsedLimit.limit; const after = url.searchParams.get("after") || undefined; const batches = listBatches(apiKeyId || undefined, limit + 1, after); diff --git a/tests/unit/batch-list-limit-validation.test.ts b/tests/unit/batch-list-limit-validation.test.ts new file mode 100644 index 0000000000..1126ea7f59 --- /dev/null +++ b/tests/unit/batch-list-limit-validation.test.ts @@ -0,0 +1,46 @@ +import { describe, test } from "node:test"; +import assert from "node:assert/strict"; +import { + parseBatchListLimit, + DEFAULT_BATCH_LIST_LIMIT, + MAX_BATCH_LIST_LIMIT, +} from "../../src/app/api/v1/batches/parseListLimit.ts"; + +describe("parseBatchListLimit", () => { + test("absent or empty limit falls back to the default", () => { + assert.deepEqual(parseBatchListLimit(null), { ok: true, limit: DEFAULT_BATCH_LIST_LIMIT }); + assert.deepEqual(parseBatchListLimit(""), { ok: true, limit: DEFAULT_BATCH_LIST_LIMIT }); + }); + + test("valid in-range integers are accepted", () => { + assert.deepEqual(parseBatchListLimit("1"), { ok: true, limit: 1 }); + assert.deepEqual(parseBatchListLimit("20"), { ok: true, limit: 20 }); + assert.deepEqual(parseBatchListLimit("50"), { ok: true, limit: 50 }); + assert.deepEqual(parseBatchListLimit(String(MAX_BATCH_LIST_LIMIT)), { + ok: true, + limit: MAX_BATCH_LIST_LIMIT, + }); + }); + + test("non-numeric limit is rejected instead of reaching the DB (no more 500 on ?limit=abc)", () => { + const r = parseBatchListLimit("abc"); + assert.equal(r.ok, false); + assert.match((r as { message: string }).message, /between 1 and 100/); + }); + + test("negative and zero limits are rejected (no more incoherent has_more)", () => { + assert.equal(parseBatchListLimit("-1").ok, false); + assert.equal(parseBatchListLimit("0").ok, false); + }); + + test("above-max limit is rejected (bounds the read)", () => { + assert.equal(parseBatchListLimit("101").ok, false); + assert.equal(parseBatchListLimit("999999999").ok, false); + // Number("1e9") === 1e9, which parseInt would have truncated to 1 — also out of range. + assert.equal(parseBatchListLimit("1e9").ok, false); + }); + + test("non-integer numeric limits are rejected", () => { + assert.equal(parseBatchListLimit("20.5").ok, false); + }); +});