Skip to content

Commit fd9c32c

Browse files
albert748teknium1
authored andcommitted
fix(email): drop non-allowlisted senders before dispatch to prevent mail loops
Add EMAIL_ALLOWED_USERS check in EmailAdapter._dispatch_message() to silently discard emails from senders not in the allowlist. This prevents the adapter from creating thread context and dispatching a MessageEvent for unauthorized senders, which could race with the gateway authorization check and result in SMTP replies being sent despite the handler returning None. Test: tests/gateway/test_email.py::TestDispatchMessage::test_non_allowlisted_sender_dropped Test: tests/gateway/test_email.py::TestDispatchMessage::test_allowlisted_sender_proceeds Test: tests/gateway/test_email.py::TestDispatchMessage::test_empty_allowlist_allows_all
1 parent 20edca7 commit fd9c32c

2 files changed

Lines changed: 97 additions & 0 deletions

File tree

gateway/platforms/email.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,18 @@ async def _dispatch_message(self, msg_data: Dict[str, Any]) -> None:
416416
logger.debug("[Email] Dropping automated sender at dispatch: %s", sender_addr)
417417
return
418418

419+
# Skip senders not in EMAIL_ALLOWED_USERS — prevents the adapter
420+
# from creating a MessageEvent (and thus thread context) for senders
421+
# that the gateway will never authorize. Without this early guard,
422+
# a race between dispatch and authorization can result in the adapter
423+
# sending a reply even though the handler returned None.
424+
allowed_raw = os.getenv("EMAIL_ALLOWED_USERS", "").strip()
425+
if allowed_raw:
426+
allowed = {addr.strip().lower() for addr in allowed_raw.split(",") if addr.strip()}
427+
if sender_addr.lower() not in allowed:
428+
logger.debug("[Email] Dropping non-allowlisted sender at dispatch: %s", sender_addr)
429+
return
430+
419431
subject = msg_data["subject"]
420432
body = msg_data["body"].strip()
421433
attachments = msg_data["attachments"]

tests/gateway/test_email.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,91 @@ async def capture_handle(event):
425425
self.assertEqual(event.source.user_name, "John Doe")
426426
self.assertEqual(event.source.chat_type, "dm")
427427

428+
def test_non_allowlisted_sender_dropped(self):
429+
"""Senders not in EMAIL_ALLOWED_USERS should be dropped before dispatch."""
430+
import asyncio
431+
with patch.dict(os.environ, {
432+
"EMAIL_ALLOWED_USERS": "hermes@test.com,admin@test.com",
433+
}):
434+
adapter = self._make_adapter()
435+
adapter._message_handler = MagicMock()
436+
437+
msg_data = {
438+
"uid": b"99",
439+
"sender_addr": "outsider@evil.com",
440+
"sender_name": "Spammer",
441+
"subject": "Buy now!!!",
442+
"message_id": "<spam@evil.com>",
443+
"in_reply_to": "",
444+
"body": "Cheap meds",
445+
"attachments": [],
446+
"date": "",
447+
}
448+
449+
asyncio.run(adapter._dispatch_message(msg_data))
450+
# Handler should NOT be called for non-allowlisted sender
451+
adapter._message_handler.assert_not_called()
452+
# Thread context should NOT be created
453+
self.assertNotIn("outsider@evil.com", adapter._thread_context)
454+
455+
def test_allowlisted_sender_proceeds(self):
456+
"""Senders in EMAIL_ALLOWED_USERS should proceed to dispatch normally."""
457+
import asyncio
458+
with patch.dict(os.environ, {
459+
"EMAIL_ALLOWED_USERS": "hermes@test.com,admin@test.com",
460+
}):
461+
adapter = self._make_adapter()
462+
captured_events = []
463+
464+
async def mock_handler(event):
465+
captured_events.append(event)
466+
return None
467+
468+
adapter._message_handler = mock_handler
469+
470+
msg_data = {
471+
"uid": b"100",
472+
"sender_addr": "admin@test.com",
473+
"sender_name": "Admin",
474+
"subject": "Important",
475+
"message_id": "<msg@test.com>",
476+
"in_reply_to": "",
477+
"body": "Hello",
478+
"attachments": [],
479+
"date": "",
480+
}
481+
482+
asyncio.run(adapter._dispatch_message(msg_data))
483+
self.assertEqual(len(captured_events), 1)
484+
self.assertEqual(captured_events[0].source.chat_id, "admin@test.com")
485+
486+
def test_empty_allowlist_allows_all(self):
487+
"""When EMAIL_ALLOWED_USERS is not set, all senders should proceed."""
488+
import asyncio
489+
with patch.dict(os.environ, {}, clear=False):
490+
# Ensure EMAIL_ALLOWED_USERS is not in the env
491+
if "EMAIL_ALLOWED_USERS" in os.environ:
492+
del os.environ["EMAIL_ALLOWED_USERS"]
493+
494+
adapter = self._make_adapter()
495+
adapter._message_handler = MagicMock()
496+
497+
msg_data = {
498+
"uid": b"101",
499+
"sender_addr": "anyone@test.com",
500+
"sender_name": "Anyone",
501+
"subject": "Hey",
502+
"message_id": "<any@test.com>",
503+
"in_reply_to": "",
504+
"body": "Hi",
505+
"attachments": [],
506+
"date": "",
507+
}
508+
509+
asyncio.run(adapter._dispatch_message(msg_data))
510+
# Handler should be called when no allowlist is configured
511+
adapter._message_handler.assert_called()
512+
428513

429514
class TestThreadContext(unittest.TestCase):
430515
"""Test email reply threading logic."""

0 commit comments

Comments
 (0)