Skip to content

fix(sender/email): don't log email body, log size instead#285

Merged
umputun merged 1 commit into
masterfrom
fix/email-sender-redact-body
May 8, 2026
Merged

fix(sender/email): don't log email body, log size instead#285
umputun merged 1 commit into
masterfrom
fix/email-sender-redact-body

Conversation

@paskal
Copy link
Copy Markdown
Collaborator

@paskal paskal commented May 8, 2026

Summary

The verify provider sends one-shot magic-link tokens via email. The Email sender's debug log dumped the full body verbatim:

[DEBUG] send "Confirmation token: <jwt>" to victim@example.com

Anyone with log access (centralized logging, crash bundles, support tools, mail-gateway observability) could redeem that token within its TTL — independently of the legitimate recipient. PR #281 (verify replay) limits reuse to one consumption, but the log reader can still race the user for that one consumption. This PR closes the leak at the source.

Change

Log the recipient and body length only:

[DEBUG] send 142-byte message to victim@example.com

Same redaction in v1 (provider/sender/email.go:84) and v2 (v2/provider/sender/email.go:84), single PR.

Test

TestEmail_SendDoesNotLogBody captures logger output, sends a known-secret body to a non-existent SMTP host (Send fails fast — that's fine, we only assert on the log line), and verifies:

  • the body substring is absent from the log
  • the recipient address is present (operators still need this for ops)

Added in both modules. Full go test -race ./... green; golangci-lint run --new-from-rev=master 0 issues.

The verify provider sends one-shot magic-link tokens by email; the
Email sender's debug log dumped the full body verbatim:

    [DEBUG] send "Confirmation token: <jwt>" to victim@example.com

Anyone with log access (centralized logging, crash bundles, support
tools, mail-gateway-adjacent observability) could redeem that token
within its TTL — independently of the legitimate recipient. The
verify-replay PR (#281) limits reuse to one consumption, but the
log-reader can still race the user for that one consumption.

Log only the recipient and the body length:

    [DEBUG] send 142-byte message to victim@example.com

Same fix in v1 (provider/sender/email.go:84) and v2
(v2/provider/sender/email.go:84), single PR.

Test: TestEmail_SendDoesNotLogBody captures logger output, sends a
known-secret body to a non-existent SMTP host, and asserts the body
substring is absent while the recipient is present. Added in both
modules.
@paskal paskal requested a review from umputun as a code owner May 8, 2026 18:31
@coveralls
Copy link
Copy Markdown

coveralls commented May 8, 2026

Coverage Report for CI Build 25572603805

Coverage increased (+0.4%) to 84.66%

Details

  • Coverage increased (+0.4%) from the base build.
  • Patch coverage: 1 of 1 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 3305
Covered Lines: 2798
Line Coverage: 84.66%
Coverage Strength: 7.77 hits per line

💛 - Coveralls

Copy link
Copy Markdown
Member

@umputun umputun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

one small nit: the new test uses func(format string, args ...interface{}) for the capturing logger, but #283 (just merged) modernized all existing interface{} to any across the codebase. Worth swapping the new test's signature to args ...any here too, otherwise it's a tiny regression on the modernization. Not blocking.

@umputun umputun merged commit fbfe635 into master May 8, 2026
9 checks passed
@umputun umputun deleted the fix/email-sender-redact-body branch May 8, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants