From 7f3b8171a8011ce318a57e471f38c52e87bcd533 Mon Sep 17 00:00:00 2001 From: erKam Date: Sat, 7 Nov 2026 15:52:00 +0000 Subject: [PATCH 1/5] fix(overlay): close redaction leaks found by a leak corpus Add a corpus of fake secrets in many shapes (JWTs, auth schemes, key prefixes, PEM keys, DSNs, values under secret names) and run it through the redaction path of every collector and of the server: serialize, NgRx, router, forms, errors, HTTP calls, previews and headers, Analog reports and server logs, and Capacitor calls. It found five leaks, fixed here: - A secret pair after another word and a colon (`Error: password: "x"`, `Error: password="x"`) kept the password, because `password:` was read as the value of `Error`. text-redact.ts now reads such a value again as a key. `xmlns:auth="urn:a"` and prose like `token: expired` stay as they were. - In an Analog server body that is not JSON, `Authorization: ApiKey ` kept the key: the pair rule masked `ApiKey` first and left the key for the credential rule to miss. Credentials, JWTs and bearer tokens are now masked before the pairs (redactTokens in forms-privacy.ts, which the forms messages use too). - The Analog server log ignored secret pairs in the `#fragment` and in `;matrix` params. - Headers with a camelCase secret name (`newPassword`) were not masked, since the name was only checked after lowercasing (http-redact.ts). - Tokens used as NgRx state keys were not masked, though security.md says they are (ngrx-shared.ts). Two keys that mask to the same text both stay, the second with a ` (2)` suffix. Refs #371 --- packages/devtools/CHANGELOG.md | 1 + .../__tests__/fixtures/redaction-corpus.json | 160 ++++++++++++++ .../src/__tests__/redaction-corpus.test.ts | 203 ++++++++++++++++++ .../__tests__/redaction-leak-fixes.test.ts | 51 +++++ packages/devtools/src/analog-server-log.ts | 25 ++- packages/devtools/src/forms-privacy.ts | 10 +- packages/devtools/src/http-redact.ts | 3 +- packages/devtools/src/ngrx-shared.ts | 21 +- packages/devtools/src/text-redact.ts | 11 + 9 files changed, 467 insertions(+), 18 deletions(-) create mode 100644 packages/devtools/src/__tests__/fixtures/redaction-corpus.json create mode 100644 packages/devtools/src/__tests__/redaction-corpus.test.ts create mode 100644 packages/devtools/src/__tests__/redaction-leak-fixes.test.ts diff --git a/packages/devtools/CHANGELOG.md b/packages/devtools/CHANGELOG.md index 034683a0..2672a37d 100644 --- a/packages/devtools/CHANGELOG.md +++ b/packages/devtools/CHANGELOG.md @@ -229,6 +229,7 @@ What remains: `/config` exports `DEFAULT_POPUP_HOTKEY`, `PANGULAR_ACTIONS`, `PAN - The **Pipes** tab no longer marks two `| async` usages of a Promise, a Subject, an `asObservable()` view of one or an NgRx store select as duplicates, since their subscribers share one execution (#369). Any other source that two `| async` usages subscribe to shows as **possible duplicate subscription**, and `explain-pipe` says **Possible duplicate subscriptions**, because the page can't see `share()` or `shareReplay()` in a pipe. - Browser DevTools show the app's own line as the caller of a `console` call again. The browser entries ship source maps that list the console tap in `x_google_ignoreList`, so its wrapper frame is skipped (#369). - The Angular Native overlay gates the injector profiler like the NativeScript overlay: in a development build with no `window`, importing it defines one until Angular publishes its debug utilities, so the **Providers** lists fill (#371). +- Redaction leaks found by a corpus of fake secrets run through every redaction path are closed: a secret pair after another word and a colon (`Error: password: "…"`), the key after `Authorization: ApiKey` in an Analog server body that is not JSON, secret pairs in the fragment or matrix params of an Analog server message, headers with a camelCase secret name such as `newPassword`, and tokens used as NgRx state keys. ### Docs diff --git a/packages/devtools/src/__tests__/fixtures/redaction-corpus.json b/packages/devtools/src/__tests__/fixtures/redaction-corpus.json new file mode 100644 index 00000000..60081940 --- /dev/null +++ b/packages/devtools/src/__tests__/fixtures/redaction-corpus.json @@ -0,0 +1,160 @@ +{ + "_comment": "Fake secrets in the shapes the redaction rules promise to hide. `shapes` hide themselves wherever they appear; `named` hide under a secret key. `secret` is the part that must never come out. None of these is a real credential. Provider keys that secret scanners flag (Stripe, Slack webhooks, GitLab) are tested in credential-redaction.test.ts instead.", + "shapes": [ + { + "name": "JWT", + "value": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkFkYSJ9.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U", + "secret": "dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U" + }, + { + "name": "unsigned JWT", + "value": "eyJhbGciOiJub25lIn0.eyJzdWIiOiJhZGEtbG92ZWxhY2UifQ.", + "secret": "eyJzdWIiOiJhZGEtbG92ZWxhY2UifQ" + }, + { + "name": "bearer token", + "value": "Bearer 9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c", + "secret": "9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c" + }, + { + "name": "Basic credentials", + "value": "Basic YWRhOmNvcnJlY3Rob3JzZQ==", + "secret": "YWRhOmNvcnJlY3Rob3JzZQ" + }, + { + "name": "Digest credentials", + "value": "Digest username=\"ada\", response=\"6629fae49393a05397450978507c4ef1\"", + "secret": "6629fae49393a05397450978507c4ef1" + }, + { + "name": "Token credentials", + "value": "Token 9944b09199c62bcf9418ad846dd0e4bbdfc6ee4b", + "secret": "9944b09199c62bcf9418ad846dd0e4bbdfc6ee4b" + }, + { + "name": "Authorization header", + "value": "Authorization: ApiKey Zq7Lm2Pw9Xr4Tn8Vb3Kd", + "secret": "Zq7Lm2Pw9Xr4Tn8Vb3Kd" + }, + { + "name": "URL password", + "value": "https://ada:Tr0ub4dorPw@db.example.com/app", + "secret": "Tr0ub4dorPw" + }, + { + "name": "Stripe webhook secret", + "value": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", + "secret": "MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw" + }, + { + "name": "GitHub token", + "value": "ghp_16C7e42F292c6912E7710c838347Ae178B4a", + "secret": "16C7e42F292c6912E7710c838347Ae178B4a" + }, + { + "name": "GitHub fine-grained token", + "value": "github_pat_11ABCDEFG0123456789_abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJK", + "secret": "11ABCDEFG0123456789_abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJK" + }, + { + "name": "Slack token", + "value": "xoxb-2442-1234567890-AbCdEfGhIjKlMnOpQrStUvWx", + "secret": "2442-1234567890-AbCdEfGhIjKlMnOpQrStUvWx" + }, + { + "name": "AWS access key id", + "value": "AKIAIOSFODNN7EXAMPLE", + "secret": "IOSFODNN7EXAMPLE" + }, + { + "name": "Google API key", + "value": "AIzaSyD-9tSrke72PouQMnMX-a7eZSW0jkFMBWY", + "secret": "SyD-9tSrke72PouQMnMX-a7eZSW0jkFMBWY" + }, + { + "name": "OpenAI project key", + "value": "sk-proj-AbCdEfGhIjKlMnOpQrStUvWxYz0123456789abcd", + "secret": "AbCdEfGhIjKlMnOpQrStUvWxYz0123456789abcd" + }, + { + "name": "Anthropic key", + "value": "sk-ant-api03-AbCdEfGhIjKlMnOpQrStUvWxYz0123456789-_abcd", + "secret": "api03-AbCdEfGhIjKlMnOpQrStUvWxYz0123456789-_abcd" + }, + { + "name": "npm token", + "value": "npm_AbCdEfGhIjKlMnOpQrStUvWxYz0123456789", + "secret": "AbCdEfGhIjKlMnOpQrStUvWxYz0123456789" + }, + { + "name": "Hugging Face token", + "value": "hf_AbCdEfGhIjKlMnOpQrStUvWxYz01234567", + "secret": "AbCdEfGhIjKlMnOpQrStUvWxYz01234567" + }, + { + "name": "SendGrid key", + "value": "SG.AbCdEfGhIjKlMnOpQrStUv.WxYz0123456789AbCdEfGhIjKlMn", + "secret": "WxYz0123456789AbCdEfGhIjKlMn" + }, + { + "name": "PEM private key", + "value": "-----BEGIN RSA PRIVATE KEY-----\nMIIEowIBAAKCAQEA7bq98fakeKeyMaterialLine\n-----END RSA PRIVATE KEY-----", + "secret": "MIIEowIBAAKCAQEA7bq98fakeKeyMaterialLine" + }, + { + "name": "Sentry DSN", + "value": "https://0123456789abcdef0123456789abcdef@o1.ingest.sentry.io/42", + "secret": "0123456789abcdef0123456789abcdef" + } + ], + "named": [ + { + "key": "password", + "secret": "Tr0ub4dor-3" + }, + { + "key": "newPassword", + "secret": "C0rrectHorse" + }, + { + "key": "apiKey", + "secret": "k3y-AbC-998877" + }, + { + "key": "client_secret", + "secret": "cs-Zz9Yy8Xx7" + }, + { + "key": "access_token", + "secret": "at-Qq1Ww2Ee3" + }, + { + "key": "refreshToken", + "secret": "rt-Pp0Oo9Ii8" + }, + { + "key": "sessionId", + "secret": "sess-Mm4Nn5Bb6" + }, + { + "key": "cardNumber", + "secret": "4111111111111111" + }, + { + "key": "cvv", + "secret": "737-cvv" + }, + { + "key": "otp", + "secret": "otp-482913" + }, + { + "key": "privateKey", + "secret": "pk-Ll7Kk6Jj5" + }, + { + "key": "signature", + "secret": "sig-Hh4Gg3Ff2" + } + ] +} diff --git a/packages/devtools/src/__tests__/redaction-corpus.test.ts b/packages/devtools/src/__tests__/redaction-corpus.test.ts new file mode 100644 index 00000000..9f23144a --- /dev/null +++ b/packages/devtools/src/__tests__/redaction-corpus.test.ts @@ -0,0 +1,203 @@ +// @vitest-environment jsdom +/** + * The leak corpus: fake secrets in many shapes (fixtures/redaction-corpus.json) + * run through the redaction path of every collector and of the server, to + * prove none of them comes out readable. A new redaction path belongs in one + * of the tables below; a new secret shape belongs in the fixture. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import corpus from './fixtures/redaction-corpus.json' with { type: 'json' }; +import { + REDACTED, + isRedactedKey, + redactMessage, + SecretSet, + setRedaction, +} from '../forms-privacy.ts'; +import { redactRecord, redactText, redactUrl } from '../router.ts'; +import { serialize } from '../serialize.ts'; +import { redactCall, redactHeaders, redactPreview } from '../http-redact.ts'; +import { redactJsonText } from '../json-text-redact.ts'; +import { redactAnalogReport } from '../analog-redact.ts'; +import { redactBody, redactMessage as redactServerMessage } from '../analog-server-log.ts'; +import { maskedArgs } from '../capacitor-redact.ts'; +import { serialize as ngrxSerialize } from '../ngrx-shared.ts'; +import { consoleText, valueText } from '../errors-collector.ts'; +import { serializeFormValue } from '../forms.ts'; +import type { AnalogRuntimeReport } from '../analog-runtime.ts'; +import type { HttpCall } from '../http-rules.ts'; + +afterEach(() => setRedaction()); + +type Path = (input: string) => unknown; +type ObjectPath = (input: Record) => unknown; + +const call = (fields: Partial) => + redactCall({ id: 1, method: 'GET', url: '/api', ...fields } as HttpCall); + +const report = (fields: Partial) => + redactAnalogReport({ + pageId: 'p1', + url: '/', + analog: true, + chain: [], + hydrated: 0, + transferState: false, + hydrationErrors: [], + configPaths: [], + ...fields, + }); + +const show = (value: unknown): string => + typeof value === 'string' ? value : (JSON.stringify(value) ?? ''); + +/** Paths that take free text: messages, errors, previews, values. */ +const PAIR_TEXT_PATHS: Record = { + 'serialize (component inputs, signals, pipes, providers)': (t) => serialize(t), + 'NgRx serialize': (t) => ngrxSerialize(t), + 'router redactText (titles, guards, warnings)': (t) => redactText(t), + 'router redactRecord (params and data)': (t) => redactRecord({ note: t }), + 'errors valueText': (t) => valueText(t), + 'errors consoleText': (t) => consoleText(['failed:', t]), + 'errors valueText of an Error': (t) => valueText(new Error(t)), + 'HTTP call error': (t) => call({ error: t }), + 'HTTP response preview (text)': (t) => redactPreview(t), + 'HTTP header value': (t) => redactHeaders({ 'x-debug': t }), + 'Analog hydration error': (t) => report({ hydrationErrors: [t] }), + 'Capacitor plugin argument': (t) => maskedArgs('App', { message: t }, {}), +}; + +/** Paths that hide value shapes in text, but not `key=value` pairs. */ +const SHAPE_ONLY_TEXT_PATHS: Record = { + // Object keys are masked by shape: a key is a name, not a sentence. + 'serialize, as an object key': (t) => serialize({ [t]: 1 }), + 'NgRx serialize, as an object key': (t) => ngrxSerialize({ [t]: 1 }), + 'forms redactMessage (field errors)': (t) => redactMessage(t), + 'forms SecretSet (event log)': (t) => new SecretSet().redact(t), + 'Analog server log message': (t) => redactServerMessage(t), + 'Analog server log body': (t) => redactBody(t), +}; + +/** Paths that take an object and hide its values by key. */ +const OBJECT_PATHS: Record = { + serialize: (o) => serialize(o), + 'NgRx serialize': (o) => ngrxSerialize(o), + 'router redactRecord': (o) => redactRecord(o), + 'forms serializeFormValue': (o) => serializeFormValue(o), + 'HTTP response preview (JSON)': (o) => redactPreview(JSON.stringify(o)), + 'HTTP response preview (cut JSON)': (o) => redactPreview(JSON.stringify(o).slice(0, -2)), + 'HTTP call preview': (o) => call({ preview: JSON.stringify(o) }), + 'JSON text redaction': (o) => redactJsonText(JSON.stringify(o), isRedactedKey), + 'Capacitor plugin argument': (o) => maskedArgs('Preferences', o, {}), + 'Analog load() preview': (o) => + report({ load: { preview: JSON.stringify(o), bytes: 10, keys: [] } }), + 'Analog server log body': (o) => redactBody(JSON.stringify(o)), + 'errors valueText': (o) => valueText(o), + 'errors consoleText': (o) => consoleText(['state', o]), + 'HTTP headers': (o) => + redactHeaders(Object.fromEntries(Object.entries(o).map(([k, v]) => [k, String(v)]))), +}; + +/** Paths that take a URL. */ +const URL_PATHS: Record = { + 'router redactUrl': (u) => redactUrl(u), + 'HTTP call URL': (u) => call({ url: u }), + 'HTTP redirect target': (u) => call({ redirectedTo: u }), + 'Analog page URL': (u) => report({ url: u }), + 'Capacitor HTTP URL': (u) => maskedArgs('CapacitorHttp', { url: u, method: 'GET' }, {}), + 'Analog server log message': (u) => redactServerMessage(u), +}; + +function expectHidden(path: string, input: string, out: unknown, secret: string) { + const text = show(out); + expect( + text, + `${path} leaked ${JSON.stringify(secret)} from ${JSON.stringify(input)}`, + ).not.toContain(secret); + expect(text, `${path} hid ${JSON.stringify(input)} without a marker`).toMatch( + /\[redacted\]|%5Bredacted%5D/, + ); +} + +describe('the leak corpus', () => { + it('has every kind of secret it promises', () => { + expect(corpus.shapes.length).toBeGreaterThanOrEqual(20); + expect(corpus.named.length).toBeGreaterThanOrEqual(10); + for (const entry of corpus.shapes) expect(entry.value).toContain(entry.secret); + }); + + describe('value shapes hide themselves in any text', () => { + const paths = { ...PAIR_TEXT_PATHS, ...SHAPE_ONLY_TEXT_PATHS }; + for (const [path, run] of Object.entries(paths)) { + it(path, () => { + for (const { value, secret } of corpus.shapes) { + for (const input of [value, `request failed (${value}) retry later`]) { + expectHidden(path, input, run(input), secret); + } + } + }); + } + }); + + describe('values under secret names hide in key=value text', () => { + for (const [path, run] of Object.entries(PAIR_TEXT_PATHS)) { + it(path, () => { + for (const { key, secret } of corpus.named) { + for (const input of [ + `${key}=${secret}`, + `retry with ${key}=${secret}&user=ada`, + `{"${key}":"${secret}","user":"ada"}`, + `${key}: "${secret}"`, + ]) { + expectHidden(path, input, run(input), secret); + } + } + }); + } + }); + + describe('values under secret names hide in objects', () => { + for (const [path, run] of Object.entries(OBJECT_PATHS)) { + it(path, () => { + for (const { key, secret } of corpus.named) { + for (const input of [{ [key]: secret, user: 'ada' }, { form: { [key]: secret } }]) { + if (path === 'HTTP headers' && 'form' in input) continue; + expectHidden(path, JSON.stringify(input), run(input), secret); + } + } + }); + } + }); + + describe('values under secret names hide in URLs', () => { + for (const [path, run] of Object.entries(URL_PATHS)) { + it(path, () => { + for (const { key, secret } of corpus.named) { + for (const input of [ + `https://app.example.com/cb?${key}=${secret}&page=2`, + `/login#${key}=${encodeURIComponent(secret)}`, + ]) { + expectHidden(path, input, run(input), secret); + } + } + }); + } + }); + + describe('value shapes hide in URLs', () => { + const inUrl = corpus.shapes.filter((entry) => /^(https?:|eyJ)/.test(entry.value)); + for (const [path, run] of Object.entries(URL_PATHS)) { + it(path, () => { + for (const { value, secret } of inUrl) { + const input = value.startsWith('eyJ') ? `/verify/${value}?page=2` : value; + expectHidden(path, input, run(input), secret); + } + }); + } + }); + + it('keeps the marker every path writes the same', () => { + expect(REDACTED).toBe('[redacted]'); + expect(redactJsonText('{"password":"x"}', isRedactedKey)).toBe(`{"password":"${REDACTED}"}`); + }); +}); diff --git a/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts new file mode 100644 index 00000000..5f8d82db --- /dev/null +++ b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts @@ -0,0 +1,51 @@ +// @vitest-environment jsdom +// Leaks the corpus test (redaction-corpus.test.ts) found, one case each. +import { afterEach, describe, expect, it } from 'vitest'; +import { setRedaction } from '../forms-privacy.ts'; +import { redactText } from '../text-redact.ts'; +import { redactBody, redactMessage } from '../analog-server-log.ts'; +import { redactHeaders } from '../http-redact.ts'; +import { serialize as ngrxSerialize } from '../ngrx-shared.ts'; + +afterEach(() => setRedaction()); + +const JWT = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U'; + +describe('leaks found by the corpus', () => { + it('masks a pair whose key was read as the value of the word before it', () => { + expect(redactText('Error: password: "Tr0ub4dor-3"')).toBe('Error: password: "[redacted]"'); + expect(redactText('Error: token: "abc123"')).toBe('Error: token: "[redacted]"'); + expect(redactText('Error: password="abc123"')).toBe('Error: password="[redacted]"'); + expect(redactText('')).toBe(''); + // Prose stays as it was. + expect(redactText('Error: token: expired')).toBe('Error: token: expired'); + expect(redactText('Status: code: 404')).toBe('Status: code: 404'); + }); + + it('masks the credential after a scheme in an Analog server body that is not JSON', () => { + expect(redactBody('Authorization: ApiKey Zq7Lm2Pw9Xr4Tn8Vb3Kd')).not.toContain( + 'Zq7Lm2Pw9Xr4Tn8Vb3Kd', + ); + }); + + it('masks secret pairs in the fragment and matrix params of an Analog server message', () => { + expect(redactMessage('/login#access_token=abc123')).toBe('/login#access_token=[redacted]'); + expect(redactMessage('/a;sid=abc123')).toBe('/a;sid=[redacted]'); + }); + + it('masks a header whose camelCase name looks secret', () => { + expect(redactHeaders({ newPassword: 'C0rrectHorse', 'x-request-id': '42' })).toEqual({ + newpassword: '[redacted]', + 'x-request-id': '42', + }); + }); + + it('masks tokens used as NgRx state keys, keeping both entries', () => { + const other = JWT.replace('dozj', 'zzzz'); + const out = ngrxSerialize({ [JWT]: 1, [other]: 2, user: 'ada' }) as Record; + expect(JSON.stringify(out)).not.toContain('eyJ'); + expect(Object.values(out).sort()).toEqual([1, 2, 'ada']); + expect(out['user']).toBe('ada'); + }); +}); diff --git a/packages/devtools/src/analog-server-log.ts b/packages/devtools/src/analog-server-log.ts index d0c65ca5..93f0944c 100644 --- a/packages/devtools/src/analog-server-log.ts +++ b/packages/devtools/src/analog-server-log.ts @@ -2,13 +2,12 @@ import type { IncomingMessage, ServerResponse } from 'node:http'; import { ANALOG_RENDER_TIMING } from './analog-call-tags.ts'; import { beginRender, bindRender, tagRequest } from './analog-call-pages.ts'; import { runInAnalogRender } from './ssr-context.ts'; -import { redactCredentials } from './credential-redact.ts'; -import { isRedactedKey, redactJwt } from './forms-privacy.ts'; +import { REDACTED, isRedactedKey, redactTokens } from './forms-privacy.ts'; import { redactJsonText } from './json-text-redact.ts'; import { maskEmails } from './text-redact.ts'; -const BEARER = /\bBearer\s+[\w.~+/=-]+/gi; -const QUERY_PAIR = /([?&])([^=&#\s"'\\]*)=([^&#\s"'\\]*)/g; +// A fragment (`#access_token=…`) and matrix params (`;sid=…`) carry pairs like a query. +const QUERY_PAIR = /([?&#;])([^=&#;\s"'\\]*)=([^&#\s"'\\]*)/g; const SECRET_QUERY_KEY = /token|secret|password|key|code|session|^(sig|signature|auth|authorization|credentials?|jwt)$|^x-(amz|goog)-(signature|credential|security-token)$/i; @@ -23,17 +22,15 @@ function queryKey(key: string): string { } export function redactMessage(text: string): string { - return redactJwt(redactCredentials(text, '[redacted]')) - .replace(BEARER, 'Bearer [redacted]') - .replace(QUERY_PAIR, (whole, sep: string, key: string) => { - const name = queryKey(key); - return SECRET_QUERY_KEY.test(name) || isRedactedKey(name) ? `${sep}${key}=[redacted]` : whole; - }); + return redactTokens(text).replace(QUERY_PAIR, (whole, sep: string, key: string) => { + const name = queryKey(key); + return SECRET_QUERY_KEY.test(name) || isRedactedKey(name) ? `${sep}${key}=${REDACTED}` : whole; + }); } function redactPairs(text: string): string { return text.replace(SECRET_PAIR, (whole, key: string, sep: string, quote: string) => - isSecretJsonKey(key) ? `${key}${sep}${quote}[redacted]${quote}` : whole, + isSecretJsonKey(key) ? `${key}${sep}${quote}${REDACTED}${quote}` : whole, ); } @@ -149,7 +146,7 @@ function redactJson(value: unknown, depth = 0): unknown { if (Array.isArray(value)) return value.slice(0, 50).map((item) => redactJson(item, depth + 1)); const out: Record = {}; for (const [key, item] of Object.entries(value as Record).slice(0, 50)) { - out[key] = isSecretJsonKey(key) ? '[redacted]' : redactJson(item, depth + 1); + out[key] = isSecretJsonKey(key) ? REDACTED : redactJson(item, depth + 1); } return out; } @@ -159,7 +156,9 @@ export function redactBody(body: string): string { try { text = JSON.stringify(redactJson(JSON.parse(body))); } catch { - text = redactPairs(redactJsonText(body, isSecretJsonKey)); + // Credentials first, so a pair can't hide `ApiKey` in `Authorization: ApiKey ` and + // leave the key for the credential rule to miss. + text = redactPairs(redactJsonText(redactTokens(body), isSecretJsonKey)); } return redactMessage(text); } diff --git a/packages/devtools/src/forms-privacy.ts b/packages/devtools/src/forms-privacy.ts index ad11323f..9d5efc60 100644 --- a/packages/devtools/src/forms-privacy.ts +++ b/packages/devtools/src/forms-privacy.ts @@ -323,6 +323,14 @@ export function redactMessage(text: string, secrets: Iterable = []): str return redactMessageWith(text, longestFirst(secrets)); } +/** + * Credentials inside text (`credential-redact.ts`), JWTs and bearer tokens: + * the value-shape rules every message and log line goes through. + */ +export function redactTokens(text: string): string { + return redactJwt(redactCredentials(text, REDACTED)).replace(BEARER, `Bearer ${REDACTED}`); +} + /** `secrets` as `redactMessageWith` takes them: 3 characters or more, longest first. */ export function longestFirst(secrets: Iterable): string[] { const out = [...secrets].filter((secret) => secret.length >= 3); @@ -332,7 +340,7 @@ export function longestFirst(secrets: Iterable): string[] { /** `redactMessage` with secrets `longestFirst` already prepared, for many texts in a row. */ export function redactMessageWith(text: string, ordered: readonly string[]): string { - let out = redactJwt(redactCredentials(text, REDACTED)).replace(BEARER, `Bearer ${REDACTED}`); + let out = redactTokens(text); for (const secret of ordered) out = out.split(secret).join(REDACTED); return out; } diff --git a/packages/devtools/src/http-redact.ts b/packages/devtools/src/http-redact.ts index e436fc62..934f5554 100644 --- a/packages/devtools/src/http-redact.ts +++ b/packages/devtools/src/http-redact.ts @@ -78,7 +78,8 @@ export function redactHeaders(headers: HeaderRecord): HeaderRecord { const name = rawName.toLowerCase(); if (typeof value !== 'string') continue; out[name] = - ALWAYS_MASKED.has(name) || isRedactedKey(name) + // The name as sent too, since lowercasing loses camelCase words (`newPassword`). + ALWAYS_MASKED.has(name) || isRedactedKey(name) || isRedactedKey(rawName) ? REDACTED : clip(redactText(value), MAX_HEADER_VALUE); } diff --git a/packages/devtools/src/ngrx-shared.ts b/packages/devtools/src/ngrx-shared.ts index 096e258f..93451912 100644 --- a/packages/devtools/src/ngrx-shared.ts +++ b/packages/devtools/src/ngrx-shared.ts @@ -1,4 +1,4 @@ -import { isRedactedKey, REDACTED } from './forms-privacy.ts'; +import { isRedactedKey, REDACTED, redactMessage } from './forms-privacy.ts'; import { redactText } from './serialize.ts'; import { clip, overflowKey } from './text.ts'; import type { NgrxEntryDetail, NgrxSession } from './ngrx-session.ts'; @@ -303,8 +303,10 @@ export function serialize(value: unknown, options: SerializeOptions = {}): unkno const out: Record = {}; const keys = Object.keys(obj); for (const key of keys.slice(0, maxKeys)) { + // A token used as a key (`{ [jwt]: … }`) is masked like a value. + const shown = shownKey(key, out); if (isRedactedKey(key)) { - out[key] = REDACTED; + out[shown] = REDACTED; continue; } let item: unknown; @@ -313,7 +315,7 @@ export function serialize(value: unknown, options: SerializeOptions = {}): unkno } catch { item = '[Unreadable]'; } - out[key] = walk(item, depth + 1); + out[shown] = walk(item, depth + 1); } if (keys.length > maxKeys) out[overflowKey(out)] = `${keys.length - maxKeys} more keys`; return out; @@ -325,6 +327,19 @@ export function serialize(value: unknown, options: SerializeOptions = {}): unkno return walk(value, 0); } +const MIN_TOKEN_KEY = 8; + +/** `key` with tokens masked, and a ` (2)` suffix when another key already masked to it. */ +function shownKey(key: string, taken: Record): string { + // Every shape the masks know is longer than this, so a plain state key costs no regex. + if (key.length < MIN_TOKEN_KEY) return key; + const safe = redactMessage(key); + if (safe === key) return key; + let shown = safe; + for (let n = 2; Object.prototype.hasOwnProperty.call(taken, shown); n++) shown = `${safe} (${n})`; + return shown; +} + export function serializeSlice( key: PropertyKey, value: unknown, diff --git a/packages/devtools/src/text-redact.ts b/packages/devtools/src/text-redact.ts index fbbe2839..8244f32b 100644 --- a/packages/devtools/src/text-redact.ts +++ b/packages/devtools/src/text-redact.ts @@ -185,6 +185,17 @@ function replacePairs(text: string, pair: PairReplacer): string { // Punctuation that ends the sentence or the list stays: `(api_key=x, user=7)`. end += (tail?.[0] ?? '').replace(TRAILING_PUNCTUATION, '').length; } + // A value that is itself the next key (`Error: password: "x"` reads `password:` as the + // value of `Error`) is read again from its start, so that pair is found too. A `key=` + // value counts only after `: ` with a space, so `xmlns:auth="urn:a"` stays one name. + const nextKey = /:$/.test(value) || (/=$/.test(value) && /\s/.test(eq)); + if (replaced === match && value && !/^["']/.test(value) && nextKey) { + const valueAt = at + match.length - value.length; + out += text.slice(last, valueAt); + last = valueAt; + re.lastIndex = valueAt; + continue; + } out += text.slice(last, at) + replaced; last = end; re.lastIndex = Math.max(end, at + 1); From ced39c03ad060be0c6bc89f081fd090811b26310 Mon Sep 17 00:00:00 2001 From: erKam Date: Sat, 7 Nov 2026 16:13:25 +0000 Subject: [PATCH 2/5] fix(overlay): keep the next-key rescan linear on long pair chains Reading a pair value such as password: again as the next key rescanned the rest of the text whenever the value ended in a colon, so a chain like a:a:a:... took cubic time in the error collector. Only a value that is a single key name and its : or = is read again now, which keeps both compact and spaced chains linear and still finds Error: password: "x". --- .../src/__tests__/redaction-leak-fixes.test.ts | 18 ++++++++++++++++++ packages/devtools/src/text-redact.ts | 6 +++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts index 5f8d82db..f8a0afd3 100644 --- a/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts +++ b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts @@ -49,3 +49,21 @@ describe('leaks found by the corpus', () => { expect(out['user']).toBe('ada'); }); }); + +describe('a value read again as the next key', () => { + // Reading `password:` again as a key must not rescan the rest of a long chain of pairs. + it.each([ + ['compact', 'a:'], + ['spaced', 'a: '], + ['spaced with =', 'a= '], + ])('stays linear on a %s chain of 20000 pairs', (_name, unit) => { + const started = performance.now(); + redactText(unit.repeat(20_000), [], 'text'); + expect(performance.now() - started).toBeLessThan(4000); + }); + + it('still finds a secret after a chain of plain pairs', () => { + const out = redactText(`${'a: '.repeat(50)}password: "hunter2"`, [], 'text'); + expect(out).not.toContain('hunter2'); + }); +}); diff --git a/packages/devtools/src/text-redact.ts b/packages/devtools/src/text-redact.ts index 8244f32b..3a298690 100644 --- a/packages/devtools/src/text-redact.ts +++ b/packages/devtools/src/text-redact.ts @@ -166,6 +166,9 @@ type PairReplacer = ( * strong secret name also takes the rest of its token (`=`) or line (`:`), * so a separator inside a password doesn't leave its tail readable. */ +/** A pair value that is one key name and its `:` or `=`, read again as the next pair. */ +const NEXT_KEY_VALUE = /^[\w$%.[\]-]+[:=]$/; + function replacePairs(text: string, pair: PairReplacer): string { const re = new RegExp(TEXT_PAIR.source, TEXT_PAIR.flags); let out = ''; @@ -188,7 +191,8 @@ function replacePairs(text: string, pair: PairReplacer): string { // A value that is itself the next key (`Error: password: "x"` reads `password:` as the // value of `Error`) is read again from its start, so that pair is found too. A `key=` // value counts only after `: ` with a space, so `xmlns:auth="urn:a"` stays one name. - const nextKey = /:$/.test(value) || (/=$/.test(value) && /\s/.test(eq)); + // Only a single key is read again, never a chain like `a:a:a:`, so the scan stays linear. + const nextKey = NEXT_KEY_VALUE.test(value) && (value.endsWith(':') || /\s/.test(eq)); if (replaced === match && value && !/^["']/.test(value) && nextKey) { const valueAt = at + match.length - value.length; out += text.slice(last, valueAt); From 3536e0022f520f698cd786e1c42a1b5e5c2913f7 Mon Sep 17 00:00:00 2001 From: erKam Date: Sat, 7 Nov 2026 16:21:23 +0000 Subject: [PATCH 3/5] fix(overlay): mask a secret after a compact chain of key names In 'Error: context:password: "x"' the whole chain 'context:password:' was read as the value of 'Error', and only a single key was read again, so the password showed. A value that is a chain of key names is now read again from its last key, which skips the chain in one step and keeps the scan linear. --- .../src/__tests__/redaction-leak-fixes.test.ts | 9 +++++++++ packages/devtools/src/text-redact.ts | 11 ++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts index f8a0afd3..03226129 100644 --- a/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts +++ b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts @@ -66,4 +66,13 @@ describe('a value read again as the next key', () => { const out = redactText(`${'a: '.repeat(50)}password: "hunter2"`, [], 'text'); expect(out).not.toContain('hunter2'); }); + + it.each([ + 'Error: context:password: "hunter2"', + 'Error: context:password: hunter2', + 'Error: a:b:c:token: hunter2', + `Error: ${'a:'.repeat(5000)}password: hunter2`, + ])('finds a secret after a compact chain of key names: %s', (text) => { + expect(redactText(text, [], 'text')).not.toContain('hunter2'); + }); }); diff --git a/packages/devtools/src/text-redact.ts b/packages/devtools/src/text-redact.ts index 3a298690..9672e5bc 100644 --- a/packages/devtools/src/text-redact.ts +++ b/packages/devtools/src/text-redact.ts @@ -168,6 +168,8 @@ type PairReplacer = ( */ /** A pair value that is one key name and its `:` or `=`, read again as the next pair. */ const NEXT_KEY_VALUE = /^[\w$%.[\]-]+[:=]$/; +/** A pair value that is a chain of key names, each with its `:` (`context:password:`). */ +const KEY_CHAIN_VALUE = /^(?:[\w$%.[\]-]+:)+$/; function replacePairs(text: string, pair: PairReplacer): string { const re = new RegExp(TEXT_PAIR.source, TEXT_PAIR.flags); @@ -191,10 +193,13 @@ function replacePairs(text: string, pair: PairReplacer): string { // A value that is itself the next key (`Error: password: "x"` reads `password:` as the // value of `Error`) is read again from its start, so that pair is found too. A `key=` // value counts only after `: ` with a space, so `xmlns:auth="urn:a"` stays one name. - // Only a single key is read again, never a chain like `a:a:a:`, so the scan stays linear. - const nextKey = NEXT_KEY_VALUE.test(value) && (value.endsWith(':') || /\s/.test(eq)); + // For a chain such as `context:password:` only its last key is read again, so a chain + // like `a:a:a:` is skipped in one step and the scan stays linear. + const chain = !NEXT_KEY_VALUE.test(value) && KEY_CHAIN_VALUE.test(value); + const nextKey = chain || (NEXT_KEY_VALUE.test(value) && (value.endsWith(':') || /\s/.test(eq))); if (replaced === match && value && !/^["']/.test(value) && nextKey) { - const valueAt = at + match.length - value.length; + const lastKey = chain ? value.lastIndexOf(':', value.length - 2) + 1 : 0; + const valueAt = at + match.length - value.length + lastKey; out += text.slice(last, valueAt); last = valueAt; re.lastIndex = valueAt; From f482bd782f9b1c84fcab8a08d58c37b3324a1c36 Mon Sep 17 00:00:00 2001 From: erKam Date: Sat, 7 Nov 2026 16:26:47 +0000 Subject: [PATCH 4/5] fix(overlay): mask a secret after a key chain that ends in = In 'Error: context:password= "x"' the chain 'context:password=' was read as the value of 'Error' and not read again, so the password showed. A chain ending in '=' is now read again from its last key when a space follows the '=', so 'xmlns:auth="urn:a"' still stays one attribute. --- .../src/__tests__/redaction-leak-fixes.test.ts | 6 ++++++ packages/devtools/src/text-redact.ts | 14 +++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts index 03226129..37bc6182 100644 --- a/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts +++ b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts @@ -71,8 +71,14 @@ describe('a value read again as the next key', () => { 'Error: context:password: "hunter2"', 'Error: context:password: hunter2', 'Error: a:b:c:token: hunter2', + 'Error: context:password= "hunter2"', + 'Error: context:password= hunter2', `Error: ${'a:'.repeat(5000)}password: hunter2`, ])('finds a secret after a compact chain of key names: %s', (text) => { expect(redactText(text, [], 'text')).not.toContain('hunter2'); }); + + it('keeps a namespaced attribute after a key as one name', () => { + expect(redactText('Log: xmlns:auth="urn:a"', [], 'text')).toContain('urn:a'); + }); }); diff --git a/packages/devtools/src/text-redact.ts b/packages/devtools/src/text-redact.ts index 9672e5bc..70b46eee 100644 --- a/packages/devtools/src/text-redact.ts +++ b/packages/devtools/src/text-redact.ts @@ -168,8 +168,11 @@ type PairReplacer = ( */ /** A pair value that is one key name and its `:` or `=`, read again as the next pair. */ const NEXT_KEY_VALUE = /^[\w$%.[\]-]+[:=]$/; -/** A pair value that is a chain of key names, each with its `:` (`context:password:`). */ -const KEY_CHAIN_VALUE = /^(?:[\w$%.[\]-]+:)+$/; +/** + * A pair value that is a chain of key names (`context:password:`), the last one + * possibly ending in `=` (`context:password=`). + */ +const KEY_CHAIN_VALUE = /^(?:[\w$%.[\]-]+:)+[\w$%.[\]-]*[:=]$/; function replacePairs(text: string, pair: PairReplacer): string { const re = new RegExp(TEXT_PAIR.source, TEXT_PAIR.flags); @@ -195,7 +198,12 @@ function replacePairs(text: string, pair: PairReplacer): string { // value counts only after `: ` with a space, so `xmlns:auth="urn:a"` stays one name. // For a chain such as `context:password:` only its last key is read again, so a chain // like `a:a:a:` is skipped in one step and the scan stays linear. - const chain = !NEXT_KEY_VALUE.test(value) && KEY_CHAIN_VALUE.test(value); + // A chain ending in `=` counts only when a space follows, as in `context:password= "x"`, + // so `xmlns:auth="urn:a"` stays one attribute. + const chain = + !NEXT_KEY_VALUE.test(value) && + KEY_CHAIN_VALUE.test(value) && + (value.endsWith(':') || /\s/.test(text.charAt(at + match.length))); const nextKey = chain || (NEXT_KEY_VALUE.test(value) && (value.endsWith(':') || /\s/.test(eq))); if (replaced === match && value && !/^["']/.test(value) && nextKey) { const lastKey = chain ? value.lastIndexOf(':', value.length - 2) + 1 : 0; From 786228253920d19d962bd9e7e42c9f74059a36dc Mon Sep 17 00:00:00 2001 From: erKam Date: Sat, 7 Nov 2026 16:32:48 +0000 Subject: [PATCH 5/5] fix(overlay): mask a quoted secret after a key chain ending in = In 'Error: context:password="x"' no space follows the '=', so the chain was not read again and the password showed. A chain whose last key is a strong secret name is now read again from that key, unless the chain starts with xmlns:, so namespaced XML attributes stay one name. --- .../src/__tests__/redaction-leak-fixes.test.ts | 2 ++ packages/devtools/src/text-redact.ts | 10 +++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts index 37bc6182..fdc44555 100644 --- a/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts +++ b/packages/devtools/src/__tests__/redaction-leak-fixes.test.ts @@ -73,6 +73,7 @@ describe('a value read again as the next key', () => { 'Error: a:b:c:token: hunter2', 'Error: context:password= "hunter2"', 'Error: context:password= hunter2', + 'Error: context:password="hunter2"', `Error: ${'a:'.repeat(5000)}password: hunter2`, ])('finds a secret after a compact chain of key names: %s', (text) => { expect(redactText(text, [], 'text')).not.toContain('hunter2'); @@ -80,5 +81,6 @@ describe('a value read again as the next key', () => { it('keeps a namespaced attribute after a key as one name', () => { expect(redactText('Log: xmlns:auth="urn:a"', [], 'text')).toContain('urn:a'); + expect(redactText('Log: xmlns:password="urn:a"', [], 'text')).toContain('urn:a'); }); }); diff --git a/packages/devtools/src/text-redact.ts b/packages/devtools/src/text-redact.ts index 70b46eee..f490b5d7 100644 --- a/packages/devtools/src/text-redact.ts +++ b/packages/devtools/src/text-redact.ts @@ -198,12 +198,16 @@ function replacePairs(text: string, pair: PairReplacer): string { // value counts only after `: ` with a space, so `xmlns:auth="urn:a"` stays one name. // For a chain such as `context:password:` only its last key is read again, so a chain // like `a:a:a:` is skipped in one step and the scan stays linear. - // A chain ending in `=` counts only when a space follows, as in `context:password= "x"`, - // so `xmlns:auth="urn:a"` stays one attribute. + // A chain ending in `=` counts when a space follows (`context:password= "x"`) or when its + // last key is a strong secret name outside an XML namespace (`context:password="x"`), so + // `xmlns:auth="urn:a"` stays one attribute. + const chainKey = value.slice(value.lastIndexOf(':', value.length - 2) + 1, -1); const chain = !NEXT_KEY_VALUE.test(value) && KEY_CHAIN_VALUE.test(value) && - (value.endsWith(':') || /\s/.test(text.charAt(at + match.length))); + (value.endsWith(':') || + /\s/.test(text.charAt(at + match.length)) || + (!/^xmlns:/i.test(value) && isStrongSecretKey(decode(chainKey)))); const nextKey = chain || (NEXT_KEY_VALUE.test(value) && (value.endsWith(':') || /\s/.test(eq))); if (replaced === match && value && !/^["']/.test(value) && nextKey) { const lastKey = chain ? value.lastIndexOf(':', value.length - 2) + 1 : 0;