refactor(log): source sensitive redaction lists from config as pure extensions

config/defaults/development.config.js

@@ -81,6 +81,22 @@ const config = {
       maxFiles: 2,
       json: false,
     },
+    // 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,

lib/helpers/redactUrl.js

@@ -1,12 +1,59 @@
+/**
+ * Module dependencies.
+ */
+import config from '../../config/index.js';
+
+/**
+ * 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'];
+
+/**
+ * @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.<key>` 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.
  *
  * `inviteToken` rides the query string of `POST /api/auth/signup?inviteToken=…`
  * (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`.
+ *
+ * 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, 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 = ['inviteToken'];
+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
@@ -15,8 +62,15 @@ 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.
+ *
+ * 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, 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 = ['reset', 'verify', 'verify-email'];
+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

lib/helpers/tests/redactUrl.unit.tests.js

@@ -1,5 +1,6 @@
-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';
+import developmentConfig from '../../../config/defaults/development.config.js';
 
 describe('redactUrl', () => {
   test('redacts an inviteToken value, preserving the path', () => {
@@ -113,3 +114,131 @@ 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), 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)', () => {
+  /**
+   * 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<Object>} the freshly-loaded module exports
+   */
+  const loadWithConfig = async (mockConfig) => {
+    jest.resetModules();
+    jest.unstable_mockModule('../../../config/index.js', () => ({ default: mockConfig }));
+    return import('../redactUrl.js');
+  };
+
+  test('unions a module-extended path marker from config with the built-in defaults', async () => {
+    const mod = await loadWithConfig({
+      log: { sensitivePathMarkers: ['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('unions a module-extended query key from config with the built-in defaults', async () => {
+    const mod = await loadWithConfig({
+      log: { sensitiveQueryKeys: ['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']);
+    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']);
+  });
+
+  /**
+   * 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(() => {});
+    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(() => {});
+    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();
+    }
+  });
+});
+
+/**
+ * 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);
+  });
+});