[BUG] NPM v7 uses SSH instead of an explicit HTTPS for GitHub repos

[uhop]

Current Behavior:

When I use a git repository via an HTTP link NPM "takes liberties" with it, which breaks my build:

$ npm init -y
Wrote to /Users/eugene.lazutkin/Work/temp/package.json:

{
  "name": "temp",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}


$ npm i --save https://github.com/uhop/stream-chain.git

added 1 package, and audited 2 packages in 3s

found 0 vulnerabilities

It produces package-lock.json:

{
  "name": "temp",
  "version": "1.0.0",
  "lockfileVersion": 2,
  "requires": true,
  "packages": {
    "": {
      "version": "1.0.0",
      "license": "ISC",
      "dependencies": {
        "stream-chain": "github:uhop/stream-chain"
      }
    },
    "node_modules/stream-chain": {
      "version": "2.2.4",
      "resolved": "git+ssh://[email protected]/uhop/stream-chain.git#459f5a1708c138b6e0abaae4cf103c3488e1e78e",
      "license": "BSD-3-Clause"
    }
  },
  "dependencies": {
    "stream-chain": {
      "version": "git+ssh://[email protected]/uhop/stream-chain.git#459f5a1708c138b6e0abaae4cf103c3488e1e78e",
      "from": "stream-chain@github:uhop/stream-chain"
    }
  }
}

Note that https://github.com/uhop/stream-chain.git was replaced with github:uhop/stream-chain, which is probably OK in this case. But other two links (?) are rewritten from https://github.com/uhop/stream-chain.git to git+ssh://[email protected]/uhop/stream-chain.git, which is clearly bad.

The problem is that a build bot we use in similar situations can access private git repositories using HTTP, but not SSH for security reasons. It fails on an authentication. Rewriting https://github.com/uhop/stream-chain.git to git+ssh://[email protected]/uhop/stream-chain.git is not acceptable for that reasons.

The fix is relatively minor yet unpleasant: we have to replace npm ci with npm i, which takes more time and introduced instabilities with other dependencies.

Expected Behavior:

When running npm ci it should use the original URL with the HTTP authentication instead of SSH.

Steps To Reproduce:

See the description and do the same steps using git repositories (github only?) as dependencies.

Environment:

OS: Mac Node: 15.7.0 NPM: 7.4.3

[ljharb]

Can you try with npm v7.5.2? I think this has already been fixed.

[uhop]

Just tried with exactly same results.

$ npm --version
7.5.2

package-lock.json is absolutely identical.

[uhop]

I think there is no error in the code.

Just to point it out explicitly:

• The dependency is set as https://github.com/uhop/stream-chain.git — note https:// part of it.
• In package-lock.json it is mentioned three times.
  • Two if them as git+ssh://[email protected]/uhop/stream-chain.git#459f5a1708c138b6e0abaae4cf103c3488e1e78e.
  • Note git+ssh://[email protected] part.
  • It definitely tries to access the repository using ssh rather than https protocol and fails.
    • Why is it important?
      • Those protocols have a different way to authenticate a user.
      • Our build environment is not set up for ssh, yet we can use https.

I don't know if the bug is specific to GitHub: adding https://github.com/uhop/stream-chain.git was transformed into github:uhop/stream-chain (please peruse my bug report above), and who knows what protocol it assumes. But not all repositories support ssh. We clearly have a bug on our hands.

I hope I explained the problem better now. Please do not hesitate to ask, if something is left unclear.

[uhop]

