From 10f569cd506fa39fa8ff7c507268bb562843a362 Mon Sep 17 00:00:00 2001 From: medmunds Date: Tue, 11 Jan 2022 16:30:07 -0800 Subject: [PATCH] Fix: don't include sender/recipient in AnymailError description Remove `AnymailError.describe_send`, which added sender and recipient email addresses to every AnymailError message (whether or not relevant to the error). Addresses #245 --- CHANGELOG.rst | 8 ++++++++ anymail/exceptions.py | 16 ---------------- tests/test_general_backend.py | 20 +++++++++++++++++++- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 16b9967..e275627 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -37,6 +37,13 @@ Fixes an EmailMessage's `body`, and generally improve alternative part handling for consistency with Django's SMTP EmailBackend. (Thanks to `@cjsoftuk`_ for reporting the issue.) +* Remove "sending a message from *sender* to *recipient*" from `AnymailError` + text, as this can unintentionally leak personal information into logs. + [Note that `AnymailError` *does* still include any error description + from your ESP, and this often contains email addresses and other content + from the sent message. If this is a concern, you can adjust Django's logging + config to limit collection from Anymail or implement custom PII filtering.] + (Thanks to `@coupa-anya`_ for reporting the issue.) v8.4 @@ -1257,6 +1264,7 @@ Features .. _@chrisgrande: https://github.com/chrisgrande .. _@cjsoftuk: https://github.com/cjsoftuk .. _@costela: https://github.com/costela +.. _@coupa-anya: https://github.com/coupa-anya .. _@decibyte: https://github.com/decibyte .. _@dominik-lekse: https://github.com/dominik-lekse .. _@ewingrj: https://github.com/ewingrj diff --git a/anymail/exceptions.py b/anymail/exceptions.py index 085eb53..7e2b349 100644 --- a/anymail/exceptions.py +++ b/anymail/exceptions.py @@ -39,26 +39,10 @@ class AnymailError(Exception): parts = [ " ".join([str(arg) for arg in self.args]), self.describe_cause(), - self.describe_send(), self.describe_response(), ] return "\n".join(filter(None, parts)) - def describe_send(self): - """Return a string describing the ESP send in self.email_message, or None""" - if self.email_message is None: - return None - description = "Sending a message" - try: - description += " to %s" % ','.join(self.email_message.to) - except AttributeError: - pass - try: - description += " from %s" % self.email_message.from_email - except AttributeError: - pass - return description - def describe_response(self): """Return a formatted string of self.status_code and response, or None""" if self.status_code is None: diff --git a/tests/test_general_backend.py b/tests/test_general_backend.py index ebe73bf..0c54589 100644 --- a/tests/test_general_backend.py +++ b/tests/test_general_backend.py @@ -11,7 +11,7 @@ from django.utils.timezone import utc from django.utils.translation import gettext_lazy from anymail.backends.test import EmailBackend as TestBackend, TestPayload -from anymail.exceptions import AnymailConfigurationError, AnymailInvalidAddress, AnymailUnsupportedFeature +from anymail.exceptions import AnymailConfigurationError, AnymailError, AnymailInvalidAddress, AnymailUnsupportedFeature from anymail.message import AnymailMessage from anymail.utils import get_anymail_setting @@ -364,6 +364,24 @@ class CatchCommonErrorsTests(TestBackendTestCase): " in `extra_headers['From']`. (Maybe missing quotes around a display-name?)"): self.message.send() + def test_error_minimizes_pii_leakage(self): + """ + AnymailError messages should generally avoid including + email addresses where not relevant to the error. + + (This is not a guarantee that exceptions will never include + email addresses or other PII. The ESP's own error--which *is* + deliberately included in the message--will often include the + email address, and Anymail makes no attempt to filter that.) + """ + # Cause an error (not related to the specific email addresses involved): + self.message.attach_alternative("...", "audio/mpeg4") + with self.assertRaises(AnymailError) as cm: + self.message.send() + error = cm.exception + self.assertNotIn("from@example.com", str(error)) + self.assertNotIn("to@example.com", str(error)) + def flatten_emails(emails): return [str(email) for email in emails]