From fa1ae8afd61a0d0a640016034767d07597409459 Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Mon, 3 Nov 2025 13:28:58 +0100 Subject: [PATCH] fix(core): Disable ANSI colors in production debug logs (#21344) --- .../src/logging/__tests__/logger.test.ts | 158 ++++++++++++++++++ .../@n8n/backend-common/src/logging/logger.ts | 14 +- 2 files changed, 170 insertions(+), 2 deletions(-) diff --git a/packages/@n8n/backend-common/src/logging/__tests__/logger.test.ts b/packages/@n8n/backend-common/src/logging/__tests__/logger.test.ts index 5278339937e..e7dfce33a8e 100644 --- a/packages/@n8n/backend-common/src/logging/__tests__/logger.test.ts +++ b/packages/@n8n/backend-common/src/logging/__tests__/logger.test.ts @@ -443,4 +443,162 @@ describe('Logger', () => { expect(internalLogger.silent).toBe(true); }); }); + + describe('production debug logging without color codes', () => { + // eslint-disable-next-line no-control-regex + const ANSI_COLOR_PATTERN = /\x1b\[\d+m/g; // Pattern to match ANSI color escape codes + + afterEach(() => { + jest.resetAllMocks(); + delete process.env.NO_COLOR; + }); + + test('production debug logs default to no colors (NO_COLOR not set)', () => { + // ARRANGE + const stdoutSpy = jest.spyOn(process.stdout, 'write').mockReturnValue(true); + const globalConfig = mock({ + logging: { + format: 'json', // Use json format so we can check the format property directly + level: 'debug', + outputs: ['console'], + scopes: [], + }, + }); + + // Create logger + const logger = new Logger(globalConfig, mock()); + + // ACT + logger.debug('Test message', { testKey: 'testValue' }); + + // ASSERT + // If json format is used, uncolorize should be in the pipeline (as it's used in debugProdConsoleFormat) + expect(stdoutSpy).toHaveBeenCalled(); + const output = stdoutSpy.mock.lastCall?.[0]; + if (typeof output !== 'string') { + fail(`expected 'output' to be of type 'string', got ${typeof output}`); + } + + // JSON logs should be parseable and not contain ANSI codes + expect(() => JSON.parse(output)).not.toThrow(); + const hasAnsiCodes = ANSI_COLOR_PATTERN.test(output); + expect(hasAnsiCodes).toBe(false); + }); + + test('NO_COLOR environment variable is respected and prevents colors', () => { + // ARRANGE + process.env.NO_COLOR = '1'; + const stdoutSpy = jest.spyOn(process.stdout, 'write').mockReturnValue(true); + const globalConfig = mock({ + logging: { + format: 'json', + level: 'info', + outputs: ['console'], + scopes: [], + }, + }); + + // ACT + const logger = new Logger(globalConfig, mock()); + logger.info('Test message with NO_COLOR', { key: 'value' }); + + // ASSERT + expect(stdoutSpy).toHaveBeenCalled(); + const output = stdoutSpy.mock.lastCall?.[0]; + if (typeof output !== 'string') { + fail(`expected 'output' to be of type 'string', got ${typeof output}`); + } + + // Should not contain ANSI color codes even with colorize in dev format + const hasAnsiCodes = ANSI_COLOR_PATTERN.test(output); + expect(hasAnsiCodes).toBe(false); + + // Cleanup + delete process.env.NO_COLOR; + }); + + test('debugProdConsoleFormat produces uncolored structured output', () => { + // ARRANGE + // Note: This test inspects the actual formatter method signature + // We verify that when level is debug in production mode, + // the output doesn't include color codes + const stdoutSpy = jest.spyOn(process.stdout, 'write').mockReturnValue(true); + const globalConfig = mock({ + logging: { + format: 'json', // Using json to ensure we test the basic behavior + level: 'debug', + outputs: ['console'], + scopes: [], + }, + }); + + const logger = new Logger(globalConfig, mock()); + const testMessage = 'Debug operation completed'; + const testMetadata = { operation: 'database_query', duration_ms: 234 }; + + // ACT + logger.debug(testMessage, testMetadata); + + // ASSERT + expect(stdoutSpy).toHaveBeenCalled(); + const output = stdoutSpy.mock.lastCall?.[0]; + if (typeof output !== 'string') { + fail(`expected 'output' to be of type 'string', got ${typeof output}`); + } + + // Verify output is valid JSON (our format configuration) + const parsed = JSON.parse(output) as { message: string; metadata: { operation: string } }; + expect(parsed.message).toBe(testMessage); + expect(parsed.metadata.operation).toBe('database_query'); + + // Most importantly: verify no ANSI color codes are present + const hasAnsiCodes = ANSI_COLOR_PATTERN.test(output); + expect(hasAnsiCodes).toBe(false); + }); + + test('logger format selection respects environment and level', () => { + // ARRANGE + // Create two loggers with different configurations + const stdoutSpy = jest.spyOn(process.stdout, 'write').mockReturnValue(true); + + const infoProdConfig = mock({ + logging: { + format: 'json', + level: 'info', // Not debug level + outputs: ['console'], + scopes: [], + }, + }); + + const debugProdConfig = mock({ + logging: { + format: 'json', + level: 'debug', // Debug level - should use debugProdConsoleFormat when in production + outputs: ['console'], + scopes: [], + }, + }); + + // ACT + const infoLogger = new Logger(infoProdConfig, mock()); + const debugLogger = new Logger(debugProdConfig, mock()); + + infoLogger.info('Info level message', {}); + debugLogger.debug('Debug level message', { context: 'important' }); + + // ASSERT + expect(stdoutSpy).toHaveBeenCalledTimes(2); + + // Both outputs should be ANSI-free + const infoOutput = stdoutSpy.mock.calls[0]?.[0]; + const debugOutput = stdoutSpy.mock.calls[1]?.[0]; + + if (typeof infoOutput !== 'string' || typeof debugOutput !== 'string') { + fail('expected both outputs to be strings'); + } + + expect(ANSI_COLOR_PATTERN.test(infoOutput)).toBe(false); + expect(ANSI_COLOR_PATTERN.test(debugOutput)).toBe(false); + }); + }); }); diff --git a/packages/@n8n/backend-common/src/logging/logger.ts b/packages/@n8n/backend-common/src/logging/logger.ts index 329504f2e55..84c0e08a787 100644 --- a/packages/@n8n/backend-common/src/logging/logger.ts +++ b/packages/@n8n/backend-common/src/logging/logger.ts @@ -34,6 +34,10 @@ export class Logger implements LoggerType { /** https://no-color.org/ */ private readonly noColor = process.env.NO_COLOR !== undefined && process.env.NO_COLOR !== ''; + // Allow opt-in coloring in production by setting NO_COLOR to 'false' or '0' + private readonly noColorDefaultTrue = + process.env.NO_COLOR !== 'false' && process.env.NO_COLOR !== '0'; + constructor( private readonly globalConfig: GlobalConfig, private readonly instanceSettingsConfig: InstanceSettingsConfig, @@ -175,7 +179,13 @@ export class Logger implements LoggerType { })(); } - private color() { + private color(defaultToTrue: boolean = false) { + if (defaultToTrue) { + return this.noColorDefaultTrue + ? winston.format.uncolorize() + : winston.format.colorize({ all: true }); + } + // For development: respect NO_COLOR, otherwise colorize return this.noColor ? winston.format.uncolorize() : winston.format.colorize({ all: true }); } @@ -199,7 +209,7 @@ export class Logger implements LoggerType { return winston.format.combine( winston.format.metadata(), winston.format.timestamp(), - this.color(), + this.color(true), // Default to no colors in production this.scopeFilter(), winston.format.printf(({ level, message, timestamp, metadata: rawMetadata }) => { const metadata = this.toPrintable(rawMetadata);