diff --git a/integration-tests/debugger/redact.spec.js b/integration-tests/debugger/redact.spec.js new file mode 100644 index 00000000000..62a948b80a8 --- /dev/null +++ b/integration-tests/debugger/redact.spec.js @@ -0,0 +1,49 @@ +'use strict' + +const { assert } = require('chai') +const { setup } = require('./utils') + +// Default settings is tested in unit tests, so we only need to test the env vars here +describe('Dynamic Instrumentation snapshot PII redaction', function () { + describe('DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS=foo,bar', function () { + const t = setup({ env: { DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS: 'foo,bar' } }) + + it('should respect DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS', function (done) { + t.triggerBreakpoint() + + t.agent.on('debugger-input', ({ payload: [{ 'debugger.snapshot': { captures } }] }) => { + const { locals } = captures.lines[t.breakpoint.line] + + assert.deepPropertyVal(locals, 'foo', { type: 'string', notCapturedReason: 'redactedIdent' }) + assert.deepPropertyVal(locals, 'bar', { type: 'string', notCapturedReason: 'redactedIdent' }) + assert.deepPropertyVal(locals, 'baz', { type: 'string', value: 'c' }) + + // existing redaction should not be impacted + assert.deepPropertyVal(locals, 'secret', { type: 'string', notCapturedReason: 'redactedIdent' }) + + done() + }) + + t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true })) + }) + }) + + describe('DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS=secret', function () { + const t = setup({ env: { DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS: 'secret' } }) + + it('should respect DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS', function (done) { + t.triggerBreakpoint() + + t.agent.on('debugger-input', ({ payload: [{ 'debugger.snapshot': { captures } }] }) => { + const { locals } = captures.lines[t.breakpoint.line] + + assert.deepPropertyVal(locals, 'secret', { type: 'string', value: 'shh!' }) + assert.deepPropertyVal(locals, 'password', { type: 'string', notCapturedReason: 'redactedIdent' }) + + done() + }) + + t.agent.addRemoteConfig(t.generateRemoteConfig({ captureSnapshot: true })) + }) + }) +}) diff --git a/integration-tests/debugger/target-app/redact.js b/integration-tests/debugger/target-app/redact.js new file mode 100644 index 00000000000..3ac7b51953c --- /dev/null +++ b/integration-tests/debugger/target-app/redact.js @@ -0,0 +1,26 @@ +'use strict' + +require('dd-trace/init') +const Fastify = require('fastify') + +const fastify = Fastify() + +fastify.get('/', function () { + /* eslint-disable no-unused-vars */ + const foo = 'a' + const bar = 'b' + const baz = 'c' + const secret = 'shh!' + const password = 'shh!' + /* eslint-enable no-unused-vars */ + + return { hello: 'world' } // BREAKPOINT: / +}) + +fastify.listen({ port: process.env.APP_PORT }, (err) => { + if (err) { + fastify.log.error(err) + process.exit(1) + } + process.send({ port: process.env.APP_PORT }) +}) diff --git a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js index ec6e2a1fd75..8cf52e709f6 100644 --- a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js +++ b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js @@ -1,7 +1,7 @@ 'use strict' const { join } = require('path') -const { Worker } = require('worker_threads') +const { Worker, threadId: parentThreadId } = require('worker_threads') const { randomUUID } = require('crypto') const log = require('../../log') @@ -46,29 +46,47 @@ class TestVisDynamicInstrumentation { return this._readyPromise } - start () { + start (config) { if (this.worker) return const { NODE_OPTIONS, ...envWithoutNodeOptions } = process.env log.debug('Starting Test Visibility - Dynamic Instrumentation client...') + const rcChannel = new MessageChannel() // mock channel + const configChannel = new MessageChannel() // mock channel + this.worker = new Worker( join(__dirname, 'worker', 'index.js'), { execArgv: [], env: envWithoutNodeOptions, workerData: { + config: config.serialize(), + parentThreadId, + rcPort: rcChannel.port1, + configPort: configChannel.port1, breakpointSetChannel: this.breakpointSetChannel.port1, breakpointHitChannel: this.breakpointHitChannel.port1 }, - transferList: [this.breakpointSetChannel.port1, this.breakpointHitChannel.port1] + transferList: [ + rcChannel.port1, + configChannel.port1, + this.breakpointSetChannel.port1, + this.breakpointHitChannel.port1 + ] } ) this.worker.on('online', () => { log.debug('Test Visibility - Dynamic Instrumentation client is ready') this._onReady() }) + this.worker.on('error', (err) => { + log.error('Test Visibility - Dynamic Instrumentation worker error', err) + }) + this.worker.on('messageerror', (err) => { + log.error('Test Visibility - Dynamic Instrumentation worker messageerror', err) + }) // Allow the parent to exit even if the worker is still running this.worker.unref() diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index a16df70ee07..09ce9d5fd66 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -473,6 +473,8 @@ class Config { this._setValue(defaults, 'dogstatsd.port', '8125') this._setValue(defaults, 'dsmEnabled', false) this._setValue(defaults, 'dynamicInstrumentationEnabled', false) + this._setValue(defaults, 'dynamicInstrumentationRedactedIdentifiers', []) + this._setValue(defaults, 'dynamicInstrumentationRedactionExcludedIdentifiers', []) this._setValue(defaults, 'env', undefined) this._setValue(defaults, 'experimental.enableGetRumData', false) this._setValue(defaults, 'experimental.exporter', undefined) @@ -600,6 +602,8 @@ class Config { DD_DOGSTATSD_HOST, DD_DOGSTATSD_PORT, DD_DYNAMIC_INSTRUMENTATION_ENABLED, + DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS, + DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS, DD_ENV, DD_EXPERIMENTAL_API_SECURITY_ENABLED, DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED, @@ -747,6 +751,12 @@ class Config { this._setString(env, 'dogstatsd.port', DD_DOGSTATSD_PORT) this._setBoolean(env, 'dsmEnabled', DD_DATA_STREAMS_ENABLED) this._setBoolean(env, 'dynamicInstrumentationEnabled', DD_DYNAMIC_INSTRUMENTATION_ENABLED) + this._setArray(env, 'dynamicInstrumentationRedactedIdentifiers', DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS) + this._setArray( + env, + 'dynamicInstrumentationRedactionExcludedIdentifiers', + DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS + ) this._setString(env, 'env', DD_ENV || tags.env) this._setBoolean(env, 'traceEnabled', DD_TRACE_ENABLED) this._setBoolean(env, 'experimental.enableGetRumData', DD_TRACE_EXPERIMENTAL_GET_RUM_DATA_ENABLED) @@ -927,6 +937,16 @@ class Config { } this._setBoolean(opts, 'dsmEnabled', options.dsmEnabled) this._setBoolean(opts, 'dynamicInstrumentationEnabled', options.experimental?.dynamicInstrumentationEnabled) + this._setArray( + opts, + 'dynamicInstrumentationRedactedIdentifiers', + options.experimental?.dynamicInstrumentationRedactedIdentifiers + ) + this._setArray( + opts, + 'dynamicInstrumentationRedactionExcludedIdentifiers', + options.experimental?.dynamicInstrumentationRedactionExcludedIdentifiers + ) this._setString(opts, 'env', options.env || tags.env) this._setBoolean(opts, 'experimental.enableGetRumData', options.experimental?.enableGetRumData) this._setString(opts, 'experimental.exporter', options.experimental?.exporter) @@ -1312,6 +1332,22 @@ class Config { this.sampler.sampleRate = this.sampleRate updateConfig(changes, this) } + + // TODO: Refactor the Config class so it never produces any config objects that are incompatible with MessageChannel + /** + * Serializes the config object so it can be passed over a Worker Thread MessageChannel. + * @returns {Object} The serialized config object. + */ + serialize () { + // URL objects cannot be serialized over the MessageChannel, so we need to convert them to strings first + if (this.url instanceof URL) { + const config = { ...this } + config.url = this.url.toString() + return config + } + + return this + } } function maybeInt (number) { diff --git a/packages/dd-trace/src/debugger/devtools_client/config.js b/packages/dd-trace/src/debugger/devtools_client/config.js index 950d8938872..4880bbe5fdb 100644 --- a/packages/dd-trace/src/debugger/devtools_client/config.js +++ b/packages/dd-trace/src/debugger/devtools_client/config.js @@ -5,6 +5,8 @@ const { format } = require('node:url') const log = require('../../log') const config = module.exports = { + dynamicInstrumentationRedactedIdentifiers: parentConfig.dynamicInstrumentationRedactedIdentifiers, + dynamicInstrumentationRedactionExcludedIdentifiers: parentConfig.dynamicInstrumentationRedactionExcludedIdentifiers, runtimeId: parentConfig.tags['runtime-id'], service: parentConfig.service, commitSHA: parentConfig.commitSHA, diff --git a/packages/dd-trace/src/debugger/devtools_client/snapshot/processor.js b/packages/dd-trace/src/debugger/devtools_client/snapshot/processor.js index ea52939ab0e..a7b14987987 100644 --- a/packages/dd-trace/src/debugger/devtools_client/snapshot/processor.js +++ b/packages/dd-trace/src/debugger/devtools_client/snapshot/processor.js @@ -1,6 +1,7 @@ 'use strict' const { collectionSizeSym, fieldCountSym } = require('./symbols') +const { normalizeName, REDACTED_IDENTIFIERS } = require('./redaction') module.exports = { processRawState: processProperties @@ -24,7 +25,14 @@ function processProperties (props, maxLength) { return result } +// TODO: Improve performance of redaction algorithm. +// This algorithm is probably slower than if we embedded the redaction logic inside the functions below. +// That way we didn't have to traverse objects that will just be redacted anyway. function getPropertyValue (prop, maxLength) { + return redact(prop, getPropertyValueRaw(prop, maxLength)) +} + +function getPropertyValueRaw (prop, maxLength) { // Special case for getters and setters which does not have a value property if ('get' in prop) { const hasGet = prop.get.type !== 'undefined' @@ -185,8 +193,11 @@ function toMap (type, pairs, maxLength) { // `pair.value` is a special wrapper-object with subtype `internal#entry`. This can be skipped and we can go // directly to its children, of which there will always be exactly two, the first containing the key, and the // second containing the value of this entry of the Map. + const shouldRedact = shouldRedactMapValue(pair.value.properties[0]) const key = getPropertyValue(pair.value.properties[0], maxLength) - const val = getPropertyValue(pair.value.properties[1], maxLength) + const val = shouldRedact + ? notCapturedRedacted(pair.value.properties[1].value.type) + : getPropertyValue(pair.value.properties[1], maxLength) result.entries[i++] = [key, val] } @@ -240,6 +251,25 @@ function arrayBufferToString (bytes, size) { return buf.toString() } +function redact (prop, obj) { + const name = getNormalizedNameFromProp(prop) + return REDACTED_IDENTIFIERS.has(name) ? notCapturedRedacted(obj.type) : obj +} + +function shouldRedactMapValue (key) { + const isSymbol = key.value.type === 'symbol' + if (!isSymbol && key.value.type !== 'string') return false // WeakMaps uses objects as keys + const name = normalizeName( + isSymbol ? key.value.description : key.value.value, + isSymbol + ) + return REDACTED_IDENTIFIERS.has(name) +} + +function getNormalizedNameFromProp (prop) { + return normalizeName(prop.name, 'symbol' in prop) +} + function setNotCaptureReasonOnCollection (result, collection) { if (collectionSizeSym in collection) { result.notCapturedReason = 'collectionSize' @@ -250,3 +280,7 @@ function setNotCaptureReasonOnCollection (result, collection) { function notCapturedDepth (type) { return { type, notCapturedReason: 'depth' } } + +function notCapturedRedacted (type) { + return { type, notCapturedReason: 'redactedIdent' } +} diff --git a/packages/dd-trace/src/debugger/devtools_client/snapshot/redaction.js b/packages/dd-trace/src/debugger/devtools_client/snapshot/redaction.js new file mode 100644 index 00000000000..5ccb58f4053 --- /dev/null +++ b/packages/dd-trace/src/debugger/devtools_client/snapshot/redaction.js @@ -0,0 +1,116 @@ +'use strict' + +const config = require('../config') + +const excludedIdentifiers = config.dynamicInstrumentationRedactionExcludedIdentifiers.map((name) => normalizeName(name)) + +const REDACTED_IDENTIFIERS = new Set( + [ + '2fa', + '_csrf', + '_csrf_token', + '_session', + '_xsrf', + 'access_token', + 'address', + 'aiohttp_session', + 'api_key', + 'apisecret', + 'apisignature', + 'applicationkey', + 'appkey', + 'auth', + 'authtoken', + 'authorization', + 'cc_number', + 'certificatepin', + 'cipher', + 'client_secret', + 'clientid', + 'config', + 'connect.sid', + 'connectionstring', + 'cookie', + 'credentials', + 'creditcard', + 'csrf', + 'csrf_token', + 'cvv', + 'databaseurl', + 'db_url', + 'email', + 'encryption_key', + 'encryptionkeyid', + 'geo_location', + 'gpg_key', + 'ip_address', + 'jti', + 'jwt', + 'license_key', + 'masterkey', + 'mysql_pwd', + 'nonce', + 'oauth', + 'oauthtoken', + 'otp', + 'passhash', + 'passwd', + 'password', + 'passwordb', + 'pem_file', + 'pgp_key', + 'PHPSESSID', + 'phonenumber', + 'pin', + 'pincode', + 'pkcs8', + 'private_key', + 'publickey', + 'pwd', + 'recaptcha_key', + 'refresh_token', + 'remote_addr', + 'routingnumber', + 'salt', + 'secret', + 'secretKey', + 'securitycode', + 'security_answer', + 'security_question', + 'serviceaccountcredentials', + 'session', + 'sessionid', + 'sessionkey', + 'set_cookie', + 'signature', + 'signaturekey', + 'ssh_key', + 'ssn', + 'symfony', + 'token', + 'transactionid', + 'twilio_token', + 'user_session', + 'uuid', + 'voterid', + 'x-auth-token', + 'x_api_key', + 'x_csrftoken', + 'x_forwarded_for', + 'x_real_ip', + 'XSRF-TOKEN', + ...config.dynamicInstrumentationRedactedIdentifiers + ] + .map((name) => normalizeName(name)) + .filter((name) => excludedIdentifiers.includes(name) === false) +) + +function normalizeName (name, isSymbol) { + if (isSymbol) name = name.slice(7, -1) // Remove `Symbol(` and `)` + return name.toLowerCase().replace(/[-_@$.]/g, '') +} + +module.exports = { + REDACTED_IDENTIFIERS, + normalizeName +} diff --git a/packages/dd-trace/src/debugger/index.js b/packages/dd-trace/src/debugger/index.js index fee514f32f1..a1a94d9e321 100644 --- a/packages/dd-trace/src/debugger/index.js +++ b/packages/dd-trace/src/debugger/index.js @@ -48,7 +48,7 @@ function start (config, rc) { execArgv: [], // Avoid worker thread inheriting the `-r` command line argument env, // Avoid worker thread inheriting the `NODE_OPTIONS` environment variable (in case it contains `-r`) workerData: { - config: serializableConfig(config), + config: config.serialize(), parentThreadId, rcPort: rcChannel.port1, configPort: configChannel.port1 @@ -88,16 +88,5 @@ function start (config, rc) { function configure (config) { if (configChannel === null) return - configChannel.port2.postMessage(serializableConfig(config)) -} - -// TODO: Refactor the Config class so it never produces any config objects that are incompatible with MessageChannel -function serializableConfig (config) { - // URL objects cannot be serialized over the MessageChannel, so we need to convert them to strings first - if (config.url instanceof URL) { - config = { ...config } - config.url = config.url.toString() - } - - return config + configChannel.port2.postMessage(config.serialize()) } diff --git a/packages/dd-trace/src/proxy.js b/packages/dd-trace/src/proxy.js index fd814c9d6e3..874945eeecc 100644 --- a/packages/dd-trace/src/proxy.js +++ b/packages/dd-trace/src/proxy.js @@ -184,7 +184,7 @@ class Tracer extends NoopProxy { if (config.isTestDynamicInstrumentationEnabled) { const testVisibilityDynamicInstrumentation = require('./ci-visibility/dynamic-instrumentation') - testVisibilityDynamicInstrumentation.start() + testVisibilityDynamicInstrumentation.start(config) } } catch (e) { log.error('Error initialising tracer', e) diff --git a/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/target-app/test-visibility-dynamic-instrumentation-script.js b/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/target-app/test-visibility-dynamic-instrumentation-script.js index fedfaefdc6c..39382ea0089 100644 --- a/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/target-app/test-visibility-dynamic-instrumentation-script.js +++ b/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/target-app/test-visibility-dynamic-instrumentation-script.js @@ -3,11 +3,12 @@ const path = require('path') const tvDynamicInstrumentation = require('../../../../src/ci-visibility/dynamic-instrumentation') const sum = require('./di-dependency') +const Config = require('../../../../src/config') // keep process alive const intervalId = setInterval(() => {}, 5000) -tvDynamicInstrumentation.start() +tvDynamicInstrumentation.start(new Config()) tvDynamicInstrumentation.isReady().then(() => { const [ diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index 8e87b6fa855..49e691afae8 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -232,6 +232,8 @@ describe('Config', () => { expect(config).to.have.property('logLevel', 'debug') expect(config).to.have.nested.property('codeOriginForSpans.enabled', false) expect(config).to.have.property('dynamicInstrumentationEnabled', false) + expect(config).to.have.deep.property('dynamicInstrumentationRedactedIdentifiers', []) + expect(config).to.have.deep.property('dynamicInstrumentationRedactionExcludedIdentifiers', []) expect(config).to.have.property('traceId128BitGenerationEnabled', true) expect(config).to.have.property('traceId128BitLoggingEnabled', false) expect(config).to.have.property('spanAttributeSchema', 'v0') @@ -314,6 +316,8 @@ describe('Config', () => { { name: 'dogstatsd.port', value: '8125', origin: 'default' }, { name: 'dsmEnabled', value: false, origin: 'default' }, { name: 'dynamicInstrumentationEnabled', value: false, origin: 'default' }, + { name: 'dynamicInstrumentationRedactedIdentifiers', value: [], origin: 'default' }, + { name: 'dynamicInstrumentationRedactionExcludedIdentifiers', value: [], origin: 'default' }, { name: 'env', value: undefined, origin: 'default' }, { name: 'experimental.enableGetRumData', value: false, origin: 'default' }, { name: 'experimental.exporter', value: undefined, origin: 'default' }, @@ -457,6 +461,8 @@ describe('Config', () => { process.env.DD_TRACE_REPORT_HOSTNAME = 'true' process.env.DD_ENV = 'test' process.env.DD_DYNAMIC_INSTRUMENTATION_ENABLED = 'true' + process.env.DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS = 'foo,bar' + process.env.DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS = 'a,b,c' process.env.DD_TRACE_GLOBAL_TAGS = 'foo:bar,baz:qux' process.env.DD_TRACE_SAMPLE_RATE = '0.5' process.env.DD_TRACE_RATE_LIMIT = '-1' @@ -552,6 +558,8 @@ describe('Config', () => { expect(config).to.have.property('reportHostname', true) expect(config).to.have.nested.property('codeOriginForSpans.enabled', true) expect(config).to.have.property('dynamicInstrumentationEnabled', true) + expect(config).to.have.deep.property('dynamicInstrumentationRedactedIdentifiers', ['foo', 'bar']) + expect(config).to.have.deep.property('dynamicInstrumentationRedactionExcludedIdentifiers', ['a', 'b', 'c']) expect(config).to.have.property('env', 'test') expect(config).to.have.property('sampleRate', 0.5) expect(config).to.have.property('traceEnabled', true) @@ -656,6 +664,8 @@ describe('Config', () => { { name: 'dogstatsd.hostname', value: 'dsd-agent', origin: 'env_var' }, { name: 'dogstatsd.port', value: '5218', origin: 'env_var' }, { name: 'dynamicInstrumentationEnabled', value: true, origin: 'env_var' }, + { name: 'dynamicInstrumentationRedactedIdentifiers', value: ['foo', 'bar'], origin: 'env_var' }, + { name: 'dynamicInstrumentationRedactionExcludedIdentifiers', value: ['a', 'b', 'c'], origin: 'env_var' }, { name: 'env', value: 'test', origin: 'env_var' }, { name: 'experimental.enableGetRumData', value: true, origin: 'env_var' }, { name: 'experimental.exporter', value: 'log', origin: 'env_var' }, @@ -851,6 +861,8 @@ describe('Config', () => { experimental: { b3: true, dynamicInstrumentationEnabled: true, + dynamicInstrumentationRedactedIdentifiers: ['foo', 'bar'], + dynamicInstrumentationRedactionExcludedIdentifiers: ['a', 'b', 'c'], traceparent: true, runtimeId: true, exporter: 'log', @@ -896,6 +908,8 @@ describe('Config', () => { expect(config).to.have.property('service', 'service') expect(config).to.have.property('version', '0.1.0') expect(config).to.have.property('dynamicInstrumentationEnabled', true) + expect(config).to.have.deep.property('dynamicInstrumentationRedactedIdentifiers', ['foo', 'bar']) + expect(config).to.have.deep.property('dynamicInstrumentationRedactionExcludedIdentifiers', ['a', 'b', 'c']) expect(config).to.have.property('env', 'test') expect(config).to.have.property('sampleRate', 0.5) expect(config).to.have.property('logger', logger) @@ -974,6 +988,8 @@ describe('Config', () => { { name: 'dogstatsd.hostname', value: 'agent-dsd', origin: 'code' }, { name: 'dogstatsd.port', value: '5218', origin: 'code' }, { name: 'dynamicInstrumentationEnabled', value: true, origin: 'code' }, + { name: 'dynamicInstrumentationRedactedIdentifiers', value: ['foo', 'bar'], origin: 'code' }, + { name: 'dynamicInstrumentationRedactionExcludedIdentifiers', value: ['a', 'b', 'c'], origin: 'code' }, { name: 'env', value: 'test', origin: 'code' }, { name: 'experimental.enableGetRumData', value: true, origin: 'code' }, { name: 'experimental.exporter', value: 'log', origin: 'code' }, @@ -1175,6 +1191,8 @@ describe('Config', () => { process.env.DD_TRACE_REPORT_HOSTNAME = 'true' process.env.DD_ENV = 'test' process.env.DD_DYNAMIC_INSTRUMENTATION_ENABLED = 'true' + process.env.DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS = 'foo,bar' + process.env.DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS = 'a,b,c' process.env.DD_API_KEY = '123' process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = 'v0' process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'false' @@ -1253,6 +1271,8 @@ describe('Config', () => { experimental: { b3: false, dynamicInstrumentationEnabled: false, + dynamicInstrumentationRedactedIdentifiers: ['foo2', 'bar2'], + dynamicInstrumentationRedactionExcludedIdentifiers: ['a2', 'b2'], traceparent: false, runtimeId: false, exporter: 'agent', @@ -1318,6 +1338,8 @@ describe('Config', () => { expect(config).to.have.property('version', '1.0.0') expect(config).to.have.nested.property('codeOriginForSpans.enabled', false) expect(config).to.have.property('dynamicInstrumentationEnabled', false) + expect(config).to.have.deep.property('dynamicInstrumentationRedactedIdentifiers', ['foo2', 'bar2']) + expect(config).to.have.deep.property('dynamicInstrumentationRedactionExcludedIdentifiers', ['a2', 'b2']) expect(config).to.have.property('env', 'development') expect(config).to.have.property('clientIpEnabled', true) expect(config).to.have.property('clientIpHeader', 'x-true-client-ip') diff --git a/packages/dd-trace/test/debugger/devtools_client/snapshot/redaction.spec.js b/packages/dd-trace/test/debugger/devtools_client/snapshot/redaction.spec.js new file mode 100644 index 00000000000..cd1b4a959a8 --- /dev/null +++ b/packages/dd-trace/test/debugger/devtools_client/snapshot/redaction.spec.js @@ -0,0 +1,90 @@ +'use strict' + +require('../../../setup/mocha') + +const { expect } = require('chai') +const { getTargetCodePath, enable, teardown, assertOnBreakpoint, setAndTriggerBreakpoint } = require('./utils') + +const target = getTargetCodePath(__filename) +const BREAKPOINT_LINE_NUMBER = 32 + +describe('debugger -> devtools client -> snapshot.getLocalStateForCallFrame', function () { + describe('redaction', function () { + beforeEach(enable(__filename)) + + afterEach(teardown) + + // Non-default configuration is tested in the integration tests + it('should replace PII in keys/properties/variables with expected notCapturedReason', function (done) { + assertOnBreakpoint(done, (state) => { + expect(state).to.have.all.keys( + 'nonNormalizedSecretToken', 'foo', 'secret', 'Se_cret_$', 'weakMapKey', 'obj' + ) + + expect(state).to.have.deep.property('foo', { type: 'string', value: 'bar' }) + expect(state).to.have.deep.property('secret', { type: 'string', notCapturedReason: 'redactedIdent' }) + expect(state).to.have.deep.property('Se_cret_$', { type: 'string', notCapturedReason: 'redactedIdent' }) + expect(state).to.have.deep.property('weakMapKey', { + type: 'Object', + fields: { secret: { type: 'string', notCapturedReason: 'redactedIdent' } } + }) + expect(state).to.have.deep.property('obj') + expect(state.obj).to.have.property('type', 'Object') + + const { fields } = state.obj + expect(fields).to.have.all.keys( + 'foo', 'secret', '@Se-cret_$_', 'nested', 'arr', 'map', 'weakmap', 'password', + 'Symbol(secret)', 'Symbol(@Se-cret_$_)' + ) + + expect(fields).to.have.deep.property('foo', { type: 'string', value: 'bar' }) + expect(fields).to.have.deep.property('secret', { type: 'string', notCapturedReason: 'redactedIdent' }) + expect(fields).to.have.deep.property('@Se-cret_$_', { type: 'string', notCapturedReason: 'redactedIdent' }) + expect(fields).to.have.deep.property('nested', { + type: 'Object', + fields: { secret: { type: 'string', notCapturedReason: 'redactedIdent' } } + }) + expect(fields).to.have.deep.property('arr', { + type: 'Array', + elements: [{ type: 'Object', fields: { secret: { type: 'string', notCapturedReason: 'redactedIdent' } } }] + }) + expect(fields).to.have.deep.property('map', { + type: 'Map', + entries: [ + [ + { type: 'string', value: 'foo' }, + { type: 'string', value: 'bar' } + ], + [ + { type: 'string', value: 'secret' }, + { type: 'string', notCapturedReason: 'redactedIdent' } + ], + [ + { type: 'string', value: '@Se-cret_$.' }, + { type: 'string', notCapturedReason: 'redactedIdent' } + ], + [ + { type: 'symbol', value: 'Symbol(secret)' }, + { type: 'string', notCapturedReason: 'redactedIdent' } + ], + [ + { type: 'symbol', value: 'Symbol(@Se-cret_$.)' }, + { notCapturedReason: 'redactedIdent', type: 'string' } + ] + ] + }) + expect(fields).to.have.deep.property('weakmap', { + type: 'WeakMap', + entries: [[ + { type: 'Object', fields: { secret: { type: 'string', notCapturedReason: 'redactedIdent' } } }, + { type: 'number', value: '42' } + ]] + }) + expect(fields).to.have.deep.property('password', { type: 'string', notCapturedReason: 'redactedIdent' }) + expect(fields).to.have.deep.property('Symbol(secret)', { type: 'string', notCapturedReason: 'redactedIdent' }) + }) + + setAndTriggerBreakpoint(target, BREAKPOINT_LINE_NUMBER) + }) + }) +}) diff --git a/packages/dd-trace/test/debugger/devtools_client/snapshot/target-code/redaction.js b/packages/dd-trace/test/debugger/devtools_client/snapshot/target-code/redaction.js new file mode 100644 index 00000000000..45e76a23a9c --- /dev/null +++ b/packages/dd-trace/test/debugger/devtools_client/snapshot/target-code/redaction.js @@ -0,0 +1,35 @@ +'use strict' + +function run () { + const nonNormalizedSecretToken = '@Se-cret_$.' + const foo = 'bar' // eslint-disable-line no-unused-vars + const secret = 'shh!' + const Se_cret_$ = 'shh!' // eslint-disable-line camelcase, no-unused-vars + const weakMapKey = { secret: 'shh!' } + const obj = { + foo: 'bar', + secret, + [nonNormalizedSecretToken]: 'shh!', + nested: { secret: 'shh!' }, + arr: [{ secret: 'shh!' }], + map: new Map([ + ['foo', 'bar'], + ['secret', 'shh!'], + [nonNormalizedSecretToken, 'shh!'], + [Symbol('secret'), 'shh!'], + [Symbol(nonNormalizedSecretToken), 'shh!'] + ]), + weakmap: new WeakMap([[weakMapKey, 42]]), + [Symbol('secret')]: 'shh!', + [Symbol(nonNormalizedSecretToken)]: 'shh!' + } + + Object.defineProperty(obj, 'password', { + value: 'shh!', + enumerable: false + }) + + return obj // breakpoint at this line +} + +module.exports = { run } diff --git a/packages/dd-trace/test/debugger/devtools_client/snapshot/utils.js b/packages/dd-trace/test/debugger/devtools_client/snapshot/utils.js index 215b93a4002..22f7610205f 100644 --- a/packages/dd-trace/test/debugger/devtools_client/snapshot/utils.js +++ b/packages/dd-trace/test/debugger/devtools_client/snapshot/utils.js @@ -10,6 +10,13 @@ session['@noCallThru'] = true proxyquire('../src/debugger/devtools_client/snapshot/collector', { '../session': session }) +proxyquire('../src/debugger/devtools_client/snapshot/redaction', { + '../config': { + dynamicInstrumentationRedactedIdentifiers: [], + dynamicInstrumentationRedactionExcludedIdentifiers: [], + '@noCallThru': true + } +}) const { getLocalStateForCallFrame } = require('../../../../src/debugger/devtools_client/snapshot') @@ -75,16 +82,16 @@ async function setAndTriggerBreakpoint (path, line) { run() } -function assertOnBreakpoint (done, config, callback) { - if (typeof config === 'function') { - callback = config - config = undefined +function assertOnBreakpoint (done, snapshotConfig, callback) { + if (typeof snapshotConfig === 'function') { + callback = snapshotConfig + snapshotConfig = undefined } session.once('Debugger.paused', ({ params }) => { expect(params.hitBreakpoints.length).to.eq(1) - getLocalStateForCallFrame(params.callFrames[0], config).then((process) => { + getLocalStateForCallFrame(params.callFrames[0], snapshotConfig).then((process) => { callback(process()) done() }).catch(done)