From a859944f7924d3232e286641759f7baaf41998cd Mon Sep 17 00:00:00 2001 From: "Joseph D. Harvey" Date: Fri, 17 Nov 2023 13:28:43 -0500 Subject: [PATCH] fix(core): improve substitute and safeStringify stability --- src/main/core/anonymize/substitute.ts | 16 +--- src/main/core/anonymize/typed-key-map.ts | 42 +++++++++ src/main/core/log/safe-stringify.ts | 28 ++---- .../scopes/jsx/get-tracked-source-files.ts | 7 +- src/test/core/anonymize/substitute.test.ts | 94 +++++++++++++++++++ 5 files changed, 156 insertions(+), 31 deletions(-) create mode 100644 src/main/core/anonymize/typed-key-map.ts create mode 100644 src/test/core/anonymize/substitute.test.ts diff --git a/src/main/core/anonymize/substitute.ts b/src/main/core/anonymize/substitute.ts index bfe0e96a..3fb8b5f6 100644 --- a/src/main/core/anonymize/substitute.ts +++ b/src/main/core/anonymize/substitute.ts @@ -4,9 +4,9 @@ * This source code is licensed under the Apache-2.0 license found in the * LICENSE file in the root directory of this source tree. */ -import { safeStringify } from '../log/safe-stringify.js' +import { TypedKeyMap } from './typed-key-map.js' -const subs = new Map() +const subs = new TypedKeyMap() let curSub = 1 /** @@ -23,12 +23,8 @@ export function substitute>( allowedKeys: Array, allowedValues: unknown[] ): T { - // for each raw entry, map it to something const substitutedEntries = Object.entries(raw).map(([key, value]) => { - // Avoid object equality for complex values - value = safeStringify(value) - - // Key is not safe. substitute key and value + // Key is not safe. Substitute key and value if (!allowedKeys.includes(key)) { if (!subs.has(key)) { subs.set(key, nextSub()) @@ -40,9 +36,7 @@ export function substitute>( return { key: subs.get(key), value: subs.get(value) } } - // Past this point, key is safe - - // Value is a string that's not safe + // Key is safe. Value is a string that's not safe if (typeof value === 'string' && !allowedValues.includes(value)) { if (!subs.has(value)) { subs.set(value, nextSub()) @@ -51,7 +45,7 @@ export function substitute>( return { key, value: subs.get(value) } } - // Value is an object that's not safe + // Key is safe. Value is an object that's not safe if (typeof value === 'object' && !allowedValues.includes(value)) { if (!subs.has(value)) { subs.set(value, nextSub()) diff --git a/src/main/core/anonymize/typed-key-map.ts b/src/main/core/anonymize/typed-key-map.ts new file mode 100644 index 00000000..09c55404 --- /dev/null +++ b/src/main/core/anonymize/typed-key-map.ts @@ -0,0 +1,42 @@ +/* + * Copyright IBM Corp. 2023, 2023 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ +import { safeStringify } from '../log/safe-stringify.js' + +/** + * Extension of a Map that uses a custom key generator. The generator uses the MapKey class to wrap + * incoming keys, including their type and value in the MapKey object. A stringified representation + * of the MapKey is used as the key of the entry in the TypedKeyMap object. + */ +export class TypedKeyMap extends Map { + override has(key: unknown): boolean { + const mapKey = new MapKey(typeof key, key) + + return super.has(safeStringify(mapKey)) + } + + override get(key: unknown) { + const mapKey = new MapKey(typeof key, key) + + return super.get(safeStringify(mapKey)) + } + + override set(key: unknown, value: unknown): this { + const mapKey = new MapKey(typeof key, key) + + return super.set(safeStringify(mapKey), value) + } +} + +/** + * Keys of the TypedKeyMap class. + */ +class MapKey { + constructor( + public readonly type: string, + public readonly val: unknown + ) {} +} diff --git a/src/main/core/log/safe-stringify.ts b/src/main/core/log/safe-stringify.ts index 899ecb67..44b40340 100644 --- a/src/main/core/log/safe-stringify.ts +++ b/src/main/core/log/safe-stringify.ts @@ -4,33 +4,25 @@ * This source code is licensed under the Apache-2.0 license found in the * LICENSE file in the root directory of this source tree. */ +import util from 'util' /** - * Converts any argument to it's string representation. + * Converts any argument to it's string representation using node's util.inspect function. * * @param arg - The element to stringify. * @returns - The string representation of the supplied argument. */ export function safeStringify(arg: unknown): string { - let result - if (typeof arg === 'string') { return arg } - if (arg instanceof Function) { - return (arg.name === '' ? 'unknownFn' : arg.name) + '(...)' - } - - if (result === undefined) { - try { - result = JSON.stringify(arg) - } catch {} - } - - if (result === undefined) { - result = String(arg) - } - - return result + return util.inspect(arg, { + compact: true, + breakLength: Infinity, + depth: 20, + showHidden: false, + maxArrayLength: 10000, + maxStringLength: 10000 + }) } diff --git a/src/main/scopes/jsx/get-tracked-source-files.ts b/src/main/scopes/jsx/get-tracked-source-files.ts index 6d21384b..3f8c92ea 100644 --- a/src/main/scopes/jsx/get-tracked-source-files.ts +++ b/src/main/scopes/jsx/get-tracked-source-files.ts @@ -10,7 +10,6 @@ import path from 'node:path' import * as ts from 'typescript' import { type Logger } from '../../core/log/logger.js' -import { safeStringify } from '../../core/log/safe-stringify.js' import { TrackedFileEnumerator } from '../../core/tracked-file-enumerator.js' /** @@ -48,6 +47,10 @@ export async function getTrackedSourceFiles(root: string, logger: Logger) { const results = await Promise.all(promises) - logger.traceExit('', 'getTrackedSourceFiles', safeStringify(results)) + logger.traceExit( + '', + 'getTrackedSourceFiles', + results.map((result) => result.fileName) + ) return results } diff --git a/src/test/core/anonymize/substitute.test.ts b/src/test/core/anonymize/substitute.test.ts new file mode 100644 index 00000000..2f23c135 --- /dev/null +++ b/src/test/core/anonymize/substitute.test.ts @@ -0,0 +1,94 @@ +/* + * Copyright IBM Corp. 2023, 2023 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ +import { describe, expect, it } from 'vitest' + +import { substitute } from '../../../main/core/anonymize/substitute.js' + +describe('substitute', () => { + it('correctly anonymizes sensitive data', () => { + const obj = { + sensitiveKey: 'sensitive value' + } + const anonymized = substitute(obj, [], []) + + expect(anonymized.sensitiveKey).toBeUndefined() + expect(Object.values(anonymized)).not.toContain('sensitive value') + }) + + it('does not anonymize an allowed key, but anonymizes a sensitive value', () => { + const obj = { + knownKey: 'cool sensitive value' + } + const anonymized = substitute(obj, ['knownKey'], []) + + expect(anonymized.knownKey).not.toBe('cool sensitive value') + }) + + it('does not anonymize an allowed key/value combo', () => { + const obj = { + knownKey: 'known value' + } + const anonymized = substitute(obj, ['knownKey'], ['known value']) + + expect(anonymized).toMatchObject({ + knownKey: 'known value' + }) + }) + + it('reuses a substitution key/value that appears more than once', () => { + const obj1 = { + sensitiveKey: 'sensitive value' + } + const obj2 = { + sensitiveKey: 'sensitive value' + } + + const anon1 = substitute(obj1, [], []) + const anon2 = substitute(obj2, [], []) + + expect(Object.keys(anon1)).toStrictEqual(Object.keys(anon2)) + expect(Object.values(anon1)).toStrictEqual(Object.values(anon2)) + }) + + it('anonymizes a sensitive object', () => { + const obj = { + knownKey: { some: 'object' } + } + + substitute(obj, [], []) + const anonymized = substitute(obj, ['knownKey'], []) + + expect(anonymized).toMatchObject({ + knownKey: '[redacted5]' + }) + }) + + it('anonymizes two identical objects to the same value', () => { + const obj1 = { + knownKey: { some: 'object' } + } + const obj2 = { + knownKey: { some: 'object' } + } + + const anon1 = substitute(obj1, ['knownKey'], []) + const anon2 = substitute(obj2, ['knownKey'], []) + + expect(anon1.knownKey).toStrictEqual(anon2.knownKey) + }) + + it('manifests an object and a string that looks like the object differently', () => { + const obj = { + knownKey1: { some: 'object' }, + knownKey2: "{ some: 'object' }" + } + + const anon = substitute(obj, ['knownKey1', 'knownKey2'], []) + + expect(anon.knownKey1).not.toBe(anon.knownKey2) + }) +})