From 77d1d14e08b5075537937ea2e3454555ccf04287 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 3 Feb 2026 21:35:15 -0600 Subject: [PATCH] fix(#289): Prevent private key decryption error data leaks Modified decrypt() error handling to only log error type without stack traces, error details, or encrypted content. Added test to verify sensitive data is not exposed in logs. Security improvement: Prevents leakage of encrypted data or partial decryption results through error logs. Fixes #289 Co-Authored-By: Claude Sonnet 4.5 --- .../api/src/federation/crypto.service.spec.ts | 28 ++++++++++++++++++- apps/api/src/federation/crypto.service.ts | 5 +++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/apps/api/src/federation/crypto.service.spec.ts b/apps/api/src/federation/crypto.service.spec.ts index ce0d76b..d1b1d7c 100644 --- a/apps/api/src/federation/crypto.service.spec.ts +++ b/apps/api/src/federation/crypto.service.spec.ts @@ -2,10 +2,11 @@ * Crypto Service Tests */ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, vi } from "vitest"; import { Test, TestingModule } from "@nestjs/testing"; import { ConfigService } from "@nestjs/config"; import { CryptoService } from "./crypto.service"; +import { Logger } from "@nestjs/common"; describe("CryptoService", () => { let service: CryptoService; @@ -137,6 +138,31 @@ describe("CryptoService", () => { // Act & Assert expect(() => service.decrypt(corrupted)).toThrow("Failed to decrypt data"); }); + + it("should not log sensitive data in error messages", () => { + // Arrange + const loggerErrorSpy = vi.spyOn(Logger.prototype, "error"); + const corruptedData = "corrupted:data:here"; + + // Act & Assert + expect(() => service.decrypt(corruptedData)).toThrow("Failed to decrypt data"); + + // Verify logger was called with safe message + expect(loggerErrorSpy).toHaveBeenCalled(); + const logCall = loggerErrorSpy.mock.calls[0]; + + // First argument should contain error type but not sensitive data + expect(logCall[0]).toMatch(/Decryption failed:/); + + // Should NOT log the actual error object with stack traces + expect(logCall.length).toBe(1); // Only one argument (the message) + + // Verify the corrupted data is not in the log + const logMessage = logCall[0] as string; + expect(logMessage).not.toContain(corruptedData); + + loggerErrorSpy.mockRestore(); + }); }); describe("encrypt/decrypt round-trip", () => { diff --git a/apps/api/src/federation/crypto.service.ts b/apps/api/src/federation/crypto.service.ts index 56140d6..8acee35 100644 --- a/apps/api/src/federation/crypto.service.ts +++ b/apps/api/src/federation/crypto.service.ts @@ -90,7 +90,10 @@ export class CryptoService { return decrypted; } catch (error) { - this.logger.error("Decryption failed", error); + // Security: Do not log error details which may contain sensitive data + // Only log error type/code without stack trace or encrypted content + const errorType = error instanceof Error ? error.constructor.name : "Unknown"; + this.logger.error(`Decryption failed: ${errorType}`); throw new Error("Failed to decrypt data"); } }