Potential fix for code scanning alert no. 9: Exposure of private information#8
Potential fix for code scanning alert no. 9: Exposure of private information#8shimakaze09 merged 1 commit intomainfrom
Conversation
…rmation Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdded a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| var sanitizedToAddress = toAddress.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", ""); | ||
| _logger.LogDebug("Sending email, subject: {Subject}, recipient: {ToAddress}", subject, sanitizedToAddress); | ||
| var maskedToAddress = MaskEmail(sanitizedToAddress); | ||
| _logger.LogDebug("Sending email, subject: {Subject}, recipient: {ToAddress}", subject, maskedToAddress); |
There was a problem hiding this comment.
Pull request overview
This PR aims to address code scanning alert #9 (“Exposure of private information”) by preventing full recipient email addresses from being written to logs in EmailService.SendEmailAsync, while keeping email sending behavior unchanged.
Changes:
- Added a private
MaskEmail(string email)helper to redact email addresses for logging. - Updated the debug log in
SendEmailAsyncto log the masked recipient instead of the raw address.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static string MaskEmail(string email) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(email)) return string.Empty; | ||
|
|
||
| var atIndex = email.IndexOf('@'); | ||
| if (atIndex <= 0 || atIndex == email.Length - 1) return "***"; | ||
|
|
||
| var localPart = email.Substring(0, atIndex); | ||
| var domainPart = email.Substring(atIndex + 1); | ||
|
|
||
| if (localPart.Length <= 1) | ||
| return "*@" + domainPart; | ||
|
|
||
| return localPart[0] + new string('*', Math.Max(1, localPart.Length - 1)) + "@" + domainPart; | ||
| } |
There was a problem hiding this comment.
MaskEmail preserves the full domain (and local-part length), which can still be considered private information (e.g., corporate domains) and may still match email-detection rules in code scanning. Consider masking the domain as well (e.g., keep only the TLD/first char) or logging a non-reversible identifier (hash) instead of any recognizable email form.
| var maskedToAddress = MaskEmail(sanitizedToAddress); | ||
| _logger.LogDebug("Sending email, subject: {Subject}, recipient: {ToAddress}", subject, maskedToAddress); | ||
| body += $"<br><p>This message was automatically sent by {BlogLink}, no need to reply.</p>"; | ||
| return await EmailUtils.SendEmailAsync(_emailAccountConfig, subject, body, toName, toAddress); |
There was a problem hiding this comment.
sanitizedToAddress is computed but the actual send still uses the original toAddress. Since toAddress originates from user input in some flows (e.g., link exchange) and may contain unexpected control characters, consider sending using the sanitized value (or performing strict validation and rejecting invalid addresses) to avoid header-injection style issues or unexpected exceptions in MailboxAddress parsing.
| return await EmailUtils.SendEmailAsync(_emailAccountConfig, subject, body, toName, toAddress); | |
| return await EmailUtils.SendEmailAsync(_emailAccountConfig, subject, body, toName, sanitizedToAddress); |
Potential fix for https://github.com/shimakaze09/Blog/security/code-scanning/9
In general, the fix is to ensure that private data (the user’s email address) is not written verbatim to logs or other external locations. For this case, that means changing the debug log line in
EmailService.SendEmailAsyncso it does not include the fulltoAddress, but instead either omits the email entirely or logs only a redacted/masked form (for example, partial local part and domain). We should preserve existing functionality—emails must still be sent to the real address, and logs remain useful for diagnostics.The best minimal fix is to introduce a small helper in
EmailServicethat masks an email address (e.g., keeps the first character of the local part and the domain, replacing the rest with asterisks), and use that masked value in the debug log instead of the raw address. This keeps the log informative (“which email roughly was used”) without exposing the full address. Concretely, inWeb/Services/EmailService.cs, we will: (1) add a private static methodMaskEmail(string email)aboveSendEmailAsync; (2) inSendEmailAsync, keep the newline sanitization for safety but then derive amaskedToAddress = MaskEmail(sanitizedToAddress);; and (3) change theLogDebugcall to logmaskedToAddressinstead ofsanitizedToAddress. No changes are required toCommentControllerorCommentServicebecause the only problematic sink is the logging line.Suggested fixes powered by Copilot Autofix. Review carefully before merging.
Summary by CodeRabbit