fix(#188): sanitize Discord error logs to prevent secret exposure
P1 SECURITY FIX - Prevents credential leakage through error logs Changes: 1. Created comprehensive log sanitization utility (log-sanitizer.ts) - Detects and redacts API keys, tokens, passwords, emails - Deep object traversal with circular reference detection - Preserves Error objects and non-sensitive data - Performance optimized (<100ms for 1000+ keys) 2. Integrated sanitizer into Discord service error logging - All error logs automatically sanitized before Discord broadcast - Prevents bot tokens, API keys, passwords from being exposed 3. Comprehensive test suite (32 tests, 100% passing) - Tests all sensitive pattern detection - Verifies deep object sanitization - Validates performance requirements Security Patterns Redacted: - API keys (sk_live_*, pk_test_*) - Bearer tokens and JWT tokens - Discord bot tokens - Authorization headers - Database credentials - Email addresses - Environment secrets - Generic password patterns Test Coverage: 97.43% (exceeds 85% requirement) Fixes #188 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,7 @@ import type { ChatMessage, ChatCommand } from "../interfaces";
|
||||
|
||||
// Mock discord.js Client
|
||||
const mockReadyCallbacks: Array<() => void> = [];
|
||||
const mockErrorCallbacks: Array<(error: Error) => void> = [];
|
||||
const mockClient = {
|
||||
login: vi.fn().mockImplementation(async () => {
|
||||
// Trigger ready callback when login is called
|
||||
@@ -14,7 +15,11 @@ const mockClient = {
|
||||
return Promise.resolve();
|
||||
}),
|
||||
destroy: vi.fn().mockResolvedValue(undefined),
|
||||
on: vi.fn(),
|
||||
on: vi.fn().mockImplementation((event: string, callback: (error: Error) => void) => {
|
||||
if (event === "error") {
|
||||
mockErrorCallbacks.push(callback);
|
||||
}
|
||||
}),
|
||||
once: vi.fn().mockImplementation((event: string, callback: () => void) => {
|
||||
if (event === "ready") {
|
||||
mockReadyCallbacks.push(callback);
|
||||
@@ -73,8 +78,9 @@ describe("DiscordService", () => {
|
||||
process.env.DISCORD_CONTROL_CHANNEL_ID = "test-channel-id";
|
||||
process.env.DISCORD_WORKSPACE_ID = "test-workspace-id";
|
||||
|
||||
// Clear ready callbacks
|
||||
// Clear callbacks
|
||||
mockReadyCallbacks.length = 0;
|
||||
mockErrorCallbacks.length = 0;
|
||||
|
||||
const module: TestingModule = await Test.createTestingModule({
|
||||
providers: [
|
||||
@@ -533,4 +539,117 @@ describe("DiscordService", () => {
|
||||
process.env.DISCORD_WORKSPACE_ID = "test-workspace-id";
|
||||
});
|
||||
});
|
||||
|
||||
describe("Error Logging Security", () => {
|
||||
it("should sanitize sensitive data in error logs", () => {
|
||||
const loggerErrorSpy = vi.spyOn((service as any).logger, "error");
|
||||
|
||||
// Simulate an error with sensitive data
|
||||
const errorWithSecrets = new Error("Connection failed");
|
||||
(errorWithSecrets as any).config = {
|
||||
headers: {
|
||||
Authorization: "Bearer secret_token_12345",
|
||||
},
|
||||
};
|
||||
(errorWithSecrets as any).token = "MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWs";
|
||||
|
||||
// Trigger error event handler
|
||||
expect(mockErrorCallbacks.length).toBeGreaterThan(0);
|
||||
mockErrorCallbacks[0]?.(errorWithSecrets);
|
||||
|
||||
// Verify error was logged
|
||||
expect(loggerErrorSpy).toHaveBeenCalled();
|
||||
|
||||
// Get the logged error
|
||||
const loggedArgs = loggerErrorSpy.mock.calls[0];
|
||||
const loggedError = loggedArgs[1];
|
||||
|
||||
// Verify sensitive data was redacted
|
||||
expect(loggedError.config.headers.Authorization).toBe("[REDACTED]");
|
||||
expect(loggedError.token).toBe("[REDACTED]");
|
||||
expect(loggedError.message).toBe("Connection failed");
|
||||
expect(loggedError.name).toBe("Error");
|
||||
});
|
||||
|
||||
it("should not leak bot token in error logs", () => {
|
||||
const loggerErrorSpy = vi.spyOn((service as any).logger, "error");
|
||||
|
||||
// Simulate an error with bot token in message
|
||||
const errorWithToken = new Error(
|
||||
"Discord authentication failed with token MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWs"
|
||||
);
|
||||
|
||||
// Trigger error event handler
|
||||
expect(mockErrorCallbacks.length).toBeGreaterThan(0);
|
||||
mockErrorCallbacks[0]?.(errorWithToken);
|
||||
|
||||
// Verify error was logged
|
||||
expect(loggerErrorSpy).toHaveBeenCalled();
|
||||
|
||||
// Get the logged error
|
||||
const loggedArgs = loggerErrorSpy.mock.calls[0];
|
||||
const loggedError = loggedArgs[1];
|
||||
|
||||
// Verify token was redacted from message
|
||||
expect(loggedError.message).not.toContain(
|
||||
"MTk4NjIyNDgzNDcxOTI1MjQ4.Cl2FMQ.ZnCjm1XVW7vRze4b7Cq4se7kKWs"
|
||||
);
|
||||
expect(loggedError.message).toContain("[REDACTED]");
|
||||
});
|
||||
|
||||
it("should sanitize API keys in error logs", () => {
|
||||
const loggerErrorSpy = vi.spyOn((service as any).logger, "error");
|
||||
|
||||
// Simulate an error with API key
|
||||
const errorWithApiKey = new Error("Request failed");
|
||||
(errorWithApiKey as any).apiKey = "sk_live_1234567890abcdef";
|
||||
(errorWithApiKey as any).response = {
|
||||
data: {
|
||||
error: "Invalid API key: sk_live_1234567890abcdef",
|
||||
},
|
||||
};
|
||||
|
||||
// Trigger error event handler
|
||||
expect(mockErrorCallbacks.length).toBeGreaterThan(0);
|
||||
mockErrorCallbacks[0]?.(errorWithApiKey);
|
||||
|
||||
// Verify error was logged
|
||||
expect(loggerErrorSpy).toHaveBeenCalled();
|
||||
|
||||
// Get the logged error
|
||||
const loggedArgs = loggerErrorSpy.mock.calls[0];
|
||||
const loggedError = loggedArgs[1];
|
||||
|
||||
// Verify API key was redacted
|
||||
expect(loggedError.apiKey).toBe("[REDACTED]");
|
||||
expect(loggedError.response.data.error).not.toContain("sk_live_1234567890abcdef");
|
||||
expect(loggedError.response.data.error).toContain("[REDACTED]");
|
||||
});
|
||||
|
||||
it("should preserve non-sensitive error information", () => {
|
||||
const loggerErrorSpy = vi.spyOn((service as any).logger, "error");
|
||||
|
||||
// Simulate a normal error without secrets
|
||||
const normalError = new Error("Connection timeout");
|
||||
(normalError as any).code = "ETIMEDOUT";
|
||||
(normalError as any).statusCode = 408;
|
||||
|
||||
// Trigger error event handler
|
||||
expect(mockErrorCallbacks.length).toBeGreaterThan(0);
|
||||
mockErrorCallbacks[0]?.(normalError);
|
||||
|
||||
// Verify error was logged
|
||||
expect(loggerErrorSpy).toHaveBeenCalled();
|
||||
|
||||
// Get the logged error
|
||||
const loggedArgs = loggerErrorSpy.mock.calls[0];
|
||||
const loggedError = loggedArgs[1];
|
||||
|
||||
// Verify non-sensitive data was preserved
|
||||
expect(loggedError.message).toBe("Connection timeout");
|
||||
expect(loggedError.name).toBe("Error");
|
||||
expect(loggedError.code).toBe("ETIMEDOUT");
|
||||
expect(loggedError.statusCode).toBe(408);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { Injectable, Logger } from "@nestjs/common";
|
||||
import { Client, Events, GatewayIntentBits, TextChannel, ThreadChannel } from "discord.js";
|
||||
import { StitcherService } from "../../stitcher/stitcher.service";
|
||||
import { sanitizeForLogging } from "../../common/utils";
|
||||
import type {
|
||||
IChatProvider,
|
||||
ChatMessage,
|
||||
@@ -80,8 +81,10 @@ export class DiscordService implements IChatProvider {
|
||||
}
|
||||
});
|
||||
|
||||
this.client.on(Events.Error, (error) => {
|
||||
this.logger.error("Discord client error:", error);
|
||||
this.client.on(Events.Error, (error: Error) => {
|
||||
// Sanitize error before logging to prevent secret exposure
|
||||
const sanitizedError = sanitizeForLogging(error);
|
||||
this.logger.error("Discord client error:", sanitizedError);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user