diff --git a/.changeset/mask-encoded-secrets.md b/.changeset/mask-encoded-secrets.md new file mode 100644 index 0000000000..5e5c51ecc7 --- /dev/null +++ b/.changeset/mask-encoded-secrets.md @@ -0,0 +1,5 @@ +--- +'@redocly/respect-core': patch +--- + +Fixed an issue where secrets masking did not cover encoded secrets in `har-output`. diff --git a/packages/respect-core/src/modules/__tests__/logger-output/mask-secrets.test.ts b/packages/respect-core/src/modules/__tests__/logger-output/mask-secrets.test.ts index 76f8b8b3d7..71f3194269 100644 --- a/packages/respect-core/src/modules/__tests__/logger-output/mask-secrets.test.ts +++ b/packages/respect-core/src/modules/__tests__/logger-output/mask-secrets.test.ts @@ -103,6 +103,67 @@ describe('maskSecrets', () => { expect(result).toEqual('Bearer ********'); }); + it('should mask a URL-encoded occurrence of a secret', () => { + const result = maskSecrets('password=p%40ss+word', new Set(['p@ss word'])); + expect(result).toEqual('password=********'); + }); + + it('should mask a JSON-escaped occurrence of a secret', () => { + const result = maskSecrets(JSON.stringify({ password: 'pa"ss' }), new Set(['pa"ss'])); + expect(result).toEqual('{"password":"********"}'); + }); + + it('should mask every occurrence of a secret within one value', () => { + const result = maskSecrets( + { body: '{"password":"hunter2","confirmPassword":"hunter2"}' }, + new Set(['hunter2']) + ); + expect(result).toEqual({ body: '{"password":"********","confirmPassword":"********"}' }); + }); + + it('should mask secrets everywhere in a HAR capture', () => { + const har = { + log: { + entries: [ + { + request: { + headers: [{ name: 'authorization', value: 'Bearer hunter2' }], + postData: { + mimeType: 'application/x-www-form-urlencoded', + text: 'password=p%40ss+word', + }, + }, + response: { + content: { mimeType: 'application/json', text: '{"token":"hunter2"}' }, + }, + }, + ], + }, + }; + + const result = maskSecrets(har, new Set(['hunter2', 'p@ss word'])); + + expect(result.log.entries[0].request.headers[0].value).toBe('Bearer ********'); + expect(result.log.entries[0].request.postData.text).toBe('password=********'); + expect(result.log.entries[0].response.content.text).toBe('{"token":"********"}'); + }); + + it('should mask a longer secret that starts with a shorter one', () => { + const result = maskSecrets('Bearer passphrase123', new Set(['pass', 'passphrase123'])); + expect(result).toEqual('Bearer ********'); + }); + + it('should ignore a whitespace-only secret', () => { + const result = maskSecrets('filter=a+b', new Set([' '])); + expect(result).toEqual('filter=a+b'); + }); + + it('should mask a secret containing a lone surrogate', () => { + const secret = `tok${String.fromCharCode(0xd800)}en`; + const result = maskSecrets({ access_token: secret }, new Set([secret])); + expect(result).toEqual({ access_token: '********' }); + }); + it('should preserve ArrayBuffer objects without breaking them', () => { const originalArrayBuffer = new ArrayBuffer(8); const originalData = new Uint8Array(originalArrayBuffer); diff --git a/packages/respect-core/src/modules/logger-output/mask-secrets.ts b/packages/respect-core/src/modules/logger-output/mask-secrets.ts index fa17917746..3bc5a7fc0e 100644 --- a/packages/respect-core/src/modules/logger-output/mask-secrets.ts +++ b/packages/respect-core/src/modules/logger-output/mask-secrets.ts @@ -10,39 +10,56 @@ export const POTENTIALLY_SECRET_FIELDS = [ 'client_secret', ]; +/** + * A secret can appear in a log or capture already encoded: a JSON body + * escapes quotes and backslashes, a form-urlencoded body or a URL + * percent-encodes special characters. Mask those variants along with the + * raw value. + */ +function collectSecretPatterns(secretsSet: Set): string[] { + const patterns = new Set(); + for (const secret of secretsSet) { + if (!secret.trim()) continue; + patterns.add(secret); + patterns.add(JSON.stringify(secret).slice(1, -1)); + // A lone surrogate reaching us through a captured `\uD800` escape makes + // encodeURIComponent throw; URLSearchParams substitutes U+FFFD instead. + if (secret.isWellFormed()) { + patterns.add(encodeURIComponent(secret)); + } + // URLSearchParams serializes to `secret=value`; drop the key to keep + // the form-urlencoded variant (spaces become `+`, unlike encodeURIComponent). + patterns.add(new URLSearchParams([['secret', secret]]).toString().slice('secret='.length)); + } + // Longest first: a short secret nested in a longer one would otherwise mask + // only the prefix and leave the rest of the longer secret visible. + return Array.from(patterns).sort((left, right) => right.length - left.length); +} + export function maskSecrets( target: T, secretsSet: Set ): T { - const maskValue = (value: string, secret: string): string => { - return value.replace(secret, '*'.repeat(8)); - }; - - if (typeof target === 'string') { - let maskedString = target as string; - secretsSet.forEach((secret) => { - maskedString = maskedString.split(secret).join('*'.repeat(8)); - }); - return maskedString as T; - } - - const masked = deepCopy(target); - const maskIfContainsSecret = (value: string): string => { + const patterns = collectSecretPatterns(secretsSet); + const maskString = (value: string): string => { let maskedValue = value; - - for (const secret of secretsSet) { - if (maskedValue.includes(secret)) { - maskedValue = maskValue(maskedValue, secret); + for (const pattern of patterns) { + if (maskedValue.includes(pattern)) { + maskedValue = maskedValue.split(pattern).join('*'.repeat(8)); } } - return maskedValue; }; + if (typeof target === 'string') { + return maskString(target) as T; + } + + const masked = deepCopy(target); const maskRecursive = (current: any) => { for (const key in current) { if (typeof current[key] === 'string') { - current[key] = maskIfContainsSecret(current[key]); + current[key] = maskString(current[key]); } else if (isPlainObject(current[key]) || Array.isArray(current[key])) { // Skip special objects that should not be modified if (