Skip to content

crypto: do not leak scrypt param check errors - #66671

Open
lazerg wants to merge 1 commit into
nodejs:mainfrom
lazerg:fix/issue-66668-scrypt-error-queue
Open

crypto: do not leak scrypt param check errors#66671
lazerg wants to merge 1 commit into
nodejs:mainfrom
lazerg:fix/issue-66668-scrypt-error-queue

Conversation

@lazerg

@lazerg lazerg commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Fixes: #66668

When checkScryptParams() fails on the memory check, OpenSSL queues an error. ScryptTraits::AdditionalConfig() only peeks at it to build the message and leaves it on the queue, so the next unrelated call on the thread, like createPrivateKey() with a valid key, throws ERR_OSSL_EVP_MEMORY_LIMIT_EXCEEDED.

This pops the error with MarkPopErrorOnReturn once the check is done. The thrown message stays the same.

@lazerg
crypto: do not leak scrypt param check errors
cd7d6f8 
Signed-off-by: lazerg <[email protected]>
Assisted-by: Claude Code
@nodejs-github-bot

nodejs-github-bot commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Oct 12, 2026
@github-actions

github-actions Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Caution

AgentScan found account activity patterns that may be consistent with automation. This is a heuristic, not proof that this pull request was opened by an agent or violates policy. AI-assisted contributions are permitted, but automated tooling must not open pull requests without advance approval, and contributors must personally understand, test, verify, and take responsibility for every submitted change. See the AgentScan analysis, AI use policy, and automation policy for additional context.

@codecov

codecov Bot commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.47%. Comparing base (3b639be) to head (cd7d6f8).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #66671      +/-   ##
==========================================
- Coverage   92.82%   90.47%   -2.35%     
==========================================
  Files         422      791     +369     
  Lines      194189   277089   +82900     
  Branches    30027    53271   +23244     
==========================================
+ Hits       180249   250690   +70441     
- Misses      13614    16812    +3198     
- Partials      326     9587    +9261
Files with missing lines Coverage Δ
src/crypto/crypto_scrypt.cc 73.07% <100.00%> (ø)

... and 499 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Oct 12, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 12, 2026
@nodejs-github-bot

nodejs-github-bot commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/78330/

@panva panva added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Oct 12, 2026
@github-actions github-actions Bot added resume-ci-failed Resuming CI with the resume-ci label failed and requires manual intervention. and removed resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. labels Oct 12, 2026
@github-actions

github-actions Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Failed to resume CI

✖  Cannot resume PR CI job 78330: Jenkins does not offer a "Resume build" action. Check the existing CI run in Jenkins and rebase the PR if needed. To start a new CI run manually: ncu-ci run https://github.com/nodejs/node/pull/66671
   ✖  https://ci.nodejs.org/job/node-test-pull-request/78330/
Full Auto Start CI output 
�[36m⠋�[39m Validating Jenkins credentials
�[36m⠋�[39m Validating Jenkins credentials
✔  Jenkins credentials valid
�[36m⠙�[39m Looking for CI runs for pull request 66671
�[36m⠙�[39m Looking for CI runs for pull request 66671
�[36m⠙�[39m Getting PR from nodejs/node/pull/66671
�[36m⠙�[39m Getting reviews from nodejs/node/pull/66671
�[36m⠙�[39m Getting comments from nodejs/node/pull/66671
✔  Found PR CI job 78330
�[36m⠹�[39m Querying data for job/node-test-pull-request/78330/
�[36m⠹�[39m Querying data for job/node-test-pull-request/78330/
�[36m⠹�[39m Querying API for job/node-test-pull-request/78330/
✔  Build data downloaded
�[36m⠹�[39m Checking whether PR CI job 78330 can be resumed
�[36m⠹�[39m Checking whether PR CI job 78330 can be resumed
✖  Cannot resume PR CI job 78330: Jenkins does not offer a "Resume build" action. Check the existing CI run in Jenkins and rebase the PR if needed. To start a new CI run manually: ncu-ci run https://github.com/nodejs/node/pull/66671
   ✖  https://ci.nodejs.org/job/node-test-pull-request/78330/

View workflow run

@nodejs-github-bot

nodejs-github-bot commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/78343/

@panva panva added resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. and removed resume-ci-failed Resuming CI with the resume-ci label failed and requires manual intervention. labels Oct 12, 2026
@github-actions github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Oct 13, 2026
@nodejs-github-bot

nodejs-github-bot commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/78349/

@panva panva added the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@panva panva panva approved these changes

Assignees

No one assigned

Labels

agentscan:community-flagged author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue PRs queued for automated landing through the Commit Queue. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

crypto: scrypt with params over maxmem leaves an OpenSSL error queued; the next createPrivateKey fails

3 participants

@lazerg @nodejs-github-bot @panva