fix: ensure that resolved git ref matches expected sha (#439)

In npm/rfcs#525 about ignoring `integrity` values in lockfiles it was
stated:
> the sha is already what gets stored in the resolved field today

This is only true for resolutions from non-commits to commits.

A dependency like `git://...#4b559c4c663a23f988f6be5094c9a45faf6231bc`
will be stored using the same "reference" in `resolved` even when it
cloned a branch or a tag that resolved to a different sha.

The update is only done if it hasn't been resolved yet, which is already
the case if a full "commit" was specified:

https://github.com/npm/pacote/blob/4b559c4c663a23f988f6be5094c9a45faf6231bc/lib/git.js#L263-L265

This also applies to `npm ci` after reading `package-lock.json` as it
will use the same resolution.

This will compare the newly returned commit-hash with a previously set
`resolvedSha` and prevent that from happening.

Co-authored-by: pacotedev <i+pacotedev@izs.me>

Author: klassiker

lib/git.js

@@ -25,6 +25,14 @@ const repoUrl = (h, opts) =>
 // add git+ to the url, but only one time.
 const addGitPlus = url => url && `git+${url}`.replace(/^(git\+)+/, 'git+')
 
+const checkoutError = (expected, found) => {
+  const err = new Error(`Commit mismatch: expected SHA ${expected} and cloned HEAD ${found}`)
+  err.code = 'EGITCHECKOUT'
+  err.sha = expected
+  err.head = found
+  return err
+}
+
 class GitFetcher extends Fetcher {
   constructor (spec, opts) {
     super(spec, opts)
@@ -259,6 +267,10 @@ class GitFetcher extends Fetcher {
         h ? this.#cloneHosted(ref, tmp)
         : this.#cloneRepo(this.spec.fetchSpec, ref, tmp)
       )
+      // if we already have a resolved sha ensure it doesn't change
+      if (this.resolvedSha && this.resolvedSha !== sha) {
+        throw checkoutError(this.resolvedSha, sha)
+      }
       this.resolvedSha = sha
       if (!this.resolved) {
         await this.#addGitSha(sha)

test/git.js

@@ -72,6 +72,7 @@ HostedGit.addHost('localhostssh', {
 })
 
 const remote = `git://localhost:${gitPort}/repo`
+const remoteBroken = `git://localhost:${gitPort}/broken`
 const remoteHosted = `git://127.0.0.1:${gitPort}/repo`
 const submodsRemote = `git://localhost:${gitPort}/submodule-repo`
 const workspacesRemote = `git://localhost:${gitPort}/workspaces-repo`
@@ -86,6 +87,7 @@ const me = t.testdir({
   cache: {},
 })
 const repo = resolve(me, 'repo')
+const broken = resolve(me, 'broken')
 const cache = resolve(me, 'cache')
 const cycleA = resolve(me, 'cycle-a')
 const cycleB = resolve(me, 'cycle-b')
@@ -455,6 +457,37 @@ t.test('ignores integrity for git deps', { skip: isWindows && 'posix only' }, as
   t.end()
 })
 
+t.test('detects changes in the resolved sha', {}, async (t) => {
+  const git = (...cmd) => spawnGit(cmd, { cwd: broken })
+  const write = (f, c) => fs.writeFileSync(f, c)
+
+  await mkdir(broken, { recursive: true })
+    .then(() => git('config', 'user.name', 'pacotedev'))
+    .then(() => git('config', 'user.email', 'i+pacotedev@izs.me'))
+    .then(() => git('config', 'tag.gpgSign', 'false'))
+    .then(() => git('config', 'commit.gpgSign', 'false'))
+    .then(() => git('config', 'tag.forceSignAnnotated', 'false'))
+    .then(() => git('init'))
+    .then(() => write(`${broken}/package.json`, JSON.stringify({
+      name: 'repo',
+      version: '0.0.0',
+    })))
+    .then(() => git('add', 'package.json'))
+    .then(() => git('commit', '-m', 'package json file'))
+    .then(() => git('checkout', '-b', REPO_HEAD))
+
+  const { stdout: actual } = await git('rev-parse', 'HEAD')
+
+  const fetcher = new GitFetcher(remoteBroken + '#' + REPO_HEAD, opts)
+  await t.rejects(() => fetcher.manifest(), {
+    message: `Commit mismatch: expected SHA ${REPO_HEAD} and cloned HEAD ${actual}`,
+    code: 'EGITCHECKOUT',
+    sha: REPO_HEAD,
+    head: actual,
+  })
+  t.end()
+})
+
 t.test('weird hosted that doesnt provide any fetch targets', { skip: isWindows && 'posix only' },
   t => {
     const hosted = {