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

Prevent IMAP passwords from going to the logging system #270

Merged
merged 1 commit into from Aug 22, 2017

Conversation

3 participants
@NicolasLM
Collaborator

NicolasLM commented Aug 18, 2017

Even if the final user did not enable DEBUG logging for its
application, some logs records can end up in tools like Sentry
via breadcrumbs.

This commit makes sure that passwords are never sent to the
logging system by verifying and redacting the log message before
passing it to the logger.

In the future, once we get rid of imaplib, the code replacing it
could avoid logging raw data containing sensitive information in
the first place.

@mlorant

This comment has been minimized.

Show comment
Hide comment
@mlorant

mlorant Aug 18, 2017

Contributor

Awesome, I did something equivalent because of Sentry too. However, could you also support oauth2_login and strip the user and access_token in that case? Thanks!

Contributor

mlorant commented Aug 18, 2017

Awesome, I did something equivalent because of Sentry too. However, could you also support oauth2_login and strip the user and access_token in that case? Thanks!

@NicolasLM

This comment has been minimized.

Show comment
Hide comment
@NicolasLM

NicolasLM Aug 18, 2017

Collaborator

I've added the same thing for the AUTHENTICATE command.

Collaborator

NicolasLM commented Aug 18, 2017

I've added the same thing for the AUTHENTICATE command.

@mjs mjs added this to the 2.0.0 milestone Aug 18, 2017

@mjs

mjs approved these changes Aug 18, 2017

Looks great, thanks. Just one small suggestion.

Show outdated Hide outdated imapclient/test/test_imapclient.py Outdated
@mjs

This comment has been minimized.

Show comment
Hide comment
@mjs

mjs Aug 21, 2017

Owner

I'm happy to merge this but there's a conflict.

Owner

mjs commented Aug 21, 2017

I'm happy to merge this but there's a conflict.

Prevent IMAP passwords from going to the logging system
Even if the final user did not enable DEBUG logging for its
application, some logs records can end up in tools like Sentry
via breadcrumbs.

This commit makes sure that secrets are never sent to the
logging system by verifying and redacting the log message before
passing it to the logger.

In the future, once we get rid of imaplib, the code replacing it
could avoid logging raw data containing sensitive information in
the first place.
@NicolasLM

This comment has been minimized.

Show comment
Hide comment
@NicolasLM

NicolasLM Aug 22, 2017

Collaborator

Rebased

Collaborator

NicolasLM commented Aug 22, 2017

Rebased

@mjs mjs merged commit e431078 into mjs:master Aug 22, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mjs

This comment has been minimized.

Show comment
Hide comment
@mjs

mjs Aug 22, 2017

Owner

Thanks again.

Owner

mjs commented Aug 22, 2017

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment