diff --git a/gateway/platforms/email.py b/gateway/platforms/email.py index 94cd840bd..a54bd94bb 100644 --- a/gateway/platforms/email.py +++ b/gateway/platforms/email.py @@ -337,60 +337,63 @@ class EmailAdapter(BasePlatformAdapter): results = [] try: imap = imaplib.IMAP4_SSL(self._imap_host, self._imap_port, timeout=30) - imap.login(self._address, self._password) - imap.select("INBOX") + try: + imap.login(self._address, self._password) + imap.select("INBOX") - status, data = imap.uid("search", None, "UNSEEN") - if status != "OK" or not data or not data[0]: - imap.logout() - return results + status, data = imap.uid("search", None, "UNSEEN") + if status != "OK" or not data or not data[0]: + return results - for uid in data[0].split(): - if uid in self._seen_uids: - continue - self._seen_uids.add(uid) - # Trim periodically to prevent unbounded memory growth - if len(self._seen_uids) > self._seen_uids_max: - self._trim_seen_uids() + for uid in data[0].split(): + if uid in self._seen_uids: + continue + self._seen_uids.add(uid) + # Trim periodically to prevent unbounded memory growth + if len(self._seen_uids) > self._seen_uids_max: + self._trim_seen_uids() - status, msg_data = imap.uid("fetch", uid, "(RFC822)") - if status != "OK": - continue + status, msg_data = imap.uid("fetch", uid, "(RFC822)") + if status != "OK": + continue - raw_email = msg_data[0][1] - msg = email_lib.message_from_bytes(raw_email) + raw_email = msg_data[0][1] + msg = email_lib.message_from_bytes(raw_email) - sender_raw = msg.get("From", "") - sender_addr = _extract_email_address(sender_raw) - sender_name = _decode_header_value(sender_raw) - # Remove email from name if present - if "<" in sender_name: - sender_name = sender_name.split("<")[0].strip().strip('"') + sender_raw = msg.get("From", "") + sender_addr = _extract_email_address(sender_raw) + sender_name = _decode_header_value(sender_raw) + # Remove email from name if present + if "<" in sender_name: + sender_name = sender_name.split("<")[0].strip().strip('"') - subject = _decode_header_value(msg.get("Subject", "(no subject)")) - message_id = msg.get("Message-ID", "") - in_reply_to = msg.get("In-Reply-To", "") - # Skip automated/noreply senders before any processing - msg_headers = dict(msg.items()) - if _is_automated_sender(sender_addr, msg_headers): - logger.debug("[Email] Skipping automated sender: %s", sender_addr) - continue - body = _extract_text_body(msg) - attachments = _extract_attachments(msg, skip_attachments=self._skip_attachments) + subject = _decode_header_value(msg.get("Subject", "(no subject)")) + message_id = msg.get("Message-ID", "") + in_reply_to = msg.get("In-Reply-To", "") + # Skip automated/noreply senders before any processing + msg_headers = dict(msg.items()) + if _is_automated_sender(sender_addr, msg_headers): + logger.debug("[Email] Skipping automated sender: %s", sender_addr) + continue + body = _extract_text_body(msg) + attachments = _extract_attachments(msg, skip_attachments=self._skip_attachments) - results.append({ - "uid": uid, - "sender_addr": sender_addr, - "sender_name": sender_name, - "subject": subject, - "message_id": message_id, - "in_reply_to": in_reply_to, - "body": body, - "attachments": attachments, - "date": msg.get("Date", ""), - }) - - imap.logout() + results.append({ + "uid": uid, + "sender_addr": sender_addr, + "sender_name": sender_name, + "subject": subject, + "message_id": message_id, + "in_reply_to": in_reply_to, + "body": body, + "attachments": attachments, + "date": msg.get("Date", ""), + }) + finally: + try: + imap.logout() + except Exception: + pass except Exception as e: logger.error("[Email] IMAP fetch error: %s", e) return results @@ -503,10 +506,15 @@ class EmailAdapter(BasePlatformAdapter): msg.attach(MIMEText(body, "plain", "utf-8")) smtp = smtplib.SMTP(self._smtp_host, self._smtp_port, timeout=30) - smtp.starttls(context=ssl.create_default_context()) - smtp.login(self._address, self._password) - smtp.send_message(msg) - smtp.quit() + try: + smtp.starttls(context=ssl.create_default_context()) + smtp.login(self._address, self._password) + smtp.send_message(msg) + finally: + try: + smtp.quit() + except Exception: + smtp.close() logger.info("[Email] Sent reply to %s (subject: %s)", to_addr, subject) return msg_id @@ -590,10 +598,15 @@ class EmailAdapter(BasePlatformAdapter): msg.attach(part) smtp = smtplib.SMTP(self._smtp_host, self._smtp_port, timeout=30) - smtp.starttls(context=ssl.create_default_context()) - smtp.login(self._address, self._password) - smtp.send_message(msg) - smtp.quit() + try: + smtp.starttls(context=ssl.create_default_context()) + smtp.login(self._address, self._password) + smtp.send_message(msg) + finally: + try: + smtp.quit() + except Exception: + smtp.close() return msg_id diff --git a/tests/gateway/test_email.py b/tests/gateway/test_email.py index 16a418da8..b6da07921 100644 --- a/tests/gateway/test_email.py +++ b/tests/gateway/test_email.py @@ -1057,5 +1057,122 @@ class TestSendEmailStandalone(unittest.TestCase): self.assertIn("not configured", result["error"]) +class TestSmtpConnectionCleanup(unittest.TestCase): + """Verify SMTP connections are closed even when send_message raises.""" + + @patch.dict(os.environ, { + "EMAIL_ADDRESS": "hermes@test.com", + "EMAIL_PASSWORD": "secret", + "EMAIL_IMAP_HOST": "imap.test.com", + "EMAIL_SMTP_HOST": "smtp.test.com", + "EMAIL_SMTP_PORT": "587", + }, clear=False) + def _make_adapter(self): + from gateway.config import PlatformConfig + from gateway.platforms.email import EmailAdapter + return EmailAdapter(PlatformConfig(enabled=True)) + + @patch.dict(os.environ, { + "EMAIL_ADDRESS": "hermes@test.com", + "EMAIL_PASSWORD": "secret", + "EMAIL_IMAP_HOST": "imap.test.com", + "EMAIL_SMTP_HOST": "smtp.test.com", + "EMAIL_SMTP_PORT": "587", + }, clear=False) + def test_smtp_quit_called_on_send_message_failure(self): + """SMTP quit() must be called even when send_message() raises.""" + adapter = self._make_adapter() + mock_smtp = MagicMock() + mock_smtp.send_message.side_effect = Exception("send failed") + + with patch("smtplib.SMTP", return_value=mock_smtp): + with self.assertRaises(Exception): + adapter._send_email("user@test.com", "Hello") + + mock_smtp.quit.assert_called_once() + + @patch.dict(os.environ, { + "EMAIL_ADDRESS": "hermes@test.com", + "EMAIL_PASSWORD": "secret", + "EMAIL_IMAP_HOST": "imap.test.com", + "EMAIL_SMTP_HOST": "smtp.test.com", + "EMAIL_SMTP_PORT": "587", + }, clear=False) + def test_smtp_close_called_when_quit_also_fails(self): + """If both send_message() and quit() fail, close() is the fallback.""" + adapter = self._make_adapter() + mock_smtp = MagicMock() + mock_smtp.send_message.side_effect = Exception("send failed") + mock_smtp.quit.side_effect = Exception("quit failed") + + with patch("smtplib.SMTP", return_value=mock_smtp): + with self.assertRaises(Exception): + adapter._send_email("user@test.com", "Hello") + + mock_smtp.close.assert_called_once() + + +class TestImapConnectionCleanup(unittest.TestCase): + """Verify IMAP connections are closed even when fetch raises.""" + + @patch.dict(os.environ, { + "EMAIL_ADDRESS": "hermes@test.com", + "EMAIL_PASSWORD": "secret", + "EMAIL_IMAP_HOST": "imap.test.com", + "EMAIL_IMAP_PORT": "993", + "EMAIL_SMTP_HOST": "smtp.test.com", + }, clear=False) + def _make_adapter(self): + from gateway.config import PlatformConfig + from gateway.platforms.email import EmailAdapter + return EmailAdapter(PlatformConfig(enabled=True)) + + @patch.dict(os.environ, { + "EMAIL_ADDRESS": "hermes@test.com", + "EMAIL_PASSWORD": "secret", + "EMAIL_IMAP_HOST": "imap.test.com", + "EMAIL_IMAP_PORT": "993", + "EMAIL_SMTP_HOST": "smtp.test.com", + }, clear=False) + def test_imap_logout_called_on_uid_fetch_failure(self): + """IMAP logout() must be called even when uid fetch raises.""" + adapter = self._make_adapter() + mock_imap = MagicMock() + + def uid_handler(command, *args): + if command == "search": + return ("OK", [b"1"]) + if command == "fetch": + raise Exception("fetch failed") + return ("NO", []) + + mock_imap.uid.side_effect = uid_handler + + with patch("imaplib.IMAP4_SSL", return_value=mock_imap): + results = adapter._fetch_new_messages() + + self.assertEqual(results, []) + mock_imap.logout.assert_called_once() + + @patch.dict(os.environ, { + "EMAIL_ADDRESS": "hermes@test.com", + "EMAIL_PASSWORD": "secret", + "EMAIL_IMAP_HOST": "imap.test.com", + "EMAIL_IMAP_PORT": "993", + "EMAIL_SMTP_HOST": "smtp.test.com", + }, clear=False) + def test_imap_logout_called_on_early_return(self): + """IMAP logout() must be called even when returning early (no unseen).""" + adapter = self._make_adapter() + mock_imap = MagicMock() + mock_imap.uid.return_value = ("OK", [b""]) + + with patch("imaplib.IMAP4_SSL", return_value=mock_imap): + results = adapter._fetch_new_messages() + + self.assertEqual(results, []) + mock_imap.logout.assert_called_once() + + if __name__ == "__main__": unittest.main()