feat(redact): add a dictionary of secrets to redact MCP-29 #569

kmruiz · 2025-09-02T17:50:10Z

Description

With this feature we support redacting known secrets from a dictionary. This is valuable for environments like the MCP Server where we create new users (user/password) or when we can infer secrets from CLI arguments (--user, --pasword).

Open Questions

Checklist

I have signed the Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)
New tests and/or benchmarks are included
Documentation is changed or added

With this feature we support redacting known secrets from a dictionary. This is value for environments like the MCP Server where we create new users (user/password) or when we can infer secrets from CLI arguments (--user, --pasword).

Copilot

Pull Request Overview

This PR adds support for redacting known secrets from a dictionary in the mongodb-redact package. The feature allows environments like MCP Server to redact specific user credentials, passwords, and other sensitive data that can be inferred from CLI arguments or user creation flows.

Key changes:

Add a new secrets.ts module with redactSecrets function and secret type definitions
Extract utility functions to utils.ts for code reusability
Update the main redact function to accept optional secrets dictionary parameter

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/mongodb-redact/tsconfig.json	Adds esModuleInterop compiler option for better module compatibility
packages/mongodb-redact/src/utils.ts	Extracts isPlainObject utility function for code reuse
packages/mongodb-redact/src/secrets.ts	Implements core secret redaction functionality with type definitions
packages/mongodb-redact/src/secrets.spec.ts	Comprehensive test suite for secret redaction functionality
packages/mongodb-redact/src/index.ts	Updates main redact function to support optional secrets parameter
packages/mongodb-redact/src/index.spec.ts	Adds integration test for secret redaction feature
packages/mongodb-redact/package.json	Adds regexp.escape dependency and type definitions

Comments suppressed due to low confidence (1)

packages/mongodb-redact/src/index.ts:1

The recursive call to redact(value) doesn't pass the secrets parameter, so secret redaction won't work for nested objects and arrays. It should be redact(value, secrets).

import { regexes } from './regexes';

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-02T17:51:08Z

packages/mongodb-redact/src/utils.ts

+  if (proto === null) return true;
+  if (!Object.prototype.hasOwnProperty.call(proto, 'constructor')) return false;
+  const ctor = proto.constructor;
+  if (typeof ctor !== 'function') return ctor;


This line returns ctor when it should return a boolean. The function signature indicates it should return val is object, but here it's returning the constructor itself when it's not a function.

Suggested change

if (typeof ctor !== 'function') return ctor;

if (typeof ctor !== 'function') return false;

^ Copilot seems right about this one?

If the constructor is not a function, it doesn't have a constructor, so returning a "truthy" value is correct here because it's a plain object. Right?

This code was as is, so after taking a look it seemed to be right, but maybe I misunderstood it.

Yeah, I wrote that previous code, so this is just Copilot pointing out that I'm wrong, not you 😅

If the constructor is not a function, it doesn't have a constructor

Not really, right? Does Object.create({ constructor: 42 }) "have" a constructor? Certainly not in the sense of a constructor function, but we shouldn't count this as a "plain object" in the sense of "objects whose prototype is null or Object.prototype"

so returning a "truthy" value is correct here because it's a plain object. Right?

Right now, this would say that Object.create({ constructor: true }) is a plain object and Object.create({ constructor: false }) isn't, which is wrong – these two are definitely the same degree of "not-plain-object-y" 🙂

Then, the condition would have to be:

if (ctor) return false;

right? We don't mind about the type of the constructor, we only care if it exists or not. Isn't it?

Edit:

Btw, both your cases with create would return true, because neither true and false are functions, so the constructor is considered just a plain property with a primitive.

We don't mind about the type of the constructor, we only care if it exists or not. Isn't it?

Once we've established that val has a prototype proto, we want to return true if proto.constructor is the JS Object builtin, and false otherwise – that's the goal here

packages/mongodb-redact/src/index.ts

packages/mongodb-redact/src/secrets.ts

addaleax · 2025-09-02T18:49:18Z

packages/mongodb-redact/src/secrets.ts

+    const regex = new RegExp(`\\b${escape(value)}\\b`);
+    result = result.replace(regex, `<${kind}>`) as T;


Suggested change

const regex = new RegExp(`\\b${escape(value)}\\b`);

result = result.replace(regex, `<${kind}>`) as T;

const regex = new RegExp(`\\b${escape(value)}\\b`, 'g');

