Repository: axllent/mailpit

Issue:
Add switch to modify behavior for not accepting mails
We use mailpit in a test environment and have a restriction on the domain for which mailpit accepts emails (--smtp-allowed-recipients).

Some standard-software and custom developed software is not too happy with receiving a "550 5.1.0 Requested action not taken: mailbox unavailable" if the address is not on the allowlist. 

Those software packages expect a forwarding relay on the other end that simply takes all emails and returns a 2xx and then forwards the mails asynchronously (and potentially fails then).

Would you be willing to accept a PR for a switch that enables for mails to be just accepted with a 2xx code and then dropped silently instead of receiving a 550? 

Discussion:
@axllent: Hi @gliwka. If I understand you correctly, you effectively want a new feature with something like `--smtp-drop-rejected-recipients` to effectively accept the email and then just delete it? I'm not sure that's the best flag name due to the length of it, but you may have a better suggestion.

Assuming I am correct, I'm not sure this is possible, at least not easily and not without rewriting how the smtpd library works. I haven't looked too deep into it, but from what I can see the smtpd uses a `handlerRcpt()` function which returns a bool, and this function determines whether the `SMTPAllowedRecipientsRegexp` (the compiled regular expression based on the `--smtp-allowed-recipients` flag) exists and matches the address.

I think this could be a useful addition to Mailpit, so I would be willing to **consider** a PR for this if you are able to find an elegant way to integrate this in a matter that does not require a major hack to the smtpd library. The upside is that the smtpd library is included in Mailpit (not a third-party library any more as I needed to make some other minor changes in the past), however I would prefer not to fill it with hacks or completely change how it works. You may think of a better approach though if you're keen on looking into that further.

There are additional complications though which you should be aware of and which need to be considered, for instance auto-relaying & auto-forwarding of messages which obviously should not be run on ignored messages either.  Most of the core functionality you would be looking at would be found in `internal/smtpd/main.go` and `internal/smtpd/smtd.go`.@gliwka: Thank you for the pointers. Yes, I propose to introduce the functionality with a few isolated changes to the main loop of smtpd:

