Skip to content

Commit 332fa41

Browse files
authored
fix(logging): handle credential redaction in some cases with special characters MONGOSH-2991 (#2581)
1 parent e9f6465 commit 332fa41

21 files changed

+743
-296
lines changed

package-lock.json

Lines changed: 631 additions & 232 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/cli-repl/package.json

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,30 +87,31 @@
8787
"js-yaml": "^4.1.0",
8888
"mongodb-connection-string-url": "^3.0.2",
8989
"mongodb-log-writer": "^2.4.3",
90+
"mongodb-redact": "^1.3.0",
9091
"numeral": "^2.0.6",
9192
"pretty-repl": "^4.0.1",
9293
"semver": "^7.5.4",
9394
"strip-ansi": "^6.0.0",
9495
"text-table": "^0.2.0",
95-
"yargs-parser": "^20.2.4",
96-
"glibc-version": "^1.0.0"
96+
"glibc-version": "^1.0.0",
97+
"yargs-parser": "^20.2.4"
9798
},
9899
"devDependencies": {
99-
"mongodb": "^6.19.0",
100100
"@mongodb-js/eslint-config-mongosh": "^1.0.0",
101101
"@mongodb-js/prettier-config-devtools": "^1.0.1",
102102
"@mongodb-js/sbom-tools": "^0.8.1",
103103
"@mongodb-js/tsconfig-mongosh": "^1.0.0",
104104
"@types/ansi-escape-sequences": "^4.0.0",
105+
"@types/chai-as-promised": "^7.1.3",
105106
"@types/js-yaml": "^4.0.5",
106107
"@types/node": "^22.15.30",
107108
"@types/numeral": "^2.0.2",
108109
"@types/text-table": "^0.2.1",
109110
"@types/yargs-parser": "^15.0.0",
110-
"@types/chai-as-promised": "^7.1.3",
111111
"chai-as-promised": "^7.1.1",
112112
"depcheck": "^1.4.7",
113113
"eslint": "^7.25.0",
114+
"mongodb": "^6.19.0",
114115
"mongodb-crypt-library-dummy": "^1.0.2",
115116
"prettier": "^2.8.8",
116117
"webpack-merge": "^5.8.0"

packages/cli-repl/src/cli-repl.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@ import {
33
MongoshRuntimeError,
44
MongoshWarning,
55
} from '@mongosh/errors';
6-
import { redactURICredentials } from '@mongosh/history';
6+
import { redactConnectionString, redact } from 'mongodb-redact';
77
import i18n from '@mongosh/i18n';
88
import type { AutoEncryptionOptions } from '@mongosh/service-provider-core';
99
import { EJSON, ObjectId } from 'bson';
1010
import { NodeDriverServiceProvider } from '@mongosh/service-provider-node-driver';
1111
import type { CliOptions, DevtoolsConnectOptions } from '@mongosh/arg-parser';
1212
import { SnippetManager } from '@mongosh/snippet-manager';
1313
import { Editor } from '@mongosh/editor';
14-
import { redactSensitiveData } from '@mongosh/history';
1514
import type { Analytics as SegmentAnalytics } from '@segment/analytics-node';
1615
import askpassword from 'askpassword';
1716
import { EventEmitter, once } from 'events';
@@ -306,7 +305,7 @@ export class CliRepl implements MongoshIOProvider {
306305
'Starting log',
307306
{
308307
execPath: process.execPath,
309-
envInfo: redactSensitiveData(this.getLoggedEnvironmentVariables()),
308+
envInfo: redact(this.getLoggedEnvironmentVariables()),
310309
...(await buildInfo()),
311310
}
312311
);
@@ -905,7 +904,7 @@ export class CliRepl implements MongoshIOProvider {
905904
this.output.write(
906905
i18n.__(CONNECTING) +
907906
'\t\t' +
908-
this.clr(redactURICredentials(driverUri), 'mongosh:uri') +
907+
this.clr(redactConnectionString(driverUri), 'mongosh:uri') +
909908
'\n'
910909
);
911910
}

packages/cli-repl/src/run.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { getStoragePaths, getGlobalConfigPaths } from './config-directory';
2323
import { getCryptLibraryPaths } from './crypt-library-paths';
2424
import { getTlsCertificateSelector } from './tls-certificate-selector';
2525
import { applyPacProxyS390XPatch } from './pac-proxy-s390x-patch';
26-
import { redactURICredentials } from '@mongosh/history';
26+
import { redactConnectionString } from 'mongodb-redact';
2727
import { generateConnectionInfoFromCliArgs } from '@mongosh/arg-parser';
2828
import askcharacter from 'askcharacter';
2929
import { PassThrough } from 'stream';
@@ -218,7 +218,7 @@ async function main() {
218218
driverInfo: { name: 'mongosh', version },
219219
};
220220

221-
const title = `mongosh ${redactURICredentials(
221+
const title = `mongosh ${redactConnectionString(
222222
connectionInfo.connectionString
223223
)}`;
224224
process.title = title;

packages/cli-repl/src/smoke-tests.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import assert from 'assert';
22
import { promises as fs } from 'fs';
33
import { once } from 'events';
4-
import { redactURICredentials } from '@mongosh/history';
4+
import { redactConnectionString } from 'mongodb-redact';
55
import fleSmokeTestScript from './smoke-tests-fle';
66
import { baseBuildInfo, buildInfo } from './build-info';
77
import escapeRegexp from 'escape-string-regexp';
@@ -469,7 +469,7 @@ async function runSmokeTest({
469469
stderr: includeStderr ? stderr : '',
470470
executable,
471471
actualExitCode,
472-
args: args.map((arg) => redactURICredentials(arg)),
472+
args: args.map((arg) => redactConnectionString(arg)),
473473
};
474474
try {
475475
assert.match(includeStderr ? `${stdout}\n${stderr}` : stdout, output);

packages/history/package.json

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,14 @@
3535
"unitTestsOnly": true
3636
},
3737
"dependencies": {
38-
"mongodb-connection-string-url": "^3.0.2",
39-
"mongodb-redact": "^1.1.5"
38+
"mongodb-redact": "^1.3.0"
4039
},
4140
"devDependencies": {
4241
"@mongodb-js/eslint-config-mongosh": "^1.0.0",
4342
"@mongodb-js/prettier-config-devtools": "^1.0.1",
4443
"@mongodb-js/tsconfig-mongosh": "^1.0.0",
4544
"depcheck": "^1.4.7",
4645
"eslint": "^7.25.0",
47-
"mongodb-connection-string-url": "^3.0.2",
4846
"prettier": "^2.8.8"
4947
}
5048
}

packages/history/src/history.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
import redactSensitiveData from 'mongodb-redact';
2-
3-
export const HIDDEN_COMMANDS = String.raw`\b(createUser|auth|updateUser|changeUserPassword|connect|Mongo)\b`;
1+
import { shouldRedactCommand, redact } from 'mongodb-redact';
42

53
/**
64
* Modifies the most recent command in history based on sensitive information.
@@ -11,16 +9,13 @@ export const HIDDEN_COMMANDS = String.raw`\b(createUser|auth|updateUser|changeUs
119
*/
1210
export function changeHistory(
1311
history: string[],
14-
redact: 'redact-sensitive-data' | 'keep-sensitive-data'
12+
redactMode: 'redact-sensitive-data' | 'keep-sensitive-data'
1513
): void {
1614
if (history.length === 0) return;
17-
const hiddenCommands = new RegExp(HIDDEN_COMMANDS, 'g');
1815

19-
if (hiddenCommands.test(history[0])) {
16+
if (shouldRedactCommand(history[0])) {
2017
history.shift();
21-
} else if (redact === 'redact-sensitive-data') {
22-
history[0] = redactSensitiveData(history[0]);
18+
} else if (redactMode === 'redact-sensitive-data') {
19+
history[0] = redact(history[0]);
2320
}
2421
}
25-
26-
export { redactSensitiveData };

packages/history/src/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1 @@
1-
export { changeHistory, redactSensitiveData, HIDDEN_COMMANDS } from './history';
2-
export { redactConnectionString as redactURICredentials } from 'mongodb-connection-string-url';
1+
export { changeHistory } from './history';

packages/logging/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@
2020
"@mongodb-js/device-id": "^0.2.1",
2121
"@mongodb-js/devtools-connect": "^3.9.4",
2222
"@mongosh/errors": "2.4.4",
23-
"@mongosh/history": "2.4.9",
2423
"@mongosh/types": "^3.14.0",
2524
"mongodb-log-writer": "^2.4.3",
26-
"mongodb-redact": "^1.1.5",
25+
"mongodb-redact": "^1.3.0",
2726
"native-machine-id": "^0.1.1"
2827
},
2928
"devDependencies": {

packages/logging/src/logging-and-telemetry.spec.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,6 +1132,54 @@ describe('MongoshLoggingAndTelemetry', function () {
11321132
expect(analyticsOutput).to.be.empty;
11331133
});
11341134

1135+
it('redacts logging of sensitive commands', async function () {
1136+
loggingAndTelemetry.attachLogger(logger);
1137+
await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise;
1138+
1139+
expect(logOutput).to.have.lengthOf(0);
1140+
1141+
// Test that sensitive commands are redacted
1142+
bus.emit('mongosh:evaluate-input', {
1143+
input: 'db.createUser({user: "admin", pwd: "password", roles: []})',
1144+
});
1145+
bus.emit('mongosh:evaluate-input', { input: 'db.auth("user", "pass")' });
1146+
bus.emit('mongosh:evaluate-input', {
1147+
input: 'db.updateUser("user", {pwd: "newpass"})',
1148+
});
1149+
bus.emit('mongosh:evaluate-input', {
1150+
input: 'db.changeUserPassword("user", "newpass")',
1151+
});
1152+
bus.emit('mongosh:evaluate-input', {
1153+
input: 'connect("mongodb://user:pass@localhost/")',
1154+
});
1155+
bus.emit('mongosh:evaluate-input', {
1156+
input: 'new Mongo("mongodb://user:pass@localhost/")',
1157+
});
1158+
1159+
// Test that non-sensitive commands are still logged
1160+
bus.emit('mongosh:evaluate-input', { input: 'db.getUsers()' });
1161+
bus.emit('mongosh:evaluate-input', { input: 'show dbs' });
1162+
1163+
// Should only have logged the non-sensitive commands
1164+
expect(logOutput).to.have.lengthOf(8);
1165+
expect(logOutput[0].msg).to.equal('Evaluating input');
1166+
expect(logOutput[0].attr.input).to.equal('<sensitive command used>');
1167+
expect(logOutput[1].msg).to.equal('Evaluating input');
1168+
expect(logOutput[1].attr.input).to.equal('<sensitive command used>');
1169+
expect(logOutput[2].msg).to.equal('Evaluating input');
1170+
expect(logOutput[2].attr.input).to.equal('<sensitive command used>');
1171+
expect(logOutput[3].msg).to.equal('Evaluating input');
1172+
expect(logOutput[3].attr.input).to.equal('<sensitive command used>');
1173+
expect(logOutput[4].msg).to.equal('Evaluating input');
1174+
expect(logOutput[4].attr.input).to.equal('<sensitive command used>');
1175+
expect(logOutput[5].msg).to.equal('Evaluating input');
1176+
expect(logOutput[5].attr.input).to.equal('<sensitive command used>');
1177+
expect(logOutput[6].msg).to.equal('Evaluating input');
1178+
expect(logOutput[6].attr.input).to.equal('db.getUsers()');
1179+
expect(logOutput[7].msg).to.equal('Evaluating input');
1180+
expect(logOutput[7].attr.input).to.equal('show dbs');
1181+
});
1182+
11351183
it('tracks custom logging events', async function () {
11361184
expect(logOutput).to.have.lengthOf(0);
11371185
expect(analyticsOutput).to.be.empty;

0 commit comments

Comments
 (0)