It came to my attention that this is a documented behavior that breaks private repositories and environments without ssh access (from https://blog.npmjs.org/post/626173315965468672/npm-v7-series-beta-release-and-semver-major):

Git dependencies on known git hosts (GitHub, BitBucket, etc.) will always attempt to fetch package contents from the relevant tarball CDNs if possible, falling back to git+ssh for private packages. resolved value in package-lock.json will always reflect the git+ssh url value. Saved value in package.json dependencies will always reflect the canonical shorthand value.

Clearly this is not correct and should be fixed.

[plaa]

This is a breaking change in npm v7 and prevents anyone using even public packages without ssh keys.

We have made a fork of an npm package and host it as a public repo. We refer to it using a github: link. In v6 this github link was populated to package-lock.json as well, which worked fine. In v7 it is converted into git+ssh: which fails in our CI/CD pipeline as it doesn't have any ssh keys configured.

We don't want to add ssh keys to our CI pipeline just to fetch public packages, so I don't see any way we can upgrade. I'd be glad to hear of any workaround.

Specific example:

In package.json referring to https://github.com/Vincit/winston-azure-transport:

    "winston-azure-transport": "github:Vincit/winston-azure-transport#ed07d0685ca601638ab8ebcf660128848dd30215"

Diff of package-lock.json between npm v6 and v7:

     "winston-azure-transport": {
-      "version": "github:Vincit/winston-azure-transport#ed07d0685ca601638ab8ebcf660128848dd30215",
-      "from": "github:Vincit/winston-azure-transport#ed07d0685ca601638ab8ebcf660128848dd30215",```
+      "version": "git+ssh://[email protected]/Vincit/winston-azure-transport.git#ed07d0685ca601638ab8ebcf660128848dd30215",
+      "from": "winston-azure-transport@github:Vincit/winston-azure-transport#ed07d0685ca601638ab8ebcf660128848dd30215",

Our CI now fails with:

npm ERR! Error while executing:
npm ERR! /usr/bin/git ls-remote -h -t ssh://[email protected]/Vincit/winston-azure-transport.git
npm ERR! 
npm ERR! [email protected]: Permission denied (publickey).
npm ERR! fatal: Could not read from remote repository.
npm ERR! 
npm ERR! Please make sure you have the correct access rights
npm ERR! and the repository exists.
npm ERR! 
npm ERR! exited with error code: 128

[isaacs]

This is tricky.

We currently keep the https url if auth is set in the url, because presumably you need that auth to access the repo.

The tricky bit is that using https without auth in the url will trigger a prompt to enter a username and password on the command line, which will fail (or at least be confusing and strange) in many cases where an install with ssh would work fine.

If the repo is public, and a git sha is specified in the spec that we're fetching, then we never hit it via git at all; npm will just fetch the tarball from the appropriate CDN url for the known host. However, if you do not have a git sha in the spec (typically, because there's no lockfile present), then we will have to do a git rev-list to figure out which sha we need to fetch, and that's where it runs into trouble.

Perhaps an approach that both works in build scenarios without ssh keys and for dev machines where ssh keys are present, while minimizing the scenarios where we need to prompt for a username/password for basic auth, would be to do the following for resolving any known-host git spec:

1. If the spec has a git sha, just fetch the tarball from the CDN, and we're done. No git commands needed. If that fails, continue (either a private package or an invalid git sha).
2. If the spec is git+https, and has auth specified, save the full git+https url, and only ever access via https.
3. Attempt to fetch the rev-list via the git+ssh url for the known host/repo. This will fail if ssh keys are not available, but at least will do so without confusingly prompting for user interaction, in the case of private repos where ssh keys are available. If that fails, continue.
4. Attempt to fetch the rev-list via the git+https url for the known host/repo. This will fail if auth is required for a private package. (Note: best to just close stdin immediately, so that it fails as quickly as possible.) But, it will succeed in cases where auth is not required and ssh keys are not available.

Maybe we could also add a config to tell npm which git interaction to prefer, thus swapping the order of (3) and (4) above? --git-prefer=(ssh-first|https-first|ssh-only|https-only), defaulting to ssh-first? Then you could set npm_config_git_prefer=https-only in the build environment, and have it never try ssh at all.

[Tallyb]

Having an all repo config is probably not the best idea, as packages come in all shapes and forms. I think respecting the installation method provided during the npm install is probably the best approach. If anyone needs ssh or https, they should simply add the package this way.

[isaacs]

Having an all repo config is probably not the best idea, as packages come in all shapes and forms.

This is only relevant when pulling packages from known hosts like GitHub or GitLab, where the "shape and size" is fairly well understood. The difference is in the current environment capabilities. Ie, can it use ssh? should it fail if it can't get the repo via git+https, or should it fall back to ssh? should it use https first, or ssh first? These are all system-specific, not dependency- or repo-specific.

[uhop]

These are all system-specific, not dependency- or repo-specific.

Could these decisions be left for a developer, who provides explicit dependencies? Like if a URL starts with https:// it is, well, https, and if it is git+ssh:// then it should be, well, git over ssh? Just asking for a friend...

[uhop]

Having well-known hosts is admirable, as long as it is transparent, yet it smells like magic — an extra-bandwidth knowledge, which makes the system behave differently on unexpected factors. In my experience, it is a major source of support incidents and perceived bugs even when everything works fine.

[themightychris]

this magic is unacceptable, there are no two ways about it. Do what you want with a github: link but if I address a dependency explicitly with https:// or git+https:// then NPM needs to use the requested protocol

[reconbot]

This has broken quite a few of my github actions as I don't have ssh keys setup there.

[iarna]

I saw this being talked about on Twitter and thought I could add a bit:

What Isaac is describing in terms of magic fallback behavior isn't new, and it's not what's breaking y'all. What evidently has changed is that the transport it ended up using is now recorded as the "version". I'd argue this is wrong, that's what "from" is for. For registry dependencies "version" is the version we resolved our specifier to, from the tarball source. For non-registry dependencies, "version" should be the specifier that was originally requested. Making "from" be the actual resource fetched seems reasonable, assuming that npm's of all versions ignore it, or fall back to the "version" if fetching it fails. Making "version" this, well gestures at ticket.

Edit: See below for a more accurate discussion.

[reconbot]

@iarna I think this makes sense to me. Locally ssh works, so my v2 lockfile records the git+ssh that it was always using behind the scenes I didn't know about it. On github actions, it doesn't have my ssh key and the saved resolved specifier doesn't work anymore.

I'm not 100% clear on the structure of a v2 lockfile but I do see what you're talking about

  "packages": {
    "": {
      "version": "1.0.0",
      "license": "ISC",
      "dependencies": {
        "@architect/functions": "git+https://github.com/reconbot/functions.git#9994014",

// later

    "node_modules/@architect/functions": {
      "resolved": "git+ssh://[email protected]/reconbot/functions.git#99940146d4bbd5705b0b65804dadee0134f8df4d",

// later
    "@architect/functions": {
      "version": "git+ssh://[email protected]/reconbot/functions.git#99940146d4bbd5705b0b65804dadee0134f8df4d",
      "from": "@architect/functions@git+https://github.com/reconbot/functions.git#9994014",

[uhop]

In the discussion one point was missed: the difference between npm i and npm ci security-wise (see the original ticket). Yes, npm i can choose different versions, yet the security model should be exactly the same. If listed packages and their dependencies didn't change, their behavior should be identical. (This is my case actually). Yet they behave differently. I hope everyone agrees that it clearly is a bug, not something that was designed consciously.

Likely @iarna is totally right and the bug lies in how package-lock.json is generated.

PS: Actually because of the discrepancy in npm i and npm ci I can still work around this bug, so it is not a showstopper for me, but merely a nuisance.

[iarna]

Hah, yeah, you caught me in the process of correcting myself:

1. The from field, for ALL types, is only ever the specifier that was used to pick the version recorded in the lockfile. That hasn't changed -- npm@7 is fine here. (My brain was crosswiring it with resolved, when it's used. Resolved wouldn't be a terrible place to record the transport, but I'd be hesitant to do so.)

There seem to be layers of changes here:

First, if you install iarna/aproba, it correctly records github:iarna/aproba to your package.json. But if you install https://github.com/iarna/aproba it surprisingly records github:iarna/aproba. That is, it stops using the default representation for the specifier you gave and switches to always using the shortcut.

Second, the lock file:

In npm@7 the transport it ended up using is recorded to the lockfile, packages section as:

    "node_modules/aproba": {
      "version": "2.0.0",
      "resolved": "git+ssh://[email protected]/iarna/aproba.git#ab9127d496bd0729d2d82037459b5da0e408362e",
      "license": "ISC"
    }

and legacy dependencies section as (which I presume is ignored by npm@7, but will impact npm@6):

    "aproba": {
      "version": "git+ssh://[email protected]/iarna/aproba.git#ab9127d496bd0729d2d82037459b5da0e408362e",
      "from": "aproba@github:iarna/aproba"
    }
  }

Versus npm@6, where it is not:

    "aproba": {
      "version": "github:iarna/aproba#ab9127d496bd0729d2d82037459b5da0e408362e",
      "from": "github:iarna/aproba"
    }

Similarly, if the package.json specifier was an https or git+https URL, then that was recorded as such to the npm@6 lockfile, in npm@7 it's recorded in the "from" but the resolved/version it the git+ssh as above.

I dunno, it's weird, 'cause historically it would be parsed with npm-package-arg and that wouldn't treat https://github.com/iarna/aproba as a "hosted" dep. It'd treat it as an opaque git URL and other than prepending git+ (to eliminate ambiguity between git deps and tarball deps) would leave it alone. Apparently some where things are being passed to hosted-git-info FIRST, and routing based on that?

Edit: Ah maybe I do see how this could happen:

npm-package-arg (npa) results do include a hosted object regardless of form. If you select based on that, then you might get into trouble. There was some of this before, but the details elude me. Clearly not the way it is now.

npa returns a fetchSpec in cases where it knows exactly the URL you should be fetching. So while it has a hosted object for git+https://github.com/iarna/aproba, it ALSO has a fetchSpec of https://github.com/iarna/aproba which is what you were supposed to use when fetching. For shortcuts, eg github:iarna/aproba it DOES NOT have a fetchSpec, because shortcuts imply you want the magic "guess the protocol" behavior. Somewhere along the line, this distinction was lost. (This is a distinct problem from the protocol being recorded to the lockfiles -- but probably related -- with the "always treat hosted git urls as magic" behavior it means that recording a specific transport to the lockfile doesn't change overall behavior as it would have previously.)

[iarna]

I do vaguely recall something protocol specific URLs not actually disabling magic entirely? But instead changing the order that things were tried? Maybe that plays a role? (eg, github: results in trying ssh, https, git, where as git+https:// results in https, ssh, git and git:// results in git, ssh, https?

Anyway, regardless, the result was that the protocol you asked for was always the first thing tried, even if there may have been fallbacks.

[isaacs]

@iarna The entirety of the git handling in pacote was rewritten for npm v7. We now always try to fetch the CDN tarball first, since that's invariably faster (even when it fails, it fails very fast, so compared with the overhead of spawning a git process and doing the fetch that way, it's a worthwhile gamble). But basically, all the code that did this in npm v6 is gone.

The "magic" being complained about here is actually providing some value; https won't work in some cases, and ssh won't work in some others. It makes sense to first use the protocol specified. It also makes sense to try another protocol that we know will result in the same bits on disk, if it's a host we know how to talk to. The choice of whether to speak git over https or ssh is really dependent on the platform currently performing the install, not on the first platform where the install was performed. That says to me that the precedence of which to try should be a config.

We do store the https to the lockfile now if it contains auth, because otherwise information is lost, since auth can't be provided to the ssh or shorthand forms, and if that is present we only attempt to use git over https. The suggestion above should make this successfully install for all cases described. The remaining objection reduces to "No, I want it to break if I don't have ssh and I specified ssh", and... well... npm installs packages. If we can satisfy the dependency contracts, we're going to. Once it's in the lockfile with a sha, we don't use git over https or ssh; we don't even use git at all, we just fetch the tarball from the known host. The only time we do a git checkout for a known host is if the CDN doesn't have the tarball (ie, for private repos).

Re the lockfile stuff, npm v7 will only use the dependencies section if there is no packages section present. It will create that section for backwards compatibility, but it'll go away once npm v6 is no longer relevant. If the bug was "we installed with npm v7, and then npm v6 stopped working", then it would be worth investigating there, but otherwise, I don't think that's relevant to this.

[reconbot]

I’m not sure if I understand. I installed locally with npm7 and it broke on GitHub actions with npm7. That’s my bug.

[Tallyb]

I want to make sure I understand it correctly: If I am doing npm i <some public github repo> from a machine that is configured with SSH, I will end up with git+ssh definition in my package-lock.json. If a teammate is doing it from a machine that has no SSH configured, it will end up with the git+https definition. I do not think that this is acceptable in real-life scenarios, as I cannot verify the whole team is aligned on SSH, and most likely they will not be.

[themightychris]

The "magic" being complained about here is actually providing some value; https won't work in some cases, and ssh won't work in some others. It makes sense to first use the protocol specified. It also makes sense to try another protocol that we know will result in the same bits on disk, if it's a host we know how to talk to

@isaacs this is not what happens when a v2 lockfile is added to the picture -- if SSH works for the first person, it gets written into the lockfile, and if they commit that then npm install and npm ci fails for teammates and CI environments without SSH configured. On top of that, if a lockfile is committed with the HTTPS variant, anyone running npm install in an environment where SSH works will overwrite the lockfile to SSH and be compelled to commit it

The current behavior essentially makes it impossible to maintain an HTTPS reference in a v2 lockfile no matter what you put in package.json as long as anyone on the team has SSH configured, as people are widely trained to commit any changes to lockfiles without reviewing them in detail. There is then no way for CI environments to work unless you set up github SSH keys on them even if they only pull public packages, or you've set them up to authenticate github requests via HTTPS

[isaacs]

The fix described in my comment above has not been implemented.

The way forward is to fetch packages in a more resilient way from known git hosts, and save metadata in a way that is as consistent as possible (to reduce unnecessary/hazardous lockfile churn) and does not lose information (eg, saving ssh when we really need the auth to make https work properly).

This will be addressed by https://github.com/npm/pacote/issues/65

[themightychris]

@isaacs one other potential issue I've encountered in all this, that is a bit tangential but maybe should be considered at the same time. It is common in GitHub Actions settings to do something like this to authenticate requests to github either using the limited token that is automatically made available to each actions run, or a Personal Access Token stored in secrets for a service account. In your reference comment above you mention preserving an explicit git+https reference only when it includes auth, but there's a decently common use case where the HTTP auth isn't stored in package.json but rather injected into the runtime environment with git's insteadOf facility.

[isaacs]

@themightychris Yep. All the more reason why it's important to fall back on failure. Install environments are quite often configured such that https will always work and ssh never will, or vice versa.

We save the git+https url when it contains auth so that we don't lose the auth information that the user provided, since presumably that is useful at least sometimes, and it is surprising to have it just disappear. But we should still fall back by default to ssh when an authed git+https fails.

[uhop]

We save the git+https url when it contains auth

We are discouraged by our security policy to commit any kind of credentials to repositories. Our dependency URLs do not contain auth. It is supplied externally by the build system, e.g., AWS CodeBuild.

And there is an issue that npm i works fine (as expected), while npm ci doesn't. I understand that should work the same way if no external changes to dependencies.

[isaacs]

@uhop I'm confused by why it might be working with npm i but not with npm ci. Are you using npm v7? Is there a full node_modules tree present?

In npm v7, npm ci is essentially just rm -rf node_modules ; npm i. They call into the exact same code path, so I'd be surprised if they worked significantly differently with respect to resolving git deps.

[tstibbs]

For the benefit of anyone looking for a workaround who struggled to understand the conversation above, if you are installing something from a public github repo and want to force https, this should work:

npm install git+https://<githubUsername>@github.com/<project>/<repo>.git

It potentially doesn't matter what github username you use, because everyone can access public repos. The use of the username qualifies as 'auth' and therefore forces the protocol to be kept, as explained in point 2 in https://github.com/npm/cli/issues/2610#issuecomment-780761122.

---

Just to add a +1 to the opinion expressed multiple times above, if I do npm install github:user/repo then do your magic. It's appreciated. I think the logic is currently slightly flawed, but the magic of "try all the protocols as one of them might work" seems useful. However, if I do npm install git+https://github.com/user/repo.git then please please please respect my choice. Try a fall back if you really think that helps, but start off by using my choice of protocol - I specified it for a reason!

[uhop]

@isaacs While I do use npm v7 locally and it forms package-lock.json, code builds may run other versions. I just checked one AWS CodeBuild we have and it runs node v12.18.0 (with npm 6.14.4). It is tripped by the existing package-lock.json and works only with npm i, which is not what I want.

The relevant portion of the log with versions and up to the warning (different version, but compatible):

[Container] 2021/02/03 15:57:12 Running command n $NODE_12_VERSION
 installed : v12.18.0 (with npm 6.14.4)

[Container] 2021/02/03 15:57:27 Moving to directory /codebuild/output/src685134267/src/github.com/ZZZ/YYY
[Container] 2021/02/03 15:57:27 Registering with agent
[Container] 2021/02/03 15:57:27 Phases found in YAML: 3
[Container] 2021/02/03 15:57:27  INSTALL: 1 commands
[Container] 2021/02/03 15:57:27  PRE_BUILD: 4 commands
[Container] 2021/02/03 15:57:27  BUILD: 1 commands
[Container] 2021/02/03 15:57:27 Phase complete: DOWNLOAD_SOURCE State: SUCCEEDED
[Container] 2021/02/03 15:57:27 Phase context status code:  Message:
[Container] 2021/02/03 15:57:27 Entering phase INSTALL
[Container] 2021/02/03 15:57:27 Running command npm install
npm WARN read-shrinkwrap This version of npm is compatible with lockfileVersion@1, but package-lock.json was generated for lockfileVersion@2. I'll try to do my best with it!

[liamdawson]

3. This will fail if ssh keys are not available, but at least will do so without confusingly prompting for user interaction

I use SSH via a PKCS11 smartcard, and it prompts me to physically touch it whenever I'm establishing an SSH connection (and it's been 15s since the last touch*). Automatically mapping public GitHub URLs to attempt SSH access first would mean I'd possibly have to keep touching the key within a single npm install operation. Additionally, if I run npm install and walk away to do something else, git will hang for ~60s every time it's trying to use that SSH key, so that could potentially lengthen install times too.

Not only would that be immensely irritating, it would also condition me to keep touching whenever I do an npm install, which would defeat the purpose if I ever installed a malicious package that immediately established SSH connections itself.

* GnuPG smartcard support didn't even give me this option — I had to touch the key for every unique connection

[tchakabam]

Tbh I am still quite confused by the cause and fix for this on npm side ... :D

To fix it on our side, I made sure our CI uses latest npm now - it was using some prior version and failing with a public repo where we had no protocol specified.

Yes I haven't read all the conversation after the first explanations on the internal procedure to fetch git dependencies, but I guess some update/summary from NPM dev side would be appropriate, since this is still open status?

What I don't understand about the explanation from @isaacs here is:

This will fail if ssh keys are not available, but at least will do so without confusingly prompting for user interaction, in the case of private repos where ssh keys are available. If that fails, continue.

I can understand it attempts SSH by default if the URL has no scheme at all (but also that apparently wasn't always the case, it is a breaking change, and should be optional).

But, if https is specified, then I don't see that it possibly failing & prompting for input as a reasonable rationale for performing the step described here ("falling back" to SSH). Indeed, it is as a fallback, without actually having attempted HTTPS first at all. I am pretty sure that git can be passed an option or locally configured so that it can silently fail HTTP auth without any input prompt (if it all, since this would usually not be the case with github public repos).

I strongly agree with the other users pointing out that ignoring/overriding the explicit protocol specified in the package can't be a viable choice at all.

I am bit surprised that npm as a software is doing such things, that remind be a bit of software that is doing things in non-transparent ways, to solve other problems in the wrong way.

I understand you might have wanted to fix the issue of user-prompting on https auth failure for some users case here.

But, generally, I think that such a behavior released should if at all be optional (especially since it is a potentially breaking change for many), and if it runs by default, be indicated with a warning and possible workaround (the https+git@username "mode" indicated above). I admit I am a bit sceptical about npm releasing such changes in such basic routines w/o thinking further about the impact and different use-cases.