Skip to content

fix(bcrypt): require version 2 for sha256 digests - #355

Merged
james-d-elliott merged 1 commit into
masterfrom
fix/bcrypt-sha256-version
Sep 25, 2026
Merged

james-d-elliott merged 1 commit into
masterfrom
fix/bcrypt-sha256-version

Conversation

@james-d-elliott

@james-d-elliott james-d-elliott commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

The SHA256 variant ignored the v parameter, so digests with any version, or none at all, were decoded as version 2 and encoded again as v=2. Only version 2 is implemented, where the password is hashed with HMAC-SHA256 keyed by the salt, so a digest claiming another version would be verified with the wrong algorithm.

Decoding now returns ErrEncodedHashInvalidVersion unless the v parameter is present and equal to 2, which is the version this library and passlib encode. Digests in the passlib version 1 format use a different layout and were already rejected.

Summary by CodeRabbit

  • Bug Fixes
    • SHA-256 bcrypt hashes now require a supported version. Hashes with a missing or unsupported version are rejected, while valid version 2 hashes can be decoded and verified.

@james-d-elliott
james-d-elliott requested a review from a team as a code owner September 24, 2026 23:54
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 0e13547f-9ebe-41c9-a7fc-178a82b588b5

📥 Commits

Reviewing files that changed from the base of the PR and between d308022 and dc40db4.

📒 Files selected for processing (3)
  • algorithm/bcrypt/const.go
  • algorithm/bcrypt/decoder.go
  • algorithm/bcrypt/regression_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

SHA256 bcrypt decoding now requires an explicit version 2 option. The decoder rejects hashes with a missing or unsupported version. Regression tests cover rejected versions and successful version-2 decoding.

Changes

SHA256 bcrypt version validation

Layer / File(s) Summary
Require version 2
algorithm/bcrypt/const.go, algorithm/bcrypt/decoder.go, algorithm/bcrypt/regression_test.go
The decoder now checks that a version option is present and equals version 2. Regression tests cover invalid, missing, and supported versions. A supported digest must decode, re-encode identically, and match the password.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to dc40d

The decoder’s version checks match the format emitted by the encoder, with no identified issue requiring resolution before merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: requiring version 2 for bcrypt-SHA256 digests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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
The command is terminated due to an 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

The SHA256 variant ignored the v parameter, so digests with any version,
or none at all, were decoded as version 2 and encoded again as v=2. Only
version 2 is implemented, where the password is hashed with HMAC-SHA256
keyed by the salt, so a digest claiming another version would be
verified with the wrong algorithm.

Decoding now returns ErrEncodedHashInvalidVersion unless the v parameter
is present and equal to 2, which is the version this library and passlib
encode. Digests in the passlib version 1 format use a different layout
and were already rejected.
@james-d-elliott
james-d-elliott force-pushed the fix/bcrypt-sha256-version branch from 75ec0bd to dc40db4 Compare September 24, 2026 23:57
@codecov

codecov Bot commented Sep 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.19%. Comparing base (d308022) to head (dc40db4).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
+ Coverage   83.07%   83.19%   +0.12%     
==========================================
  Files          49       49              
  Lines        1737     1744       +7     
==========================================
+ Hits         1443     1451       +8     
+ Misses        294      293       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@james-d-elliott

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@james-d-elliott
james-d-elliott merged commit c8faeb5 into master Sep 25, 2026
13 checks passed
@james-d-elliott
james-d-elliott deleted the fix/bcrypt-sha256-version branch September 25, 2026 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant