feat: add dictionary-based redaction for secrets in logs (#508)

Author: kmruiz

package-lock.json

@@ -100,15 +100,15 @@
     "@mongodb-js/devtools-connect": "^3.9.3",
     "@mongodb-js/devtools-proxy-support": "^0.5.2",
     "@mongosh/arg-parser": "^3.14.0",
-    "@mongosh/service-provider-node-driver": "^3.12.0",
+    "@mongosh/service-provider-node-driver": "~3.12.0",
     "@vitest/eslint-plugin": "^1.3.4",
     "bson": "^6.10.4",
     "express": "^5.1.0",
     "lru-cache": "^11.1.0",
     "mongodb": "^6.19.0",
     "mongodb-connection-string-url": "^3.0.2",
     "mongodb-log-writer": "^2.4.1",
-    "mongodb-redact": "^1.1.8",
+    "mongodb-redact": "^1.2.0",
     "mongodb-schema": "^12.6.2",
     "node-fetch": "^3.3.2",
     "node-machine-id": "1.1.12",

package.json

@@ -3,6 +3,8 @@ import os from "os";
 import argv from "yargs-parser";
 import type { CliOptions, ConnectionInfo } from "@mongosh/arg-parser";
 import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser";
+import { Keychain } from "./keychain.js";
+import type { Secret } from "./keychain.js";
 
 // From: https://github.com/mongodb-js/mongosh/blob/main/packages/cli-repl/src/arg-parser.ts
 const OPTIONS = {
@@ -316,6 +318,29 @@ function commaSeparatedToArray<T extends string[]>(str: string | string[] | unde
     return str as T;
 }
 
+export function registerKnownSecretsInRootKeychain(userConfig: Partial<UserConfig>): void {
+    const keychain = Keychain.root;
+
+    const maybeRegister = (value: string | undefined, kind: Secret["kind"]): void => {
+        if (value) {
+            keychain.register(value, kind);
+        }
+    };
+
+    maybeRegister(userConfig.apiClientId, "user");
+    maybeRegister(userConfig.apiClientSecret, "password");
+    maybeRegister(userConfig.awsAccessKeyId, "password");
+    maybeRegister(userConfig.awsIamSessionToken, "password");
+    maybeRegister(userConfig.awsSecretAccessKey, "password");
+    maybeRegister(userConfig.awsSessionToken, "password");
+    maybeRegister(userConfig.password, "password");
+    maybeRegister(userConfig.tlsCAFile, "url");
+    maybeRegister(userConfig.tlsCRLFile, "url");
+    maybeRegister(userConfig.tlsCertificateKeyFile, "url");
+    maybeRegister(userConfig.tlsCertificateKeyFilePassword, "password");
+    maybeRegister(userConfig.username, "user");
+}
+
 export function setupUserConfig({
     cli,
     env,
@@ -369,6 +394,7 @@ export function setupUserConfig({
         }
     }
 
+    registerKnownSecretsInRootKeychain(userConfig);
     return userConfig;
 }
 

src/common/config.ts

@@ -199,19 +199,15 @@ export class MCPConnectionManager extends ConnectionManager {
         }
 
         try {
-            const connectionType = MCPConnectionManager.inferConnectionTypeFromSettings(
-                this.userConfig,
-                connectionInfo
-            );
-            if (connectionType.startsWith("oidc")) {
+            if (connectionStringAuthType.startsWith("oidc")) {
                 void this.pingAndForget(serviceProvider);
 
                 return this.changeState("connection-request", {
                     tag: "connecting",
                     connectedAtlasCluster: settings.atlas,
                     serviceProvider,
-                    connectionStringAuthType: connectionType,
-                    oidcConnectionType: connectionType as OIDCConnectionAuthType,
+                    connectionStringAuthType,
+                    oidcConnectionType: connectionStringAuthType as OIDCConnectionAuthType,
                 });
             }
 
@@ -221,7 +217,7 @@ export class MCPConnectionManager extends ConnectionManager {
                 tag: "connected",
                 connectedAtlasCluster: settings.atlas,
                 serviceProvider,
-                connectionStringAuthType: connectionType,
+                connectionStringAuthType,
             });
         } catch (error: unknown) {
             const errorReason = error instanceof Error ? error.message : `${error as string}`;

src/common/connectionManager.ts

@@ -0,0 +1,40 @@
+import type { Secret } from "mongodb-redact";
+export type { Secret } from "mongodb-redact";
+
+/**
+ * This class holds the secrets of a single server. Ideally, we might want to have a keychain
+ * per session, but right now the loggers are set up by server and are not aware of the concept
+ * of session and this would require a bigger refactor.
+ *
+ * Whenever we identify or create a secret (for example, Atlas login, CLI arguments...) we
+ * should register them in the root Keychain (`Keychain.root.register`) or preferably
+ * on the session keychain if available `this.session.keychain`.
+ **/
+export class Keychain {
+    private secrets: Secret[];
+    private static rootKeychain: Keychain = new Keychain();
+
+    constructor() {
+        this.secrets = [];
+    }
+
+    static get root(): Keychain {
+        return Keychain.rootKeychain;
+    }
+
+    register(value: Secret["value"], kind: Secret["kind"]): void {
+        this.secrets.push({ value, kind });
+    }
+
+    clearAllSecrets(): void {
+        this.secrets = [];
+    }
+
+    get allSecrets(): Secret[] {
+        return [...this.secrets];
+    }
+}
+
+export function registerGlobalSecretToRedact(value: Secret["value"], kind: Secret["kind"]): void {
+    Keychain.root.register(value, kind);
+}

src/common/keychain.ts

@@ -1,10 +1,11 @@
 import fs from "fs/promises";
 import type { MongoLogId, MongoLogWriter } from "mongodb-log-writer";
 import { mongoLogId, MongoLogManager } from "mongodb-log-writer";
-import redact from "mongodb-redact";
+import { redact } from "mongodb-redact";
 import type { LoggingMessageNotification } from "@modelcontextprotocol/sdk/types.js";
 import { EventEmitter } from "events";
 import type { Server } from "../lib.js";
+import type { Keychain } from "./keychain.js";
 
 export type LogLevel = LoggingMessageNotification["params"]["level"];
 
@@ -84,6 +85,10 @@ type DefaultEventMap = [never];
 export abstract class LoggerBase<T extends EventMap<T> = DefaultEventMap> extends EventEmitter<T> {
     private readonly defaultUnredactedLogger: LoggerType = "mcp";
 
+    constructor(private readonly keychain: Keychain | undefined) {
+        super();
+    }
+
     public log(level: LogLevel, payload: LogPayload): void {
         // If no explicit value is supplied for unredacted loggers, default to "mcp"
         const noRedaction = payload.noRedaction !== undefined ? payload.noRedaction : this.defaultUnredactedLogger;
@@ -122,7 +127,7 @@ export abstract class LoggerBase<T extends EventMap<T> = DefaultEventMap> extend
             return message;
         }
 
-        return redact(message);
+        return redact(message, this.keychain?.allSecrets ?? []);
     }
 
     public info(payload: LogPayload): void {
@@ -180,6 +185,10 @@ export abstract class LoggerBase<T extends EventMap<T> = DefaultEventMap> extend
 export class ConsoleLogger extends LoggerBase {
     protected readonly type: LoggerType = "console";
 
+    public constructor(keychain: Keychain) {
+        super(keychain);
+    }
+
     protected logCore(level: LogLevel, payload: LogPayload): void {
         const { id, context, message } = payload;
         console.error(
@@ -201,8 +210,8 @@ export class DiskLogger extends LoggerBase<{ initialized: [] }> {
     private bufferedMessages: { level: LogLevel; payload: LogPayload }[] = [];
     private logWriter?: MongoLogWriter;
 
-    public constructor(logPath: string, onError: (error: Error) => void) {
-        super();
+    public constructor(logPath: string, onError: (error: Error) => void, keychain: Keychain) {
+        super(keychain);
 
         void this.initialize(logPath, onError);
     }
@@ -262,8 +271,11 @@ export class McpLogger extends LoggerBase {
         "emergency",
     ] as const;
 
-    public constructor(private readonly server: Server) {
-        super();
+    public constructor(
+        private readonly server: Server,
+        keychain: Keychain
+    ) {
+        super(keychain);
     }
 
     protected readonly type: LoggerType = "mcp";
@@ -295,7 +307,9 @@ export class CompositeLogger extends LoggerBase {
     private readonly attributes: Record<string, string> = {};
 
     constructor(...loggers: LoggerBase[]) {
-        super();
+        // composite logger does not redact, only the actual delegates do the work
+        // so we don't need the Keychain here
+        super(undefined);
 
         this.loggers = loggers;
     }
@@ -327,6 +341,10 @@ export class CompositeLogger extends LoggerBase {
 export class NullLogger extends LoggerBase {
     protected type?: LoggerType;
 
+    constructor() {
+        super(undefined);
+    }
+
     protected logCore(): void {
         // No-op logger, does not log anything
     }

src/common/logger.ts

@@ -15,6 +15,7 @@ import type {
 import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver";
 import { ErrorCodes, MongoDBError } from "./errors.js";
 import type { ExportsManager } from "./exportsManager.js";
+import type { Keychain } from "./keychain.js";
 
 export interface SessionOptions {
     apiBaseUrl: string;
@@ -23,6 +24,7 @@ export interface SessionOptions {
     logger: CompositeLogger;
     exportsManager: ExportsManager;
     connectionManager: ConnectionManager;
+    keychain: Keychain;
 }
 
 export type SessionEvents = {
@@ -37,6 +39,8 @@ export class Session extends EventEmitter<SessionEvents> {
     readonly exportsManager: ExportsManager;
     readonly connectionManager: ConnectionManager;
     readonly apiClient: ApiClient;
+    readonly keychain: Keychain;
+
     mcpClient?: {
         name?: string;
         version?: string;
@@ -52,9 +56,11 @@ export class Session extends EventEmitter<SessionEvents> {
         logger,
         connectionManager,
         exportsManager,
+        keychain,
     }: SessionOptions) {
         super();
 
+        this.keychain = keychain;
         this.logger = logger;
         const credentials: ApiClientCredentials | undefined =
             apiClientId && apiClientSecret

src/common/session.ts

@@ -42,6 +42,7 @@ import { packageInfo } from "./common/packageInfo.js";
 import { StdioRunner } from "./transports/stdio.js";
 import { StreamableHttpRunner } from "./transports/streamableHttp.js";
 import { systemCA } from "@mongodb-js/devtools-proxy-support";
+import { Keychain } from "./common/keychain.js";
 
 async function main(): Promise<void> {
     systemCA().catch(() => undefined); // load system CA asynchronously as in mongosh
@@ -121,7 +122,7 @@ main().catch((error: unknown) => {
     // At this point, we may be in a very broken state, so we can't rely on the logger
     // being functional. Instead, create a brand new ConsoleLogger and log the error
     // to the console.
-    const logger = new ConsoleLogger();
+    const logger = new ConsoleLogger(Keychain.root);
     logger.emergency({
         id: LogId.serverStartFailure,
         context: "server",

src/index.ts

@@ -19,3 +19,5 @@ export type {
 } from "./common/connectionErrorHandler.js";
 export { ErrorCodes } from "./common/errors.js";
 export { Telemetry } from "./telemetry/telemetry.js";
+export { Keychain, registerGlobalSecretToRedact } from "./common/keychain.js";
+export type { Secret } from "./common/keychain.js";

src/lib.ts

@@ -115,6 +115,9 @@ export class ConnectClusterTool extends AtlasToolBase {
         cn.password = password;
         cn.searchParams.set("authSource", "admin");
 
+        this.session.keychain.register(username, "user");
+        this.session.keychain.register(password, "password");
+
         return { connectionString: cn.toString(), atlas: connectedAtlasCluster };
     }