fix: resolve user data leakage in BCC mass email templates (#5444) [skip ci] - #5444
Conversation
🦈 RepoShark Health Check
View full analysis → · Powered by RepoShark |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMassEmailSender’s BCC path now resolves template placeholders with a placeholder user instead of the first valid recipient. The BCC test was updated to use placeholder variables in the email body and to assert the resolved message value. ChangesMassEmail BCC placeholder fix
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes a privacy bug in MassEmailSender where BCC mass emails with personalized template placeholders could leak the first recipient’s username/alias to all BCC recipients by resolving the template against the first valid user.
Changes:
- Resolve BCC template placeholders using a generic dummy
OmbiUser(UserName/Aliasset to"User") instead of the first recipient. - Update the BCC unit test to include
{Alias}and{UserName}in the body and assert they resolve to"User".
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Ombi.Core/Senders/MassEmailSender.cs | Avoids user data leakage in BCC mode by resolving template placeholders against a generic dummy user. |
| src/Ombi.Core.Tests/Senders/MassEmailSenderTests.cs | Extends BCC test coverage to validate {Alias} / {UserName} resolve to "User" in BCC emails. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
b41b868 to
9e68699
Compare
|
tidusjar
changed the title
|
🎉 Thank you for your contribution! Your changes have been successfully merged and will be included in the next release. What you contributed:
Thank you for helping make Ombi better! 🙏 |
📝 Description
When sending mass emails via BCC with templates containing personalized placeholders (like
{Alias},{UserName}), the template was previously resolved once using the first user in the recipient list (validUsers.FirstOrDefault()). Because BCC sends a single copy of the email body to all recipients, everyone received a message addressed to that first user, exposing their real username/alias.This PR fixes the data leakage by instantiating a generic dummy
OmbiUser(UserName = "User", Alias = "User") to resolve placeholders to a generic"User"value when in BCC mode.🔗 Related Issues
Fixes #5239
🧪 Testing
Updated the unit test in
MassEmailSenderTests.cs(SendMassEmail_Bcc) to include{Alias}and{UserName}in the test message body and verified that they correctly resolve to"User".📸 Screenshots (if applicable)
N/A (Backend change)
📋 Checklist
🎯 Type of Change
📚 Additional Notes
This PR was vibe coded with love by Antigravity AI assistant pair-programming with the repository owner.
Summary by CodeRabbit