Skip to content

fix: add smtp ssl - #30248

Merged
yuneng-berri merged 4 commits into
BerriAI:litellm_yj_june13from
mubashir1osmani:litellm_smtp_ssl_support
Jun 13, 2026
Merged

fix: add smtp ssl#30248
yuneng-berri merged 4 commits into
BerriAI:litellm_yj_june13from
mubashir1osmani:litellm_smtp_ssl_support

Conversation

@mubashir1osmani

@mubashir1osmani mubashir1osmani commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Relevant issues

LIT-2665

Screenshot 2026-06-11 at 3 28 29 PM

uses SSL by default - user would only have to configure smtp as stated in these docs

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Screenshots / Proof of Fix

Type

🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test

Changes

@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a hang when users configure LiteLLM's SMTP integration against a server that requires implicit SSL (port 465). It extracts two helpers — _should_use_smtp_ssl and _create_smtp_connection — and correctly skips starttls() when SMTP_SSL is used, since calling starttls() on an already-SSL connection raises SMTPNotSupportedError.

  • _should_use_smtp_ssl returns True for port 465 or when SMTP_USE_SSL=True is set; existing STARTTLS (port 587) behaviour is unchanged.
  • The send_email function now wraps connection creation through the helper and guards starttls() with not isinstance(server, smtplib.SMTP_SSL).
  • Three unit tests are added for _should_use_smtp_ssl; _create_smtp_connection itself has no test asserting which constructor is called.

Confidence Score: 4/5

Safe to merge; the change is narrowly scoped to SMTP connection setup and does not touch the request-handling or auth paths.

The logic for choosing between SMTP_SSL and plain SMTP is correct, the isinstance guard before starttls() is necessary and present, and port-587 users see no behaviour change. The only gap is that _create_smtp_connection lacks a direct test to verify it returns the right connection type.

tests/test_litellm/proxy/test_proxy_utils.py — a test for _create_smtp_connection would improve coverage of the actual branching logic.

Important Files Changed

Filename Overview
litellm/proxy/utils.py Extracts _should_use_smtp_ssl and _create_smtp_connection helpers; skips starttls() when SMTP_SSL is used; fixes hang on port 465 implicit-SSL servers
tests/test_litellm/proxy/test_proxy_utils.py Adds three unit tests for _should_use_smtp_ssl covering port 465, explicit SMTP_USE_SSL=True, and port 587; _create_smtp_connection itself is not tested

Reviews (1): Last reviewed commit: "add smtp ssl" | Re-trigger Greptile

Comment thread tests/test_litellm/proxy/test_proxy_utils.py
@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread litellm/proxy/utils.py Outdated
@veria-ai

veria-ai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 2 · PR risk: 0/10

@yassin-berriai yassin-berriai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

veria score is 7/10. fix ci issues and get acceptable veria score

Comment thread litellm/proxy/utils.py Outdated
@yuneng-berri
yuneng-berri changed the base branch from litellm_internal_staging to litellm_yj_june13 June 13, 2026 20:51
@yuneng-berri
yuneng-berri merged commit 5a19bfc into BerriAI:litellm_yj_june13 Jun 13, 2026
73 checks passed
yuneng-berri added a commit that referenced this pull request Jun 13, 2026
* fix: add smtp ssl (#30248)

* add smtp ssl

* fix comments'

* fix(proxy): verify SMTP server certificate on starttls

* dont read ssl from env

* test(proxy): restore regression test for SMTP_TLS=False starttls skip

---------

Co-authored-by: mubashir1osmani <mubashir.osmani777@gmail.com>
michaelxer pushed a commit to michaelxer/litellm that referenced this pull request Jun 17, 2026
* fix: add smtp ssl (BerriAI#30248)

* add smtp ssl

* fix comments'

* fix(proxy): verify SMTP server certificate on starttls

* dont read ssl from env

* test(proxy): restore regression test for SMTP_TLS=False starttls skip

---------

Co-authored-by: mubashir1osmani <mubashir.osmani777@gmail.com>
michaelxer pushed a commit to michaelxer/litellm that referenced this pull request Jun 17, 2026
* fix: add smtp ssl (BerriAI#30248)

* add smtp ssl

* fix comments'

* fix(proxy): verify SMTP server certificate on starttls

* dont read ssl from env

* test(proxy): restore regression test for SMTP_TLS=False starttls skip

---------

Co-authored-by: mubashir1osmani <mubashir.osmani777@gmail.com>
koladefaj pushed a commit to koladefaj/litellm that referenced this pull request Jun 17, 2026
* fix: add smtp ssl (BerriAI#30248)

* add smtp ssl

* fix comments'

* fix(proxy): verify SMTP server certificate on starttls

* dont read ssl from env

* test(proxy): restore regression test for SMTP_TLS=False starttls skip

---------

Co-authored-by: mubashir1osmani <mubashir.osmani777@gmail.com>
factnn pushed a commit to factnn/litellm that referenced this pull request Jun 18, 2026
* fix: add smtp ssl (BerriAI#30248)

* add smtp ssl

* fix comments'

* fix(proxy): verify SMTP server certificate on starttls

* dont read ssl from env

* test(proxy): restore regression test for SMTP_TLS=False starttls skip

---------

Co-authored-by: mubashir1osmani <mubashir.osmani777@gmail.com>
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
* fix: add smtp ssl (BerriAI#30248)

* add smtp ssl

* fix comments'

* fix(proxy): verify SMTP server certificate on starttls

* dont read ssl from env

* test(proxy): restore regression test for SMTP_TLS=False starttls skip

---------

Co-authored-by: mubashir1osmani <mubashir.osmani777@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants