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
This commit is contained in:
medmunds
2022-01-11 16:30:07 -08:00
committed by Mike Edmunds
parent 60fbe1e896
commit 10f569cd50
3 changed files with 27 additions and 17 deletions

View File

@@ -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

View File

@@ -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:

View File

@@ -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]