Open lleirborras opened 1 month ago
The submodule is only needed for running tests - are you suggesting the install script will fail because it's present?
I'm not concerned with people doing development on nvm needing a normal unobstructed network connection, that should be a baseline, and that commit is unreleased, so nobody should be hitting it with the install script yet - but I definitely wouldn't want to cause issues once it is released.
The submodule is automatically cloned through git clone command. We saw this issue too just by doing git clone https://github.com/nvm-sh/nvm.git
.
Yes, we also use Ansible to clone through https. This should not happen and is a major issue, since it being an optional submodule for testing purposes being on the master branch
concerned with
Using standar ansible git , trying to perform a git clone
, it tries to fetch the submodule and in our case firewall issues arise and it fails.
At the same time, a module used only for testing should not be mandatory by default on a normal clone
- name: Download nvm
git:
repo: https://github.com/nvm-sh/nvm.git
dest: "{{ XXX.home }}/.nvm"
diff: no
changed_when: false
thanks for the heads up (but nobody should be using the master branch; only tagged releases)
So, the solution for your problem is to fix that step - because an end user of nvm never should be doing a normal clone, only a clone via the install script. Can you use the install script instead?
thanks for the heads up (but nobody should be using the master branch; only tagged releases)
So, the solution for your problem is to fix that step - because an end user of nvm never should be doing a normal clone, only a clone via the install script. Can you use the install script instead?
And that's a bad idea, the whole trend of curl | bash
is a potential security hole , there's documentation about it as early as 2016
https://www.idontplaydarts.com/2016/04/detecting-curl-pipe-bash-server-side/ (apologies for the link with broken certificate)
For now we will stick to the latest tag without that but I still think this submodule used only for testing proposes should not be handled like that.
We first pull the repo, then check for latest tag and checkout to that. However the initial pull just fails due to this.
Piping to bash from a curl is definitely a security risk
setting aside the way WE install, this will cause issues to clone the repo through https
is a potential security hole
no, it's not, actually :-) worrying about that is security theater. The curl is from github, and github isn't doing any detection that would cause this problem.
nvm recently had a security audit in partnership with OSTIF, and this simply was not an identified security risk as considered by a veteran security firm.
I'll leave this issue open so I can remember to test the standard install script before the next release, so that end users won't fetch the submodule by default.
One of the latest commit on the master branch https://github.com/nvm-sh/nvm/commit/29dce5edfd0976f9a1728c5746715c24061fd404 introduces nvmrc as a submodule, and for that it uses the ssh URL. This will (is) causing firewall issues on more sensitive systems where https traffic is allowed but ssh is blocked by default. https://github.com/nvm-sh/nvm/commit/29dce5edfd0976f9a1728c5746715c24061fd404#diff-fe7afb5c9c916e521401d3fcfb4277d5071798c3baf83baf11d6071742823584R3
The correct way of dealing with this would be either allowing the submodule to be an optional feature, or clone it over https
Thanks