feat!: redesign bcrypt API while preserving stored hash verification - #1207
Brooooooklyn wants to merge 12 commits into
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ebacd2e69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Brooooooklyn
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06c3b9576b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abcf89c7aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Settle the public Promise before removing the abort listener, and remove it with the same options, so throwing or capture-flag EventTargets cannot hang a call or leak the listener. - Keep signal.reason as a non-enumerable AbortError cause, so callers can tell a TimeoutError or custom reason apart. - Throw TypeError for option values of the wrong type and keep RangeError for out-of-range or malformed values. - Copy only the 72 password bytes bcrypt reads for async work, and wipe those copies on drop; replace the unreachable!() salt branch. - Declare package exports so internal files cannot be deep-imported. - Stop declaring the WASI package as an optional dependency, so installs do not pull it and a missing native binary fails loudly. - Leave the version bump to the release step, add the 2.0.0 changelog, and preapprove this repo's own @node-rs/* packages. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
….timeout AbortSignal.timeout() uses a timer that does not keep the event loop alive, and the controlled backend has no native work that would. The test passed only when other tests kept the worker busy, and failed with "Promise returned by test never resolved" on the Linux and WASI jobs. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
With the bcrypt workspace back at 1.10.9, yarn's transparent workspaces link bcrypt-previous (npm:@node-rs/bcrypt@1.10.9) to the workspace itself whenever the lockfile is regenerated, so the previous-release tests would compare the new code with itself. No workspace depends on another, so only the workspace: protocol needs to link workspaces. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
main moved @napi-rs/cli from 3.9.0 to 3.10.5. Regenerate the bcrypt loaders and type declarations from native and wasm32-wasip1-threads builds so the committed files match what the release build produces. binding.js still parses as ES2019 for Node 10 and 12. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
4ebb5c5 to
7ebc07f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ebc07f50c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ed calls A signal whose removeEventListener throws made a successful call raise an unhandled rejection, which exits modern Node, and on abort it skipped the native cancellation. A signal whose reason getter throws stopped the abort listener before it settled, so the call stayed pending or resolved after the abort. Ignore cleanup failures once the call has settled, and read the optional reason defensively. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Bundle two more contract changes into the 2.0 break so the error and
rehash-on-login story is complete at release:
- Thrown errors carry Node-style codes: TypeError ERR_INVALID_ARG_TYPE,
RangeError ERR_OUT_OF_RANGE (including native InvalidArg translations),
and AbortError ABORT_ERR, matching Node's own AbortError.
- parseOptions(hash) returns { version, cost } through the verifier's
parser (bcrypt::HashParts), so every hash verify accepts is parseable,
including +4 costs and imported 2x labels; hashes verify always rejects
throw RangeError. Mirrors argon2's parseOptions in this repo.
- AbortSignalLike.removeEventListener declares the options argument the
adapter actually passes.
- Drop unused blowfish and quickcheck dependencies from the bcrypt crate
and the workspace.
Native and wasm32-wasip1-threads bindings are regenerated. api.cjs still
parses as ES2019 for Node 10 and 12.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Use one options type for add/removeEventListener on AbortSignalLike, matching the comment that the same object is passed to both. - Give parseOptions its own arity message; it never had positional options to drop. - Cover the 60-byte non-ASCII parser branch in the test, which the previous 61-byte input missed. - Mention in the README that parseOptions throws for hashes verify always rejects. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
A stale platform package now fails at import time with code ERR_BCRYPT_INCOMPATIBLE_BINARY, so every error the package throws sits inside the documented ErrorCode contract. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 participant
Bcrypt's positional salt API clips or pads text instead of parsing encoded salts, and its salt generator emits padding. This prepares bcrypt 2.0.0 with explicit creation and verification contracts while preserving existing stored-hash verification behavior.
API changes
2a,2b, and2yfor creation.rejectLongPasswordscreation policy that accepts exactly 72 bytes.TypeErrorwithcode: 'ERR_INVALID_ARG_TYPE'; values of the right type that are out of range or malformed throwRangeErrorwithcode: 'ERR_OUT_OF_RANGE'(nativeInvalidArgerrors are translated the same way).parseOptions(hash)returning{ version, cost }through the verifier's parser (bcrypt::HashParts), mirroring argon2'sparseOptionsin this repo. Every hashverifycan accept is parseable, including the+4cost spelling and imported2xlabels; hashesverifyalways rejects throwRangeError. The strict creation parser is not involved. This gives rehash-on-login flows a supported way to read the stored cost.AbortSignalLike, without requiring global cancellation constructors. Pending public calls reject on abort even if native work is already running, with anAbortError(code: 'ABORT_ERR', like Node's) whosecauseissignal.reasonwhen defined.code: 'ERR_BCRYPT_INCOMPATIBLE_BINARY'. Generated bindings are refreshed with@napi-rs/cli3.10.5 from native andwasm32-wasip1-threadsbuilds. The WASI package is not an automatic dependency; browser builds install@node-rs/bcrypt-wasm32-wasiexplicitly.exports, so only the package root andpackage.jsoncan be imported.blowfishandquickcheckdependencies from the bcrypt crate and the workspace.Stored credentials
No database rewrite, prefix replacement, compatibility flag, or password reset is required. Bcrypt retains its existing verifier parser and computation, including accepted noncanonical cost encodings, current prefix handling, raw password bytes, and long-password suffix equivalence. The stricter salt parser applies only to creation. Invalid UTF-8 hash bytes now return
falseunder the documented error contract.Applications that authenticate by recomputing a hash with a separately saved original salt should migrate to
verify(password, storedHash). The migration guide covers this and the new call shapes.Release
binding.js(yarn buildinpackages/bcrypt), becausenapi versiondoes not update its expected platform-package version.CHANGELOG.mdhas a2.0.0 (Unreleased)entry; set the date at release..yarnrc.ymlpreapproves this repo's own@node-rs/*packages and disables transparent workspaces, sobcrypt-previous(npm:@node-rs/bcrypt@1.10.9) always comes from npm instead of linking to the workspace.Validation
supported-node.cjsandpolyfill-cancellation.cjspass there too (now also coveringparseOptionsand the error codes); the Node 10/12 CI jobs run them on the advertised runtimes.api.cjsstill parses as ES2019.parseOptionsand compared against the text of its own prefix and cost.--locked, warnings denied) pass.Cross-platform results will come from this PR's CI. No packages have been published.