Skip to content

[PM-37648] Fix double HTML encoding in emergency access emails - #7652

Merged
addisonbeck merged 4 commits into
bitwarden:mainfrom
singhvishalkr:fix-emergency-access-email-encoding
Jul 13, 2026
Merged

[PM-37648] Fix double HTML encoding in emergency access emails#7652
addisonbeck merged 4 commits into
bitwarden:mainfrom
singhvishalkr:fix-emergency-access-email-encoding

Conversation

@singhvishalkr

Copy link
Copy Markdown
Contributor

Fixes #4845

When sending emergency access emails, names with special characters like German umlauts appear as HTML entities (e.g., Stephan Windmüller instead of Stephan Windmüller).

The root cause is double HTML encoding:

  1. CoreHelpers.SanitizeForEmail(name) encodes the name (htmlEncode defaults to true)
  2. Handlebars templates also escape values when rendering

Other email methods in the same file already pass false to avoid this (e.g., SendProviderInviteEmailAsync). This change applies the same pattern to all emergency access email methods:

  • SendEmergencyAccessInviteEmailAsync
  • SendEmergencyAccessConfirmedEmailAsync
  • SendEmergencyAccessRecoveryInitiated
  • SendEmergencyAccessRecoveryApproved
  • SendEmergencyAccessRecoveryRejected
  • SendEmergencyAccessRecoveryReminder
  • SendEmergencyAccessRecoveryTimedOut

Tested by reviewing the code path and confirming the fix matches existing patterns.

@singhvishalkr
singhvishalkr requested a review from a team as a code owner May 16, 2026 06:40
@singhvishalkr
singhvishalkr requested a review from dereknance May 16, 2026 06:40
@CLAassistant

CLAassistant commented May 16, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@bitwarden-bot

Copy link
Copy Markdown
Collaborator

Thank you for your contribution! We've added this to our internal tracking system for review.
ID: PM-37648
Link: https://bitwarden.atlassian.net/browse/PM-37648

Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process.

@bitwarden-bot bitwarden-bot changed the title Fix double HTML encoding in emergency access emails [PM-37648] Fix double HTML encoding in emergency access emails May 16, 2026
@addisonbeck

Copy link
Copy Markdown
Contributor

SendEmergencyAccessConfirmedEmailAsync
SendEmergencyAccessRecoveryApproved
SendEmergencyAccessRecoveryReminder

It doesn't look like these are actually changed.

@addisonbeck
addisonbeck requested review from addisonbeck and removed request for dereknance June 15, 2026 17:53

@addisonbeck addisonbeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see above

@singhvishalkr

Copy link
Copy Markdown
Contributor Author

Current head now changes the three emergency-access mail paths the review called out (SendEmergencyAccessConfirmedEmailAsync, SendEmergencyAccessRecoveryApproved, and SendEmergencyAccessRecoveryReminder) and adds a regression test that checks the rendered HTML encodes the user-facing name exactly once.

@djsmith85
djsmith85 requested a review from addisonbeck June 16, 2026 14:45
The SanitizeForEmail method HTML-encodes by default, but Handlebars
templates also escape values. This caused names with special characters
like umlauts to display as HTML entities (e.g. Windm&bitwarden#252;ller).

Pass htmlEncode=false to match the pattern used elsewhere in this file.
@singhvishalkr
singhvishalkr force-pushed the fix-emergency-access-email-encoding branch from b6cbc1c to 5554d6c Compare June 16, 2026 15:00
@singhvishalkr

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and resolved the test-file conflict in 5554d6c16. The three emergency-access paths from the review are covered by the new single-encoding regression test, while the upstream cloud-region URL test remains intact.

Local checks here were limited to git diff --check and static review because this shell does not have dotnet installed.

@addisonbeck

Copy link
Copy Markdown
Contributor

I'll get this passed over to QA. Once that passes, we'll get it merged. Thanks for the contribution!

@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.53%. Comparing base (0fce684) to head (549ddd6).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/Core/Platform/Mail/HandlebarsMailService.cs 42.85% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7652      +/-   ##
==========================================
+ Coverage   61.49%   61.53%   +0.03%     
==========================================
  Files        2253     2253              
  Lines       99103    99103              
  Branches     8960     8960              
==========================================
+ Hits        60945    60984      +39     
+ Misses      36007    35968      -39     
  Partials     2151     2151              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@addisonbeck addisonbeck added t:bugfix Change Type - Bugfix and removed needs-qa labels Jul 13, 2026
@addisonbeck
addisonbeck merged commit 6d1e6e4 into bitwarden:main Jul 13, 2026
25 of 60 checks passed
@addisonbeck

Copy link
Copy Markdown
Contributor

Thanks for contributing @singhvishalkr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-pr t:bugfix Change Type - Bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Emergency Access E-Mails use Wrong Encoding for Names

5 participants