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

Copy system cert configs when installing new versions #2118

Open
wants to merge 1 commit into
base: master
from

Conversation

@squarebracket
Copy link

squarebracket commented Nov 4, 2019

This PR adds support for copying any ca, cert, or cafile configs from the system's npm config file to the newly-installed npm prefix's config. It does this by default, but can be disabled with the --skip-system-certs flag.

Manually tested in sh, bash, and zsh. Also tested in ksh (per CONTRIBUTING.md), but that doesn't seem to work unrelated to my changes.

Any suggestions on how I could approach automated testing for these changes? Specifically, how I would mock out the nvm system exec npm config ls call.

@squarebracket squarebracket force-pushed the squarebracket:feature/copy-certs branch 2 times, most recently from 981dd38 to c3fd9a9 Nov 4, 2019
@squarebracket squarebracket force-pushed the squarebracket:feature/copy-certs branch from c3fd9a9 to f6f5b20 Nov 4, 2019
@@ -310,6 +311,10 @@ object-inspect@1.0.2
stevemao/left-pad
```

### Copy system certificate preferences when installing

If you have any of `ca`, `cafile`, or `cert` present in your npm config when installing a new version, those keys will be copied into the newly-installed version's global config file. If you don't want this behavior, pass the `--skip-system-certs` flag when installing a new version

This comment has been minimized.

Copy link
@ljharb

ljharb Nov 5, 2019

Member

Can you explain more about why this complexity is desired or needed?

I don't think having these present is a common case, and looking these up from the current node version (which might be "the system node") will slow things down for the majority that would never need it.

It seems simpler for the user to manually rerun their npm config commands, or better, for each project to define its own .npmrc file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.