From 6df47c42fdde44e721a322794567a0bcf1b82cfa Mon Sep 17 00:00:00 2001
From: Omkar Chebale <omkarchebale0@gmail.com>
Date: Sat, 3 Oct 2026 00:42:20 +0530
Subject: [PATCH] Answer a non-JSON body to the approvals API with 400, not 500

The approvals routes' onError mapped only the approval and Zod errors,
so the SyntaxError from req.json() answered 500 with the parser's
message. Map it to 400 as the delivery routes do.
---
 CHANGELOG.md                   |  6 ++++++
 server/src/approvals/routes.ts | 21 +++++++++++++--------
 server/tests/approvals.test.ts | 22 ++++++++++++++++++++++
 3 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9febb9a50..c9e2096d5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,12 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### A request to the approvals API that is not JSON answers 400
+
+A body that could not be parsed as JSON, sent to any approvals route that reads one, such as
+`PATCH /api/approvals/preferences` or `POST /api/approvals/rules`, answered 500 with the parser's own
+message. It now answers 400 "Supply a valid request.", as the delivery routes do.
+
 ### A playground component's Published switch publishes its source too
 
 The Published switch on an admin component page called the generic publication endpoint for every
diff --git a/server/src/approvals/routes.ts b/server/src/approvals/routes.ts
index 689e3fc52..6af698cdf 100644
--- a/server/src/approvals/routes.ts
+++ b/server/src/approvals/routes.ts
@@ -42,14 +42,19 @@ export function createApprovalRoutes(
   const routes = new Hono<{ Variables: AppVariables }>();
   routes.use("*", requireUser);
   routes.onError((error, context) =>
-    context.json(
-      { error: error.message },
-      error instanceof ApprovalNotFoundError
-        ? 404
-        : error instanceof ApprovalRefusedError || error instanceof z.ZodError
-          ? 400
-          : 500,
-    ),
+    // A body that is not JSON is the caller's mistake, as the delivery routes answer it, not a
+    // 500 carrying the parser's message.
+    error instanceof SyntaxError
+      ? context.json({ error: "Supply a valid request." }, 400)
+      : context.json(
+          { error: error.message },
+          error instanceof ApprovalNotFoundError
+            ? 404
+            : error instanceof ApprovalRefusedError ||
+                error instanceof z.ZodError
+              ? 400
+              : 500,
+        ),
   );
   routes.get("/", async (context) =>
     context.json(await service.inbox(context.var.actor.id)),
diff --git a/server/tests/approvals.test.ts b/server/tests/approvals.test.ts
index 2a4ec2a46..7fb8fdf7f 100644
--- a/server/tests/approvals.test.ts
+++ b/server/tests/approvals.test.ts
@@ -716,3 +716,25 @@ test.each([
   expect(continued).toHaveLength(1);
   expect(continued[0]?.result.error).toContain("Not done");
 });
+
+test("a request body that is not JSON answers 400, not a 500 with the parser's message", async () => {
+  const routes = createApprovalRoutes(
+    {} as serviceModule.ApprovalService,
+    async (ctx, next) => {
+      ctx.set("actor", {
+        id: "owner",
+        email: "owner@example.com",
+        role: "user",
+      });
+      await next();
+    },
+  );
+  for (const [method, path] of [
+    ["PATCH", "/preferences"],
+    ["POST", "/rules"],
+  ]) {
+    const response = await routes.request(path, { method, body: "{oops" });
+    expect(response.status).toBe(400);
+    expect(await response.json()).toEqual({ error: "Supply a valid request." });
+  }
+});