From b10096228cca0275150ea1b4e1b4416fb70cbe96 Mon Sep 17 00:00:00 2001
From: Utsav Shah <utsavkunalshah@gmail.com>
Date: Thu, 26 Dec 2024 20:48:59 -0500
Subject: [PATCH] fix(git-resolver): handle private git repo resolution (#8906)

* fix(git-resolver): handle private git repo resolution

In the case where:

1. No git auth token was specified by the user
2. The package requested to be fetched via https
3. The user does not have SSH access to the repo but has HTTPS access
4. The package was hosted in a private GitHub repo

pnpm would fallback to using SSH since it was a "likely private repo"
and would fail to resolve the package. Now, rather than only checking if
there is an auth token specified, it also checks both:

1. Is the repo private
2. Does the user have access to ls-remote it.

And if these conditions are true, it tries to use https anyway.

This matches the behavior of npm and Yarn berry. Yarn classic also has
this bug, and there's a code comment that alludes to it.
---
 .changeset/loud-pans-matter.md                |  6 +++++
 resolving/git-resolver/src/parsePref.ts       |  4 ++--
 resolving/git-resolver/test/index.ts          | 23 +++++++++++++++++++
 resolving/git-resolver/test/parsePref.test.ts |  7 ++++++
 4 files changed, 38 insertions(+), 2 deletions(-)
 create mode 100644 .changeset/loud-pans-matter.md

diff --git a/.changeset/loud-pans-matter.md b/.changeset/loud-pans-matter.md
new file mode 100644
index 0000000000..bdf952327e
--- /dev/null
+++ b/.changeset/loud-pans-matter.md
@@ -0,0 +1,6 @@
+---
+"@pnpm/git-resolver": patch
+"pnpm": patch
+---
+
+Do not fall back to SSH, when resolving a git-hosted package if `git ls-remote` works via HTTPS [#8906](https://github.com/pnpm/pnpm/pull/8906).
diff --git a/resolving/git-resolver/src/parsePref.ts b/resolving/git-resolver/src/parsePref.ts
index e568c1cc4f..4c4cf3cd36 100644
--- a/resolving/git-resolver/src/parsePref.ts
+++ b/resolving/git-resolver/src/parsePref.ts
@@ -79,7 +79,7 @@ async function fromHostedGit (hosted: any): Promise<HostedPackageSpec> { // esli
   if (!fetchSpec) {
     const httpsUrl: string | null = hosted.https({ noGitPlus: true, noCommittish: true })
     if (httpsUrl) {
-      if (hosted.auth && await accessRepository(httpsUrl)) {
+      if ((hosted.auth || !await isRepoPublic(httpsUrl)) && await accessRepository(httpsUrl)) {
         return {
           fetchSpec: httpsUrl,
           hosted: {
@@ -94,7 +94,7 @@ async function fromHostedGit (hosted: any): Promise<HostedPackageSpec> { // esli
         try {
           // when git ls-remote private repo, it asks for login credentials.
           // use HTTP HEAD request to test whether this is a private repo, to avoid login prompt.
-          // this is very similar to yarn's behavior.
+          // this is very similar to yarn classic's behavior.
           // npm instead tries git ls-remote directly which prompts user for login credentials.
 
           // HTTP HEAD on https://domain/user/repo, strip out ".git"
diff --git a/resolving/git-resolver/test/index.ts b/resolving/git-resolver/test/index.ts
index ad8a597490..c08b98db98 100644
--- a/resolving/git-resolver/test/index.ts
+++ b/resolving/git-resolver/test/index.ts
@@ -461,6 +461,29 @@ test('resolve a private repository using the HTTPS protocol without auth token',
   })
 })
 
+test('resolve a private repository using the HTTPS protocol with a commit hash', async () => {
+  git.mockImplementation(async (args: string[]) => {
+    expect(args).toContain('ls-remote')
+    expect(args).toContain('https://github.com/foo/bar.git')
+    return {
+      // cspell:ignore aabbccddeeff
+      stdout: 'aabbccddeeff\tHEAD',
+    }
+  })
+  const resolveResult = await resolveFromGit({ pref: 'git+https://github.com/foo/bar.git#aabbccddeeff' })
+  expect(resolveResult).toStrictEqual({
+    id: 'git+https://github.com/foo/bar.git#aabbccddeeff',
+    normalizedPref: 'git+https://github.com/foo/bar.git',
+    resolution: {
+      // cspell:ignore aabbccddeeff
+      commit: 'aabbccddeeff',
+      repo: 'https://github.com/foo/bar.git',
+      type: 'git',
+    },
+    resolvedVia: 'git-repository',
+  })
+})
+
 test('resolve a private repository using the HTTPS protocol and an auth token', async () => {
   git.mockImplementation(async (args: string[]) => {
     if (!args.includes('https://0000000000000000000000000000000000000000:x-oauth-basic@github.com/foo/bar.git')) throw new Error('')
diff --git a/resolving/git-resolver/test/parsePref.test.ts b/resolving/git-resolver/test/parsePref.test.ts
index a7f280ba9d..166c8476b5 100644
--- a/resolving/git-resolver/test/parsePref.test.ts
+++ b/resolving/git-resolver/test/parsePref.test.ts
@@ -44,3 +44,10 @@ test.each([
   const parsed = await parsePref(input)
   expect(parsed?.path).toBe(output)
 })
+
+test.each([
+  ['git+https://github.com/pnpm/pnpm.git', 'https://github.com/pnpm/pnpm.git'],
+])('the fetchSpec of %s should be %s', async (input, output) => {
+  const parsed = await parsePref(input)
+  expect(parsed?.fetchSpec).toBe(output)
+})