Modernize phpass and add portable password verification - #4
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds verification for the portable
$P$and$H$hashes requested in #2 and modernizes the normal bcrypt path. The old code generated salts with Math.random, used the constructor's cost instead of the stored hash's cost during verification, and encoded non-ASCII passwords as UTF-16 low bytes. Standard bcrypt now uses bcrypt.js with cryptographic salts, UTF-8, and stored-cost verification.This prepares 1.0.0 because new hashes use
$2b$, the default generation cost changes from 8 to 10, Node.js 22+ is required, inputs/costs are validated, and new passwords beyond bcrypt's 72-byte limit are rejected. Existing non-ASCII node-phpass records have an explicitcheckPasswordLegacy()migration path; normal checks never silently fall back to the old encoding. Portable generation remains unsupported.Adds Promise methods (portable verification runs in a worker), verification cost ceilings, declarations, a lockfile, explicit package contents, GitHub Actions, and a rewritten README covering supported formats, concurrency, limits, and migration. Existing license/third-party attribution is retained.
Validation: 12 passing tests, including independent native-bcrypt interoperability; 12 portable vectors generated by Openwall's C reference at a recorded commit; old-release ASCII/Unicode fixtures; malformed inputs and excessive-cost rejection; cryptographic randomness without Math.random; async verification and worker timer responsiveness. Strict TypeScript CJS/ESM consumers compile. Installed the packed archive and ran CommonJS, ESM, portable-worker, and README examples. Native bcrypt is development-only.
Fixes #2. Publication remains a separate step.