Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: stop ignoring NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not defined #316

Merged

Conversation

brunoargolo
Copy link
Contributor

@brunoargolo brunoargolo commented Sep 19, 2024

Currently NODE_TLS_REJECT_UNAUTHORIZED is simply ignored as options.rejectUnauthorized is always set to false when strictSSL is not defined.

Most notably this causes issues for users behind corporate proxies using npm and pnpm when installing a package that uses node-gyp. Example: nodejs/node-gyp#2663

This change only takes into account NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not passed to fetch.

unit tests were added to ensure strictSSL is still the primary driver.

@wraithgar
Copy link
Member

This was brought up before in #257. Given its use in node-gyp it makes sense to revisit though.

Is this a breaking change?

@brunoargolo
Copy link
Contributor Author

Hey @wraithgar, confirming this is a non-breaking change.
The behaviour is only modified if NODE_TLS_REJECT_UNAUTHORIZED is set AND the fetch function caller has not specified the strictSSL param (undefined or null).
I've included new unit tests to cover scenarios when the env variable is set.

test/options.js Dismissed Show dismissed Hide dismissed
@wraithgar wraithgar changed the title follow NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not defined fix: stop ignoring NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not defined Oct 16, 2024
@wraithgar wraithgar changed the title fix: stop ignoring NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not defined fix!: stop ignoring NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not defined Oct 16, 2024
This was referenced Oct 16, 2024
@wraithgar
Copy link
Member

NODE_TLS_REJECT_UNAUTHORIZED is a very intentional user directive, treating this as a fix.

@wraithgar wraithgar changed the title fix!: stop ignoring NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not defined fix: stop ignoring NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not defined Oct 21, 2024
@wraithgar wraithgar merged commit 3e0fe87 into npm:main Oct 21, 2024
27 of 28 checks passed
@github-actions github-actions bot mentioned this pull request Oct 21, 2024
wraithgar pushed a commit that referenced this pull request Oct 23, 2024
🤖 I have created a release *beep* *boop*
---


##
[14.0.3](v14.0.2...v14.0.3)
(2024-10-21)
### Bug Fixes
*
[`3e0fe87`](3e0fe87)
[#316](#316) stop ignoring
NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not defined (#316)
(@brunoargolo, Bruno Oliveira)
### Dependencies
*
[`05fbcc3`](05fbcc3)
[#327](#327) bump
negotiator from 0.6.4 to 1.0.0 (#327) (@dependabot[bot])

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants