Skip to content

fix(email): use implicit TLS for SMTPS port 465 - #27680

Closed
timothykersten wants to merge 1 commit into
NousResearch:mainfrom
timothykersten:fix/smtps-465-implicit-tls
Closed

fix(email): use implicit TLS for SMTPS port 465#27680
timothykersten wants to merge 1 commit into
NousResearch:mainfrom
timothykersten:fix/smtps-465-implicit-tls

Conversation

@timothykersten

@timothykersten timothykersten commented May 18, 2026

Copy link
Copy Markdown

Summary

  • Use smtplib.SMTP_SSL for SMTP port 465, which requires implicit TLS.
  • Keep STARTTLS behavior for port 587 and other non-465 SMTP ports.
  • Apply the same behavior to the gateway email adapter and standalone send_message email path.
  • Add regression tests for SMTPS 465 behavior.

Test Plan

  • python -m pytest tests/gateway/test_email.py -q

Result: 62 passed in 2.79s.

Related upstream tracking

Fixes #19152.

Related / competing prior PRs for the same SMTP 465 area: #13564 and #12161. Original bug report: #11842.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/email Email (IMAP/SMTP) adapter labels May 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related: fixes #19152 (feature request for implicit TLS on port 465). Competing PRs for the same fix: #13564 and #12161 — those should likely be closed if this one is merged. Original bug report: #11842 (closed).

@teknium1

teknium1 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. I verified the premise against current main and did not find a blocking problem in the diff.

Current origin/main still forces smtplib.SMTP(...); starttls(...) for every SMTP port in the gateway connection check (gateway/platforms/email.py:319-320), gateway send path (gateway/platforms/email.py:558-560), multi/single attachment paths (gateway/platforms/email.py:680-682, gateway/platforms/email.py:759-761), and standalone send_message email path (tools/send_message_tool.py:1427-1428). That matches #19152’s reported implicit-TLS-on-465 failure.

The PR’s diff applies the same smtp_port == 465smtplib.SMTP_SSL(..., context=...) behavior across those sibling paths while preserving STARTTLS for non-465 ports, and adds regression coverage for the gateway connect path and standalone _send_email path. This also fits the project rubric in AGENTS.md: it fixes a real bug by extending existing code rather than adding new core surface.

No blocking change requests from this automated hermes-sweeper review; human maintainers may still choose how to salvage/merge.

@swissly

swissly commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Hi! Heads up — I opened #47167 which partially overlaps with this PR.

Overlap: tools/send_message_tool.py SMTP_SSL fix (same change, I also added int() normalization per Copilot review feedback).

Unique to #47167: cron/scheduler.py fix — the delivery loop crashes when the first email fails, preventing subsequent targets from receiving messages (fixes #47163).

Happy to:

  1. Remove the send_message_tool.py change from fix(cron): prevent delivery loop crash on multi-target email delivery #47167 and rebase on top of this PR once merged
  2. Or you can incorporate the scheduler fix here if you prefer

Either way, the scheduler fix and the SMTP fix are separate concerns and should probably land independently. Let me know what works best!

@timothykersten

Copy link
Copy Markdown
Author

Thanks for the heads up. I agree these are separate concerns.

My preference: keep this PR focused on the SMTP/SMTPS port 465 fix, and keep the scheduler delivery-loop fix in #47167 (or a dedicated PR). If you can remove the overlapping tools/send_message_tool.py change from #47167 and rebase after this lands, that would avoid duplicating the SMTP fix while preserving the scheduler fix independently.

@timothykersten
timothykersten force-pushed the fix/smtps-465-implicit-tls branch from 82218d5 to 3f04c7a Compare June 18, 2026 00:28
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.
Fixes #19152 (port 465 needs implicit TLS / SMTP_SSL, not STARTTLS). Original bug #11842 is closed. Open sibling #47269 touches the same send_message_tool.py for a different concern (IPv4 fallback) — related, not a duplicate. Prior closed attempts: #31811, #13564, #12161. Flagging the cluster for maintainer consolidation.

@teknium1

teknium1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for this — closing as already implemented on main. The standalone _send_email path this PR targeted was refactored into plugins/platforms/email/adapter.py::_standalone_send (#41112), and its _connect_smtp already selects SMTP_SSL for port 465 (implicit TLS) with a STARTTLS path for other ports, plus an IPv4-only fallback for broken-IPv6 networks. So the port-465 behavior you're fixing is already in place. Appreciate the report.

@teknium1 teknium1 closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/email Email (IMAP/SMTP) adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: mail gateway does not support implicit TLS SMTP on port 465

4 participants