From e288f7fda7adf3499a8bc289f8d09802bbbd6fa6 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 18 Apr 2026 11:14:34 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=90=20fix:=20Redact=20Sensitive=20Toke?= =?UTF-8?q?ns=20Embedded=20in=20JSON=20Metadata?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps in the existing console redaction that became user-visible once warn/error lines started emitting structured metadata: 1. The OpenAI-key regex (`/^(sk-)[^\s]+/`) was anchored to start-of-line, so keys embedded inside JSON payloads (e.g. `{"apiKey":"sk-..."}`) were never redacted. Every console line begins with a timestamp, so the anchor effectively made this pattern dead code. 2. `formatConsoleMeta` stringified metadata values verbatim; a sensitive string value was only redacted by the whole-line regex pass, which missed the anchored `sk-` case above. Fix: - Drop the `^` anchor; add `/g` so every occurrence is redacted, not just the first. - Also exclude `"` and `'` from the token body so JSON-embedded values terminate at the closing quote rather than chewing into the next field. - Simplify `redactMessage` to apply patterns directly (dropping the `getMatchingSensitivePatterns` filter) — the filter used `.test()` which has stateful behavior on `/g` regexes and is no longer needed. - `formatConsoleMeta` now runs `redactMessage` over every string value before JSON serialization, so the metadata trailer is safe even on the warn path. - Add regression tests covering both fixes. Reviewed-by: Codex (P1 finding on PR #12737, commit 68c31b6). --- api/config/__tests__/parsers.spec.js | 55 +++++++++++++++++++++++++++- api/config/parsers.js | 47 +++++++++--------------- 2 files changed, 72 insertions(+), 30 deletions(-) diff --git a/api/config/__tests__/parsers.spec.js b/api/config/__tests__/parsers.spec.js index 41adcdcc9c..217e97dc17 100644 --- a/api/config/__tests__/parsers.spec.js +++ b/api/config/__tests__/parsers.spec.js @@ -1,4 +1,4 @@ -const { formatConsoleMeta } = jest.requireActual('../parsers'); +const { formatConsoleMeta, redactMessage } = jest.requireActual('../parsers'); describe('formatConsoleMeta', () => { it('returns empty string when there is no user metadata', () => { @@ -80,4 +80,57 @@ describe('formatConsoleMeta', () => { expect(meta).toBe(''); }); + + it('redacts sensitive patterns inside string metadata values', () => { + const meta = formatConsoleMeta({ + level: 'error', + message: 'leak test', + timestamp: 'ts', + openaiKey: 'sk-abc123def456', + auth: 'Bearer eyJhbGciOi...tokenvalue', + google: 'https://example.com/?key=AIzaSyXX', + }); + + expect(meta).not.toContain('sk-abc123def456'); + expect(meta).not.toContain('eyJhbGciOi...tokenvalue'); + expect(meta).not.toContain('AIzaSyXX'); + expect(meta).toContain('sk-[REDACTED]'); + expect(meta).toContain('Bearer [REDACTED]'); + expect(meta).toContain('key=[REDACTED]'); + }); + + it('redacts multiple occurrences of the same pattern in one value', () => { + const meta = formatConsoleMeta({ + level: 'error', + message: 'two keys', + timestamp: 'ts', + combined: 'first sk-aaa and then sk-bbb', + }); + + expect(meta).not.toContain('sk-aaa'); + expect(meta).not.toContain('sk-bbb'); + expect(meta.match(/sk-\[REDACTED\]/g)?.length).toBe(2); + }); +}); + +describe('redactMessage', () => { + it('redacts sk- keys that are not at line start (inside JSON-like text)', () => { + const input = '{"apiKey":"sk-abc123"}'; + expect(redactMessage(input)).toBe('{"apiKey":"sk-[REDACTED]"}'); + }); + + it('redacts all sk- occurrences in a single pass', () => { + const input = 'sk-one sk-two sk-three'; + expect(redactMessage(input)).toBe('sk-[REDACTED] sk-[REDACTED] sk-[REDACTED]'); + }); + + it('trims redacted output when trimLength is provided', () => { + const input = 'Bearer supersecretvalue'; + expect(redactMessage(input, 10)).toBe('Bearer [RE...'); + }); + + it('returns empty string for falsy input', () => { + expect(redactMessage('')).toBe(''); + expect(redactMessage(undefined)).toBe(''); + }); }); diff --git a/api/config/parsers.js b/api/config/parsers.js index 997f06977b..27364d0bae 100644 --- a/api/config/parsers.js +++ b/api/config/parsers.js @@ -8,26 +8,12 @@ const CONSOLE_JSON_STRING_LENGTH = parseInt(process.env.CONSOLE_JSON_STRING_LENG const DEBUG_MESSAGE_LENGTH = parseInt(process.env.DEBUG_MESSAGE_LENGTH) || 150; const sensitiveKeys = [ - /^(sk-)[^\s]+/, // OpenAI API key pattern - /(Bearer )[^\s]+/, // Header: Bearer token pattern - /(api-key:? )[^\s]+/, // Header: API key pattern - /(key=)[^\s]+/, // URL query param: sensitive key pattern (Google) + /(sk-)[^\s"']+/g, // OpenAI API key pattern (also catches keys embedded in JSON/quoted strings) + /(Bearer )[^\s"']+/g, // Header: Bearer token pattern + /(api-key:? )[^\s"']+/g, // Header: API key pattern + /(key=)[^\s"']+/g, // URL query param: sensitive key pattern (Google) ]; -/** - * Determines if a given value string is sensitive and returns matching regex patterns. - * - * @param {string} valueStr - The value string to check. - * @returns {Array} An array of regex patterns that match the value string. - */ -function getMatchingSensitivePatterns(valueStr) { - if (valueStr) { - // Filter and return all regex patterns that match the value string - return sensitiveKeys.filter((regex) => regex.test(valueStr)); - } - return []; -} - /** * Redacts sensitive information from a console message and trims it to a specified length if provided. * @param {string} str - The console message to be redacted. @@ -39,16 +25,16 @@ function redactMessage(str, trimLength) { return ''; } - const patterns = getMatchingSensitivePatterns(str); - patterns.forEach((pattern) => { - str = str.replace(pattern, '$1[REDACTED]'); - }); - - if (trimLength !== undefined && str.length > trimLength) { - return `${str.substring(0, trimLength)}...`; + let redacted = str; + for (const pattern of sensitiveKeys) { + redacted = redacted.replace(pattern, '$1[REDACTED]'); } - return str; + if (trimLength !== undefined && redacted.length > trimLength) { + return `${redacted.substring(0, trimLength)}...`; + } + + return redacted; } /** @@ -141,10 +127,13 @@ function formatConsoleMeta(info) { } try { return JSON.stringify(meta, (_key, value) => { - if (typeof value === 'string' && value.length > CONSOLE_JSON_STRING_LENGTH) { - return `${value.substring(0, CONSOLE_JSON_STRING_LENGTH)}...`; + if (typeof value !== 'string') { + return value; } - return value; + const safe = redactMessage(value); + return safe.length > CONSOLE_JSON_STRING_LENGTH + ? `${safe.substring(0, CONSOLE_JSON_STRING_LENGTH)}...` + : safe; }); } catch { return '';