Repository navigation
fix: redact connection-string secrets from ConnectionEntry.lastError - #1501
Merged
gagik merged 2 commits intoSep 10, 2026
Merged
Conversation
lastError is set from the raw driver/arg-parser error message on a failed connect. That message can embed the connection string verbatim (e.g. a malformed URI, missing host after the credentials), and lastError is read back unredacted by summarizeConnection() and returned by list-connections with no arguments and no confirmation. The existing redaction in MongoDBToolBase.handleError() only covers the response for the call that is currently failing, not this persisted field. Redact at the point of capture instead, using the root keychain the same way the rest of the codebase already does. Mainly matters for the preconfigured connection: it's dialed lazily, kept outside per-session scoping, and never removed from the store on a failed dial, so a malformed MDB_MCP_CONNECTION_STRING leaves this value sitting there until the config is fixed.
gagik
reviewed
Sep 10, 2026
Comment on lines
+137
to
+139
| // No mock needed here: a connection string with credentials but no host | ||
| // fails inside @mongosh/arg-parser before the driver is ever called, and | ||
| // the real arg-parser embeds the string verbatim in the thrown error. |
Collaborator
There was a problem hiding this comment.
this comment can be removed
Contributor
Author
There was a problem hiding this comment.
Removed, thanks.
Collaborator
|
It's not a critical leakage but it is always good to harden these things so changes seem good to me 🙂 Please confirm that you have signed the MongoDB CLA. One small comment beyond that. |
SyedAnas01
changed the title
Contributor
Author
|
Hi @gagik, confirming I've signed the MongoDB CLA. Let me know if anything else is needed. |
gagik
added
no-title-validation
gagik
enabled auto-merge (squash)
September 10, 2026 14:04
gagik
approved these changes
Sep 10, 2026
gagik
disabled auto-merge
September 10, 2026 14:05
Collaborator
|
@SyedAnas01 LGTM, thank you for your contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ConnectionEntry.lastErroris set from the raw driver/arg-parser error message when a connect attempt fails. That message can embed the connection string verbatim (for example,mongodb+srv://user:pass@with no host, which throwsProtocol and host list are required in "..."with the credentials still in it).lastErrorgets read back unredacted bysummarizeConnection()and returned bylist-connections, which takes no arguments and needs no confirmation.The redaction in
MongoDBToolBase.handleError()only covers the response for the call that's currently failing — it never touchesConnectionEntry.lastError, which is written one layer earlier inConnectionEntry.connect().This mostly matters for the preconfigured connection: it dials lazily, sits outside per-session scoping, and isn't removed from the store when a dial fails, so if
MDB_MCP_CONNECTION_STRINGis malformed this way, the raw string stays inlastErroruntil the config is fixed — visible to any client callinglist-connections, including other sessions in a shared HTTP deployment.Fix: redact at the point of capture in
ConnectionEntry.connect(), usingKeychain.root.redact()— same pattern already used elsewhere in this codebase for driver errors that might carry secrets.Test: added a regression test that fails a connect with a real (unmocked)
@mongosh/arg-parsercall against a truncatedmongodb+srv://string and assertslastErrordoesn't contain the credential. Confirmed it fails without the fix, passes with it.Related to GHSA-grqv-8942-f78m.