Require mailsuite >= 2.3.3 and simplify the Maildir poison-message test (#933)

mailsuite 2.3.3 (GHSA-9jx3-p8wm-g8vw) returns stored bytes from fetch_message() and renames the file in move_message(), so poison messages now always land in the Invalid folder.

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Changes
+
+- **mailsuite 2.3.3 or later is now required.** mailsuite 2.3.3 ([GHSA-9jx3-p8wm-g8vw](https://github.com/seanthegeek/mailsuite/security/advisories/GHSA-9jx3-p8wm-g8vw)) makes the Maildir connection return a message's stored bytes from `fetch_message()` and rename the file in `move_message()`, instead of re-serializing it. A report email that cannot be re-serialized (the [GHSA-qxwv-2vwj-v828](https://github.com/domainaware/parsedmarc/security/advisories/GHSA-qxwv-2vwj-v828) messages) is therefore now moved to the `Invalid` folder on Maildir too, instead of staying in the inbox and being logged on every run.
+
 ## 11.1.0
 
 ### Security

parsedmarc/__init__.py

@@ -2625,9 +2625,10 @@ def _fetch_mailbox_message(
     Returns ``(message_id, msg_content)``; ``message_id`` is the
     backend-appropriate id to use for later move/delete calls.
 
-    Some backends re-serialize the stored message while fetching it (the
-    Maildir backend calls ``Message.as_string()``), so the message's own
-    content can make the fetch fail: ``email.errors.HeaderWriteError`` on a
+    A backend that re-serializes the stored message while fetching it (the
+    Maildir backend of mailsuite before 2.3.3 called ``Message.as_string()``;
+    2.3.3 returns the stored bytes) can fail on the message's own content:
+    ``email.errors.HeaderWriteError`` on a
     control character in a header, ``RecursionError`` on deeply nested
     multiparts. Those content errors are returned, not raised, as
     ``msg_content`` -- a ``ParserError`` the caller treats like a parse
@@ -2675,14 +2676,14 @@ def _dispose_invalid_message(
     A delete or move the backend rejects is logged and the message is left
     in place, as the archival disposal of parsed reports does, rather than
     aborting the rest of the batch. Besides transient backend errors, this
-    covers messages the backend cannot rewrite: the Maildir backend moves a
-    message by storing it again with ``mailbox.Maildir.add()``, which
-    re-serializes it through ``email.generator.BytesGenerator``. That raises
-    ``RecursionError`` for deeply nested multiparts on every Python, and
+    covers messages the backend cannot rewrite: the Maildir backend of
+    mailsuite before 2.3.3 moved a message by storing it again with
+    ``mailbox.Maildir.add()``, which re-serializes it and raises
+    ``RecursionError`` for deeply nested multiparts (and
     ``email.errors.HeaderWriteError`` for a control character in a header
-    only where ``BytesGenerator`` also verifies generated headers (some
-    distribution builds; upstream CPython checks only the text generator
-    used by ``as_string()``) (GHSA-qxwv-2vwj-v828).
+    where ``BytesGenerator`` verifies headers), so such a message stayed in
+    the inbox. mailsuite 2.3.3 renames the file instead
+    (GHSA-qxwv-2vwj-v828, GHSA-9jx3-p8wm-g8vw).
     """
     try:
         if delete:

pyproject.toml

@@ -56,7 +56,7 @@ dependencies = [
     # No extras: the base mailsuite supplies IMAP and Maildir connections
     # plus mail-parser. Gmail and Microsoft Graph come from the gmail and
     # msgraph extras below.
-    "mailsuite>=2.3.2",
+    "mailsuite>=2.3.3",
     "maxminddb>=2.0.0",
     "publicsuffixlist>=0.10.0",
     # Imported directly at utils.py:39 (dateutil.parser). It used to
@@ -93,13 +93,13 @@ loganalytics = [
     "azure-monitor-ingestion>=1.0.0",
 ]
 msgraph = [
-    "mailsuite[msgraph]>=2.3.1",
+    "mailsuite[msgraph]>=2.3.3",
     # Imported directly in cli.py for Graph error handling; otherwise
     # only a transitive dep of mailsuite[msgraph] -> msgraph-sdk.
     "microsoft-kiota-abstractions>=1.8.0",
 ]
 gmail = [
-    "mailsuite[gmail]>=2.3.1",
+    "mailsuite[gmail]>=2.3.3",
 ]
 # `all` must stay the union of every extra above — and must keep
 # excluding `postgresql`: psycopg's prebuilt binary wheels do not exist
@@ -115,7 +115,7 @@ all = [
     "boto3>=1.16.63",
     "elasticsearch>=8.18,<9",
     "kafka-python>=2.3.2",
-    "mailsuite[gmail,msgraph]>=2.3.1",
+    "mailsuite[gmail,msgraph]>=2.3.3",
     "microsoft-kiota-abstractions>=1.8.0",
     "opensearch-py>=2.4.2,<=4.0.0",
     "pygelf>=0.4.2",

tests/test_init.py

@@ -4975,46 +4975,30 @@ class TestGetDmarcReportsFromMailboxMaildir(unittest.TestCase):
                 "is not a poison message here"
             )
 
-    def _maildir_stores_parsed(self, poison):
-        """Whether this Python's ``mailbox.Maildir.add()`` can store
-        ``poison`` once it has been read back as a message object, which is
-        what MaildirConnection.move_message() does."""
-        scratch = mkdtemp(dir=self._tmp)
-        source = mailbox.Maildir(os.path.join(scratch, "source"), create=True)
-        destination = mailbox.Maildir(os.path.join(scratch, "dest"), create=True)
-        try:
-            destination.add(source[source.add(poison)])
-        except (email.errors.MessageError, RecursionError):
-            return False
-        finally:
-            rmtree(scratch, ignore_errors=True)
-        return True
-
     def test_poison_message_does_not_abort_the_batch(self):
-        """A message the Maildir backend cannot re-serialize while fetching
-        it no longer aborts the batch: the other report is still parsed and
-        archived, sequentially and with n_procs=2.
+        """A message that cannot be re-serialized no longer aborts the
+        batch: the other report is still parsed and archived, sequentially
+        and with n_procs=2, and the poison message is moved to
+        ``Archive/Invalid``.
 
-        Regression test for GHSA-qxwv-2vwj-v828: MaildirConnection's
-        fetch_message() calls Message.as_string(), so the
+        Regression test for GHSA-qxwv-2vwj-v828: parse_report_email()
+        serializes the message's parts, so the
         email.errors.HeaderWriteError (a control character in a sub-message
         header) or RecursionError (deeply nested multiparts) escaped from the
-        fetch, outside the per-message ``except ParserError``, and every
-        run aborted on the same message.
+        mailbox loop's per-message ``except ParserError`` and every run
+        aborted on the same message. parse_report_email() now raises a
+        ParserError, which logs "Unable to parse message with subject ..."
+        and disposes of the message as invalid.
 
-        The message is then disposed of as invalid. move_message() stores
-        it with ``mailbox.Maildir.add()``, which re-serializes it through
-        ``email.generator.BytesGenerator``. The nested message overflows the
-        stack there on every Python, so its move fails, is logged, and the
-        message stays in the INBOX instead of aborting the batch a second
-        way. Whether the control-character message can be stored depends on
-        the interpreter: upstream CPython (e.g. 3.12.9) applies the
-        CVE-2024-6923 header check only in the text ``Generator`` that
-        ``as_string()`` uses, so the move succeeds, while some distribution
-        builds (e.g. Ubuntu's 3.12.3) also apply it in ``BytesGenerator``,
-        so the move fails like the nested one. The expected outcome is
-        decided per message, at runtime."""
-        kept_in_inbox = 0
+        mailsuite 2.3.3 (GHSA-9jx3-p8wm-g8vw) returns the stored bytes from
+        fetch_message() and renames the file in move_message(), so neither
+        step re-serializes the message: the fetch succeeds and the failure
+        surfaces in the parser, and the move always succeeds. (Before 2.3.3
+        the fetch raised, and the move re-added the message through
+        ``mailbox.Maildir.add()``, which left the nested message in the
+        inbox.) A Python whose ``Message.as_string()`` can write the
+        control-character message is skipped for that message, as it is not
+        a poison message there."""
         for poison_name, poison in (
             ("header", POISON_HEADER_EMAIL),
             ("nested", _nested_multipart_email(POISON_NESTING_LEVELS)),
@@ -5022,15 +5006,10 @@ class TestGetDmarcReportsFromMailboxMaildir(unittest.TestCase):
             if _reserializes(poison):
                 # Not a poison message on this Python (see _reserializes).
                 continue
-            stored = self._maildir_stores_parsed(poison)
-            if not stored:
-                kept_in_inbox += 1
             for n_procs in (1, 2):
                 with self.subTest(poison=poison_name, n_procs=n_procs):
                     parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear()
-                    path, poison_key = self._poison_maildir(
-                        f"{poison_name}-{n_procs}", poison
-                    )
+                    path, _ = self._poison_maildir(f"{poison_name}-{n_procs}", poison)
                     conn = MaildirConnection(path, maildir_create=True)
                     with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
                         result = parsedmarc.get_dmarc_reports_from_mailbox(
@@ -5048,32 +5027,18 @@ class TestGetDmarcReportsFromMailboxMaildir(unittest.TestCase):
                     self.assertEqual(len(conn.fetch_messages("Archive/Aggregate")), 1)
                     self.assertTrue(
                         any(
-                            f"Unable to fetch message UID {poison_key}" in line
+                            "Unable to parse message with subject" in line
                             for line in cm.output
                         ),
                         cm.output,
                     )
-                    move_errors = [
-                        line
-                        for line in cm.output
-                        if "Mailbox error: Error moving invalid message UID "
-                        f"{poison_key}" in line
-                    ]
-                    if stored:
-                        self.assertEqual(conn.fetch_messages("INBOX"), [])
-                        self.assertEqual(len(conn.fetch_messages("Archive/Invalid")), 1)
-                        self.assertFalse(
-                            [line for line in cm.output if "Mailbox error" in line],
-                            cm.output,
-                        )
-                    else:
-                        self.assertEqual(conn.fetch_messages("INBOX"), [poison_key])
-                        self.assertEqual(conn.fetch_messages("Archive/Invalid"), [])
-                        self.assertTrue(move_errors, cm.output)
-        # The nested message cannot be stored on any Python, so the
-        # kept-in-INBOX branch always runs; the stored branch runs for the
-        # control-character message on upstream CPython.
-        self.assertGreater(kept_in_inbox, 0)
+                    # The poison message was moved, not left to fail again.
+                    self.assertEqual(conn.fetch_messages("INBOX"), [])
+                    self.assertEqual(len(conn.fetch_messages("Archive/Invalid")), 1)
+                    self.assertFalse(
+                        [line for line in cm.output if "Mailbox error" in line],
+                        cm.output,
+                    )
 
     def test_poison_message_is_deleted_with_delete_invalid(self):
         """With delete_invalid, a message that failed to fetch is deleted