fix(sha1crypt): apply the default salt length - #350
Conversation
SaltLengthMin was 0, so the hasher's check for an unset salt length (bytesSalt < SaltLengthMin) was never true and the default of 8 was never applied. As a result every digest produced with the default options had an empty salt, meaning identical passwords produced identical digests. SaltLengthMin is now 1, matching the documented minimum for WithSaltLength. This also means WithSaltLength(0) and HashWithSalt with an empty salt now return an error. Existing digests with an empty salt can still be decoded and verified.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe ChangesSHA-1 crypt salt validation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The salt-length change fixes default hashing while preserving verification of existing digests. No material merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 golangci-lint (2.13.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #350 +/- ##
==========================================
+ Coverage 82.28% 82.34% +0.05%
==========================================
Files 49 49
Lines 1716 1716
==========================================
+ Hits 1412 1413 +1
+ Misses 304 303 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
SaltLengthMin was 0, so the hasher's check for an unset salt length (bytesSalt < SaltLengthMin) was never true and the default of 8 was never applied. As a result every digest produced with the default options had an empty salt, meaning identical passwords produced identical digests.
SaltLengthMin is now 1, matching the documented minimum for WithSaltLength. This also means WithSaltLength(0) and HashWithSalt with an empty salt now return an error. Existing digests with an empty salt can still be decoded and verified.
Summary by CodeRabbit