fix(email): harden adapter against malformed IMAP responses (#2794 salvage)

plugins/platforms/email/adapter.py

@@ -673,7 +673,25 @@ def _fetch_new_messages(self) -> List[Dict[str, Any]]:
                     if status != "OK":
                         continue
 
-                    raw_email = msg_data[0][1]
+                    # IMAP fetch can return unexpected structures (e.g. a
+                    # single bytes item instead of a list of tuples). Guard
+                    # against IndexError / TypeError so one malformed response
+                    # doesn't abort the batch — the UID is already in
+                    # _seen_uids, so an abort would permanently skip the
+                    # remaining messages in this batch.
+                    try:
+                        raw_email = msg_data[0][1]
+                    except (IndexError, TypeError):
+                        logger.warning(
+                            "[Email] Unexpected IMAP response structure for UID %s, skipping",
+                            uid,
+                        )
+                        continue
+                    if not isinstance(raw_email, (bytes, bytearray)):
+                        logger.warning(
+                            "[Email] Non-bytes IMAP payload for UID %s, skipping", uid
+                        )
+                        continue
                     msg = email_lib.message_from_bytes(raw_email)
 
                     sender_raw = msg.get("From", "")
@@ -776,7 +794,17 @@ async def _dispatch_message(self, msg_data: Dict[str, Any]) -> None:
         # a race between dispatch and authorization can result in the adapter
         # sending a reply even though the handler returned None.
         allowed_raw = os.getenv("EMAIL_ALLOWED_USERS", "").strip()
-        if allowed_raw:
+        if not allowed_raw:
+            if os.getenv("EMAIL_ALLOW_ALL_USERS", "").strip().lower() not in {"true", "1", "yes"} and (
+                os.getenv("GATEWAY_ALLOW_ALL_USERS", "").strip().lower() not in {"true", "1", "yes"}
+            ):
+                logger.debug(
+                    "[Email] Dropping sender at dispatch — EMAIL_ALLOWED_USERS is unset "
+                    "and open access is not opted in: %s",
+                    sender_addr,
+                )
+                return
+        else:
             allowed = {addr.strip().lower() for addr in allowed_raw.split(",") if addr.strip()}
             if sender_addr.lower() not in allowed:
                 logger.debug("[Email] Dropping non-allowlisted sender at dispatch: %s", sender_addr)
@@ -880,6 +908,16 @@ async def send(
             logger.error("[Email] Send failed to %s: %s", chat_id, e)
             return SendResult(success=False, error=str(e))
 
+    def _message_id_domain(self) -> str:
+        """Domain part for generated Message-IDs.
+
+        EMAIL_ADDRESS may lack an ``@`` (misconfiguration); fall back to
+        ``localhost`` instead of crashing send with an IndexError.
+        """
+        if "@" in self._address:
+            return self._address.rsplit("@", 1)[-1] or "localhost"
+        return "localhost"
+
     def _send_email(
         self,
         to_addr: str,
@@ -905,7 +943,7 @@ def _send_email(
             msg["References"] = original_msg_id
 
         msg["Date"] = formatdate(localtime=True)
-        msg_id = f"<hermes-{uuid.uuid4().hex[:12]}@{self._address.split('@')[1]}>"
+        msg_id = f"<hermes-{uuid.uuid4().hex[:12]}@{self._message_id_domain()}>"
         msg["Message-ID"] = msg_id
 
         msg.attach(MIMEText(body, "plain", "utf-8"))
@@ -1018,7 +1056,7 @@ def _send_email_with_attachments(
             msg["References"] = original_msg_id
 
         msg["Date"] = formatdate(localtime=True)
-        msg_id = f"<hermes-{uuid.uuid4().hex[:12]}@{self._address.split('@')[1]}>"
+        msg_id = f"<hermes-{uuid.uuid4().hex[:12]}@{self._message_id_domain()}>"
         msg["Message-ID"] = msg_id
 
         if body:
@@ -1098,7 +1136,7 @@ def _send_email_with_attachment(
             msg["References"] = original_msg_id
 
         msg["Date"] = formatdate(localtime=True)
-        msg_id = f"<hermes-{uuid.uuid4().hex[:12]}@{self._address.split('@')[1]}>"
+        msg_id = f"<hermes-{uuid.uuid4().hex[:12]}@{self._message_id_domain()}>"
         msg["Message-ID"] = msg_id
 
         if body:

scripts/release.py

@@ -46,6 +46,7 @@
 # Auto-extracted from noreply emails + manual overrides
 AUTHOR_MAP = {
     "aqdrgg19@gmail.com": "VolodymyrBg",  # PR #2861 salvage (webhook: drop the unused full request payload from retained _delivery_info entries — up to ~1MB dead weight per delivery for the 1h idempotency TTL)
+    "ohyes9711@gmail.com": "CharmingGroot",  # PR #2794 salvage (email: guard msg_data[0][1] against malformed IMAP fetch structures so one bad response can't abort the batch and permanently lose seen-marked messages; Message-ID domain falls back to localhost when EMAIL_ADDRESS lacks '@')
     "sahibzada@fastino.ai": "sahibzada-allahyar",  # PR #39227 salvage (desktop: configured terminal.cwd overrides a stale remembered workspace-cwd localStorage value when no session is active; #38855)
     "jvsantos.cunha@gmail.com": "plcunha",  # PR #55300 salvage (gateway: record child gateway peer metadata after a compression session-id rotation and repoint stale sessions.json compression-parent entries to the recovered live child; consolidated in the compression-routing-integrity salvage)
     "jakepresent1@gmail.com": "jakepresent",  # PR #55721 salvage (gateway: identity-guard stale in-flight compression splits — a late run may publish its compressed child only if its run generation is still current and the session key still points at the run's original parent, so an old run can't overwrite a newer /new or moved binding)

tests/gateway/test_email_robustness.py

@@ -0,0 +1,109 @@
+"""Email adapter robustness against malformed IMAP responses (salvage of #2794).
+
+Validates that:
+- Malformed IMAP fetch responses are skipped instead of aborting the batch
+  (UIDs are marked seen before fetch, so an abort permanently loses messages)
+- Message-ID generation handles a missing '@' in EMAIL_ADDRESS
+"""
+
+import os
+import unittest
+import uuid
+from email.mime.text import MIMEText
+from unittest.mock import MagicMock, patch
+
+
+def _make_adapter(address="hermes@test.com"):
+    from gateway.config import PlatformConfig
+
+    with patch.dict(os.environ, {
+        "EMAIL_ADDRESS": address,
+        "EMAIL_PASSWORD": "secret",
+        "EMAIL_IMAP_HOST": "imap.test.com",
+        "EMAIL_SMTP_HOST": "smtp.test.com",
+    }):
+        from plugins.platforms.email.adapter import EmailAdapter
+
+        adapter = EmailAdapter(PlatformConfig(enabled=True))
+    return adapter
+
+
+def _raw_email(sender="user@test.com", subject="Hello"):
+    msg = MIMEText("Test body", "plain", "utf-8")
+    msg["From"] = sender
+    msg["Subject"] = subject
+    msg["Message-ID"] = f"<{uuid.uuid4().hex[:8]}@test.com>"
+    return msg.as_bytes()
+
+
+class TestImapResponseGuard(unittest.TestCase):
+    """_fetch_new_messages skips messages with unexpected IMAP structure."""
+
+    def _fetch_with(self, fetch_responses):
+        adapter = _make_adapter()
+        uids = b" ".join(
+            str(i + 1).encode() for i in range(len(fetch_responses))
+        )
+        fetch_iter = iter(fetch_responses)
+
+        def uid_handler(command, *args):
+            if command == "search":
+                return ("OK", [uids])
+            if command == "fetch":
+                return next(fetch_iter)
+            return ("NO", [])
+
+        mock_imap = MagicMock()
+        mock_imap.uid.side_effect = uid_handler
+        with patch("imaplib.IMAP4_SSL", return_value=mock_imap):
+            return adapter._fetch_new_messages()
+
+    def test_normal_response_parses(self):
+        results = self._fetch_with([("OK", [(b"1 (RFC822 {123}", _raw_email())])])
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0]["sender_addr"], "user@test.com")
+
+    def test_none_element_skipped(self):
+        results = self._fetch_with([("OK", [None])])
+        self.assertEqual(results, [])
+
+    def test_empty_list_skipped(self):
+        results = self._fetch_with([("OK", [])])
+        self.assertEqual(results, [])
+
+    def test_bare_bytes_element_skipped(self):
+        # Single bytes item instead of a (header, payload) tuple
+        results = self._fetch_with([("OK", [b"not-a-tuple"])])
+        self.assertEqual(results, [])
+
+    def test_non_bytes_payload_skipped(self):
+        results = self._fetch_with([("OK", [(b"1", None)])])
+        self.assertEqual(results, [])
+
+    def test_malformed_does_not_abort_batch(self):
+        """A malformed response mid-batch must not lose the messages after it."""
+        results = self._fetch_with([
+            ("OK", [None]),                                # UID 1 malformed
+            ("OK", [(b"2 (RFC822 {123}", _raw_email())]),  # UID 2 fine
+        ])
+        self.assertEqual(len(results), 1)
+
+
+class TestMessageIdDomain(unittest.TestCase):
+    """Message-ID generation tolerates EMAIL_ADDRESS without '@'."""
+
+    def test_normal_address(self):
+        adapter = _make_adapter("hermes@example.org")
+        self.assertEqual(adapter._message_id_domain(), "example.org")
+
+    def test_address_without_at(self):
+        adapter = _make_adapter("not-an-email")
+        self.assertEqual(adapter._message_id_domain(), "localhost")
+
+    def test_address_trailing_at(self):
+        adapter = _make_adapter("weird@")
+        self.assertEqual(adapter._message_id_domain(), "localhost")
+
+
+if __name__ == "__main__":
+    unittest.main()