-
Notifications
You must be signed in to change notification settings - Fork 20
[PM-24468] Introduce CipherRiskClient #499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Claude finished @shane-melton's task —— View job Code Review Complete ✅Summary of ChangesThis PR introduces
Key additions:
Critical Issues🔴 Security: Potential Password Leakage in Error MessagesLocation: Network errors from HIBP API could potentially expose sensitive information if passwords are included in error messages. While Recommendation: Add explicit error wrapping to ensure passwords are never logged: let exposed_count = if options.check_exposed {
Some(
Self::check_password_exposed(&http_client, &details.password, &base_url)
.await
.map_err(|e| {
// Strip any potential sensitive data from error message
CipherRiskError::Reqwest(e)
})?,
)
} else {
None
}; Additionally, verify that Reference CLAUDE.md line 29: "Do not log keys, passwords, or vault data in logs or error paths. Redact sensitive data."
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #499 +/- ##
==========================================
+ Coverage 77.98% 78.26% +0.28%
==========================================
Files 287 289 +2
Lines 27672 28165 +493
==========================================
+ Hits 21579 22044 +465
- Misses 6093 6121 +28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
915fe76
to
a10fef6
Compare
Remove performance test
|
🎟️ Tracking
PM-24468
📔 Objective
Implement the cipher risk evaluation logic in the Vault SDK as a separate
CipherRiskClient
so that it can be re-used in mobile and gain potential performance improvements.The
CipherRiskClient
contains logic to check if a multiple login ciphers' passwords are exposed (via HIBP), weak (via zxcvbn), or reused.⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes