-
Notifications
You must be signed in to change notification settings - Fork 9
feat(mongodb-react): add shouldRedactCommand, redactUriCredentials MONGOSH-2991 #599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
386b4dd to
7209ba0
Compare
7209ba0 to
8b3ea74
Compare
8b3ea74 to
7e75778
Compare
packages/mongodb-redact/package.json
Outdated
| "dependencies": { | ||
| "regexp.escape": "^2.0.1" | ||
| "regexp.escape": "^2.0.1", | ||
| "mongodb-connection-string-url": "^3.0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by the way this is now at 7.0.0, I'm keeping at same version as mongosh to minimize side effects for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you can do, if you want to, is to specify
| "mongodb-connection-string-url": "^3.0.1" | |
| "mongodb-connection-string-url": "^3.0.1 || ^7.0.0" |
and give control to the user of this package, since it's pretty reasonable to assume that both of these would work just fine
packages/mongodb-redact/package.json
Outdated
| "dependencies": { | ||
| "regexp.escape": "^2.0.1" | ||
| "regexp.escape": "^2.0.1", | ||
| "mongodb-connection-string-url": "^3.0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you can do, if you want to, is to specify
| "mongodb-connection-string-url": "^3.0.1" | |
| "mongodb-connection-string-url": "^3.0.1 || ^7.0.0" |
and give control to the user of this package, since it's pretty reasonable to assume that both of these would work just fine
| @@ -0,0 +1,5 @@ | |||
| import { redactConnectionString } from 'mongodb-connection-string-url'; | |||
|
|
|||
| export function redactUriCredentials(uri: string): string { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this redactConnectionString to keep identifier continuity? Or is there a specific reason that this should be named redactUriCredentials? Being able to grep for something and find all instances of that thing is generally quite helpful
(in this particular case, "URI credentials" might also be a bit misleading because in the context of URIs, credentials refer to username and password specifically, but we also filter out known credentials in the connection string like AWS session tokens or private key file passwords because we're aware of connection string semantics here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was meant to be redactURICredentials solely because mongosh history's function this is copying was named that, so not really a deep reason.
redactConnectionString sounds better to me
This will let us use mongodb-redact as one source of truth for redacting logic in both
@mongosh/historyand@mongosh/logging