fix(notifications): wire per-recipient mute + List-Unsubscribe into production sender (refs #297)

The per-recipient mute + List-Unsubscribe feature was dead in production: the
sender returned by email.NewSenderFromEnvironment was never decorated with the
mute checker or the unsubscribe base URL, so the bare sender's mute checker was
always nil and no List-Unsubscribe header was ever emitted. The SMTP transport
bypassed the mute logic entirely.

- app.go: decorate the factory-produced sender via a new decorateSenderWithMute
  helper that type-switches on *email.Sender / *email.SMTPSender and calls
  WithMuteChecker(configStore) + WithUnsubscribeBaseURL(trimmed DASHBOARD_URL).
  configStore (config.StoreInterface) satisfies email.MuteChecker via
  IsNotificationMuted. The base URL reuses the same DASHBOARD_URL the email
  templates already use, trimmed to match resolveOIDCIssuerURL.
- email/mute.go: extract the transport-agnostic mute + List-Unsubscribe
  decision logic (isRecipientMuted, filterMutedRecipients,
  unsubscribeURLFor, unsubscribeHeaderValuesFor) into shared free functions so
  the SES and SMTP paths cannot diverge; *Sender delegates to them.
- email/smtp_sender.go: add muteChecker + unsubscribeBaseURL fields and
  WithMuteChecker/WithUnsubscribeBaseURL mirroring *Sender, and apply the mute
  skip + CC filter + RFC 8058 List-Unsubscribe headers in
  SendPurchaseApprovalRequest.
- Wiring tests exercise the real factory/decoration path (decorateSenderWithMute
  on a factory-shaped sender) and the SMTP transport: a muted recipient is
  suppressed and the List-Unsubscribe header is emitted with the wired base URL.
  Both fail against the pre-wiring behavior (nil checker, no base URL).

Author: cristim

internal/email/mute.go

@@ -0,0 +1,72 @@
+package email
+
+import (
+	"context"
+	"net/url"
+
+	"github.com/LeanerCloud/CUDly/pkg/common"
+	"github.com/LeanerCloud/CUDly/pkg/logging"
+)
+
+// This file holds the transport-agnostic mute + List-Unsubscribe logic shared
+// by the SES (*Sender) and SMTP (*SMTPSender) paths. Both transports must apply
+// the same per-recipient mute suppression, CC filtering, and RFC 8058
+// List-Unsubscribe headers; keeping the logic here (instead of duplicating it
+// per transport) prevents the two paths from diverging.
+
+// isRecipientMuted returns true when (email, scope) is muted. A nil checker or a
+// store error is treated as "not muted" (fail-open) so a transient DB outage
+// does not silently block approval emails.
+func isRecipientMuted(ctx context.Context, mc MuteChecker, email, scope string) bool {
+	if mc == nil {
+		return false
+	}
+	muted, err := mc.IsNotificationMuted(ctx, email, scope)
+	if err != nil {
+		logging.Warnf("email: mute check failed for scope=%s: %v", scope, err)
+		return false
+	}
+	return muted
+}
+
+// filterMutedRecipients returns a copy of addrs with any address muted for scope
+// removed. The original slice is not modified. Errors from the mute store are
+// treated as "not muted" (fail-open).
+func filterMutedRecipients(ctx context.Context, mc MuteChecker, addrs []string, scope string) []string {
+	if mc == nil || len(addrs) == 0 {
+		return addrs
+	}
+	out := make([]string, 0, len(addrs))
+	for _, addr := range addrs {
+		if !isRecipientMuted(ctx, mc, addr, scope) {
+			out = append(out, addr)
+		}
+	}
+	return out
+}
+
+// unsubscribeURLFor constructs the one-click unsubscribe URL for the given
+// (email, scope) pair. Returns "" when baseURL is empty.
+func unsubscribeURLFor(baseURL, email, scope string) string {
+	if baseURL == "" {
+		return ""
+	}
+	token := common.DeriveMuteToken(muteKey(), email, scope)
+	q := url.Values{
+		"token": {token},
+		"email": {email},
+		"scope": {scope},
+	}
+	return baseURL + "/api/notifications/unsubscribe?" + q.Encode()
+}
+
+// unsubscribeHeaderValuesFor returns the List-Unsubscribe and
+// List-Unsubscribe-Post header values (RFC 8058) for the given (email, scope)
+// pair. Returns ("", "") when baseURL is empty.
+func unsubscribeHeaderValuesFor(baseURL, email, scope string) (headerValue, postValue string) {
+	unsubURL := unsubscribeURLFor(baseURL, email, scope)
+	if unsubURL == "" {
+		return "", ""
+	}
+	return "<" + unsubURL + ">", "List-Unsubscribe=One-Click"
+}

internal/email/sender.go

@@ -5,11 +5,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net/url"
 	"os"
 	"strings"
 
-	"github.com/LeanerCloud/CUDly/pkg/common"
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 	"github.com/aws/aws-sdk-go-v2/aws"
 	awsconfig "github.com/aws/aws-sdk-go-v2/config"
@@ -140,60 +138,29 @@ func muteKey() []byte {
 // buildUnsubscribeURL constructs the one-click unsubscribe URL for the given
 // (email, scope) pair. Returns ("", "") when unsubscribeBaseURL is empty.
 func (s *Sender) buildUnsubscribeURL(email, scope string) (unsubURL, mailtoURL string) {
-	if s.unsubscribeBaseURL == "" {
-		return "", ""
-	}
-	token := common.DeriveMuteToken(muteKey(), email, scope)
-	q := url.Values{
-		"token": {token},
-		"email": {email},
-		"scope": {scope},
-	}
-	unsubURL = s.unsubscribeBaseURL + "/api/notifications/unsubscribe?" + q.Encode()
-	return unsubURL, ""
+	return unsubscribeURLFor(s.unsubscribeBaseURL, email, scope), ""
 }
 
 // listUnsubscribeHeaders returns the List-Unsubscribe and List-Unsubscribe-Post
 // header values for the given (email, scope) pair (RFC 8058).
 // Returns ("", "") when no base URL is configured.
 func (s *Sender) listUnsubscribeHeaders(email, scope string) (headerValue, postValue string) {
-	unsubURL, _ := s.buildUnsubscribeURL(email, scope)
-	if unsubURL == "" {
-		return "", ""
-	}
-	return "<" + unsubURL + ">", "List-Unsubscribe=One-Click"
+	return unsubscribeHeaderValuesFor(s.unsubscribeBaseURL, email, scope)
 }
 
 // isMuted returns true when the given address is muted for this scope. When the
 // mute checker is nil or returns an error the address is treated as not muted so
 // a transient DB outage doesn't silently block approval emails.
 func (s *Sender) isMuted(ctx context.Context, email, scope string) bool {
-	if s.muteChecker == nil {
-		return false
-	}
-	muted, err := s.muteChecker.IsNotificationMuted(ctx, email, scope)
-	if err != nil {
-		logging.Warnf("email: mute check failed for scope=%s: %v", scope, err)
-		return false
-	}
-	return muted
+	return isRecipientMuted(ctx, s.muteChecker, email, scope)
 }
 
 // filterMutedAddresses returns a copy of addrs with any muted (for scope)
 // entries removed. The original slice is not modified. Errors from the mute
 // store are treated as "not muted" (fail-open) so a DB hiccup does not
 // silently suppress approval emails.
 func (s *Sender) filterMutedAddresses(ctx context.Context, addrs []string, scope string) []string {
-	if s.muteChecker == nil || len(addrs) == 0 {
-		return addrs
-	}
-	out := make([]string, 0, len(addrs))
-	for _, addr := range addrs {
-		if !s.isMuted(ctx, addr, scope) {
-			out = append(out, addr)
-		}
-	}
-	return out
+	return filterMutedRecipients(ctx, s.muteChecker, addrs, scope)
 }
 
 // SendNotification sends a notification email via SNS

internal/email/smtp_mute_test.go

@@ -0,0 +1,91 @@
+package email
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/LeanerCloud/CUDly/pkg/common"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// smtpApprovalData builds a minimal purchase-approval NotificationData.
+func smtpApprovalData(recipient string) NotificationData {
+	return NotificationData{
+		RecipientEmail: recipient,
+		DashboardURL:   "https://dash.example.com",
+		ApprovalToken:  "tok",
+		Recommendations: []RecommendationSummary{
+			{Service: "ec2", Region: "us-east-1", Count: 1, MonthlySavings: 100},
+		},
+	}
+}
+
+// TestSMTPSender_PurchaseApproval_MutedRecipient_NoSend verifies the SMTP
+// transport applies the same per-recipient mute suppression as SES: a muted
+// recipient never reaches the wire (no DATA section is transmitted).
+func TestSMTPSender_PurchaseApproval_MutedRecipient_NoSend(t *testing.T) {
+	server := newMockSMTPServer(t, false)
+	server.start(t)
+	defer server.stop()
+
+	base := &SMTPSender{
+		host:      "127.0.0.1",
+		port:      server.port,
+		fromEmail: "sender@test.com",
+		useTLS:    false,
+	}
+	mc := &wiringMuteCheckerSMTP{muted: map[string]bool{"muted@test.com": true}}
+	sender := base.WithMuteChecker(mc).WithUnsubscribeBaseURL("https://dash.example.com")
+
+	err := sender.SendPurchaseApprovalRequest(context.Background(), smtpApprovalData("muted@test.com"))
+	require.NoError(t, err)
+
+	server.stop() // flush the connection goroutine before reading receivedMsg
+	server.mu.Lock()
+	got := server.receivedMsg
+	server.mu.Unlock()
+	assert.NotContains(t, got, "Purchase Approval Required",
+		"muted recipient must not have a message body transmitted over SMTP")
+}
+
+// TestSMTPSender_PurchaseApproval_EmitsListUnsubscribe verifies the SMTP
+// approval send attaches the RFC 8058 List-Unsubscribe header sourced from the
+// wired-in unsubscribe base URL.
+func TestSMTPSender_PurchaseApproval_EmitsListUnsubscribe(t *testing.T) {
+	server := newMockSMTPServer(t, false)
+	server.start(t)
+	defer server.stop()
+
+	base := &SMTPSender{
+		host:      "127.0.0.1",
+		port:      server.port,
+		fromEmail: "sender@test.com",
+		useTLS:    false,
+	}
+	mc := &wiringMuteCheckerSMTP{muted: map[string]bool{}}
+	sender := base.WithMuteChecker(mc).WithUnsubscribeBaseURL("https://dash.example.com")
+
+	err := sender.SendPurchaseApprovalRequest(context.Background(), smtpApprovalData("approver@test.com"))
+	require.NoError(t, err)
+
+	server.stop()
+	server.mu.Lock()
+	got := server.receivedMsg
+	server.mu.Unlock()
+	assert.Contains(t, got, "List-Unsubscribe:",
+		"SMTP approval send must carry a List-Unsubscribe header")
+	assert.Contains(t, got, "https://dash.example.com/api/notifications/unsubscribe")
+	assert.Contains(t, got, "scope="+string(common.ScopePurchaseApprovals))
+	assert.True(t, strings.Contains(got, "List-Unsubscribe-Post:"),
+		"SMTP approval send must carry a List-Unsubscribe-Post header")
+}
+
+type wiringMuteCheckerSMTP struct {
+	muted map[string]bool
+}
+
+func (w *wiringMuteCheckerSMTP) IsNotificationMuted(_ context.Context, recipientEmail, _ string) (bool, error) {
+	return w.muted[recipientEmail], nil
+}