feat: add IMAP STARTTLS support with RFC 8314 security enum#121
Open
codefuturist wants to merge 10 commits intoai-zerolab:mainfrom
Open
feat: add IMAP STARTTLS support with RFC 8314 security enum#121codefuturist wants to merge 10 commits intoai-zerolab:mainfrom
codefuturist wants to merge 10 commits intoai-zerolab:mainfrom
Conversation
Replace ambiguous use_ssl/start_ssl boolean pair with a clean ConnectionSecurity enum (tls/starttls/none) per RFC 8314. Changes: - Add ConnectionSecurity enum to config.py - Add model_validator for backward compat with use_ssl/start_ssl - Implement IMAP STARTTLS transport upgrade via asyncio.loop.start_tls() - Add _create_imap_connection() factory for TLS/STARTTLS/plaintext - Add IMAP verify_ssl support for self-signed certificates - Wire SMTP flags from security enum in EmailClient - Add MCP_EMAIL_SERVER_IMAP_SECURITY/SMTP_SECURITY env vars - Update existing tests to use _connect_imap instead of imap_class - Add comprehensive tests for new security features - Update README with security modes, env vars, and ProtonMail Bridge example Existing configs with use_ssl/start_ssl continue to work unchanged. The start_ssl field was previously ignored for IMAP — this fixes that. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive IMAP STARTTLS support by replacing the ambiguous use_ssl/start_ssl boolean configuration with a clean ConnectionSecurity enum (tls/starttls/none) aligned with RFC 8314. This addresses a critical gap where IMAP STARTTLS was not implemented, causing failures with services like ProtonMail Bridge that require STARTTLS on port 1143.
Changes:
- Introduced
ConnectionSecurityenum to replace booleanuse_ssl/start_sslfields with clearer security modes - Implemented IMAP STARTTLS protocol upgrade using
asyncio.loop.start_tls()following the same pattern as existing SMTP code - Added full backward compatibility through model validators and legacy env var support
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
mcp_email_server/config.py |
Added ConnectionSecurity enum, model validator for legacy config migration, new env vars (*_SECURITY, IMAP_VERIFY_SSL), and helper functions for security parsing |
mcp_email_server/emails/classic.py |
Implemented _imap_starttls() function, _create_imap_connection() factory, _create_imap_ssl_context(), and replaced all direct IMAP instantiation with factory pattern |
mcp_email_server/ui.py |
Replaced SSL/Start SSL checkboxes with security dropdown, added verify SSL checkbox for IMAP, updated all form handling to use enum values |
tests/test_imap_starttls.py |
Added 39 comprehensive tests covering enum behavior, backward compatibility, STARTTLS protocol flow, SSL contexts, env var parsing, and integration |
tests/test_email_client.py |
Updated tests to mock _connect_imap() instead of imap_class |
tests/test_email_attachments.py |
Updated tests to mock _connect_imap() instead of imap_class |
README.md |
Documented security modes table, new env vars, ProtonMail Bridge configuration example, and marked deprecated env vars |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add 'TOML Key' column to environment variables table mapping each variable to its corresponding TOML config path using dotted notation - Add missing env vars to table: IMAP/SMTP-specific user_name/password, CONFIG_PATH - Add 'TOML Configuration Reference' section with complete single-account example showing all supported fields - Add multi-account example (Gmail + ProtonMail Bridge) demonstrating [[emails]] array-of-tables syntax with different security modes - Add cross-reference note linking env var table to TOML reference Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Test _parse_security_env with None input, valid values, and invalid values (including warning log assertion) - Test model validator non-dict passthrough edge case - Improves patch coverage for config.py from 92.1% to ~97% Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Previously, setting only use_ssl=False (without start_ssl) would default to security='none' (plaintext), which is a security risk. Now plaintext mode requires both legacy fields to be explicitly set to False. A single use_ssl=False or start_ssl=False alone falls back to TLS (secure by default). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add event handlers on IMAP/SMTP security dropdowns that automatically set the standard port when the user changes the connection security: - TLS: IMAP 993, SMTP 465 - STARTTLS: IMAP 143, SMTP 587 - None: IMAP 143, SMTP 25 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add 'Edit Existing Account' dropdown to load/edit saved accounts - Add 'Test IMAP' and 'Test SMTP' buttons with user-friendly error messages - Add 'Use same security for SMTP' checkbox to mirror IMAP settings - Add password masking (type=password) with placeholder for edit mode - Add update_email() method to Settings model for atomic updates - Add test_imap_connection() and test_smtp_connection() helpers - Extract _resolve_passwords() to reduce save complexity (C901) - Add 10 new tests for connection test helpers and update_email - 188 tests passing, make check green Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add 7 tests for SMTP error branches (SSL, timeout, generic exception) - Add 2 tests for IMAP generic OSError and generic exception paths - Add test for invalid SMTP_SECURITY env var fallback - Add tests for EmailSettings.init() legacy SMTP SSL/StartSSL paths - Patch coverage: classic.py 96.5%, config.py 98.7% (196 total tests) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add integration test infrastructure with pure-Python (Tier 1) and Docker-based (Tier 2) test tiers for real protocol validation. Tier 1 — pytest-localserver + MockImapServer (18 tests): - IMAP tests: connection, count, body, attachment, delete, append - SMTP tests: send, CC/BCC, HTML, attachments, reply headers, unicode - Pure Python, no Docker required, runs via 'make test-integration' Tier 2 — GreenMail Docker (3 tests): - Full SMTP→IMAP roundtrip: send, read body, delete - Auto-skips if Docker unavailable, runs via 'make test-docker' Infrastructure: - pytest markers (integration, docker) with default exclusion - Dependency groups in pyproject.toml (not required by default) - Makefile targets: test-integration, test-docker, test-all - 'make test' unchanged (196 unit tests, zero new deps) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
for more information, see https://pre-commit.ci
Add TLS and STARTTLS integration tests across all tiers: - 16 new security tests (Tier 1): Implicit TLS IMAP/SMTP, STARTTLS SMTP, security mismatch detection, verify_ssl rejection of self-signed certs - 3 new TLS Docker roundtrip tests (Tier 2): SMTPS/IMAPS end-to-end - Self-signed certificate generation fixture (cryptography library) - TLS-aware SMTP/IMAP fixtures for both tiers Add defensive IMAP resource cleanup in test_imap_connection(): - _force_close_imap() helper closes transport and cancels _client_task - Prevents leaked connections when IMAP operations fail or time out - Workaround for aioimaplib#128 (tasks ignore asyncio cancellation) Blocked tests (commented out with issue references): - IMAP mismatch: plaintext-on-TLS + verify_ssl rejection (aioimaplib#128) - Docker STARTTLS roundtrip: GreenMail lacks STARTTLS support (greenmail#135) Update dependency versions: cryptography>=44.0.0 added to integration group. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
codefuturist
force-pushed
the
feat/imap-starttls
branch
from
a2aeb31 to
68ef4c9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds comprehensive IMAP STARTTLS support, RFC 8314-compliant connection security configuration, and a modernized Gradio settings UI.
Changes
Core: Connection Security
ConnectionSecurityenum (tls/starttls/none) replacing ambiguoususe_ssl/start_sslbooleansasyncio.loop.start_tls()transport upgrade (aioimaplib has no built-in support)_create_imap_connection()factory — handles TLS, STARTTLS, and plaintext uniformlyverify_sslsupport (was SMTP-only before) with shared_create_imap_ssl_context()mode="before") — explicitsecuritytakes precedence over legacyuse_ssl/start_ssl; plaintext requires both flags explicitly false (secure by default)Configuration
MCP_EMAIL_SERVER_IMAP_SECURITY/SMTP_SECURITYenv varsMCP_EMAIL_SERVER_IMAP_VERIFY_SSLenv varemails[].incoming.security/emails[].outgoing.securityGradio UI
type="password"for screen privacyDocumentation
Tests
make check+make testgreenBreaking Changes
None —
use_ssl/start_sslfields are preserved as deprecated optional aliases with full backward compatibility.