diff --git a/internal/email/coverage_extra_test.go b/internal/email/coverage_extra_test.go
index 8be4f25b9..bf9314117 100644
--- a/internal/email/coverage_extra_test.go
+++ b/internal/email/coverage_extra_test.go
@@ -11,14 +11,22 @@ import (
"github.com/stretchr/testify/require"
)
-// Tests for containsColon
-func TestContainsColon(t *testing.T) {
- assert.True(t, containsColon("arn:aws:secretsmanager:us-east-1:123:secret:foo"))
- assert.True(t, containsColon("a:b"))
- assert.True(t, containsColon(":"))
- assert.False(t, containsColon(""))
- assert.False(t, containsColon("nodivider"))
- assert.False(t, containsColon("just/a/path"))
+// Tests for isSecretManagerReference (replaces the deleted containsColon helper -- 07-L2/L3)
+func TestIsSecretManagerReference(t *testing.T) {
+ // AWS ARN
+ assert.True(t, isSecretManagerReference("arn:aws:secretsmanager:us-east-1:123:secret:foo"))
+ // GCP resource path
+ assert.True(t, isSecretManagerReference("projects/my-project/secrets/my-secret/versions/latest"))
+ // Azure Key Vault URL
+ assert.True(t, isSecretManagerReference("https://myvault.vault.azure.net/secrets/mysecret"))
+ // Generic slash path
+ assert.True(t, isSecretManagerReference("/my/secret/path"))
+ // Plain API key -- not a reference
+ assert.False(t, isSecretManagerReference("SG.abcdefghijklmnopqrstuvwx"))
+ // Empty string
+ assert.False(t, isSecretManagerReference(""))
+ // Value with colon but not an ARN/Vault URL
+ assert.False(t, isSecretManagerReference("user:password"))
}
// Tests for warnIfPlaintext – exercising all branches
@@ -26,17 +34,20 @@ func TestWarnIfPlaintext(t *testing.T) {
// Empty value: should be a no-op (no panic)
assert.NotPanics(t, func() { warnIfPlaintext("VAR", "") })
- // Short value (< 20 chars, no colon, no leading slash) → warning branch
- assert.NotPanics(t, func() { warnIfPlaintext("VAR", "short") })
+ // Plaintext API key (not a secret-manager reference) -- warning branch
+ assert.NotPanics(t, func() { warnIfPlaintext("VAR", "SG.averylongplaintextpassword") })
- // Long value with colon → secret-manager reference, no warning
+ // AWS ARN -- treated as secret-manager reference, no warning
assert.NotPanics(t, func() { warnIfPlaintext("VAR", "arn:aws:secretsmanager:us-east-1:123456789012:secret:mykey") })
- // Long value starting with slash → secret-manager reference, no warning
- assert.NotPanics(t, func() { warnIfPlaintext("VAR", "/my/secret/path/that/is/long") })
+ // GCP resource path -- treated as secret-manager reference, no warning
+ assert.NotPanics(t, func() { warnIfPlaintext("VAR", "projects/my-project/secrets/my-secret") })
+
+ // Azure Key Vault URL -- treated as secret-manager reference, no warning
+ assert.NotPanics(t, func() { warnIfPlaintext("VAR", "https://myvault.vault.azure.net/secrets/foo") })
- // Long value without colon and not starting with slash → warning branch
- assert.NotPanics(t, func() { warnIfPlaintext("VAR", "averylongplaintextpasswordwithnospecialchars") })
+ // Generic leading slash path -- treated as secret-manager reference, no warning
+ assert.NotPanics(t, func() { warnIfPlaintext("VAR", "/my/secret/path/that/is/long") })
}
// Tests for renderTemplate error path
@@ -408,12 +419,14 @@ func TestSMTPSender_SendRIExchangePendingApproval_WithNotifyEmail(t *testing.T)
// Tests for Sender.SendRIExchangePendingApproval, SendRIExchangeCompleted, SendPurchaseApprovalRequest
// using the mock SNS sender (no SNS topic → no-op path)
-func TestSender_SendRIExchangePendingApproval_NoTopic(t *testing.T) {
+func TestSender_SendRIExchangePendingApproval_NoRecipient(t *testing.T) {
+ // When RecipientEmail is empty the send must be rejected — the body carries
+ // per-exchange approval tokens that must not be broadcast via SNS.
mockSNS := &mockSNSPublisher{}
mockSES := &mockSESEmailSender{}
sender := NewSenderWithClients(mockSNS, mockSES, SenderConfig{
- // TopicARN is empty → SendNotification is a no-op
FromEmail: "from@example.com",
+ TopicARN: "arn:aws:sns:us-east-1:123:topic",
})
data := RIExchangeNotificationData{
@@ -421,12 +434,14 @@ func TestSender_SendRIExchangePendingApproval_NoTopic(t *testing.T) {
TotalPayment: "300.00",
Exchanges: []RIExchangeItem{
{RecordID: "r4", SourceRIID: "ri-4", SourceInstanceType: "m4.large",
- TargetInstanceType: "m4.xlarge", TargetCount: 1, PaymentDue: "300.00", UtilizationPct: 55.0},
+ TargetInstanceType: "m4.xlarge", TargetCount: 1, PaymentDue: "300.00",
+ ApprovalToken: "secret-token", UtilizationPct: 55.0},
},
+ // RecipientEmail intentionally empty
}
err := sender.SendRIExchangePendingApproval(context.Background(), data)
- require.NoError(t, err)
+ require.ErrorIs(t, err, ErrNoRecipient)
}
func TestSender_SendRIExchangeCompleted_NoTopic(t *testing.T) {
diff --git a/internal/email/coverage_test.go b/internal/email/coverage_test.go
index b13d0d354..2672fbd10 100644
--- a/internal/email/coverage_test.go
+++ b/internal/email/coverage_test.go
@@ -632,7 +632,9 @@ func TestSMTPSender_SendToEmail_ConnectionFails(t *testing.T) {
assert.Contains(t, err.Error(), "failed to send email via SMTP")
}
-// TestSMTPSender_SendToEmail_ConnectionFails_NoTLS tests with useTLS=false
+// TestSMTPSender_SendToEmail_ConnectionFails_NoTLS tests with useTLS=false and
+// credentials configured. Since 07-H2 the cleartext-auth guard fires before
+// any network dial, so the error message differs from a connection failure.
func TestSMTPSender_SendToEmail_ConnectionFails_NoTLS(t *testing.T) {
sender := &SMTPSender{
host: "localhost",
@@ -642,14 +644,15 @@ func TestSMTPSender_SendToEmail_ConnectionFails_NoTLS(t *testing.T) {
fromEmail: "sender@example.com",
fromName: "",
useTLS: false,
+ // allowInsecure intentionally false: the cleartext-auth guard must fire.
}
ctx := context.Background()
err := sender.SendToEmail(ctx, "recipient@example.com", "Test Subject", "Test Body")
- // This should fail because there's no SMTP server on that port
+ // 07-H2: guard fires before any network dial.
require.Error(t, err)
- assert.Contains(t, err.Error(), "failed to send email via SMTP")
+ assert.Contains(t, err.Error(), "SMTP auth over non-TLS connection is refused")
}
// TestSMTPSender_SendToEmail_NoAuth tests without authentication
@@ -1305,13 +1308,15 @@ func TestSender_SendMethods_ErrorPaths(t *testing.T) {
assert.Contains(t, err.Error(), "failed to publish to SNS")
})
- t.Run("ScheduledPurchase_propagates_error", func(t *testing.T) {
+ t.Run("ScheduledPurchase_no_recipient", func(t *testing.T) {
+ // RecipientEmail is empty: must return ErrNoRecipient, not broadcast via SNS.
err := sender.SendScheduledPurchaseNotification(ctx, NotificationData{
- DashboardURL: "https://test.com",
- PlanName: "Test",
+ DashboardURL: "https://test.com",
+ PlanName: "Test",
+ ApprovalToken: "tok",
+ // RecipientEmail intentionally empty
})
- require.Error(t, err)
- assert.Contains(t, err.Error(), "failed to publish to SNS")
+ require.ErrorIs(t, err, ErrNoRecipient)
})
t.Run("PurchaseConfirmation_propagates_error", func(t *testing.T) {
diff --git a/internal/email/factory.go b/internal/email/factory.go
index 7acf61334..642c31d90 100644
--- a/internal/email/factory.go
+++ b/internal/email/factory.go
@@ -6,6 +6,7 @@ import (
"fmt"
"os"
"strconv"
+ "strings"
"github.com/LeanerCloud/CUDly/internal/secrets"
"github.com/LeanerCloud/CUDly/pkg/logging"
@@ -86,6 +87,22 @@ func NewSenderFromEnvironment(ctx context.Context) (SenderInterface, error) {
}
}
+// isSecretManagerReference reports whether value looks like a secret manager
+// reference rather than a plaintext credential (07-L3). Recognised patterns:
+// - AWS ARN: starts with "arn:"
+// - GCP resource: starts with "projects/"
+// - Azure Key Vault secret URL: contains ".vault.azure.net/"
+// - Generic path: starts with "/" (local-file or Vault path)
+//
+// A value that does not match any pattern is treated as a potential plaintext
+// credential and triggers a warning via warnIfPlaintext.
+func isSecretManagerReference(value string) bool {
+ return strings.HasPrefix(value, "arn:") ||
+ strings.HasPrefix(value, "projects/") ||
+ strings.Contains(value, ".vault.azure.net/") ||
+ strings.HasPrefix(value, "/")
+}
+
// warnIfPlaintext logs a warning when a credential value is set as a plaintext
// environment variable rather than a secret manager reference (ARN/resource name).
// TODO: migrate all email credentials to AWS Secrets Manager / GCP Secret Manager /
@@ -94,20 +111,9 @@ func warnIfPlaintext(envVar, value string) {
if value == "" {
return
}
- // Heuristic: secret manager references contain ":" or "/" (ARN, resource path, secret name)
- if len(value) < 20 || (value[0] != '/' && !containsColon(value)) {
- logging.Warnf("security: %s is set as a plaintext env var; consider using a Secrets Manager reference instead", envVar)
- }
-}
-
-// containsColon returns true if s contains a colon character.
-func containsColon(s string) bool {
- for _, c := range s {
- if c == ':' {
- return true
- }
+ if !isSecretManagerReference(value) {
+ logging.Warnf("security: %s is set as a plaintext env var; consider using a Secrets Manager reference (ARN, projects/..., vault URL, or /path) instead", envVar)
}
- return false
}
// newGCPSenderFromEnv creates a SendGrid-based email sender from environment variables
diff --git a/internal/email/sender.go b/internal/email/sender.go
index 75b3d3011..cca2ffd88 100644
--- a/internal/email/sender.go
+++ b/internal/email/sender.go
@@ -27,6 +27,12 @@ var (
// out. Distinct from ErrNoRecipient so the caller can report which side
// of the wire is unconfigured.
ErrNoFromEmail = errors.New("email: no from address")
+
+ // ErrTokenInBroadcast is returned by SendNotification when the message
+ // body contains a "token=" query parameter. Broadcasting a body with an
+ // approval token leaks it to every SNS subscriber. Use SendToEmailWithCC
+ // (targeted SES) for any message that carries an approval URL.
+ ErrTokenInBroadcast = errors.New("email: message body contains an approval token; use targeted SES send, not SNS broadcast")
)
// SenderConfig holds configuration for the email sender
@@ -100,8 +106,26 @@ func NewSenderWithClients(snsClient SNSPublisher, sesClient SESEmailSender, cfg
}
}
-// SendNotification sends a notification email via SNS
+// snsMaxSubjectLen is the maximum byte length SNS accepts for a Subject.
+// Subjects longer than 100 bytes are rejected with InvalidParameter at runtime.
+const snsMaxSubjectLen = 100
+
+// SendNotification sends a notification email via SNS.
+//
+// Guard: messages whose body contains "token=" (case-insensitive) are rejected
+// with ErrTokenInBroadcast. Approval tokens must never reach the SNS broadcast
+// topic because every subscriber would receive a working action link. Use
+// SendToEmailWithCC for any message that carries an approval URL.
+//
+// Subject sanitization (07-M3): the subject is passed through sanitizeHeader
+// (strips CR/LF to prevent SNS parameter injection) and truncated to 100 bytes
+// (SNS limit). Subjects built from user-controlled fields such as PlanName can
+// otherwise cause an InvalidParameter error at publish time.
func (s *Sender) SendNotification(ctx context.Context, subject, message string) error {
+ if strings.Contains(strings.ToLower(message), "token=") {
+ return ErrTokenInBroadcast
+ }
+
if s.topicARN == "" {
logging.Debug("No SNS topic configured, skipping email notification")
return nil
@@ -111,16 +135,22 @@ func (s *Sender) SendNotification(ctx context.Context, subject, message string)
return fmt.Errorf("SNS client not initialized")
}
+ // Sanitize and truncate the SNS Subject (07-M3).
+ safeSubject := sanitizeHeader(subject)
+ if len(safeSubject) > snsMaxSubjectLen {
+ safeSubject = safeSubject[:snsMaxSubjectLen]
+ }
+
_, err := s.snsClient.Publish(ctx, &sns.PublishInput{
TopicArn: aws.String(s.topicARN),
- Subject: aws.String(subject),
+ Subject: aws.String(safeSubject),
Message: aws.String(message),
})
if err != nil {
return fmt.Errorf("failed to publish to SNS: %w", err)
}
- logging.Debugf("Sent notification: %s", subject)
+ logging.Debugf("Sent notification: %s", safeSubject)
return nil
}
@@ -283,15 +313,26 @@ func (s *Sender) ensureSandboxRecipientVerified(ctx context.Context, toEmail str
logging.Warnf("Recipient email %s is not verified in sandbox mode - creating verification request", redactEmail(toEmail))
if err := s.createVerificationRequest(ctx, toEmail); err != nil {
logging.Warnf("Failed to create verification request: %v", err)
+ // Use the real address in the user-facing message so the recipient
+ // knows which inbox to check; log only the redacted form (07-M2).
return fmt.Errorf("recipient email %s is not verified in SES sandbox mode. A verification email has been sent - please check inbox and click the verification link before trying again", toEmail)
}
- return fmt.Errorf("recipient email %s is not verified in SES sandbox mode. A verification email has been sent to %s - please check inbox and click the verification link, then try the password reset again", toEmail, toEmail)
+ // The user-facing message intentionally includes the full address so the
+ // recipient can verify the correct inbox. Wrapped errors/logs use the
+ // redacted form per the package's PII stance (07-M2).
+ return fmt.Errorf("recipient email %s is not verified in SES sandbox mode. A verification email has been sent - please check inbox and click the verification link, then try again", toEmail)
}
// buildSESSendEmailInputMultipart constructs a sesv2.SendEmailInput with
// both a plain-text and an HTML alternative body. SES handles the
// multipart/alternative MIME assembly server-side when both Text and Html
// fields are populated on types.Body.
+//
+// Header injection note (07-M1): SES v2 SendEmail accepts structured fields
+// (Subject.Data, Body.Text.Data, Body.Html.Data) and builds the MIME envelope
+// server-side. CR/LF injection via Subject.Data is not possible through the
+// structured API. Any future raw-MIME path MUST sanitize the subject with
+// sanitizeHeader before composing the header string, matching the SMTP path.
func buildSESSendEmailInputMultipart(fromEmail, toEmail string, cc []string, subject, textBody, htmlBody string) *sesv2.SendEmailInput {
destination := &types.Destination{
ToAddresses: []string{toEmail},
@@ -325,6 +366,7 @@ func buildSESSendEmailInputMultipart(fromEmail, toEmail string, cc []string, sub
// buildSESSendEmailInput constructs a sesv2.SendEmailInput with the
// destination To + (optional) Cc list and a plain-text body.
+// See buildSESSendEmailInputMultipart for the header-injection safety note (07-M1).
func buildSESSendEmailInput(fromEmail, toEmail string, cc []string, subject, body string) *sesv2.SendEmailInput {
destination := &types.Destination{
ToAddresses: []string{toEmail},
@@ -464,6 +506,15 @@ type RIExchangeNotificationData struct {
Exchanges []RIExchangeItem
Skipped []SkippedExchange
TotalPayment string
+ // RecipientEmail is the primary (To) inbox for the approval-required flow.
+ // Must be set when Exchanges contain ApprovalToken values; leave empty
+ // only for completion/broadcast notifications that carry no tokens.
+ // When non-empty, SendRIExchangePendingApproval routes through targeted
+ // SES (not the SNS broadcast topic). Mirrors NotificationData.RecipientEmail.
+ RecipientEmail string
+ // CCEmails carries additional recipients informed of the pending exchanges
+ // but not the authorised approvers. Deduplicated against RecipientEmail.
+ CCEmails []string
}
// RIExchangeItem represents a single exchange in an email notification
diff --git a/internal/email/sender_test.go b/internal/email/sender_test.go
index c5fa24262..261b6dc48 100644
--- a/internal/email/sender_test.go
+++ b/internal/email/sender_test.go
@@ -878,3 +878,252 @@ func TestSender_SendToEmailWithCCMultipart_FallsBackWhenHTMLEmpty_Issue287(t *te
require.NotNil(t, captured.Content.Simple.Body.Text)
assert.Nil(t, captured.Content.Simple.Body.Html, "empty HTML body should NOT be sent as multipart")
}
+
+// ---------------------------------------------------------------------------
+// Regression tests for issue #1015: approval tokens broadcast via SNS
+// ---------------------------------------------------------------------------
+
+// TestSendNotification_RejectsTokenBearingBody verifies the structural guard:
+// SendNotification must return ErrTokenInBroadcast when the message body
+// contains "token=" so a body with an approval URL can never reach the SNS
+// broadcast topic, regardless of how it got there.
+func TestSendNotification_RejectsTokenBearingBody(t *testing.T) {
+ t.Parallel()
+ mockSNS := new(MockSNSClient)
+ // No Publish calls expected — the guard fires before the client is touched.
+ t.Cleanup(func() { mockSNS.AssertExpectations(t) })
+
+ sender := NewSenderWithClients(mockSNS, nil, SenderConfig{
+ TopicARN: "arn:aws:sns:us-east-1:123456789012:topic",
+ })
+
+ ctx := context.Background()
+ tokenBodies := []string{
+ "Review: https://app.example.com/approve?token=abc123",
+ "token=somesecret",
+ "?token=xyz&action=pause",
+ // Mixed-case variants: the guard is case-insensitive so future
+ // template changes that capitalize the query param can't slip a
+ // token-bearing body past the broadcast guard.
+ "Review: https://app.example.com/approve?Token=abc123",
+ "?TOKEN=xyz&action=pause",
+ }
+ for _, body := range tokenBodies {
+ err := sender.SendNotification(ctx, "Test", body)
+ assert.ErrorIsf(t, err, ErrTokenInBroadcast,
+ "expected ErrTokenInBroadcast for body %q", body)
+ }
+}
+
+// TestSendNotification_AllowsTokenFreeBody verifies that a well-formed
+// broadcast body that contains no token reaches SNS normally.
+func TestSendNotification_AllowsTokenFreeBody(t *testing.T) {
+ t.Parallel()
+ mockSNS := new(MockSNSClient)
+ mockSNS.On("Publish", mock.Anything, mock.AnythingOfType("*sns.PublishInput")).
+ Return(&sns.PublishOutput{MessageId: aws.String("msg-ok")}, nil).Once()
+ t.Cleanup(func() { mockSNS.AssertExpectations(t) })
+
+ sender := NewSenderWithClients(mockSNS, nil, SenderConfig{
+ TopicARN: "arn:aws:sns:us-east-1:123456789012:topic",
+ })
+
+ ctx := context.Background()
+ err := sender.SendNotification(ctx, "Savings found", "3 new recommendations. Sign in to review.")
+ require.NoError(t, err)
+}
+
+// TestSendScheduledPurchaseNotification_UsesSESNotSNS verifies that a
+// scheduled-purchase notification with RecipientEmail set reaches SES
+// (targeted delivery) and does NOT call the SNS publisher.
+//
+// Regression test for issue #1015: previously the method called
+// s.SendNotification which broadcast the token-bearing body to every
+// SNS subscriber.
+func TestSendScheduledPurchaseNotification_UsesSESNotSNS(t *testing.T) {
+ t.Parallel()
+ mockSNS := new(MockSNSClient)
+ // SNS Publish must NOT be called.
+ t.Cleanup(func() { mockSNS.AssertExpectations(t) })
+
+ mockSES := new(MockSESClient)
+ mockSES.On("GetAccount", mock.Anything, mock.AnythingOfType("*sesv2.GetAccountInput")).
+ Return(&sesv2.GetAccountOutput{ProductionAccessEnabled: true}, nil).Once()
+ mockSES.On("SendEmail", mock.Anything, mock.AnythingOfType("*sesv2.SendEmailInput")).
+ Return(&sesv2.SendEmailOutput{MessageId: aws.String("ses-msg-1")}, nil).Once()
+ t.Cleanup(func() { mockSES.AssertExpectations(t) })
+
+ sender := NewSenderWithClients(mockSNS, mockSES, SenderConfig{
+ TopicARN: "arn:aws:sns:us-east-1:123456789012:topic",
+ FromEmail: "noreply@example.com",
+ })
+
+ data := NotificationData{
+ DashboardURL: "https://app.example.com",
+ ApprovalToken: "supersecret-token",
+ RecipientEmail: "notify@example.com",
+ TotalSavings: 800.0,
+ DaysUntilPurchase: 3,
+ PlanName: "Test Plan",
+ }
+
+ ctx := context.Background()
+ err := sender.SendScheduledPurchaseNotification(ctx, data)
+ require.NoError(t, err)
+}
+
+// TestSendScheduledPurchaseNotification_ErrNoRecipientWhenEmpty verifies that
+// omitting RecipientEmail returns ErrNoRecipient, not a silent broadcast.
+func TestSendScheduledPurchaseNotification_ErrNoRecipientWhenEmpty(t *testing.T) {
+ t.Parallel()
+ mockSNS := new(MockSNSClient)
+ mockSES := new(MockSESClient)
+ // Neither SNS Publish nor SES SendEmail should be called.
+ t.Cleanup(func() {
+ mockSNS.AssertExpectations(t)
+ mockSES.AssertExpectations(t)
+ })
+
+ sender := NewSenderWithClients(mockSNS, mockSES, SenderConfig{
+ TopicARN: "arn:aws:sns:us-east-1:123456789012:topic",
+ FromEmail: "noreply@example.com",
+ })
+
+ data := NotificationData{
+ DashboardURL: "https://app.example.com",
+ ApprovalToken: "supersecret-token",
+ PlanName: "Test Plan",
+ // RecipientEmail intentionally absent
+ }
+
+ err := sender.SendScheduledPurchaseNotification(context.Background(), data)
+ require.ErrorIs(t, err, ErrNoRecipient)
+}
+
+// TestSendRIExchangePendingApproval_UsesSESNotSNS verifies that an RI-exchange
+// pending-approval notification with RecipientEmail set reaches SES (targeted
+// delivery) and does NOT call the SNS publisher.
+//
+// Regression test for issue #1015: previously the method called
+// s.SendNotification which broadcast per-exchange approve/reject tokens to
+// every SNS subscriber, allowing unauthorised spend approval.
+func TestSendRIExchangePendingApproval_UsesSESNotSNS(t *testing.T) {
+ t.Parallel()
+ mockSNS := new(MockSNSClient)
+ // SNS Publish must NOT be called.
+ t.Cleanup(func() { mockSNS.AssertExpectations(t) })
+
+ mockSES := new(MockSESClient)
+ mockSES.On("GetAccount", mock.Anything, mock.AnythingOfType("*sesv2.GetAccountInput")).
+ Return(&sesv2.GetAccountOutput{ProductionAccessEnabled: true}, nil).Once()
+ mockSES.On("SendEmail", mock.Anything, mock.AnythingOfType("*sesv2.SendEmailInput")).
+ Return(&sesv2.SendEmailOutput{MessageId: aws.String("ses-msg-2")}, nil).Once()
+ t.Cleanup(func() { mockSES.AssertExpectations(t) })
+
+ sender := NewSenderWithClients(mockSNS, mockSES, SenderConfig{
+ TopicARN: "arn:aws:sns:us-east-1:123456789012:topic",
+ FromEmail: "noreply@example.com",
+ })
+
+ data := RIExchangeNotificationData{
+ DashboardURL: "https://app.example.com",
+ Mode: "manual",
+ RecipientEmail: "notify@example.com",
+ TotalPayment: "150.00",
+ Exchanges: []RIExchangeItem{
+ {
+ RecordID: "rec-1",
+ ApprovalToken: "exchange-secret-token",
+ SourceRIID: "ri-abc",
+ SourceInstanceType: "m5.large",
+ TargetInstanceType: "m5.xlarge",
+ TargetCount: 1,
+ PaymentDue: "150.00",
+ UtilizationPct: 45.0,
+ },
+ },
+ }
+
+ ctx := context.Background()
+ err := sender.SendRIExchangePendingApproval(ctx, data)
+ require.NoError(t, err)
+}
+
+// TestSendRIExchangePendingApproval_ErrNoRecipientWhenEmpty verifies that
+// omitting RecipientEmail returns ErrNoRecipient, not a silent broadcast.
+func TestSendRIExchangePendingApproval_ErrNoRecipientWhenEmpty(t *testing.T) {
+ t.Parallel()
+ mockSNS := new(MockSNSClient)
+ mockSES := new(MockSESClient)
+ // Neither SNS Publish nor SES SendEmail should be called.
+ t.Cleanup(func() {
+ mockSNS.AssertExpectations(t)
+ mockSES.AssertExpectations(t)
+ })
+
+ sender := NewSenderWithClients(mockSNS, mockSES, SenderConfig{
+ TopicARN: "arn:aws:sns:us-east-1:123456789012:topic",
+ FromEmail: "noreply@example.com",
+ })
+
+ data := RIExchangeNotificationData{
+ DashboardURL: "https://app.example.com",
+ Mode: "manual",
+ // RecipientEmail intentionally absent
+ Exchanges: []RIExchangeItem{
+ {RecordID: "rec-1", ApprovalToken: "exchange-secret-token"},
+ },
+ }
+
+ err := sender.SendRIExchangePendingApproval(context.Background(), data)
+ require.ErrorIs(t, err, ErrNoRecipient)
+}
+
+// TestSendNotification_SubjectSanitizedAndTruncated verifies that 07-M3 is
+// enforced: SendNotification strips CR/LF from the subject and truncates it
+// to 100 bytes before publishing to SNS. A subject longer than 100 bytes or
+// containing newlines would cause an InvalidParameter error at runtime.
+func TestSendNotification_SubjectSanitizedAndTruncated(t *testing.T) {
+ t.Parallel()
+
+ t.Run("newline_stripped", func(t *testing.T) {
+ var captured *sns.PublishInput
+ mockSNS := new(MockSNSClient)
+ mockSNS.On("Publish", mock.Anything, mock.MatchedBy(func(in *sns.PublishInput) bool {
+ captured = in
+ return true
+ })).Return(&sns.PublishOutput{MessageId: aws.String("id-1")}, nil)
+ t.Cleanup(func() { mockSNS.AssertExpectations(t) })
+
+ sender := NewSenderWithClients(mockSNS, nil, SenderConfig{
+ TopicARN: "arn:aws:sns:us-east-1:123:topic",
+ })
+
+ err := sender.SendNotification(context.Background(), "Plan A\r\nInjected", "clean body")
+ require.NoError(t, err)
+ require.NotNil(t, captured)
+ assert.NotContains(t, *captured.Subject, "\r")
+ assert.NotContains(t, *captured.Subject, "\n")
+ })
+
+ t.Run("long_subject_truncated", func(t *testing.T) {
+ const longSubject = "AAAAAAAAAA AAAAAAAAAA AAAAAAAAAA AAAAAAAAAA AAAAAAAAAA AAAAAAAAAA AAAAAAAAAA AAAAAAAAAA AAAAAAAAAA AAAAAAAAAA"
+ var captured *sns.PublishInput
+ mockSNS := new(MockSNSClient)
+ mockSNS.On("Publish", mock.Anything, mock.MatchedBy(func(in *sns.PublishInput) bool {
+ captured = in
+ return true
+ })).Return(&sns.PublishOutput{MessageId: aws.String("id-2")}, nil)
+ t.Cleanup(func() { mockSNS.AssertExpectations(t) })
+
+ sender := NewSenderWithClients(mockSNS, nil, SenderConfig{
+ TopicARN: "arn:aws:sns:us-east-1:123:topic",
+ })
+
+ err := sender.SendNotification(context.Background(), longSubject, "clean body")
+ require.NoError(t, err)
+ require.NotNil(t, captured)
+ assert.LessOrEqual(t, len(*captured.Subject), snsMaxSubjectLen,
+ "SNS subject must not exceed %d bytes", snsMaxSubjectLen)
+ })
+}
diff --git a/internal/email/smtp_sender.go b/internal/email/smtp_sender.go
index f632737b0..5e0e92ddc 100644
--- a/internal/email/smtp_sender.go
+++ b/internal/email/smtp_sender.go
@@ -3,7 +3,9 @@ package email
import (
"context"
+ "crypto/rand"
"crypto/tls"
+ "encoding/hex"
"fmt"
"net"
"net/smtp"
@@ -22,18 +24,25 @@ type SMTPConfig struct {
FromName string
NotifyEmail string // Notification recipient email (defaults to FromEmail if empty)
UseTLS bool // Use STARTTLS (default true)
+ // AllowInsecure, when true, permits sending with credentials over a
+ // non-TLS connection. This must never be set in production; it exists
+ // only for integration tests against a local plaintext SMTP stub.
+ // When false (the default), dispatchSMTP returns an error if auth is
+ // configured but UseTLS is false (07-H2).
+ AllowInsecure bool
}
// SMTPSender handles sending email via SMTP (works for SendGrid, Azure ACS, and others)
type SMTPSender struct {
- host string
- port int
- username string
- password string
- fromEmail string
- fromName string
- notifyEmail string
- useTLS bool
+ host string
+ port int
+ username string
+ password string
+ fromEmail string
+ fromName string
+ notifyEmail string
+ useTLS bool
+ allowInsecure bool
}
// NewSMTPSender creates a new SMTP email sender
@@ -59,21 +68,29 @@ func NewSMTPSender(cfg SMTPConfig) (*SMTPSender, error) {
}
return &SMTPSender{
- host: cfg.Host,
- port: cfg.Port,
- username: cfg.Username,
- password: cfg.Password,
- fromEmail: cfg.FromEmail,
- fromName: cfg.FromName,
- notifyEmail: notifyEmail,
- useTLS: cfg.UseTLS,
+ host: cfg.Host,
+ port: cfg.Port,
+ username: cfg.Username,
+ password: cfg.Password,
+ fromEmail: cfg.FromEmail,
+ fromName: cfg.FromName,
+ notifyEmail: notifyEmail,
+ useTLS: cfg.UseTLS,
+ allowInsecure: cfg.AllowInsecure,
}, nil
}
-// SendNotification sends a notification email via SMTP
-// Note: SMTP doesn't have SNS-like pub/sub, so this sends directly
+// SendNotification is a no-op for SMTP senders (07-N4).
+// SMTP has no pub/sub equivalent of SNS: there is no topic to publish to and
+// no subscriber list to fan out to. As a result, GCP (SendGrid) and Azure
+// (ACS SMTP) deployments do not receive broadcast notifications (new-recs,
+// scheduled-purchase reminders without a recipient email, etc.). Callers that
+// need broadcast behaviour on non-AWS deployments must wire their own fan-out
+// or configure an SNS-compatible endpoint. Targeted approval emails
+// (SendPurchaseApprovalRequest, SendScheduledPurchaseNotification) are
+// unaffected because they use SendToEmailWithCC directly.
func (s *SMTPSender) SendNotification(ctx context.Context, subject, message string) error {
- logging.Debug("SMTP notification would be sent (not implemented for pub/sub)")
+ logging.Debug("SMTP broadcast notification is a no-op (SMTP has no pub/sub mechanism)")
return nil
}
@@ -163,13 +180,26 @@ func sanitizeCCList(toEmail string, ccEmails []string) []string {
return out
}
+// mimeRandBoundary returns a per-message random MIME boundary (07-N2).
+// Using a fixed constant boundary risks corruption if a message body
+// literally contains that string. crypto/rand gives a collision-free 8-byte
+// hex suffix; falls back to a fixed literal on the (exceptional) rand failure
+// so messages still deliver in degraded environments.
+func mimeRandBoundary() string {
+ b := make([]byte, 8)
+ if _, err := rand.Read(b); err != nil {
+ return "cudly-mp-fallback-7e3b1c89af04d2"
+ }
+ return "cudly-mp-" + hex.EncodeToString(b)
+}
+
// buildSMTPMessageMultipart assembles a multipart/alternative RFC-5322
-// message with both a plain-text and an HTML part. The boundary is a
-// fixed literal — small risk of body-content collision, mitigated by the
-// boundary's unlikely-to-occur shape. `cc` is already sanitized and
-// deduped by the caller.
+// message with both a plain-text and an HTML part. A per-message random
+// boundary is generated via mimeRandBoundary to eliminate the theoretical
+// body-collision risk of a fixed literal (07-N2). `cc` is already sanitized
+// and deduped by the caller.
func (s *SMTPSender) buildSMTPMessageMultipart(toEmail string, cc []string, subject, textBody, htmlBody string) []byte {
- const boundary = "cudly-mp-7e3b1c89af04d2"
+ boundary := mimeRandBoundary()
from := s.fromEmail
if s.fromName != "" {
from = fmt.Sprintf("%s <%s>", sanitizeHeader(s.fromName), s.fromEmail)
@@ -215,12 +245,22 @@ func (s *SMTPSender) buildSMTPMessage(toEmail string, cc []string, subject, body
// dispatchSMTP runs the actual SMTP SendMail call, routing through
// STARTTLS when s.useTLS is true. The rcpts list carries every address
// that should receive the message (To + Cc).
+//
+// Security (07-H2): if credentials are configured but TLS is disabled the
+// call is refused unless AllowInsecure was explicitly set. Sending SMTP AUTH
+// over a non-TLS connection exposes credentials (SendGrid API key, Azure ACS
+// password) and token-bearing message bodies in cleartext. AllowInsecure must
+// never be set in production; it exists solely for integration tests against a
+// local plaintext stub server.
func (s *SMTPSender) dispatchSMTP(rcpts []string, msg []byte) error {
addr := fmt.Sprintf("%s:%d", s.host, s.port)
var auth smtp.Auth
if s.username != "" && s.password != "" {
auth = smtp.PlainAuth("", s.username, s.password, s.host)
}
+ if auth != nil && !s.useTLS && !s.allowInsecure {
+ return fmt.Errorf("SMTP auth over non-TLS connection is refused: set UseTLS=true or, for test-only stubs, AllowInsecure=true")
+ }
if s.useTLS {
if err := s.sendMailTLS(addr, auth, s.fromEmail, rcpts, msg); err != nil {
return fmt.Errorf("failed to send email via SMTP: %w", err)
diff --git a/internal/email/smtp_server_test.go b/internal/email/smtp_server_test.go
index 32951308a..8061f88fc 100644
--- a/internal/email/smtp_server_test.go
+++ b/internal/email/smtp_server_test.go
@@ -160,20 +160,23 @@ func TestSMTPSender_SendToEmail_WithMockServer_NoTLS(t *testing.T) {
require.NoError(t, err)
}
-// TestSMTPSender_SendToEmail_WithMockServer_WithAuth tests with auth (no TLS)
+// TestSMTPSender_SendToEmail_WithMockServer_WithAuth tests with auth over a
+// local plaintext SMTP stub. AllowInsecure is required for cleartext-auth
+// stubs in tests (07-H2: production senders must use UseTLS=true).
func TestSMTPSender_SendToEmail_WithMockServer_WithAuth(t *testing.T) {
server := newMockSMTPServer(t, false)
server.start(t)
defer server.stop()
sender := &SMTPSender{
- host: "127.0.0.1",
- port: server.port,
- username: "testuser",
- password: "testpass",
- fromEmail: "sender@test.com",
- fromName: "Sender Name",
- useTLS: false,
+ host: "127.0.0.1",
+ port: server.port,
+ username: "testuser",
+ password: "testpass",
+ fromEmail: "sender@test.com",
+ fromName: "Sender Name",
+ useTLS: false,
+ allowInsecure: true, // test-only stub; never set in production
}
ctx := context.Background()
@@ -182,7 +185,46 @@ func TestSMTPSender_SendToEmail_WithMockServer_WithAuth(t *testing.T) {
require.NoError(t, err)
}
+// TestSMTPSender_DispatchSMTP_RefusesAuthOverCleartext asserts that
+// dispatchSMTP returns an error when credentials are configured but TLS is
+// disabled and AllowInsecure is false (07-H2 guard).
+func TestSMTPSender_DispatchSMTP_RefusesAuthOverCleartext(t *testing.T) {
+ sender := &SMTPSender{
+ host: "127.0.0.1",
+ port: 9999, // unreachable; we expect the guard to fire before connecting
+ username: "user",
+ password: "pass",
+ fromEmail: "sender@test.com",
+ useTLS: false,
+ allowInsecure: false,
+ }
+ err := sender.dispatchSMTP([]string{"r@example.com"}, []byte("msg"))
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "SMTP auth over non-TLS connection is refused")
+}
+
+// TestSMTPSender_DispatchSMTP_AllowsAuthWithAllowInsecure checks the escape
+// hatch for test-only stubs: AllowInsecure=true should not trigger the guard.
+// The actual send will fail (no server running on port 1), which is expected.
+func TestSMTPSender_DispatchSMTP_AllowsAuthWithAllowInsecure(t *testing.T) {
+ sender := &SMTPSender{
+ host: "127.0.0.1",
+ port: 1, // unreachable, but guard must not fire
+ username: "user",
+ password: "pass",
+ fromEmail: "sender@test.com",
+ useTLS: false,
+ allowInsecure: true,
+ }
+ err := sender.dispatchSMTP([]string{"r@example.com"}, []byte("msg"))
+ require.Error(t, err)
+ // Guard error must NOT appear; a connection error is expected instead.
+ assert.NotContains(t, err.Error(), "SMTP auth over non-TLS connection is refused")
+}
+
// TestSMTPSender_SendToEmail_WithMockServer_AuthFailure tests auth failure
+// against a stub server that rejects AUTH. AllowInsecure is needed because
+// the stub is a local plaintext server (07-H2).
func TestSMTPSender_SendToEmail_WithMockServer_AuthFailure(t *testing.T) {
// Create server that rejects auth
server := newMockSMTPServer(t, true)
@@ -190,13 +232,14 @@ func TestSMTPSender_SendToEmail_WithMockServer_AuthFailure(t *testing.T) {
defer server.stop()
sender := &SMTPSender{
- host: "127.0.0.1",
- port: server.port,
- username: "baduser",
- password: "badpass",
- fromEmail: "sender@test.com",
- fromName: "",
- useTLS: false,
+ host: "127.0.0.1",
+ port: server.port,
+ username: "baduser",
+ password: "badpass",
+ fromEmail: "sender@test.com",
+ fromName: "",
+ useTLS: false,
+ allowInsecure: true, // test-only stub; never set in production
}
ctx := context.Background()
diff --git a/internal/email/template_renderers.go b/internal/email/template_renderers.go
index fd117bec6..a72f3d688 100644
--- a/internal/email/template_renderers.go
+++ b/internal/email/template_renderers.go
@@ -5,14 +5,27 @@ import (
"fmt"
"html/template"
"net/url"
+ texttemplate "text/template"
)
// templateFuncs provides common functions available in email templates.
+// Both html/template and text/template engines accept the same FuncMap shape;
+// the urlquery func behaves identically in both contexts.
var templateFuncs = template.FuncMap{
"urlquery": url.QueryEscape,
}
-// renderTemplate parses and executes a named template with the given data.
+// textTemplateFuncs mirrors templateFuncs for text/template so plain-text
+// renderers share the same urlquery behaviour without crossing package types.
+var textTemplateFuncs = texttemplate.FuncMap{
+ "urlquery": url.QueryEscape,
+}
+
+// renderTemplate parses and executes a named template with html/template.
+// Use this for HTML email bodies -- html/template performs context-aware
+// escaping that prevents XSS in rendered HTML. Never use it for plain-text
+// bodies: it would HTML-escape ampersands and angle-brackets in data fields
+// (e.g. names, email addresses), corrupting the plain-text output.
func renderTemplate(name, tmplText string, data any) (string, error) {
tmpl, err := template.New(name).Funcs(templateFuncs).Parse(tmplText)
if err != nil {
@@ -27,9 +40,26 @@ func renderTemplate(name, tmplText string, data any) (string, error) {
return buf.String(), nil
}
-// RenderPasswordResetEmail renders the password reset email template
+// renderTextTemplate parses and executes a named template with text/template.
+// Use this for plain-text email bodies -- text/template emits data verbatim
+// so values like "O'Brien" and "a&b@x.com" are preserved unchanged.
+func renderTextTemplate(name, tmplText string, data any) (string, error) {
+ tmpl, err := texttemplate.New(name).Funcs(textTemplateFuncs).Parse(tmplText)
+ if err != nil {
+ return "", fmt.Errorf("failed to parse template: %w", err)
+ }
+
+ var buf bytes.Buffer
+ if err := tmpl.Execute(&buf, data); err != nil {
+ return "", fmt.Errorf("failed to execute template: %w", err)
+ }
+
+ return buf.String(), nil
+}
+
+// RenderPasswordResetEmail renders the plain-text password reset email template.
func RenderPasswordResetEmail(email, resetURL string) (string, error) {
- return renderTemplate("reset", passwordResetTemplate, PasswordResetData{
+ return renderTextTemplate("reset", passwordResetTemplate, PasswordResetData{
Email: email,
ResetURL: resetURL,
})
@@ -45,9 +75,9 @@ func RenderPasswordResetEmailHTML(email, resetURL string) (string, error) {
})
}
-// RenderWelcomeEmail renders the welcome email template
+// RenderWelcomeEmail renders the plain-text welcome email template.
func RenderWelcomeEmail(email, dashboardURL, role string) (string, error) {
- return renderTemplate("welcome", welcomeUserTemplate, WelcomeUserData{
+ return renderTextTemplate("welcome", welcomeUserTemplate, WelcomeUserData{
Email: email,
DashboardURL: dashboardURL,
Role: role,
@@ -64,9 +94,9 @@ func RenderWelcomeEmailHTML(email, dashboardURL, role string) (string, error) {
})
}
-// RenderUserInviteEmail renders the user-invite email template.
+// RenderUserInviteEmail renders the plain-text user-invite email template.
func RenderUserInviteEmail(email, setupURL string) (string, error) {
- return renderTemplate("user-invite", userInviteTemplate, UserInviteData{
+ return renderTextTemplate("user-invite", userInviteTemplate, UserInviteData{
Email: email,
SetupURL: setupURL,
})
@@ -81,42 +111,42 @@ func RenderUserInviteEmailHTML(email, setupURL string) (string, error) {
})
}
-// RenderNewRecommendationsEmail renders the new recommendations email template
+// RenderNewRecommendationsEmail renders the plain-text new recommendations email template.
func RenderNewRecommendationsEmail(data NotificationData) (string, error) {
- return renderTemplate("recommendations", newRecommendationsTemplate, data)
+ return renderTextTemplate("recommendations", newRecommendationsTemplate, data)
}
-// RenderScheduledPurchaseEmail renders the scheduled purchase email template
+// RenderScheduledPurchaseEmail renders the plain-text scheduled purchase email template.
func RenderScheduledPurchaseEmail(data NotificationData) (string, error) {
- return renderTemplate("scheduled", scheduledPurchaseTemplate, data)
+ return renderTextTemplate("scheduled", scheduledPurchaseTemplate, data)
}
-// RenderPurchaseConfirmationEmail renders the purchase confirmation email template
+// RenderPurchaseConfirmationEmail renders the plain-text purchase confirmation email template.
func RenderPurchaseConfirmationEmail(data NotificationData) (string, error) {
- return renderTemplate("confirmation", purchaseConfirmationTemplate, data)
+ return renderTextTemplate("confirmation", purchaseConfirmationTemplate, data)
}
-// RenderPurchaseFailedEmail renders the purchase failed email template
+// RenderPurchaseFailedEmail renders the plain-text purchase failed email template.
func RenderPurchaseFailedEmail(data NotificationData) (string, error) {
- return renderTemplate("failed", purchaseFailedTemplate, data)
+ return renderTextTemplate("failed", purchaseFailedTemplate, data)
}
-// RenderRIExchangePendingApprovalEmail renders the RI exchange pending approval email template
+// RenderRIExchangePendingApprovalEmail renders the plain-text RI exchange pending approval email template.
func RenderRIExchangePendingApprovalEmail(data RIExchangeNotificationData) (string, error) {
- return renderTemplate("ri-exchange-pending", riExchangePendingApprovalTemplate, data)
+ return renderTextTemplate("ri-exchange-pending", riExchangePendingApprovalTemplate, data)
}
-// RenderRIExchangeCompletedEmail renders the RI exchange completed email template
+// RenderRIExchangeCompletedEmail renders the plain-text RI exchange completed email template.
func RenderRIExchangeCompletedEmail(data RIExchangeNotificationData) (string, error) {
- return renderTemplate("ri-exchange-completed", riExchangeCompletedTemplate, data)
+ return renderTextTemplate("ri-exchange-completed", riExchangeCompletedTemplate, data)
}
// RenderPurchaseApprovalRequestEmail renders the plain-text purchase
// approval request email template. Issue #287: this is the multipart
-// `text/plain` half — pair with RenderPurchaseApprovalRequestEmailHTML
+// text/plain half -- pair with RenderPurchaseApprovalRequestEmailHTML
// for the styled HTML half.
func RenderPurchaseApprovalRequestEmail(data NotificationData) (string, error) {
- return renderTemplate("purchase-approval-request", purchaseApprovalRequestTemplate, data)
+ return renderTextTemplate("purchase-approval-request", purchaseApprovalRequestTemplate, data)
}
// RenderPurchaseApprovalRequestEmailHTML renders the HTML half of the
@@ -129,12 +159,12 @@ func RenderPurchaseApprovalRequestEmailHTML(data NotificationData) (string, erro
return renderTemplate("purchase-approval-request-html", purchaseApprovalRequestHTMLTemplate, data)
}
-// RenderRegistrationReceivedEmail renders the admin notification for a new registration.
+// RenderRegistrationReceivedEmail renders the plain-text admin notification for a new registration.
func RenderRegistrationReceivedEmail(data RegistrationNotificationData) (string, error) {
- return renderTemplate("registration-received", registrationReceivedTemplate, data)
+ return renderTextTemplate("registration-received", registrationReceivedTemplate, data)
}
-// RenderRegistrationDecisionEmail renders the registrant notification for approval/rejection.
+// RenderRegistrationDecisionEmail renders the plain-text registrant notification for approval/rejection.
func RenderRegistrationDecisionEmail(data RegistrationDecisionData) (string, error) {
- return renderTemplate("registration-decision", registrationDecisionTemplate, data)
+ return renderTextTemplate("registration-decision", registrationDecisionTemplate, data)
}
diff --git a/internal/email/template_renderers_test.go b/internal/email/template_renderers_test.go
index 57a842bca..a6827ac6a 100644
--- a/internal/email/template_renderers_test.go
+++ b/internal/email/template_renderers_test.go
@@ -496,3 +496,77 @@ func TestRenderWelcomeEmailHTML(t *testing.T) {
assert.Contains(t, html, "Hello carol@example.com,")
assert.Contains(t, html, "admin")
}
+
+// TestPlainTextTemplates_NoHTMLEscaping asserts that special characters in data
+// fields (ampersand, apostrophe, angle brackets) survive verbatim in plain-text
+// email bodies. Previously all renderers used html/template which emitted HTML
+// entities (&, ', <) in plain-text output (07-H1 regression).
+func TestPlainTextTemplates_NoHTMLEscaping(t *testing.T) {
+ // RequestedByName with apostrophe and email with ampersand -- classic
+ // html/template escape targets. They must be preserved verbatim in
+ // plain-text bodies so recipients can copy-paste the address.
+ const name = "O'Brien"
+ const email = "a&b@example.com"
+ const reason = "Account name "
+
+ t.Run("PasswordReset_email_verbatim", func(t *testing.T) {
+ body, err := RenderPasswordResetEmail(email, "https://example.com/reset")
+ require.NoError(t, err)
+ assert.Contains(t, body, email, "plain-text password reset must preserve & in address")
+ assert.NotContains(t, body, "&")
+ })
+
+ t.Run("UserInvite_email_verbatim", func(t *testing.T) {
+ body, err := RenderUserInviteEmail(email, "https://example.com/setup")
+ require.NoError(t, err)
+ assert.Contains(t, body, email)
+ assert.NotContains(t, body, "&")
+ })
+
+ t.Run("RegistrationDecision_rejection_reason_verbatim", func(t *testing.T) {
+ data := RegistrationDecisionData{
+ AccountName: "ACME & Co",
+ Decision: "Rejected",
+ RejectionReason: reason,
+ }
+ body, err := RenderRegistrationDecisionEmail(data)
+ require.NoError(t, err)
+ assert.Contains(t, body, reason, "rejection reason must survive verbatim in plain-text body")
+ assert.NotContains(t, body, "&")
+ assert.NotContains(t, body, "<")
+ assert.NotContains(t, body, "'")
+ })
+
+ t.Run("PurchaseApprovalRequest_requestedByName_verbatim", func(t *testing.T) {
+ data := NotificationData{
+ DashboardURL: "https://example.com",
+ ApprovalToken: "tok",
+ ExecutionID: "exec-1",
+ RequestedByName: name,
+ RequestedByEmail: email,
+ Recommendations: []RecommendationSummary{{Service: "ec2", ResourceType: "m5.large", Region: "us-east-1", Count: 1}},
+ }
+ body, err := RenderPurchaseApprovalRequestEmail(data)
+ require.NoError(t, err)
+ assert.Contains(t, body, name, "RequestedByName with apostrophe must be verbatim in plain-text approval")
+ assert.Contains(t, body, email, "RequestedByEmail with & must be verbatim in plain-text approval")
+ assert.NotContains(t, body, "'")
+ assert.NotContains(t, body, "&")
+ })
+
+ t.Run("HTML_renderers_still_escape", func(t *testing.T) {
+ // HTML halves MUST still escape data -- that is the XSS defence.
+ // A name with a script tag must not survive literally in the HTML body.
+ const xssName = ``
+ data := NotificationData{
+ DashboardURL: "https://example.com",
+ ApprovalToken: "tok",
+ ExecutionID: "exec-2",
+ RequestedByName: xssName,
+ Recommendations: []RecommendationSummary{{Service: "ec2", ResourceType: "m5.large", Region: "us-east-1", Count: 1}},
+ }
+ html, err := RenderPurchaseApprovalRequestEmailHTML(data)
+ require.NoError(t, err)
+ assert.NotContains(t, html, xssName, "html/template must escape