result = result.replace(regex, `<${kind}>`) as T;

Otherwise this only replaces the first instance of the secret (this should probably also be tested 🙂)

addaleax · 2025-09-02T18:51:05Z

packages/mongodb-redact/src/utils.ts

+  if (proto === null) return true;
+  if (!Object.prototype.hasOwnProperty.call(proto, 'constructor')) return false;
+  const ctor = proto.constructor;
+  if (typeof ctor !== 'function') return ctor;


^ Copilot seems right about this one?

addaleax · 2025-09-02T18:52:43Z

packages/mongodb-redact/src/secrets.ts

+    return Object.fromEntries(
+      Object.entries(message).map(([key, value]) => [
+        key,
+        redactSecrets(value, secrets),
+      ]),
+    ) as T;


Suggested change

return Object.fromEntries(

Object.entries(message).map(([key, value]) => [

key,

redactSecrets(value, secrets),

]),

) as T;

return Object.setPrototypeOf(Object.fromEntries(

Object.entries(message).map(([key, value]) => [

key,

redactSecrets(value, secrets),

]),

), Object.getPrototypeOf(message)) as T;

since isPlainObject() also (correctly imo) returns true for objects with a null prototype?

packages/mongodb-redact/src/index.ts

addaleax · 2025-09-02T18:55:35Z

packages/mongodb-redact/src/index.ts

+  secrets: Secret[] | undefined = undefined,
+): T {
+  if (secrets) {
+    message = redactSecrets(message, secrets);
  }


Not a blocker for sure, but I'd suggest merging the object walking here with the object walking in redactSecrets(), they do very similar transformations in a very similar fashion

We don't want to return a truthy value if the ctor is a non function value (like an object). Plain objects either have a plain object as a prototype or a null prototype

feat(redact): add a dictionary of secrets to redact

cb5a83b

With this feature we support redacting known secrets from a dictionary. This is value for environments like the MCP Server where we create new users (user/password) or when we can infer secrets from CLI arguments (--user, --pasword).

kmruiz requested review from addaleax, nirinchev, himanshusinghs and Copilot September 2, 2025 17:50

kmruiz self-assigned this Sep 2, 2025

Copilot AI reviewed Sep 2, 2025

View reviewed changes

chore: add support for arbitrarily deep object redacting

d163d6b

addaleax reviewed Sep 2, 2025

View reviewed changes

kmruiz added 3 commits September 3, 2025 16:32

chore: refactor to reuse the iteration, instead of iterating twice

02f6dd2

chore: fix linter warnings

0a077d2

chore: fix an issue when the constructor is not a function

971bb70

We don't want to return a truthy value if the ctor is a non function value (like an object). Plain objects either have a plain object as a prototype or a null prototype

addaleax approved these changes Sep 3, 2025

View reviewed changes

kmruiz merged commit 3b1d6e3 into main Sep 3, 2025
31 checks passed

kmruiz deleted the feat/mcp-29 branch September 3, 2025 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(redact): add a dictionary of secrets to redact MCP-29 #569

feat(redact): add a dictionary of secrets to redact MCP-29 #569

Uh oh!

kmruiz commented Sep 2, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

addaleax Sep 2, 2025

Uh oh!

kmruiz Sep 2, 2025

Uh oh!

addaleax Sep 3, 2025

Uh oh!

kmruiz Sep 3, 2025 •

edited

Loading

Uh oh!

addaleax Sep 3, 2025

Uh oh!

Uh oh!

Uh oh!

addaleax Sep 2, 2025

Uh oh!

addaleax Sep 2, 2025

Uh oh!

addaleax Sep 2, 2025

Uh oh!

Uh oh!

addaleax Sep 2, 2025

Uh oh!

Uh oh!

Uh oh!

	if (typeof ctor !== 'function') return ctor;
	if (typeof ctor !== 'function') return false;

		const regex = new RegExp(`\\b${escape(value)}\\b`);
		result = result.replace(regex, `<${kind}>`) as T;

feat(redact): add a dictionary of secrets to redact MCP-29 #569

feat(redact): add a dictionary of secrets to redact MCP-29 #569

Uh oh!

Conversation

kmruiz commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Open Questions

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmruiz Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kmruiz commented Sep 2, 2025 •

edited

Loading

kmruiz Sep 3, 2025 •

edited

Loading