From 075bb8e39e25b44c8c8325d71113668f509bfa08 Mon Sep 17 00:00:00 2001 From: CleanDev-Fix <219162456+CleanDev-Fix@users.noreply.github.com> Date: Wed, 24 Jun 2026 23:13:33 -0400 Subject: [PATCH] security: redact internal error messages from 500 responses --- README.md | 4 ++ src/error-redaction.test.ts | 110 ++++++++++++++++++++++++++++++++++++ src/routes/errors.ts | 23 +++++++- 3 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 src/error-redaction.test.ts diff --git a/README.md b/README.md index fa721c7..8953afe 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,10 @@ agentpay-backend/ | `npm run dev` | Run with ts-node | | `npm start` | Run production build | +## Error response policy + +Unexpected `500 internal_error` responses return the fixed message `Unexpected server error` while preserving `error`, `method`, `path`, and `requestId` for clients. The raw thrown error message and stack stay in the server log with the same `requestId` for operator debugging. Safe caller-actionable errors such as validation `400` responses and oversized payload `413` responses continue to return specific messages. + ## CI/CD On push/PR to `main`, GitHub Actions runs: diff --git a/src/error-redaction.test.ts b/src/error-redaction.test.ts new file mode 100644 index 0000000..fb1abec --- /dev/null +++ b/src/error-redaction.test.ts @@ -0,0 +1,110 @@ +import { 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"; + +function createThrowingApp(kind: "error" | "non-error") { + const app = express(); + installPreRouteMiddleware(app); + app.get("/boom", (_req, _res, next) => { + if (kind === "error") { + next(new Error("database password file at /var/secrets/prod.env failed")); + return; + } + next("raw token abc123 from /srv/private/config"); + }); + installErrorHandlers(app); + return app; +} + +async function withCapturedConsoleError( + callback: (messages: string[]) => Promise +): Promise { + const originalConsoleError = console.error; + const messages: string[] = []; + console.error = (...args: unknown[]) => { + messages.push(args.map(String).join(" ")); + }; + try { + return await callback(messages); + } finally { + console.error = originalConsoleError; + } +} + +void describe("error redaction", () => { + void it("redacts sensitive Error messages from 500 responses while logging details", async () => { + await withCapturedConsoleError(async (messages) => { + const requestId = "redaction-test-error"; + const res = await request(createThrowingApp("error")) + .get("/boom") + .set("X-Request-Id", requestId); + + assert.strictEqual(res.status, 500); + assert.deepStrictEqual(res.body, { + error: "internal_error", + message: "Unexpected server error", + method: "GET", + path: "/boom", + requestId, + }); + assert.ok(!res.text.includes("/var/secrets/prod.env")); + assert.ok(!res.text.includes("database password file")); + + const logLine = messages.join("\n"); + assert.match(logLine, /redaction-test-error/); + assert.match( + logLine, + /database password file at \/var\/secrets\/prod\.env failed/ + ); + assert.match(logLine, /Error: database password file/); + }); + }); + + void it("redacts non-Error thrown values from 500 responses while logging details", async () => { + await withCapturedConsoleError(async (messages) => { + const requestId = "redaction-test-non-error"; + const res = await request(createThrowingApp("non-error")) + .get("/boom") + .set("X-Request-Id", requestId); + + assert.strictEqual(res.status, 500); + assert.strictEqual(res.body.error, "internal_error"); + assert.strictEqual(res.body.message, "Unexpected server error"); + assert.strictEqual(res.body.requestId, requestId); + assert.ok(!res.text.includes("abc123")); + assert.ok(!res.text.includes("/srv/private/config")); + + const logLine = messages.join("\n"); + assert.match(logLine, /redaction-test-non-error/); + assert.match(logLine, /raw token abc123 from \/srv\/private\/config/); + }); + }); + + void it("keeps existing validation 400 responses caller-actionable", async () => { + const res = await request(createApp()) + .post("/api/v1/usage") + .send({ agent: "", serviceId: "weather", requests: 1 }); + + assert.strictEqual(res.status, 400); + assert.strictEqual(res.body.error, "invalid_request"); + assert.match( + res.body.message as string, + /agent must be a non-empty string up to 256 chars/ + ); + assert.ok(res.body.requestId); + }); + + void it("keeps existing 413 payload-too-large responses stable", async () => { + const res = await request(createApp()) + .post("/api/v1/usage") + .send({ value: "x".repeat(101 * 1024) }); + + assert.strictEqual(res.status, 413); + assert.strictEqual(res.body.error, "payload_too_large"); + assert.strictEqual(res.body.message, "request body exceeds the 100 KiB limit"); + }); +}); diff --git a/src/routes/errors.ts b/src/routes/errors.ts index fd76332..9723a72 100644 --- a/src/routes/errors.ts +++ b/src/routes/errors.ts @@ -6,6 +6,8 @@ import { } from "express"; import { getRequestId } from "../types.js"; +const INTERNAL_ERROR_MESSAGE = "Unexpected server error"; + /** * Installs the terminal 404 and error handlers after all route modules. */ @@ -18,6 +20,10 @@ export function installErrorHandlers(app: Application): void { }); }); + /** + * Logs unexpected internal errors with request context while returning a + * fixed client-facing message so implementation details stay server-side. + */ app.use((err: unknown, req: Request, res: Response, _next: NextFunction) => { if ( err && @@ -32,13 +38,24 @@ export function installErrorHandlers(app: Application): void { }); return; } - const message = err instanceof Error ? err.message : "Unexpected server error"; + + const requestId = getRequestId(req); + console.error( + JSON.stringify({ + requestId, + method: req.method, + path: req.path, + error: err instanceof Error ? err.message : String(err), + stack: err instanceof Error ? err.stack : undefined, + }) + ); + res.status(500).json({ error: "internal_error", - message, + message: INTERNAL_ERROR_MESSAGE, method: req.method, path: req.path, - requestId: getRequestId(req), + requestId, }); }); }