From 22f86b388991b011b17e5592468095b4edd2d300 Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Thu, 24 Sep 2026 01:56:52 -0400 Subject: [PATCH] feat(logger): pf-4589 support sanitized logs * logger.helpers, log sanitizers for header, url, tokens * logger, integrate default sanitizing, expose optional settings * server.helpers, isBase64Like helper, allow parseUrl to return URL obj --- cspell.config.json | 1 + .../__snapshots__/logger.test.ts.snap | 2 + src/__tests__/logger.helpers.test.ts | 143 ++++++++++++++ src/__tests__/logger.test.ts | 16 ++ src/__tests__/server.helpers.test.ts | 133 ++++++++++++- src/logger.helpers.ts | 175 ++++++++++++++++++ src/logger.ts | 64 ++++--- src/server.helpers.ts | 108 ++++++++--- 8 files changed, 590 insertions(+), 52 deletions(-) create mode 100644 src/__tests__/logger.helpers.test.ts create mode 100644 src/logger.helpers.ts diff --git a/cspell.config.json b/cspell.config.json index 9c3c6072..f04e8ace 100644 --- a/cspell.config.json +++ b/cspell.config.json @@ -2,6 +2,7 @@ "language": "en", "words": [ "amet", + "apikey", "codemods", "containerfile", "deprioritized", diff --git a/src/__tests__/__snapshots__/logger.test.ts.snap b/src/__tests__/__snapshots__/logger.test.ts.snap index 974b24d3..298444d4 100644 --- a/src/__tests__/__snapshots__/logger.test.ts.snap +++ b/src/__tests__/__snapshots__/logger.test.ts.snap @@ -43,6 +43,8 @@ exports[`formatLogEvent should return a formatted log event, partial 1`] = `"[DE exports[`formatLogEvent should return a formatted log event, undefined 1`] = `"[INFO]:"`; +exports[`formatLogEvent should support sanitizing messages: sanitized 1`] = `"[INFO]: Authorization: Bearer [REDACTED] :https://patternfly.org/?lorem=ipsum&private_token=%5BREDACTED%5D"`; + exports[`formatUnknownError should attempt to return a formatted error on non-errors, bigint 1`] = `"9007199254740991n"`; exports[`formatUnknownError should attempt to return a formatted error on non-errors, boolean 1`] = `"Non-Error thrown: true"`; diff --git a/src/__tests__/logger.helpers.test.ts b/src/__tests__/logger.helpers.test.ts new file mode 100644 index 00000000..aa4d2ac4 --- /dev/null +++ b/src/__tests__/logger.helpers.test.ts @@ -0,0 +1,143 @@ +import { + sanitizeHeaderContent, + sanitizeMessage, + sanitizeTokenContent, + sanitizeUrlContent +} from '../logger.helpers'; + +describe('sanitizeHeaderContent', () => { + it.each([ + { + description: 'authorization bearer token', + input: 'Authorization: Bearer abc123', + expected: 'Authorization: Bearer [REDACTED]' + }, + { + description: 'private token header', + input: 'Private-Token: abc123', + expected: 'Private-Token: [REDACTED]' + }, + { + description: 'non-string input', + input: undefined, + expected: undefined + }, + { + description: 'custom redacted marker', + input: 'Authorization: Bearer abc123', + options: { redacted: '' }, + expected: 'Authorization: Bearer ' + } + ])('should sanitize header content, $description', ({ input, options, expected }) => { + expect(sanitizeHeaderContent(input, options as any)).toBe(expected); + }); +}); + +describe('sanitizeUrlContent', () => { + it.each([ + { + description: 'redact secret query param in absolute URL', + input: 'https://example.com/docs?private_token=abc123&page=1', + expected: 'https://example.com/docs?private_token=%5BREDACTED%5D&page=1' + }, + { + description: 'redact basic auth credentials', + input: 'https://user:pass@example.com/docs', + expected: 'https://%5BREDACTED%5D:%5BREDACTED%5D@example.com/docs' + }, + { + description: 'leave non-url content unchanged', + input: 'not a url', + expected: 'not a url' + }, + { + description: 'sanitize url embedded in text', + input: 'Fetch https://example.com/api?access_token=xyz now', + expected: 'Fetch https://example.com/api?access_token=%5BREDACTED%5D now' + }, + { + description: 'non-string input', + input: null, + expected: undefined + }, + { + description: 'custom redacted marker', + input: 'https://example.com/docs?token=abc', + options: { redacted: '' }, + expected: 'https://example.com/docs?token=%3CMASKED%3E' + } + ])('should sanitize url content, $description', ({ input, options, expected }) => { + expect(sanitizeUrlContent(input as any, options as any)).toBe(expected); + }); +}); + +describe('sanitizeTokenContent', () => { + it.each([ + { + description: 'redact sha-like token', + input: 'token 0123456789abcdef0123456789abcdef', + expected: 'token [REDACTED]' + }, + { + description: 'redact base64-like token', + input: 'token QUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVo=', + expected: 'token [REDACTED]' + }, + { + description: 'keep normal sentence intact', + input: 'this is normal text', + expected: 'this is normal text' + }, + { + description: 'custom min length avoids short token redaction', + input: 'token abcdef12', + options: { minLength: 64 }, + expected: 'token abcdef12' + }, + { + description: 'custom redacted marker', + input: 'token 0123456789abcdef0123456789abcdef', + options: { redacted: '' }, + expected: 'token ' + }, + { + description: 'non-string input', + input: { token: 'abc' }, + expected: undefined + }, + { + description: 'does not redact natural language, "adipiscing" false positive', + input: 'consectetur adipiscing elit', + expected: 'consectetur adipiscing elit' + }, + { + description: 'does not redact natural language in a serialized object', + input: '{"lorem":"ipsum dolor sit amet","dolor":"sit amet","amet":"consectetur adipiscing elit"}', + expected: '{"lorem":"ipsum dolor sit amet","dolor":"sit amet","amet":"consectetur adipiscing elit"}' + } + ])('should sanitize token content, $description', ({ input, options, expected }) => { + expect(sanitizeTokenContent(input as any, options as any)).toBe(expected); + }); +}); + +describe('sanitizeMessage', () => { + it.each([ + { + description: 'sanitize bearer token and url query token in one message', + input: 'Authorization: Bearer abc123 https://example.com/api?access_token=xyz', + expected: 'Authorization: Bearer [REDACTED] https://example.com/api?access_token=%5BREDACTED%5D' + }, + { + description: 'sanitize secret in Error message', + input: new Error('Private-Token: abc123'), + expected: 'Error: Private-Token: [REDACTED]' + }, + { + description: 'non-string serializable object', + input: { auth: 'Authorization: Bearer abc123' }, + expected: '{"auth":"Authorization: Bearer [REDACTED]"}' + } + ])('should sanitize message content, $description', ({ input, expected }) => { + expect(sanitizeMessage(input)).toContain(expected); + }); +}); diff --git a/src/__tests__/logger.test.ts b/src/__tests__/logger.test.ts index ea31a491..cd0aebfb 100644 --- a/src/__tests__/logger.test.ts +++ b/src/__tests__/logger.test.ts @@ -111,6 +111,12 @@ describe('formatUnknownError', () => { ])('should attempt to return a formatted error on non-errors, $description', ({ err }) => { expect(formatUnknownError(err)).toMatchSnapshot(); }); + + it('should support sanitizing messages', () => { + const input = 'Authorization: Bearer abc123'; + + expect(formatUnknownError(input)).toBe('Authorization: Bearer [REDACTED]'); + }); }); describe('formatLogEvent', () => { @@ -152,6 +158,16 @@ describe('formatLogEvent', () => { ])('should return a formatted log event, $description', ({ event }) => { expect(formatLogEvent(event as any)).toMatchSnapshot(); }); + + it('should support sanitizing messages', () => { + const event = { + level: 'info', + msg: 'Authorization: Bearer abc123', + args: ['https://patternfly.org?lorem=ipsum&private_token=dolor'] + }; + + expect(formatLogEvent(event as any)).toMatchSnapshot('sanitized'); + }); }); describe('publish', () => { diff --git a/src/__tests__/server.helpers.test.ts b/src/__tests__/server.helpers.test.ts index 1ba8a46f..4741fb40 100644 --- a/src/__tests__/server.helpers.test.ts +++ b/src/__tests__/server.helpers.test.ts @@ -20,7 +20,7 @@ import { portValid, splitUri, stringJoin, - timeoutFunction + timeoutFunction, isBase64Like } from '../server.helpers'; describe('buildSearchString', () => { @@ -709,6 +709,137 @@ describe('isReferenceLike', () => { }); }); +describe('isBase64Like', () => { + it.each([ + { + description: 'default strict accepts valid padded base64', + value: 'YWJjZA==', + expected: true + }, + { + description: 'default strict rejects missing padding', + value: 'YWJjZA', + expected: false + }, + { + description: 'loose valid unpadded base64, match modulo-4', + value: 'TWFu', // "Man" + options: { isStrict: false }, + expected: true + }, + { + description: 'loose valid unpadded base64, requireSignalChars rejects alpha-only base64', + value: 'TWFu', + options: { isStrict: false, requireSignalChars: true }, + expected: false + }, + { + description: 'loose valid padded base64 with ==', + value: 'YWJjZA==', // "abcd" + options: { isStrict: false }, + expected: true + }, + { + description: 'loose missing trailing padding still accepted', + value: 'YWJjZA', // decodes/re-encodes to YWJjZA== + options: { isStrict: false }, + expected: true + }, + { + description: 'loose trims leading/trailing whitespace', + value: ' YWJjZA== ', + options: { isStrict: false }, + expected: true + }, + { + description: 'loose invalid alphabet character', + value: 'YWJjZA*=', + options: { isStrict: false }, + expected: false + }, + { + description: 'loose invalid padding shape', + value: 'abcde=', + options: { isStrict: false }, + expected: false + }, + { + description: 'loose too short for configured minLength', + value: 'TWFu', + options: { isStrict: false, minLength: 8 }, + expected: false + }, + { + description: 'strict valid 4-char unpadded base64, potential false positive', + value: 'TWFu', + options: { isStrict: true }, + expected: true + }, + { + description: 'strict valid 4-char unpadded base64, potential false positive, requireSignalChars', + value: 'TWFu', + options: { isStrict: true, requireSignalChars: true }, + expected: false + }, + { + description: 'strict valid padded base64 with ==', + value: 'YWJjZA==', + options: { isStrict: true }, + expected: true + }, + { + description: 'strict valid padded base64 with ==, requireSignalChars', + value: 'YWJjZA==', + options: { isStrict: true, requireSignalChars: true }, + expected: true + }, + { + description: 'strict fails modulo-4 length check when padding is missing', + value: 'YWJjZA', // length 6 + options: { isStrict: true }, + expected: false + }, + { + description: 'strict invalid alphabet character', + value: 'YWJjZA*=', + options: { isStrict: true }, + expected: false + }, + { + description: 'strict too short for configured minLength', + value: 'TWFu', + options: { isStrict: true, minLength: 8 }, + expected: false + }, + { + description: 'guard loose non-string number', + value: 1234, + options: { isStrict: false }, + expected: false + }, + { + description: 'guard strict non-string null', + value: null, + options: { isStrict: true }, + expected: false + }, + { + description: 'strict behavior unchanged when requireSignalChars is false', + value: 'QUJDREVG', + options: { isStrict: true, requireSignalChars: false }, + expected: true + }, + { + description: 'strict behavior when requireSignalChars is true', + value: 'QUJDREVG', + options: { isStrict: true, requireSignalChars: true }, + expected: false + } + ])('check if value is base64-like, $description', ({ value, options = {}, expected }) => { + expect(isBase64Like(value, { minLength: 4, ...options })).toBe(expected); + }); +}); + describe('isShaHexLike', () => { it.each([ { diff --git a/src/logger.helpers.ts b/src/logger.helpers.ts new file mode 100644 index 00000000..406dfa12 --- /dev/null +++ b/src/logger.helpers.ts @@ -0,0 +1,175 @@ +import { isBase64Like, isShaHexLike, parseUrl } from './server.helpers'; + +/** + * Secret search string replacement. + */ +const REDACTED = '[REDACTED]'; + +/** + * Match potential secret-like param names. + */ +const SECRET_PARAM_REGEXP = /(access[_-]?token|private[_-]?token|token|auth|authorization|api[_-]?key|apikey|key)/i; + +/** + * Match "Bearer" authorization header facets. + */ +const BEARER_HEADER_REGEXP = /(authorization\s*:\s*)(bearer\s+)([^\s"'}]+)(\s*)/gi; + +/** + * Match for private token assignments. + */ +const PRIVATE_TOKEN_REGEXP = /(private[_-]?token\s*[:=]\s*)([^\s&]+)/gi; + +/** + * Sanitize header-like content. + * + * @param input - String to sanitize. + * @param [options] - Configurable options. + * @param [options.redacted] - Redact default value. + * @param [options.bearerHeaderRegExp] - Bearer header regex. + * @param [options.privateTokenRegExp] - Private token regex. + */ +const sanitizeHeaderContent = (input?: unknown, { + redacted = REDACTED, + bearerHeaderRegExp = BEARER_HEADER_REGEXP, + privateTokenRegExp = PRIVATE_TOKEN_REGEXP +}: { redacted?: string; bearerHeaderRegExp?: RegExp; privateTokenRegExp?: RegExp } = {}) => { + if (typeof input !== 'string') { + return undefined; + } + + const withBearerRedaction = input.replace( + bearerHeaderRegExp, + (_m, g1, g2, _g3, suffix) => `${g1}${g2}${redacted}${suffix}` + ); + + return withBearerRedaction.replace(privateTokenRegExp, (_m, g1) => `${g1}${redacted}`); +}; + +/** + * Sanitize a URL string. + * + * @param input - String to sanitize. + * @param [options] - Configurable options. + * @param [options.redacted] - Redact default value. + * @param [options.secretParamRegExp] - Secret param regex. + */ +const sanitizeUrlContent = ( + input?: unknown, + { redacted = REDACTED, secretParamRegExp = SECRET_PARAM_REGEXP }: { redacted?: string; secretParamRegExp?: RegExp } = {} +) => { + if (typeof input !== 'string') { + return undefined; + } + + const sanitize = (content: string) => { + const parsed = parseUrl(content, { asUrlObject: true }) as URL; + + if (!parsed) { + return content; + } + + if (parsed.username) { + parsed.username = redacted; + } + if (parsed.password) { + parsed.password = redacted; + } + + for (const [paramName] of parsed.searchParams) { + const isSecretParam = secretParamRegExp.test(paramName); + + if (isSecretParam) { + parsed.searchParams.set(paramName, redacted); + } + } + + return parsed.toString(); + }; + + return input.replace(/https?:\/\/[^\s)]+/gi, (urlCandidate: string) => + sanitize(urlCandidate) || urlCandidate); +}; + +/** + * Scrub long token-like substrings (hex/base64-like) using helpers. + * This is a conservative pass that avoids changing normal text. + * + * @param input - String to sanitize. + * @param [options] - Configurable options. + * @param [options.minLength] - Min length of token-like substrings to redact. + * @param [options.redacted] - Redact default value. + */ +const sanitizeTokenContent = ( + input: unknown, + { minLength = 8, redacted = REDACTED }: { minLength?: number; redacted?: string } = {} +) => { + if (typeof input !== 'string') { + return undefined; + } + + const placeholder = '__REDACTED_PLACEHOLDER__'; + const maskedInput = input.replaceAll(redacted, placeholder); + const parts = maskedInput.split(/([\s"'`()<>{}[\],;:]+)/g); + + for (let index = 0; index < parts.length; index += 1) { + const segment = parts[index]; + + if (!segment || segment.includes(placeholder) || segment.includes(redacted)) { + continue; + } + + if (isShaHexLike(segment, { minLength }) || isBase64Like(segment, { minLength, requireSignalChars: true })) { + parts[index] = redacted; + } + } + + return parts.join('').replaceAll(placeholder, redacted); +}; + +/** + * Sanitize arbitrary text for logs by redacting: + * - Authorization/Private-Token header patterns + * - Secrets embedded in URLs (query params, basic auth) via sanitizeUrl + * - Long token-like substrings (hex/base64-like) + * + * @param input + */ +const sanitizeMessage = ( + input: unknown +): string => { + let content: string; + + if (typeof input === 'string') { + content = input; + } else if (input instanceof Error) { + if (input.stack) { + content = input.stack; + } else if (input.message) { + content = input.message; + } else { + content = String(input); + } + } else { + try { + content = JSON.stringify(input); + } catch { + content = String(input); + } + } + // Sweep common header styles + content = sanitizeHeaderContent(content) || content; + + // Sweep inside URLs + content = sanitizeUrlContent(content) || content; + + // Sweep for token-like strings. + return sanitizeTokenContent(content) || content; +}; + +export { + sanitizeHeaderContent, + sanitizeMessage, + sanitizeUrlContent, + sanitizeTokenContent +}; diff --git a/src/logger.ts b/src/logger.ts index 3f73feb5..b7291e27 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -2,6 +2,7 @@ import { channel, unsubscribe, subscribe } from 'node:diagnostics_channel'; import { inspect } from 'node:util'; import { type LoggingSession } from './options.defaults'; import { getLoggerOptions } from './options.context'; +import { sanitizeMessage } from './logger.helpers'; type LogLevel = LoggingSession['level']; @@ -82,53 +83,68 @@ const truncate = (str: string, { max = 250, suffix = '...[truncated]' }: { max?: * Format an unknown value as a string, for logging. * * @param value + * @param [options] - Configurable options + * @param [options.sanitize] - Sanitize the output string, defaults to `true`. * @returns Formatted string */ -const formatUnknownError = (value: unknown): string => { - if (value instanceof Error) { - const message = value.stack || value.message; +const formatUnknownError = (value: unknown, { sanitize = true }: { sanitize?: boolean } = {}): string => { + const getMessage = () => { + if (value instanceof Error) { + const message = value.stack || value.message; - if (message) { - return message; - } + if (message) { + return message; + } - try { - return String(value); - } catch { - return Object.prototype.toString.call(value); + try { + return String(value); + } catch { + return Object.prototype.toString.call(value); + } } - } - if (typeof value === 'string') { - return value; - } + if (typeof value === 'string') { + return value; + } - try { - return `Non-Error thrown: ${truncate(JSON.stringify(value))}`; - } catch { try { - return truncate(inspect(value, { depth: 3, maxArrayLength: 50, breakLength: 120 })); + return `Non-Error thrown: ${truncate(JSON.stringify(value))}`; } catch { - return Object.prototype.toString.call(value); + try { + return truncate(inspect(value, { + depth: 3, + maxArrayLength: 50, + breakLength: 120 + })); + } catch { + return Object.prototype.toString.call(value); + } } - } + }; + + return sanitize ? sanitizeMessage(getMessage()) : getMessage(); }; /** * Format a structured log event for output to stderr. * * @param event - Log event to format + * @param [options] - Configurable options + * @param [options.sanitize] - Sanitize the output string, defaults to `true`. */ -const formatLogEvent = (event: LogEvent) => { +const formatLogEvent = (event: LogEvent, { sanitize = true }: { sanitize?: boolean } = {}) => { + const sanitizeInput = (input: string) => (sanitize ? sanitizeMessage(input) : input); const level = event?.level?.toUpperCase() || 'INFO'; const eventLevel = `[${level}]`; - const message = event?.msg || ''; + const message = sanitizeInput(event?.msg || ''); const rest = event?.args?.map(arg => { try { - return typeof arg === 'string' ? arg : JSON.stringify(arg); + const updatedArg = typeof arg === 'string' ? arg : JSON.stringify(arg); + + return sanitizeInput(updatedArg); } catch { - return String(arg); + return sanitizeInput(String(arg)); } }).join(' ') || ''; diff --git a/src/server.helpers.ts b/src/server.helpers.ts index 80a3d678..1ebaee77 100644 --- a/src/server.helpers.ts +++ b/src/server.helpers.ts @@ -417,6 +417,49 @@ const generateHash = (anyValue: unknown, { isLowercase = false }: { isLowercase? return hashCode(isLowercase ? stringify.toLowerCase() : stringify); }; +/** + * Check if a value is a Base64-like string. + * + * @param value - Value to check. + * @param [options] - Options. + * @param [options.isStrict] - Enforce strict Base64 validation. Defaults to `true`. + * @param [options.minLength] - Minimum length of the Base64 string. + * @param [options.requireSignalChars] - Require characters potentially unique to Base64. Defaults to `false`. + * @returns `true` if the value is a Base64-like string + */ +const isBase64Like = (value: unknown, { + isStrict = true, + minLength = 8, + requireSignalChars = false +}: { isStrict?: boolean; minLength?: number, requireSignalChars?: boolean } = {}) => { + const updatedValue = typeof value === 'string' ? value.trim() : ''; + + if (!updatedValue || updatedValue.length < minLength || (isStrict && updatedValue.length % 4 !== 0)) { + return false; + } + + const looseBase64Regex = /^[A-Za-z0-9+/]+={0,2}$/; + const strictBase64Regex = /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/; + const base64Regex = isStrict ? strictBase64Regex : looseBase64Regex; + + if (!base64Regex.test(updatedValue)) { + return false; + } + + if (requireSignalChars && /^[A-Za-z]+$/.test(updatedValue)) { + return false; + } + + const buff = Buffer.from(updatedValue, 'base64'); + const recoded = buff.toString('base64'); + + if (isStrict) { + return updatedValue === recoded; + } + + return updatedValue === recoded || updatedValue === recoded.replace(/=+$/, ''); +}; + /** * Check if a value is an SHA-1 hex string. * @@ -543,9 +586,18 @@ const listIncrementalCombinations = (values: string[]): string[][] => * This will match against the provided URI. If the URI does not start with the prefix, `undefined` is returned. * @param [options.normalizeSearchParamKeys=true] - If `true`, search param keys are normalized to lowercase. Default: `true` * @param [options.isStrict] - If `true`, only strict URL and path validation is performed. Default: `true` + * @param [options.asUrlObject] - If `true`, the parsed URL is returned as a URL object. Default: `false` * @returns Parsed URI, or `undefined` if parsing fails. */ -const parseUrl = (url: string, { prefix, normalizeSearchParamKeys = true, isStrict = true }: { prefix?: string, normalizeSearchParamKeys?: boolean, isStrict?: boolean } = {}) => { +const parseUrl = ( + url: string, + { + prefix, + normalizeSearchParamKeys = true, + isStrict = true, + asUrlObject = false + }: { prefix?: string, normalizeSearchParamKeys?: boolean, isStrict?: boolean, asUrlObject?: boolean } = {} +) => { const isPrefix = typeof prefix === 'string' && prefix.length > 0 && !prefix.includes(':') && !prefix.includes('/'); const opts = isPrefix ? { allowedProtocols: [prefix] } : {}; const isUri = isUrl(url, { ...opts, isStrict }); @@ -561,37 +613,38 @@ const parseUrl = (url: string, { prefix, normalizeSearchParamKeys = true, isStri return Object.fromEntries(searchParams); }; - if (isUri) { - try { - const updatedUrl = new URL(url); - - return { - protocol: updatedUrl.protocol, - hostname: updatedUrl.hostname, - path: updatedUrl.pathname.replace(/^\//, ''), - params: normalizeParamKeys(updatedUrl.searchParams) - }; - } catch { - return undefined; + const buildUrl = (): URL | undefined => { + if (isUri) { + return new URL(url); } - } - if (isPrefix && isPath(url, { isStrict })) { - try { - const updatedUrl = new URL(`${prefix}://${url}`); - - return { - protocol: updatedUrl.protocol, - hostname: updatedUrl.hostname, - path: updatedUrl.pathname.replace(/^\//, ''), - params: normalizeParamKeys(updatedUrl.searchParams) - }; - } catch { + if (isPrefix && isPath(url, { isStrict })) { + return new URL(`${prefix}://${url}`); + } + + return undefined; + }; + + try { + const updatedUrl = buildUrl(); + + if (!updatedUrl) { return undefined; } - } - return undefined; + if (asUrlObject) { + return updatedUrl; + } + + return { + protocol: updatedUrl.protocol, + hostname: updatedUrl.hostname, + path: updatedUrl.pathname.replace(/^\//, ''), + params: normalizeParamKeys(updatedUrl.searchParams) + }; + } catch { + return undefined; + } }; /** @@ -821,6 +874,7 @@ export { hashCode, hashNormalizeValue, isAsync, + isBase64Like, isObject, isPath, isPlainObject,