diff --git a/lib/core/connection/apm.js b/lib/core/connection/apm.js index 1acf22cb7ab..f6e293b4293 100644 --- a/lib/core/connection/apm.js +++ b/lib/core/connection/apm.js @@ -17,6 +17,8 @@ const SENSITIVE_COMMANDS = new Set([ 'copydb' ]); +const HELLO_COMMANDS = new Set(['hello', 'ismaster', 'isMaster']); + // helper methods const extractCommandName = commandDoc => Object.keys(commandDoc)[0]; const namespace = command => command.ns; @@ -24,7 +26,11 @@ const databaseName = command => command.ns.split('.')[0]; const collectionName = command => command.ns.split('.')[1]; const generateConnectionId = pool => pool.options ? `${pool.options.host}:${pool.options.port}` : pool.address; -const maybeRedact = (commandName, result) => (SENSITIVE_COMMANDS.has(commandName) ? {} : result); +const maybeRedact = (commandName, cmd, result) => + SENSITIVE_COMMANDS.has(commandName) || + (HELLO_COMMANDS.has(commandName) && cmd.speculativeAuthenticate) + ? {} + : result; const isLegacyPool = pool => pool.s && pool.queue; const LEGACY_FIND_QUERY_MAP = { @@ -181,17 +187,11 @@ class CommandStartedEvent { const commandName = extractCommandName(cmd); const connectionDetails = extractConnectionDetails(pool); - // NOTE: remove in major revision, this is not spec behavior - if (SENSITIVE_COMMANDS.has(commandName)) { - this.commandObj = {}; - this.commandObj[commandName] = true; - } - Object.assign(this, connectionDetails, { requestId: command.requestId, databaseName: databaseName(command), commandName, - command: cmd + command: maybeRedact(commandName, cmd, cmd) }); } } @@ -215,7 +215,7 @@ class CommandSucceededEvent { requestId: command.requestId, commandName, duration: calculateDurationInMs(started), - reply: maybeRedact(commandName, extractReply(command, reply)) + reply: maybeRedact(commandName, cmd, extractReply(command, reply)) }); } } @@ -239,7 +239,7 @@ class CommandFailedEvent { requestId: command.requestId, commandName, duration: calculateDurationInMs(started), - failure: maybeRedact(commandName, error) + failure: maybeRedact(commandName, cmd, error) }); } } diff --git a/test/functional/apm.test.js b/test/functional/apm.test.js index a0bcb539328..5a7350c02f6 100644 --- a/test/functional/apm.test.js +++ b/test/functional/apm.test.js @@ -622,7 +622,7 @@ describe('APM', function() { expect(started).to.have.length(1); expect(succeeded).to.have.length(1); expect(failed).to.have.length(0); - expect(started[0].commandObj).to.eql({ getnonce: true }); + expect(started[0].command).to.eql({}); expect(succeeded[0].reply).to.eql({}); return client.close(); }); @@ -1129,22 +1129,12 @@ describe('APM', function() { describe('command monitoring unified spec tests', () => { for (const loadedSpec of loadSpecTests('command-monitoring/unified')) { expect(loadedSpec).to.include.all.keys(['description', 'tests']); - // TODO: NODE-3356 unskip redaction tests - const testsToSkip = - loadedSpec.description === 'redacted-commands' - ? loadedSpec.tests - .map(test => test.description) - .filter( - description => - description !== 'hello without speculative authenticate is not redacted' - ) - : []; context(String(loadedSpec.description), function() { for (const test of loadedSpec.tests) { it(String(test.description), { metadata: { sessions: { skipLeakTests: true } }, test() { - return runUnifiedTest(this, loadedSpec, test, testsToSkip); + return runUnifiedTest(this, loadedSpec, test); } }); }