From 518fafa86e53bff07fb8ccdbca929a46d89b0539 Mon Sep 17 00:00:00 2001 From: Dariusz Koryto Date: Tue, 6 Oct 2026 15:39:22 +0200 Subject: [PATCH] fix(api): report bad request bodies and fractional limits as client errors (#484) A malformed JSON body, or a body over the route's size limit, made Hono throw an HTTPException that the API reported as a 500 internal error. In the companion API the same requests returned 503 "unavailable, please retry", so clients retried a request that could never succeed. These now return 400 bad_request and 413 payload_too_large in the API's usual error shape. A fractional limit or offset such as ?limit=1.5 was passed to SQLite as a float and failed with a datatype mismatch. Limits, offsets and cursors are now truncated to whole numbers, and the two private copies of the clamp helper reuse the shared one. --- apps/api/src/api-request-errors.test.mjs | 87 ++++++++++++++++++++++++ apps/api/src/companion-routes.ts | 4 +- apps/api/src/entity-utils.ts | 4 +- apps/api/src/http-errors.ts | 12 ++++ apps/api/src/index.ts | 4 ++ apps/api/src/memo-list-service.ts | 6 +- apps/api/src/memo-service.ts | 4 +- 7 files changed, 111 insertions(+), 10 deletions(-) create mode 100644 apps/api/src/api-request-errors.test.mjs diff --git a/apps/api/src/api-request-errors.test.mjs b/apps/api/src/api-request-errors.test.mjs new file mode 100644 index 00000000..958e7545 --- /dev/null +++ b/apps/api/src/api-request-errors.test.mjs @@ -0,0 +1,87 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { Database } from "bun:sqlite"; +import { mkdtemp, readdir, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { clampNumber } from "./entity-utils.ts"; +import { fetchEdgeEverApp } from "./index.ts"; +import { createSelfHostedStorageAdapter } from "./self-hosted-storage-adapter.ts"; + +const temporaryDirectories = []; +const databases = []; +afterEach(async () => { + for (const database of databases.splice(0)) database.close(); + await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))); +}); + +const createEnvironment = async () => { + const directory = await mkdtemp(`${tmpdir()}/edgeever-request-errors-`); + temporaryDirectories.push(directory); + const database = new Database(join(directory, "edgeever.sqlite"), { create: true }); + databases.push(database); + database.exec("PRAGMA foreign_keys = ON"); + const migrationDirectory = new URL("../../../migrations/", import.meta.url); + for (const name of (await readdir(migrationDirectory)).filter((value) => value.endsWith(".sql")).sort()) { + database.exec(await readFile(new URL(name, migrationDirectory), "utf8")); + } + database.query("INSERT OR IGNORE INTO workspaces (id, name, is_personal) VALUES ('ws_default', 'Personal', 1)").run(); + return { + storage: createSelfHostedStorageAdapter(database, join(directory, "resources")), + EDGE_EVER_ALLOW_UNAUTHENTICATED: "true", + EDGE_EVER_RUNTIME: "self-hosted-bun", + }; +}; + +const request = (environment, path, init = {}) => fetchEdgeEverApp( + new Request(`http://edgeever.test${path}`, init), + environment, + { waitUntil: () => undefined, passThroughOnException: () => undefined }, +); + +const jsonPost = (body) => ({ + method: "POST", + headers: { "Content-Type": "application/json" }, + body, +}); + +describe("API request errors", () => { + test("reports a malformed JSON body as a client error", async () => { + const environment = await createEnvironment(); + const response = await request(environment, "/api/v1/notebooks", jsonPost("{\"name\":")); + + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ + error: { code: "bad_request", message: "Malformed JSON in request body" }, + }); + }); + + test("reports an oversized body as payload too large", async () => { + const environment = await createEnvironment(); + const oversized = JSON.stringify({ prompt: "x".repeat(450_000) }); + + for (const path of ["/api/v1/plugins/ai/generate", "/api/v1/companion/turns"]) { + const response = await request(environment, path, jsonPost(oversized)); + expect({ path, status: response.status }).toEqual({ path, status: 413 }); + expect(await response.json()).toEqual({ + error: { code: "payload_too_large", message: "The request body is too large." }, + }); + } + }); + + test("accepts a fractional list limit instead of failing in SQL", async () => { + const environment = await createEnvironment(); + const response = await request(environment, "/api/v1/memos?limit=1.5"); + + expect(response.status).toBe(200); + expect(Array.isArray((await response.json()).memos)).toBe(true); + }); + + test("clamps limits and offsets to whole numbers", () => { + expect(clampNumber(1.5, 1, 100)).toBe(1); + expect(clampNumber(99.9, 1, 100)).toBe(99); + expect(clampNumber(250, 1, 100)).toBe(100); + expect(clampNumber(-3, 0, 100)).toBe(0); + expect(clampNumber(Number.NaN, 1, 100)).toBe(1); + expect(clampNumber(Number.POSITIVE_INFINITY, 0, 50)).toBe(50); + }); +}); diff --git a/apps/api/src/companion-routes.ts b/apps/api/src/companion-routes.ts index 596a9459..f2b79c5d 100644 --- a/apps/api/src/companion-routes.ts +++ b/apps/api/src/companion-routes.ts @@ -7,7 +7,7 @@ import { bodyLimit } from "hono/body-limit"; import type { AppContext, AppEnv, Bindings } from "./api-context"; import { AppError } from "./app-error"; import { loadDefaultAiModel, loadDefaultAiModelCredentials } from "./ai-service"; -import { apiError, forbidden, notFound } from "./http-errors"; +import { apiError, forbidden, httpExceptionError, notFound } from "./http-errors"; import { requireUser, getWorkspaceId } from "./request-auth"; import { beginCompanionTurn, checkpointCompanionTurn, clearCompanionHistory, companionRevision, forgetCompanionMemory, getCompanionTurn, listCompanionMemories, listCompanionTurns, mapCompanionTurn, @@ -23,6 +23,8 @@ import { acknowledgeDiscovery, rememberDiscoveryFeedback, checkDiscoveries, comp const scopeFor = (c: AppContext): CompanionScope => ({ workspaceId: getWorkspaceId(c), ownerId: c.get("auth").actorId! }); const fail = (c: AppContext, error: unknown) => { if (error instanceof AppError) return apiError(c, error.code, error.message, error.status); + const requestError = httpExceptionError(c, error); + if (requestError) return requestError; console.error("companion_failed", error instanceof Error ? error.name : "unknown"); return apiError(c, "companion_failed", "The companion is unavailable. Please retry later.", 503); }; diff --git a/apps/api/src/entity-utils.ts b/apps/api/src/entity-utils.ts index 4413e69a..fe48c51e 100644 --- a/apps/api/src/entity-utils.ts +++ b/apps/api/src/entity-utils.ts @@ -2,8 +2,10 @@ export const createId = (prefix: string) => `${prefix}_${crypto.randomUUID().rep export const isoNow = () => new Date().toISOString(); +// Limits, offsets and cursors are bound to SQL integer parameters, which +// reject fractions, so a value such as `?limit=1.5` is truncated first. export const clampNumber = (value: number, min: number, max: number) => - Number.isNaN(value) ? min : Math.min(Math.max(value, min), max); + Number.isNaN(value) ? min : Math.min(Math.max(Math.trunc(value), min), max); export const parseJsonArray = (json: string): string[] => { try { diff --git a/apps/api/src/http-errors.ts b/apps/api/src/http-errors.ts index 65afbd1e..d3692dce 100644 --- a/apps/api/src/http-errors.ts +++ b/apps/api/src/http-errors.ts @@ -1,4 +1,5 @@ import type { Context } from "hono"; +import { HTTPException } from "hono/http-exception"; export const apiError = (c: Context, code: string, message: string, status: number) => c.json( @@ -41,3 +42,14 @@ export const unauthorized = (c: Context, message: string) => export const forbidden = (c: Context, message: string) => apiError(c, "forbidden", message, 403); + +// Hono middleware (JSON validation, body limits) rejects bad requests by +// throwing HTTPException. Report those as client errors in the API's error +// shape instead of letting them surface as an internal server error. +export const httpExceptionError = (c: Context, error: unknown) => { + if (!(error instanceof HTTPException) || error.status < 400 || error.status >= 500) return null; + if (error.status === 413) { + return apiError(c, "payload_too_large", "The request body is too large.", 413); + } + return apiError(c, "bad_request", error.message || "The request could not be read.", error.status); +}; diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index ab3cc1c5..7662a973 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -58,6 +58,7 @@ import { apiError, authNotConfigured, databaseNotReady, + httpExceptionError, unauthorized, } from "./http-errors"; import { audit } from "./audit"; @@ -478,6 +479,9 @@ app.onError((error, c) => { return apiError(c, error.code, error.message, error.status); } + const requestError = httpExceptionError(c, error); + if (requestError) return requestError; + if (isDatabaseNotReadyError(error)) { console.error("EdgeEver database readiness check failed", error); return databaseNotReady(c); diff --git a/apps/api/src/memo-list-service.ts b/apps/api/src/memo-list-service.ts index bbfbd230..761c2ad4 100644 --- a/apps/api/src/memo-list-service.ts +++ b/apps/api/src/memo-list-service.ts @@ -8,7 +8,7 @@ import { getTableSummary, type MemoSummary, } from "@edgeever/shared"; -import { parseJsonArray } from "./entity-utils"; +import { clampNumber, parseJsonArray } from "./entity-utils"; import type { DatabaseAdapter } from "./storage-contract"; export type MemoSummaryRow = { @@ -95,10 +95,6 @@ const normalizeMemoListSort = (value: string | undefined): MemoListSortMode => const normalizeMemoListFilter = (value: string | undefined): MemoListFilterMode => value === "tagged" || value === "untagged" || value === "pinned" ? value : "all"; -const clampNumber = (value: number, min: number, max: number) => { - if (Number.isNaN(value)) return min; - return Math.min(Math.max(value, min), max); -}; const encodeMemoListCursor = ( memo: MemoSummaryRow, diff --git a/apps/api/src/memo-service.ts b/apps/api/src/memo-service.ts index 0f23d8d4..739e7b3e 100644 --- a/apps/api/src/memo-service.ts +++ b/apps/api/src/memo-service.ts @@ -22,7 +22,7 @@ import { import { auditStatement } from "./audit"; import type { AppContext, AuditActor, AuthContext, Bindings } from "./api-context"; import { AppError } from "./app-error"; -import { createId, isoNow, parseJsonArray } from "./entity-utils"; +import { clampNumber, createId, isoNow, parseJsonArray } from "./entity-utils"; import { workspaceInboxId } from "./notebook-service"; import { sha256 } from "./hash-utils"; import { getRequiredString } from "./mcp-json-rpc"; @@ -53,8 +53,6 @@ export type MemoMutationCommit = { after: (memoId: string) => PreparedStatementAdapter[]; }; -const clampNumber = (value: number, min: number, max: number) => - Number.isNaN(value) ? min : Math.min(Math.max(value, min), max); export type MemoDetailRow = MemoSummaryRow & { content_json: string;