diff --git a/.changeset/git-ci-https-fallback.md b/.changeset/git-ci-https-fallback.md new file mode 100644 index 0000000000..0ad30c49dc --- /dev/null +++ b/.changeset/git-ci-https-fallback.md @@ -0,0 +1,17 @@ +--- +"@pnpm/resolving.git-resolver": patch +"pnpm": patch +--- + +Fixed a CI regression where `github:owner/repo` dependencies (and other shorthand Git specifiers) would fail to install with `Permission denied (publickey)` on CI runners that lack SSH keys. The Git resolver no longer records an SSH URL unless the user explicitly wrote one (e.g. `git+ssh://` or `git@host:...`): + +- The repository visibility probe (an HTTP HEAD request) now retries transient failures such as `429 Too Many Requests`, so host throttling of CI runners is no longer mistaken for a private repository. +- For non-SSH specifiers, anonymous HTTPS `git ls-remote` access is now tried before SSH, so a public repository whose visibility probe fails still resolves to a portable HTTPS URL instead of an SSH URL that only works where SSH keys are configured. +- When every probe fails, the resolver falls back to HTTPS for shorthand and HTTPS-style specifiers, and only guesses SSH when the user explicitly provided an SSH URL. +- A repository that could not be confirmed public is no longer resolved to the host's anonymous archive URL (e.g. `codeload.github.com`, which would fail to download for a private repository); it stays a regular `git` resolution so installs can use ambient Git credentials such as credential helpers and tokens. + +Note that a private repository that is reachable both over authenticated HTTPS and over SSH now resolves to its HTTPS URL, where previous versions recorded the SSH URL. + +Fixes [pnpm/pnpm#13276](https://github.com/pnpm/pnpm/issues/13276). + + diff --git a/pnpm11/resolving/git-resolver/src/parseBareSpecifier.ts b/pnpm11/resolving/git-resolver/src/parseBareSpecifier.ts index ba4e925650..a10280611e 100644 --- a/pnpm11/resolving/git-resolver/src/parseBareSpecifier.ts +++ b/pnpm11/resolving/git-resolver/src/parseBareSpecifier.ts @@ -70,53 +70,47 @@ function urlToFetchSpec (url: URL): string { async function fromHostedGit (hosted: any, dispatcherOptions: DispatcherOptions): Promise { // eslint-disable-line let fetchSpec: string | null = null - // try git/https url before fallback to ssh url - const gitHttpsUrl = hosted.https({ noCommittish: true, noGitPlus: true }) - if (gitHttpsUrl && await isRepoPublic(gitHttpsUrl, dispatcherOptions) && await accessRepository(gitHttpsUrl)) { - fetchSpec = gitHttpsUrl - } else { - const gitSshUrl = hosted.ssh({ noCommittish: true }) - if (gitSshUrl && await accessRepository(gitSshUrl)) { - fetchSpec = gitSshUrl - } + const httpsUrl: string | null = hosted.https({ noCommittish: true, noGitPlus: true }) + const sshUrl: string | null = hosted.ssh({ noCommittish: true }) + // SSH is probed before the HTTPS fallbacks (and used as the last-resort guess) + // only when the user explicitly wrote an SSH URL. For every other representation + // (`shortcut`, `https`, ...) an SSH remote was never asked for, and recording one + // in the lockfile breaks installs in environments without SSH keys, so every + // HTTPS transport is exhausted first. + const preferSsh = hosted.default === 'sshurl' + const repoIsPublic = httpsUrl != null && await isRepoPublic(httpsUrl, dispatcherOptions) + if (httpsUrl && repoIsPublic && await accessRepository(httpsUrl)) { + fetchSpec = httpsUrl } - - if (!fetchSpec) { - const httpsUrl: string | null = hosted.https({ noGitPlus: true, noCommittish: true }) - if (httpsUrl) { - if ((hosted.auth || !await isRepoPublic(httpsUrl, dispatcherOptions)) && await accessRepository(httpsUrl)) { - return { - fetchSpec: httpsUrl, - hosted: { - ...hosted, - _fill: hosted._fill, - tarball: undefined, - }, - normalizedBareSpecifier: `git+${httpsUrl}`, - ...parseGitParams(hosted.committish), - } - } else { - 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 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" - const response = await fetchWithDispatcher(httpsUrl.replace(/\.git$/, ''), { method: 'HEAD', redirect: 'manual', retry: { retries: 0 }, dispatcherOptions }) - if (response.ok) { - fetchSpec = httpsUrl - } - } catch { - // ignore - } + if (!fetchSpec && preferSsh && sshUrl && await accessRepository(sshUrl)) { + fetchSpec = sshUrl + } + if (!fetchSpec && httpsUrl) { + if ((hosted.auth || !repoIsPublic) && await accessRepository(httpsUrl)) { + // Reachable over HTTPS without being provably public, so resolve as + // `type: git` against this exact URL: the host's archive endpoint would + // carry neither the URL's credentials nor ambient ones (helpers, tokens). + return { + fetchSpec: httpsUrl, + hosted: { + ...hosted, + _fill: hosted._fill, + tarball: undefined, + }, + normalizedBareSpecifier: `git+${httpsUrl}`, + ...parseGitParams(hosted.committish), } } + if (repoIsPublic) { + fetchSpec = httpsUrl + } + } + if (!fetchSpec && !preferSsh && sshUrl && await accessRepository(sshUrl)) { + fetchSpec = sshUrl } if (!fetchSpec) { - // use ssh url for likely private repo - fetchSpec = hosted.sshurl({ noCommittish: true }) + fetchSpec = preferSsh ? hosted.sshurl({ noCommittish: true }) : httpsUrl } return { @@ -125,7 +119,10 @@ async function fromHostedGit (hosted: any, dispatcherOptions: DispatcherOptions) ...hosted, tarballtemplate: hosted.type === 'gitlab' ? gitlabTarballTemplate : hosted.tarballtemplate, _fill: hosted._fill, - tarball: hosted.tarball, + // Same rationale as the early return above: without proof that the repo + // is public, the host's anonymous archive endpoint cannot be assumed to + // work, so the resolution must stay `type: git`. + tarball: repoIsPublic ? hosted.tarball : undefined, }, normalizedBareSpecifier: hosted.shortcut(), ...parseGitParams(hosted.committish), @@ -140,9 +137,20 @@ function gitlabTarballTemplate ({ domain, user, project, committish }: { domain: return `https://${domain}/${user}/${project}/-/archive/${ref}/${project}-${ref}.tar.gz` } +// An HTTP HEAD on the project page (without ".git") instead of `git ls-remote`: +// probing a private repo with ls-remote would trigger a credential prompt. This is +// very similar to yarn classic's behavior; npm instead tries git ls-remote directly, +// which prompts for login credentials. Transient failures (429/5xx/network errors) +// are retried by the fetch layer so registry throttling of CI runners is not +// mistaken for a private repository. async function isRepoPublic (httpsUrl: string, dispatcherOptions: DispatcherOptions): Promise { try { - const response = await fetchWithDispatcher(httpsUrl.replace(/\.git$/, ''), { method: 'HEAD', redirect: 'manual', retry: { retries: 0 }, dispatcherOptions }) + const response = await fetchWithDispatcher(httpsUrl.replace(/\.git$/, ''), { + method: 'HEAD', + redirect: 'manual', + retry: { retries: 2, factor: 2, minTimeout: 500, maxTimeout: 2_000 }, + dispatcherOptions, + }) return response.ok } catch { return false diff --git a/pnpm11/resolving/git-resolver/test/index.ts b/pnpm11/resolving/git-resolver/test/index.ts index 2fcf47788f..7c7f610e46 100644 --- a/pnpm11/resolving/git-resolver/test/index.ts +++ b/pnpm11/resolving/git-resolver/test/index.ts @@ -515,11 +515,11 @@ test('resolveFromGit() private repo with commit hash', async () => { mockFetchAsPrivate() const resolveResult = await resolveFromGit({ bareSpecifier: 'fake/private-repo#2fa0531ab04e300a24ef4fd7fb3a280eccb7ccc5' }) expect(resolveResult).toStrictEqual({ - id: 'git+ssh://git@github.com/fake/private-repo.git#2fa0531ab04e300a24ef4fd7fb3a280eccb7ccc5', + id: 'git+https://github.com/fake/private-repo.git#2fa0531ab04e300a24ef4fd7fb3a280eccb7ccc5', normalizedBareSpecifier: 'github:fake/private-repo#2fa0531ab04e300a24ef4fd7fb3a280eccb7ccc5', resolution: { commit: '2fa0531ab04e300a24ef4fd7fb3a280eccb7ccc5', - repo: 'git+ssh://git@github.com/fake/private-repo.git', + repo: 'https://github.com/fake/private-repo.git', type: 'git', }, resolvedVia: 'git-repository', @@ -528,7 +528,11 @@ test('resolveFromGit() private repo with commit hash', async () => { test('resolve a private repository using the HTTPS protocol without auth token', async () => { jest.mocked(git).mockImplementation(async (args: string[]) => { - expect(args).toContain('git+ssh://git@github.com/foo/bar.git') + // Probes use --exit-code, resolution calls use --. Fail probes, succeed resolution. + if (args.includes('--exit-code')) { + throw new Error('access denied') + } + expect(args).toContain('https://github.com/foo/bar.git') return { stdout: '0'.repeat(40) + '\tHEAD', } @@ -536,11 +540,55 @@ test('resolve a private repository using the HTTPS protocol without auth token', mockFetchAsPrivate() const resolveResult = await resolveFromGit({ bareSpecifier: 'git+https://github.com/foo/bar.git' }) expect(resolveResult).toStrictEqual({ - id: 'git+ssh://git@github.com/foo/bar.git#0000000000000000000000000000000000000000', + id: 'git+https://github.com/foo/bar.git#0000000000000000000000000000000000000000', normalizedBareSpecifier: 'github:foo/bar', resolution: { commit: '0000000000000000000000000000000000000000', - repo: 'git+ssh://git@github.com/foo/bar.git', + repo: 'https://github.com/foo/bar.git', + type: 'git', + }, + resolvedVia: 'git-repository', + }) +}) + +test('resolve over HTTPS when the visibility probe fails but anonymous HTTPS git access works', async () => { + // A public repo whose HEAD probe is throttled (e.g. GitHub rate-limiting a CI + // runner) must still resolve over HTTPS: recording SSH would poison the + // lockfile for every environment without SSH keys. + mockFetchAsPrivate() + const gitCalls: string[][] = [] + jest.mocked(git).mockImplementation(async (args: string[]) => { + gitCalls.push(args) + if (args.some((arg) => arg.includes('git@'))) throw new Error('Permission denied (publickey)') + return { stdout: '0'.repeat(40) + '\tHEAD' } + }) + const resolveResult = await resolveFromGit({ bareSpecifier: 'foo/bar' }) + expect(resolveResult).toStrictEqual({ + id: `git+https://github.com/foo/bar.git#${'0'.repeat(40)}`, + normalizedBareSpecifier: 'git+https://github.com/foo/bar.git', + resolution: { + commit: '0'.repeat(40), + repo: 'https://github.com/foo/bar.git', + type: 'git', + }, + resolvedVia: 'git-repository', + }) + expect(gitCalls.flat().some((arg) => arg.includes('git@'))).toBe(false) +}) + +test('resolve an explicit SSH specifier over SSH when only SSH access works', async () => { + mockFetchAsPrivate() + jest.mocked(git).mockImplementation(async (args: string[]) => { + if (!args.includes('git@github.com:foo/bar.git')) throw new Error('access denied') + return { stdout: '0'.repeat(40) + '\tHEAD' } + }) + const resolveResult = await resolveFromGit({ bareSpecifier: 'git+ssh://git@github.com/foo/bar.git' }) + expect(resolveResult).toStrictEqual({ + id: `git+ssh://git@github.com/foo/bar.git#${'0'.repeat(40)}`, + normalizedBareSpecifier: 'github:foo/bar', + resolution: { + commit: '0'.repeat(40), + repo: 'git@github.com:foo/bar.git', type: 'git', }, resolvedVia: 'git-repository',