README.md

@@ -11,7 +11,7 @@ First generate a new configuration file following this example:
 # Port the SMTP server will listen on
 Port = 11025
 
-# Credentials for the SMTP server (default to username/password if not set/empty)
+# Credentials for the SMTP server (required — the server refuses to start without them)
 Username = "user"
 Password = "nometokens"
 
@@ -47,7 +47,21 @@ Targets = ["ntfy://ntfy.sh/catch-all-topic?tags=unmatched"]
 The server refuses to start if the configuration cannot forward anything — no
 recipients and no catch-all, a recipient without addresses or usable targets, or
 a mistyped table name such as `[[Recipient]]`, which TOML would otherwise accept
-silently.
+silently. It also refuses to start without a `Username` and a `Password`: these
+used to fall back to `username`/`password`, which left the authentication gate
+open to the most obvious guess there is.
+
+Clients must authenticate with the configured `Username` and `Password` before
+starting a mail transaction; an unauthenticated client is refused rather than
+relayed. A client that used to send without issuing `AUTH` has to be configured
+with the credentials above.
+
+The reply the sending client gets reflects what happened to the notification:
+`250` once at least one target has accepted it, `451` when none did so the
+sender retries, and `550` for a message that cannot be parsed at all — a
+malformed `Content-Type`, for instance — which no retry could fix. A partial
+failure is logged but still accepted, since a retry would redeliver to every
+target and duplicate the notification on the ones that already have it.
 
 ### From releases
 

backend.go

@@ -1,6 +1,9 @@
 package smtp2shoutrrr
 
 import (
+	"crypto/sha256"
+	"crypto/subtle"
+	"errors"
 	"fmt"
 	"io"
 	"log/slog"
@@ -8,12 +11,47 @@ import (
 	"net/url"
 	"slices"
 	"strings"
+	"sync"
 
 	"github.com/containrrr/shoutrrr"
 	"github.com/emersion/go-sasl"
 	"github.com/emersion/go-smtp"
 )
 
+// errNoUsableTargets marks a configuration fault rather than a delivery one:
+// the recipient matched, but there is nothing to notify. No retry can add
+// targets to a configuration.
+var errNoUsableTargets = errors.New("recipient has no usable targets")
+
+var (
+	// errUnreadableMessage rejects a message for good. Both causes — a
+	// message that is not parseable as mail, and a Content-Type this server
+	// cannot make sense of — fail identically on every redelivery.
+	errUnreadableMessage = &smtp.SMTPError{
+		Code:         550,
+		EnhancedCode: smtp.EnhancedCode{5, 6, 0},
+		Message:      "Message could not be parsed",
+	}
+
+	// errMisconfiguredRecipient is permanent for the same reason: the server
+	// has nowhere to send this recipient's mail until its configuration
+	// changes, which no amount of redelivery brings about.
+	errMisconfiguredRecipient = &smtp.SMTPError{
+		Code:         550,
+		EnhancedCode: smtp.EnhancedCode{5, 3, 5},
+		Message:      "Recipient has no notification targets configured",
+	}
+
+	// errForwardingFailed asks the sender to try again later. The 250 this
+	// replaces told the client a message had been delivered when no target
+	// had received it, so the client dropped a notification nobody ever saw.
+	errForwardingFailed = &smtp.SMTPError{
+		Code:         451,
+		EnhancedCode: smtp.EnhancedCode{4, 3, 0},
+		Message:      "Failed to forward message to notification targets",
+	}
+)
+
 type Backend struct {
 	config *Config
 }
@@ -23,9 +61,8 @@ func (bkd *Backend) sendNotifications(recipient ConfigRecipient, email ReceivedE
 	targetURLs := recipient.GetTargetURLs()
 
 	if len(targetURLs) == 0 {
-		slog.Warn("no valid targets provided for recipient",
-			slog.String("recipient", strings.Join(recipient.Addresses, ",")))
-		return nil
+		return fmt.Errorf("%w: %s", errNoUsableTargets,
+			strings.Join(recipient.Addresses, ","))
 	}
 
 	// Prepare email body once (reuse for all targets)
@@ -73,10 +110,18 @@ func (bkd *Backend) sendNotifications(recipient ConfigRecipient, email ReceivedE
 		}
 	}
 
-	// Return aggregated error if any failed
+	// Only a total failure is reported to the sender. A retry redelivers to
+	// every target, so pushing back on a partial one would duplicate the
+	// notification on the targets that already accepted it.
+	if len(errs) == len(targetURLs) {
+		return fmt.Errorf("no target accepted the notification: %w", errors.Join(errs...))
+	}
+
 	if len(errs) > 0 {
-		return fmt.Errorf("failed to send to %d/%d targets: %v",
-			len(errs), len(targetURLs), errs)
+		slog.Warn("notification reached only some targets",
+			slog.Int("failed", len(errs)),
+			slog.Int("targets", len(targetURLs)),
+			slog.String("recipient", strings.Join(recipient.Addresses, ",")))
 	}
 
 	return nil
@@ -121,57 +166,161 @@ func (bkd *Backend) forwardEmail(email ReceivedEmail) error {
 }
 
 type Session struct {
+	// mu guards the transaction state below. go-smtp runs Data on its own
+	// goroutine for a BDAT transfer and keeps serving commands on the
+	// connection goroutine meanwhile, so an RSET can land in the middle of
+	// one and reset the session from under it.
+	mu sync.Mutex
+
 	addresses []string
 
+	// authenticated gates the mail transaction. go-smtp advertises AUTH but
+	// never enforces it — that is the backend's job — so without this the
+	// server relays for anyone who can open a connection.
+	authenticated bool
+
 	config *Config
 
 	forwarderFunc func(ReceivedEmail) error
 }
 
+func (s *Session) isAuthenticated() bool {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	return s.authenticated
+}
+
+// recipients copies the transaction's recipients so a concurrent Reset cannot
+// empty the slice a forward in progress is still reading.
+func (s *Session) recipients() []string {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	return slices.Clone(s.addresses)
+}
+
 func (s *Session) AuthMechanisms() []string {
 	return []string{sasl.Plain}
 }
 
 func (s *Session) Auth(mech string) (sasl.Server, error) {
+	if mech != sasl.Plain {
+		return nil, smtp.ErrAuthUnknownMechanism
+	}
+
 	return sasl.NewPlainServer(func(identity, username, password string) error {
-		if username != s.config.Username && password != s.config.Password {
-			return fmt.Errorf("invalid credentials")
+		// PLAIN carries an authorization identity distinct from the
+		// authenticating one (RFC 4616). There is nobody to act on behalf of
+		// here, so only the empty and self-referential forms are accepted.
+		if identity != "" && identity != username {
+			return smtp.ErrAuthFailed
 		}
+
+		// Both comparisons run before either is acted on: short-circuiting on
+		// the username would time-distinguish a wrong username from a right
+		// one with a wrong password, handing an attacker the username.
+		usernameMatches := credentialMatches(username, s.config.Username)
+		passwordMatches := credentialMatches(password, s.config.Password)
+
+		if !usernameMatches || !passwordMatches {
+			// Logged without the attempted values: a misconfigured client
+			// sends its password in the username field often enough that
+			// recording them turns the log into a credential store.
+			slog.Warn("rejected authentication attempt")
+			return smtp.ErrAuthFailed
+		}
+
+		s.mu.Lock()
+		s.authenticated = true
+		s.mu.Unlock()
+
 		return nil
 	}), nil
 }
 
+// credentialMatches compares a supplied credential against the configured one
+// in time independent of how much of it is right. Both sides are hashed first
+// because subtle.ConstantTimeCompare returns early when the lengths differ,
+// which would still leak the length of the configured credential.
+func credentialMatches(supplied, configured string) bool {
+	suppliedSum := sha256.Sum256([]byte(supplied))
+	configuredSum := sha256.Sum256([]byte(configured))
+
+	return subtle.ConstantTimeCompare(suppliedSum[:], configuredSum[:]) == 1
+}
+
 func (s *Session) Mail(from string, opts *smtp.MailOptions) error {
+	if !s.isAuthenticated() {
+		return smtp.ErrAuthRequired
+	}
+
 	slog.Debug("Mail from", slog.String("from", from))
+
 	return nil
 }
 
 func (s *Session) Rcpt(to string, opts *smtp.RcptOptions) error {
+	if !s.isAuthenticated() {
+		return smtp.ErrAuthRequired
+	}
+
 	slog.Debug("Rcpt to", slog.String("to", to))
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	s.addresses = append(s.addresses, to)
+
 	return nil
 }
 
 func (s *Session) Data(r io.Reader) error {
+	if !s.isAuthenticated() {
+		return smtp.ErrAuthRequired
+	}
+
+	// Taken before the message is read: an RSET arriving mid-transfer aborts
+	// the read, and this is the transaction the recipients belong to either
+	// way.
+	recipients := s.recipients()
+
 	msg, err := mail.ReadMessage(r)
 	if err != nil {
 		slog.Error("Error reading data", slog.String("err", err.Error()))
-		return fmt.Errorf("error reading data: %w", err)
+		return errUnreadableMessage
 	}
 
-	slog.Info("Received email", slog.String("destination", strings.Join(s.addresses, ",")))
+	slog.Info("Received email", slog.String("destination", strings.Join(recipients, ",")))
 
 	if err := s.forwarderFunc(ReceivedEmail{
-		Recipients: s.addresses,
+		Recipients: recipients,
 		Msg:        msg,
 	}); err != nil {
 		slog.Error("Error forwarding email", slog.String("err", err.Error()))
+
+		switch {
+		case errors.Is(err, errMalformedMessage):
+			return errUnreadableMessage
+		case errors.Is(err, errNoUsableTargets):
+			return errMisconfiguredRecipient
+		default:
+			return errForwardingFailed
+		}
 	}
 
 	return nil
 }
 
-func (s *Session) Reset() {}
+// Reset ends the current mail transaction. go-smtp calls it after every
+// message, so the recipients have to go: leaving them would forward the next
+// message on the connection to the previous one's targets as well.
+// Authentication deliberately survives, as RFC 5321 scopes RSET to the
+// transaction.
+func (s *Session) Reset() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.addresses = nil
+}
 
 func (s *Session) Logout() error {
 	return nil

backend_test.go

@@ -1,10 +1,15 @@
 package smtp2shoutrrr
 
 import (
+	"bufio"
+	"encoding/base64"
+	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/smtp"
+	"strings"
 	"testing"
 	"time"
 
@@ -282,3 +287,415 @@ func TestInvalidTargetURLs(t *testing.T) {
 	// Verify only valid URLs received notifications (2 out of 4)
 	require.Len(t, notifications, 2)
 }
+
+// Regression: the credential check joined the two comparisons with &&, so it
+// only rejected a client that got both halves wrong. A correct username with
+// any password — or a correct password with any username — authenticated, and
+// anyone who guessed either half could relay to every configured target.
+func TestAuthenticationRejectsPartiallyCorrectCredentials(t *testing.T) {
+	notifications := 0
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = io.ReadAll(r.Body)
+		notifications++
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer mockServer.Close()
+
+	config := &Config{
+		Port:     0,
+		Username: "testuser",
+		Password: "testpass",
+		Recipients: []ConfigRecipient{
+			{
+				Addresses: []string{"test@example.com"},
+				Targets:   []string{"generic+" + mockServer.URL + "/?template=json"},
+			},
+		},
+	}
+
+	addr, _, _ := startServer(t, NewSMTPServer(config))
+
+	for name, credentials := range map[string][2]string{
+		"correct username, wrong password": {"testuser", "wrong"},
+		"wrong username, correct password": {"wrong", "testpass"},
+		"both wrong":                       {"wrong", "wrong"},
+		"empty credentials":                {"", ""},
+	} {
+		t.Run(name, func(t *testing.T) {
+			err := smtp.SendMail(
+				addr,
+				smtp.PlainAuth("", credentials[0], credentials[1], "localhost"),
+				"sender@example.com",
+				[]string{"test@example.com"},
+				[]byte("Subject: Should Not Arrive\r\n\r\nbody"),
+			)
+			require.Error(t, err)
+			require.Contains(t, err.Error(), "535", "authentication should be refused permanently")
+		})
+	}
+
+	require.Zero(t, notifications, "a rejected client must not get anything forwarded")
+}
+
+// Regression: go-smtp advertises AUTH but leaves enforcement to the backend,
+// so before the session tracked it a client could skip AUTH entirely and still
+// have its message forwarded.
+func TestUnauthenticatedClientCannotSendMail(t *testing.T) {
+	notifications := 0
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = io.ReadAll(r.Body)
+		notifications++
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer mockServer.Close()
+
+	config := &Config{
+		Port:     0,
+		Username: "testuser",
+		Password: "testpass",
+		Recipients: []ConfigRecipient{
+			{
+				Addresses: []string{"test@example.com"},
+				Targets:   []string{"generic+" + mockServer.URL + "/?template=json"},
+			},
+		},
+	}
+
+	addr, _, _ := startServer(t, NewSMTPServer(config))
+
+	client, err := smtp.Dial(addr)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = client.Close() })
+
+	require.NoError(t, client.Hello("localhost"))
+
+	err = client.Mail("sender@example.com")
+	require.Error(t, err, "the transaction must not start without authentication")
+	require.Contains(t, err.Error(), "502")
+
+	require.Zero(t, notifications)
+}
+
+// Regression: a malformed Content-Type reached log.Fatalf, so one message with
+// an attacker-controlled header terminated the daemon and took every other
+// in-flight message with it.
+func TestMalformedContentTypeIsRejectedAndServerKeepsServing(t *testing.T) {
+	notifications := make([]string, 0)
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		body, _ := io.ReadAll(r.Body)
+		notifications = append(notifications, string(body))
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer mockServer.Close()
+
+	config := &Config{
+		Port:     0,
+		Username: "testuser",
+		Password: "testpass",
+		Recipients: []ConfigRecipient{
+			{
+				Addresses: []string{"test@example.com"},
+				Targets:   []string{"generic+" + mockServer.URL + "/?template=json"},
+			},
+		},
+	}
+
+	addr, _, _ := startServer(t, NewSMTPServer(config))
+	auth := smtp.PlainAuth("", config.Username, config.Password, "localhost")
+
+	err := smtp.SendMail(
+		addr,
+		auth,
+		"sender@example.com",
+		[]string{"test@example.com"},
+		[]byte("Subject: Malformed\r\nContent-Type: text/plain; charset\r\n\r\nbody"),
+	)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "550", "an unparseable message should be refused permanently")
+
+	err = smtp.SendMail(
+		addr,
+		auth,
+		"sender@example.com",
+		[]string{"test@example.com"},
+		[]byte("Subject: Well Formed\r\n\r\nthe server is still up"),
+	)
+	require.NoError(t, err)
+
+	require.Len(t, notifications, 1)
+	require.Contains(t, notifications[0], "the server is still up")
+}
+
+// Regression: Data logged the forwarding error and returned 250, so a client
+// whose notification reached no target at all considered the message delivered,
+// dropped it and never retried.
+func TestTotalForwardingFailureIsReportedToSender(t *testing.T) {
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = io.ReadAll(r.Body)
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer mockServer.Close()
+
+	config := &Config{
+		Port:     0,
+		Username: "testuser",
+		Password: "testpass",
+		Recipients: []ConfigRecipient{
+			{
+				Addresses: []string{"test@example.com"},
+				Targets: []string{
+					"generic+" + mockServer.URL + "/one?template=json",
+					"generic+" + mockServer.URL + "/two?template=json",
+				},
+			},
+		},
+	}
+
+	addr, _, _ := startServer(t, NewSMTPServer(config))
+
+	err := smtp.SendMail(
+		addr,
+		smtp.PlainAuth("", config.Username, config.Password, "localhost"),
+		"sender@example.com",
+		[]string{"test@example.com"},
+		[]byte("Subject: Nobody Received This\r\n\r\nbody"),
+	)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "451", "the sender should be asked to retry")
+}
+
+// A partial failure stays a 250: the sender would redeliver to every target,
+// duplicating the notification on the ones that already accepted it.
+func TestPartialForwardingFailureIsAcceptedFromSender(t *testing.T) {
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = io.ReadAll(r.Body)
+		if r.URL.Path == "/fail" {
+			w.WriteHeader(http.StatusInternalServerError)
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer mockServer.Close()
+
+	config := &Config{
+		Port:     0,
+		Username: "testuser",
+		Password: "testpass",
+		Recipients: []ConfigRecipient{
+			{
+				Addresses: []string{"test@example.com"},
+				Targets: []string{
+					"generic+" + mockServer.URL + "/fail?template=json",
+					"generic+" + mockServer.URL + "/ok?template=json",
+				},
+			},
+		},
+	}
+
+	addr, _, _ := startServer(t, NewSMTPServer(config))
+
+	require.NoError(t, smtp.SendMail(
+		addr,
+		smtp.PlainAuth("", config.Username, config.Password, "localhost"),
+		"sender@example.com",
+		[]string{"test@example.com"},
+		[]byte("Subject: Reached One Target\r\n\r\nbody"),
+	))
+}
+
+// Regression: Session.Reset was empty, so recipients accumulated across the
+// messages of a single connection and the second one was also forwarded to the
+// first one's targets.
+func TestRecipientsDoNotLeakBetweenMessagesOnOneConnection(t *testing.T) {
+	received := make(map[string][]string)
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		body, _ := io.ReadAll(r.Body)
+		received[r.URL.Path] = append(received[r.URL.Path], string(body))
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer mockServer.Close()
+
+	config := &Config{
+		Port:     0,
+		Username: "testuser",
+		Password: "testpass",
+		Recipients: []ConfigRecipient{
+			{
+				Addresses: []string{"first@example.com"},
+				Targets:   []string{"generic+" + mockServer.URL + "/first?template=json"},
+			},
+			{
+				Addresses: []string{"second@example.com"},
+				Targets:   []string{"generic+" + mockServer.URL + "/second?template=json"},
+			},
+		},
+	}
+
+	addr, _, _ := startServer(t, NewSMTPServer(config))
+
+	client, err := smtp.Dial(addr)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = client.Close() })
+
+	require.NoError(t, client.Hello("localhost"))
+	require.NoError(t, client.Auth(smtp.PlainAuth("", config.Username, config.Password, "localhost")))
+
+	for _, recipient := range []string{"first@example.com", "second@example.com"} {
+		require.NoError(t, client.Mail("sender@example.com"))
+		require.NoError(t, client.Rcpt(recipient))
+
+		w, err := client.Data()
+		require.NoError(t, err)
+		_, err = w.Write([]byte("Subject: For " + recipient + "\r\n\r\nbody for " + recipient))
+		require.NoError(t, err)
+		require.NoError(t, w.Close())
+	}
+
+	require.NoError(t, client.Quit())
+
+	require.Len(t, received["/first"], 1)
+	require.Contains(t, received["/first"][0], "body for first@example.com")
+	require.Len(t, received["/second"], 1)
+	require.Contains(t, received["/second"][0], "body for second@example.com")
+}
+
+// smtpProbe drives a raw SMTP conversation. net/smtp cannot issue BDAT, and
+// the transfer below needs a chunked one with an RSET pipelined behind it.
+type smtpProbe struct {
+	t    *testing.T
+	conn net.Conn
+	r    *bufio.Reader
+}
+
+func dialProbe(t *testing.T, addr string) *smtpProbe {
+	t.Helper()
+
+	conn, err := net.DialTimeout("tcp", addr, 5*time.Second)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = conn.Close() })
+	require.NoError(t, conn.SetDeadline(time.Now().Add(30*time.Second)))
+
+	probe := &smtpProbe{t: t, conn: conn, r: bufio.NewReader(conn)}
+	probe.expect("220")
+
+	return probe
+}
+
+func (p *smtpProbe) send(format string, args ...any) {
+	p.t.Helper()
+
+	_, err := fmt.Fprintf(p.conn, format, args...)
+	require.NoError(p.t, err)
+}
+
+// expect reads one full reply, skipping the continuation lines of a multiline
+// one such as the EHLO capability list, and asserts its status code.
+func (p *smtpProbe) expect(code string) {
+	p.t.Helper()
+
+	for {
+		line, err := p.r.ReadString('\n')
+		require.NoError(p.t, err)
+
+		if len(line) > 3 && line[3] == '-' {
+			continue
+		}
+
+		require.Truef(p.t, strings.HasPrefix(line, code), "expected %s, got %q", code, line)
+
+		return
+	}
+}
+
+func (p *smtpProbe) authenticate(username, password string) {
+	p.t.Helper()
+
+	p.send("EHLO localhost\r\n")
+	p.expect("250")
+	p.send("AUTH PLAIN %s\r\n",
+		base64.StdEncoding.EncodeToString([]byte("\x00"+username+"\x00"+password)))
+	p.expect("235")
+}
+
+// Regression: Reset writes the transaction's recipients on the connection
+// goroutine, while go-smtp runs Data on its own goroutine for a BDAT transfer
+// and does not block RSET during one. Under -race this reported a data race
+// between Reset and the forward in Data; it only passes while the transaction
+// state stays guarded.
+func TestResetDuringChunkedTransferIsSynchronized(t *testing.T) {
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = io.ReadAll(r.Body)
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer mockServer.Close()
+
+	config := &Config{
+		Port:     0,
+		Username: "testuser",
+		Password: "testpass",
+		Recipients: []ConfigRecipient{
+			{
+				Addresses: []string{"test@example.com"},
+				Targets:   []string{"generic+" + mockServer.URL + "/?template=json"},
+			},
+		},
+	}
+
+	addr, _, _ := startServer(t, NewSMTPServer(config))
+
+	// The overlap is a few instructions wide, so the transfer is repeated
+	// rather than attempted once.
+	for range 30 {
+		probe := dialProbe(t, addr)
+		probe.authenticate(config.Username, config.Password)
+
+		probe.send("MAIL FROM:<sender@example.com>\r\n")
+		probe.expect("250")
+		probe.send("RCPT TO:<test@example.com>\r\n")
+		probe.expect("250")
+
+		// Headers without their terminating blank line, so the transfer
+		// goroutine parks inside mail.ReadMessage.
+		const headers = "Subject: Chunked\r\n"
+		probe.send("BDAT %d\r\n%s", len(headers), headers)
+		probe.expect("250")
+
+		// The blank line releases mail.ReadMessage on the transfer goroutine;
+		// the RSET pipelined behind it reaches Session.Reset on the connection
+		// goroutine at the same moment.
+		probe.send("BDAT 2\r\n\r\nRSET\r\n")
+		probe.expect("250")
+		probe.expect("250")
+
+		// The connection outlives the aborted transaction.
+		probe.send("QUIT\r\n")
+		probe.expect("221")
+	}
+}
+
+// A recipient with no usable targets is a configuration fault, not a delivery
+// one: no retry can add targets to a config, so it is refused permanently
+// rather than with the 451 the other forwarding failures get. LoadConfig
+// blocks such a config, so this is only reachable from one built in code.
+func TestRecipientWithoutTargetsIsRejectedPermanently(t *testing.T) {
+	config := &Config{
+		Port:     0,
+		Username: "testuser",
+		Password: "testpass",
+		Recipients: []ConfigRecipient{
+			{Addresses: []string{"test@example.com"}, Targets: []string{"://not-a-url"}},
+		},
+	}
+
+	addr, _, _ := startServer(t, NewSMTPServer(config))
+
+	err := smtp.SendMail(
+		addr,
+		smtp.PlainAuth("", config.Username, config.Password, "localhost"),
+		"sender@example.com",
+		[]string{"test@example.com"},
+		[]byte("Subject: Nowhere To Go\r\n\r\nbody"),
+	)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "550", "a retry cannot add targets to a configuration")
+}

config.go

@@ -67,23 +67,25 @@ func (c *Config) SetDefaults() {
 	if c.Port == 0 {
 		c.Port = 11125
 	}
+}
 
+// Validate rejects configuration that would leave the server relaying for
+// anyone, or accepting mail it can never forward. A TOML decode ignores
+// unknown keys, so a mistyped table or field name ([[Recipient]], Adresses)
+// otherwise yields a server that answers 250 OK to everything and sends
+// nothing.
+func (c *Config) Validate() error {
+	// Credentials used to fall back to username/password when unset, which
+	// left the AUTH gate open to the most obvious guess there is — for
+	// exactly the deployments that had configured the least.
 	if c.Username == "" {
-		slog.Warn("no username provided, using default: username")
-		c.Username = "username"
+		return errors.New("no Username configured")
 	}
 
 	if c.Password == "" {
-		slog.Warn("no password provided, using default: password")
-		c.Password = "password"
+		return errors.New("no Password configured")
 	}
-}
 
-// Validate rejects configuration that would leave the server accepting mail it
-// can never forward. A TOML decode ignores unknown keys, so a mistyped table or
-// field name ([[Recipient]], Adresses) otherwise yields a server that answers
-// 250 OK to everything and sends nothing.
-func (c *Config) Validate() error {
 	for i := range c.Recipients {
 		r := &c.Recipients[i]
 		if len(r.Addresses) == 0 {

config_test.go

@@ -10,6 +10,14 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// credentials is prepended to fixtures that are not themselves about
+// authentication, so each one still fails (or passes) for the reason it names
+// rather than for the credentials it does not mention.
+const credentials = `
+Username = "user"
+Password = "secret"
+`
+
 func writeConfig(t *testing.T, contents string) string {
 	t.Helper()
 
@@ -54,7 +62,7 @@ Targets = ["ntfy://ntfy.sh/catch-all"]
 }
 
 func TestLoadConfigAppliesDefaults(t *testing.T) {
-	path := writeConfig(t, `
+	path := writeConfig(t, credentials+`
 [[Recipients]]
 Addresses = ["user@example.com"]
 Targets = ["ntfy://ntfy.sh/topic"]
@@ -64,9 +72,38 @@ Targets = ["ntfy://ntfy.sh/topic"]
 	require.NoError(t, err)
 
 	require.Equal(t, 11125, config.Port)
-	require.Equal(t, "username", config.Username)
-	require.Equal(t, "password", config.Password)
 	require.Nil(t, config.CatchAll)
+
+	// Credentials deliberately have no default; see
+	// TestLoadConfigRejectsUnsetCredentials.
+	require.Equal(t, "user", config.Username)
+	require.Equal(t, "secret", config.Password)
+}
+
+// Regression: unset credentials used to fall back to username/password with
+// only a warning, so a config naming nothing but a port and its recipients
+// left the AUTH gate open to the most obvious guess there is — and the README
+// promised an authentication that had never been configured.
+func TestLoadConfigRejectsUnsetCredentials(t *testing.T) {
+	recipient := `
+[[Recipients]]
+Addresses = ["user@example.com"]
+Targets = ["ntfy://ntfy.sh/topic"]
+`
+
+	for name, contents := range map[string]string{
+		"neither credential": recipient,
+		"no Username":        `Password = "secret"` + recipient,
+		"no Password":        `Username = "user"` + recipient,
+		"empty Username":     `Username = ""` + "\n" + `Password = "secret"` + recipient,
+		"empty Password":     `Username = "user"` + "\n" + `Password = ""` + recipient,
+	} {
+		t.Run(name, func(t *testing.T) {
+			config, err := LoadConfig(writeConfig(t, contents))
+			require.Error(t, err)
+			require.Nil(t, config)
+		})
+	}
 }
 
 func TestLoadConfigMissingFile(t *testing.T) {
@@ -116,14 +153,14 @@ Port = 2525
 `,
 	} {
 		t.Run(name, func(t *testing.T) {
-			_, err := LoadConfig(writeConfig(t, contents))
+			_, err := LoadConfig(writeConfig(t, credentials+contents))
 			require.Error(t, err)
 		})
 	}
 }
 
 func TestLoadConfigAcceptsCatchAllOnly(t *testing.T) {
-	config, err := LoadConfig(writeConfig(t, `
+	config, err := LoadConfig(writeConfig(t, credentials+`
 [CatchAll]
 Targets = ["ntfy://ntfy.sh/catch-all"]
 `))
@@ -141,7 +178,7 @@ func TestLoadConfigWarnsAboutIgnoredKeys(t *testing.T) {
 	slog.SetDefault(slog.New(slog.NewTextHandler(&logged, &slog.HandlerOptions{Level: slog.LevelWarn})))
 	t.Cleanup(func() { slog.SetDefault(restore) })
 
-	config, err := LoadConfig(writeConfig(t, `
+	config, err := LoadConfig(writeConfig(t, credentials+`
 [[Recipient]]
 Addresses = ["user@example.com"]
 Targets = ["ntfy://ntfy.sh/topic"]

email.go

@@ -2,9 +2,9 @@ package smtp2shoutrrr
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
-	"log"
 	"log/slog"
 	"mime"
 	"mime/multipart"
@@ -12,6 +12,11 @@ import (
 	"strings"
 )
 
+// errMalformedMessage marks a message this server can never turn into a
+// notification. Redelivering the same bytes produces the same failure, so the
+// sender is told to give up rather than retry forever.
+var errMalformedMessage = errors.New("malformed message")
+
 type ReceivedEmail struct {
 	Recipients []string
 	Msg        *mail.Message
@@ -31,7 +36,7 @@ func (re *ReceivedEmail) Body() (string, error) {
 		} else {
 			mediaType, params, err := mime.ParseMediaType(contentType)
 			if err != nil {
-				log.Fatalf("Failed to parse Content-Type: %v", err)
+				return "", fmt.Errorf("%w: parsing Content-Type %q: %w", errMalformedMessage, contentType, err)
 			}
 
 			if strings.HasPrefix(mediaType, "multipart/") {

email_test.go

@@ -0,0 +1,90 @@
+package smtp2shoutrrr
+
+import (
+	"net/mail"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func readMessage(t *testing.T, raw string) *mail.Message {
+	t.Helper()
+
+	msg, err := mail.ReadMessage(strings.NewReader(raw))
+	require.NoError(t, err)
+
+	return msg
+}
+
+// Regression: this header reached log.Fatalf, which exits the process. The
+// value is attacker-controlled and needs nothing but the ability to deliver a
+// message.
+func TestBodyRejectsMalformedContentType(t *testing.T) {
+	for name, contentType := range map[string]string{
+		"parameter without a value": "text/plain; charset",
+		"duplicate parameter":       "text/plain; charset=utf-8; charset=ascii",
+		"no media type":             ";",
+		"unterminated quoted value": `multipart/mixed; boundary="unterminated`,
+	} {
+		t.Run(name, func(t *testing.T) {
+			email := ReceivedEmail{
+				Msg: readMessage(t, "Subject: Test\r\nContent-Type: "+contentType+"\r\n\r\nbody\r\n"),
+			}
+
+			body, err := email.Body()
+			require.Error(t, err)
+			require.ErrorIs(t, err, errMalformedMessage,
+				"an unparseable header is permanent, so the sender must not be told to retry")
+			require.Empty(t, body)
+		})
+	}
+}
+
+func TestBodyReadsMessageWithoutContentType(t *testing.T) {
+	email := ReceivedEmail{Msg: readMessage(t, "Subject: Test\r\n\r\nplain body\r\n")}
+
+	body, err := email.Body()
+	require.NoError(t, err)
+	require.Equal(t, "plain body\r\n", body)
+}
+
+func TestBodyPrefersPlainTextPartOverHTML(t *testing.T) {
+	email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{
+		"Subject: Test",
+		`Content-Type: multipart/alternative; boundary="b"`,
+		"",
+		"--b",
+		"Content-Type: text/html; charset=utf-8",
+		"",
+		"<p>html body</p>",
+		"--b",
+		"Content-Type: text/plain; charset=utf-8",
+		"",
+		"plain body",
+		"--b--",
+		"",
+	}, "\r\n"))}
+
+	body, err := email.Body()
+	require.NoError(t, err)
+	require.Equal(t, "plain body", body)
+}
+
+func TestBodyFallsBackToHTMLPart(t *testing.T) {
+	email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{
+		"Subject: Test",
+		`Content-Type: multipart/alternative; boundary="b"`,
+		"",
+		"--b",
+		"Content-Type: text/html; charset=utf-8",
+		"",
+		"<p>html body</p>",
+		"--b--",
+		"",
+	}, "\r\n"))}
+
+	body, err := email.Body()
+	require.NoError(t, err)
+	require.Equal(t, "<p>html body</p>", body)
+}