mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-28 07:35:27 -04:00
fix(email): don't probe IMAP for send-only (SMTP-only) accounts (#4830)
An account configured with SMTP only (no imap_host) has no inbox, but the
inbox list path still called _imap_connect, which handed an empty host to
imaplib. imaplib.IMAP4("", 993) silently dials localhost:993 and fails with
"[Errno 111] Connection refused", so the email panel's poll logged a
"Failed to list emails" ERROR every ~60s and surfaced a scary error in the UI.
_imap_connect now fails fast with a typed EmailNotConfiguredError (subclass of
RuntimeError, so existing broad handlers keep working) when no imap_host is set,
and the inbox list returns an empty result for that case instead of an error.
SMTP send is unaffected.
This commit is contained in:
@@ -40,6 +40,16 @@ from src.secret_storage import decrypt as _decrypt
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class EmailNotConfiguredError(RuntimeError):
|
||||
"""Raised when an IMAP operation is attempted on an account that has no
|
||||
inbox configured (e.g. a send-only / SMTP-only account).
|
||||
|
||||
Subclasses RuntimeError so existing broad ``except Exception`` handlers
|
||||
keep working; callers that want to treat "no inbox" as an empty result
|
||||
rather than a failure can catch this type specifically.
|
||||
"""
|
||||
|
||||
|
||||
def _xoauth2_raw(user: str, access_token: str) -> str:
|
||||
"""The SASL XOAUTH2 initial-response string (unencoded).
|
||||
|
||||
@@ -929,6 +939,14 @@ def _imap_connect(account_id: str | None = None, owner: str = "",
|
||||
# `timeout` is overridable so short-lived callers (e.g. the service-health
|
||||
# probe) can impose a tighter budget than the default IMAP timeout.
|
||||
cfg = _get_email_config(account_id, owner=owner)
|
||||
# Send-only (SMTP-only) account: no IMAP host means there is no inbox to
|
||||
# read. Bail out with a clear, typed error instead of handing an empty
|
||||
# host to imaplib — IMAP4("", 993) silently dials localhost:993 and fails
|
||||
# with a confusing "[Errno 111] Connection refused" on every inbox poll.
|
||||
if not cfg.get("imap_host"):
|
||||
raise EmailNotConfiguredError(
|
||||
f"IMAP is not configured for account {cfg.get('account_name') or 'default'!r}"
|
||||
)
|
||||
# Connection mode:
|
||||
# STARTTLS on → plain + upgrade
|
||||
# STARTTLS off + port 993 → implicit SSL (IMAPS)
|
||||
|
||||
@@ -46,6 +46,7 @@ from routes.email_helpers import (
|
||||
_send_smtp_message, _smtp_security_mode,
|
||||
_IMAP_TIMEOUT_SECONDS, _open_imap_connection,
|
||||
make_oauth_state, verify_oauth_state,
|
||||
EmailNotConfiguredError,
|
||||
_imap_connect, _imap, _decode_header, _detect_sent_folder, _detect_drafts_folder,
|
||||
_extract_attachment_text, _list_attachments_from_msg, _has_visible_attachments, _is_likely_signature_image_attachment,
|
||||
_extract_attachment_to_disk, _extract_html, _extract_text,
|
||||
@@ -1029,6 +1030,11 @@ def setup_email_routes():
|
||||
logger.debug(f"Bulk summary attach skipped: {_summary_err}")
|
||||
|
||||
return {"emails": emails, "total": total, "folder": folder, "offset": offset}
|
||||
except EmailNotConfiguredError:
|
||||
# Send-only (SMTP-only) account: there is no inbox to read, so the
|
||||
# poll returns an empty list instead of a per-minute error. SMTP
|
||||
# send is unaffected.
|
||||
return {"emails": [], "total": 0, "folder": folder, "offset": offset}
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to list emails: {e}")
|
||||
detail = str(e).strip()
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
"""A send-only (SMTP-only) account has no inbox to read.
|
||||
|
||||
`_imap_connect` must fail fast with a clear, typed error instead of handing an
|
||||
empty host to imaplib — `imaplib.IMAP4("", 993)` silently dials localhost:993
|
||||
and surfaces a confusing "[Errno 111] Connection refused" on every inbox poll.
|
||||
"""
|
||||
import os
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
_tmp_data = Path(tempfile.mkdtemp(prefix="odysseus-email-send-only-test-"))
|
||||
os.environ.setdefault("DATA_DIR", str(_tmp_data))
|
||||
os.environ.setdefault("DATABASE_URL", f"sqlite:///{_tmp_data / 'app.db'}")
|
||||
|
||||
import routes.email_helpers as helpers
|
||||
from routes.email_helpers import EmailNotConfiguredError, _imap_connect
|
||||
|
||||
|
||||
_SEND_ONLY_CFG = {
|
||||
"account_id": "acct-send-only",
|
||||
"account_name": "send-only",
|
||||
"smtp_host": "smtp.example.org",
|
||||
"smtp_port": 465,
|
||||
"smtp_user": "noreply@example.org",
|
||||
"smtp_password": "secret",
|
||||
"imap_host": "", # <- the send-only marker
|
||||
"imap_port": 993,
|
||||
"imap_user": "",
|
||||
"imap_password": "",
|
||||
"imap_starttls": True,
|
||||
"from_address": "noreply@example.org",
|
||||
}
|
||||
|
||||
|
||||
def test_not_configured_error_is_runtime_error():
|
||||
# Subclassing RuntimeError keeps existing broad `except Exception` handlers
|
||||
# working while letting the inbox poll catch this case specifically.
|
||||
assert issubclass(EmailNotConfiguredError, RuntimeError)
|
||||
|
||||
|
||||
def test_imap_connect_send_only_raises_and_never_dials(monkeypatch):
|
||||
monkeypatch.setattr(helpers, "_get_email_config", lambda *a, **k: dict(_SEND_ONLY_CFG))
|
||||
|
||||
def _boom(*a, **k): # opening a connection means we dialed an empty host
|
||||
raise AssertionError("send-only account must not open an IMAP connection")
|
||||
|
||||
monkeypatch.setattr(helpers, "_open_imap_connection", _boom)
|
||||
|
||||
with pytest.raises(EmailNotConfiguredError):
|
||||
_imap_connect("acct-send-only")
|
||||
|
||||
|
||||
def test_imap_connect_with_host_still_connects(monkeypatch):
|
||||
# Guard must not regress normal accounts: a configured imap_host still
|
||||
# reaches _open_imap_connection.
|
||||
cfg = dict(_SEND_ONLY_CFG, imap_host="imap.example.org", imap_user="u", imap_password="p")
|
||||
monkeypatch.setattr(helpers, "_get_email_config", lambda *a, **k: cfg)
|
||||
|
||||
opened = {}
|
||||
|
||||
class _FakeConn:
|
||||
def login(self, user, password):
|
||||
opened["login"] = (user, password)
|
||||
|
||||
def _fake_open(host, port, *, starttls, timeout):
|
||||
opened["host"] = host
|
||||
return _FakeConn()
|
||||
|
||||
monkeypatch.setattr(helpers, "_open_imap_connection", _fake_open)
|
||||
|
||||
conn = _imap_connect("acct-with-imap")
|
||||
assert opened["host"] == "imap.example.org"
|
||||
assert isinstance(conn, _FakeConn)
|
||||
Reference in New Issue
Block a user