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

[BUG] Not Respecting NODE_TLS_REJECT_UNAUTHORIZED = 0 #61

Closed
1 task done
om-mani-padme-hum opened this issue May 25, 2022 · 12 comments
Closed
1 task done

[BUG] Not Respecting NODE_TLS_REJECT_UNAUTHORIZED = 0 #61

om-mani-padme-hum opened this issue May 25, 2022 · 12 comments
Assignees
Labels
Needs Triage needs an initial review

Comments

@om-mani-padme-hum
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

It fails due to a self-signed certificate error, despite being told not to reject unauthorized certificates (my company can't get me the .pem file):

image

This prevents node-gyp and several other repos from being installed over npm for people such as me.

The workaround we've implemented is to edit your module and pass the option to not reject unauthorized:

image

Expected Behavior

To install the modules properly over npm, e.g.

image

Steps To Reproduce

  1. In this environment...

Have a self-signed certificate in your certificate chain.

  1. With this config...

export NODE_TLS_REJECT_UNAUTHORIZED=0

  1. Run '...'

npm i node-gyp

or

npm i smartsheet

  1. See error...

Environment

  • npm: 8.8.0
  • Node: 16.15.0
  • OS: Windows 10 Enterprise
  • platform: cygwin64
@om-mani-padme-hum om-mani-padme-hum added the Needs Triage needs an initial review label May 25, 2022
@cdavid15
Copy link

We are experiencing the same issue which arose when we bumped out node-lts from 16 to 18 this week. What had worked previously using NODE_TLS_REJECT_UNAUTHORIZED=0 yarn install no longer works now.

The only way we can by pass the self signed certificate issue is by adding options.rejectUnauthorized = false; on line 71 of index.js.

@garrettboone
Copy link

This worked for me:

  return new Promise((resolve, reject) => {
    // build request object
    const request = new Request(url, opts)
    let options
    try {
      options = getNodeRequestOptions(request)
      options.agent.options.rejectUnauthorized = false
    } catch (er) {
      return reject(er)
    }

@wlarch
Copy link

wlarch commented Feb 21, 2023

We have encountered the same issue when updating Node v16.13.2 → v18.14.1. The same self-signed certificates have been used and are properly working when using Node v16.13.2 in a local environment.


Follow-up : our solution was to overwrite the fetcher of the Apollo Gateway buildService method.

const fetcher = require('make-fetch-happen');

const gateway = new ApolloGateway({
  buildService({name, url}) {
    return new RemoteGraphQLDataSource({
      name,
      url,
      fetcher: fetcher.defaults({strictSSL: false})
    });
  }
});

make-fetch-happen@11.0.3 has minipass "^4.0.0" and minipass-fetch "^3.0.0" dependencies.

@casyalex
Copy link

casyalex commented Jul 20, 2023

Anyone fix this?

======================

After my trace this lib did respect the NODE_TLS_REJECT_UNAUTHORIZED, but was overriden by node-gyp. That is not this libs fault

I opened an PR to fix this

@wraithgar
Copy link
Member

Has anyone tried using the npm config that disables this behavior? https://docs.npmjs.com/cli/v7/using-npm/config#strict-ssl

@om-mani-padme-hum
Copy link
Author

Setting strict-ssl to false was not sufficient to overcome the issue for me. The only resolution was to patch the minipass-fetch file with options.rejectUnauthorized = false; This has become a standard step in our development environment setup at this point, and confirmed among several developers as being the only option that works.

@casyalex
Copy link

casyalex commented Jul 21, 2023

The root cause is node-gyp use this package in plain node.js enviroment, so .npmrc won't work. But make-fetch-happen is pretty much written only for npm-cli usecase. That cause this issue.

@wraithgar
Copy link
Member

cc @lukekarrys in case there is something node-gyp could be doing here to interpret that environment variable and update the params it sends to this module.

@jbgomond
Copy link

Seems like the issue is not in this library (that supports NODE_TLS_REJECT_UNAUTHORIZED), but in make-fetch-happen itself (overriding the strictSSL parameter)

@seng1e
Copy link

seng1e commented Dec 18, 2023

This worked for me:

  return new Promise((resolve, reject) => {
    // build request object
    const request = new Request(url, opts)
    let options
    try {
      options = getNodeRequestOptions(request)
      options.agent.options.rejectUnauthorized = false
    } catch (er) {
      return reject(er)
    }

Thanks @garrettboone answers.

When I changed options.agent.options.rejectUnauthorized = false to options.rejectUnauthorized = false , it actually worked!

enviroment: minipass-fetch@2.1.2, patched in index.js line 61.

@pbeast
Copy link

pbeast commented Apr 30, 2024

That is what I added to my local copy. At least works for me:

if (process.env['NODE_TLS_REJECT_UNAUTHORIZED'] == '0') {
      console.warn("-----------------------[ minipass-fetch ]-----------------------------");
      console.warn("- NODE_TLS_REJECT_UNAUTHORIZED is set to 0. This is not recommended. -");
      console.warn("----------------------------------------------------------------------");

      options.agent.options.rejectUnauthorized = false;
}

const req = send(options)

@reggi
Copy link
Contributor

reggi commented May 21, 2024

Hey all 👋 I've added a test that shows minipass-fetch currently honors the env var, I believe the issue is specific issue lies elsewhere, it's possible that node-gyp needs to pass the strictSSL option to make-fetch-happen

@reggi reggi closed this as completed May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Triage needs an initial review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

12 participants