AnymailError leaks PII into logs

coupa-anya commented 3 years ago

When Anymail can't connect to the ESP or gets an error response, the log message and stack trace includes the recipients email addresses, as well as their full names if they're included in the email message object. This can be useful for debugging, but the result is that our customers' PII could be leaked into our logs, which are ingested by a 3rd party. The culprit is the describe_send function in AnymailError. For now, we have monkey patched that function in our code so that the PII is obfuscated, but it would be great if there were an official fix!

Thank you!

Anymail version: 8.4
ESP (Mailgun, SendGrid, etc.): Mandrill
Your ANYMAIL settings (change secrets to "redacted"): NA
Versions of Django, requests, python: Django 3.2.4, requests 2.25.1, python 3.8
Exact error message and/or stack trace: Names/emails changed in the example error message below raise AnymailRequestsAPIError( anymail.exceptions.AnymailRequestsAPIError: Sending a message to First Name <first.name@domain.com> from Sender <sender.address@domain.com> Mandrill API response 500 (Internal Server Error): '<html>\r\n<head><title>500 Internal Server Error</title></head>\r\n<body bgcolor="white">\r\n<center><h1>500 Internal Server Error</h1></center>\r\n<hr><center>nginx/1.1.19</center>\r\n</body>\r\n</html>\r\n'
Any other relevant code and settings (e.g., for problems sending, your code that sends the message): NA

medmunds commented 3 years ago

Thanks for pointing this out.

I'm inclined to just remove the Sending a message to Recipient Name <recipient@example.com> from Sender Name <sender@example.com> line from Anymail's error messages altogether. For most common error cases, I don't think it's adding any value.

medmunds commented 2 years ago

[Sorry for the delay.]

I've removed the AnymailError code that indiscriminately added sender and recipient emails to every error message.

Note that AnymailError does still include any error description from your ESP. This is usually critical to understanding what went wrong, so I wouldn't want to remove that. However, it will often contains email addresses or other content from the sent message. (And there's no easy way to determine what part of an ESP error message might be considered PII.)

Also, AnymailError messages will still include email addresses where relevant to the error. (E.g., attempts to use an invalidly-formatted email address will include the invalid address in the error.)

If this is a problem, there are some fully supported (non-monkeypatch) ways to alter logging and log data collection using Django's logging configuration. For example, you might be able to add your own custom PII sanitization. (See Sentry's data collector for an example—it does something similar to avoid collecting API keys and other secrets.) Worst case, you could implement a logging filter on any error coming from the anymail package, and sanitize away the entire error message if necessary (keeping only the exception class and stack trace). That might complicate diagnosing problems, but would guarantee no PII snuck through.

medmunds commented 2 years ago

[Improvement released in Anymail v8.5]

anymail / django-anymail

AnymailError leaks PII into logs #245