ソースを参照

fix(email): add Date and Message-ID headers to outgoing mail (#3376)

- Neither sender set Date or Message-ID, so strict receivers (amavis
  bad_header) quarantined the mail after the SMTP server accepted it
- New add_required_headers() in email_service, used by send_email and
  by the email notification provider; Message-ID domain comes from
  the sender address, fixed fallback instead of a hostname lookup
- From is built with formataddr, so a non-ASCII or comma'd sender name
  no longer swallows the address
- Log says "accepted by SMTP server" instead of "sent successfully"
maziggy 17 時間 前
親
コミット
cd05936cb9

+ 1 - 0
CHANGELOG.md

@@ -192,6 +192,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
 
 ### Fixed
+- **Mail from Bambuddy is no longer quarantined for missing headers (#3376, reported by @BenjaminCs)** — Bambuddy's mail had no `Date` and no `Message-ID` header, both of which RFC 5322 requires. A strict mail server (e.g. amavis with its default header check) accepted the message and then quarantined it, so Bambuddy logged a success and nothing arrived. Elsewhere a missing `Date` counts as a spam signal. Every mail now carries both: the SMTP test, welcome, password-reset and two-factor mails, and the email notification provider. The `Message-ID` uses the sender address's domain. A sender name with accented letters or a comma also broke the `From` header so that no address was left in it; the name is now encoded on its own and the address stays readable. The log now says "Email accepted by SMTP server", since Bambuddy cannot see delivery.
 - **Bambu PLA Silk+ spools added by RFID get their colour name** — An RFID roll is named from the colour catalogue by its hex and the product line the AMS reports. A single-colour Silk+ roll reports "PLA Silk+", but the catalogue had the Silk+ colours (Silver, Titan Gray, Gold and the rest) filed under "PLA Silk", the line the AMS reports for the Dual/Tri Color silk, so every Silk+ roll arrived without a name, in both inventory modes. The seeded colours are now filed under "PLA Silk+". On an existing install the update moves the seeded Bambu Lab rows that are still as seeded (rows you edited or added stay as they are), and names the RFID-added Bambu spools that came in without a colour name; a name already set is never changed.
 - **The X2D's auxiliary fans are named the right way round (#3373, reported by @Spegeli)** — On the X2D, the printer card's **Left Auxiliary Fan** drove the fan on the right of the chamber, and **Auxiliary Fan** the one on the left. The names came from the P2S, which numbers these two fans the other way round. On an X2D the card now shows **Left Auxiliary Fan** then **Right Auxiliary Fan**, still left to right, and the confirmation after setting a speed names the same side. The P2S is unchanged. The API keeps its field and fan names (`left_aux_fan_speed`, `aux`, `aux2`): on an X2D, `aux2` and `left_aux_fan_speed` are the right fan. Wiki updated.
 - **The full-page G-code preview draws on wide screens (#3371, reported by @Underscan007)** — On a window 1024 px wide or more, with classic (non-overlay) scrollbars, **3D Preview** stayed black while the page scrollbar flickered on and off. The canvas was briefly wider than its pane after each resize and pushed the page into scrolling, which narrowed the pane again, without end. The preview is now clipped to its pane, so it settles at once and draws.

+ 24 - 3
backend/app/services/email_service.py

@@ -12,6 +12,7 @@ from datetime import datetime, timezone
 from email.mime.image import MIMEImage
 from email.mime.multipart import MIMEMultipart
 from email.mime.text import MIMEText
+from email.utils import formataddr, formatdate, make_msgid, parseaddr
 from typing import Any
 
 from sqlalchemy import select
@@ -174,6 +175,24 @@ async def save_smtp_settings(db: AsyncSession, smtp_settings: SMTPSettings) -> N
         await upsert_setting(db, Settings, key, value)
 
 
+def add_required_headers(msg: MIMEMultipart, from_email: str) -> None:
+    """Add the Date and Message-ID headers RFC 5322 requires (#3376).
+
+    Neither smtplib nor the MIME classes add them, and strict receivers
+    (amavis's bad_header policy) quarantine mail without a Date after the
+    SMTP server already accepted it. The Message-ID domain comes from the
+    sender address; without one, make_msgid() would look up the container's
+    hostname, which can block on DNS and leaks an internal name.
+    """
+    _local, at, domain = parseaddr(from_email)[1].rpartition("@")
+    if not at or not domain:
+        domain = "bambuddy.local"
+    if "Date" not in msg:
+        msg["Date"] = formatdate(localtime=True)
+    if "Message-ID" not in msg:
+        msg["Message-ID"] = make_msgid(domain=domain)
+
+
 def send_email(
     smtp_settings: SMTPSettings,
     to_email: str,
@@ -203,7 +222,7 @@ def send_email(
         # multipart/related wraps multipart/alternative so the inline image
         # travels with the HTML part without becoming a visible attachment.
         msg = MIMEMultipart("related")
-        msg["From"] = f"{smtp_settings.smtp_from_name} <{smtp_settings.smtp_from_email}>"
+        msg["From"] = formataddr((smtp_settings.smtp_from_name, smtp_settings.smtp_from_email))
         msg["To"] = to_email
         msg["Subject"] = subject
 
@@ -218,7 +237,7 @@ def send_email(
         msg.attach(img)
     else:
         msg = MIMEMultipart("alternative")
-        msg["From"] = f"{smtp_settings.smtp_from_name} <{smtp_settings.smtp_from_email}>"
+        msg["From"] = formataddr((smtp_settings.smtp_from_name, smtp_settings.smtp_from_email))
         msg["To"] = to_email
         msg["Subject"] = subject
 
@@ -229,6 +248,8 @@ def send_email(
         if body_html:
             msg.attach(MIMEText(body_html, "html"))
 
+    add_required_headers(msg, smtp_settings.smtp_from_email)
+
     # Send email
     try:
         security = smtp_settings.smtp_security
@@ -258,7 +279,7 @@ def send_email(
                 if auth_enabled and smtp_settings.smtp_password and smtp_settings.smtp_username:
                     server.login(smtp_settings.smtp_username, smtp_settings.smtp_password)
                 server.send_message(msg)
-        logger.info(f"Email sent successfully to {to_email}")
+        logger.info(f"Email accepted by SMTP server for {to_email}")
     except Exception as e:
         logger.error(f"Failed to send email to {to_email}: {e}")
         raise

+ 3 - 0
backend/app/services/notification_service.py

@@ -27,6 +27,7 @@ from backend.app.models.notification import (
     TelegramPendingVerdict,
 )
 from backend.app.models.notification_template import NotificationTemplate
+from backend.app.services.email_service import add_required_headers
 from backend.app.services.notify_client import NotifyClient, NotifyError, notify_credentials, notify_supports_photos
 from backend.app.services.print_confirmation import one_tap_url
 from backend.app.utils.notification_photos import save_notification_photo
@@ -1021,6 +1022,8 @@ class NotificationService:
                 msg["Subject"] = f"[Bambuddy] {subject}"
                 msg.attach(MIMEText(body, "plain"))
 
+            add_required_headers(msg, from_email)
+
             # smtplib is synchronous and blocking: a wedged / greylisting /
             # firewall-dropped relay leaves recv() stuck. Two problems, two
             # fixes (#2572):

+ 143 - 0
backend/tests/unit/services/test_email_required_headers.py

@@ -0,0 +1,143 @@
+"""Outgoing mail carries Date and Message-ID, and a readable From (#3376).
+
+Neither smtplib nor the MIME classes add Date or Message-ID, so strict
+receivers (amavis's bad_header policy) quarantined every Bambuddy mail after
+the SMTP server had accepted it. Both senders are covered: send_email (SMTP
+test, welcome, password reset, MFA) and the notification provider.
+"""
+
+from email import message_from_string, policy
+from email.utils import parsedate_to_datetime
+from unittest.mock import patch
+
+import pytest
+
+from backend.app.schemas.auth import SMTPSettings
+from backend.app.services.email_service import add_required_headers, send_email
+from backend.app.services.notification_service import NotificationService
+
+JPEG = b"\xff\xd8\xff\xe0jpeg"
+PHOTO_URL = "https://printer.local/api/v1/archives/42/photos/finish.jpg"
+
+
+def _assert_required_headers(msg, domain: str) -> None:
+    assert parsedate_to_datetime(msg["Date"]).tzinfo is not None
+    message_id = msg["Message-ID"]
+    assert message_id.startswith("<")
+    assert message_id.endswith(f"@{domain}>")
+
+
+def _fake_smtp(captured: list):
+    class FakeSMTP:
+        def __init__(self, host, port, timeout=None):
+            pass
+
+        def __enter__(self):
+            return self
+
+        def __exit__(self, *exc):
+            return False
+
+        def starttls(self):
+            pass
+
+        def login(self, u, p):
+            pass
+
+        def send_message(self, msg):
+            # As sent: serialise and re-parse, so header encoding counts.
+            captured.append(message_from_string(msg.as_string()))
+
+        def sendmail(self, frm, to, body):
+            captured.append(message_from_string(body))
+
+        def quit(self):
+            pass
+
+    return FakeSMTP
+
+
+def _smtp_settings(**kw) -> SMTPSettings:
+    base = {
+        "smtp_host": "smtp.example.com",
+        "smtp_port": 587,
+        "smtp_security": "starttls",
+        "smtp_auth_enabled": False,
+        "smtp_from_email": "farm@mail.example.org",
+        "smtp_from_name": "Bambuddy",
+    }
+    base.update(kw)
+    return SMTPSettings(**base)
+
+
+@pytest.mark.parametrize("image", [None, JPEG], ids=["alternative", "related"])
+def test_send_email_sets_date_and_message_id(image):
+    captured: list = []
+    with patch("backend.app.services.email_service.smtplib.SMTP", _fake_smtp(captured)):
+        send_email(_smtp_settings(), "to@example.com", "Test", "body", "<p>body</p>", image_data=image)
+
+    (msg,) = captured
+    _assert_required_headers(msg, "mail.example.org")
+
+
+@pytest.mark.parametrize("image", [None, JPEG], ids=["alternative", "related"])
+def test_send_email_from_keeps_the_address_readable(image):
+    """A non-ASCII or comma'd sender name is encoded on its own; the address
+    stays plain, so receivers still find a valid From."""
+    captured: list = []
+    settings = _smtp_settings(smtp_from_name="Bämbuddy Farm, Inc")
+    with patch("backend.app.services.email_service.smtplib.SMTP", _fake_smtp(captured)):
+        send_email(settings, "to@example.com", "Test", "body", "<p>body</p>", image_data=image)
+
+    (msg,) = captured
+    assert "<farm@mail.example.org>" in msg["From"]
+    # Decoded the way a mail client does.
+    (sender,) = message_from_string(msg.as_string(), policy=policy.default)["From"].addresses
+    assert (sender.display_name, sender.addr_spec) == ("Bämbuddy Farm, Inc", "farm@mail.example.org")
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("inline_photo", [False, True], ids=["text", "related"])
+async def test_notification_email_sets_date_and_message_id(inline_photo):
+    config = {
+        "smtp_server": "smtp.example.com",
+        "smtp_port": "587",
+        "from_email": "printer@farm.example.net",
+        "to_email": "alice@example.com",
+        "security": "starttls",
+        "auth_enabled": "false",
+    }
+    body = f"Done.\n\nSnapshot: {PHOTO_URL}" if inline_photo else "Done."
+    captured: list = []
+    with patch("backend.app.services.notification_service.smtplib.SMTP", _fake_smtp(captured)):
+        ok, _ = await NotificationService()._send_email(
+            config, "Print Finished", body, image_data=JPEG, finish_photo_url=PHOTO_URL
+        )
+
+    assert ok is True
+    (msg,) = captured
+    assert msg.get_content_type() == ("multipart/related" if inline_photo else "multipart/mixed")
+    _assert_required_headers(msg, "farm.example.net")
+
+
+@pytest.mark.parametrize("from_email", ["", "no-at-sign", "user@"])
+def test_message_id_without_a_sender_domain_uses_a_fixed_one(from_email):
+    """Never make_msgid()'s hostname lookup: it can block on DNS and puts
+    the container's name in the header."""
+    from email.mime.multipart import MIMEMultipart
+
+    msg = MIMEMultipart()
+    with patch("socket.getfqdn", side_effect=AssertionError("hostname lookup")):
+        add_required_headers(msg, from_email)
+    _assert_required_headers(msg, "bambuddy.local")
+
+
+def test_existing_headers_are_kept():
+    from email.mime.multipart import MIMEMultipart
+
+    msg = MIMEMultipart()
+    msg["Date"] = "Sat, 24 Oct 2026 19:40:19 +0000"
+    msg["Message-ID"] = "<fixed@example.com>"
+    add_required_headers(msg, "a@b.example")
+    assert msg.get_all("Date") == ["Sat, 24 Oct 2026 19:40:19 +0000"]
+    assert msg.get_all("Message-ID") == ["<fixed@example.com>"]