From 7d199ee1eea1e5c24c0305e7811a67f0fe7b8b82 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 16 Jul 2026 16:13:00 +0200 Subject: [PATCH 1/4] refactor(log): source sensitive redaction markers from config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lib/ must stay module-agnostic — SENSITIVE_PATH_MARKERS and SENSITIVE_QUERY_KEYS in lib/helpers/redactUrl.js hardcoded auth/invitations route vocabulary owned by feature modules (#3935). Move both lists to config.log.sensitivePathMarkers / config.log.sensitiveQueryKeys (current values as defaults in config/defaults/development.config.js) so a module extends redaction coverage from its own config without editing shared lib/. redactUrl.js reads the lists from config once at module load, falling back to the built-in literals when config or the key is absent — redaction degrades safely instead of silently redacting nothing in an edge context. Exports/API unchanged (existing consumers: lib/middlewares/analytics.js, lib/services/express.js). Closes #3953 Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup --- config/defaults/development.config.js | 9 ++++ lib/helpers/redactUrl.js | 29 +++++++++++- lib/helpers/tests/redactUrl.unit.tests.js | 54 ++++++++++++++++++++++- 3 files changed, 89 insertions(+), 3 deletions(-) diff --git a/config/defaults/development.config.js b/config/defaults/development.config.js index badc71e93..0d585f2f1 100644 --- a/config/defaults/development.config.js +++ b/config/defaults/development.config.js @@ -81,6 +81,15 @@ const config = { maxFiles: 2, json: false, }, + // Path segments consumed by `lib/helpers/redactUrl.js` (`redactPathSecrets`) + // to redact single-use secrets carried as a PATH parameter (e.g. + // `/api/auth/reset/:token`). A module that adds its own token-bearing route + // extends this list from its own config instead of editing shared lib/. + sensitivePathMarkers: ['reset', 'verify', 'verify-email'], + // Query-string parameter names consumed by `lib/helpers/redactUrl.js` + // (`redactUrl`) to redact single-use secrets carried in the query string + // (e.g. `POST /api/auth/signup?inviteToken=…`). + sensitiveQueryKeys: ['inviteToken'], }, csrf: { csrf: false, diff --git a/lib/helpers/redactUrl.js b/lib/helpers/redactUrl.js index eb35dd9c4..68eeb1d8e 100644 --- a/lib/helpers/redactUrl.js +++ b/lib/helpers/redactUrl.js @@ -1,3 +1,20 @@ +/** + * Module dependencies. + */ +import config from '../../config/index.js'; + +/** + * Built-in fallback values. `lib/` must stay module-agnostic — the real source + * of truth is `config.log.sensitiveQueryKeys` / `config.log.sensitivePathMarkers` + * (see below), which a module extends from its own config without editing this + * shared helper. These literals are the ultimate fallback: if config is absent + * or does not define the key (e.g. an edge context that builds a partial config + * object), redaction still degrades safely to the current known-sensitive + * routes instead of silently redacting nothing. + */ +const DEFAULT_SENSITIVE_QUERY_KEYS = ['inviteToken']; +const DEFAULT_SENSITIVE_PATH_MARKERS = ['reset', 'verify', 'verify-email']; + /** * Sensitive query-string parameters that must never reach the request log. * @@ -5,8 +22,12 @@ * (the Vue client puts it there, not in the body). The morgan log pattern logs * `:url`, so without redaction a live single-use invite token lands in prod logs * (and any log shipper / aggregator downstream). Redact it to `REDACTED`. + * + * Sourced from `config.log.sensitiveQueryKeys` (extendable per-module without + * editing this shared helper); falls back to `DEFAULT_SENSITIVE_QUERY_KEYS` + * when config/the key is absent so redaction never silently degrades to nothing. */ -const SENSITIVE_QUERY_KEYS = ['inviteToken']; +const SENSITIVE_QUERY_KEYS = config?.log?.sensitiveQueryKeys ?? DEFAULT_SENSITIVE_QUERY_KEYS; /** * Path segments that are immediately followed by a single-use secret carried as @@ -15,8 +36,12 @@ const SENSITIVE_QUERY_KEYS = ['inviteToken']; * `GET /api/*(auth/)?invitations/verify/:token` embed a still-valid token * directly in the path, so the segment right after one of these markers must be * redacted before the URL reaches any log/analytics store. + * + * Sourced from `config.log.sensitivePathMarkers` (extendable per-module without + * editing this shared helper); falls back to `DEFAULT_SENSITIVE_PATH_MARKERS` + * when config/the key is absent so redaction never silently degrades to nothing. */ -const SENSITIVE_PATH_MARKERS = ['reset', 'verify', 'verify-email']; +const SENSITIVE_PATH_MARKERS = config?.log?.sensitivePathMarkers ?? DEFAULT_SENSITIVE_PATH_MARKERS; /** * @desc Redact single-use secrets embedded as PATH parameters. Any segment that diff --git a/lib/helpers/tests/redactUrl.unit.tests.js b/lib/helpers/tests/redactUrl.unit.tests.js index d069382c8..829cf1041 100644 --- a/lib/helpers/tests/redactUrl.unit.tests.js +++ b/lib/helpers/tests/redactUrl.unit.tests.js @@ -1,4 +1,4 @@ -import { describe, test, expect } from '@jest/globals'; +import { jest, describe, test, expect } from '@jest/globals'; import redactUrl, { SENSITIVE_QUERY_KEYS, SENSITIVE_PATH_MARKERS, redactPathSecrets } from '../redactUrl.js'; describe('redactUrl', () => { @@ -113,3 +113,55 @@ describe('redactPathSecrets', () => { expect(SENSITIVE_PATH_MARKERS).toEqual(expect.arrayContaining(['reset', 'verify', 'verify-email'])); }); }); + +/** + * `SENSITIVE_QUERY_KEYS` / `SENSITIVE_PATH_MARKERS` are read from + * `config.log.*` once, at module-evaluation time (#3953). These tests mock + * `config/index.js` and re-import the module fresh (via `jest.resetModules()` + * + dynamic `import()` — see `posthog-context.middleware.unit.tests.js` for + * the same pattern) to exercise both the config-honoring path and the + * fail-safe fallback path, without disturbing the static-import tests above + * (which exercise the real config's current defaults, unchanged). + */ +describe('redactUrl config sourcing (#3953)', () => { + /** + * Load a fresh instance of the module under a given mocked config. + * @param {Object|undefined} mockConfig - the value `config/index.js` default-exports + * @returns {Promise} the freshly-loaded module exports + */ + const loadWithConfig = async (mockConfig) => { + jest.resetModules(); + jest.unstable_mockModule('../../../config/index.js', () => ({ default: mockConfig })); + return import('../redactUrl.js'); + }; + + test('honors a module-extended path marker from config', async () => { + const mod = await loadWithConfig({ + log: { sensitivePathMarkers: ['reset', 'verify', 'verify-email', 'unsubscribe'] }, + }); + expect(mod.SENSITIVE_PATH_MARKERS).toContain('unsubscribe'); + expect(mod.redactPathSecrets('/api/newsletter/unsubscribe/SECRETTOKEN')).toBe('/api/newsletter/unsubscribe/REDACTED'); + }); + + test('honors a module-extended query key from config', async () => { + const mod = await loadWithConfig({ + log: { sensitiveQueryKeys: ['inviteToken', 'magicToken'] }, + }); + expect(mod.SENSITIVE_QUERY_KEYS).toContain('magicToken'); + expect(mod.default('/x?magicToken=secret')).toBe('/x?magicToken=REDACTED'); + }); + + test('falls back to the built-in defaults when config.log is missing', async () => { + const mod = await loadWithConfig({}); + expect(mod.SENSITIVE_PATH_MARKERS).toEqual(['reset', 'verify', 'verify-email']); + expect(mod.SENSITIVE_QUERY_KEYS).toEqual(['inviteToken']); + expect(mod.redactPathSecrets('/api/auth/reset/SECRETTOKEN')).toBe('/api/auth/reset/REDACTED'); + expect(mod.default('/x?inviteToken=secret')).toBe('/x?inviteToken=REDACTED'); + }); + + test('falls back to the built-in defaults when config itself is undefined', async () => { + const mod = await loadWithConfig(undefined); + expect(mod.SENSITIVE_PATH_MARKERS).toEqual(['reset', 'verify', 'verify-email']); + expect(mod.SENSITIVE_QUERY_KEYS).toEqual(['inviteToken']); + }); +}); From e88cebf2668888c4286b77d0bf602d9276171b92 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 16 Jul 2026 16:49:24 +0200 Subject: [PATCH 2/4] fix(log): union config-provided redaction lists with built-in defaults deepMerge replaces arrays (no union), so a module-level config.log.sensitivePathMarkers/sensitiveQueryKeys extension clobbered the global default instead of extending it, and a `??` fallback let an explicit empty array silently disable redaction for that vector. Compute the effective lists as a Set-deduped union of the built-in DEFAULT_SENSITIVE_* base with config.log.*, so config is purely additive from any layer and redaction can never be disabled via config. development.config.js's entries become empty arrays (the extension point), killing the literal duplication with redactUrl.js. Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup --- config/defaults/development.config.js | 14 ++++--- lib/helpers/redactUrl.js | 35 ++++++++++-------- lib/helpers/tests/redactUrl.unit.tests.js | 45 +++++++++++++++++------ 3 files changed, 62 insertions(+), 32 deletions(-) diff --git a/config/defaults/development.config.js b/config/defaults/development.config.js index 0d585f2f1..50b4c9864 100644 --- a/config/defaults/development.config.js +++ b/config/defaults/development.config.js @@ -83,13 +83,17 @@ const config = { }, // Path segments consumed by `lib/helpers/redactUrl.js` (`redactPathSecrets`) // to redact single-use secrets carried as a PATH parameter (e.g. - // `/api/auth/reset/:token`). A module that adds its own token-bearing route - // extends this list from its own config instead of editing shared lib/. - sensitivePathMarkers: ['reset', 'verify', 'verify-email'], + // `/api/auth/reset/:token`). Extension-only: unioned with the built-in + // defaults in lib/helpers/redactUrl.js, so a module adds its own + // token-bearing route here without editing shared lib/ or duplicating + // ['reset', 'verify', 'verify-email'], which already apply unconditionally. + sensitivePathMarkers: [], // Query-string parameter names consumed by `lib/helpers/redactUrl.js` // (`redactUrl`) to redact single-use secrets carried in the query string - // (e.g. `POST /api/auth/signup?inviteToken=…`). - sensitiveQueryKeys: ['inviteToken'], + // (e.g. `POST /api/auth/signup?inviteToken=…`). Extension-only: unioned + // with the built-in defaults in lib/helpers/redactUrl.js (already + // includes 'inviteToken' unconditionally). + sensitiveQueryKeys: [], }, csrf: { csrf: false, diff --git a/lib/helpers/redactUrl.js b/lib/helpers/redactUrl.js index 68eeb1d8e..f8b378879 100644 --- a/lib/helpers/redactUrl.js +++ b/lib/helpers/redactUrl.js @@ -4,13 +4,14 @@ import config from '../../config/index.js'; /** - * Built-in fallback values. `lib/` must stay module-agnostic — the real source - * of truth is `config.log.sensitiveQueryKeys` / `config.log.sensitivePathMarkers` - * (see below), which a module extends from its own config without editing this - * shared helper. These literals are the ultimate fallback: if config is absent - * or does not define the key (e.g. an edge context that builds a partial config - * object), redaction still degrades safely to the current known-sensitive - * routes instead of silently redacting nothing. + * Built-in, authoritative base lists. `lib/` must stay module-agnostic, so a + * module/downstream project never edits this shared helper to add its own + * token-bearing routes — it extends `config.log.sensitiveQueryKeys` / + * `config.log.sensitivePathMarkers` (see below) instead. These built-ins + * ALWAYS apply, unioned with whatever config contributes: config can only + * ADD markers/keys, never remove or replace these, so redaction can never be + * silently disabled (an absent key or an explicit empty array in config both + * degrade to exactly these defaults, never to nothing). */ const DEFAULT_SENSITIVE_QUERY_KEYS = ['inviteToken']; const DEFAULT_SENSITIVE_PATH_MARKERS = ['reset', 'verify', 'verify-email']; @@ -23,11 +24,13 @@ const DEFAULT_SENSITIVE_PATH_MARKERS = ['reset', 'verify', 'verify-email']; * `:url`, so without redaction a live single-use invite token lands in prod logs * (and any log shipper / aggregator downstream). Redact it to `REDACTED`. * - * Sourced from `config.log.sensitiveQueryKeys` (extendable per-module without - * editing this shared helper); falls back to `DEFAULT_SENSITIVE_QUERY_KEYS` - * when config/the key is absent so redaction never silently degrades to nothing. + * Union of `DEFAULT_SENSITIVE_QUERY_KEYS` and `config.log.sensitiveQueryKeys`: + * a module/downstream project extends the set purely additively from its own + * config, without editing this shared helper. Deduplicated via `Set`. Missing + * config, a missing key, or an explicit empty array all resolve to exactly the + * built-in defaults — config can never shrink or clobber this list. */ -const SENSITIVE_QUERY_KEYS = config?.log?.sensitiveQueryKeys ?? DEFAULT_SENSITIVE_QUERY_KEYS; +const SENSITIVE_QUERY_KEYS = [...new Set([...DEFAULT_SENSITIVE_QUERY_KEYS, ...(config?.log?.sensitiveQueryKeys ?? [])])]; /** * Path segments that are immediately followed by a single-use secret carried as @@ -37,11 +40,13 @@ const SENSITIVE_QUERY_KEYS = config?.log?.sensitiveQueryKeys ?? DEFAULT_SENSITIV * directly in the path, so the segment right after one of these markers must be * redacted before the URL reaches any log/analytics store. * - * Sourced from `config.log.sensitivePathMarkers` (extendable per-module without - * editing this shared helper); falls back to `DEFAULT_SENSITIVE_PATH_MARKERS` - * when config/the key is absent so redaction never silently degrades to nothing. + * Union of `DEFAULT_SENSITIVE_PATH_MARKERS` and `config.log.sensitivePathMarkers`: + * a module/downstream project extends the set purely additively from its own + * config, without editing this shared helper. Deduplicated via `Set`. Missing + * config, a missing key, or an explicit empty array all resolve to exactly the + * built-in defaults — config can never shrink or clobber this list. */ -const SENSITIVE_PATH_MARKERS = config?.log?.sensitivePathMarkers ?? DEFAULT_SENSITIVE_PATH_MARKERS; +const SENSITIVE_PATH_MARKERS = [...new Set([...DEFAULT_SENSITIVE_PATH_MARKERS, ...(config?.log?.sensitivePathMarkers ?? [])])]; /** * @desc Redact single-use secrets embedded as PATH parameters. Any segment that diff --git a/lib/helpers/tests/redactUrl.unit.tests.js b/lib/helpers/tests/redactUrl.unit.tests.js index 829cf1041..be1a66060 100644 --- a/lib/helpers/tests/redactUrl.unit.tests.js +++ b/lib/helpers/tests/redactUrl.unit.tests.js @@ -116,12 +116,17 @@ describe('redactPathSecrets', () => { /** * `SENSITIVE_QUERY_KEYS` / `SENSITIVE_PATH_MARKERS` are read from - * `config.log.*` once, at module-evaluation time (#3953). These tests mock - * `config/index.js` and re-import the module fresh (via `jest.resetModules()` - * + dynamic `import()` — see `posthog-context.middleware.unit.tests.js` for - * the same pattern) to exercise both the config-honoring path and the - * fail-safe fallback path, without disturbing the static-import tests above - * (which exercise the real config's current defaults, unchanged). + * `config.log.*` once, at module-evaluation time (#3953), and UNIONED with + * the built-in `DEFAULT_SENSITIVE_*` lists — config is extension-only, it can + * never shrink or clobber the built-ins (deepMerge replaces arrays wholesale, + * so a naive fallback would let a module-level config array silently clobber + * the global default instead of extending it; union sidesteps that entirely). + * These tests mock `config/index.js` and re-import the module fresh (via + * `jest.resetModules()` + dynamic `import()` — see + * `posthog-context.middleware.unit.tests.js` for the same pattern) to + * exercise the union, the empty-array no-op, and the absent-config fallback, + * without disturbing the static-import tests above (which exercise the real + * config's current defaults, unchanged). */ describe('redactUrl config sourcing (#3953)', () => { /** @@ -135,22 +140,38 @@ describe('redactUrl config sourcing (#3953)', () => { return import('../redactUrl.js'); }; - test('honors a module-extended path marker from config', async () => { + test('unions a module-extended path marker from config with the built-in defaults', async () => { const mod = await loadWithConfig({ - log: { sensitivePathMarkers: ['reset', 'verify', 'verify-email', 'unsubscribe'] }, + log: { sensitivePathMarkers: ['unsubscribe'] }, }); - expect(mod.SENSITIVE_PATH_MARKERS).toContain('unsubscribe'); + expect(mod.SENSITIVE_PATH_MARKERS).toEqual(expect.arrayContaining(['reset', 'verify', 'verify-email', 'unsubscribe'])); + // built-in marker still redacts — extension never clobbers it + expect(mod.redactPathSecrets('/api/auth/reset/SECRETTOKEN')).toBe('/api/auth/reset/REDACTED'); + // config-added marker redacts too expect(mod.redactPathSecrets('/api/newsletter/unsubscribe/SECRETTOKEN')).toBe('/api/newsletter/unsubscribe/REDACTED'); }); - test('honors a module-extended query key from config', async () => { + test('unions a module-extended query key from config with the built-in defaults', async () => { const mod = await loadWithConfig({ - log: { sensitiveQueryKeys: ['inviteToken', 'magicToken'] }, + log: { sensitiveQueryKeys: ['magicToken'] }, }); - expect(mod.SENSITIVE_QUERY_KEYS).toContain('magicToken'); + expect(mod.SENSITIVE_QUERY_KEYS).toEqual(expect.arrayContaining(['inviteToken', 'magicToken'])); + // built-in key still redacts — extension never clobbers it + expect(mod.default('/x?inviteToken=secret')).toBe('/x?inviteToken=REDACTED'); + // config-added key redacts too expect(mod.default('/x?magicToken=secret')).toBe('/x?magicToken=REDACTED'); }); + test('an explicit empty array in config is a no-op — built-in defaults still apply', async () => { + const mod = await loadWithConfig({ + log: { sensitivePathMarkers: [], sensitiveQueryKeys: [] }, + }); + expect(mod.SENSITIVE_PATH_MARKERS).toEqual(['reset', 'verify', 'verify-email']); + expect(mod.SENSITIVE_QUERY_KEYS).toEqual(['inviteToken']); + expect(mod.redactPathSecrets('/api/auth/reset/SECRETTOKEN')).toBe('/api/auth/reset/REDACTED'); + expect(mod.default('/x?inviteToken=secret')).toBe('/x?inviteToken=REDACTED'); + }); + test('falls back to the built-in defaults when config.log is missing', async () => { const mod = await loadWithConfig({}); expect(mod.SENSITIVE_PATH_MARKERS).toEqual(['reset', 'verify', 'verify-email']); From 6a48c6ad06225539afcef4fb59659f1a6c96e743 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 16 Jul 2026 17:02:55 +0200 Subject: [PATCH 3/4] fix(log): make redaction config extension layer-proof and shape-safe Phase-0 iteration-2 review findings on #3953: - Remove log.sensitivePathMarkers/sensitiveQueryKeys from config/defaults/development.config.js. deepMerge (config/index.js) replaces arrays wholesale, so declaring these as [] at Layer 2 (global defaults) silently clobbered any Layer 1 (module config) extension of the same key. Omitting the key lets a module value pass through untouched; a comment documents the extension point. - Guard redactUrl.js against non-array config values via a sanitizeConfigList helper: an object/number would throw at module-eval (boot crash), a string would spread char-by-char (silent over-redaction). Malformed values now fall back to [] (defaults still apply) and log a console.warn identifying the offending config path. - Add malformed-value tests (object + string) and a lockstep guard test pinning that development.config.js never re-declares either key. Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup --- config/defaults/development.config.js | 29 +++++++------ lib/helpers/redactUrl.js | 36 +++++++++++++--- lib/helpers/tests/redactUrl.unit.tests.js | 50 +++++++++++++++++++++++ 3 files changed, 96 insertions(+), 19 deletions(-) diff --git a/config/defaults/development.config.js b/config/defaults/development.config.js index 50b4c9864..b05d7fccf 100644 --- a/config/defaults/development.config.js +++ b/config/defaults/development.config.js @@ -81,19 +81,22 @@ const config = { maxFiles: 2, json: false, }, - // Path segments consumed by `lib/helpers/redactUrl.js` (`redactPathSecrets`) - // to redact single-use secrets carried as a PATH parameter (e.g. - // `/api/auth/reset/:token`). Extension-only: unioned with the built-in - // defaults in lib/helpers/redactUrl.js, so a module adds its own - // token-bearing route here without editing shared lib/ or duplicating - // ['reset', 'verify', 'verify-email'], which already apply unconditionally. - sensitivePathMarkers: [], - // Query-string parameter names consumed by `lib/helpers/redactUrl.js` - // (`redactUrl`) to redact single-use secrets carried in the query string - // (e.g. `POST /api/auth/signup?inviteToken=…`). Extension-only: unioned - // with the built-in defaults in lib/helpers/redactUrl.js (already - // includes 'inviteToken' unconditionally). - sensitiveQueryKeys: [], + // Redaction extension points (optional — do NOT declare either key here, + // not even as `[]`): a module or project config may set + // `log.sensitivePathMarkers` / `log.sensitiveQueryKeys` (array of + // strings) to extend the built-in redaction lists consumed by + // `lib/helpers/redactUrl.js` (`redactPathSecrets` for PATH segments e.g. + // `/api/auth/reset/:token`; `redactUrl` for query params e.g. + // `?inviteToken=…`). Values are UNIONED with the built-in defaults there + // (['reset', 'verify', 'verify-email'] / ['inviteToken']), never + // replacing them. + // + // These two keys are intentionally OMITTED from this file: `deepMerge` + // (config/index.js) replaces arrays wholesale rather than merging them, + // so an explicit value here (Layer 2: global defaults) would silently + // clobber a module's own extension of the same key (Layer 1: module + // config) instead of unioning with it. An omitted key lets a Layer-1 + // value pass through untouched to redactUrl.js. }, csrf: { csrf: false, diff --git a/lib/helpers/redactUrl.js b/lib/helpers/redactUrl.js index f8b378879..d92dfd537 100644 --- a/lib/helpers/redactUrl.js +++ b/lib/helpers/redactUrl.js @@ -16,6 +16,28 @@ import config from '../../config/index.js'; const DEFAULT_SENSITIVE_QUERY_KEYS = ['inviteToken']; const DEFAULT_SENSITIVE_PATH_MARKERS = ['reset', 'verify', 'verify-email']; +/** + * @desc Sanitize a config-provided redaction list so it is always an array + * before it is spread into the union below. A config typo (an object, a + * number, a bare string, …) must never throw at module-eval time — that + * would crash the whole app on boot — and must never silently degrade into + * a per-character array either (spreading a string splits it into individual + * characters, which is not the extension list a config author intended and + * would slip straight past an `Array.isArray`-less spread). Falls back to + * an empty array, which is a no-op for the union below (identical to an + * absent/missing key — the built-in defaults still apply), and logs a + * warning identifying the offending config path so the typo gets noticed. + * @param {*} value - the raw `config.log.` value + * @param {String} label - dotted config path for the warning message, e.g. 'log.sensitivePathMarkers' + * @returns {Array} `value` unchanged when it is already an array, otherwise `[]` + */ +const sanitizeConfigList = (value, label) => { + if (Array.isArray(value)) return value; + if (value == null) return []; + console.warn(`[redactUrl] config.${label} ignored: expected an array, got ${typeof value}`); + return []; +}; + /** * Sensitive query-string parameters that must never reach the request log. * @@ -27,10 +49,11 @@ const DEFAULT_SENSITIVE_PATH_MARKERS = ['reset', 'verify', 'verify-email']; * Union of `DEFAULT_SENSITIVE_QUERY_KEYS` and `config.log.sensitiveQueryKeys`: * a module/downstream project extends the set purely additively from its own * config, without editing this shared helper. Deduplicated via `Set`. Missing - * config, a missing key, or an explicit empty array all resolve to exactly the - * built-in defaults — config can never shrink or clobber this list. + * config, a missing key, an explicit empty array, or a malformed (non-array) + * value all resolve to exactly the built-in defaults — config can never + * shrink or clobber this list. */ -const SENSITIVE_QUERY_KEYS = [...new Set([...DEFAULT_SENSITIVE_QUERY_KEYS, ...(config?.log?.sensitiveQueryKeys ?? [])])]; +const SENSITIVE_QUERY_KEYS = [...new Set([...DEFAULT_SENSITIVE_QUERY_KEYS, ...sanitizeConfigList(config?.log?.sensitiveQueryKeys, 'log.sensitiveQueryKeys')])]; /** * Path segments that are immediately followed by a single-use secret carried as @@ -43,10 +66,11 @@ const SENSITIVE_QUERY_KEYS = [...new Set([...DEFAULT_SENSITIVE_QUERY_KEYS, ...(c * Union of `DEFAULT_SENSITIVE_PATH_MARKERS` and `config.log.sensitivePathMarkers`: * a module/downstream project extends the set purely additively from its own * config, without editing this shared helper. Deduplicated via `Set`. Missing - * config, a missing key, or an explicit empty array all resolve to exactly the - * built-in defaults — config can never shrink or clobber this list. + * config, a missing key, an explicit empty array, or a malformed (non-array) + * value all resolve to exactly the built-in defaults — config can never + * shrink or clobber this list. */ -const SENSITIVE_PATH_MARKERS = [...new Set([...DEFAULT_SENSITIVE_PATH_MARKERS, ...(config?.log?.sensitivePathMarkers ?? [])])]; +const SENSITIVE_PATH_MARKERS = [...new Set([...DEFAULT_SENSITIVE_PATH_MARKERS, ...sanitizeConfigList(config?.log?.sensitivePathMarkers, 'log.sensitivePathMarkers')])]; /** * @desc Redact single-use secrets embedded as PATH parameters. Any segment that diff --git a/lib/helpers/tests/redactUrl.unit.tests.js b/lib/helpers/tests/redactUrl.unit.tests.js index be1a66060..72991a45b 100644 --- a/lib/helpers/tests/redactUrl.unit.tests.js +++ b/lib/helpers/tests/redactUrl.unit.tests.js @@ -1,5 +1,6 @@ import { jest, describe, test, expect } from '@jest/globals'; import redactUrl, { SENSITIVE_QUERY_KEYS, SENSITIVE_PATH_MARKERS, redactPathSecrets } from '../redactUrl.js'; +import developmentConfig from '../../../config/defaults/development.config.js'; describe('redactUrl', () => { test('redacts an inviteToken value, preserving the path', () => { @@ -185,4 +186,53 @@ describe('redactUrl config sourcing (#3953)', () => { expect(mod.SENSITIVE_PATH_MARKERS).toEqual(['reset', 'verify', 'verify-email']); expect(mod.SENSITIVE_QUERY_KEYS).toEqual(['inviteToken']); }); + + /** + * A shape mismatch on the config value (e.g. a typo'd object instead of an + * array) must degrade the same way an absent/empty value does — never throw + * at module-eval time (`[...{}]` is not iterable, which would crash the + * whole app on boot) and never silently spread a string char-by-char + * (`[...'oops']` → `['o','o','p','s']`, which would over-redact app-wide). + */ + test('an object value for sensitivePathMarkers is ignored — defaults still apply, no throw, warns', async () => { + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const mod = await loadWithConfig({ + log: { sensitivePathMarkers: { notAnArray: true } }, + }); + expect(mod.SENSITIVE_PATH_MARKERS).toEqual(['reset', 'verify', 'verify-email']); + expect(mod.redactPathSecrets('/api/auth/reset/SECRETTOKEN')).toBe('/api/auth/reset/REDACTED'); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('log.sensitivePathMarkers')); + warnSpy.mockRestore(); + }); + + test('a string value for sensitiveQueryKeys is ignored (not spread char-by-char) — defaults still apply, no throw, warns', async () => { + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const mod = await loadWithConfig({ + log: { sensitiveQueryKeys: 'oops' }, + }); + expect(mod.SENSITIVE_QUERY_KEYS).toEqual(['inviteToken']); + expect(mod.default('/x?inviteToken=secret')).toBe('/x?inviteToken=REDACTED'); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('log.sensitiveQueryKeys')); + warnSpy.mockRestore(); + }); +}); + +/** + * Lockstep guard (#3953): `config/defaults/development.config.js` must never + * declare `log.sensitivePathMarkers` / `log.sensitiveQueryKeys` again, not + * even as `[]`. `deepMerge` (config/index.js) replaces arrays wholesale + * instead of merging them, so a Layer-2 (global defaults) value — including + * an explicit empty array — would silently clobber a Layer-1 (module config) + * extension of the same key. Omitting the key entirely is what lets a + * module's value pass through untouched to `lib/helpers/redactUrl.js`. This + * test pins that fix against regression. + */ +describe('development.config.js lockstep guard (#3953)', () => { + test('does not declare log.sensitivePathMarkers', () => { + expect(Object.prototype.hasOwnProperty.call(developmentConfig.log, 'sensitivePathMarkers')).toBe(false); + }); + + test('does not declare log.sensitiveQueryKeys', () => { + expect(Object.prototype.hasOwnProperty.call(developmentConfig.log, 'sensitiveQueryKeys')).toBe(false); + }); }); From 605959ca14d51feb1a0f684321db684cc1efbfe3 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 16 Jul 2026 21:36:23 +0200 Subject: [PATCH 4/4] test(log): restore console.warn spy in finally to prevent mock leakage Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup --- lib/helpers/tests/redactUrl.unit.tests.js | 34 +++++++++++++---------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/helpers/tests/redactUrl.unit.tests.js b/lib/helpers/tests/redactUrl.unit.tests.js index 72991a45b..434508866 100644 --- a/lib/helpers/tests/redactUrl.unit.tests.js +++ b/lib/helpers/tests/redactUrl.unit.tests.js @@ -196,24 +196,30 @@ describe('redactUrl config sourcing (#3953)', () => { */ test('an object value for sensitivePathMarkers is ignored — defaults still apply, no throw, warns', async () => { const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - const mod = await loadWithConfig({ - log: { sensitivePathMarkers: { notAnArray: true } }, - }); - expect(mod.SENSITIVE_PATH_MARKERS).toEqual(['reset', 'verify', 'verify-email']); - expect(mod.redactPathSecrets('/api/auth/reset/SECRETTOKEN')).toBe('/api/auth/reset/REDACTED'); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('log.sensitivePathMarkers')); - warnSpy.mockRestore(); + try { + const mod = await loadWithConfig({ + log: { sensitivePathMarkers: { notAnArray: true } }, + }); + expect(mod.SENSITIVE_PATH_MARKERS).toEqual(['reset', 'verify', 'verify-email']); + expect(mod.redactPathSecrets('/api/auth/reset/SECRETTOKEN')).toBe('/api/auth/reset/REDACTED'); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('log.sensitivePathMarkers')); + } finally { + warnSpy.mockRestore(); + } }); test('a string value for sensitiveQueryKeys is ignored (not spread char-by-char) — defaults still apply, no throw, warns', async () => { const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - const mod = await loadWithConfig({ - log: { sensitiveQueryKeys: 'oops' }, - }); - expect(mod.SENSITIVE_QUERY_KEYS).toEqual(['inviteToken']); - expect(mod.default('/x?inviteToken=secret')).toBe('/x?inviteToken=REDACTED'); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('log.sensitiveQueryKeys')); - warnSpy.mockRestore(); + try { + const mod = await loadWithConfig({ + log: { sensitiveQueryKeys: 'oops' }, + }); + expect(mod.SENSITIVE_QUERY_KEYS).toEqual(['inviteToken']); + expect(mod.default('/x?inviteToken=secret')).toBe('/x?inviteToken=REDACTED'); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('log.sensitiveQueryKeys')); + } finally { + warnSpy.mockRestore(); + } }); });