[codex] Harden terminal error responses

README.md

@@ -231,6 +231,20 @@ BASE_URL=http://localhost:3001
    }
    ```
 
+## Error responses
+
+Write endpoints return stable JSON envelopes for body-level failures. Malformed
+JSON is reported as `400 invalid_request` with the message
+`Malformed JSON in request body`; the raw parser message and request body are not
+echoed back to clients. Bodies over the 100 KiB JSON limit remain
+`413 payload_too_large`.
+
+Unhandled server exceptions are logged with the request id, method, path, error
+message, and stack trace. Client-facing `500 internal_error` responses keep the
+request id for correlation but always use the generic message
+`Unexpected server error` so internal paths or secrets from exception messages
+are not leaked.
+
 ## CI/CD
 
 On push/PR to `main`, GitHub Actions runs:

src/error-handling.test.ts

@@ -0,0 +1,99 @@
+import { afterEach, beforeEach, describe, it } from "node:test";
+import assert from "node:assert";
+import express from "express";
+import request from "supertest";
+import { createApp } from "./index.js";
+import { installPreRouteMiddleware } from "./middleware/index.js";
+import { installErrorHandlers } from "./routes/errors.js";
+import { usageStore } from "./store/state.js";
+
+const originalConsoleError = console.error;
+
+beforeEach(() => {
+  usageStore.clear();
+});
+
+afterEach(() => {
+  console.error = originalConsoleError;
+});
+
+function createThrowingApp() {
+  const app = express();
+  installPreRouteMiddleware(app);
+  app.get("/boom", () => {
+    throw new Error("sensitive path /var/private/token.txt");
+  });
+  installErrorHandlers(app);
+  return app;
+}
+
+void describe("terminal error handling", () => {
+  void it("returns a structured 400 for malformed JSON without leaking parser text", async () => {
+    const res = await request(createApp())
+      .post("/api/v1/usage")
+      .set("Content-Type", "application/json")
+      .set("X-Request-Id", "bad-json-request")
+      .send('{"agent":');
+
+    assert.strictEqual(res.status, 400);
+    assert.deepStrictEqual(res.body, {
+      error: "invalid_request",
+      message: "Malformed JSON in request body",
+      requestId: "bad-json-request",
+    });
+    assert.strictEqual(JSON.stringify(res.body).includes("Unexpected end"), false);
+    assert.strictEqual(JSON.stringify(res.body).includes('{"agent":'), false);
+  });
+
+  void it("keeps valid JSON writes unaffected", async () => {
+    const res = await request(createApp())
+      .post("/api/v1/usage")
+      .set("X-Request-Id", "valid-json-request")
+      .send({ agent: "agent-json", serviceId: "svc-json", requests: 1 });
+
+    assert.strictEqual(res.status, 201);
+    assert.deepStrictEqual(res.body, {
+      agent: "agent-json",
+      serviceId: "svc-json",
+      total: 1,
+    });
+  });
+
+  void it("keeps oversized JSON mapped to payload_too_large", async () => {
+    const res = await request(createApp())
+      .post("/api/v1/usage")
+      .set("Content-Type", "application/json")
+      .set("X-Request-Id", "oversized-request")
+      .send({ agent: "a".repeat(120 * 1024), serviceId: "svc-json", requests: 1 });
+
+    assert.strictEqual(res.status, 413);
+    assert.deepStrictEqual(res.body, {
+      error: "payload_too_large",
+      message: "request body exceeds the 100 KiB limit",
+      requestId: "oversized-request",
+    });
+  });
+
+  void it("redacts client-facing 500 messages while logging internal details", async () => {
+    const logged: string[] = [];
+    console.error = (...args: unknown[]) => {
+      logged.push(args.map(String).join(" "));
+    };
+
+    const res = await request(createThrowingApp())
+      .get("/boom")
+      .set("X-Request-Id", "boom-request");
+
+    assert.strictEqual(res.status, 500);
+    assert.deepStrictEqual(res.body, {
+      error: "internal_error",
+      message: "Unexpected server error",
+      method: "GET",
+      path: "/boom",
+      requestId: "boom-request",
+    });
+    assert.strictEqual(JSON.stringify(res.body).includes("/var/private"), false);
+    assert.ok(logged.some((line) => line.includes("boom-request")));
+    assert.ok(logged.some((line) => line.includes("/var/private/token.txt")));
+  });
+});

src/routes/errors.ts

@@ -6,39 +6,98 @@ import {
 } from "express";
 import { getRequestId } from "../types.js";
 
+type ExpressError = Error & {
+  body?: unknown;
+  status?: number;
+  statusCode?: number;
+  type?: string;
+};
+
+function requestIdForError(req: Request): string | undefined {
+  const middlewareId = getRequestId(req);
+  if (middlewareId) return middlewareId;
+
+  const incoming = req.header("x-request-id");
+  return incoming && incoming.length <= 200 ? incoming : undefined;
+}
+
+function isPayloadTooLargeError(err: unknown): boolean {
+  return (
+    err !== null &&
+    typeof err === "object" &&
+    "type" in err &&
+    (err as ExpressError).type === "entity.too.large"
+  );
+}
+
+function isMalformedJsonError(err: unknown): boolean {
+  if (!(err instanceof SyntaxError)) return false;
+  const expressError = err as ExpressError;
+  return (
+    expressError.type === "entity.parse.failed" ||
+    expressError.status === 400 ||
+    expressError.statusCode === 400
+  );
+}
+
+function logInternalError(
+  err: unknown,
+  req: Request,
+  requestId: string | undefined
+): void {
+  const error = err instanceof Error ? err : new Error(String(err));
+  console.error(
+    JSON.stringify({
+      requestId,
+      method: req.method,
+      path: req.path,
+      message: error.message,
+      stack: error.stack,
+    })
+  );
+}
+
 /**
  * Installs the terminal 404 and error handlers after all route modules.
  */
 export function installErrorHandlers(app: Application): void {
   app.use((req: Request, res: Response) => {
+    const requestId = requestIdForError(req);
     res.status(404).json({
       error: "not_found",
       message: `No route for ${req.method} ${req.path}`,
-      requestId: getRequestId(req),
+      requestId,
     });
   });
 
   app.use((err: unknown, req: Request, res: Response, _next: NextFunction) => {
-    if (
-      err &&
-      typeof err === "object" &&
-      "type" in err &&
-      (err as { type: string }).type === "entity.too.large"
-    ) {
+    const requestId = requestIdForError(req);
+
+    if (isMalformedJsonError(err)) {
+      res.status(400).json({
+        error: "invalid_request",
+        message: "Malformed JSON in request body",
+        requestId,
+      });
+      return;
+    }
+
+    if (isPayloadTooLargeError(err)) {
       res.status(413).json({
         error: "payload_too_large",
         message: "request body exceeds the 100 KiB limit",
-        requestId: getRequestId(req),
+        requestId,
       });
       return;
     }
-    const message = err instanceof Error ? err.message : "Unexpected server error";
+
+    logInternalError(err, req, requestId);
     res.status(500).json({
       error: "internal_error",
-      message,
+      message: "Unexpected server error",
       method: req.method,
       path: req.path,
-      requestId: getRequestId(req),
+      requestId,
     });
   });
 }