fix(notifications): stop logging PII in email body preview#31
Open
Thosine-01 wants to merge 7 commits into
Open
fix(notifications): stop logging PII in email body preview#31Thosine-01 wants to merge 7 commits into
Thosine-01 wants to merge 7 commits into
Conversation
Contributor
|
Appreciate the direction + the spec coverage, but I noticed the original |
The outer if (emailPreviewEnabled) block was missing its closing brace, causing a TS1005 error because catch was orphaned from try. Co-Authored-By: Claude <noreply@anthropic.com>
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
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
EmailService.sendwas logging the full rendered email body (via the dev-onlyJSON transport's
info.message) at debug level. That payload includes donornames, donation amounts, campaign titles, and recipient addresses — all of
which can leak into log aggregators (Datadog, Loki, CloudWatch) if
LOG_LEVEL=debugis set, even outside production.This PR removes that exposure per the issue's recommendations.
Changes
this.logger.debug(\Email body preview: ${(info as any).message}`)` call — the HTML body is never logged again, under any condition.maskEmail()helper, e.g.donor@example.com→do***@example.com.EMAIL_PREVIEW=1env flag. When enabled, it logs onlysubject+ masked recipient — never the body.NODE_ENV=production, regardless of howEMAIL_PREVIEWis set, as a safety net against misconfiguration.Log lines like
Email sent to ${options.to}now readEmail sent to ${maskEmail(options.to)}.If anything downstream (dashboards, alerting, grep-based tooling) was parsing
full email addresses out of these logs, it will need updating to expect the
masked format instead.
Testing
Added
email.service.spec.tswith 6 tests covering:jsonTransportreturns it oninfo.messageEMAIL_PREVIEWunset)EMAIL_PREVIEW=1in non-productionNODE_ENV=production, even ifEMAIL_PREVIEW=1Close #4