diff --git a/src/__tests__/convert.test.ts b/src/__tests__/convert.test.ts index f97aaa1..b552576 100644 --- a/src/__tests__/convert.test.ts +++ b/src/__tests__/convert.test.ts @@ -53,25 +53,25 @@ describe("POST /v1/convert/html", () => { expect(res.headers["content-type"]).toMatch(/application\/pdf/); }); - it("returns 429 on QUEUE_FULL", async () => { + it("returns 503 on QUEUE_FULL", async () => { const { renderPdf } = await import("../services/browser.js"); vi.mocked(renderPdf).mockRejectedValue(new Error("QUEUE_FULL")); const res = await request(app) .post("/v1/convert/html") .set("content-type", "application/json") .send({ html: "

Hello

" }); - expect(res.status).toBe(429); + expect(res.status).toBe(503); }); - it("returns 500 on PDF_TIMEOUT", async () => { + it("returns 504 on PDF_TIMEOUT", async () => { const { renderPdf } = await import("../services/browser.js"); vi.mocked(renderPdf).mockRejectedValue(new Error("PDF_TIMEOUT")); const res = await request(app) .post("/v1/convert/html") .set("content-type", "application/json") .send({ html: "

Hello

" }); - expect(res.status).toBe(500); - expect(res.body.error).toMatch(/PDF_TIMEOUT/); + expect(res.status).toBe(504); + expect(res.body.error).toBe("PDF generation timed out."); }); it("wraps fragments (no { diff --git a/src/__tests__/error-responses.test.ts b/src/__tests__/error-responses.test.ts new file mode 100644 index 0000000..2dafab0 --- /dev/null +++ b/src/__tests__/error-responses.test.ts @@ -0,0 +1,268 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import express from "express"; +import request from "supertest"; + +/** + * Test suite for error response security and consistency (TDD) + * + * Issues being fixed: + * 1. Convert routes leak internal error messages via err.message + * 2. Templates route leaks error details + * 3. Convert routes don't handle PDF_TIMEOUT (should be 504) + * 4. Inconsistent QUEUE_FULL status codes (should be 503, not 429) + */ + +describe("Error Response Security - Convert Routes", () => { + let app: express.Express; + + beforeEach(async () => { + vi.clearAllMocks(); + vi.resetModules(); + + const { renderPdf } = await import("../services/browser.js"); + vi.mocked(renderPdf).mockResolvedValue({ pdf: Buffer.from("%PDF-1.4 mock"), durationMs: 10 }); + + const { convertRouter } = await import("../routes/convert.js"); + app = express(); + app.use(express.json({ limit: "500kb" })); + app.use("/v1/convert", convertRouter); + }); + + describe("QUEUE_FULL handling", () => { + it("returns 503 (not 429) for QUEUE_FULL on /html", async () => { + const { renderPdf } = await import("../services/browser.js"); + vi.mocked(renderPdf).mockRejectedValue(new Error("QUEUE_FULL")); + + const res = await request(app) + .post("/v1/convert/html") + .set("content-type", "application/json") + .send({ html: "

Test

" }); + + expect(res.status).toBe(503); + expect(res.body.error).toBe("Server busy — too many concurrent PDF generations. Please try again in a few seconds."); + }); + + it("returns 503 (not 429) for QUEUE_FULL on /markdown", async () => { + const { renderPdf } = await import("../services/browser.js"); + vi.mocked(renderPdf).mockRejectedValue(new Error("QUEUE_FULL")); + + const res = await request(app) + .post("/v1/convert/markdown") + .set("content-type", "application/json") + .send({ markdown: "# Test" }); + + expect(res.status).toBe(503); + expect(res.body.error).toBe("Server busy — too many concurrent PDF generations. Please try again in a few seconds."); + }); + + it("returns 503 (not 429) for QUEUE_FULL on /url", async () => { + vi.mock("node:dns/promises", () => ({ + default: { lookup: vi.fn().mockResolvedValue({ address: "93.184.216.34", family: 4 }) }, + })); + + const { renderUrlPdf } = await import("../services/browser.js"); + vi.mocked(renderUrlPdf).mockRejectedValue(new Error("QUEUE_FULL")); + + const res = await request(app) + .post("/v1/convert/url") + .set("content-type", "application/json") + .send({ url: "https://example.com" }); + + expect(res.status).toBe(503); + expect(res.body.error).toBe("Server busy — too many concurrent PDF generations. Please try again in a few seconds."); + }); + }); + + describe("PDF_TIMEOUT handling", () => { + it("returns 504 for PDF_TIMEOUT on /html", async () => { + const { renderPdf } = await import("../services/browser.js"); + vi.mocked(renderPdf).mockRejectedValue(new Error("PDF_TIMEOUT")); + + const res = await request(app) + .post("/v1/convert/html") + .set("content-type", "application/json") + .send({ html: "

Test

" }); + + expect(res.status).toBe(504); + expect(res.body.error).toBe("PDF generation timed out."); + }); + + it("returns 504 for PDF_TIMEOUT on /markdown", async () => { + const { renderPdf } = await import("../services/browser.js"); + vi.mocked(renderPdf).mockRejectedValue(new Error("PDF_TIMEOUT")); + + const res = await request(app) + .post("/v1/convert/markdown") + .set("content-type", "application/json") + .send({ markdown: "# Test" }); + + expect(res.status).toBe(504); + expect(res.body.error).toBe("PDF generation timed out."); + }); + + it("returns 504 for PDF_TIMEOUT on /url", async () => { + vi.mock("node:dns/promises", () => ({ + default: { lookup: vi.fn().mockResolvedValue({ address: "93.184.216.34", family: 4 }) }, + })); + + const { renderUrlPdf } = await import("../services/browser.js"); + vi.mocked(renderUrlPdf).mockRejectedValue(new Error("PDF_TIMEOUT")); + + const res = await request(app) + .post("/v1/convert/url") + .set("content-type", "application/json") + .send({ url: "https://example.com" }); + + expect(res.status).toBe(504); + expect(res.body.error).toBe("PDF generation timed out."); + }); + }); + + describe("Generic error handling (no information disclosure)", () => { + it("does not expose internal error message on /html", async () => { + const { renderPdf } = await import("../services/browser.js"); + const internalError = new Error("Puppeteer crashed: SIGSEGV in Chrome process"); + vi.mocked(renderPdf).mockRejectedValue(internalError); + + const res = await request(app) + .post("/v1/convert/html") + .set("content-type", "application/json") + .send({ html: "

Test

" }); + + expect(res.status).toBe(500); + expect(res.body.error).toBe("PDF generation failed."); + expect(res.body.error).not.toContain("Puppeteer"); + expect(res.body.error).not.toContain("SIGSEGV"); + expect(res.body.error).not.toContain("Chrome"); + }); + + it("does not expose internal error message on /markdown", async () => { + const { renderPdf } = await import("../services/browser.js"); + const internalError = new Error("Page.evaluate() failed: Cannot read property 'x' of undefined"); + vi.mocked(renderPdf).mockRejectedValue(internalError); + + const res = await request(app) + .post("/v1/convert/markdown") + .set("content-type", "application/json") + .send({ markdown: "# Test" }); + + expect(res.status).toBe(500); + expect(res.body.error).toBe("PDF generation failed."); + expect(res.body.error).not.toContain("evaluate"); + expect(res.body.error).not.toContain("undefined"); + }); + + it("does not expose internal error message on /url", async () => { + vi.mock("node:dns/promises", () => ({ + default: { lookup: vi.fn().mockResolvedValue({ address: "93.184.216.34", family: 4 }) }, + })); + + const { renderUrlPdf } = await import("../services/browser.js"); + const internalError = new Error("Browser context crashed with exit code 137"); + vi.mocked(renderUrlPdf).mockRejectedValue(internalError); + + const res = await request(app) + .post("/v1/convert/url") + .set("content-type", "application/json") + .send({ url: "https://example.com" }); + + expect(res.status).toBe(500); + expect(res.body.error).toBe("PDF generation failed."); + expect(res.body.error).not.toContain("context crashed"); + expect(res.body.error).not.toContain("exit code"); + }); + }); +}); + +describe("Error Response Security - Templates Route", () => { + let app: express.Express; + + beforeEach(async () => { + vi.clearAllMocks(); + vi.resetModules(); + + const { renderPdf } = await import("../services/browser.js"); + vi.mocked(renderPdf).mockResolvedValue({ pdf: Buffer.from("%PDF-1.4 mock"), durationMs: 10 }); + + const { templatesRouter } = await import("../routes/templates.js"); + app = express(); + app.use(express.json({ limit: "500kb" })); + app.use("/v1/templates", templatesRouter); + }); + + it("does not expose error details (no 'detail' field)", async () => { + const { renderPdf } = await import("../services/browser.js"); + const internalError = new Error("Handlebars compilation failed: Unexpected token"); + vi.mocked(renderPdf).mockRejectedValue(internalError); + + const res = await request(app) + .post("/v1/templates/invoice/render") + .set("content-type", "application/json") + .send({ + invoiceNumber: "INV-001", + date: "2026-03-07", + from: { name: "Test Company" }, + to: { name: "Customer" }, + items: [{ description: "Test", quantity: 1, unitPrice: 100 }] + }); + + expect(res.status).toBe(500); + expect(res.body.error).toBe("Template rendering failed"); + expect(res.body).not.toHaveProperty("detail"); + expect(JSON.stringify(res.body)).not.toContain("Handlebars"); + expect(JSON.stringify(res.body)).not.toContain("Unexpected token"); + }); +}); + +describe("Error Response Security - Admin Cleanup", () => { + let app: express.Express; + + beforeEach(async () => { + vi.clearAllMocks(); + vi.resetModules(); + + // Mock auth middlewares + const mockAuthMiddleware = (req: any, res: any, next: any) => next(); + const mockAdminAuth = (req: any, res: any, next: any) => next(); + + // Mock database functions + vi.mock("../services/database.js", () => ({ + cleanupStaleData: vi.fn(), + })); + + const { cleanupStaleData } = await import("../services/database.js"); + vi.mocked(cleanupStaleData).mockResolvedValue({ deletedCount: 5 }); + + // Create minimal app + app = express(); + app.use(express.json()); + + // Mock the cleanup endpoint directly + app.post("/admin/cleanup", mockAuthMiddleware, mockAdminAuth, async (_req: any, res: any) => { + try { + const results = await cleanupStaleData(); + res.json({ status: "ok", cleaned: results }); + } catch (err: any) { + // This should match the fixed behavior + res.status(500).json({ error: "Cleanup failed" }); + } + }); + }); + + it("does not expose error message (no 'message' field)", async () => { + const { cleanupStaleData } = await import("../services/database.js"); + const internalError = new Error("Database connection pool exhausted"); + vi.mocked(cleanupStaleData).mockRejectedValue(internalError); + + const res = await request(app) + .post("/admin/cleanup") + .set("content-type", "application/json") + .send({}); + + expect(res.status).toBe(500); + expect(res.body.error).toBe("Cleanup failed"); + expect(res.body).not.toHaveProperty("message"); + expect(JSON.stringify(res.body)).not.toContain("Database"); + expect(JSON.stringify(res.body)).not.toContain("exhausted"); + }); +}); diff --git a/src/index.ts b/src/index.ts index 898b476..6255d84 100644 --- a/src/index.ts +++ b/src/index.ts @@ -210,7 +210,7 @@ app.post("/admin/cleanup", authMiddleware, adminAuth, async (_req: any, res: any res.json({ status: "ok", cleaned: results }); } catch (err: any) { logger.error({ err }, "Admin cleanup failed"); - res.status(500).json({ error: "Cleanup failed", message: err.message }); + res.status(500).json({ error: "Cleanup failed" }); } }); diff --git a/src/routes/convert.ts b/src/routes/convert.ts index 69a2dc3..7551379 100644 --- a/src/routes/convert.ts +++ b/src/routes/convert.ts @@ -125,10 +125,14 @@ convertRouter.post("/html", async (req: Request & { acquirePdfSlot?: () => Promi } catch (err: any) { logger.error({ err }, "Convert HTML error"); if (err.message === "QUEUE_FULL") { - res.status(429).json({ error: "Server busy - too many concurrent PDF generations. Please try again in a few seconds." }); + res.status(503).json({ error: "Server busy — too many concurrent PDF generations. Please try again in a few seconds." }); return; } - res.status(500).json({ error: `PDF generation failed: ${err.message}` }); + if (err.message === "PDF_TIMEOUT") { + res.status(504).json({ error: "PDF generation timed out." }); + return; + } + res.status(500).json({ error: "PDF generation failed." }); } finally { if (slotAcquired && req.releasePdfSlot) { req.releasePdfSlot(); @@ -227,10 +231,14 @@ convertRouter.post("/markdown", async (req: Request & { acquirePdfSlot?: () => P } catch (err: any) { logger.error({ err }, "Convert MD error"); if (err.message === "QUEUE_FULL") { - res.status(429).json({ error: "Server busy - too many concurrent PDF generations. Please try again in a few seconds." }); + res.status(503).json({ error: "Server busy — too many concurrent PDF generations. Please try again in a few seconds." }); return; } - res.status(500).json({ error: `PDF generation failed: ${err.message}` }); + if (err.message === "PDF_TIMEOUT") { + res.status(504).json({ error: "PDF generation timed out." }); + return; + } + res.status(500).json({ error: "PDF generation failed." }); } finally { if (slotAcquired && req.releasePdfSlot) { req.releasePdfSlot(); @@ -360,10 +368,14 @@ convertRouter.post("/url", async (req: Request & { acquirePdfSlot?: () => Promis } catch (err: any) { logger.error({ err }, "Convert URL error"); if (err.message === "QUEUE_FULL") { - res.status(429).json({ error: "Server busy - too many concurrent PDF generations. Please try again in a few seconds." }); + res.status(503).json({ error: "Server busy — too many concurrent PDF generations. Please try again in a few seconds." }); return; } - res.status(500).json({ error: `PDF generation failed: ${err.message}` }); + if (err.message === "PDF_TIMEOUT") { + res.status(504).json({ error: "PDF generation timed out." }); + return; + } + res.status(500).json({ error: "PDF generation failed." }); } finally { if (slotAcquired && req.releasePdfSlot) { req.releasePdfSlot(); diff --git a/src/routes/templates.ts b/src/routes/templates.ts index 7653ba0..b9c2d40 100644 --- a/src/routes/templates.ts +++ b/src/routes/templates.ts @@ -174,6 +174,6 @@ templatesRouter.post("/:id/render", async (req: Request, res: Response) => { res.send(pdf); } catch (err: any) { logger.error({ err }, "Template render error"); - res.status(500).json({ error: "Template rendering failed", detail: err.message }); + res.status(500).json({ error: "Template rendering failed" }); } });