diff --git a/apps/api/src/filters/global-exception.filter.spec.ts b/apps/api/src/filters/global-exception.filter.spec.ts new file mode 100644 index 0000000..09f6492 --- /dev/null +++ b/apps/api/src/filters/global-exception.filter.spec.ts @@ -0,0 +1,237 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { HttpException, HttpStatus } from "@nestjs/common"; +import { GlobalExceptionFilter } from "./global-exception.filter"; +import type { ArgumentsHost } from "@nestjs/common"; + +describe("GlobalExceptionFilter", () => { + let filter: GlobalExceptionFilter; + let mockJson: ReturnType; + let mockStatus: ReturnType; + let mockHost: ArgumentsHost; + + beforeEach(() => { + filter = new GlobalExceptionFilter(); + mockJson = vi.fn(); + mockStatus = vi.fn().mockReturnValue({ json: mockJson }); + + mockHost = { + switchToHttp: vi.fn().mockReturnValue({ + getResponse: vi.fn().mockReturnValue({ + status: mockStatus, + }), + getRequest: vi.fn().mockReturnValue({ + method: "GET", + url: "/test", + }), + }), + } as unknown as ArgumentsHost; + }); + + describe("HttpException handling", () => { + it("should return HttpException message for client errors", () => { + const exception = new HttpException("Not Found", HttpStatus.NOT_FOUND); + + filter.catch(exception, mockHost); + + expect(mockStatus).toHaveBeenCalledWith(404); + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + success: false, + message: "Not Found", + statusCode: 404, + }) + ); + }); + + it("should return generic message for 500 errors in production", () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = "production"; + + const exception = new HttpException( + "Internal Server Error", + HttpStatus.INTERNAL_SERVER_ERROR + ); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "An unexpected error occurred", + statusCode: 500, + }) + ); + + process.env.NODE_ENV = originalEnv; + }); + }); + + describe("Error handling", () => { + it("should return generic message for non-HttpException in production", () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = "production"; + + const exception = new Error("Database connection failed"); + + filter.catch(exception, mockHost); + + expect(mockStatus).toHaveBeenCalledWith(500); + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "An unexpected error occurred", + }) + ); + + process.env.NODE_ENV = originalEnv; + }); + + it("should return error message in development", () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = "development"; + + const exception = new Error("Test error message"); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "Test error message", + }) + ); + + process.env.NODE_ENV = originalEnv; + }); + }); + + describe("Sensitive information redaction", () => { + it("should redact messages containing password", () => { + const exception = new HttpException("Invalid password format", HttpStatus.BAD_REQUEST); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "An unexpected error occurred", + }) + ); + }); + + it("should redact messages containing API key", () => { + const exception = new HttpException("Invalid api_key provided", HttpStatus.UNAUTHORIZED); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "An unexpected error occurred", + }) + ); + }); + + it("should redact messages containing database errors", () => { + const exception = new HttpException( + "Database error: connection refused", + HttpStatus.BAD_REQUEST + ); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "An unexpected error occurred", + }) + ); + }); + + it("should redact messages containing file paths", () => { + const exception = new HttpException( + "File not found at /home/user/data", + HttpStatus.NOT_FOUND + ); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "An unexpected error occurred", + }) + ); + }); + + it("should redact messages containing IP addresses", () => { + const exception = new HttpException( + "Failed to connect to 192.168.1.1", + HttpStatus.BAD_REQUEST + ); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "An unexpected error occurred", + }) + ); + }); + + it("should redact messages containing Prisma errors", () => { + const exception = new HttpException("Prisma query failed", HttpStatus.INTERNAL_SERVER_ERROR); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "An unexpected error occurred", + }) + ); + }); + + it("should allow safe error messages", () => { + const exception = new HttpException("Resource not found", HttpStatus.NOT_FOUND); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + message: "Resource not found", + }) + ); + }); + }); + + describe("Response structure", () => { + it("should include errorId in response", () => { + const exception = new HttpException("Test error", HttpStatus.BAD_REQUEST); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + errorId: expect.stringMatching(/^[0-9a-f-]{36}$/), + }) + ); + }); + + it("should include timestamp in response", () => { + const exception = new HttpException("Test error", HttpStatus.BAD_REQUEST); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + timestamp: expect.stringMatching(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/), + }) + ); + }); + + it("should include path in response", () => { + const exception = new HttpException("Test error", HttpStatus.BAD_REQUEST); + + filter.catch(exception, mockHost); + + expect(mockJson).toHaveBeenCalledWith( + expect.objectContaining({ + path: "/test", + }) + ); + }); + }); +}); diff --git a/apps/api/src/filters/global-exception.filter.ts b/apps/api/src/filters/global-exception.filter.ts index e1ae17d..0a1c351 100644 --- a/apps/api/src/filters/global-exception.filter.ts +++ b/apps/api/src/filters/global-exception.filter.ts @@ -1,4 +1,11 @@ -import { ExceptionFilter, Catch, ArgumentsHost, HttpException, HttpStatus } from "@nestjs/common"; +import { + ExceptionFilter, + Catch, + ArgumentsHost, + HttpException, + HttpStatus, + Logger, +} from "@nestjs/common"; import type { Request, Response } from "express"; import { randomUUID } from "crypto"; @@ -11,9 +18,36 @@ interface ErrorResponse { statusCode: number; } +/** + * Patterns that indicate potentially sensitive information in error messages + */ +const SENSITIVE_PATTERNS = [ + /password/i, + /secret/i, + /api[_-]?key/i, + /token/i, + /credential/i, + /connection.*string/i, + /database.*error/i, + /sql.*error/i, + /prisma/i, + /postgres/i, + /mysql/i, + /redis/i, + /mongodb/i, + /stack.*trace/i, + /at\s+\S+\s+\(/i, // Stack trace pattern + /\/home\//i, // File paths + /\/var\//i, + /\/usr\//i, + /\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}/, // IP addresses +]; + @Catch() export class GlobalExceptionFilter implements ExceptionFilter { - catch(exception: unknown, host: ArgumentsHost) { + private readonly logger = new Logger(GlobalExceptionFilter.name); + + catch(exception: unknown, host: ArgumentsHost): void { const ctx = host.switchToHttp(); const response = ctx.getResponse(); const request = ctx.getRequest(); @@ -23,9 +57,11 @@ export class GlobalExceptionFilter implements ExceptionFilter { let status = HttpStatus.INTERNAL_SERVER_ERROR; let message = "An unexpected error occurred"; + let isHttpException = false; if (exception instanceof HttpException) { status = exception.getStatus(); + isHttpException = true; const exceptionResponse = exception.getResponse(); message = typeof exceptionResponse === "string" @@ -37,27 +73,22 @@ export class GlobalExceptionFilter implements ExceptionFilter { const isProduction = process.env.NODE_ENV === "production"; - // Structured error logging - const logPayload = { - level: "error", + // Always log the full error internally + this.logger.error({ errorId, - timestamp, method: request.method, url: request.url, statusCode: status, message: exception instanceof Error ? exception.message : String(exception), - stack: !isProduction && exception instanceof Error ? exception.stack : undefined, - }; + stack: exception instanceof Error ? exception.stack : undefined, + }); - console.error(isProduction ? JSON.stringify(logPayload) : logPayload); + // Determine the safe message for client response + const clientMessage = this.getSafeClientMessage(message, status, isProduction, isHttpException); - // Sanitized client response const errorResponse: ErrorResponse = { success: false, - message: - isProduction && status === HttpStatus.INTERNAL_SERVER_ERROR - ? "An unexpected error occurred" - : message, + message: clientMessage, errorId, timestamp, path: request.url, @@ -66,4 +97,45 @@ export class GlobalExceptionFilter implements ExceptionFilter { response.status(status).json(errorResponse); } + + /** + * Get a sanitized error message safe for client response + * - In production, always sanitize 5xx errors + * - Check for sensitive patterns and redact if found + * - HttpExceptions are generally safe (intentionally thrown) + */ + private getSafeClientMessage( + message: string, + status: number, + isProduction: boolean, + isHttpException: boolean + ): string { + const genericMessage = "An unexpected error occurred"; + + // Always sanitize 5xx errors in production (server-side errors) + if (isProduction && status >= 500) { + return genericMessage; + } + + // For non-HttpExceptions, always sanitize in production + // (these are unexpected errors that might leak internals) + if (isProduction && !isHttpException) { + return genericMessage; + } + + // Check for sensitive patterns + if (this.containsSensitiveInfo(message)) { + this.logger.warn(`Redacted potentially sensitive error message (errorId in logs)`); + return genericMessage; + } + + return message; + } + + /** + * Check if a message contains potentially sensitive information + */ + private containsSensitiveInfo(message: string): boolean { + return SENSITIVE_PATTERNS.some((pattern) => pattern.test(message)); + } }