From c04bddd304131b7c2cc8f31e772c28bcf89087a0 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 17 Jun 2026 12:31:32 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=B5=20refactor:=20Bound=20Log=20Traver?= =?UTF-8?q?sal=20And=20Remove=20Legacy=20api/config=20Logger=20(#13813)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ๐Ÿ›ก๏ธ fix: Bound object-traverse against DAG fan-out and shared refs Detect cycles via the ancestor chain (so shared, non-circular references in sibling branches / DAGs are traversed correctly) and add defensive maxNodes (100k) / maxDepth (100) caps. The removed global visited set was implicitly bounding work at O(distinct nodes); ancestor-chain-only detection is O(root-to-node paths), exponential on DAGs (a depth-24 diamond went from 26 to 50M visits / 1.6s of synchronous work). The caps bound it to ~9ms while leaving normal traversal untouched. Adds a spec covering shared refs, cycles, DAGs, and both bounds. The lone consumer, debugTraverse, inherits the defaults with no change. * ๐Ÿชต refactor: Remove legacy api/config logger duplicate The api/config winston logger was a stale parallel implementation of the canonical @librechat/data-schemas logger, with unbounded redaction (regex-only redactFormat, npm traverse-based debugTraverse). Its winston instance and the logger export from api/config/index.js had zero consumers โ€” every ~/config importer uses the MCP/flow-manager exports. The only live tie was ToolService's use of redactMessage. Re-export redactMessage from @librechat/data-schemas (behaviorally identical, a superset of the regex set), point ToolService at it, delete api/config/winston.js and api/config/parsers.js, drop the dead logger export, and remove the orphaned ~/config/parsers mock from the global test setup. * ๐Ÿงน chore: Drop orphaned traverse dep and stale legacy logger tests Deleting api/config/{winston,parsers}.js left the npm 'traverse' package unused in api/package.json (flagged by the detect-unused-packages CI check) and orphaned two tests that imported the deleted modules. Remove the traverse dependency (sync package-lock), and delete api/config/__tests__/{parsers,logToFile}.spec.js โ€” the canonical logger's behavior is covered by packages/data-schemas/src/config/parsers.spec.ts. * ๐Ÿฉน fix: Make object-traverse caps bound work and survive update() Address Codex review: (1) break the child loops as soon as the node budget is spent and iterate objects via for...in instead of materializing Object.entries/Object.keys, so maxNodes actually bounds work for wide arrays/objects; (2) detect ancestor cycles against an immutable original-node stack rather than context.node, which a callback's update() can reassign (the debug formatter rewrites array nodes in place). Adds tests for the wide-array bound and the update()-cycle case. * ๐ŸŽš๏ธ fix: Tighten object-traverse defaults to a ~1ms log budget Lower maxNodes 100000 -> 2500 and maxDepth 100 -> 5. Measured cost is ~140ns/node with the debug formatter callback, so 2500 nodes keeps a single log under ~1ms even on slower prod hardware; real log objects are ~25-30 nodes at depth 3-4, leaving ample headroom. maxNodes is the fan-out/cost lever; maxDepth bounds recursion and output readability (depth-5 covers typical logs, deeper renders compactly). --- api/config/__tests__/logToFile.spec.js | 72 --- api/config/__tests__/parsers.spec.js | 430 ------------------ api/config/index.js | 2 - api/config/parsers.js | 388 ---------------- api/config/winston.js | 224 --------- api/package.json | 1 - api/server/services/ToolService.js | 3 +- api/test/__mocks__/logger.js | 9 - package-lock.json | 12 - packages/data-schemas/src/index.ts | 1 + .../src/utils/object-traverse.spec.ts | 158 +++++++ .../data-schemas/src/utils/object-traverse.ts | 104 ++++- 12 files changed, 241 insertions(+), 1163 deletions(-) delete mode 100644 api/config/__tests__/logToFile.spec.js delete mode 100644 api/config/__tests__/parsers.spec.js delete mode 100644 api/config/parsers.js delete mode 100644 api/config/winston.js create mode 100644 packages/data-schemas/src/utils/object-traverse.spec.ts diff --git a/api/config/__tests__/logToFile.spec.js b/api/config/__tests__/logToFile.spec.js deleted file mode 100644 index 4b3170f95a..0000000000 --- a/api/config/__tests__/logToFile.spec.js +++ /dev/null @@ -1,72 +0,0 @@ -const fs = require('fs'); - -const ORIGINAL_ENV = process.env; - -const mockDataSchemas = () => { - jest.doMock('@librechat/data-schemas', () => ({ - getTenantId: jest.fn(), - getUserId: jest.fn(), - getRequestId: jest.fn(), - SYSTEM_TENANT_ID: 'system', - })); -}; - -const mockReadOnlyDockerLogDir = () => { - const originalExistsSync = fs.existsSync; - const originalMkdirSync = fs.mkdirSync; - - jest.spyOn(process, 'cwd').mockReturnValue('/app'); - jest - .spyOn(fs, 'existsSync') - .mockImplementation((target) => - target === '/app/logs' ? false : originalExistsSync.call(fs, target), - ); - - return jest.spyOn(fs, 'mkdirSync').mockImplementation((target, options) => { - if (target === '/app/logs') { - throw new Error('Attempted to create Docker log directory'); - } - return originalMkdirSync.call(fs, target, options); - }); -}; - -const prepareLoggerWithoutFileLogging = () => { - jest.resetModules(); - jest.clearAllMocks(); - mockDataSchemas(); - - process.env = { - ...ORIGINAL_ENV, - DEBUG_LOGGING: 'true', - LOG_TO_FILE: 'false', - }; - - return mockReadOnlyDockerLogDir(); -}; - -describe('LOG_TO_FILE', () => { - afterEach(() => { - process.env = ORIGINAL_ENV; - jest.restoreAllMocks(); - }); - - it('does not create the API log directory when winston file logging is disabled', () => { - const mkdirSyncSpy = prepareLoggerWithoutFileLogging(); - - expect(() => require('../winston')).not.toThrow(); - - const winston = require('winston'); - expect(winston.transports.DailyRotateFile).not.toHaveBeenCalled(); - expect(mkdirSyncSpy).not.toHaveBeenCalledWith('/app/logs', expect.anything()); - }); - - it('does not create the API log directory when Meili file logging is disabled', () => { - const mkdirSyncSpy = prepareLoggerWithoutFileLogging(); - - expect(() => require('../meiliLogger')).not.toThrow(); - - const winston = require('winston'); - expect(winston.transports.DailyRotateFile).not.toHaveBeenCalled(); - expect(mkdirSyncSpy).not.toHaveBeenCalledWith('/app/logs', expect.anything()); - }); -}); diff --git a/api/config/__tests__/parsers.spec.js b/api/config/__tests__/parsers.spec.js deleted file mode 100644 index 4c78360953..0000000000 --- a/api/config/__tests__/parsers.spec.js +++ /dev/null @@ -1,430 +0,0 @@ -jest.unmock('winston'); - -const { formatConsoleMeta, redactMessage, redactFormat, debugTraverse } = - jest.requireActual('../parsers'); -const SPLAT_SYMBOL = Symbol.for('splat'); - -describe('formatConsoleMeta', () => { - it('returns empty string when there is no user metadata', () => { - expect( - formatConsoleMeta({ - level: 'error', - message: 'oops', - timestamp: '2026-04-18 02:25:22', - }), - ).toBe(''); - }); - - it('serializes user-supplied metadata keys', () => { - const meta = formatConsoleMeta({ - level: 'error', - message: '[agents:summarize] Summarization LLM call failed', - timestamp: '2026-04-18 02:25:22', - provider: 'azureOpenAI', - model: 'gpt-5.4-mini', - messagesToRefineCount: 42, - }); - - expect(meta).toContain('"provider":"azureOpenAI"'); - expect(meta).toContain('"model":"gpt-5.4-mini"'); - expect(meta).toContain('"messagesToRefineCount":42'); - }); - - it('omits the system tenant sentinel from metadata trailers', () => { - const meta = formatConsoleMeta({ - level: 'warn', - message: 'system task', - timestamp: 'ts', - tenantId: '__SYSTEM__', - userId: 'user-1', - }); - - expect(meta).toBe('{"userId":"user-1"}'); - }); - - it('ignores reserved winston keys but preserves legitimate fields like _id', () => { - const meta = formatConsoleMeta({ - level: 'error', - message: 'boom', - timestamp: 'ts', - splat: [1, 2], - _id: '507f191e810c19729de860ea', - userField: 'keep', - }); - - expect(meta).toContain('"_id":"507f191e810c19729de860ea"'); - expect(meta).toContain('"userField":"keep"'); - expect(meta).not.toContain('"splat"'); - }); - - it('drops numeric-index-like keys (splat artifacts from primitive args)', () => { - const meta = formatConsoleMeta({ - level: 'warn', - message: 'Unhandled step:', - timestamp: 'ts', - 0: 'f', - 1: 'o', - 2: 'o', - realField: 'real', - }); - - expect(meta).toBe('{"realField":"real"}'); - }); - - it('drops empty, null, undefined, function, and symbol values', () => { - const meta = formatConsoleMeta({ - level: 'warn', - message: 'noise', - timestamp: 'ts', - empty: '', - nullish: null, - undef: undefined, - fn: () => 1, - sym: Symbol('x'), - kept: 'yes', - }); - - expect(meta).toBe('{"kept":"yes"}'); - }); - - it('truncates very long string values to avoid console spam', () => { - const longString = 'x'.repeat(5000); - const meta = formatConsoleMeta({ - level: 'error', - message: 'long', - timestamp: 'ts', - errorStack: longString, - }); - - expect(meta.length).toBeLessThan(longString.length); - expect(meta).toContain('...'); - }); - - it('preserves non-circular fields when one value is circular', () => { - const circular = {}; - circular.self = circular; - const meta = formatConsoleMeta({ - level: 'error', - message: 'circular', - timestamp: 'ts', - provider: 'openai', - model: 'gpt-5.4-mini', - circular, - }); - - expect(meta).toContain('"provider":"openai"'); - expect(meta).toContain('"model":"gpt-5.4-mini"'); - expect(meta).toContain('[Circular]'); - }); - - it('falls back to per-field serialization when a value toJSON throws', () => { - const meta = formatConsoleMeta({ - level: 'error', - message: 'crash', - timestamp: 'ts', - provider: 'azure', - model: 'gpt-5.4-mini', - broken: { - toJSON() { - throw new Error('nope'); - }, - }, - }); - - expect(meta).toContain('"provider":"azure"'); - expect(meta).toContain('"model":"gpt-5.4-mini"'); - expect(meta).toContain('[Unserializable]'); - }); - - it('redacts sensitive strings nested inside metadata objects', () => { - const meta = formatConsoleMeta({ - level: 'error', - message: 'nested leak', - timestamp: 'ts', - config: { - headers: { - authorization: 'Bearer eyJhbGciOi.nestedTokenValue', - }, - query: 'https://example.com/?key=AIzaNested', - }, - openaiKey: 'sk-outerKey123', - }); - - expect(meta).not.toContain('eyJhbGciOi.nestedTokenValue'); - expect(meta).not.toContain('AIzaNested'); - expect(meta).not.toContain('sk-outerKey123'); - expect(meta).toContain('Bearer [REDACTED]'); - expect(meta).toContain('key=[REDACTED]'); - expect(meta).toContain('sk-[REDACTED]'); - }); - - it('redacts the Azure-style mixed-case Api-Key header', () => { - const meta = formatConsoleMeta({ - level: 'error', - message: 'azure call', - timestamp: 'ts', - headers: 'Api-Key: 0123456789abcdef', - }); - - expect(meta).not.toContain('0123456789abcdef'); - expect(meta).toContain('Api-Key: [REDACTED]'); - }); - - 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(''); - }); - - it('does not redact ordinary words that contain "sk-" inside them', () => { - expect(redactMessage('task-runner failed')).toBe('task-runner failed'); - expect(redactMessage('mask-value computed')).toBe('mask-value computed'); - expect(redactMessage('desk-lamp is on')).toBe('desk-lamp is on'); - }); - - it('does not redact words that contain "key=" inside them', () => { - expect(redactMessage('monkey=10 bananas')).toBe('monkey=10 bananas'); - }); - - it('still redacts standalone sk- keys at word boundaries', () => { - expect(redactMessage('token: sk-abc123def')).toBe('token: sk-[REDACTED]'); - expect(redactMessage('"sk-abc123def"')).toBe('"sk-[REDACTED]"'); - }); -}); - -describe('redactFormat', () => { - const runFormat = (info) => redactFormat().transform(info) || info; - - it('redacts info.message for error level before any colorize step runs', () => { - const info = runFormat({ level: 'error', message: 'Bearer secretvalue' }); - expect(info.message).toBe('Bearer [REDACTED]'); - }); - - it('redacts info.message for warn level too (avoids ANSI boundary issues later)', () => { - const info = runFormat({ level: 'warn', message: 'apiKey=sk-abc123def' }); - expect(info.message).toContain('sk-[REDACTED]'); - }); - - it('leaves info.message untouched for info and debug levels', () => { - const infoInfo = runFormat({ level: 'info', message: 'Bearer looksSensitive' }); - expect(infoInfo.message).toBe('Bearer looksSensitive'); - - const infoDebug = runFormat({ level: 'debug', message: 'Bearer looksSensitive' }); - expect(infoDebug.message).toBe('Bearer looksSensitive'); - }); -}); - -describe('debugTraverse', () => { - const runFormatter = (info) => { - const transformed = debugTraverse.transform(info); - const MESSAGE = Symbol.for('message'); - if (transformed && typeof transformed === 'object') { - return transformed[MESSAGE] ?? String(transformed); - } - return String(transformed); - }; - - const buildInfo = (level, meta) => { - const info = { - level, - message: 'test', - timestamp: 'ts', - ...meta, - }; - info[SPLAT_SYMBOL] = [meta]; - return info; - }; - - it('redacts sensitive strings in metadata for error level', () => { - const out = runFormatter(buildInfo('error', { auth: 'Bearer eyJabc123', openai: 'sk-abc123' })); - expect(out).not.toContain('eyJabc123'); - expect(out).not.toContain('sk-abc123'); - expect(out).toContain('Bearer [REDACTED]'); - expect(out).toContain('sk-[REDACTED]'); - }); - - it('redacts sensitive strings in metadata for warn level', () => { - const out = runFormatter(buildInfo('warn', { header: 'Bearer supersecrettoken' })); - expect(out).not.toContain('supersecrettoken'); - expect(out).toContain('Bearer [REDACTED]'); - }); - - it('preserves debug-level metadata unmodified (existing behavior)', () => { - const out = runFormatter(buildInfo('debug', { someField: 'not-sensitive' })); - expect(out).toContain('not-sensitive'); - }); - - it('prefers structured metadata over a consumed printf arg in SPLAT[0]', () => { - const info = { - level: 'warn', - message: 'failed for tenant-7', - timestamp: 'ts', - provider: 'openai', - [SPLAT_SYMBOL]: ['tenant-7', { provider: 'openai' }], - }; - const out = runFormatter(info); - expect(out).toContain('openai'); - const tenantMatches = out.match(/tenant-7/g) ?? []; - expect(tenantMatches.length).toBeLessThanOrEqual(1); - }); - - it('does not duplicate a consumed %s arg when there is no structured metadata', () => { - const info = { - level: 'warn', - message: 'failed for tenant-7', - timestamp: 'ts', - [SPLAT_SYMBOL]: ['tenant-7'], - }; - const out = runFormatter(info); - const tenantMatches = out.match(/tenant-7/g) ?? []; - expect(tenantMatches.length).toBe(1); - }); - - it('appends request context metadata for non-debug lines', () => { - const out = runFormatter( - buildInfo('info', { - tenantId: 'tenant-1', - userId: 'user-1', - requestId: 'req-1', - }), - ); - - expect(out).toContain('"tenantId":"tenant-1"'); - expect(out).toContain('"userId":"user-1"'); - expect(out).toContain('"requestId":"req-1"'); - }); - - it('does not append the system tenant sentinel as tenantId', () => { - const out = runFormatter( - buildInfo('info', { - tenantId: '__SYSTEM__', - userId: 'user-1', - requestId: 'req-1', - }), - ); - - expect(out).not.toContain('__SYSTEM__'); - expect(out).not.toContain('"tenantId"'); - expect(out).toContain('"userId":"user-1"'); - expect(out).toContain('"requestId":"req-1"'); - }); - - it('omits the system tenant sentinel from debug object metadata', () => { - const out = runFormatter( - buildInfo('debug', { - tenantId: '__SYSTEM__', - userId: 'user-1', - }), - ); - - expect(out).not.toContain('__SYSTEM__'); - expect(out).not.toMatch(/tenantId:/); - expect(out).toContain('userId'); - }); - - it('appends request context metadata for debug lines without object metadata', () => { - const info = { - level: 'debug', - message: 'prefix:', - timestamp: 'ts', - tenantId: 'tenant-1', - userId: 'user-1', - requestId: 'req-1', - [SPLAT_SYMBOL]: ['detailValueXYZ'], - }; - const out = runFormatter(info); - - expect(out).toContain('detailValueXYZ'); - expect(out).toContain('"tenantId":"tenant-1"'); - expect(out).toContain('"userId":"user-1"'); - expect(out).toContain('"requestId":"req-1"'); - }); - - it('omits numeric splat-artifact keys from the traversed output', () => { - const info = { - level: 'error', - message: 'boom', - timestamp: 'ts', - 0: 'x', - 1: 'y', - realField: 'keep', - [SPLAT_SYMBOL]: [{ realField: 'keep' }], - }; - const out = runFormatter(info); - expect(out).toContain('realField'); - expect(out).toContain('keep'); - expect(out).not.toMatch(/^\s*0:/m); - expect(out).not.toMatch(/^\s*1:/m); - }); - - it('surfaces unconsumed primitive SPLAT[0] (no %s in message) for debug level', () => { - const info = { - level: 'debug', - message: 'prefix:', - timestamp: 'ts', - [SPLAT_SYMBOL]: ['detailValueXYZ'], - }; - const out = runFormatter(info); - expect(out).toContain('detailValueXYZ'); - }); - - it('still surfaces array metadata in SPLAT[0] when no object is extracted', () => { - const info = { - level: 'debug', - message: 'list', - timestamp: 'ts', - [SPLAT_SYMBOL]: [['alpha', 'beta', 'gamma']], - }; - const out = runFormatter(info); - expect(out).toContain('alpha'); - expect(out).toContain('beta'); - expect(out).toContain('gamma'); - }); -}); diff --git a/api/config/index.js b/api/config/index.js index 9adfee5637..6d9f70ecbb 100644 --- a/api/config/index.js +++ b/api/config/index.js @@ -7,7 +7,6 @@ const { MCPServersRegistry, OAuthReconnectionManager, } = require('@librechat/api'); -const logger = require('./winston'); global.EventSource = EventSource; @@ -47,7 +46,6 @@ function getActionFlowStateManager(flowsCache) { } module.exports = { - logger, createMCPServersRegistry: MCPServersRegistry.createInstance, getMCPServersRegistry: MCPServersRegistry.getInstance, createMCPManager: MCPManager.createInstance, diff --git a/api/config/parsers.js b/api/config/parsers.js deleted file mode 100644 index 477e371253..0000000000 --- a/api/config/parsers.js +++ /dev/null @@ -1,388 +0,0 @@ -const { klona } = require('klona'); -const winston = require('winston'); -const traverse = require('traverse'); - -const SPLAT_SYMBOL = Symbol.for('splat'); -const MESSAGE_SYMBOL = Symbol.for('message'); -const CONSOLE_JSON_STRING_LENGTH = parseInt(process.env.CONSOLE_JSON_STRING_LENGTH) || 255; -const DEBUG_MESSAGE_LENGTH = parseInt(process.env.DEBUG_MESSAGE_LENGTH) || 150; - -const sensitiveKeys = [ - // OpenAI API key: `sk-` at a word boundary, followed by the documented - // charset for keys. `\b` keeps `task-runner`, `mask-value`, etc. from - // being mis-redacted. - /\b(sk-)[a-zA-Z0-9_-]+/g, - /\b(Bearer )[^\s"']+/g, // Header: Bearer token pattern - /\b(api-key:? )[^\s"']+/gi, // Header: API key pattern (case-insensitive; covers `Api-Key:`, `API-KEY:`) - /\b(key=)[^\s"'&]+/g, // URL query param: sensitive key pattern (Google) -]; - -const NUMERIC_KEY_RE = /^\d+$/; -const LOG_CONTEXT_KEYS = ['tenantId', 'userId', 'requestId']; -const SYSTEM_TENANT_ID = '__SYSTEM__'; - -/** - * 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. - * @param {number} [trimLength] - The optional length at which to trim the redacted message. - * @returns {string} - The redacted and optionally trimmed console message. - */ -function redactMessage(str, trimLength) { - if (!str) { - return ''; - } - - let redacted = str; - for (const pattern of sensitiveKeys) { - redacted = redacted.replace(pattern, '$1[REDACTED]'); - } - - if (trimLength !== undefined && redacted.length > trimLength) { - return `${redacted.substring(0, trimLength)}...`; - } - - return redacted; -} - -/** - * Redacts sensitive information from log messages when the log level is - * `error` or `warn`. Runs on the raw `info.message` before any colorize / - * splat transforms so the sensitive-token regexes don't have to contend - * with ANSI escape sequences (whose trailing `m` would otherwise defeat - * `\b` anchors). - * - * Note: Intentionally mutates the object. - * @param {Object} info - The log information object. - * @returns {Object} - The modified log information object. - */ -const redactFormat = winston.format((info) => { - if (info.level === 'error' || info.level === 'warn') { - if (typeof info.message === 'string') { - info.message = redactMessage(info.message); - } - if (typeof info[MESSAGE_SYMBOL] === 'string') { - info[MESSAGE_SYMBOL] = redactMessage(info[MESSAGE_SYMBOL]); - } - } - return info; -}); - -/** - * Truncates long strings, especially base64 image data, within log messages. - * - * @param {any} value - The value to be inspected and potentially truncated. - * @param {number} [length] - The length at which to truncate the value. Default: 100. - * @returns {any} - The truncated or original value. - */ -const truncateLongStrings = (value, length = 100) => { - if (typeof value === 'string') { - return value.length > length ? value.substring(0, length) + '... [truncated]' : value; - } - - return value; -}; - -/** - * An array mapping function that truncates long strings (objects converted to JSON strings). - * @param {any} item - The item to be condensed. - * @returns {any} - The condensed item. - */ -const condenseArray = (item) => { - if (typeof item === 'string') { - return truncateLongStrings(JSON.stringify(item)); - } else if (typeof item === 'object') { - return truncateLongStrings(JSON.stringify(item)); - } - return item; -}; - -const RESERVED_LOG_KEYS = new Set(['level', 'message', 'timestamp', 'splat']); - -/** - * Extracts user-supplied metadata from a winston info object. Filters out: - * - Reserved winston keys (`level`, `message`, `timestamp`, `splat`). - * - Numeric-string keys (`"0"`, `"1"`, ...) that `format.splat()` can - * synthesize when a primitive is passed as an extra log argument. - * - Values that are undefined, null, empty strings, functions, or symbols. - * - * Underscore-prefixed keys are intentionally preserved so legitimate - * fields like MongoDB `_id` survive. - * - * @param {Record} source - The object to extract metadata from. - * @returns {Record | undefined} - The extracted metadata, or undefined if empty. - */ -function extractMetaObject(source) { - if (source == null || typeof source !== 'object') { - return undefined; - } - const meta = {}; - for (const key of Object.keys(source)) { - if (RESERVED_LOG_KEYS.has(key)) { - continue; - } - if (NUMERIC_KEY_RE.test(key)) { - continue; - } - const value = source[key]; - if (key === 'tenantId' && value === SYSTEM_TENANT_ID) { - continue; - } - if (value === undefined || value === null || value === '') { - continue; - } - if (typeof value === 'function' || typeof value === 'symbol') { - continue; - } - meta[key] = value; - } - return Object.keys(meta).length > 0 ? meta : undefined; -} - -/** - * Formats the metadata portion of a winston info object as a compact - * single-line JSON trailer, suitable for appending to the console message. - * Returns an empty string when there is no meaningful metadata. - * - * @param {Record} info - The winston info object. - * @returns {string} - The serialized metadata, or an empty string. - */ -function formatConsoleMeta(info) { - const meta = extractMetaObject(info); - if (!meta) { - return ''; - } - const seen = new WeakSet(); - const replacer = (_key, value) => { - if (typeof value === 'string') { - const safe = redactMessage(value); - return safe.length > CONSOLE_JSON_STRING_LENGTH - ? `${safe.substring(0, CONSOLE_JSON_STRING_LENGTH)}...` - : safe; - } - if (value !== null && typeof value === 'object') { - if (seen.has(value)) { - return '[Circular]'; - } - seen.add(value); - } - return value; - }; - - try { - return JSON.stringify(meta, replacer); - } catch { - /* - * Fall back to per-field serialization: a single unserializable field - * shouldn't drop every other scalar in the trailer. Scalars are emitted - * as-is; values that still fail serialization are replaced with a - * placeholder so `provider`, `model`, etc. continue to surface. - */ - const parts = []; - for (const key of Object.keys(meta)) { - const perFieldSeen = new WeakSet(); - const perFieldReplacer = (k, value) => { - if (typeof value === 'string') { - return replacer(k, value); - } - if (value !== null && typeof value === 'object') { - if (perFieldSeen.has(value)) { - return '[Circular]'; - } - perFieldSeen.add(value); - } - return value; - }; - try { - parts.push(`${JSON.stringify(key)}:${JSON.stringify(meta[key], perFieldReplacer)}`); - } catch { - parts.push(`${JSON.stringify(key)}:"[Unserializable]"`); - } - } - return parts.length > 0 ? `{${parts.join(',')}}` : ''; - } -} - -function formatRequestContext(info) { - if (info == null || typeof info !== 'object') { - return ''; - } - const context = {}; - for (const key of LOG_CONTEXT_KEYS) { - const value = info[key]; - if (key === 'tenantId' && value === SYSTEM_TENANT_ID) { - continue; - } - if (typeof value === 'string' && value) { - context[key] = value; - } - } - return Object.keys(context).length > 0 ? JSON.stringify(context) : ''; -} - -/** - * Formats log messages for file and debug-console transports. Three paths: - * - `warn` / `error`: append a compact single-line JSON metadata trailer - * (via `formatConsoleMeta`) and pass the full line through `redactMessage` - * so sensitive patterns are scrubbed. - * - `debug`: perform the detailed multi-line object traversal of - * `SPLAT_SYMBOL[0]`, with long-string truncation and array condensation. - * Redaction on this path is not applied here (debug-file consumers - * historically accept raw detail). - * - Other levels: return the truncated `" : "` - * line with request context metadata when present. - * - * @param {Object} options - The options for formatting log messages. - * @param {string} options.level - The log level. - * @param {string} options.message - The log message. - * @param {string} options.timestamp - The timestamp of the log message. - * @param {Object} options.metadata - Additional metadata associated with the log message. - * @returns {string} - The formatted log message. - */ -const debugTraverse = winston.format.printf(({ level, message, timestamp, ...metadata }) => { - if (!message) { - return `${timestamp} ${level}`; - } - - if (!message?.trim || typeof message !== 'string') { - return `${timestamp} ${level}: ${JSON.stringify(message)}`; - } - - let msg = `${timestamp} ${level}: ${truncateLongStrings(message?.trim(), DEBUG_MESSAGE_LENGTH)}`; - const levelStr = typeof level === 'string' ? level : String(level); - const isErrorOrWarn = levelStr.includes('error') || levelStr.includes('warn'); - - /* - * Warn/error follow a simpler code path: append a single-line JSON - * metadata trailer (same shape as the console formatter) and pass the - * result through `redactMessage`. The complex object-traversal below is - * kept for debug level only, where detailed multi-line output is the - * intended behavior and its splat/interpolation interactions were - * already tolerated. - */ - if (isErrorOrWarn) { - const trailer = formatConsoleMeta(metadata); - const line = trailer ? `${msg} ${trailer}` : msg; - return redactMessage(line); - } - - try { - if (level !== 'debug') { - const trailer = formatRequestContext(metadata); - return trailer ? `${msg} ${trailer}` : msg; - } - - if (!metadata) { - return msg; - } - - const appendMetadataTrailer = (line) => { - const trailer = formatRequestContext(metadata); - return trailer ? `${line} ${trailer}` : line; - }; - - const debugValue = metadata[SPLAT_SYMBOL]?.[0]; - - if (!debugValue) { - return appendMetadataTrailer(msg); - } - - if (debugValue && Array.isArray(debugValue)) { - msg += `\n${JSON.stringify(debugValue.map(condenseArray))}`; - return appendMetadataTrailer(msg); - } - - if (typeof debugValue !== 'object') { - msg += ` ${debugValue}`; - return appendMetadataTrailer(msg); - } - - msg += '\n{'; - - const copy = klona(metadata); - if (copy.tenantId === SYSTEM_TENANT_ID) { - delete copy.tenantId; - } - traverse(copy).forEach(function (value) { - if (typeof this?.key === 'symbol') { - return; - } - - let _parentKey = ''; - const parent = this.parent; - - if (typeof parent?.key !== 'symbol' && parent?.key) { - _parentKey = parent.key; - } - - const parentKey = `${parent && parent.notRoot ? _parentKey + '.' : ''}`; - - const tabs = `${parent && parent.notRoot ? ' ' : ' '}`; - - const currentKey = this?.key ?? 'unknown'; - - if (this.isLeaf && typeof value === 'string') { - const truncatedText = truncateLongStrings(value); - msg += `\n${tabs}${parentKey}${currentKey}: ${JSON.stringify(truncatedText)},`; - } else if (this.notLeaf && Array.isArray(value) && value.length > 0) { - const currentMessage = `\n${tabs}// ${value.length} ${currentKey.replace(/s$/, '')}(s)`; - this.update(currentMessage, true); - msg += currentMessage; - const stringifiedArray = value.map(condenseArray); - msg += `\n${tabs}${parentKey}${currentKey}: [${stringifiedArray}],`; - } else if (this.isLeaf && typeof value === 'function') { - msg += `\n${tabs}${parentKey}${currentKey}: function,`; - } else if (this.isLeaf) { - msg += `\n${tabs}${parentKey}${currentKey}: ${value},`; - } - }); - - msg += '\n}'; - return msg; - } catch (e) { - return (msg += `\n[LOGGER PARSING ERROR] ${e.message}`); - } -}); - -const jsonTruncateFormat = winston.format((info) => { - const truncateLongStrings = (str, maxLength) => { - return str.length > maxLength ? str.substring(0, maxLength) + '...' : str; - }; - - const seen = new WeakSet(); - - const truncateObject = (obj) => { - if (typeof obj !== 'object' || obj === null) { - return obj; - } - - // Handle circular references - if (seen.has(obj)) { - return '[Circular]'; - } - seen.add(obj); - - if (Array.isArray(obj)) { - return obj.map((item) => truncateObject(item)); - } - - const newObj = {}; - Object.entries(obj).forEach(([key, value]) => { - if (typeof value === 'string') { - newObj[key] = truncateLongStrings(value, CONSOLE_JSON_STRING_LENGTH); - } else { - newObj[key] = truncateObject(value); - } - }); - return newObj; - }; - - return truncateObject(info); -}); - -module.exports = { - redactFormat, - redactMessage, - debugTraverse, - jsonTruncateFormat, - formatConsoleMeta, -}; diff --git a/api/config/winston.js b/api/config/winston.js deleted file mode 100644 index 983205fc70..0000000000 --- a/api/config/winston.js +++ /dev/null @@ -1,224 +0,0 @@ -const path = require('path'); -const fs = require('fs'); -const winston = require('winston'); -require('winston-daily-rotate-file'); -const { - getTenantId, - getUserId, - getRequestId, - SYSTEM_TENANT_ID, -} = require('@librechat/data-schemas'); -const { - redactFormat, - redactMessage, - debugTraverse, - jsonTruncateFormat, - formatConsoleMeta, -} = require('./parsers'); - -/** - * Determine the log directory. - * Priority: - * 1. LIBRECHAT_LOG_DIR environment variable (allows user override) - * 2. /app/logs if running in Docker (bind-mounted with correct permissions) - * 3. api/logs relative to this file (local development) - */ -const getLogDir = () => { - if (process.env.LIBRECHAT_LOG_DIR) { - return process.env.LIBRECHAT_LOG_DIR; - } - - // Check if running in Docker container (cwd is /app) - if (process.cwd() === '/app') { - const dockerLogDir = '/app/logs'; - // Ensure the directory exists - if (!fs.existsSync(dockerLogDir)) { - fs.mkdirSync(dockerLogDir, { recursive: true }); - } - return dockerLogDir; - } - - // Local development: use api/logs relative to this file - return path.join(__dirname, '..', 'logs'); -}; - -const { - NODE_ENV, - DEBUG_LOGGING = true, - CONSOLE_JSON = false, - DEBUG_CONSOLE = false, - LOG_TO_FILE = true, -} = process.env; - -const useConsoleJson = - (typeof CONSOLE_JSON === 'string' && CONSOLE_JSON?.toLowerCase() === 'true') || - CONSOLE_JSON === true; - -const useDebugConsole = - (typeof DEBUG_CONSOLE === 'string' && DEBUG_CONSOLE?.toLowerCase() === 'true') || - DEBUG_CONSOLE === true; - -const useDebugLogging = - (typeof DEBUG_LOGGING === 'string' && DEBUG_LOGGING?.toLowerCase() === 'true') || - DEBUG_LOGGING === true; - -const useFileLogging = - (typeof LOG_TO_FILE === 'string' && LOG_TO_FILE?.toLowerCase() !== 'false') || - LOG_TO_FILE === true; - -const levels = { - error: 0, - warn: 1, - info: 2, - http: 3, - verbose: 4, - debug: 5, - activity: 6, - silly: 7, -}; - -const LOG_CONTEXT_KEYS = ['tenantId', 'userId', 'requestId']; - -const getLogTenantId = () => { - const tenantId = getTenantId(); - return tenantId === SYSTEM_TENANT_ID ? undefined : tenantId; -}; - -const requestContextFormat = winston.format((info) => { - if (info.tenantId === SYSTEM_TENANT_ID) { - delete info.tenantId; - } - const context = { - tenantId: getLogTenantId(), - userId: getUserId(), - requestId: getRequestId(), - }; - LOG_CONTEXT_KEYS.forEach((key) => { - if (context[key] && info[key] == null) { - info[key] = context[key]; - } - }); - return info; -}); - -const formatRequestContext = (info) => { - const context = {}; - LOG_CONTEXT_KEYS.forEach((key) => { - const value = info[key]; - if (key === 'tenantId' && value === SYSTEM_TENANT_ID) { - return; - } - if (typeof value === 'string' && value) { - context[key] = value; - } - }); - return Object.keys(context).length > 0 ? JSON.stringify(context) : ''; -}; - -winston.addColors({ - info: 'green', // fontStyle color - warn: 'italic yellow', - error: 'red', - debug: 'blue', -}); - -const level = () => { - const env = NODE_ENV || 'development'; - const isDevelopment = env === 'development'; - return isDevelopment ? 'debug' : 'warn'; -}; - -const fileFormat = winston.format.combine( - redactFormat(), - winston.format.timestamp({ format: () => new Date().toISOString() }), - winston.format.errors({ stack: true }), - winston.format.splat(), - requestContextFormat(), - // redactErrors(), -); - -const transports = []; - -if (useFileLogging) { - const logDir = getLogDir(); - - transports.push( - new winston.transports.DailyRotateFile({ - level: 'error', - filename: `${logDir}/error-%DATE%.log`, - datePattern: 'YYYY-MM-DD', - zippedArchive: true, - maxSize: '20m', - maxFiles: '14d', - format: fileFormat, - }), - ); - - if (useDebugLogging) { - transports.push( - new winston.transports.DailyRotateFile({ - level: 'debug', - filename: `${logDir}/debug-%DATE%.log`, - datePattern: 'YYYY-MM-DD', - zippedArchive: true, - maxSize: '20m', - maxFiles: '14d', - format: winston.format.combine(fileFormat, debugTraverse), - }), - ); - } -} - -const consoleFormat = winston.format.combine( - redactFormat(), - requestContextFormat(), - winston.format.colorize({ all: true }), - winston.format.timestamp({ format: 'YYYY-MM-DD HH:mm:ss' }), - // redactErrors(), - winston.format.printf((info) => { - const base = `${info.timestamp} ${info.level}: ${info.message}`; - const isErrorOrWarn = info.level.includes('error') || info.level.includes('warn'); - const metaTrailer = isErrorOrWarn ? formatConsoleMeta(info) : formatRequestContext(info); - const line = metaTrailer ? `${base} ${metaTrailer}` : base; - return isErrorOrWarn ? redactMessage(line) : line; - }), -); - -// Determine console log level -let consoleLogLevel = 'info'; -if (useDebugConsole) { - consoleLogLevel = 'debug'; -} - -if (useDebugConsole) { - transports.push( - new winston.transports.Console({ - level: consoleLogLevel, - format: useConsoleJson - ? winston.format.combine(fileFormat, jsonTruncateFormat(), winston.format.json()) - : winston.format.combine(fileFormat, debugTraverse), - }), - ); -} else if (useConsoleJson) { - transports.push( - new winston.transports.Console({ - level: consoleLogLevel, - format: winston.format.combine(fileFormat, jsonTruncateFormat(), winston.format.json()), - }), - ); -} else { - transports.push( - new winston.transports.Console({ - level: consoleLogLevel, - format: consoleFormat, - }), - ); -} - -const logger = winston.createLogger({ - level: level(), - levels, - transports, -}); - -module.exports = logger; diff --git a/api/package.json b/api/package.json index dc8eb96686..74a73add47 100644 --- a/api/package.json +++ b/api/package.json @@ -124,7 +124,6 @@ "rate-limit-redis": "^4.2.0", "sanitize-html": "^2.13.0", "sharp": "^0.33.5", - "traverse": "^0.6.7", "ua-parser-js": "^1.0.36", "undici": "^7.24.1", "winston": "^3.11.0", diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 99054b1fca..5624aef65f 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -1,4 +1,4 @@ -const { logger } = require('@librechat/data-schemas'); +const { logger, redactMessage } = require('@librechat/data-schemas'); const { tool: toolFn, DynamicStructuredTool } = require('@librechat/agents/langchain/tools'); const { sleep, @@ -72,7 +72,6 @@ const { createMCPPermissionContext, resolveConfigServers } = require('~/server/s const { getMCPRequestContext } = require('~/server/services/MCPRequestContext'); const { recordUsage } = require('~/server/services/Threads'); const { loadTools } = require('~/app/clients/tools/util'); -const { redactMessage } = require('~/config/parsers'); const { findPluginAuthsByKeys } = require('~/models'); const { getFlowStateManager, getMCPServersRegistry } = require('~/config'); const { getLogStores } = require('~/cache'); diff --git a/api/test/__mocks__/logger.js b/api/test/__mocks__/logger.js index 94dd08bb1c..699a94883f 100644 --- a/api/test/__mocks__/logger.js +++ b/api/test/__mocks__/logger.js @@ -58,12 +58,3 @@ jest.mock('~/config', () => { }, }; }); - -jest.mock('~/config/parsers', () => { - return { - redactMessage: jest.fn(), - redactFormat: jest.fn(), - debugTraverse: jest.fn(), - formatConsoleMeta: jest.fn(() => ''), - }; -}); diff --git a/package-lock.json b/package-lock.json index 66cf865512..c45c70dc9b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -139,7 +139,6 @@ "rate-limit-redis": "^4.2.0", "sanitize-html": "^2.13.0", "sharp": "^0.33.5", - "traverse": "^0.6.7", "ua-parser-js": "^1.0.36", "undici": "^7.24.1", "winston": "^3.11.0", @@ -41767,17 +41766,6 @@ "node": ">=18" } }, - "node_modules/traverse": { - "version": "0.6.8", - "resolved": "https://registry.npmjs.org/traverse/-/traverse-0.6.8.tgz", - "integrity": "sha512-aXJDbk6SnumuaZSANd21XAo15ucCDE38H4fkqiGsc3MhCK+wOlZvLP9cB/TvpHT0mOyWgC4Z8EwRlzqYSUzdsA==", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/tree-kill": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/tree-kill/-/tree-kill-1.2.2.tgz", diff --git a/packages/data-schemas/src/index.ts b/packages/data-schemas/src/index.ts index 9753ca6e6d..3d99b90675 100644 --- a/packages/data-schemas/src/index.ts +++ b/packages/data-schemas/src/index.ts @@ -29,6 +29,7 @@ export type * from './types'; export type * from './methods'; export { default as logger } from './config/winston'; export { default as meiliLogger } from './config/meiliLogger'; +export { redactMessage } from './config/parsers'; export { tenantStorage, getTenantId, diff --git a/packages/data-schemas/src/utils/object-traverse.spec.ts b/packages/data-schemas/src/utils/object-traverse.spec.ts new file mode 100644 index 0000000000..555d8740c0 --- /dev/null +++ b/packages/data-schemas/src/utils/object-traverse.spec.ts @@ -0,0 +1,158 @@ +import type { TraverseContext } from './object-traverse'; +import traverse from './object-traverse'; + +/** Collects the dotted leaf paths visited during a traversal. */ +function collectLeafPaths(input: unknown): string[] { + const paths: string[] = []; + traverse(input).forEach(function (this: TraverseContext) { + if (this.isLeaf && !this.isRoot) { + paths.push(this.path.join('.')); + } + }); + return paths; +} + +/** Counts every node (including the root) visited during a traversal. */ +function countVisits(input: unknown, options?: { maxNodes?: number; maxDepth?: number }): number { + let count = 0; + traverse(input, options).forEach(function () { + count++; + }); + return count; +} + +describe('object-traverse', () => { + describe('correctness', () => { + it('visits every leaf of a plain nested object', () => { + const paths = collectLeafPaths({ a: 1, b: { c: 2, d: 3 } }); + expect(paths).toEqual(expect.arrayContaining(['a', 'b.c', 'b.d'])); + }); + + it('traverses shared (non-circular) references that appear in multiple branches', () => { + const shared = { secret: 'value' }; + const paths = collectLeafPaths({ a: shared, b: shared }); + + expect(paths).toContain('a.secret'); + expect(paths).toContain('b.secret'); + }); + + it('traverses an array containing the same object multiple times', () => { + const shared = { value: 42 }; + expect(collectLeafPaths([shared, shared, shared])).toEqual(['0.value', '1.value', '2.value']); + }); + + it('invokes the callback for each occurrence of a shared reference', () => { + const shared = { name: 'duplicate' }; + const visited: unknown[] = []; + traverse({ first: shared, second: shared }).forEach(function (value: unknown) { + visited.push(value); + }); + + expect(visited.filter((node) => node === shared)).toHaveLength(2); + }); + + it('does not treat a diamond-shaped (DAG) structure as circular', () => { + const leaf = { v: 1 }; + const paths = collectLeafPaths({ left: { leaf }, right: { leaf } }); + + expect(paths).toContain('left.leaf.v'); + expect(paths).toContain('right.leaf.v'); + }); + }); + + describe('cycle safety', () => { + it('does not infinitely recurse on a self-referential object', () => { + const node: Record = { id: 1 }; + node.self = node; + + const paths = collectLeafPaths(node); + + expect(paths).toContain('id'); + expect(paths).not.toContain('self.id'); + }); + + it('does not infinitely recurse on a multi-node cycle', () => { + const a: Record = { name: 'a' }; + const b: Record = { name: 'b' }; + a.next = b; + b.prev = a; + + expect(() => countVisits(a)).not.toThrow(); + const paths = collectLeafPaths(a); + expect(paths).toContain('name'); + expect(paths).toContain('next.name'); + }); + }); + + describe('bounds', () => { + it('caps total work via maxNodes on a fan-out DAG', () => { + // A diamond chain has 2^depth root-to-leaf paths; without a bound this + // would visit millions of nodes. The cap keeps it finite. + let node: Record = { leaf: 1 }; + for (let i = 0; i < 24; i++) { + const child = node; + node = { l: child, r: child }; + } + + expect(countVisits(node, { maxNodes: 5000 })).toBeLessThanOrEqual(5000); + }); + + it('stops descending past maxDepth, visiting deep nodes as leaves', () => { + const deep = { a: { b: { c: { d: { e: 'too deep' } } } } }; + + const leafLevels: number[] = []; + traverse(deep, { maxDepth: 2 }).forEach(function (this: TraverseContext) { + if (this.isLeaf) { + leafLevels.push(this.level); + } + }); + + expect(Math.max(...leafLevels)).toBe(2); + }); + + it('does not truncate ordinary objects under the default bounds', () => { + // root, a, b, b.c, b.d, and the three array elements. + expect(countVisits({ a: 1, b: { c: 2, d: [3, 4, 5] } })).toBe(8); + }); + + it('stops iterating array children once the node budget is exhausted', () => { + let indexReads = 0; + const big = Array.from({ length: 1000 }, (_, i) => ({ i })); + const probed = new Proxy(big, { + get(target, prop, receiver) { + if (typeof prop === 'string' && /^\d+$/.test(prop)) { + indexReads++; + } + return Reflect.get(target, prop, receiver); + }, + }); + + // The budget is spent on the root alone, so the child loop must break + // before touching all 1000 elements (no O(n) work after the cap). + countVisits(probed, { maxNodes: 1 }); + + expect(indexReads).toBeLessThan(10); + }); + }); + + describe('mutation safety', () => { + it('detects an ancestor cycle even when a callback replaces the node via update()', () => { + const arr: unknown[] = []; + arr.push(arr); + + let selfVisits = 0; + traverse(arr, { maxDepth: 50 }).forEach(function (this: TraverseContext, value: unknown) { + if (value === arr) { + selfVisits++; + } + // Mimic the debug formatter, which rewrites array nodes in place. + if (this.notLeaf && Array.isArray(value)) { + this.update('[summary]'); + } + }); + + // The self-reference is skipped as a true cycle, not re-expanded per level. + expect(selfVisits).toBe(1); + }); + }); +}); diff --git a/packages/data-schemas/src/utils/object-traverse.ts b/packages/data-schemas/src/utils/object-traverse.ts index 836f59a345..c61fd5cab2 100644 --- a/packages/data-schemas/src/utils/object-traverse.ts +++ b/packages/data-schemas/src/utils/object-traverse.ts @@ -3,6 +3,28 @@ * Simplified implementation focused on the forEach use case */ +/** + * Defensive bounds for traversal. Cycles are detected via the ancestor chain, + * but a shared (non-circular) reference reachable through many paths can still + * fan out super-linearly on a DAG. These caps keep traversal off the event loop + * floor for pathological inputs (e.g. logging a deeply shared object) without + * affecting normal use. Tune via the options argument to `traverse`. + */ +export interface TraverseOptions { + /** Maximum number of nodes visited before traversal stops. */ + maxNodes?: number; + /** Maximum depth descended; deeper nodes are visited as leaves, not expanded. */ + maxDepth?: number; +} + +// Tuned for the sole consumer, the debug logger. Measured cost is ~140ns/node +// with the formatter callback, so ~2.5k nodes keeps one log under ~1ms even on +// slower prod hardware, while real log objects are ~25-30 nodes at depth 3-4 โ€” +// ample headroom. maxNodes bounds fan-out (the cost lever); maxDepth bounds +// recursion/readability. Callers needing more override via the options argument. +const DEFAULT_MAX_NODES = 2_500; +const DEFAULT_MAX_DEPTH = 5; + export interface TraverseContext { node: unknown; path: (string | number)[]; @@ -87,37 +109,60 @@ function deleteProperty(obj: TraversableObject, key: string | number): void { } } -function forEach(obj: unknown, callback: ForEachCallback): void { - const visited = new WeakSet(); +function hasOwnEnumerable(node: TraversableObject): boolean { + for (const key in node) { + if (Object.prototype.hasOwnProperty.call(node, key)) { + return true; + } + } + return false; +} + +function forEach(obj: unknown, callback: ForEachCallback, options?: TraverseOptions): void { + const maxNodes = options?.maxNodes ?? DEFAULT_MAX_NODES; + const maxDepth = options?.maxDepth ?? DEFAULT_MAX_DEPTH; + let visitedCount = 0; + // Original (never-mutated) node references for the current DFS path, paired + // with their contexts. Cycle detection compares against these rather than + // context.node, which a callback's update() can reassign. + const ancestors: { node: TraversableObject; context: TraverseContext }[] = []; + + function findAncestorCycle(node: TraversableObject): TraverseContext | null { + for (let i = ancestors.length - 1; i >= 0; i--) { + if (ancestors[i].node === node) { + return ancestors[i].context; + } + } + return null; + } function walk(node: unknown, path: (string | number)[] = [], parent?: TraverseContext): void { - // Check for circular references + if (visitedCount >= maxNodes) { + return; // Bound total work; stop once the node budget is exhausted. + } + + // Detect cycles via the current DFS path's original node refs. A shared + // (non-circular) reference appearing in multiple branches is traversed + // independently; the node/depth caps keep a DAG of shared references from + // fanning out unboundedly. let circular: TraverseContext | null = null; if (isObject(node)) { - if (visited.has(node)) { - // Find the circular reference in the parent chain - let p = parent; - while (p) { - if (p.node === node) { - circular = p; - break; - } - p = p.parent; - } - return; // Skip circular references + circular = findAncestorCycle(node); + if (circular) { + return; // Skip true cycles to avoid infinite recursion. } - visited.add(node); } const key = path.length > 0 ? path[path.length - 1] : undefined; const isRoot = path.length === 0; const level = path.length; + const tooDeep = level >= maxDepth; // Determine if this is a leaf node const isLeaf = + tooDeep || !isObject(node) || - (Array.isArray(node) && node.length === 0) || - Object.keys(node).length === 0; + (Array.isArray(node) ? node.length === 0 : !hasOwnEnumerable(node)); // Create context const context: TraverseContext = { @@ -149,19 +194,32 @@ function forEach(obj: unknown, callback: ForEachCallback): void { }; // Call the callback with the context + visitedCount++; callback.call(context, node); - // Traverse children if not circular and is an object - if (!circular && isObject(node) && !isLeaf) { + // Traverse children within bounds, breaking as soon as the budget is spent + // so a wide array/object can't incur O(n) work after the cap is reached. + if (isObject(node) && !isLeaf) { + ancestors.push({ node, context }); if (Array.isArray(node)) { for (let i = 0; i < node.length; i++) { + if (visitedCount >= maxNodes) { + break; + } walk(node[i], [...path, i], context); } } else { - for (const [childKey, childValue] of Object.entries(node)) { - walk(childValue, [...path, childKey], context); + for (const childKey in node) { + if (!Object.prototype.hasOwnProperty.call(node, childKey)) { + continue; + } + if (visitedCount >= maxNodes) { + break; + } + walk(node[childKey], [...path, childKey], context); } } + ancestors.pop(); } } @@ -169,10 +227,10 @@ function forEach(obj: unknown, callback: ForEachCallback): void { } // Main traverse function that returns an object with forEach method -export default function traverse(obj: unknown) { +export default function traverse(obj: unknown, options?: TraverseOptions) { return { forEach(callback: ForEachCallback): void { - forEach(obj, callback); + forEach(obj, callback, options); }, }; }