feat(email): multipart HTML+plaintext for RI exchange pending approval (closes #296)

internal/email/coverage_extra_test.go

@@ -323,24 +323,30 @@ func TestRenderRegistrationReceivedEmail_NoAdminApprovers(t *testing.T) {
 
 // Tests for SMTPSender RI exchange and approval request methods
 
-func TestSMTPSender_SendRIExchangePendingApproval_NoFromEmail(t *testing.T) {
+func TestSMTPSender_SendRIExchangePendingApproval_NoRecipient(t *testing.T) {
+	// The RI exchange approval body carries live approval tokens, so with no
+	// data.RecipientEmail and no notifyEmail fallback the SMTP sender must
+	// surface ErrNoRecipient rather than silently dropping (or broadcasting)
+	// the email. Mirrors the hardened SendPurchaseApprovalRequest contract
+	// (#1015 / #1036) and the multipart path added in #296.
 	sender := &SMTPSender{
-		host:      "smtp.example.com",
-		port:      587,
-		fromEmail: "",
+		host:        "smtp.example.com",
+		port:        587,
+		fromEmail:   "",
+		notifyEmail: "",
 	}
 
 	data := RIExchangeNotificationData{
 		DashboardURL: "https://dashboard.example.com",
 		TotalPayment: "100.00",
 		Exchanges: []RIExchangeItem{
-			{RecordID: "r1", SourceRIID: "ri-1", SourceInstanceType: "m5.large",
+			{RecordID: "r1", ApprovalToken: "tok-1", SourceRIID: "ri-1", SourceInstanceType: "m5.large",
 				TargetInstanceType: "m5.xlarge", TargetCount: 1, PaymentDue: "100.00"},
 		},
 	}
 
 	err := sender.SendRIExchangePendingApproval(context.Background(), data)
-	require.NoError(t, err)
+	require.ErrorIs(t, err, ErrNoRecipient)
 }
 
 func TestSMTPSender_SendRIExchangeCompleted_NoFromEmail(t *testing.T) {

internal/email/sender.go

@@ -480,6 +480,18 @@ type RIExchangeNotificationData struct {
 	// CCEmails carries additional recipients informed of the pending exchanges
 	// but not the authorized approvers. Deduplicated against RecipientEmail.
 	CCEmails []string
+	// RequestedByName is the human-readable display name of the user who
+	// triggered the exchange run. Empty falls back to RequestedByEmail.
+	RequestedByName string
+	// RequestedByEmail is the requester's email address. Empty omits the
+	// requested-by block from the approval email.
+	RequestedByEmail string
+	// RequestedAt is the ISO-8601 / RFC3339 timestamp the exchange was
+	// submitted. Empty omits the timestamp from the summary.
+	RequestedAt string
+	// CancellationWindowNote is short text rendered below the approve/reject
+	// buttons. Empty falls back to a generic 6-hour note.
+	CancellationWindowNote string
 }
 
 // RIExchangeItem represents a single exchange in an email notification.

internal/email/smtp_sender.go

@@ -403,14 +403,22 @@ func (s *SMTPSender) SendPurchaseFailedNotification(ctx context.Context, data No
 	return s.SendToEmail(ctx, s.notifyEmail, subject, body)
 }
 
-// SendRIExchangePendingApproval sends an RI exchange approval email via SMTP.
+// SendRIExchangePendingApproval sends an RI exchange approval email via SMTP
+// as multipart/alternative (plain-text + styled HTML). The body carries live
+// approval tokens, so it is sent only to the resolved recipient (the
+// submitter's notification email, falling back to the static SMTP notify
+// address) plus the deduplicated CC list; it never broadcasts. Returns
+// ErrNoRecipient when neither address is configured. Issue #296.
 func (s *SMTPSender) SendRIExchangePendingApproval(ctx context.Context, data RIExchangeNotificationData) error {
-	subject := fmt.Sprintf("CUDly - RI Exchange Approval Required (%d exchanges)", len(data.Exchanges))
-	body, err := RenderRIExchangePendingApprovalEmail(data)
-	if err != nil {
-		return fmt.Errorf("failed to render ri exchange pending approval email: %w", err)
+	recipient := data.RecipientEmail
+	if recipient == "" {
+		recipient = s.notifyEmail
 	}
-	return s.SendToEmail(ctx, s.notifyEmail, subject, body)
+	if recipient == "" {
+		return ErrNoRecipient
+	}
+	subject := fmt.Sprintf("CUDly - RI Exchange Approval Required (%d exchanges)", len(data.Exchanges))
+	return sendRIExchangePendingApprovalVia(ctx, s, recipient, data.CCEmails, subject, data)
 }
 
 // SendRIExchangeCompleted sends an RI exchange completion email via SMTP.

internal/email/template_renderers.go

@@ -131,11 +131,22 @@ func RenderPurchaseFailedEmail(data NotificationData) (string, error) {
 	return renderTextTemplate("failed", purchaseFailedTemplate, data)
 }
 
-// RenderRIExchangePendingApprovalEmail renders the plain-text RI exchange pending approval email template.
+// RenderRIExchangePendingApprovalEmail renders the plain-text RI exchange
+// pending approval email template. Pair with
+// RenderRIExchangePendingApprovalEmailHTML for multipart/alternative delivery.
 func RenderRIExchangePendingApprovalEmail(data RIExchangeNotificationData) (string, error) {
 	return renderTextTemplate("ri-exchange-pending", riExchangePendingApprovalTemplate, data)
 }
 
+// RenderRIExchangePendingApprovalEmailHTML renders the HTML half of the RI
+// exchange pending approval email -- inline-styled per email-client constraints.
+// The plain-text half (RenderRIExchangePendingApprovalEmail) carries the same
+// content; receiving clients pick whichever they support via the
+// multipart/alternative wrapper assembled by the sender. Issue #296.
+func RenderRIExchangePendingApprovalEmailHTML(data RIExchangeNotificationData) (string, error) {
+	return renderTemplate("ri-exchange-pending-html", riExchangePendingApprovalHTMLTemplate, data)
+}
+
 // RenderRIExchangeCompletedEmail renders the plain-text RI exchange completed email template.
 func RenderRIExchangeCompletedEmail(data RIExchangeNotificationData) (string, error) {
 	return renderTextTemplate("ri-exchange-completed", riExchangeCompletedTemplate, data)

internal/email/template_renderers_test.go

@@ -570,3 +570,223 @@ func TestPlainTextTemplates_NoHTMLEscaping(t *testing.T) {
 		assert.NotContains(t, html, xssName, "html/template must escape <script> tags in HTML bodies")
 	})
 }
+
+// Issue #296: plain-text RI exchange pending approval email carries the
+// enriched summary fields (requested-by, cancellation-window note) and uses
+// labeled Approve/Reject URLs instead of bracket-wrapped actions.
+func TestRenderRIExchangePendingApprovalEmail_Issue296(t *testing.T) {
+	data := RIExchangeNotificationData{
+		DashboardURL:           "https://dashboard.example.com",
+		TotalPayment:           "125.50",
+		RequestedByName:        "Cristi M",
+		RequestedByEmail:       "cristi@acme.com",
+		RequestedAt:            "2026-05-22T10:00:00Z",
+		CancellationWindowNote: "RI exchanges are irreversible once accepted by AWS.",
+		Exchanges: []RIExchangeItem{
+			{
+				RecordID:           "rec-001",
+				ApprovalToken:      "tok-abc",
+				SourceRIID:         "ri-0123456789abcdef0",
+				SourceInstanceType: "m5.large",
+				TargetInstanceType: "m6i.large",
+				TargetCount:        1,
+				PaymentDue:         "125.50",
+				UtilizationPct:     42.5,
+			},
+		},
+	}
+
+	body, err := RenderRIExchangePendingApprovalEmail(data)
+	require.NoError(t, err)
+
+	// Exchange details.
+	assert.Contains(t, body, "ri-0123456789abcdef0")
+	assert.Contains(t, body, "m5.large")
+	assert.Contains(t, body, "m6i.large")
+	assert.Contains(t, body, "42.5")
+	assert.Contains(t, body, "125.50")
+
+	// Requested-by block.
+	assert.Contains(t, body, "Cristi M")
+	assert.Contains(t, body, "cristi@acme.com")
+	assert.Contains(t, body, "2026-05-22T10:00:00Z")
+
+	// Labeled action URLs (not bracket notation).
+	assert.Contains(t, body, "Approve: ")
+	assert.Contains(t, body, "Reject:  ")
+	assert.Contains(t, body, "/api/ri-exchange/approve/rec-001")
+	assert.Contains(t, body, "/api/ri-exchange/reject/rec-001")
+
+	// Custom cancellation-window note overrides the generic fallback.
+	assert.Contains(t, body, "RI exchanges are irreversible once accepted by AWS.")
+	assert.NotContains(t, body, "Please approve within 6 hours")
+}
+
+// Issue #296: plain-text template omits requested-by block when email is empty.
+func TestRenderRIExchangePendingApprovalEmail_NoRequestedBy(t *testing.T) {
+	data := RIExchangeNotificationData{
+		DashboardURL: "https://dashboard.example.com",
+		TotalPayment: "50.00",
+		Exchanges: []RIExchangeItem{{
+			RecordID: "rec-002", ApprovalToken: "tok-xyz",
+			SourceRIID: "ri-aaa", SourceInstanceType: "c5.large",
+			TargetInstanceType: "c6i.large", TargetCount: 1,
+			PaymentDue: "50.00", UtilizationPct: 80.0,
+		}},
+	}
+
+	body, err := RenderRIExchangePendingApprovalEmail(data)
+	require.NoError(t, err)
+
+	assert.NotContains(t, body, "Requested by:")
+	// Generic note rendered when CancellationWindowNote is empty.
+	assert.Contains(t, body, "Please approve within 6 hours")
+}
+
+// Issue #296: HTML half of the RI exchange pending approval email carries
+// inline-styled approve/reject anchors with correct hrefs, the exchange
+// summary table, and the requested-by line.
+func TestRenderRIExchangePendingApprovalEmailHTML_Issue296(t *testing.T) {
+	data := RIExchangeNotificationData{
+		DashboardURL:           "https://dashboard.example.com",
+		TotalPayment:           "125.50",
+		RequestedByEmail:       "cristi@acme.com",
+		CancellationWindowNote: "RI exchanges are irreversible once accepted by AWS.",
+		Exchanges: []RIExchangeItem{
+			{
+				RecordID:           "rec-001",
+				ApprovalToken:      "tok-abc",
+				SourceRIID:         "ri-0123456789abcdef0",
+				SourceInstanceType: "m5.large",
+				TargetInstanceType: "m6i.large",
+				TargetCount:        1,
+				PaymentDue:         "125.50",
+				UtilizationPct:     42.5,
+			},
+		},
+	}
+
+	html, err := RenderRIExchangePendingApprovalEmailHTML(data)
+	require.NoError(t, err)
+	assert.NotEmpty(t, html)
+
+	// Inline-styled approve anchor with correct href.
+	assert.Contains(t, html, `href="https://dashboard.example.com/api/ri-exchange/approve/rec-001?token=tok-abc"`)
+	assert.Contains(t, html, `href="https://dashboard.example.com/api/ri-exchange/reject/rec-001?token=tok-abc"`)
+	assert.Contains(t, html, ">Approve<")
+	assert.Contains(t, html, ">Reject<")
+
+	// Approve button has inline green background (prove CSS classes aren't relied on).
+	assert.Regexp(t, `<a[^>]*style="[^"]*background:#16a34a[^"]*"[^>]*>Approve</a>`, html)
+
+	// Exchange table cells.
+	assert.Contains(t, html, "m5.large")
+	assert.Contains(t, html, "m6i.large")
+	assert.Contains(t, html, "42.5%")
+	assert.Contains(t, html, "125.50")
+
+	// Summary block + requested-by line.
+	assert.Contains(t, html, "cristi@acme.com")
+	assert.Contains(t, html, "RI Exchange Approval Required")
+
+	// Custom cancellation note.
+	assert.Contains(t, html, "RI exchanges are irreversible once accepted by AWS.")
+	assert.NotContains(t, html, "Please approve within 6 hours")
+}
+
+// Issue #296: HTML template falls back to generic 6-hour note when
+// CancellationWindowNote is empty.
+func TestRenderRIExchangePendingApprovalEmailHTML_DefaultCancellationNote(t *testing.T) {
+	data := RIExchangeNotificationData{
+		DashboardURL: "https://dashboard.example.com",
+		TotalPayment: "10.00",
+		Exchanges: []RIExchangeItem{{
+			RecordID: "rec-003", ApprovalToken: "tok-def",
+			SourceRIID: "ri-bbb", SourceInstanceType: "t3.medium",
+			TargetInstanceType: "t4g.medium", TargetCount: 1,
+			PaymentDue: "10.00", UtilizationPct: 60.0,
+		}},
+	}
+
+	html, err := RenderRIExchangePendingApprovalEmailHTML(data)
+	require.NoError(t, err)
+
+	assert.Contains(t, html, "Please approve within 6 hours")
+}
+
+// Issue #296: skipped exchanges block appears in HTML when Skipped is non-empty.
+func TestRenderRIExchangePendingApprovalEmailHTML_SkippedBlock(t *testing.T) {
+	data := RIExchangeNotificationData{
+		DashboardURL: "https://dashboard.example.com",
+		TotalPayment: "0.00",
+		Exchanges:    []RIExchangeItem{},
+		Skipped: []SkippedExchange{{
+			SourceRIID:         "ri-skip-01",
+			SourceInstanceType: "r5.large",
+			Reason:             "no matching target available",
+		}},
+	}
+
+	html, err := RenderRIExchangePendingApprovalEmailHTML(data)
+	require.NoError(t, err)
+
+	assert.Contains(t, html, "ri-skip-01")
+	assert.Contains(t, html, "no matching target available")
+	assert.Contains(t, html, "Skipped")
+}
+
+// Issue #296 / XSS guard: html/template must escape hostile payloads in the
+// RIExchange HTML renderer. Tests RequestedByName, CancellationWindowNote, and
+// Skipped.Reason -- the three free-text fields most likely to carry attacker
+// input. Mirrors the analogous sub-test in TestPlainTextTemplates_NoHTMLEscaping.
+func TestRenderRIExchangePendingApprovalEmailHTML_EscapesHostilePayload(t *testing.T) {
+	const xssPayload = `<script>alert('xss')</script>`
+	data := RIExchangeNotificationData{
+		DashboardURL:           "https://dashboard.example.com",
+		TotalPayment:           "0.00",
+		RequestedByName:        xssPayload,
+		RequestedByEmail:       "attacker@evil.example",
+		CancellationWindowNote: xssPayload,
+		Exchanges:              []RIExchangeItem{},
+		Skipped: []SkippedExchange{{
+			SourceRIID:         "ri-skip-xss",
+			SourceInstanceType: "m5.large",
+			Reason:             xssPayload,
+		}},
+	}
+
+	html, err := RenderRIExchangePendingApprovalEmailHTML(data)
+	require.NoError(t, err)
+
+	// The literal script tag must not appear verbatim in the HTML output.
+	assert.NotContains(t, html, xssPayload, "html/template must escape <script> tags in RequestedByName/CancellationWindowNote/Reason")
+	// The content must still appear (escaped), confirming the field is rendered at all.
+	assert.Contains(t, html, "alert(", "escaped payload content should still appear in output")
+}
+
+// Issue #296 / plain-text guard: text/template must emit special characters
+// verbatim in the RIExchange plain-text renderer (no HTML entity encoding).
+func TestRenderRIExchangePendingApprovalEmail_PlainTextVerbatim(t *testing.T) {
+	name := "O'Brien & Co"
+	emailAddr := "o.brien+tag@acme.com"
+	data := RIExchangeNotificationData{
+		DashboardURL:     "https://dashboard.example.com",
+		TotalPayment:     "0.00",
+		RequestedByName:  name,
+		RequestedByEmail: emailAddr,
+		Exchanges: []RIExchangeItem{{
+			RecordID: "rec-plain", ApprovalToken: "tok-plain",
+			SourceRIID: "ri-ccc", SourceInstanceType: "r5.large",
+			TargetInstanceType: "r6i.large", TargetCount: 1,
+			PaymentDue: "0.00", UtilizationPct: 55.0,
+		}},
+	}
+
+	body, err := RenderRIExchangePendingApprovalEmail(data)
+	require.NoError(t, err)
+
+	assert.Contains(t, body, name, "apostrophe+ampersand in RequestedByName must be verbatim in plain-text")
+	assert.Contains(t, body, emailAddr, "plus-addressed email must be verbatim in plain-text")
+	assert.NotContains(t, body, "&#39;", "plain-text must not HTML-encode apostrophe")
+	assert.NotContains(t, body, "&amp;", "plain-text must not HTML-encode ampersand")
+}