-
Notifications
You must be signed in to change notification settings - Fork 10
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
Redact potentially sensitive config keys from log messages #62
base: main
Are you sure you want to change the base?
Conversation
- It might be overkill to redact a lot of the keys, but its probably better to be safe than sorry
- I found another place where the password is getting redacted so I thought it might be good to use the same function to do it everywhere
- It lives under docker
@@ -11,15 +11,15 @@ jobs: | |||
|
|||
strategy: | |||
matrix: | |||
python-version: ["3.8", "3.9", "3.10"] | |||
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] |
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.
I wasn't sure if we wanted to remove some of the older versions, looks like Python 3.8 is EOL, but the builds run in parallel so it doesn't seem to hurt to have more versions in there.
# "sasl.login.class", | ||
# "sasl.mechanism", | ||
# "sasl.oauthbearer.jwks.endpoint.url", | ||
# "sasl.oauthbearer.token.endpoint.url", |
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.
I don't usually like having a heap of commented out code like this, but it is handy to see at a glance what keys are getting redacted so I left it in for now.
As discussed in #61 I have created a whitelist of config values that are considered non-sensitive, and all others will be redacted with the string "****"
I noticed there was some other code that redacts the SASL password so I switched that to use the new redact function so everything is consistent.
I also have made some small changes to the CI pipeline as it was not building properly when trying to test the fork (and added some newer python versions).
Fixes #61, Fixes #60