- Introduce the command line param (--smtp-drop-rejected-recipients)
- Give the smtp server struct that param as boolean (SilentlyDropRejectedRecipients)
- After the call of HandlerRcpt, short-circuit the evaluation of the accepted flag if SilentlyDropRejectedRecipients is set (https://github.com/axllent/mailpit/blob/009f3a8fd967cbbebbafdf5478eade0320756ecd/internal/smtpd/smtpd.go#L489)
- Add checks for the accepted flag before the handler gets called, so the logic falls through into the last case which doesn't call the handler and just prints `250 Queued` https://github.com/axllent/mailpit/blob/009f3a8fd967cbbebbafdf5478eade0320756ecd/internal/smtpd/smtpd.go#L540 https://github.com/axllent/mailpit/blob/009f3a8fd967cbbebbafdf5478eade0320756ecd/internal/smtpd/smtpd.go#L552
- Add logging to that else case to make it clear that the handler has been skipped
https://github.com/axllent/mailpit/blob/009f3a8fd967cbbebbafdf5478eade0320756ecd/internal/smtpd/smtpd.go#L570

I will open a PR with those changes and the neccesary covering tests. 

All the relaying, forwarding and saving etc. happens in the handlers, so it will be enough to just not execute them if accepted is not "true".@github-actions: This issue has been marked as stale because it has been open for 7 days with no activity.@gliwka: PR will open this week, got busy, but we still need this.@github-actions: This issue has been marked as stale because it has been open for 7 days with no activity.@github-actions: This issue was closed because there has been no activity since being marked as stale.@gliwka: Finally got around to implementing it here: https://github.com/axllent/mailpit/pull/549@axllent: Released in [v1.24.7](https://github.com/axllent/mailpit/releases/tag/v1.27.4).

Solution patch:
diff --git a/cmd/root.go b/cmd/root.go
index e8912cf..27a1370 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -126,6 +126,7 @@ func init() {
 	rootCmd.Flags().BoolVar(&config.SMTPStrictRFCHeaders, "smtp-strict-rfc-headers", config.SMTPStrictRFCHeaders, "Return SMTP error if message headers contain <CR><CR><LF>")
 	rootCmd.Flags().IntVar(&config.SMTPMaxRecipients, "smtp-max-recipients", config.SMTPMaxRecipients, "Maximum SMTP recipients allowed")
 	rootCmd.Flags().StringVar(&config.SMTPAllowedRecipients, "smtp-allowed-recipients", config.SMTPAllowedRecipients, "Only allow SMTP recipients matching a regular expression (default allow all)")
+	rootCmd.Flags().BoolVar(&config.SMTPSilentlyDropRejectedRecipients, "smtp-silently-drop-rejected-recipients", config.SMTPSilentlyDropRejectedRecipients, "Accept emails to rejected recipients with 2xx response but silently drop them")
 	rootCmd.Flags().BoolVar(&smtpd.DisableReverseDNS, "smtp-disable-rdns", smtpd.DisableReverseDNS, "Disable SMTP reverse DNS lookups")
 
 	// SMTP relay
@@ -301,6 +302,9 @@ func initConfigFromEnv() {
 	if len(os.Getenv("MP_SMTP_ALLOWED_RECIPIENTS")) > 0 {
 		config.SMTPAllowedRecipients = os.Getenv("MP_SMTP_ALLOWED_RECIPIENTS")
 	}
+	if getEnabledFromEnv("MP_SMTP_SILENTLY_DROP_REJECTED_RECIPIENTS") {
+		config.SMTPSilentlyDropRejectedRecipients = true
+	}
 	if getEnabledFromEnv("MP_SMTP_DISABLE_RDNS") {
 		smtpd.DisableReverseDNS = true
 	}
diff --git a/config/config.go b/config/config.go
index f5d54cf..fcba310 100644
--- a/config/config.go
+++ b/config/config.go
@@ -181,6 +181,9 @@ var (
 	// SMTPAllowedRecipientsRegexp is the compiled version of SMTPAllowedRecipients
 	SMTPAllowedRecipientsRegexp *regexp.Regexp
 
+	// SMTPSilentlyDropRejectedRecipients if true, will accept emails to rejected recipients with 2xx response but silently drop them
+	SMTPSilentlyDropRejectedRecipients bool
+
 	// POP3Listen address - if set then Mailpit will start the POP3 server and listen on this address
 	POP3Listen = "[::]:1110"
 
diff --git a/internal/smtpd/main.go b/internal/smtpd/main.go
index ca61771..f093e8b 100644
--- a/internal/smtpd/main.go
+++ b/internal/smtpd/main.go
@@ -194,15 +194,16 @@ func listenAndServe(addr string, handler MsgIDHandler, authHandler AuthHandler)
 
 	Debug = true // to enable Mailpit logging
 	srv := &Server{
-		Addr:              addr,
-		MsgIDHandler:      handler,
-		HandlerRcpt:       handlerRcpt,
-		AppName:           "Mailpit",
-		Hostname:          "",
-		AuthHandler:       nil,
-		AuthRequired:      false,
-		MaxRecipients:     config.SMTPMaxRecipients,
-		DisableReverseDNS: DisableReverseDNS,
+		Addr:                           addr,
+		MsgIDHandler:                   handler,
+		HandlerRcpt:                    handlerRcpt,
+		AppName:                        "Mailpit",
+		Hostname:                       "",
+		AuthHandler:                    nil,
+		AuthRequired:                   false,
+		MaxRecipients:                  config.SMTPMaxRecipients,
+		SilentlyDropRejectedRecipients: config.SMTPSilentlyDropRejectedRecipients,
+		DisableReverseDNS:              DisableReverseDNS,
 		LogRead: func(remoteIP, verb, line string) {
 			logger.Log().Debugf("[smtpd] %s (%s) %s", verbLogTranslator(verb), remoteIP, line)
 		},
diff --git a/internal/smtpd/smtpd.go b/internal/smtpd/smtpd.go
index 29cb30b..4ed7223 100644
--- a/internal/smtpd/smtpd.go
+++ b/internal/smtpd/smtpd.go
@@ -92,26 +92,27 @@ type LogFunc func(remoteIP, verb, line string)
 
 // Server is an SMTP server.
 type Server struct {
-	Addr              string // TCP address to listen on, defaults to ":25" (all addresses, port 25) if empty
-	AppName           string
-	AuthHandler       AuthHandler
-	AuthMechs         map[string]bool // Override list of allowed authentication mechanisms. Currently supported: LOGIN, PLAIN, CRAM-MD5. Enabling LOGIN and PLAIN will reduce RFC 4954 compliance.
-	AuthRequired      bool            // Require authentication for every command except AUTH, EHLO, HELO, NOOP, RSET or QUIT as per RFC 4954. Ignored if AuthHandler is not configured.
-	DisableReverseDNS bool            // Disable reverse DNS lookups, enforces "unknown" hostname
-	Handler           Handler
-	HandlerRcpt       HandlerRcpt
-	Hostname          string
-	LogRead           LogFunc
-	LogWrite          LogFunc
-	MaxSize           int // Maximum message size allowed, in bytes
-	MaxRecipients     int // Maximum number of recipients, defaults to 100.
-	MsgIDHandler      MsgIDHandler
-	Timeout           time.Duration
-	TLSConfig         *tls.Config
-	TLSListener       bool        // Listen for incoming TLS connections only (not recommended as it may reduce compatibility). Ignored if TLS is not configured.
-	TLSRequired       bool        // Require TLS for every command except NOOP, EHLO, STARTTLS, or QUIT as per RFC 3207. Ignored if TLS is not configured.
-	Protocol          string      // Default tcp, supports unix
-	SocketPerm        fs.FileMode // if using Unix socket, socket permissions
+	Addr                           string // TCP address to listen on, defaults to ":25" (all addresses, port 25) if empty
+	AppName                        string
+	AuthHandler                    AuthHandler
+	AuthMechs                      map[string]bool // Override list of allowed authentication mechanisms. Currently supported: LOGIN, PLAIN, CRAM-MD5. Enabling LOGIN and PLAIN will reduce RFC 4954 compliance.
+	AuthRequired                   bool            // Require authentication for every command except AUTH, EHLO, HELO, NOOP, RSET or QUIT as per RFC 4954. Ignored if AuthHandler is not configured.
+	DisableReverseDNS              bool            // Disable reverse DNS lookups, enforces "unknown" hostname
+	Handler                        Handler
+	HandlerRcpt                    HandlerRcpt
+	Hostname                       string
+	LogRead                        LogFunc
+	LogWrite                       LogFunc
+	MaxSize                        int // Maximum message size allowed, in bytes
+	MaxRecipients                  int // Maximum number of recipients, defaults to 100.
+	MsgIDHandler                   MsgIDHandler
+	SilentlyDropRejectedRecipients bool // Accept emails to rejected recipients with 2xx response but silently drop them
+	Timeout                        time.Duration
+	TLSConfig                      *tls.Config
+	TLSListener                    bool        // Listen for incoming TLS connections only (not recommended as it may reduce compatibility). Ignored if TLS is not configured.
+	TLSRequired                    bool        // Require TLS for every command except NOOP, EHLO, STARTTLS, or QUIT as per RFC 3207. Ignored if TLS is not configured.
+	Protocol                       string      // Default tcp, supports unix
+	SocketPerm                     fs.FileMode // if using Unix socket, socket permissions
 
 	inShutdown   int32 // server was closed or shutdown
 	openSessions int32 // count of open sessions
@@ -363,6 +364,7 @@ func (s *session) serve() {
 	var from string
 	var gotFrom bool
 	var to []string
+	var hasRejectedRecipients bool
 	var buffer bytes.Buffer
 
 	// RFC 5321 specifies support for minimum of 100 recipients is required.
@@ -397,6 +399,7 @@ loop:
 			from = ""
 			gotFrom = false
 			to = nil
+			hasRejectedRecipients = false
 			buffer.Reset()
 		case "EHLO":
 			s.remoteName = args
@@ -406,6 +409,7 @@ loop:
 			from = ""
 			gotFrom = false
 			to = nil
+			hasRejectedRecipients = false
 			buffer.Reset()
 		case "MAIL":
 			if s.srv.TLSConfig != nil && s.srv.TLSRequired && !s.tls {
@@ -457,6 +461,7 @@ loop:
 			}
 
 			to = nil
+			hasRejectedRecipients = false
 			buffer.Reset()
 		case "RCPT":
 			if s.srv.TLSConfig != nil && s.srv.TLSRequired && !s.tls {
@@ -492,6 +497,9 @@ loop:
 					if accept {
 						to = append(to, match[1])
 						s.writef("250 2.1.5 Ok")
+					} else if s.srv.SilentlyDropRejectedRecipients {
+						hasRejectedRecipients = true
+						s.writef("250 2.1.5 Ok")
 					} else {
 						s.writef("550 5.1.0 Requested action not taken: mailbox unavailable")
 					}
@@ -506,7 +514,8 @@ loop:
 				s.writef("530 5.7.0 Authentication required")
 				break
 			}
-			if !gotFrom || len(to) == 0 {
+			hasRecipients := len(to) > 0 || hasRejectedRecipients
+			if !gotFrom || !hasRecipients {
 				s.writef("503 5.5.1 Bad sequence of commands (MAIL & RCPT required before DATA)")
 				break
 			}
@@ -536,11 +545,13 @@ loop:
 
 			// Create Received header & write message body into buffer.
 			buffer.Reset()
-			buffer.Write(s.makeHeaders(to))
+			if len(to) > 0 {
+				buffer.Write(s.makeHeaders(to))
+			}
 			buffer.Write(data)
 
-			// Pass mail on to handler.
-			if s.srv.Handler != nil {
+			// Pass mail on to handler only if there are valid recipients.
+			if len(to) > 0 && s.srv.Handler != nil {
 				err := s.srv.Handler(s.conn.RemoteAddr(), from, to, buffer.Bytes())
 				if err != nil {
 					checkErrFormat := regexp.MustCompile(`^([2-5][0-9]{2})[\s\-](.+)$`)
@@ -552,7 +563,7 @@ loop:
 					break
 				}
 				s.writef("250 2.0.0 Ok: queued")
-			} else if s.srv.MsgIDHandler != nil {
+			} else if len(to) > 0 && s.srv.MsgIDHandler != nil {
 				msgID, err := s.srv.MsgIDHandler(s.conn.RemoteAddr(), from, to, buffer.Bytes(), s.username)
 				if err != nil {
 					checkErrFormat := regexp.MustCompile(`^([2-5][0-9]{2})[\s\-](.+)$`)
@@ -570,6 +581,13 @@ loop:
 					s.writef("250 2.0.0 Ok: queued")
 				}
 			} else {
+				if hasRejectedRecipients && Debug {
+					if s.srv.LogWrite != nil {
+						s.srv.LogWrite(s.remoteIP, "DEBUG", "Message from sender silently dropped (rejected recipients)")
+					} else {
+						log.Printf("%s DEBUG Message from sender silently dropped (rejected recipients)", s.remoteIP)
+					}
+				}
 				s.writef("250 2.0.0 Ok: queued")
 			}
 
@@ -577,6 +595,7 @@ loop:
 			from = ""
 			gotFrom = false
 			to = nil
+			hasRejectedRecipients = false
 			buffer.Reset()
 		case "QUIT":
 			s.writef("221 2.0.0 %s %s ESMTP Service closing transmission channel", s.srv.Hostname, s.srv.AppName)
@@ -590,6 +609,7 @@ loop:
 			from = ""
 			gotFrom = false
 			to = nil
+			hasRejectedRecipients = false
 			buffer.Reset()
 		case "NOOP":
 			s.writef("250 2.0.0 Ok")
@@ -666,6 +686,7 @@ loop:
 			from = ""
 			gotFrom = false
 			to = nil
+			hasRejectedRecipients = false
 			buffer.Reset()
 		case "AUTH":
 			if s.srv.TLSConfig != nil && s.srv.TLSRequired && !s.tls {



Test patch:
diff --git a/internal/smtpd/smtpd_test.go b/internal/smtpd/smtpd_test.go
index 0f75482..37a9e84 100644
--- a/internal/smtpd/smtpd_test.go
+++ b/internal/smtpd/smtpd_test.go
@@ -1598,3 +1598,172 @@ func TestCmdShutdown(t *testing.T) {
 
 	_ = conn.Close()
 }
+
+type mockDropRejectedHandler struct {
+	handlerCalled int
+	lastFrom      string
+	lastTo        []string
+	msgIDCalled   int
+	lastMsgIDFrom string
+	lastMsgIDTo   []string
+}
+
+func (m *mockDropRejectedHandler) handler(remoteAddr net.Addr, from string, to []string, data []byte) error {
+	m.handlerCalled++
+	m.lastFrom = from
+	m.lastTo = append([]string{}, to...) // copy slice
+	return nil
+}
+
+func (m *mockDropRejectedHandler) msgIDHandler(remoteAddr net.Addr, from string, to []string, data []byte, username *string) (string, error) {
+	m.msgIDCalled++
+	m.lastMsgIDFrom = from
+	m.lastMsgIDTo = append([]string{}, to...) // copy slice
+	return "test-message-id", nil
+}
+
+// Test the SilentlyDropRejectedRecipients option
+func TestSilentlyDropRejectedRecipients(t *testing.T) {
+	tests := []struct {
+		name                           string
+		silentlyDropRejectedRecipients bool
+		handlerRcpt                    func(net.Addr, string, string) bool
+		rcptCommands                   []struct{ addr, expectedCode string }
+		expectedHandlerCalls           int
+		expectedHandlerRecipients      []string
+		useMsgIDHandler                bool
+	}{
+		{
+			name:                           "Disabled_DefaultBehavior",
+			silentlyDropRejectedRecipients: false,
+			handlerRcpt: func(remoteAddr net.Addr, from string, to string) bool {
+				return !strings.HasSuffix(to, "@rejected.com")
+			},
+			rcptCommands: []struct{ addr, expectedCode string }{
+				{"valid@example.com", "250"},
+				{"invalid@rejected.com", "550"},
+			},
+			expectedHandlerCalls:      1,
+			expectedHandlerRecipients: []string{"valid@example.com"},
+		},
+		{
+			name:                           "Enabled_MixedRecipients",
+			silentlyDropRejectedRecipients: true,
+			handlerRcpt: func(remoteAddr net.Addr, from string, to string) bool {
+				return !strings.HasSuffix(to, "@rejected.com")
+			},
+			rcptCommands: []struct{ addr, expectedCode string }{
+				{"valid1@example.com", "250"},
+				{"valid2@example.com", "250"},
+				{"invalid1@rejected.com", "250"}, // Now accepted but dropped
+				{"invalid2@rejected.com", "250"}, // Now accepted but dropped
+			},
+			expectedHandlerCalls:      1,
+			expectedHandlerRecipients: []string{"valid1@example.com", "valid2@example.com"},
+		},
+		{
+			name:                           "Enabled_AllRejected",
+			silentlyDropRejectedRecipients: true,
+			handlerRcpt: func(remoteAddr net.Addr, from string, to string) bool {
+				return false // Reject all
+			},
+			rcptCommands: []struct{ addr, expectedCode string }{
+				{"test1@example.com", "250"}, // Accepted but dropped
+				{"test2@example.com", "250"}, // Accepted but dropped
+			},
+			expectedHandlerCalls:      0, // No handler calls since all rejected
+			expectedHandlerRecipients: nil,
+		},
+		{
+			name:                           "Enabled_OnlyValid",
+			silentlyDropRejectedRecipients: true,
+			handlerRcpt: func(remoteAddr net.Addr, from string, to string) bool {
+				return strings.HasSuffix(to, "@valid.com")
+			},
+			rcptCommands: []struct{ addr, expectedCode string }{
+				{"user1@valid.com", "250"},
+				{"user2@valid.com", "250"},
+				{"user3@valid.com", "250"},
+			},
+			expectedHandlerCalls:      1,
+			expectedHandlerRecipients: []string{"user1@valid.com", "user2@valid.com", "user3@valid.com"},
+		},
+		{
+			name:                           "Enabled_WithMsgIDHandler",
+			silentlyDropRejectedRecipients: true,
+			handlerRcpt: func(remoteAddr net.Addr, from string, to string) bool {
+				return !strings.HasSuffix(to, "@rejected.com")
+			},
+			rcptCommands: []struct{ addr, expectedCode string }{
+				{"valid@example.com", "250"},
+				{"invalid@rejected.com", "250"}, // Accepted but dropped
+			},
+			expectedHandlerCalls:      1,
+			expectedHandlerRecipients: []string{"valid@example.com"},
+			useMsgIDHandler:           true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mock := &mockDropRejectedHandler{}
+
+			server := &Server{
+				Hostname:                       "mail.example.com",
+				AppName:                        "TestMail",
+				MaxRecipients:                  100,
+				HandlerRcpt:                    tt.handlerRcpt,
+				SilentlyDropRejectedRecipients: tt.silentlyDropRejectedRecipients,
+			}
+
+			if tt.useMsgIDHandler {
+				server.MsgIDHandler = mock.msgIDHandler
+			} else {
+				server.Handler = mock.handler
+			}
+
+			conn := newConn(t, server)
+			defer func() { _ = conn.Close() }()
+
+			cmdCode(t, conn, "HELO host.example.com", "250")
+			cmdCode(t, conn, "MAIL FROM:<sender@example.com>", "250")
+
+			// Send RCPT commands
+			for _, rcpt := range tt.rcptCommands {
+				cmdCode(t, conn, "RCPT TO:<"+rcpt.addr+">", rcpt.expectedCode)
+			}
+
+			// Send DATA
+			cmdCode(t, conn, "DATA", "354")
+			cmdCode(t, conn, "Subject: Test\r\n\r\nTest message\r\n.", "250")
+			cmdCode(t, conn, "QUIT", "221")
+
+			// Verify handler calls
+			if tt.useMsgIDHandler {
+				if mock.msgIDCalled != tt.expectedHandlerCalls {
+					t.Errorf("Expected %d MsgIDHandler calls, got %d", tt.expectedHandlerCalls, mock.msgIDCalled)
+				}
+				if tt.expectedHandlerCalls > 0 {
+					if mock.lastMsgIDFrom != "sender@example.com" {
+						t.Errorf("Expected from 'sender@example.com', got '%s'", mock.lastMsgIDFrom)
+					}
+					if !reflect.DeepEqual(mock.lastMsgIDTo, tt.expectedHandlerRecipients) {
+						t.Errorf("Expected recipients %v, got %v", tt.expectedHandlerRecipients, mock.lastMsgIDTo)
+					}
+				}
+			} else {
+				if mock.handlerCalled != tt.expectedHandlerCalls {
+					t.Errorf("Expected %d handler calls, got %d", tt.expectedHandlerCalls, mock.handlerCalled)
+				}
+				if tt.expectedHandlerCalls > 0 {
+					if mock.lastFrom != "sender@example.com" {
+						t.Errorf("Expected from 'sender@example.com', got '%s'", mock.lastFrom)
+					}
+					if !reflect.DeepEqual(mock.lastTo, tt.expectedHandlerRecipients) {
+						t.Errorf("Expected recipients %v, got %v", tt.expectedHandlerRecipients, mock.lastTo)
+					}
+				}
+			}
+		})
+	}
+}