mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-07 02:51:50 +00:00
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
This commit is contained in:
parent
20edca75e9
commit
fd9c32c0f2
2 changed files with 97 additions and 0 deletions
|
|
@ -416,6 +416,18 @@ class EmailAdapter(BasePlatformAdapter):
|
||||||
logger.debug("[Email] Dropping automated sender at dispatch: %s", sender_addr)
|
logger.debug("[Email] Dropping automated sender at dispatch: %s", sender_addr)
|
||||||
return
|
return
|
||||||
|
|
||||||
|
# Skip senders not in EMAIL_ALLOWED_USERS — prevents the adapter
|
||||||
|
# from creating a MessageEvent (and thus thread context) for senders
|
||||||
|
# that the gateway will never authorize. Without this early guard,
|
||||||
|
# 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:
|
||||||
|
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)
|
||||||
|
return
|
||||||
|
|
||||||
subject = msg_data["subject"]
|
subject = msg_data["subject"]
|
||||||
body = msg_data["body"].strip()
|
body = msg_data["body"].strip()
|
||||||
attachments = msg_data["attachments"]
|
attachments = msg_data["attachments"]
|
||||||
|
|
|
||||||
|
|
@ -425,6 +425,91 @@ class TestDispatchMessage(unittest.TestCase):
|
||||||
self.assertEqual(event.source.user_name, "John Doe")
|
self.assertEqual(event.source.user_name, "John Doe")
|
||||||
self.assertEqual(event.source.chat_type, "dm")
|
self.assertEqual(event.source.chat_type, "dm")
|
||||||
|
|
||||||
|
def test_non_allowlisted_sender_dropped(self):
|
||||||
|
"""Senders not in EMAIL_ALLOWED_USERS should be dropped before dispatch."""
|
||||||
|
import asyncio
|
||||||
|
with patch.dict(os.environ, {
|
||||||
|
"EMAIL_ALLOWED_USERS": "hermes@test.com,admin@test.com",
|
||||||
|
}):
|
||||||
|
adapter = self._make_adapter()
|
||||||
|
adapter._message_handler = MagicMock()
|
||||||
|
|
||||||
|
msg_data = {
|
||||||
|
"uid": b"99",
|
||||||
|
"sender_addr": "outsider@evil.com",
|
||||||
|
"sender_name": "Spammer",
|
||||||
|
"subject": "Buy now!!!",
|
||||||
|
"message_id": "<spam@evil.com>",
|
||||||
|
"in_reply_to": "",
|
||||||
|
"body": "Cheap meds",
|
||||||
|
"attachments": [],
|
||||||
|
"date": "",
|
||||||
|
}
|
||||||
|
|
||||||
|
asyncio.run(adapter._dispatch_message(msg_data))
|
||||||
|
# Handler should NOT be called for non-allowlisted sender
|
||||||
|
adapter._message_handler.assert_not_called()
|
||||||
|
# Thread context should NOT be created
|
||||||
|
self.assertNotIn("outsider@evil.com", adapter._thread_context)
|
||||||
|
|
||||||
|
def test_allowlisted_sender_proceeds(self):
|
||||||
|
"""Senders in EMAIL_ALLOWED_USERS should proceed to dispatch normally."""
|
||||||
|
import asyncio
|
||||||
|
with patch.dict(os.environ, {
|
||||||
|
"EMAIL_ALLOWED_USERS": "hermes@test.com,admin@test.com",
|
||||||
|
}):
|
||||||
|
adapter = self._make_adapter()
|
||||||
|
captured_events = []
|
||||||
|
|
||||||
|
async def mock_handler(event):
|
||||||
|
captured_events.append(event)
|
||||||
|
return None
|
||||||
|
|
||||||
|
adapter._message_handler = mock_handler
|
||||||
|
|
||||||
|
msg_data = {
|
||||||
|
"uid": b"100",
|
||||||
|
"sender_addr": "admin@test.com",
|
||||||
|
"sender_name": "Admin",
|
||||||
|
"subject": "Important",
|
||||||
|
"message_id": "<msg@test.com>",
|
||||||
|
"in_reply_to": "",
|
||||||
|
"body": "Hello",
|
||||||
|
"attachments": [],
|
||||||
|
"date": "",
|
||||||
|
}
|
||||||
|
|
||||||
|
asyncio.run(adapter._dispatch_message(msg_data))
|
||||||
|
self.assertEqual(len(captured_events), 1)
|
||||||
|
self.assertEqual(captured_events[0].source.chat_id, "admin@test.com")
|
||||||
|
|
||||||
|
def test_empty_allowlist_allows_all(self):
|
||||||
|
"""When EMAIL_ALLOWED_USERS is not set, all senders should proceed."""
|
||||||
|
import asyncio
|
||||||
|
with patch.dict(os.environ, {}, clear=False):
|
||||||
|
# Ensure EMAIL_ALLOWED_USERS is not in the env
|
||||||
|
if "EMAIL_ALLOWED_USERS" in os.environ:
|
||||||
|
del os.environ["EMAIL_ALLOWED_USERS"]
|
||||||
|
|
||||||
|
adapter = self._make_adapter()
|
||||||
|
adapter._message_handler = MagicMock()
|
||||||
|
|
||||||
|
msg_data = {
|
||||||
|
"uid": b"101",
|
||||||
|
"sender_addr": "anyone@test.com",
|
||||||
|
"sender_name": "Anyone",
|
||||||
|
"subject": "Hey",
|
||||||
|
"message_id": "<any@test.com>",
|
||||||
|
"in_reply_to": "",
|
||||||
|
"body": "Hi",
|
||||||
|
"attachments": [],
|
||||||
|
"date": "",
|
||||||
|
}
|
||||||
|
|
||||||
|
asyncio.run(adapter._dispatch_message(msg_data))
|
||||||
|
# Handler should be called when no allowlist is configured
|
||||||
|
adapter._message_handler.assert_called()
|
||||||
|
|
||||||
|
|
||||||
class TestThreadContext(unittest.TestCase):
|
class TestThreadContext(unittest.TestCase):
|
||||||
"""Test email reply threading logic."""
|
"""Test email reply threading logic."""
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue