From e3ce30dfe8fb8f8d2b72ee2f9934333dd6534555 Mon Sep 17 00:00:00 2001 From: Sanjeev Sharma Date: Mon, 8 Jun 2026 20:42:20 +0530 Subject: [PATCH 1/8] fix(redact): make ** glob match zero segments so **.password redacts top-level keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pathToRegExp joined every segment with a literal `\.`, so `**.password` compiled to `/^.*\.password$/` — which requires a dot before `password` and therefore never matched a bare top-level `password` key. The JSDoc states `**` means "zero or more segments", so a top-level key (zero leading segments) must match. This was a PII leak: a top-level `password`/`token`/etc. under autoDetect stayed raw. `**` now absorbs the adjacent separator into an optional group, so it can collapse to zero segments at either end (`**.x`, `x.**`) and in the middle (`a.**.b` matches `a.b`). Consecutive `**` are coalesced. Segment boundaries are still respected (`**.password` does not match `passwordHint`). --- .../redact.utils.comprehensive.test.ts | 5 +- src/utils/__tests__/redact.utils.test.ts | 66 +++++++++++++++++++ src/utils/redact.utils.ts | 53 ++++++++++++--- 3 files changed, 113 insertions(+), 11 deletions(-) diff --git a/src/utils/__tests__/redact.utils.comprehensive.test.ts b/src/utils/__tests__/redact.utils.comprehensive.test.ts index 3c99029..06e0726 100644 --- a/src/utils/__tests__/redact.utils.comprehensive.test.ts +++ b/src/utils/__tests__/redact.utils.comprehensive.test.ts @@ -164,8 +164,9 @@ describe('applyRedaction', () => { }); describe('autoDetect: conservative', () => { - // Note: PII_CONSERVATIVE_PATHS uses `**.password` etc. which matches - // keys at any nested depth (e.g. user.password), not bare top-level keys. + // Note: PII_CONSERVATIVE_PATHS uses `**.password` etc. which matches the + // key at any depth — both nested (user.password) and bare top-level keys + // (`**` matches zero or more segments). it('redacts nested **.password fields', () => { const result = applyRedaction( { user: { password: 'secret' } }, diff --git a/src/utils/__tests__/redact.utils.test.ts b/src/utils/__tests__/redact.utils.test.ts index 9af5b0e..7157e4c 100644 --- a/src/utils/__tests__/redact.utils.test.ts +++ b/src/utils/__tests__/redact.utils.test.ts @@ -52,6 +52,72 @@ describe('redactObject — path-based redaction', () => { expect(result.x).toBe(CENSOR); }); + it('** matches zero or more segments — including a top-level key', () => { + // JSDoc for pathToRegExp: `**` matches "zero or more segments". + // '**.password' must therefore match a bare top-level `password` key + // (zero leading segments), not only nested ones. This is critical for + // PII safety: top-level `password` is the most common shape. + const obj = { password: 'hunter2', user: { password: 'nested' } }; + const result = redactObject(obj, { paths: ['**.password'] }); + expect(result.password).toBe(CENSOR); + expect((result.user as Record).password).toBe(CENSOR); + }); + + it('**.token matches a top-level token key', () => { + const result = redactObject({ token: 'tok-abc' }, { paths: ['**.token'] }); + expect(result.token).toBe(CENSOR); + }); + + it('**.password respects segment boundaries (does not match passwordHint)', () => { + // The trailing segment must match a WHOLE key, not a substring. + const result = redactObject( + { passwordHint: 'safe', password: 'x' }, + { paths: ['**.password'] } + ); + expect(result.passwordHint).toBe('safe'); + expect(result.password).toBe(CENSOR); + }); + + it('trailing ** matches the prefix key itself (zero trailing segments)', () => { + const obj = { req: { headers: { a: 'x' } } }; + const result = redactObject(obj, { paths: ['req.**'] }); + expect(result.req).toBe(CENSOR); + }); + + it('trailing ** also matches when the prefix is a leaf (req → redacted)', () => { + const result = redactObject({ req: 'leaf' }, { paths: ['req.**'] }); + expect(result.req).toBe(CENSOR); + }); + + it('middle ** matches zero intermediate segments (a.**.b matches a.b)', () => { + const obj = { a: { b: 'x' } }; + const result = redactObject(obj, { paths: ['a.**.b'] }); + expect((result.a as Record).b).toBe(CENSOR); + }); + + it('middle ** matches one or more intermediate segments (a.**.b matches a.x.b)', () => { + const obj = { a: { x: { b: 'deep' } } }; + const result = redactObject(obj, { paths: ['a.**.b'] }); + expect(((result.a as Record).x as Record).b).toBe(CENSOR); + }); + + it('collapses consecutive ** (a.**.**.b behaves like a.**.b)', () => { + const obj = { a: { b: 'zero', x: { b: 'one' } } }; + const result = redactObject(obj, { paths: ['a.**.**.b'] }); + const a = result.a as Record; + expect(a.b).toBe(CENSOR); + expect((a.x as Record).b).toBe(CENSOR); + }); + + it('collapses leading ** runs (**.**.token behaves like **.token)', () => { + const result = redactObject( + { token: 'top', a: { token: 'nested' } }, + { paths: ['**.**.token'] } + ); + expect(result.token).toBe(CENSOR); + expect((result.a as Record).token).toBe(CENSOR); + }); + it('redacts a deeply nested path (3+ levels)', () => { const obj = { a: { b: { c: { token: 'abc' } } } }; const result = redactObject(obj, { paths: ['a.b.c.token'] }); diff --git a/src/utils/redact.utils.ts b/src/utils/redact.utils.ts index d29e546..20651e6 100644 --- a/src/utils/redact.utils.ts +++ b/src/utils/redact.utils.ts @@ -143,7 +143,11 @@ function resolveConfig(config: RedactConfig): RedactConfig { /** * Convert a dot-notation path pattern to a RegExp. - * Supports `*` (one segment) and `**` (zero or more segments). + * Supports `*` (exactly one segment) and `**` (zero or more segments). + * + * Because `**` may match *zero* segments, it also absorbs the dot separator + * that joins it to its neighbour — so `**.password` matches both a bare + * top-level `password` (zero leading segments) and any nested `*.password`. * * Examples: * "password" → /^password$/ @@ -151,17 +155,48 @@ function resolveConfig(config: RedactConfig): RedactConfig { * "*.token" → /^[^.]+\.token$/ * "req.headers.*" → /^req\.headers\.[^.]+$/ * "**" → /^.*$/ + * "**.password" → /^(?:.*\.)?password$/ (matches `password` and `a.b.password`) + * "req.**" → /^req(?:\..*)?$/ (matches `req` and `req.a.b`) + * + * Consecutive `**` segments are collapsed (`a.**.**.b` ≡ `a.**.b`). */ function pathToRegExp(pattern: string): RegExp { - const regexStr = pattern + // Collapse runs of consecutive `**` — they are semantically redundant + // (`a.**.**.b` ≡ `a.**.b`) and would otherwise emit two optional groups that + // fight over the shared separator and fail to match zero-segment paths. + const segments = pattern .split('.') - .map((segment) => { - if (segment === '**') return '.*'; - if (segment === '*') return '[^.]+'; - // Escape regex special chars in the segment - return segment.replace(/[$()*+.?[\\\]^{|}]/g, '\\$&'); - }) - .join('\\.'); + .filter((seg, i, arr) => !(seg === '**' && arr[i - 1] === '**')); + const lastIndex = segments.length - 1; + + let regexStr = ''; + for (const [i, segment] of segments.entries()) { + // A `**` token emits an optional group that already contains the dot on + // one side, so the matching boundary separator must NOT be emitted here: + // - a non-final `**` (leading/middle) → group `(?:.*\.)?` owns its RIGHT dot + // - a trailing `**` → group `(?:\..*)?` owns its LEFT dot + // Every other boundary (including a middle `**`'s structural LEFT dot) is a + // literal `\.`. + const prevOwnsRightDot = i > 0 && segments[i - 1] === '**' && i - 1 < lastIndex; + const ownsLeftDot = segment === '**' && i === lastIndex && i > 0; + if (i > 0 && !prevOwnsRightDot && !ownsLeftDot) { + regexStr += String.raw`\.`; + } + + if (segment === '**') { + // `**` = zero or more path segments. + if (lastIndex === 0) { + regexStr += '.*'; // standalone `**` + } else if (i === lastIndex) { + regexStr += String.raw`(?:\..*)?`; // trailing `X.**` — optional ".segs" + } else { + regexStr += String.raw`(?:.*\.)?`; // leading/middle — optional "segs." + } + continue; + } + + regexStr += segment === '*' ? '[^.]+' : segment.replace(/[$()*+.?[\\\]^{|}]/g, '\\$&'); + } return new RegExp(`^${regexStr}$`); } From 6df0c2a3941a9893cae58be4131d6d72c8c61368 Mon Sep 17 00:00:00 2001 From: Sanjeev Sharma Date: Mon, 8 Jun 2026 20:42:30 +0530 Subject: [PATCH 2/8] fix(redact): apply autoDetect rules when no explicit paths or patterns set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _hasRedact short-circuited applyRedaction to a no-op unless redact.paths or redact.patterns was non-empty. A config of `redact: { autoDetect: 'aggressive' }` with no explicit paths/patterns left the flag false, so redaction never ran and all PII (passwords, emails, JWTs, …) was logged raw. autoDetect injects its built-in PII paths/patterns at redaction time, so it must also set the flag. --- .../logixia-logger.comprehensive.test.ts | 40 +++++++++++++++++++ src/core/logitron-logger.ts | 8 +++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/core/__tests__/logixia-logger.comprehensive.test.ts b/src/core/__tests__/logixia-logger.comprehensive.test.ts index 84ebe83..1392145 100644 --- a/src/core/__tests__/logixia-logger.comprehensive.test.ts +++ b/src/core/__tests__/logixia-logger.comprehensive.test.ts @@ -587,6 +587,46 @@ describe('setLevel', () => { }); }); +// ── Redaction integration (autoDetect-only config must apply) ───────────────── + +describe('redaction applied through the logger', () => { + it('applies autoDetect redaction even with no explicit paths/patterns (BUG 1)', async () => { + const out = spyOutput(); + const logger = new LogixiaLogger({ + ...BASE_CONFIG, + environment: 'production', + format: { json: true, colorize: false, timestamp: false }, + redact: { autoDetect: 'aggressive' }, + }); + await logger.info('t', { + password: 'hunter2', + email: 'x@y.com', + jwt: 'eyJhbGciOiJ.aaa.bbb', + }); + out.restore(); + const joined = out.joined(); + expect(joined).not.toContain('hunter2'); + expect(joined).not.toContain('x@y.com'); + expect(joined).not.toContain('eyJhbGciOiJ.aaa.bbb'); + expect(joined).toContain('[REDACTED]'); + }); + + it('redacts a top-level password key via conservative autoDetect (BUG 1 + BUG 2)', async () => { + const out = spyOutput(); + const logger = new LogixiaLogger({ + ...BASE_CONFIG, + format: { json: true, colorize: false, timestamp: false }, + redact: { autoDetect: 'conservative' }, + }); + await logger.info('t', { password: 'hunter2', keep: 'visible' }); + out.restore(); + const joined = out.joined(); + expect(joined).not.toContain('hunter2'); + expect(joined).toContain('visible'); + expect(joined).toContain('[REDACTED]'); + }); +}); + // ── Error object logging ────────────────────────────────────────────────────── describe('error() with Error object', () => { diff --git a/src/core/logitron-logger.ts b/src/core/logitron-logger.ts index 187c673..4aa9ccf 100644 --- a/src/core/logitron-logger.ts +++ b/src/core/logitron-logger.ts @@ -405,11 +405,15 @@ export class LogixiaLogger< this._formattedAppName = ''; } - // 6. Redact flag — if no redact config, skip applyRedaction entirely + // 6. Redact flag — if no redact config, skip applyRedaction entirely. + // autoDetect injects built-in PII paths/patterns at redaction time, so + // it must flip the flag on even with no explicit paths/patterns set — + // otherwise autoDetect-only configs silently leak PII. this._hasRedact = !!( this.config.redact && ((this.config.redact.paths?.length ?? 0) > 0 || - (this.config.redact.patterns?.length ?? 0) > 0) + (this.config.redact.patterns?.length ?? 0) > 0 || + !!this.config.redact.autoDetect) ); } From a7fb7f9a4cf54eb15dde59da14b063d378ff61d9 Mon Sep 17 00:00:00 2001 From: Sanjeev Sharma Date: Mon, 8 Jun 2026 20:45:55 +0530 Subject: [PATCH 3/8] fix(sampling): always emit WARN regardless of global rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SamplingConfig docs guarantee "ERROR and WARN default to 1.0 even when a lower global rate is set, unless you explicitly override them here." ALWAYS_EMIT_LEVELS listed only error and fatal, so WARN was sampled at the global rate — silently dropping warnings during incidents at low sample rates. Added warn to the set; an explicit perLevel.warn override still disables the guarantee as documented. --- .../sampling.utils.comprehensive.test.ts | 28 +++++++++++++++++++ src/utils/sampling.utils.ts | 5 +++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/utils/__tests__/sampling.utils.comprehensive.test.ts b/src/utils/__tests__/sampling.utils.comprehensive.test.ts index 4f579de..7b8d923 100644 --- a/src/utils/__tests__/sampling.utils.comprehensive.test.ts +++ b/src/utils/__tests__/sampling.utils.comprehensive.test.ts @@ -105,6 +105,34 @@ describe('Sampler', () => { expect(dropped).toBe(true); s.destroy(); }); + + it('warn always passes with any global rate (documented safety guarantee)', () => { + // SamplingConfig JSDoc: "ERROR and WARN default to 1.0 even when a lower + // global rate is set, unless you explicitly override them here." + const s = new Sampler({ rate: 0.001 }); + let allPass = true; + for (let i = 0; i < 50; i++) { + if (!s.shouldEmit('warn')) { + allPass = false; + break; + } + } + expect(allPass).toBe(true); + s.destroy(); + }); + + it('warn respects an explicit perLevel override', () => { + const s = new Sampler({ rate: 1.0, perLevel: { warn: 0.0 } }); + let dropped = false; + for (let i = 0; i < 50; i++) { + if (!s.shouldEmit('warn')) { + dropped = true; + break; + } + } + expect(dropped).toBe(true); + s.destroy(); + }); }); // ── Per-level overrides ──────────────────────────────────────────────────── diff --git a/src/utils/sampling.utils.ts b/src/utils/sampling.utils.ts index b8e57c4..561ccee 100644 --- a/src/utils/sampling.utils.ts +++ b/src/utils/sampling.utils.ts @@ -8,7 +8,10 @@ import type { SamplingConfig } from '../types'; // Safety-critical levels that are NEVER sampled below 100 % by default. -const ALWAYS_EMIT_LEVELS = new Set(['error', 'fatal']); +// Per SamplingConfig docs, ERROR and WARN are always emitted unless explicitly +// overridden via `perLevel`; `fatal` (a common custom highest-severity level) +// is included for the same reason. +const ALWAYS_EMIT_LEVELS = new Set(['error', 'warn', 'fatal']); // ── Sampler ─────────────────────────────────────────────────────────────────── From bc776d6580ba4db182dba981e29d99d4029d0daa Mon Sep 17 00:00:00 2001 From: Sanjeev Sharma Date: Tue, 9 Jun 2026 09:28:58 +0530 Subject: [PATCH 4/8] fix(logger): merge child/context data into log payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit child(context, data) stored the merged fields in contextData, but the hot-path merge in log() is gated on _hasContextData — which was initialised to false and never set true. Every field passed to child() (e.g. serviceVersion, region) was silently dropped from all logs. A redaction test passed only because the secret contextData was never emitted at all. child() now sets the flag and also inherits the parent's bound fields so nested children carry context. --- .../logixia-logger.comprehensive.test.ts | 16 ++++++ src/core/logitron-logger.ts | 49 ++++++++++--------- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/core/__tests__/logixia-logger.comprehensive.test.ts b/src/core/__tests__/logixia-logger.comprehensive.test.ts index 1392145..7a7e371 100644 --- a/src/core/__tests__/logixia-logger.comprehensive.test.ts +++ b/src/core/__tests__/logixia-logger.comprehensive.test.ts @@ -322,6 +322,22 @@ describe('Child logger', () => { expect(parsed.context).toBe('svc'); }); + it('child logger merges its context data into every log payload', async () => { + const out = spyOutput(); + const parent = new LogixiaLogger({ + ...BASE_CONFIG, + format: { json: true }, + levelOptions: { level: 'info' }, + }); + const child = parent.child('svc', { serviceVersion: '2.0', region: 'us-east' }); + await child.info('child msg', { adHoc: true }); + out.restore(); + const parsed = JSON.parse(out.joined()); + expect(parsed.payload.serviceVersion).toBe('2.0'); + expect(parsed.payload.region).toBe('us-east'); + expect(parsed.payload.adHoc).toBe(true); + }); + it('child logger does not affect parent context', () => { const parent = new LogixiaLogger({ ...BASE_CONFIG }); parent.child('child-ctx'); diff --git a/src/core/logitron-logger.ts b/src/core/logitron-logger.ts index 4aa9ccf..103067f 100644 --- a/src/core/logitron-logger.ts +++ b/src/core/logitron-logger.ts @@ -207,17 +207,15 @@ export class LogixiaLogger< const resolvedLevel = resolveInitialLevel(this.config); this.config.levelOptions = { ...this.config.levelOptions, level: resolvedLevel }; - if (!this.config.fields) { - this.config.fields = { - timestamp: '[yyyy-mm-dd HH:MM:ss.MS]', - level: '[log_level]', - appName: '[app_name]', - traceId: '[trace_id]', - message: '[message]', - payload: '[payload]', - timeTaken: '[time_taken_MS]', - }; - } + this.config.fields ??= { + timestamp: '[yyyy-mm-dd HH:MM:ss.MS]', + level: '[log_level]', + appName: '[app_name]', + traceId: '[trace_id]', + message: '[message]', + payload: '[payload]', + timeTaken: '[time_taken_MS]', + }; this.context = context ?? ''; @@ -272,7 +270,7 @@ export class LogixiaLogger< if (!shutdownCfg) return; const normalized: GracefulShutdownConfig = - shutdownCfg === true ? { enabled: true } : (shutdownCfg as GracefulShutdownConfig); + shutdownCfg === true ? { enabled: true } : shutdownCfg; if (!normalized.enabled) return; @@ -306,9 +304,7 @@ export class LogixiaLogger< */ private _buildPerfCaches(): void { // 1. Level value map — pre-merge built-ins + custom levels once - const customLevelEntries = Object.entries( - (this.config.levelOptions?.levels ?? {}) as Record - ); + const customLevelEntries = Object.entries(this.config.levelOptions?.levels ?? {}); this._levelValues = new Map([ [LogLevel.ERROR, 0], [LogLevel.WARN, 1], @@ -356,10 +352,10 @@ export class LogixiaLogger< for (const f of fieldNames) { if (this.fieldState.has(f)) { this._fieldCache.set(f, this.fieldState.get(f)!); - } else if (this.config.fields?.[f as keyof typeof this.config.fields] !== undefined) { - this._fieldCache.set(f, this.config.fields[f as keyof typeof this.config.fields] !== false); - } else { + } else if (this.config.fields?.[f as keyof typeof this.config.fields] === undefined) { this._fieldCache.set(f, true); + } else { + this._fieldCache.set(f, this.config.fields[f as keyof typeof this.config.fields] !== false); } } @@ -397,12 +393,12 @@ export class LogixiaLogger< // 5. Pre-computed "[appName] " string (gray when colorize is on) const appNameRaw = `[${this.config.appName ?? 'App'}]`; - if (this._fieldCache.get('appName') !== false) { + if (this._fieldCache.get('appName') === false) { + this._formattedAppName = ''; + } else { this._formattedAppName = colorize ? `${this._colorMap.get('gray')!}${appNameRaw}${this._colorMap.get('reset')!} ` : `${appNameRaw} `; - } else { - this._formattedAppName = ''; } // 6. Redact flag — if no redact config, skip applyRedaction entirely. @@ -495,7 +491,7 @@ export class LogixiaLogger< setLevel(level: LogLevelString): void { this.config.levelOptions = this.config.levelOptions ?? {}; - this.config.levelOptions.level = level as string; + this.config.levelOptions.level = level; // Refresh the cached numeric threshold so shouldLog() stays accurate this._minLevelValue = this._levelValues.get(level) ?? this._minLevelValue; // Rebuild level string cache in case colours are keyed per-level @@ -599,7 +595,14 @@ export class LogixiaLogger< child(context: string, data?: Record): ILogger { const childLogger = new LogixiaLogger(this.config, context); - if (data) childLogger.contextData = { ...this.contextData, ...data }; + // Inherit the parent's bound fields plus any child-specific data. The + // hot-path merge in log() is gated on _hasContextData, so it must be set + // here or the merged fields are silently dropped from every log. + const mergedContextData = { ...this.contextData, ...data }; + if (Object.keys(mergedContextData).length > 0) { + childLogger.contextData = mergedContextData; + childLogger._hasContextData = true; + } return childLogger; } From 68be1bf1fa15609bde6cde9be7e92da4a9a80315 Mon Sep 17 00:00:00 2001 From: Sanjeev Sharma Date: Tue, 9 Jun 2026 09:54:53 +0530 Subject: [PATCH 5/8] fix(plugins): isolate throwing onLog and invoke onError on transport failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two plugin contract bugs: 1. runOnLog had no error isolation, while every sibling hook (onInit, onError, onShutdown) swallows errors. A plugin throwing in onLog rejected the whole log() call — and since logger.info() is usually not awaited, that surfaced as an unhandled rejection that can crash the process. onLog errors are now caught and the chain continues with the previous entry; documented in the JSDoc. 2. The onError hook ("called when a transport write fails") was never invoked — runOnError had zero call sites, so the documented alerting use case was dead. output() now calls runOnError in the transport-failure catch, then still falls through to the stdout/stderr fallback so the log is not lost. --- src/__tests__/plugin.comprehensive.test.ts | 78 ++++++++++++++++++++++ src/core/logitron-logger.ts | 9 +++ src/plugin.ts | 17 ++++- 3 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/__tests__/plugin.comprehensive.test.ts b/src/__tests__/plugin.comprehensive.test.ts index 46e4f09..3ccd584 100644 --- a/src/__tests__/plugin.comprehensive.test.ts +++ b/src/__tests__/plugin.comprehensive.test.ts @@ -11,6 +11,7 @@ * - Logger.use() and Logger.unuse() */ +import { LogixiaLogger } from '../core/logitron-logger'; import type { LogixiaPlugin } from '../plugin'; import { globalPluginRegistry, PluginRegistry, usePlugin } from '../plugin'; import type { LogEntry } from '../types'; @@ -260,6 +261,38 @@ describe('PluginRegistry', () => { expect(result!.payload?.step).toBe(1); expect(result!.payload?.step2).toBe(2); }); + + it('isolates a throwing onLog plugin and keeps processing (no crash)', async () => { + registry.register({ + name: 'thrower', + onLog() { + throw new Error('plugin blew up'); + }, + }); + registry.register({ + name: 'after', + onLog(e) { + return { ...e, payload: { ...e.payload, reached: true } }; + }, + }); + const entry = makeEntry(); + // A buggy plugin must not crash logging; the chain continues with the + // un-transformed entry, and later plugins still run. + const result = await registry.runOnLog(entry); + expect(result).not.toBeNull(); + expect(result!.payload?.reached).toBe(true); + }); + + it('isolates a rejecting async onLog plugin', async () => { + registry.register({ + name: 'async-thrower', + async onLog() { + await Promise.resolve(); + throw new Error('async plugin blew up'); + }, + }); + await expect(registry.runOnLog(makeEntry())).resolves.not.toBeNull(); + }); }); // ── runOnError ─────────────────────────────────────────────────────────────── @@ -425,3 +458,48 @@ describe('Full plugin lifecycle', () => { expect(events).toEqual(['init', 'log', 'error', 'shutdown']); }); }); + +// ── Logger ↔ plugin integration ─────────────────────────────────────────────── + +describe('Logger plugin integration', () => { + const BASE = { + appName: 'PluginIT', + environment: 'test' as const, + format: { timestamp: false, colorize: false, json: false }, + traceId: false, + levelOptions: { level: 'info' as const }, + }; + + it('invokes a plugin onError hook when a transport write fails', async () => { + const logger = new LogixiaLogger({ ...BASE, transports: { console: { format: 'json' } } }); + const seen: Error[] = []; + logger.use({ + name: 'err-capture', + onError(e) { + seen.push(e); + }, + }); + + // Force the transport write to fail. + const tm = (logger as unknown as { transportManager: { write: () => Promise } }) + .transportManager; + const boom = new Error('transport exploded'); + tm.write = () => Promise.reject(boom); + + await logger.info('will fail to write'); + + expect(seen).toHaveLength(1); + expect(seen[0]).toBe(boom); + }); + + it('a throwing onLog plugin does not crash a logger.info call', async () => { + const logger = new LogixiaLogger({ ...BASE }); + logger.use({ + name: 'bad', + onLog() { + throw new Error('plugin crash'); + }, + }); + await expect(logger.info('still works')).resolves.toBeUndefined(); + }); +}); diff --git a/src/core/logitron-logger.ts b/src/core/logitron-logger.ts index 103067f..1befaf8 100644 --- a/src/core/logitron-logger.ts +++ b/src/core/logitron-logger.ts @@ -841,6 +841,15 @@ export class LogixiaLogger< return; } catch (error) { internalError('Transport write failed', error); + // Notify plugin onError hooks (e.g. Sentry/alerting). runOnError + // swallows hook errors internally, so this can never re-throw. + if (this._pluginRegistry.size > 0) { + await this._pluginRegistry.runOnError( + error instanceof Error ? error : new Error(String(error)), + entry + ); + } + // Fall through to the stdout/stderr fallback so the log is not lost. } } diff --git a/src/plugin.ts b/src/plugin.ts index a577c84..9608a83 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -54,6 +54,9 @@ export interface LogixiaPlugin { * * - Return the entry (modified or unchanged) to continue processing. * - Return `null` to **drop** the entry — no transport will receive it. + * + * Errors thrown inside `onLog` are swallowed so a buggy plugin cannot crash + * logging; the chain continues with the previous (un-transformed) entry. */ onLog?(entry: LogEntry): LogEntry | null | Promise; @@ -117,11 +120,19 @@ export class PluginRegistry { * (possibly transformed) entry. */ async runOnLog(entry: LogEntry): Promise { - let current: LogEntry | null = entry; + let current: LogEntry = entry; for (const plugin of this._plugins) { if (!plugin.onLog) continue; - current = await plugin.onLog(current); - if (current === null) return null; + let next: LogEntry | null; + try { + next = await plugin.onLog(current); + } catch { + // Isolate buggy plugins: a throwing onLog must not crash logging. + // Skip this plugin and continue the chain with the un-transformed entry. + continue; + } + if (next === null) return null; + current = next; } return current; } From c0108a942a242918e4050cc74ad523f5eb3b6181 Mon Sep 17 00:00:00 2001 From: Sanjeev Sharma Date: Tue, 9 Jun 2026 10:11:39 +0530 Subject: [PATCH 6/8] fix(console): compact JSON, sanitize level for CWE-117, route warn to stderr Three ConsoleTransport contract bugs: - JSON mode used JSON.stringify(obj, null, 2), emitting multi-line pretty JSON that breaks line-based log collectors; the docs promise "compact JSON mode". Now single-line. - The level field was printed without sanitize() while message/context/traceId were sanitized. Custom levels are user-controlled, so an ANSI/control sequence in a level name could inject into the operator terminal (CWE-117). Level is now sanitized too. - warn was written to stdout though the class doc says stderr handles error/warn. warn now routes to stderr. --- .../__tests__/console.transport.test.ts | 112 ++++++++++++++++++ src/transports/console.transport.ts | 35 +++--- 2 files changed, 130 insertions(+), 17 deletions(-) create mode 100644 src/transports/__tests__/console.transport.test.ts diff --git a/src/transports/__tests__/console.transport.test.ts b/src/transports/__tests__/console.transport.test.ts new file mode 100644 index 0000000..346caf5 --- /dev/null +++ b/src/transports/__tests__/console.transport.test.ts @@ -0,0 +1,112 @@ +/** + * Tests for ConsoleTransport + * + * Covers the documented contract: + * - "compact JSON mode" (one line per entry — line-based collectors) + * - CWE-117 control-char stripping applied to ALL user-controlled text in text + * mode, including the (possibly custom) level name + * - stdout/stderr routing + */ + +import type { TransportLogEntry } from '../../types/transport.types'; +import { ConsoleTransport } from '../console.transport'; + +function makeEntry(overrides: Partial = {}): TransportLogEntry { + return { + timestamp: new Date('2026-01-01T00:00:00.000Z'), + level: 'info', + message: 'hello', + appName: 'TestApp', + environment: 'test', + ...overrides, + }; +} + +function capture(): { + out: string[]; + err: string[]; + restore: () => void; +} { + const out: string[] = []; + const err: string[] = []; + const origOut = process.stdout.write.bind(process.stdout); + const origErr = process.stderr.write.bind(process.stderr); + (process.stdout as NodeJS.WriteStream).write = (chunk: unknown) => { + out.push(String(chunk ?? '')); + return true; + }; + (process.stderr as NodeJS.WriteStream).write = (chunk: unknown) => { + err.push(String(chunk ?? '')); + return true; + }; + return { + out, + err, + restore() { + (process.stdout as NodeJS.WriteStream).write = origOut as typeof process.stdout.write; + (process.stderr as NodeJS.WriteStream).write = origErr as typeof process.stderr.write; + }, + }; +} + +describe('ConsoleTransport — JSON mode', () => { + it('emits compact single-line JSON (no pretty-print newlines)', async () => { + const cap = capture(); + const t = new ConsoleTransport({ format: 'json', colorize: false }); + await t.write(makeEntry({ data: { a: 1, b: 2 } })); + cap.restore(); + + const line = cap.out.join(''); + // One trailing newline only — the JSON body itself must not contain newlines. + const body = line.replace(/\n$/, ''); + expect(body.includes('\n')).toBe(false); + // Still valid JSON. + const parsed = JSON.parse(body); + expect(parsed.message).toBe('hello'); + expect(parsed.a).toBe(1); + }); +}); + +describe('ConsoleTransport — CWE-117 sanitization', () => { + it('strips control characters from a custom level name in text mode', async () => { + const cap = capture(); + const t = new ConsoleTransport({ format: 'text', colorize: false }); + // A custom level carrying an ANSI escape — must not reach the terminal raw. + await t.write(makeEntry({ level: 'kafka\x1b[31m\x07' })); + cap.restore(); + + const line = cap.out.join(''); + expect(line).not.toContain('\x1b[31m'); + expect(line).not.toContain('\x07'); + expect(line).toContain('KAFKA'); + }); +}); + +describe('ConsoleTransport — stream routing', () => { + it('writes error entries to stderr', async () => { + const cap = capture(); + const t = new ConsoleTransport({ format: 'text', colorize: false }); + await t.write(makeEntry({ level: 'error', message: 'boom' })); + cap.restore(); + expect(cap.err.join('')).toContain('boom'); + expect(cap.out.join('')).toBe(''); + }); + + it('writes warn entries to stderr (documented: stderr handles error/warn)', async () => { + const cap = capture(); + const t = new ConsoleTransport({ format: 'text', colorize: false }); + await t.write(makeEntry({ level: 'warn', message: 'careful' })); + cap.restore(); + expect(cap.err.join('')).toContain('careful'); + expect(cap.out.join('')).toBe(''); + }); + + it('writes info entries to stdout', async () => { + const cap = capture(); + const t = new ConsoleTransport({ format: 'text', colorize: false }); + await t.write(makeEntry({ level: 'info', message: 'fyi' })); + cap.restore(); + expect(cap.out.join('')).toContain('fyi'); + expect(cap.err.join('')).toBe(''); + }); +}); diff --git a/src/transports/console.transport.ts b/src/transports/console.transport.ts index edb4175..3b983ca 100644 --- a/src/transports/console.transport.ts +++ b/src/transports/console.transport.ts @@ -62,27 +62,27 @@ export class ConsoleTransport implements ITransport { const formatted = this.formatEntry(entry) + '\n'; // Use process.stderr for errors, stdout for everything else — // avoids the extra indirection that console.log/error add. - const out = entry.level.toLowerCase() === 'error' ? process.stderr : process.stdout; + const lvl = entry.level.toLowerCase(); + // Per the class contract, error AND warn go to stderr; everything else stdout. + const out = lvl === 'error' || lvl === 'warn' ? process.stderr : process.stdout; out.write(formatted); return Promise.resolve(); } private formatEntry(entry: TransportLogEntry): string { if (this.config.format === 'json') { - return JSON.stringify( - { - timestamp: this.config.timestamp !== false ? entry.timestamp.toISOString() : undefined, - level: entry.level, - message: entry.message, - ...(entry.data || {}), - context: entry.context, - traceId: entry.traceId, - appName: entry.appName, - environment: entry.environment, - }, - null, - 2 - ); + // Compact (single-line) JSON so line-based log collectors can parse one + // entry per line. JSON.stringify already escapes control chars. + return JSON.stringify({ + timestamp: this.config.timestamp !== false ? entry.timestamp.toISOString() : undefined, + level: entry.level, + message: entry.message, + ...(entry.data || {}), + context: entry.context, + traceId: entry.traceId, + appName: entry.appName, + environment: entry.environment, + }); } // Text format @@ -94,8 +94,9 @@ export class ConsoleTransport implements ITransport { parts.push(this.colorize(timestamp, 'gray')); } - // Level - const level = entry.level.toUpperCase().padEnd(5); + // Level — sanitized too: custom levels are user-controlled and could carry + // ANSI escapes / control chars (CWE-117). + const level = ConsoleTransport.sanitize(entry.level).toUpperCase().padEnd(5); const coloredLevel = this.colorize(level, this.getLevelColor(entry.level)); parts.push(coloredLevel); From c74ae42b247efe0625bd656f4d169a717fe51e60 Mon Sep 17 00:00:00 2001 From: Sanjeev Sharma Date: Tue, 9 Jun 2026 10:29:25 +0530 Subject: [PATCH 7/8] fix(trace): parse W3C traceId out of the traceparent header traceparent is the highest-priority default propagation header and documented as W3C/OTel, but extractTraceId returned the whole header value (00---) as the trace ID, so correlation with upstream OTel-instrumented services was broken. Now the 32-hex traceId segment is parsed out when the value is a well-formed traceparent; non-W3C values pass through unchanged for backward compatibility. --- .../trace.utils.comprehensive.test.ts | 17 +++++++++++++++++ src/utils/trace.utils.ts | 18 ++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/utils/__tests__/trace.utils.comprehensive.test.ts b/src/utils/__tests__/trace.utils.comprehensive.test.ts index aad4e82..858a969 100644 --- a/src/utils/__tests__/trace.utils.comprehensive.test.ts +++ b/src/utils/__tests__/trace.utils.comprehensive.test.ts @@ -155,6 +155,23 @@ describe('extractTraceId', () => { expect(extractTraceId(req, { header: 'traceparent' })).toBe('tid-0'); }); + it('parses the trace-id segment out of a W3C traceparent header', () => { + // W3C format: version-traceId(32hex)-spanId(16hex)-flags + const req = { + headers: { + traceparent: '00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01', + }, + }; + expect(extractTraceId(req, { header: 'traceparent' })).toBe( + '4bf92f3577b34da6a3ce929d0e0e4736' + ); + }); + + it('falls back to the raw value when traceparent is not W3C-formatted', () => { + const req = { headers: { traceparent: 'not-a-w3c-value' } }; + expect(extractTraceId(req, { header: 'traceparent' })).toBe('not-a-w3c-value'); + }); + it('returns undefined when header is absent', () => { const req = { headers: { other: 'value' } }; expect(extractTraceId(req, { header: 'x-trace-id' })).toBeUndefined(); diff --git a/src/utils/trace.utils.ts b/src/utils/trace.utils.ts index 8c38a8d..bff3a5c 100644 --- a/src/utils/trace.utils.ts +++ b/src/utils/trace.utils.ts @@ -215,6 +215,19 @@ function toValidTraceId(value: unknown): string | undefined { return trimmed.length > 0 ? trimmed : undefined; } +/** W3C trace-context: `version-traceId(32hex)-spanId(16hex)-flags(2hex)`. */ +const W3C_TRACEPARENT_RE = /^[\da-f]{2}-([\da-f]{32})-[\da-f]{16}-[\da-f]{2}$/i; + +/** + * Extract the 32-hex trace-id segment from a W3C `traceparent` header value. + * Returns the parsed trace ID, or the original value unchanged when it is not a + * well-formed traceparent (so non-W3C propagation still works). + */ +function parseTraceparent(value: string): string { + const m = W3C_TRACEPARENT_RE.exec(value); + return m ? m[1]! : value; +} + /** Extract trace ID from request using configuration (header → query → body → params). */ export function extractTraceId( request: unknown, @@ -225,8 +238,9 @@ export function extractTraceId( if (config.header) { const headers = Array.isArray(config.header) ? config.header : [config.header]; for (const header of headers) { - const value = toValidTraceId(req.headers?.[header.toLowerCase()]); - if (value) return value; + const lower = header.toLowerCase(); + const value = toValidTraceId(req.headers?.[lower]); + if (value) return lower === 'traceparent' ? parseTraceparent(value) : value; } } From e30588dea1cb1b473162ce6f100f20ecdf21b09c Mon Sep 17 00:00:00 2001 From: Sanjeev Sharma Date: Tue, 9 Jun 2026 10:41:54 +0530 Subject: [PATCH 8/8] fix(shutdown): register flush handler even when other signal listeners exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit flushOnExit guarded handler registration on process.listenerCount(signal) === 0, so whenever any other SIGTERM/SIGINT listener existed (NestJS, Express, app code — the common case) logixia attached nothing and the documented flush-on-exit silently never ran, losing the last logs on deploy. Signals support multiple listeners, so logixia now registers its own handler unconditionally; the shutdownHandlerRegistered flag keeps it idempotent, and resetShutdownHandlers detaches exactly our handler. Updated the test that codified the old skip. --- src/utils/__tests__/shutdown.utils.test.ts | 24 +++++++++++++++++++--- src/utils/shutdown.utils.ts | 22 ++++++++++++++++---- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/utils/__tests__/shutdown.utils.test.ts b/src/utils/__tests__/shutdown.utils.test.ts index 4308eec..921ec26 100644 --- a/src/utils/__tests__/shutdown.utils.test.ts +++ b/src/utils/__tests__/shutdown.utils.test.ts @@ -235,20 +235,38 @@ describe('flushOnExit', () => { process.removeAllListeners('SIGUSR2'); }); - it('does not attach SIGTERM listener if another listener already exists', async () => { + it('still flushes when another SIGTERM listener already exists', async () => { + // A framework (NestJS/Express) commonly registers its own signal handler. + // logixia must still attach its own so the documented flush-on-exit runs — + // signals support multiple listeners. const preExistingHandler = jest.fn(); process.on('SIGTERM', preExistingHandler); + const logger = makeFakeLogger(); + registerForShutdown(logger); + const countBefore = process.listenerCount('SIGTERM'); flushOnExit({ timeout: 1000 }); const countAfter = process.listenerCount('SIGTERM'); - // flushOnExit skips registration when listener count > 0 - expect(countAfter).toBe(countBefore); + // logixia adds exactly one handler on top of the pre-existing one. + expect(countAfter).toBe(countBefore + 1); + + await emitSignal('SIGTERM'); + expect(logger.close).toHaveBeenCalledTimes(1); process.removeListener('SIGTERM', preExistingHandler); }); + it('does not register its own handler twice across repeated flushOnExit calls', () => { + const before = process.listenerCount('SIGTERM'); + flushOnExit({ timeout: 1000 }); + flushOnExit({ timeout: 1000 }); + flushOnExit({ timeout: 1000 }); + // Idempotent: only one logixia handler regardless of call count. + expect(process.listenerCount('SIGTERM')).toBe(before + 1); + }); + it('a failing logger close does not prevent other loggers from being flushed', async () => { const failing = makeFakeLogger(() => Promise.reject(new Error('flush failure'))); const healthy = makeFakeLogger(); diff --git a/src/utils/shutdown.utils.ts b/src/utils/shutdown.utils.ts index 60018b7..6981b54 100644 --- a/src/utils/shutdown.utils.ts +++ b/src/utils/shutdown.utils.ts @@ -22,6 +22,9 @@ type Closeable = { close(): Promise }; /** Module-level registry of all logger instances that have opted into graceful shutdown */ const registry = new Set(); let shutdownHandlerRegistered = false; +/** Our own handler + the signals we attached it to, so reset() can detach exactly it. */ +let activeHandler: ((signal: NodeJS.Signals) => void | Promise) | undefined; +let activeSignals: NodeJS.Signals[] = []; /** * Register a logger instance so it is included in the graceful shutdown flush. @@ -76,11 +79,15 @@ export function flushOnExit(options: FlushOnExitOptions = {}): void { } }; + // Register our own handler unconditionally. Signals support multiple + // listeners, so other frameworks' handlers (NestJS/Express) don't block ours — + // the previous `listenerCount === 0` guard meant the documented flush silently + // never ran whenever any other listener existed. The `shutdownHandlerRegistered` + // flag above keeps THIS function idempotent (one logixia handler, not N). + activeHandler = handler; + activeSignals = signals; for (const signal of signals) { - // Only add if not already handled (avoids double-handling in long-lived processes) - if (process.listenerCount(signal) === 0) { - process.on(signal, handler); - } + process.on(signal, activeHandler); } } @@ -90,5 +97,12 @@ export function flushOnExit(options: FlushOnExitOptions = {}): void { */ export function resetShutdownHandlers(): void { registry.clear(); + if (activeHandler) { + for (const signal of activeSignals) { + process.removeListener(signal, activeHandler); + } + } + activeHandler = undefined; + activeSignals = []; shutdownHandlerRegistered = false; }