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

Add --insecure (and proxy docs) #524

Closed
wants to merge 2 commits into from

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Aug 12, 2018

Pull Request Template:

Describe what you did

Added option for --insecure for working from behind a proxy which uses untrusted certificates.

How you did it

Separate commits so can cherrypick if desired:

  1. Added --insecure option to n, and use it to disable certificate checks for curl/wget.
  2. Added separate documentation on setting up proxy.

NB: wget options are still insecure by default. However, this change hopefully removes an obstacle to changing the wget default options in the future.

How to verify it doesn't effect the functionality of n

Code inspection that this is a new code path and will not affect previous use cases.

If this solves an issue, please put issue in PR notes.

This option is partly to make it possible to use curl now when insecure is needed, and partly to allow making wget secure by default in future and bring it into line with curl.

Makes curl insecure opt-in, and not by default as suggested here: #323 #324 #426

Make wget secure by default in future: #475 #509

Other proxy related issues where --insecure could help: #284 #430 #503 #517

If this solves an issue, please include the output of issue that had problems and then the fixed output from the same command.

Running a local proxy with MITM certificates to demonstrate:

$ export https_proxy=localhost:8080 
$ n --lts
$ n lts

Error: invalid version 

$ n --insecure --lts
8.11.3
$ n --insecure lts

     install : node-v8.11.3
       mkdir : /usr/local/n/versions/node/8.11.3
       fetch : https://nodejs.org/dist/v8.11.3/node-v8.11.3-darwin-x64.tar.gz
   installed : v8.11.3

Squash any unnecessary commits to keep history clean as possible

Place description for the changelog in PR so we can tally all changes for any future release

Added: --insecure option to disable certificate checks

[Suggested addition]
Note: if curl is not present and n is using wget, the certificate check has historically been disabled and that behaviour has not changed yet. In the future this may change (and you can add the --insecure option now so will not be affected when it does change).

@shadowspawn
Copy link
Collaborator Author

Closing as:

  • No feedback in 5 months, and would not have helped with issues reported in that period.
  • A permanent solution for user is to create .curlrc or .wgetrc file.
  • Potentially more useful to users would be to rework error handling so people can tell proxy and certificates are the problem in the first place!

@shadowspawn shadowspawn deleted the feature/n-insecure branch February 11, 2019 08:47
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.

1 participant