fix(SEC-API-24): Sanitize error messages in global exception filter

- Add sensitive pattern detection for passwords, API keys, DB errors,
  file paths, IP addresses, and stack traces
- Replace console.error with structured NestJS Logger
- Always sanitize 5xx errors in production
- Sanitize non-HttpException errors in production
- Add comprehensive test coverage (14 tests)

Refs #339

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

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<typeof vi.fn>;
+  let mockStatus: ReturnType<typeof vi.fn>;
+  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",
+        })
+      );
+    });
+  });
+});

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<Response>();
     const request = ctx.getRequest<Request>();
@@ -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
+    // Always log the full error internally
-    const logPayload = {
+    this.logger.error({
-      level: "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:
+      message: clientMessage,
-        isProduction && status === HttpStatus.INTERNAL_SERVER_ERROR
-          ? "An unexpected error occurred"
-          : message,
       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));
+  }
 }