bugfix: Prevent arborist from changing protocols as part of resolve step

tap-snapshots/test/lib/commands/ls.js.test.cjs

@@ -577,7 +577,7 @@ print-deduped-symlinks@1.0.0 {CWD}/tap-testdir-ls-ls-print-deduped-symlinks
 
 exports[`test/lib/commands/ls.js TAP ls resolved points to git ref > should output tree containing git refs 1`] = `
 test-npm-ls@1.0.0 {CWD}/tap-testdir-ls-ls-resolved-points-to-git-ref
-\`-- abbrev@1.1.1 (git+ssh://git@github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c)
+\`-- abbrev@1.1.1 (git+https://github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c)
 
 `
 

test/lib/commands/ls.js

@@ -3684,7 +3684,7 @@ t.test('ls --json', t => {
           abbrev: {
             version: '1.1.1',
             /* eslint-disable-next-line max-len */
-            resolved: 'git+ssh://git@github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c',
+            resolved: 'git+https://github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c',
           },
         },
       },
@@ -4618,7 +4618,7 @@ t.test('ls --package-lock-only', t => {
           dependencies: {
             abbrev: {
               /* eslint-disable-next-line max-len */
-              version: 'git+ssh://git@github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c',
+              version: 'git+https://github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c',
               from: 'abbrev@git+https://github.com/isaacs/abbrev-js.git',
             },
           },
@@ -4633,7 +4633,7 @@ t.test('ls --package-lock-only', t => {
           dependencies: {
             abbrev: {
               /* eslint-disable-next-line max-len */
-              resolved: 'git+ssh://git@github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c',
+              resolved: 'git+https://github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c',
             },
           },
         },

workspaces/arborist/lib/consistent-resolve.js

@@ -4,6 +4,7 @@
 // lockfiles, and for converting hosted git repos to a consistent url type.
 const npa = require('npm-package-arg')
 const relpath = require('./relpath.js')
+
 const consistentResolve = (resolved, fromPath, toPath, relPaths = false) => {
   if (!resolved) {
     return null
@@ -20,16 +21,38 @@ const consistentResolve = (resolved, fromPath, toPath, relPaths = false) => {
       raw,
     } = npa(resolved, fromPath)
     const isPath = type === 'file' || type === 'directory'
-    return isPath && !relPaths ? `file:${fetchSpec.replace(/#/g, '%23')}`
-      : isPath ? 'file:' + (toPath ? relpath(toPath, fetchSpec.replace(/#/g, '%23')) : fetchSpec.replace(/#/g, '%23'))
-      : hosted ? `git+${
-        hosted.auth ? hosted.https(hostedOpt) : hosted.sshurl(hostedOpt)
-      }`
-      : type === 'git' ? saveSpec
+
+    if (isPath && !relPaths) {
+      return `file:${fetchSpec.replace(/#/g, '%23')}`
+    }
+
+    if (isPath) {
+      if (toPath) {
+        return 'file:' + relpath(toPath, fetchSpec.replace(/#/g, '%23'))
+      } else {
+        return 'file:' + fetchSpec.replace(/#/g, '%23')
+      }
+    }
+
+    if (hosted) {
+      if (hosted.default === 'https') {
+        return `git+${hosted.https(hostedOpt)}`
+      } else {
+        return `git+${hosted.sshurl(hostedOpt)}`
+      }
+    }
+
+    if (type === 'git') {
+      return saveSpec
+    }
+
+    if (rawSpec === '' && raw.slice(-1) !== '@') {
       // always return something.  'foo' is interpreted as 'foo@' otherwise.
-      : rawSpec === '' && raw.slice(-1) !== '@' ? raw
-      // just strip off the name, but otherwise return as-is
-      : rawSpec
+      return raw
+    }
+
+    // just strip off the name, but otherwise return as-is
+    return rawSpec
   } catch (_) {
     // whatever we passed in was not acceptable to npa.
     // leave it 100% untouched.

workspaces/arborist/tap-snapshots/test/arborist/load-actual.js.test.cjs

@@ -1594,7 +1594,7 @@ ArboristNode {
       "name": "full-git-url",
       "packageName": "abbrev",
       "path": "install-types/node_modules/full-git-url",
-      "resolved": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
+      "resolved": "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
       "version": "1.1.1",
     },
     "ghshort" => ArboristNode {

workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs

@@ -13530,7 +13530,7 @@ ArboristNode {
       "location": "node_modules/full-git-url",
       "name": "full-git-url",
       "path": "{CWD}/test/fixtures/install-types/node_modules/full-git-url",
-      "resolved": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
+      "resolved": "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
     },
     "ghshort" => ArboristNode {
       "edgesIn": Set {
@@ -14219,7 +14219,7 @@ ArboristNode {
       "location": "node_modules/full-git-url",
       "name": "full-git-url",
       "path": "{CWD}/test/fixtures/install-types/node_modules/full-git-url",
-      "resolved": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
+      "resolved": "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
     },
     "ghshort" => ArboristNode {
       "edgesIn": Set {
@@ -14908,7 +14908,7 @@ ArboristNode {
       "location": "node_modules/full-git-url",
       "name": "full-git-url",
       "path": "{CWD}/test/arborist/tap-testdir-load-virtual-load-from-npm-shrinkwrap.json/node_modules/full-git-url",
-      "resolved": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
+      "resolved": "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
     },
     "ghshort" => ArboristNode {
       "edgesIn": Set {
@@ -15561,7 +15561,7 @@ ArboristNode {
       "location": "node_modules/full-git-url",
       "name": "full-git-url",
       "path": "{CWD}/test/fixtures/install-types-sw-only/node_modules/full-git-url",
-      "resolved": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
+      "resolved": "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
     },
     "ghshort" => ArboristNode {
       "location": "node_modules/ghshort",

workspaces/arborist/tap-snapshots/test/shrinkwrap.js.test.cjs

@@ -1000,7 +1000,7 @@ Object {
       "version": "1.0.0",
     },
     "node_modules/full-git-url": Object {
-      "resolved": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
+      "resolved": "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
     },
     "node_modules/ghshort": Object {
       "resolved": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
@@ -1883,7 +1883,7 @@ Object {
     },
     "full-git-url": Object {
       "from": "full-git-url@git+https://github.com/isaacs/abbrev-js.git",
-      "version": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
+      "version": "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
     },
     "ghshort": Object {
       "from": "ghshort@github:isaacs/abbrev-js",
@@ -2094,7 +2094,7 @@ Object {
     "node_modules/full-git-url": Object {
       "license": "ISC",
       "name": "abbrev",
-      "resolved": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
+      "resolved": "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
       "version": "1.1.1",
     },
     "node_modules/ghshort": Object {
@@ -14192,7 +14192,7 @@ Object {
 
 exports[`test/shrinkwrap.js TAP look up from locks and such lockfile > full git 1`] = `
 Object {
-  "resolved": "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
+  "resolved": "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb",
 }
 `
 

workspaces/arborist/tap-snapshots/test/yarn-lock.js.test.cjs

@@ -121,7 +121,7 @@ exports[`test/yarn-lock.js TAP load a yarn lock from an actual tree install-type
   "version" "1.0.0"
 
 "full-git-url@git+https://github.com/isaacs/abbrev-js.git":
-  "resolved" "git+ssh://git@github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb"
+  "resolved" "git+https://github.com/isaacs/abbrev-js.git#a9ee72ebc8fe3975f1b0c7aeb3a8f2a806a432eb"
   "version" "1.1.1"
 
 "ghshort@github:isaacs/abbrev-js":

workspaces/arborist/test/consistent-resolve.js

@@ -56,37 +56,46 @@ t.test('file and directories made consistent if toPath not set', t => {
 
 t.test('consistent hosted git info urls', t => {
   const expect = 'git+ssh://git@github.com/a/b.git'
-  const expectAuth = 'git+https://user:pass@github.com/a/b.git'
   t.equal(cr('a/b'), expect)
   t.equal(cr('github:a/b'), expect)
-  t.equal(cr('git+https://github.com/a/b'), expect)
   t.equal(cr('git://github.com/a/b'), expect)
   t.equal(cr('git+ssh://git@github.com/a/b'), expect)
-  t.equal(cr('git+https://github.com/a/b.git'), expect)
   t.equal(cr('git://github.com/a/b.git'), expect)
   t.equal(cr('git+ssh://git@github.com/a/b.git'), expect)
-  t.equal(cr('git+https://user:pass@github.com/a/b.git'), expectAuth)
 
   const hash = '#0000000000000000000000000000000000000000'
   t.equal(cr('a/b' + hash), expect + hash)
   t.equal(cr('github:a/b' + hash), expect + hash)
-  t.equal(cr('git+https://github.com/a/b' + hash), expect + hash)
   t.equal(cr('git://github.com/a/b' + hash), expect + hash)
   t.equal(cr('git+ssh://git@github.com/a/b' + hash), expect + hash)
-  t.equal(cr('git+https://github.com/a/b.git' + hash), expect + hash)
   t.equal(cr('git://github.com/a/b.git' + hash), expect + hash)
   t.equal(cr('git+ssh://git@github.com/a/b.git' + hash), expect + hash)
   t.equal(cr('xyz@a/b' + hash), expect + hash)
   t.equal(cr('xyz@github:a/b' + hash), expect + hash)
-  t.equal(cr('xyz@git+https://github.com/a/b' + hash), expect + hash)
   t.equal(cr('xyz@git://github.com/a/b' + hash), expect + hash)
   t.equal(cr('xyz@git+ssh://git@github.com/a/b' + hash), expect + hash)
-  t.equal(cr('xyz@git+https://github.com/a/b.git' + hash), expect + hash)
   t.equal(cr('xyz@git://github.com/a/b.git' + hash), expect + hash)
   t.equal(cr('xyz@git+ssh://git@github.com/a/b.git' + hash), expect + hash)
   t.end()
 })
 
+t.test('consistent resolve hosted git to https urls', t => {
+  const expect = 'git+https://github.com/a/b.git'
+  const expectAuth = 'git+https://user:pass@github.com/a/b.git'
+  const expectUserOnlyAuth = 'git+https://user@github.com/a/b.git'
+  t.equal(cr('git+https://github.com/a/b'), expect)
+  t.equal(cr('git+https://github.com/a/b.git'), expect)
+  t.equal(cr('git+https://user:pass@github.com/a/b.git'), expectAuth)
+  t.equal(cr('git+https://user@github.com/a/b.git'), expectUserOnlyAuth)
+
+  const hash = '#0000000000000000000000000000000000000000'
+  t.equal(cr('git+https://github.com/a/b' + hash), expect + hash)
+  t.equal(cr('git+https://github.com/a/b.git' + hash), expect + hash)
+  t.equal(cr('xyz@git+https://github.com/a/b' + hash), expect + hash)
+  t.equal(cr('xyz@git+https://github.com/a/b.git' + hash), expect + hash)
+  t.end()
+})
+
 t.test('unhosted git returns saveSpec', t => {
   const r = 'git+https://x.com/y.git#0000000000000000000000000000000000000000'
   t.equal(cr(r), r)