sec(email): stop broadcasting approval tokens via SNS (closes #1015) - #1036
Merged
cristim merged 4 commits intoJun 7, 2026
Merged
Conversation
…through targeted SES, not SNS broadcast (closes #1015) SendScheduledPurchaseNotification and SendRIExchangePendingApproval on the SES/SNS Sender previously rendered bodies that embed live approve/pause/cancel tokens and shipped them via SendNotification (SNS broadcast). Every topic subscriber received working action links, matching the exact threat the purchase-approval path was hardened against in sender.go:386-391 and templates.go:698-704. Fix: - Both methods now require data.RecipientEmail and route through SendToEmailWithCC (targeted SES), mirroring SendPurchaseApprovalRequest. Return ErrNoRecipient when the field is empty rather than silently broadcasting. - Add RecipientEmail/CCEmails fields to RIExchangeNotificationData. - Update call sites (purchase/notifications.go, handler_ri_exchange.go) to resolve GlobalConfig.NotificationEmail and pass it as RecipientEmail. When the global email is unconfigured, log a warning and skip the notification rather than broadcast. - Add structural guard in SendNotification: reject any message body that contains "token=" with ErrTokenInBroadcast, preventing future regressions regardless of how a token-bearing body reaches the broadcast path. - Add 8 regression tests: guard rejects token bodies, guard allows clean bodies, both methods use SES not SNS when RecipientEmail is set, both methods return ErrNoRecipient when RecipientEmail is absent.
Member
Author
|
@coderabbitai review |
Contributor
|
Warning Review limit reached
More reviews will be available in 15 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR hardens email notification security by preventing approval token broadcast via SNS. Scheduled purchase and RI exchange notifications with token-bearing approval links now route through targeted SES delivery to a configured recipient, with a broadcast guard rejecting token-containing messages and integration of global notification email configuration. ChangesApproval Token Broadcast Hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
internal/email/templates_test.go (1)
49-49: 💤 Low valueRemove no-op cleanup function.
t.Cleanup(func() {})does nothing and appears to be leftover from a refactor. The actual mock assertions cleanup is on line 55.🧹 Suggested fix
- t.Cleanup(func() {})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/email/templates_test.go` at line 49, Remove the no-op test cleanup call by deleting the stray t.Cleanup(func() {}) invocation in the test; rely on the existing mock assertion/cleanup logic (e.g., the mock expectations teardown already present) instead so there is no empty cleanup function left in templates_test.go.internal/email/sender.go (1)
116-118: ⚡ Quick winConsider case-insensitive token detection for defense-in-depth.
The guard currently uses
strings.Contains(message, "token=")which is case-sensitive. While all templates use lowercase "token=", making this check case-insensitive would provide additional safety against future template changes or edge cases.🔒 Proposed case-insensitive guard
- if strings.Contains(message, "token=") { + if strings.Contains(strings.ToLower(message), "token=") { return ErrTokenInBroadcast }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/email/sender.go` around lines 116 - 118, The current guard uses a case-sensitive check strings.Contains(message, "token=") which can miss mixed-case tokens; update the check in internal/email/sender.go so the message is checked case-insensitively (e.g., normalize message to lowercase or use a case-insensitive comparison) and still return ErrTokenInBroadcast when found; ensure you update the same guard that references ErrTokenInBroadcast and any associated unit tests that rely on this behavior.internal/server/handler_ri_exchange_test.go (1)
102-102: ⚡ Quick winConsider adding test coverage for configured NotificationEmail.
All tests pass an empty string
""for thenotifyEmailparameter. While this verifies the "no recipient configured" path, there's no test coverage for the happy path wherecfg.NotificationEmailis actually set to a valid email address.Consider adding a test case (optional, not blocking) that verifies:
- When
cfg.NotificationEmailis set to a valid email (e.g.,"admin@example.com")executeRIExchangeReshapederives and passes that email through- For manual mode with pending exchanges, the email is successfully routed to
SendRIExchangePendingApprovalwithRecipientEmailpopulatedThis would provide end-to-end confidence that the notification email configuration flows correctly through the handler layer.
Also applies to: 124-124, 255-255, 270-270, 294-294, 318-318, 341-341
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/server/handler_ri_exchange_test.go` at line 102, Add a test that sets cfg.NotificationEmail to a valid address (e.g., "admin@example.com") instead of passing "" into buildExchangeNotificationData, invoke executeRIExchangeReshape in the manual/pending-exchange scenario, and assert that SendRIExchangePendingApproval is called with a Notification/RecipientEmail field populated accordingly; specifically, update the test around buildExchangeNotificationData usage to supply the configured email, exercise executeRIExchangeReshape, and verify the stub/mock for SendRIExchangePendingApproval received RecipientEmail matching cfg.NotificationEmail.internal/server/handler_ri_exchange.go (1)
143-167: Fix RI exchange completion:RecipientEmailis ignored; completion uses SNS broadcast
SendRIExchangeCompletedignoresRIExchangeNotificationData.RecipientEmailand always callss.SendNotification(...)(SNS broadcast path). The completed email template contains notoken=links, so it won’t trip theErrTokenInBroadcastguard.- Delivery target is driven by the transport: SES/SNS goes to the SNS topic; SMTP goes to the SMTP sender’s configured
notifyEmail(also independent ofdata.RecipientEmail).- The only path that must use
RecipientEmailisSendRIExchangePendingApproval, which returnsErrNoRecipientwhenRecipientEmailis empty.Suggested clean-up if you want `RecipientEmail` to match the struct contract
- data := buildExchangeNotificationData(result, app.appConfig.DashboardURL, notifyEmail) + data := buildExchangeNotificationData(result, app.appConfig.DashboardURL, "") var err error if result.Mode == "manual" && len(result.Pending) > 0 { + data.RecipientEmail = notifyEmail err = app.Email.SendRIExchangePendingApproval(ctx, data) } else if len(result.Completed)+len(result.Failed) > 0 { err = app.Email.SendRIExchangeCompleted(ctx, data) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/server/handler_ri_exchange.go` around lines 143 - 167, The current sendExchangeNotification sets RecipientEmail for all notifications but SendRIExchangeCompleted ignores it and always broadcasts via SNS; to fix, only populate RIExchangeNotificationData.RecipientEmail when calling SendRIExchangePendingApproval and ensure for the completed path you clear or omit RecipientEmail so SendRIExchangeCompleted behaves as intended (SNS broadcast/transport-driven delivery). Update sendExchangeNotification (and buildExchangeNotificationData usage) so RecipientEmail is only set for the manual pending-approval branch and not for the completed/failed branch; if you need per-recipient completed emails instead, modify SendRIExchangeCompleted to accept/respect RecipientEmail in RIExchangeNotificationData.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/email/sender.go`:
- Around line 116-118: The current guard uses a case-sensitive check
strings.Contains(message, "token=") which can miss mixed-case tokens; update the
check in internal/email/sender.go so the message is checked case-insensitively
(e.g., normalize message to lowercase or use a case-insensitive comparison) and
still return ErrTokenInBroadcast when found; ensure you update the same guard
that references ErrTokenInBroadcast and any associated unit tests that rely on
this behavior.
In `@internal/email/templates_test.go`:
- Line 49: Remove the no-op test cleanup call by deleting the stray
t.Cleanup(func() {}) invocation in the test; rely on the existing mock
assertion/cleanup logic (e.g., the mock expectations teardown already present)
instead so there is no empty cleanup function left in templates_test.go.
In `@internal/server/handler_ri_exchange_test.go`:
- Line 102: Add a test that sets cfg.NotificationEmail to a valid address (e.g.,
"admin@example.com") instead of passing "" into buildExchangeNotificationData,
invoke executeRIExchangeReshape in the manual/pending-exchange scenario, and
assert that SendRIExchangePendingApproval is called with a
Notification/RecipientEmail field populated accordingly; specifically, update
the test around buildExchangeNotificationData usage to supply the configured
email, exercise executeRIExchangeReshape, and verify the stub/mock for
SendRIExchangePendingApproval received RecipientEmail matching
cfg.NotificationEmail.
In `@internal/server/handler_ri_exchange.go`:
- Around line 143-167: The current sendExchangeNotification sets RecipientEmail
for all notifications but SendRIExchangeCompleted ignores it and always
broadcasts via SNS; to fix, only populate
RIExchangeNotificationData.RecipientEmail when calling
SendRIExchangePendingApproval and ensure for the completed path you clear or
omit RecipientEmail so SendRIExchangeCompleted behaves as intended (SNS
broadcast/transport-driven delivery). Update sendExchangeNotification (and
buildExchangeNotificationData usage) so RecipientEmail is only set for the
manual pending-approval branch and not for the completed/failed branch; if you
need per-recipient completed emails instead, modify SendRIExchangeCompleted to
accept/respect RecipientEmail in RIExchangeNotificationData.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92d2e872-e638-4260-be2b-d1f4e162b494
📒 Files selected for processing (10)
internal/email/coverage_extra_test.gointernal/email/coverage_test.gointernal/email/sender.gointernal/email/sender_test.gointernal/email/templates.gointernal/email/templates_test.gointernal/purchase/notifications.gointernal/purchase/notifications_test.gointernal/server/handler_ri_exchange.gointernal/server/handler_ri_exchange_test.go
11 tasks
…pient contract Address CodeRabbit pass-1 nitpicks on PR #1036: - sender.go: make the SNS token-broadcast guard case-insensitive so a future template that capitalizes the query param (Token=, TOKEN=) cannot slip a token-bearing body past the guard. Add mixed-case regression cases to TestSendNotification_RejectsTokenBearingBody (fails pre-fix, passes post-fix). - handler_ri_exchange.go: only populate RIExchangeNotificationData.RecipientEmail for the pending-approval branch. The completed/failed path is transport-driven (SNS broadcast / SMTP notifyEmail) and ignores RecipientEmail, so setting it there misrepresented the struct contract. Update the buildExchangeNotificationData doc comment accordingly. - handler_ri_exchange_test.go: add coverage for a configured NotificationEmail, asserting the pending branch receives RecipientEmail populated and the completed branch leaves it empty. - templates_test.go: drop the leftover no-op t.Cleanup(func() {}).
Member
Author
|
Addressed CR pass-1 nitpicks in
gofmt/vet/build clean; 659 tests pass across @coderabbitai review |
Contributor
|
🧠 Learnings used✅ Action performedReview finished.
|
… over non-TLS (07-H1, 07-H2) Plain-text email renderers were using html/template which HTML-encodes special characters in data fields -- "O'Brien" rendered as "O'Brien" and "a&b@x.com" as "a&b@x.com" in recipients' inboxes (07-H1). Add renderTextTemplate backed by text/template and route all non-HTML renderers through it. HTML renderers retain html/template for XSS safety. Regression tests assert that &, ', and < survive verbatim in plain-text bodies and that html/template still escapes <script> in HTML bodies. Also guard dispatchSMTP against cleartext credential transmission (07-H2): if SMTP auth is configured but UseTLS is false the call is refused unless AllowInsecure is explicitly set. AllowInsecure is test-only (local SMTP stubs); factory constructors all use UseTLS=true and never set it. Update smtp_server_test.go tests that used plaintext auth stubs to set AllowInsecure, and add TestSMTPSender_DispatchSMTP_RefusesAuthOverCleartext regression. Also: per-message random MIME boundary via crypto/rand (07-N2); doc note on SMTP SendNotification no-op for GCP/Azure broadcast (07-N4). Closes #1077 (07-H1, 07-H2, 07-N2, 07-N4)
…xt heuristic (07-M1--M3, 07-L2--L3)
07-M3: SendNotification now passes the subject through sanitizeHeader
(strips CR/LF to prevent SNS InvalidParameter) and truncates to 100 bytes
(SNS hard limit) before Publish. A PlanName containing a newline would
previously cause a runtime InvalidParameter error. Regression tests cover
newline stripping and byte truncation.
07-M2: ensureSandboxRecipientVerified user-facing error retains the raw
recipient address (needed for the recipient to know which inbox to verify)
but the second fmt.Errorf no longer duplicates the address in a gratuitous
"sent to %s" suffix. Doc comments clarify the PII-vs-usability tradeoff.
07-M1: doc comments on buildSESSendEmailInput{Multipart} note that SES v2
structured API is not CRLF-injectable via Subject.Data; any future raw-MIME
path must call sanitizeHeader before header composition.
07-L2: replace hand-rolled containsColon loop with strings.Contains.
Delete containsColon helper.
07-L3: replace the weak length+colon/slash heuristic in warnIfPlaintext with
an explicit isSecretManagerReference check (HasPrefix "arn:", "projects/",
Contains ".vault.azure.net/", HasPrefix "/"). A 25-char password with a colon
no longer silently skips the warning. Update coverage_extra_test.go to test
isSecretManagerReference instead of the deleted containsColon.
Closes #1077 (07-M1, 07-M2, 07-M3, 07-L2, 07-L3)
Member
Author
FOLD-1036 additions (07-H1, 07-H2, 07-M1--M3, 07-L2--L3, 07-N2, 07-N4) -- issue #1077Two commits added on top of the C1 fix (commits 439bb3b, 51796db): commit 439bb3b --
commit 51796db --
Build/vet/test: 334 passed in @coderabbitai review |
Contributor
|
🧠 Learnings used✅ Action performedReview finished.
|
cristim
added a commit
that referenced
this pull request
Jun 8, 2026
#296) Mirror the purchaseApprovalRequestHTMLTemplate pattern for the SendRIExchangePendingApproval flow: - Add riExchangePendingApprovalHTMLTemplate: inline-styled table of exchanges with per-row Approve/Reject CTA buttons, summary block, optional requested-by line, configurable cancellation-window note, and skipped-exchanges section. - Extend riExchangePendingApprovalTemplate (plaintext): labelled Approve/Reject URLs (replacing bracket notation), optional requested-by block, configurable cancellation-window note. - Extend RIExchangeNotificationData with RequestedByName, RequestedByEmail, RequestedAt, CancellationWindowNote fields alongside the RecipientEmail/CCEmails targeting fields. - Add RenderRIExchangePendingApprovalEmailHTML renderer. - Add sendRIExchangePendingApprovalVia helper (shared by Sender and SMTPSender, non-fatal HTML degrade on render error), now CC-aware. - Sender.SendRIExchangePendingApproval: requires RecipientEmail and routes through targeted SES multipart (SendToEmailWithCCMultipart), never broadcasting approval tokens via the SNS topic. - SMTPSender.SendRIExchangePendingApproval: resolves RecipientEmail (falling back to the static notify address), fails loud with ErrNoRecipient when neither is configured, then sends multipart. - 7 new test cases covering both halves, empty/custom cancellation notes, missing requested-by, and skipped-exchanges rendering. Rebased onto feat/multicloud-web-frontend: reconciled with the #1015 / #1036 hardening that stops broadcasting approval tokens via SNS. The multipart send now preserves that fail-loud, targeted-SES contract instead of falling back to the SNS broadcast topic. Updated the SMTP no-recipient coverage test to assert ErrNoRecipient accordingly.
7 tasks
cristim
added a commit
that referenced
this pull request
Jun 19, 2026
#296) Mirror the purchaseApprovalRequestHTMLTemplate pattern for the SendRIExchangePendingApproval flow: - Add riExchangePendingApprovalHTMLTemplate: inline-styled table of exchanges with per-row Approve/Reject CTA buttons, summary block, optional requested-by line, configurable cancellation-window note, and skipped-exchanges section. - Extend riExchangePendingApprovalTemplate (plaintext): labelled Approve/Reject URLs (replacing bracket notation), optional requested-by block, configurable cancellation-window note. - Extend RIExchangeNotificationData with RequestedByName, RequestedByEmail, RequestedAt, CancellationWindowNote fields alongside the RecipientEmail/CCEmails targeting fields. - Add RenderRIExchangePendingApprovalEmailHTML renderer. - Add sendRIExchangePendingApprovalVia helper (shared by Sender and SMTPSender, non-fatal HTML degrade on render error), now CC-aware. - Sender.SendRIExchangePendingApproval: requires RecipientEmail and routes through targeted SES multipart (SendToEmailWithCCMultipart), never broadcasting approval tokens via the SNS topic. - SMTPSender.SendRIExchangePendingApproval: resolves RecipientEmail (falling back to the static notify address), fails loud with ErrNoRecipient when neither is configured, then sends multipart. - 7 new test cases covering both halves, empty/custom cancellation notes, missing requested-by, and skipped-exchanges rendering. Rebased onto feat/multicloud-web-frontend: reconciled with the #1015 / #1036 hardening that stops broadcasting approval tokens via SNS. The multipart send now preserves that fail-loud, targeted-SES contract instead of falling back to the SNS broadcast topic. Updated the SMTP no-recipient coverage test to assert ErrNoRecipient accordingly.
cristim
added a commit
that referenced
this pull request
Jul 10, 2026
#296) Mirror the purchaseApprovalRequestHTMLTemplate pattern for the SendRIExchangePendingApproval flow: - Add riExchangePendingApprovalHTMLTemplate: inline-styled table of exchanges with per-row Approve/Reject CTA buttons, summary block, optional requested-by line, configurable cancellation-window note, and skipped-exchanges section. - Extend riExchangePendingApprovalTemplate (plaintext): labelled Approve/Reject URLs (replacing bracket notation), optional requested-by block, configurable cancellation-window note. - Extend RIExchangeNotificationData with RequestedByName, RequestedByEmail, RequestedAt, CancellationWindowNote fields alongside the RecipientEmail/CCEmails targeting fields. - Add RenderRIExchangePendingApprovalEmailHTML renderer. - Add sendRIExchangePendingApprovalVia helper (shared by Sender and SMTPSender, non-fatal HTML degrade on render error), now CC-aware. - Sender.SendRIExchangePendingApproval: requires RecipientEmail and routes through targeted SES multipart (SendToEmailWithCCMultipart), never broadcasting approval tokens via the SNS topic. - SMTPSender.SendRIExchangePendingApproval: resolves RecipientEmail (falling back to the static notify address), fails loud with ErrNoRecipient when neither is configured, then sends multipart. - 7 new test cases covering both halves, empty/custom cancellation notes, missing requested-by, and skipped-exchanges rendering. Rebased onto feat/multicloud-web-frontend: reconciled with the #1015 / #1036 hardening that stops broadcasting approval tokens via SNS. The multipart send now preserves that fail-loud, targeted-SES contract instead of falling back to the SNS broadcast topic. Updated the SMTP no-recipient coverage test to assert ErrNoRecipient accordingly.
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SendScheduledPurchaseNotificationandSendRIExchangePendingApprovalon the SES/SNSSenderpreviously routed token-bearing bodies throughSendNotification(SNS broadcast), leaking working approve/pause/cancel/reject links to every topic subscriber.RecipientEmailand route throughSendToEmailWithCC(targeted SES), exactly mirroring the already-hardenedSendPurchaseApprovalRequestpath.SendNotificationrejects any body containingtoken=withErrTokenInBroadcast, preventing future regressions mechanically.RIExchangeNotificationDatagainsRecipientEmail/CCEmailsfields; call sites resolveGlobalConfig.NotificationEmailand pass it as the recipient.Closes #1015
Changes
internal/email/sender.go: addErrTokenInBroadcastsentinel; add guard inSendNotification; addRecipientEmail/CCEmailstoRIExchangeNotificationDatainternal/email/templates.go: fixSendScheduledPurchaseNotificationandSendRIExchangePendingApprovalto gate onRecipientEmailand route viaSendToEmailWithCCinternal/purchase/notifications.go: fetchGlobalConfig.NotificationEmailinsendPlanNotificationand pass it asRecipientEmail; log warning and skip when unconfiguredinternal/server/handler_ri_exchange.go: passcfg.NotificationEmailthroughsendExchangeNotificationtobuildExchangeNotificationDataErrNoRecipientwhen recipient absentTest plan
go build ./...cleango vet ./internal/email/...cleango test ./internal/email/... ./internal/purchase/... ./internal/server/...848 passedTestSendNotification_RejectsTokenBearingBody: guard blocks alltoken=patternsTestSendScheduledPurchaseNotification_UsesSESNotSNS: SNS mock has no expected calls, SES mock receives the sendTestSendRIExchangePendingApproval_UsesSESNotSNS: same pattern for RI exchangeTestSend*_ErrNoRecipientWhenEmpty: both methods returnErrNoRecipientwith emptyRecipientEmailSummary by CodeRabbit
Bug Fixes
Tests
Scope expanded
Additional findings from report 07 folded into this PR (issue #1077):
07-H1 (
template_renderers.go): Plain-text email bodies were rendered withhtml/template, HTML-encoding&,',<,>in data fields.O'Brienappeared asO'Brien;a&b@x.comasa&b@x.com. AddedrenderTextTemplate(backed bytext/template) and routed all non-HTML renderers through it. HTML renderers retainhtml/templatefor XSS safety. Regression tests assert verbatim survival of special characters in plain-text bodies and continued escaping in HTML bodies.07-H2 (
smtp_sender.go):dispatchSMTPnow refuses to send SMTP AUTH over a non-TLS connection (auth != nil && !useTLS && !allowInsecure). AnAllowInsecureescape hatch (test-only) is added toSMTPConfigandSMTPSender; factory constructors never set it. Updated integration tests that used plaintext auth stubs to passallowInsecure: true. AddedTestSMTPSender_DispatchSMTP_RefusesAuthOverCleartextregression.07-M3 (
sender.go):SendNotificationpasses the SNS subject throughsanitizeHeader(strips CR/LF) and truncates to 100 bytes (SNS hard limit) beforePublish. APlanNamewith a newline would previously cause a runtimeInvalidParameter. Regression tests cover newline stripping and byte truncation.07-M2 (
sender.go):ensureSandboxRecipientVerifiederror messages retain the full address in the user-facing string (needed for the recipient to know which inbox to verify) but drop the gratuitous address duplication in the second error. Doc comments clarify the PII/usability tradeoff.07-M1 (
sender.go): Doc comments onbuildSESSendEmailInput{Multipart}note that the SES v2 structured API is not CRLF-injectable; any future raw-MIME path must callsanitizeHeader.07-L2 (
factory.go): Replaced hand-rolledcontainsColonloop withstrings.Contains. Deleted the helper.07-L3 (
factory.go): Replaced the weak length+colon/slash heuristic inwarnIfPlaintextwith an explicitisSecretManagerReferencecheck (prefixesarn:,projects/, contains.vault.azure.net/, prefix/). A 25-char password with a colon no longer silently skips the warning.07-N2 (
smtp_sender.go):buildSMTPMessageMultipartnow generates a per-message random MIME boundary viacrypto/randinstead of the fixed literal, eliminating the theoretical body-collision risk.07-N4 (
smtp_sender.go): ExpandedSMTPSender.SendNotificationdoc comment to explain that GCP/Azure deployments receive no broadcast notifications (SMTP has no pub/sub).Also closes #1077