mesh-mail: classify an SMTP send's outcome (not taken, refused, not proven), and carry the send id in the Message-ID (ADR 0436 clause 10; issue 1197)

2026-10-19 10:03:44 +02:00
parent 1ccb617a86
commit e7dce9616c
3 changed files with 358 additions and 15 deletions
@@ -684,7 +684,7 @@ func TestMailSendRefusesAControlCharacterInTheSubjectOrAnAddress(t *testing.T) {
 func TestSendUsesTheParsedAddressForRcptAndTheFormattedAddressForTheToHeader(t *testing.T) {
 	fake := newFakeSMTPServer(t)
 	acc := Account{Name: "mesh", Address: "jschoubben@novox.be", Outgoing: &Server{Host: "127.0.0.1", Port: 25, Security: "tls"}}
-	if err := SendMail(fake.dialer(), acc, "the-password", []string{"Ops Team <ops@example.org>"}, "Status", "body", judgedAt); err != nil {
+	if err := SendMail(fake.dialer(), acc, "the-password", []string{"Ops Team <ops@example.org>"}, "Status", "body", judgedAt, "send-abc123"); err != nil {
 		t.Fatalf("SendMail: %v", err)
 	}

@@ -8,10 +8,12 @@ package main

 import (
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"mime"
 	"net/mail"
 	"net/smtp"
+	"net/textproto"
 	"strings"
 	"time"
 	"unicode"
@@ -55,10 +57,63 @@ func DialSMTP(server Server) (*smtp.Client, error) {
 	}
 }

-// SendMail sends one message through the account's outgoing server (novox/hq ADR 0361 §1, §2): signs in with the
-// password given, states every recipient, and writes the message once. Never logs the password or the body; an
-// error names the server and what it said, never either.
-func SendMail(dial SMTPDialer, a Account, password string, to []string, subject, body string, now time.Time) error {
+// SendOutcome classifies how a send attempt ended (novox/hq ADR 0436 clause 10), so the sender knows
+// whether a retry is safe, certainly wrong, or neither:
+//
+//   - NotTaken: no connection, the sign-in refused, or a 4xx reply to MAIL, RCPT or DATA, or to the end of
+//     data — the server provably never took the mail; retrying is safe.
+//   - Refused: a 5xx reply before or at the end of data — the server provably refused the mail; never
+//     retried.
+//   - NotProven: the connection was lost after the data was written, with no reply at all — the mail may or
+//     may not have gone out; never retried automatically either way.
+type SendOutcome int
+
+const (
+	NotTaken SendOutcome = iota
+	Refused
+	NotProven
+)
+
+func (o SendOutcome) String() string {
+	switch o {
+	case Refused:
+		return "refused"
+	case NotProven:
+		return "not proven"
+	default:
+		return "not taken"
+	}
+}
+
+// SendError classifies a SendMail failure (novox/hq ADR 0436 clause 10): Outcome says which, and the error
+// itself (Unwrap) carries the server's own words where there were any, never the password or the body.
+type SendError struct {
+	Outcome SendOutcome
+	Err     error
+}
+
+func (e *SendError) Error() string { return e.Err.Error() }
+func (e *SendError) Unwrap() error { return e.Err }
+
+// classifyReply is NotTaken for a 4xx reply, Refused for a 5xx, and NotTaken for anything else (no
+// connection, or the connection lost before a reply was read at all) — the safe default everywhere but the
+// one case clause 10 names its own outcome for: the connection lost after the data was written, which the
+// caller classifies as NotProven itself, never through this.
+func classifyReply(err error) SendOutcome {
+	var proto *textproto.Error
+	if errors.As(err, &proto) && proto.Code >= 500 {
+		return Refused
+	}
+	return NotTaken
+}
+
+// SendMail sends one message through the account's outgoing server (novox/hq ADR 0361 §1, §2; ADR 0436
+// clause 10): signs in with the password given, states every recipient, and writes the message once, its
+// Message-ID naming sendID so the mail is never sent twice within the day (clause 10's own guarantee, kept
+// by the sender's compare-and-set claim and the bus's duplicate window on the id alone — this is the
+// mail's own proof of it). Never logs the password or the body; an error names the server and what it
+// said, never either, and is always a *SendError (classifyReply, or NotProven below).
+func SendMail(dial SMTPDialer, a Account, password string, to []string, subject, body string, now time.Time, sendID string) error {
 	if a.Outgoing == nil {
 		return fmt.Errorf("the account %s has no outgoing server", a.Name)
 	}
@@ -76,44 +131,63 @@ func SendMail(dial SMTPDialer, a Account, password string, to []string, subject,
 	}
 	client, err := dial(*a.Outgoing)
 	if err != nil {
-		return err
+		return &SendError{Outcome: NotTaken, Err: err}
 	}
 	defer client.Close()
 	from := a.SignInAs()
 	auth := smtp.PlainAuth("", from, password, a.Outgoing.Host)
 	if err := client.Auth(auth); err != nil {
-		return fmt.Errorf("signing in to %s failed: %w", a.Outgoing.Address(), err)
+		// A refused sign-in is its own case, never classified by a reply code: the server never took the
+		// mail either way (novox/hq ADR 0436 clause 10).
+		return &SendError{Outcome: NotTaken, Err: fmt.Errorf("signing in to %s failed: %w", a.Outgoing.Address(), err)}
 	}
 	if err := client.Mail(from); err != nil {
-		return fmt.Errorf("the outgoing server refused the sender: %w", err)
+		return &SendError{Outcome: classifyReply(err), Err: fmt.Errorf("the outgoing server refused the sender: %w", err)}
 	}
 	for _, rcpt := range rcpts {
 		if err := client.Rcpt(rcpt); err != nil {
-			return fmt.Errorf("the outgoing server refused the recipient %s: %w", rcpt, err)
+			return &SendError{Outcome: classifyReply(err), Err: fmt.Errorf("the outgoing server refused the recipient %s: %w", rcpt, err)}
 		}
 	}
 	w, err := client.Data()
 	if err != nil {
-		return fmt.Errorf("the outgoing server refused the message: %w", err)
+		return &SendError{Outcome: classifyReply(err), Err: fmt.Errorf("the outgoing server refused the message: %w", err)}
 	}
-	if _, err := w.Write(composeMessage(from, headers, subject, body, now)); err != nil {
-		return fmt.Errorf("the message could not be written: %w", err)
+	if _, err := w.Write(composeMessage(from, headers, subject, body, now, sendID)); err != nil {
+		// The data was not written whole: never "after the data was written" (clause 10's not-proven case),
+		// so this is not taken, not not-proven — nothing coherent reached the server.
+		return &SendError{Outcome: NotTaken, Err: fmt.Errorf("the message could not be written: %w", err)}
 	}
 	if err := w.Close(); err != nil {
-		return fmt.Errorf("the outgoing server did not accept the message: %w", err)
+		// Close is what sends the terminating "." and reads the server's one reply to the whole message
+		// (SMTP's "end of data", clause 10): a real reply here classifies exactly as MAIL or RCPT's does; no
+		// reply at all (the connection lost after the data was written) is not proven — never retried
+		// automatically either way.
+		outcome := NotProven
+		var proto *textproto.Error
+		if errors.As(err, &proto) {
+			outcome = classifyReply(err)
+		}
+		return &SendError{Outcome: outcome, Err: fmt.Errorf("the outgoing server did not accept the message: %w", err)}
 	}
-	return client.Quit()
+	// The message is already accepted (Close's 250 reply); Quit is a courtesy goodbye, and its own failure
+	// here never turns an accepted send into a failed one.
+	_ = client.Quit()
+	return nil
 }

 // composeMessage is a plain-text message, its lines CRLF-terminated as SMTP takes them. to is the recipients
 // already formatted (mail.Address.String()), the same ones Rcpt was given, never the raw strings a caller gave.
-func composeMessage(from string, to []string, subject, body string, now time.Time) []byte {
+// Its Message-ID names sendID (novox/hq ADR 0436 clause 10), so the mail carries the proof of which send it
+// is, at <sendID>@<the account's own domain, from the From address>.
+func composeMessage(from string, to []string, subject, body string, now time.Time, sendID string) []byte {
 	var b strings.Builder
 	header := func(k, v string) { b.WriteString(k + ": " + v + "\r\n") }
 	header("From", from)
 	header("To", strings.Join(to, ", "))
 	header("Subject", encodeHeader(subject))
 	header("Date", now.Format(time.RFC1123Z))
+	header("Message-Id", "<"+sendID+"@"+messageIDDomain(from)+">")
 	header("MIME-Version", "1.0")
 	header("Content-Type", "text/plain; charset=utf-8")
 	header("Content-Transfer-Encoding", "8bit")
@@ -122,6 +196,20 @@ func composeMessage(from string, to []string, subject, body string, now time.Tim
 	return []byte(b.String())
 }

+// messageIDDomain is the domain a Message-ID is minted under: the part of from after its @, as a bare
+// address or parsed out of "Name <addr>"; "localhost" when from carries none (never empty, which is not a
+// valid Message-ID).
+func messageIDDomain(from string) string {
+	addr := from
+	if parsed, err := mail.ParseAddress(from); err == nil {
+		addr = parsed.Address
+	}
+	if _, domain, found := strings.Cut(addr, "@"); found && domain != "" {
+		return domain
+	}
+	return "localhost"
+}
+
 // encodeHeader word-encodes a header's value when it holds more than ASCII; a header a phone never needs to show.
 func encodeHeader(s string) string {
 	for _, r := range s {
@@ -0,0 +1,255 @@
+package main
+
+// SendMail classifies every failure (novox/hq ADR 0436 clause 10, issue 1197): not taken (no connection, a
+// refused sign-in, or a 4xx reply to MAIL, RCPT or DATA or to the end of data), refused (a 5xx before or at
+// the end of data), or not proven (the connection lost after the data was written, before any reply). Its
+// Message-ID names the send id.
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"net"
+	"net/smtp"
+	"strings"
+	"testing"
+)
+
+// step is one scripted SMTP reply, or a drop (the connection closed instead of a reply).
+type step struct {
+	drop bool
+	code int
+	msg  string
+}
+
+func reply(code int, msg string) step { return step{code: code, msg: msg} }
+func drop() step                      { return step{drop: true} }
+
+// scriptedSMTPServer answers EHLO, AUTH, MAIL, RCPT, DATA's own 354 and the end-of-data reply each with a
+// step a test sets, defaulting to the ordinary success sequence — so a test overrides exactly the one stage
+// its case is about.
+type scriptedSMTPServer struct {
+	ln                                        net.Listener
+	ehlo, auth, mail, rcpt, dataOpen, dataEnd step
+}
+
+func newScriptedSMTPServer(t *testing.T) *scriptedSMTPServer {
+	t.Helper()
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	s := &scriptedSMTPServer{ln: ln, ehlo: reply(250, "fake.local"), auth: reply(235, "authenticated"),
+		mail: reply(250, "OK"), rcpt: reply(250, "OK"), dataOpen: reply(354, "go ahead"), dataEnd: reply(250, "queued")}
+	go s.serve()
+	t.Cleanup(func() { _ = ln.Close() })
+	return s
+}
+
+func (s *scriptedSMTPServer) addr() string { return s.ln.Addr().String() }
+
+func (s *scriptedSMTPServer) dialer() SMTPDialer {
+	addr := s.addr()
+	return func(Server) (*smtp.Client, error) { return smtp.Dial(addr) }
+}
+
+func (s *scriptedSMTPServer) send(conn net.Conn, st step) bool {
+	if st.drop {
+		_ = conn.Close()
+		return false
+	}
+	fmt.Fprintf(conn, "%d %s\r\n", st.code, st.msg)
+	return true
+}
+
+func (s *scriptedSMTPServer) serve() {
+	for {
+		conn, err := s.ln.Accept()
+		if err != nil {
+			return
+		}
+		go s.handle(conn)
+	}
+}
+
+func (s *scriptedSMTPServer) handle(conn net.Conn) {
+	defer conn.Close()
+	r := bufio.NewReader(conn)
+	fmt.Fprintf(conn, "220 fake.local ESMTP\r\n")
+	for {
+		line, err := r.ReadString('\n')
+		if err != nil {
+			return
+		}
+		upper := strings.ToUpper(strings.TrimRight(line, "\r\n"))
+		switch {
+		case strings.HasPrefix(upper, "EHLO"), strings.HasPrefix(upper, "HELO"):
+			if !s.send(conn, s.ehlo) {
+				return
+			}
+		case strings.HasPrefix(upper, "AUTH"):
+			if !s.send(conn, s.auth) {
+				return
+			}
+		case strings.HasPrefix(upper, "MAIL"):
+			if !s.send(conn, s.mail) {
+				return
+			}
+		case strings.HasPrefix(upper, "RCPT"):
+			if !s.send(conn, s.rcpt) {
+				return
+			}
+		case upper == "DATA":
+			if !s.send(conn, s.dataOpen) {
+				return
+			}
+			if s.dataOpen.code >= 400 {
+				continue // not entering data mode; the client ends the transaction itself
+			}
+			for {
+				dl, err := r.ReadString('\n')
+				if err != nil {
+					return
+				}
+				if dl == ".\r\n" || dl == ".\n" {
+					break
+				}
+			}
+			s.send(conn, s.dataEnd) // a drop here is "the connection lost after the data was written"
+			return
+		case upper == "QUIT":
+			fmt.Fprintf(conn, "221 bye\r\n")
+			return
+		default:
+			fmt.Fprintf(conn, "500 unrecognized\r\n")
+		}
+	}
+}
+
+// classifyTestAccount is an account whose outgoing server is the given address: Host is its loopback
+// literal, which PlainAuth itself accepts without TLS (its own isLocalhost check).
+func classifyTestAccount(addr string) Account {
+	host, portStr, _ := net.SplitHostPort(addr)
+	var port int
+	_, _ = fmt.Sscanf(portStr, "%d", &port)
+	return Account{Name: "mesh", Address: "jschoubben@novox.be", Outgoing: &Server{Host: host, Port: port, Security: "tls"}}
+}
+
+func outcomeOf(t *testing.T, err error) SendOutcome {
+	t.Helper()
+	var se *SendError
+	if !errors.As(err, &se) {
+		t.Fatalf("SendMail's error is not a *SendError: %v (%T)", err, err)
+	}
+	return se.Outcome
+}
+
+// 1197/AC1: given no connection, a refused sign-in, or a 4xx reply to MAIL, RCPT or DATA or to the end of
+// data, when SendMail runs, then it says not taken with the server's words.
+func TestSendMailSaysNotTaken(t *testing.T) {
+	t.Run("no connection", func(t *testing.T) {
+		s := newScriptedSMTPServer(t)
+		addr := s.addr()
+		s.ln.Close() // nothing listens by the time SendMail dials
+		acc := classifyTestAccount(addr)
+		dial := func(Server) (*smtp.Client, error) { return smtp.Dial(addr) }
+		err := SendMail(dial, acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1")
+		if err == nil || outcomeOf(t, err) != NotTaken {
+			t.Fatalf("err %v, want NotTaken", err)
+		}
+	})
+	t.Run("a refused sign-in", func(t *testing.T) {
+		s := newScriptedSMTPServer(t)
+		s.auth = reply(535, "authentication failed")
+		acc := classifyTestAccount(s.addr())
+		err := SendMail(s.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1")
+		if err == nil || outcomeOf(t, err) != NotTaken || !strings.Contains(err.Error(), "signing in") {
+			t.Fatalf("err %v, want NotTaken naming the sign-in", err)
+		}
+	})
+	for _, c := range []struct {
+		name string
+		set  func(*scriptedSMTPServer)
+	}{
+		{"a 4xx to MAIL", func(s *scriptedSMTPServer) { s.mail = reply(451, "try again later") }},
+		{"a 4xx to RCPT", func(s *scriptedSMTPServer) { s.rcpt = reply(450, "mailbox busy") }},
+		{"a 4xx to DATA", func(s *scriptedSMTPServer) { s.dataOpen = reply(452, "insufficient storage") }},
+		{"a 4xx at the end of data", func(s *scriptedSMTPServer) { s.dataEnd = reply(451, "local error") }},
+	} {
+		t.Run(c.name, func(t *testing.T) {
+			s := newScriptedSMTPServer(t)
+			c.set(s)
+			acc := classifyTestAccount(s.addr())
+			err := SendMail(s.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1")
+			if err == nil || outcomeOf(t, err) != NotTaken {
+				t.Fatalf("err %v, want NotTaken", err)
+			}
+		})
+	}
+}
+
+// 1197/AC2: given a 5xx reply before or at the end of data, then it says refused with the server's words.
+func TestSendMailSaysRefused(t *testing.T) {
+	for _, c := range []struct {
+		name string
+		set  func(*scriptedSMTPServer)
+	}{
+		{"a 5xx to MAIL", func(s *scriptedSMTPServer) { s.mail = reply(550, "no such sender") }},
+		{"a 5xx to RCPT", func(s *scriptedSMTPServer) { s.rcpt = reply(550, "no such recipient") }},
+		{"a 5xx to DATA", func(s *scriptedSMTPServer) { s.dataOpen = reply(554, "transaction failed") }},
+		{"a 5xx at the end of data", func(s *scriptedSMTPServer) { s.dataEnd = reply(552, "quota exceeded") }},
+	} {
+		t.Run(c.name, func(t *testing.T) {
+			s := newScriptedSMTPServer(t)
+			c.set(s)
+			acc := classifyTestAccount(s.addr())
+			err := SendMail(s.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1")
+			if err == nil || outcomeOf(t, err) != Refused {
+				t.Fatalf("err %v, want Refused", err)
+			}
+		})
+	}
+}
+
+// 1197/AC3: given the connection lost after the data was written and before any reply, then it says not
+// proven.
+func TestSendMailSaysNotProvenWhenTheConnectionIsLostAfterTheDataWasWritten(t *testing.T) {
+	s := newScriptedSMTPServer(t)
+	s.dataEnd = drop()
+	acc := classifyTestAccount(s.addr())
+	err := SendMail(s.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1")
+	if err == nil || outcomeOf(t, err) != NotProven {
+		t.Fatalf("err %v, want NotProven", err)
+	}
+}
+
+// 1197/AC4: given a send id, when the message is composed, then it carries a Message-ID naming the send id
+// (<send id>@<the account's domain>). Proved both in isolation (composeMessage) and over the wire
+// (SendMail, with the existing fakeSMTPServer, which keeps the body it received).
+func TestComposeMessageCarriesTheSendIDInItsMessageID(t *testing.T) {
+	raw := composeMessage("jschoubben@novox.be", []string{"a@b.c"}, "s", "body", judgedAt, "send-abc123")
+	if !strings.Contains(string(raw), "Message-Id: <send-abc123@novox.be>") {
+		t.Fatalf("the message does not carry the send id's Message-ID:\n%s", raw)
+	}
+}
+
+// composeMessage falls back to "localhost" when From carries no domain at all (never an empty or
+// malformed Message-ID).
+func TestComposeMessageFallsBackToLocalhostWithNoDomain(t *testing.T) {
+	raw := composeMessage("postmaster", nil, "s", "body", judgedAt, "send-1")
+	if !strings.Contains(string(raw), "Message-Id: <send-1@localhost>") {
+		t.Fatalf("message: %s", raw)
+	}
+}
+
+func TestSendMailCarriesTheMessageIDOverTheWire(t *testing.T) {
+	fake := newFakeSMTPServer(t)
+	acc := classifyTestAccount(fake.addr())
+	if err := SendMail(fake.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-xyz"); err != nil {
+		t.Fatalf("SendMail: %v", err)
+	}
+	got := fake.last()
+	if !strings.Contains(got.data, "Message-Id: <send-xyz@novox.be>") {
+		t.Fatalf("the message sent does not carry the send id's Message-ID:\n%s", got.data)
+	}
+}