Potential fix for code scanning alert no. 155: Clear text storage of sensitive information#53
Conversation
…sensitive information 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR modifies the interest calculation service to mask account identifiers in logs. The ChangesAccount ID Masking
🎯 2 (Simple) | ⏱️ ~8 minutes
✨ 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (account == null) | ||
| { | ||
| _logger.LogWarning("Account {AccountId} not found for interest rate update", accountId); | ||
| _logger.LogWarning("Account {AccountId} not found for interest rate update", maskedAccountId); |
| if (newRate < 0 || newRate > 10) // Reasonable bounds | ||
| { | ||
| _logger.LogWarning("Invalid interest rate {Rate} for account {AccountId}", newRate, accountId); | ||
| _logger.LogWarning("Invalid interest rate {Rate} for account {AccountId}", newRate, maskedAccountId); |
| $"Interest rate for account {maskedAccountId} updated from {oldRate}% to {newRate}%", userId); | ||
| _logger.LogInformation("Interest rate for account {AccountId} updated from {OldRate}% to {NewRate}%", | ||
| accountId, oldRate, newRate); | ||
| maskedAccountId, oldRate, newRate); |
2 participants
Potential fix for https://github.com/Mostafa-SAID7/Bank-Api/security/code-scanning/155
General fix: do not log full sensitive identifiers; instead log a masked/tokenized representation (or encrypted value where appropriate). For this file, the least disruptive fix is to mask
accountIdbefore writing to logs/audit messages.Best single fix in
src/Bank.Application/Services/Shared/InterestCalculationService.cs:UpdateInterestRateAsync, computemaskedAccountIdonce near whereaccountIdis used for messaging.accountIdusages in:so only masked value is persisted in logs.
GetByIdAsync(accountId)) to preserve functionality.No new dependency is required.
Suggested fixes powered by Copilot Autofix. Review carefully before merging.
Summary by CodeRabbit
Release Notes