Make keychains session-specific and register config override secrets

Keychain now supports a parent-child hierarchy. Each session creates a child keychain via Keychain.root.createChild(), ensuring session-specific secrets are scoped while remaining visible to shared loggers via upward propagation. Config override secrets are now registered on the session keychain in createServer(), fixing the gap where overrides bypassed keychain registration entirely.

Author: gagik

src/common/config/parseUserConfig.ts

@@ -86,7 +86,7 @@ export function parseUserConfig({
     // TODO: Separate correctly parsed user config from all other valid
     // arguments relevant to mongosh's args-parser.
     const userConfig: UserConfig = { ...parsed, ...configParseResult.data };
-    registerKnownSecretsInRootKeychain(userConfig);
+    registerConfigSecrets(userConfig, Keychain.root);
     return {
         parsed: userConfig,
         warnings,
@@ -153,9 +153,12 @@ function parseUserConfigSources<T extends typeof UserConfigSchema>({
     };
 }
 
-function registerKnownSecretsInRootKeychain(userConfig: Partial<UserConfig>): void {
-    const keychain = Keychain.root;
-
+/**
+ * Registers known secret fields from the given config on the provided keychain.
+ * Called once on `Keychain.root` during initial config parsing, and again on
+ * each session's child keychain to cover config-override secrets.
+ */
+export function registerConfigSecrets(userConfig: Partial<UserConfig>, keychain: Keychain): void {
     const maybeRegister = (value: string | undefined, kind: Secret["kind"]): void => {
         if (value) {
             keychain.register(value, kind);

src/common/keychain.ts

@@ -2,36 +2,55 @@ 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.
+ * Holds secrets for redaction in logging and telemetry pipelines.
  *
- * 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`.
+ * Keychains form a parent-child hierarchy:
+ * - `Keychain.root` stores base config secrets (registered during config parsing).
+ * - Each session creates a child via `Keychain.root.createChild()` to hold
+ *   session-specific secrets (e.g. generated credentials, config override values).
+ *
+ * `allSecrets` aggregates secrets from the entire parent chain so that redaction
+ * covers both global and session-scoped values.
+ *
+ * When a secret is registered on a child keychain it is also propagated to the
+ * parent so that shared loggers (ConsoleLogger, DiskLogger) — which reference
+ * the root keychain — can redact session-specific secrets as well.
  **/
 export class Keychain {
     private secrets: Secret[];
     private static rootKeychain: Keychain = new Keychain();
+    private readonly parent?: Keychain;
 
-    constructor() {
+    constructor(parent?: Keychain) {
         this.secrets = [];
+        this.parent = parent;
     }
 
     static get root(): Keychain {
         return Keychain.rootKeychain;
     }
 
+    /**
+     * Creates a child keychain whose `allSecrets` includes this keychain's
+     * secrets. Secrets registered on the child also propagate upward so
+     * shared loggers that reference an ancestor still redact them.
+     */
+    createChild(): Keychain {
+        return new Keychain(this);
+    }
+
     register(value: Secret["value"], kind: Secret["kind"]): void {
         this.secrets.push({ value, kind });
+        this.parent?.register(value, kind);
     }
 
     clearAllSecrets(): void {
         this.secrets = [];
     }
 
     get allSecrets(): Secret[] {
-        return [...this.secrets];
+        const parentSecrets = this.parent?.allSecrets ?? [];
+        return [...parentSecrets, ...this.secrets];
     }
 }
 

src/lib.ts

@@ -1,7 +1,12 @@
 export { Server, type ServerOptions, type AnyToolClass } from "./server.js";
 export { Session, type SessionOptions } from "./common/session.js";
 export { type UserConfig, UserConfigSchema } from "./common/config/userConfig.js";
-export { parseUserConfig, defaultParserOptions, type ParserOptions } from "./common/config/parseUserConfig.js";
+export {
+    parseUserConfig,
+    defaultParserOptions,
+    registerConfigSecrets,
+    type ParserOptions,
+} from "./common/config/parseUserConfig.js";
 
 import { parseUserConfig } from "./common/config/parseUserConfig.js";
 import type { UserConfig } from "./common/config/userConfig.js";

src/transports/base.ts

@@ -9,6 +9,7 @@ import { CompositeLogger, ConsoleLogger, DiskLogger, McpLogger } from "../common
 import { ExportsManager } from "../common/exportsManager.js";
 import { DeviceId } from "../helpers/deviceId.js";
 import { Keychain } from "../common/keychain.js";
+import { registerConfigSecrets } from "../common/config/parseUserConfig.js";
 import { defaultCreateConnectionManager, type ConnectionManagerFactoryFn } from "../common/connectionManager.js";
 import {
     type ConnectionErrorHandler,
@@ -318,6 +319,9 @@ export abstract class TransportRunnerBase<
                   )
                 : undefined;
 
+        const sessionKeychain = Keychain.root.createChild();
+        registerConfigSecrets(userConfig, sessionKeychain);
+
         const session = new Session({
             userConfig,
             atlasLocalClient:
@@ -326,7 +330,7 @@ export abstract class TransportRunnerBase<
             connectionErrorHandler: sessionOptions?.connectionErrorHandler ?? this.connectionErrorHandler,
             exportsManager,
             connectionManager,
-            keychain: Keychain.root,
+            keychain: sessionKeychain,
             apiClient: sessionOptions?.apiClient ?? apiClient,
         });
 
@@ -356,7 +360,7 @@ export abstract class TransportRunnerBase<
         // We need to create the MCP logger after the server is constructed
         // because it needs the server instance
         if (userConfig.loggers.includes("mcp")) {
-            logger.addLogger(new McpLogger(result, Keychain.root));
+            logger.addLogger(new McpLogger(result, sessionKeychain));
         }
 
         return result;

tests/unit/common/keychain.test.ts

@@ -33,4 +33,49 @@ describe("Keychain", () => {
             expect(keychain.allSecrets).toEqual([{ value: "123456", kind: "password" }]);
         });
     });
+
+    describe("parent-child keychains", () => {
+        it("child allSecrets includes parent secrets", () => {
+            keychain.register("root-secret", "password");
+            const child = keychain.createChild();
+            child.register("child-secret", "password");
+
+            expect(child.allSecrets).toEqual([
+                { value: "root-secret", kind: "password" },
+                // root-secret again because child.register propagated it to parent
+                { value: "child-secret", kind: "password" },
+                { value: "child-secret", kind: "password" },
+            ]);
+        });
+
+        it("secrets registered on child propagate to parent", () => {
+            const child = keychain.createChild();
+            child.register("child-secret", "password");
+
+            expect(keychain.allSecrets).toContainEqual({ value: "child-secret", kind: "password" });
+        });
+
+        it("clearAllSecrets on child does not affect parent", () => {
+            keychain.register("root-secret", "password");
+            const child = keychain.createChild();
+            child.register("child-secret", "password");
+
+            child.clearAllSecrets();
+            expect(child.allSecrets).toContainEqual({ value: "root-secret", kind: "password" });
+            expect(child.allSecrets).toContainEqual({ value: "child-secret", kind: "password" });
+            expect(keychain.allSecrets).toContainEqual({ value: "root-secret", kind: "password" });
+        });
+
+        it("supports multi-level chains", () => {
+            keychain.register("root-secret", "password");
+            const child = keychain.createChild();
+            const grandchild = child.createChild();
+            grandchild.register("deep-secret", "password");
+
+            const deepSecretEntries = grandchild.allSecrets.filter((s) => s.value === "deep-secret");
+            expect(deepSecretEntries.length).toBeGreaterThanOrEqual(1);
+
+            expect(keychain.allSecrets).toContainEqual({ value: "deep-secret", kind: "password" });
+        });
+    });
 });