mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(email): resolve IMAP/SMTP host from config and validate before connecting
The email adapter read address/host purely from env vars and never stripped
them, so a missing or whitespace-padded EMAIL_IMAP_HOST reached
imaplib.IMAP4_SSL("") and surfaced as the misleading
"[Errno 8] nodename nor servname provided, or not known" — sending users down a
DNS rabbit hole when the real problem was an empty/dirty host string. A
config.yaml-only setup also left the host empty because __init__ ignored
PlatformConfig.extra, even though the "connected" check, the send helper, and
`hermes config show` already read address/imap_host/smtp_host from it.
Resolve address/imap_host/smtp_host from the env var first, then fall back to
config.extra, and strip surrounding whitespace — matching the send helper's
existing pattern. Validate the required settings at the start of connect() and
return False with an actionable message instead of attempting a connection with
an empty host.
Adds regression tests for whitespace stripping, config.extra fallback, and the
no-IMAP-attempt-on-missing-host path.
This commit is contained in:
parent
4cff0360ea
commit
b7f6cb9c8b
2 changed files with 96 additions and 4 deletions
|
|
@ -307,11 +307,20 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
def __init__(self, config: PlatformConfig):
|
||||
super().__init__(config, Platform.EMAIL)
|
||||
|
||||
self._address = os.getenv("EMAIL_ADDRESS", "")
|
||||
# Resolve connection settings from the env vars first, then fall back to
|
||||
# PlatformConfig.extra (address/imap_host/smtp_host) — the canonical dict
|
||||
# gateway.config populates and that the "connected" check, the
|
||||
# send-helper, and `hermes config show` already read. Without the
|
||||
# fallback a config.yaml-only setup left these empty. Host/address values
|
||||
# are stripped: a stray space or newline made IMAP4_SSL raise the
|
||||
# misleading ``[Errno 8] nodename nor servname`` (an unresolvable name)
|
||||
# instead of an obvious "host not set" error.
|
||||
extra = config.extra or {}
|
||||
self._address = (os.getenv("EMAIL_ADDRESS", "") or extra.get("address", "")).strip()
|
||||
self._password = os.getenv("EMAIL_PASSWORD", "")
|
||||
self._imap_host = os.getenv("EMAIL_IMAP_HOST", "")
|
||||
self._imap_host = (os.getenv("EMAIL_IMAP_HOST", "") or extra.get("imap_host", "")).strip()
|
||||
self._imap_port = env_int("EMAIL_IMAP_PORT", 993)
|
||||
self._smtp_host = os.getenv("EMAIL_SMTP_HOST", "")
|
||||
self._smtp_host = (os.getenv("EMAIL_SMTP_HOST", "") or extra.get("smtp_host", "")).strip()
|
||||
self._smtp_port = env_int("EMAIL_SMTP_PORT", 587)
|
||||
self._poll_interval = env_int("EMAIL_POLL_INTERVAL", 15)
|
||||
|
||||
|
|
@ -319,7 +328,6 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
# platforms:
|
||||
# email:
|
||||
# skip_attachments: true
|
||||
extra = config.extra or {}
|
||||
self._skip_attachments = extra.get("skip_attachments", False)
|
||||
|
||||
# Track message IDs we've already processed to avoid duplicates
|
||||
|
|
@ -396,6 +404,27 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
|
||||
async def connect(self) -> bool:
|
||||
"""Connect to the IMAP server and start polling for new messages."""
|
||||
# Validate up front so a missing host surfaces as an actionable config
|
||||
# error instead of IMAP4_SSL("") raising the cryptic
|
||||
# ``[Errno 8] nodename nor servname provided, or not known``.
|
||||
missing = [
|
||||
name
|
||||
for name, value in (
|
||||
("EMAIL_ADDRESS", self._address),
|
||||
("EMAIL_PASSWORD", self._password),
|
||||
("EMAIL_IMAP_HOST", self._imap_host),
|
||||
("EMAIL_SMTP_HOST", self._smtp_host),
|
||||
)
|
||||
if not value
|
||||
]
|
||||
if missing:
|
||||
logger.error(
|
||||
"[Email] Not configured — missing %s. Set it via `hermes gateway "
|
||||
"setup` (env) or platforms.email in config.yaml.",
|
||||
", ".join(missing),
|
||||
)
|
||||
return False
|
||||
|
||||
try:
|
||||
# Test IMAP connection
|
||||
imap = imaplib.IMAP4_SSL(self._imap_host, self._imap_port, timeout=30)
|
||||
|
|
|
|||
|
|
@ -1392,5 +1392,68 @@ class TestConnectSmtp(unittest.TestCase):
|
|||
self.assertIs(_socket.getaddrinfo, original_getaddrinfo)
|
||||
|
||||
|
||||
class TestConnectionConfigResolution(unittest.TestCase):
|
||||
"""Host/address resolution and pre-connect validation (#49736)."""
|
||||
|
||||
def test_host_and_address_whitespace_stripped(self):
|
||||
"""A stray space/newline must not reach IMAP4_SSL as part of the host.
|
||||
|
||||
Whitespace in the host produced the misleading
|
||||
``[Errno 8] nodename nor servname`` (unresolvable name) instead of a
|
||||
successful connection.
|
||||
"""
|
||||
from gateway.config import PlatformConfig
|
||||
from plugins.platforms.email.adapter import EmailAdapter
|
||||
with patch.dict(os.environ, {
|
||||
"EMAIL_ADDRESS": " hermes@test.com\n",
|
||||
"EMAIL_PASSWORD": "secret",
|
||||
"EMAIL_IMAP_HOST": " imap.test.com ",
|
||||
"EMAIL_SMTP_HOST": "smtp.test.com\n",
|
||||
}, clear=False):
|
||||
adapter = EmailAdapter(PlatformConfig(enabled=True))
|
||||
self.assertEqual(adapter._imap_host, "imap.test.com")
|
||||
self.assertEqual(adapter._smtp_host, "smtp.test.com")
|
||||
self.assertEqual(adapter._address, "hermes@test.com")
|
||||
|
||||
def test_falls_back_to_platform_config_extra(self):
|
||||
"""When env vars are absent, settings come from PlatformConfig.extra —
|
||||
the same dict gateway.config populates and `hermes config show` reads."""
|
||||
from gateway.config import PlatformConfig
|
||||
from plugins.platforms.email.adapter import EmailAdapter
|
||||
cfg = PlatformConfig(enabled=True)
|
||||
cfg.extra.update({
|
||||
"address": "hermes@test.com",
|
||||
"imap_host": "imap.test.com",
|
||||
"smtp_host": "smtp.test.com",
|
||||
})
|
||||
with patch.dict(os.environ, {
|
||||
"EMAIL_ADDRESS": "", "EMAIL_IMAP_HOST": "", "EMAIL_SMTP_HOST": "",
|
||||
"EMAIL_PASSWORD": "secret",
|
||||
}, clear=False):
|
||||
adapter = EmailAdapter(cfg)
|
||||
self.assertEqual(adapter._imap_host, "imap.test.com")
|
||||
self.assertEqual(adapter._smtp_host, "smtp.test.com")
|
||||
self.assertEqual(adapter._address, "hermes@test.com")
|
||||
|
||||
def test_connect_aborts_without_attempting_imap_when_host_missing(self):
|
||||
"""A missing host returns False without the cryptic DNS error."""
|
||||
import asyncio
|
||||
from gateway.config import PlatformConfig
|
||||
from plugins.platforms.email.adapter import EmailAdapter
|
||||
with patch.dict(os.environ, {
|
||||
"EMAIL_ADDRESS": "hermes@test.com",
|
||||
"EMAIL_PASSWORD": "secret",
|
||||
"EMAIL_IMAP_HOST": "",
|
||||
"EMAIL_SMTP_HOST": "smtp.test.com",
|
||||
}, clear=False):
|
||||
adapter = EmailAdapter(PlatformConfig(enabled=True))
|
||||
|
||||
with patch("imaplib.IMAP4_SSL") as mock_imap:
|
||||
result = asyncio.run(adapter.connect())
|
||||
|
||||
self.assertFalse(result)
|
||||
mock_imap.assert_not_called()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue