fix: separate SMTP envelope and header addresses (#21840)

## Description

When configuring a From address with a display name (e.g., `Coder System
<system@coder.com>`), the SMTP `MAIL FROM` command was incorrectly
receiving the full address string instead of just the bare email
address, causing `501 Invalid MAIL argument` errors on some SMTP
servers.

## Changes

- Updated `validateFromAddr` to return both:
  - `envelopeFrom`: bare email for SMTP `MAIL FROM` command (RFC 5321)
- `headerFrom`: original address with display name for email header (RFC
5322)

Fixes #20727

coderd/notifications/dispatch/smtp.go

@@ -156,11 +156,11 @@ func (s *SMTPHandler) dispatch(subject, htmlBody, plainBody, to string) Delivery
 		}
 
 		// Sender identification.
-		from, err := s.validateFromAddr(s.cfg.From.String())
+		envelopeFrom, headerFrom, err := s.validateFromAddr(s.cfg.From.String())
 		if err != nil {
 			return false, xerrors.Errorf("'from' validation: %w", err)
 		}
-		err = c.Mail(from, &smtp.MailOptions{})
+		err = c.Mail(envelopeFrom, &smtp.MailOptions{})
 		if err != nil {
 			// This is retryable because the server may be temporarily down.
 			return true, xerrors.Errorf("sender identification: %w", err)
@@ -200,7 +200,7 @@ func (s *SMTPHandler) dispatch(subject, htmlBody, plainBody, to string) Delivery
 		msg := &bytes.Buffer{}
 		multipartBuffer := &bytes.Buffer{}
 		multipartWriter := multipart.NewWriter(multipartBuffer)
-		_, _ = fmt.Fprintf(msg, "From: %s\r\n", from)
+		_, _ = fmt.Fprintf(msg, "From: %s\r\n", headerFrom)
 		_, _ = fmt.Fprintf(msg, "To: %s\r\n", strings.Join(recipients, ", "))
 		_, _ = fmt.Fprintf(msg, "Subject: %s\r\n", subject)
 		_, _ = fmt.Fprintf(msg, "Message-Id: %s@%s\r\n", msgID, s.hostname())
@@ -486,15 +486,25 @@ func (s *SMTPHandler) auth(ctx context.Context, mechs string) (sasl.Client, erro
 	return nil, errs
 }
 
-func (*SMTPHandler) validateFromAddr(from string) (string, error) {
+// validateFromAddr parses the "from" address and returns two values:
+// 1. envelopeFrom: The bare email address for use in the SMTP MAIL FROM command.
+// 2. headerFrom: The original address (possibly including display name) for use in the email header.
+//
+// This separation is necessary because SMTP envelope addresses (used in MAIL FROM
+// and RCPT TO commands) must be bare email addresses, while email headers can
+// include display names (e.g., "John Doe <john@example.com>").
+func (*SMTPHandler) validateFromAddr(from string) (envelopeFrom, headerFrom string, err error) {
 	addrs, err := mail.ParseAddressList(from)
 	if err != nil {
-		return "", xerrors.Errorf("parse 'from' address: %w", err)
+		return "", "", xerrors.Errorf("parse 'from' address: %w", err)
 	}
 	if len(addrs) != 1 {
-		return "", ErrValidationNoFromAddress
+		return "", "", ErrValidationNoFromAddress
 	}
-	return from, nil
+	// Use the parsed email address for the SMTP envelope (MAIL FROM command),
+	// but preserve the original string for the email header (which may include
+	// a display name).
+	return addrs[0].Address, from, nil
 }
 
 func (s *SMTPHandler) validateToAddrs(to string) ([]string, error) {

coderd/notifications/dispatch/smtp_internal_test.go

@@ -0,0 +1,81 @@
+package dispatch
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateFromAddr(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name               string
+		input              string
+		expectedEnvelope   string
+		expectedHeader     string
+		expectedErrContain string
+	}{
+		{
+			name:             "bare email address",
+			input:            "system@coder.com",
+			expectedEnvelope: "system@coder.com",
+			expectedHeader:   "system@coder.com",
+		},
+		{
+			name:             "email with display name",
+			input:            "Coder System <system@coder.com>",
+			expectedEnvelope: "system@coder.com",
+			expectedHeader:   "Coder System <system@coder.com>",
+		},
+		{
+			name:             "email with quoted display name",
+			input:            `"Coder Notifications" <notifications@coder.com>`,
+			expectedEnvelope: "notifications@coder.com",
+			expectedHeader:   `"Coder Notifications" <notifications@coder.com>`,
+		},
+		{
+			name:             "email with special characters in display name",
+			input:            `"O'Brien, John" <john@example.com>`,
+			expectedEnvelope: "john@example.com",
+			expectedHeader:   `"O'Brien, John" <john@example.com>`,
+		},
+		{
+			name:               "invalid email address",
+			input:              "not-an-email",
+			expectedErrContain: "parse 'from' address",
+		},
+		{
+			name:               "empty string",
+			input:              "",
+			expectedErrContain: "parse 'from' address",
+		},
+		{
+			name:               "multiple addresses",
+			input:              "a@example.com, b@example.com",
+			expectedErrContain: "'from' address not defined",
+		},
+	}
+
+	handler := &SMTPHandler{}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			envelope, header, err := handler.validateFromAddr(tc.input)
+
+			if tc.expectedErrContain != "" {
+				require.Error(t, err)
+				require.ErrorContains(t, err, tc.expectedErrContain)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedEnvelope, envelope,
+				"envelope address should be the bare email")
+			require.Equal(t, tc.expectedHeader, header,
+				"header address should preserve the original input")
+		})
+	}
+}

coderd/notifications/dispatch/smtp_test.go

@@ -515,3 +515,124 @@ func TestSMTP(t *testing.T) {
 		})
 	}
 }
+
+// TestSMTPEnvelopeAndHeaders verifies that SMTP envelope addresses (used in
+// MAIL FROM and RCPT TO commands) contain only bare email addresses, while
+// email headers preserve the full address including display names.
+//
+// This is important because RFC 5321 requires envelope addresses to be bare
+// emails, while RFC 5322 allows headers to include display names.
+//
+// See: https://github.com/coder/coder/issues/20727
+func TestSMTPEnvelopeAndHeaders(t *testing.T) {
+	t.Parallel()
+
+	const (
+		hello = "localhost"
+		to    = "bob@bob.com"
+
+		subject = "This is the subject"
+		body    = "This is the body"
+	)
+
+	tests := []struct {
+		name               string
+		fromConfig         string // The configured From address (may include display name)
+		expectedEnvFrom    string // Expected envelope MAIL FROM (bare email)
+		expectedHeaderFrom string // Expected From header (preserves display name)
+	}{
+		{
+			name:               "bare email address",
+			fromConfig:         "system@coder.com",
+			expectedEnvFrom:    "system@coder.com",
+			expectedHeaderFrom: "system@coder.com",
+		},
+		{
+			name:               "email with display name",
+			fromConfig:         "Coder System <system@coder.com>",
+			expectedEnvFrom:    "system@coder.com",
+			expectedHeaderFrom: "Coder System <system@coder.com>",
+		},
+		{
+			name:               "email with quoted display name",
+			fromConfig:         `"Coder Notifications" <notifications@coder.com>`,
+			expectedEnvFrom:    "notifications@coder.com",
+			expectedHeaderFrom: `"Coder Notifications" <notifications@coder.com>`,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx := testutil.Context(t, testutil.WaitShort)
+			logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
+
+			cfg := codersdk.NotificationsEmailConfig{
+				Hello: serpent.String(hello),
+				From:  serpent.String(tc.fromConfig),
+			}
+
+			backend := smtptest.NewBackend(smtptest.Config{
+				AuthMechanisms: []string{},
+			})
+
+			srv, listen, err := smtptest.CreateMockSMTPServer(backend, false)
+			require.NoError(t, err)
+			t.Cleanup(func() {
+				assert.ErrorIs(t, srv.Shutdown(ctx), smtp.ErrServerClosed)
+			})
+
+			var hp serpent.HostPort
+			require.NoError(t, hp.Set(listen.Addr().String()))
+			cfg.Smarthost = serpent.String(hp.String())
+
+			handler := dispatch.NewSMTPHandler(cfg, logger.Named("smtp"))
+
+			var wg sync.WaitGroup
+			wg.Add(1)
+			go func() {
+				defer wg.Done()
+				assert.NoError(t, srv.Serve(listen))
+			}()
+
+			require.Eventually(t, func() bool {
+				cl, err := smtptest.PingClient(listen, false, false)
+				if err != nil {
+					return false
+				}
+				_ = cl.Close()
+				return true
+			}, testutil.WaitShort, testutil.IntervalFast)
+
+			payload := types.MessagePayload{
+				Version:   "1.0",
+				UserEmail: to,
+				Labels:    make(map[string]string),
+			}
+
+			dispatchFn, err := handler.Dispatcher(payload, subject, body, helpers())
+			require.NoError(t, err)
+
+			msgID := uuid.New()
+			retryable, err := dispatchFn(ctx, msgID)
+
+			require.NoError(t, err)
+			require.False(t, retryable)
+
+			msg := backend.LastMessage()
+			require.NotNil(t, msg)
+
+			// Verify envelope address (MAIL FROM) contains only the bare email.
+			require.Equal(t, tc.expectedEnvFrom, msg.From,
+				"SMTP envelope MAIL FROM should contain only the bare email address")
+
+			// Verify header From preserves the display name.
+			require.Contains(t, msg.Contents, fmt.Sprintf("From: %s\r\n", tc.expectedHeaderFrom),
+				"Email From header should preserve the display name if present")
+
+			require.NoError(t, srv.Shutdown(ctx))
+			wg.Wait()
+		})
+	}
+}