-
Notifications
You must be signed in to change notification settings - Fork 27
feat: redact secrets in debug logging #209
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
fad3f75
to
ed37a7c
Compare
@@ -33,40 +30,7 @@ | |||
key = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.key') | |||
|
|||
|
|||
def _local_server(tls_version: int, port: int) -> Callable: |
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 moved this to a separate file, so that it's easier to use it in other tests too.
e0a28a7
to
7b11e75
Compare
Signed-off-by: Norbert Biczo <pyrooka@users.noreply.github.com>
Signed-off-by: Norbert Biczo <pyrooka@users.noreply.github.com>
Signed-off-by: Norbert Biczo <pyrooka@users.noreply.github.com>
Signed-off-by: Norbert Biczo <pyrooka@users.noreply.github.com>
ed37a7c
to
a348672
Compare
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'm testing out your feature branch with the platform-services python SDK and I've run into some problems related to the use of a redacted message. It looks like the log record contains both the fully-resolved/redacted message AND the individual arguments that were originally passed on the call to logger.debug() which are used to format the message. I think once we replace the log record's message with the fully-resolved/redacted version, then we should probably remove the arguments from the log record to avoid this error (presumably)
Update: After experimenting with your feature branch in my local sandbox, I opened a PR with a handful of changes (most of which we discussed on slack) that fix the issues that I ran into. |
Signed-off-by: Phil Adams <phil_adams@us.ibm.com>
Signed-off-by: Norbert Biczo <pyrooka@users.noreply.github.com>
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.
LGTM
Signed-off-by: Norbert Biczo <pyrooka@users.noreply.github.com>
This PR improves the debug logging logic in the core,
so that secrets are not shown in the debug logs anymore.
FYI: this PR targets the
debug-logging
feature branch.