fix(email): add SMTP security mode configuration - #42907
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for covering both reply and one-shot SMTP paths. The port-465 premise still matters for one-shot sends, but this patch needs to be salvaged onto the current plugin architecture.
Problems
gateway/platforms/email.pyandtools/send_message_tool.py::_send_emailare no longer active paths after560010547. Current one-shot delivery isplugins/platforms/email/adapter.py:_standalone_send(); it still forcesSMTP()+STARTTLSatplugins/platforms/email/adapter.py:1225-1226.- Current gateway replies already select
SMTP_SSLfor port 465 inplugins/platforms/email/adapter.py:509-549and retain an IPv4 fallback. The proposedgateway/email_smtp.pyhelper does not retain that fallback, so it cannot replace the current helper unchanged.
Suggested changes
- Port the shared policy into the current plugin adapter, including
_standalone_send(), while preserving the existing IPv4 and TLS-verification behavior. - Add tests for the current registered one-shot path on port 465 and retain the existing fallback coverage.
Automated hermes-sweeper review.
| server.login(address, password) | ||
| server.send_message(msg) | ||
| server.quit() | ||
| server = open_smtp_connection( |
There was a problem hiding this comment.
This is now a legacy call site. Current main dispatches email through the registered plugin at tools/send_message_tool.py:1068, and the active one-shot implementation is plugins/platforms/email/adapter.py:_standalone_send (:1190), which still uses SMTP() plus STARTTLS at :1225-1226. Port this policy to that plugin path.
There was a problem hiding this comment.
Thanks for the pointer - done. The policy is now ported to the plugin path (plugins/platforms/email/adapter.py), and both SMTP entry points in the adapter share it:
_standalone_send(the active one-shot path, registered as the plugin'sstandalone_sender_fnand dispatched via the registry fromtools/send_message_tool.py:1068): now uses a sharedopen_smtp_connection()instead of the hard-codedSMTP()+STARTTLS.autopicksSMTP_SSLon 465 andSMTP+STARTTLSelsewhere;starttls/implicit_tlsoverride explicitly.login/send_messagewrapped intry/finally.EmailAdapter._connect_smtp(persistent gateway path):port == 465replaced byresolve_smtp_security(), IPv4 fallback preserved.- The shared helpers (
normalize_smtp_security/resolve_smtp_security/open_smtp_connection) live inside the email plugin module rather than a new coregateway/email_smtp.py, keeping the plugin self-contained.
Config housekeeping: EMAIL_SMTP_SECURITY is now bridged to config.yaml (platforms.email.smtp_security via PlatformConfig.extra) as canonical, env var kept as backwards-compatible bridge (terminal.cwd -> TERMINAL_CWD precedent). Docs point to config.yaml.
Tests: tests/plugins/platforms/email/test_email_smtp_security.py (49 cases); tests/gateway/test_email.py still passes (88, no regression).
Port the EMAIL_SMTP_SECURITY / smtp_security policy to the email plugin adapter path (plugins/platforms/email/adapter.py), per maintainer review on PR NousResearch#42907: the legacy tools/send_message_tool.py:_send_email call site has been superseded by the registered plugin's _standalone_send, which still hard-coded SMTP()+STARTTLS. Changes: - Add normalize_smtp_security / resolve_smtp_security / open_smtp_connection as a shared SMTP security policy inside the email plugin adapter module (not a core gateway module), so the policy is self-contained in the plugin. - _connect_smtp (persistent gateway path) now resolves the mode via resolve_smtp_security instead of hard-coding port==465; the existing IPv4 fallback is preserved. - _standalone_send (one-shot path) now uses open_smtp_connection with a try/finally around login+send_message, fixing the implicit-TLS/port-465 gap the maintainer flagged. - smtp_security is read canonically from config.yaml (platforms.email.smtp_security via PlatformConfig.extra); EMAIL_SMTP_SECURITY env is kept as a backwards-compatible bridge (terminal.cwd -> TERMINAL_CWD precedent, AGENTS.md). User-facing docs point to config.yaml. - Add EMAIL_SMTP_SECURITY to the plugin's optional_env manifest, .env.example, the dashboard env reveal/description, and the local terminal env blocklist. - Document the new setting in the email user guide and environment-variables reference (EN + zh-Hans). Tests: tests/plugins/platforms/email/test_email_smtp_security.py covers normalize/resolve/open_smtp_connection/_standalone_send (auto port-based, explicit overrides, aliases, rejection of ambiguous values, STARTTLS-failure cleanup, env bridge). 49 passed; existing tests/gateway/test_email.py 88 passed (no regression).
526c1f2 to
b1a4986
Compare
|
Real-world use case supporting this approach: we run the Hermes email adapter against an SMTP endpoint that requires implicit TLS on the non-standard port The current if port in (465, 10465):
server = smtplib.SMTP_SSL(host, port, context=context)The explicit We would be happy to validate this PR against our port-10465 deployment. |
What does this PR do?
Adds shared SMTP transport-security handling for Hermes email sends.
By default,
EMAIL_SMTP_SECURITY=autopreserves the existing STARTTLS path for port 587 and other non-465 ports, while using implicit TLS /smtplib.SMTP_SSLfor port 465. It also adds explicit override modes for deployments that need to force either behavior:EMAIL_SMTP_SECURITY=autoEMAIL_SMTP_SECURITY=starttlsEMAIL_SMTP_SECURITY=implicit_tlsThis fixes email providers that require implicit TLS on SMTP port 465 without breaking existing STARTTLS setups.
Related Issue
No linked issue. This is a direct bug-fix PR from a forked branch.
Related PRs reviewed while checking for duplicates: #42026, #13564, and #12161. This PR differs by adding a shared configurable policy used by both gateway email replies and one-shot email sends, plus docs/config metadata and focused tests.
Type of Change
Changes Made
gateway/email_smtp.pyas the shared SMTP security policy/helper.tools/send_message_tool.pyto use the same policy.EMAIL_SMTP_SECURITYmetadata and.env.exampledocumentation.SMTP_SSLon port 465;starttlsandimplicit_tlsoverrides;How to Test
Targeted email/config/local-env suites:
Result:
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/A.env.exampleand env-var docs were updated.CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
N/A — this PR does not add a skill.
Screenshots / Logs
No screenshots. Targeted test output is included in How to Test.