Merge pull request 'Sanitize folded email headers before SMTP send' (#3) from fix/sanitize-folded-headers into main
This commit is contained in:
commit
07028c6dc6
@ -8,17 +8,21 @@ hearth.yml.
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import re
|
||||
import smtplib
|
||||
import uuid
|
||||
from dataclasses import dataclass
|
||||
from email.message import EmailMessage
|
||||
from email.utils import make_msgid
|
||||
|
||||
from hearth.config import Config, ConfigError, Participant
|
||||
from hearth.config import Config, Participant
|
||||
from hearth.settings import settings
|
||||
|
||||
log = logging.getLogger("hearth.mailer")
|
||||
|
||||
# Real IMAP headers often arrive folded (CRLF + whitespace). EmailMessage
|
||||
# rejects raw newlines when assigning header values, so unfold first.
|
||||
_HEADER_WS = re.compile(r"[\r\n\t ]+")
|
||||
|
||||
|
||||
class SendBlocked(RuntimeError):
|
||||
pass
|
||||
@ -30,6 +34,23 @@ class SendResult:
|
||||
dry_run: bool
|
||||
|
||||
|
||||
def sanitize_header(value: str | None) -> str:
|
||||
"""Collapse folded whitespace so headers are safe for EmailMessage."""
|
||||
if not value:
|
||||
return ""
|
||||
return _HEADER_WS.sub(" ", value).strip()
|
||||
|
||||
|
||||
def _reply_references(prior: str | None, in_reply_to: str | None) -> str:
|
||||
"""Build a single-line References chain ending with the parent Message-ID."""
|
||||
refs = sanitize_header(prior)
|
||||
parent = sanitize_header(in_reply_to)
|
||||
if parent and parent not in refs:
|
||||
refs = f"{refs} {parent}".strip() if refs else parent
|
||||
return refs
|
||||
|
||||
|
||||
|
||||
def _connect(sender: Participant) -> smtplib.SMTP:
|
||||
if sender.smtp_port == 465:
|
||||
smtp = smtplib.SMTP_SSL(sender.smtp_host, sender.smtp_port, timeout=20)
|
||||
@ -62,6 +83,9 @@ def send(
|
||||
raise SendBlocked(f"recipient email for '{recipient_key}' does not match allow-list")
|
||||
|
||||
message_id = make_msgid(domain=sender.email.split("@", 1)[-1])
|
||||
subject = sanitize_header(subject) or "(no subject)"
|
||||
parent_id = sanitize_header(in_reply_to) or None
|
||||
refs = _reply_references(references, parent_id) or None
|
||||
|
||||
if settings.dry_run or not settings.enabled:
|
||||
log.info(
|
||||
@ -78,10 +102,10 @@ def send(
|
||||
msg["To"] = recipient.email
|
||||
msg["Subject"] = subject
|
||||
msg["Message-ID"] = message_id
|
||||
if in_reply_to:
|
||||
msg["In-Reply-To"] = in_reply_to
|
||||
if references:
|
||||
msg["References"] = references
|
||||
if parent_id:
|
||||
msg["In-Reply-To"] = parent_id
|
||||
if refs:
|
||||
msg["References"] = refs
|
||||
msg.set_content(body)
|
||||
|
||||
smtp = _connect(sender)
|
||||
|
||||
@ -64,15 +64,15 @@ def _handle_message(config: Config, store: Store, mailbox_key: str, raw: bytes,
|
||||
if sender_key == mailbox_key:
|
||||
return # our own mailbox talking to itself, ignore
|
||||
|
||||
message_id = _decode(msg.get("Message-ID")).strip()
|
||||
message_id = mailer.sanitize_header(_decode(msg.get("Message-ID")))
|
||||
if not message_id:
|
||||
return
|
||||
if store.already_seen(message_id) or store.is_reply_queued(message_id):
|
||||
return
|
||||
|
||||
in_reply_to = _decode(msg.get("In-Reply-To")).strip() or None
|
||||
subject = _decode(msg.get("Subject")).strip() or "(no subject)"
|
||||
references = _decode(msg.get("References")).strip() or in_reply_to
|
||||
in_reply_to = mailer.sanitize_header(_decode(msg.get("In-Reply-To"))) or None
|
||||
subject = mailer.sanitize_header(_decode(msg.get("Subject"))) or "(no subject)"
|
||||
references = mailer.sanitize_header(_decode(msg.get("References"))) or in_reply_to
|
||||
|
||||
thread_id = store.thread_id_for(in_reply_to)
|
||||
store.record_message(
|
||||
|
||||
@ -85,3 +85,54 @@ def test_is_known_email_is_exact_match_only(config):
|
||||
# A lookalike/subdomain address must never resolve to a real participant.
|
||||
assert config.is_known_email("alerts@example.com.evil.com") is None
|
||||
assert config.is_known_email("not-friend@example.com") is None
|
||||
|
||||
|
||||
def test_sanitize_header_unfolds_folded_references():
|
||||
folded = "<a@x>\r\n <b@y>\n\t<c@z>"
|
||||
assert mailer.sanitize_header(folded) == "<a@x> <b@y> <c@z>"
|
||||
assert "\n" not in mailer.sanitize_header(folded)
|
||||
assert mailer.sanitize_header(None) == ""
|
||||
assert mailer.sanitize_header(" hi ") == "hi"
|
||||
|
||||
|
||||
def test_reply_references_appends_parent_once():
|
||||
prior = "<root@x>\r\n <mid@x>"
|
||||
parent = "<leaf@x>"
|
||||
assert mailer._reply_references(prior, parent) == "<root@x> <mid@x> <leaf@x>"
|
||||
assert mailer._reply_references(parent, parent) == "<leaf@x>"
|
||||
assert mailer._reply_references(None, parent) == "<leaf@x>"
|
||||
assert mailer._reply_references(None, None) == ""
|
||||
|
||||
|
||||
def test_send_accepts_folded_references_without_raising(config, monkeypatch):
|
||||
"""Regression: EmailMessage rejects CR/LF in header values."""
|
||||
settings.dry_run = False
|
||||
settings.enabled = True
|
||||
|
||||
captured = {}
|
||||
|
||||
class FakeSMTP:
|
||||
def send_message(self, msg):
|
||||
captured["msg"] = msg
|
||||
|
||||
def quit(self):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(mailer, "_connect", lambda sender: FakeSMTP())
|
||||
|
||||
folded = "<root@example.com>\r\n <parent@example.com>"
|
||||
result = mailer.send(
|
||||
config,
|
||||
sender_key="alerts",
|
||||
recipient_key="friend",
|
||||
subject="Re: hello\n there",
|
||||
body="hi",
|
||||
in_reply_to="<parent@example.com>",
|
||||
references=folded,
|
||||
)
|
||||
assert result.dry_run is False
|
||||
assert "\n" not in captured["msg"]["Subject"]
|
||||
assert "\n" not in captured["msg"]["References"]
|
||||
assert "\r" not in captured["msg"]["References"]
|
||||
assert captured["msg"]["In-Reply-To"] == "<parent@example.com>"
|
||||
assert captured["msg"]["References"] == "<root@example.com> <parent@example.com>"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user