diff --git a/internal/email/coverage_extra_test.go b/internal/email/coverage_extra_test.go index 1d8baf778..b2a04d0ea 100644 --- a/internal/email/coverage_extra_test.go +++ b/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) { diff --git a/internal/email/sender.go b/internal/email/sender.go index c0cdf5526..07bfbd854 100644 --- a/internal/email/sender.go +++ b/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. diff --git a/internal/email/smtp_sender.go b/internal/email/smtp_sender.go index 3968934c6..d9a1f74fb 100644 --- a/internal/email/smtp_sender.go +++ b/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. diff --git a/internal/email/template_renderers.go b/internal/email/template_renderers.go index 3af5c40aa..b6259d702 100644 --- a/internal/email/template_renderers.go +++ b/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) diff --git a/internal/email/template_renderers_test.go b/internal/email/template_renderers_test.go index 44e377d9c..08422527a 100644 --- a/internal/email/template_renderers_test.go +++ b/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 ` + 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