From 4b0e01f61335756d4984ced6c3af37157cd9232a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:18:56 -0700 Subject: [PATCH] fix(github): scope GHES host-auth cache to the executing connection (#14948) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(github): scope GHES host-auth cache to the executing connection The gh auth answer cached for a connection-backed repo is probed without the repository cwd, so it describes that connection's runtime — not the local host. Keying only on repoPath+wslDistro let a local repo and an SSH-hosted repo at the same path share one entry, and whichever resolved first decided "is this GHES host authenticated" for both. Include the connection identity in the runtime cache key. Local and WSL keys are unchanged. * fix(github): fence GHES auth cache across SSH reconnects * fix(github): fence origin cache across SSH reconnects * fix(github): fence repository identity cache on reconnect --- src/main/github/github-api-repository.test.ts | 79 +++++++++++- .../github-enterprise-repository.test.ts | 120 +++++++++++++++++- .../github/github-enterprise-repository.ts | 21 ++- 3 files changed, 208 insertions(+), 12 deletions(-) diff --git a/src/main/github/github-api-repository.test.ts b/src/main/github/github-api-repository.test.ts index bf1948588ed..fb6512dab30 100644 --- a/src/main/github/github-api-repository.test.ts +++ b/src/main/github/github-api-repository.test.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import type * as SshGitDispatch from '../providers/ssh-git-dispatch' import type * as GitHubEnterpriseRepository from './github-enterprise-repository' import type * as GhUtils from './gh-utils' -import type * as SshGitDispatch from '../providers/ssh-git-dispatch' const { getEnterpriseGitHubRepoSlugMock, @@ -268,4 +268,81 @@ describe('origin repository cache', () => { await expect(getOriginGitHubApiRepository('/repo')).resolves.toBeNull() expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(1) }) + + it('does not reuse a cached answer after the SSH provider reconnects', async () => { + const oldRepository = { + owner: 'old-owner', + repo: 'widgets', + host: 'github.acme-corp.com' + } + const newRepository = { + owner: 'new-owner', + repo: 'widgets', + host: 'github.acme-corp.com' + } + getSshGitProviderGenerationMock.mockReturnValue(1) + getEnterpriseGitHubRepoSlugMock + .mockResolvedValueOnce(oldRepository) + .mockResolvedValueOnce(newRepository) + + await expect(getOriginGitHubApiRepository('/repo', 'ssh-1')).resolves.toEqual(oldRepository) + await expect(getOriginGitHubApiRepository('/repo', 'ssh-1')).resolves.toEqual(oldRepository) + expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(1) + + getSshGitProviderGenerationMock.mockReturnValue(2) + await expect(getOriginGitHubApiRepository('/repo', 'ssh-1')).resolves.toEqual(newRepository) + expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(2) + }) + + it('does not reuse a cached negative after the SSH provider reconnects', async () => { + const repository = { + owner: 'acme', + repo: 'widgets', + host: 'github.acme-corp.com' + } + getSshGitProviderGenerationMock.mockReturnValue(1) + getEnterpriseGitHubRepoSlugMock.mockResolvedValueOnce(null).mockResolvedValueOnce(repository) + + await expect(getOriginGitHubApiRepository('/repo', 'ssh-1')).resolves.toBeNull() + await expect(getOriginGitHubApiRepository('/repo', 'ssh-1')).resolves.toBeNull() + expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(1) + + getSshGitProviderGenerationMock.mockReturnValue(2) + await expect(getOriginGitHubApiRepository('/repo', 'ssh-1')).resolves.toEqual(repository) + expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(2) + }) + + it('isolates an in-flight probe from an older SSH provider generation', async () => { + const oldRepository = { + owner: 'old-owner', + repo: 'widgets', + host: 'github.acme-corp.com' + } + const newRepository = { + owner: 'new-owner', + repo: 'widgets', + host: 'github.acme-corp.com' + } + let resolveOldProbe: (repository: typeof oldRepository) => void = () => {} + const oldProbeResult = new Promise((resolve) => { + resolveOldProbe = resolve + }) + getEnterpriseGitHubRepoSlugMock + .mockImplementationOnce(() => oldProbeResult) + .mockResolvedValueOnce(newRepository) + getSshGitProviderGenerationMock.mockReturnValue(1) + + const oldProbe = getOriginGitHubApiRepository('/repo', 'ssh-1') + await vi.waitFor(() => expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(1)) + + getSshGitProviderGenerationMock.mockReturnValue(2) + const newProbe = getOriginGitHubApiRepository('/repo', 'ssh-1') + await vi.waitFor(() => expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(2)) + await expect(newProbe).resolves.toEqual(newRepository) + + resolveOldProbe(oldRepository) + await expect(oldProbe).resolves.toEqual(oldRepository) + await expect(getOriginGitHubApiRepository('/repo', 'ssh-1')).resolves.toEqual(newRepository) + expect(getEnterpriseGitHubRepoSlugMock).toHaveBeenCalledTimes(2) + }) }) diff --git a/src/main/github/github-enterprise-repository.test.ts b/src/main/github/github-enterprise-repository.test.ts index caee8e0bccf..dac93c146e0 100644 --- a/src/main/github/github-enterprise-repository.test.ts +++ b/src/main/github/github-enterprise-repository.test.ts @@ -27,6 +27,7 @@ import { isGitHubHostAuthenticatedForGlobalCli } from './github-enterprise-repository' import { _resetSshHostnameResolutionCache } from './github-ssh-host-alias-resolution' +import { registerSshGitProvider, unregisterSshGitProvider } from '../providers/ssh-git-dispatch' function mockOriginRemote(url: string): void { gitExecFileAsyncMock.mockImplementation(async (args: string[]) => { @@ -392,15 +393,122 @@ describe('isGitHubHostAuthenticated', () => { } }) - it('shares the native gh auth probe across SSH connections', async () => { - mockHostAuthenticated() + it('does not share a cached answer between SSH connections at the same path', async () => { + ghExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: + 'github.acme-corp.com\n ✓ Logged in to github.acme-corp.com account kelora (keyring)', + stderr: '' + }) + .mockResolvedValueOnce({ + stdout: 'github.com\n ✓ Logged in to github.com account kelora (keyring)', + stderr: '' + }) - await isGitHubHostAuthenticated('github.acme-corp.com', '/remote/a', 'ssh-1') - await isGitHubHostAuthenticated('github.acme-corp.com', '/remote/b', 'ssh-2') - - expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + await expect(isGitHubHostAuthenticated('github.acme-corp.com', '/repo', 'ssh-1')).resolves.toBe( + true + ) + await expect(isGitHubHostAuthenticated('github.acme-corp.com', '/repo', 'ssh-2')).resolves.toBe( + false + ) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2) }) + it('does not reuse a cached answer after the SSH provider reconnects', async () => { + const connectionId = 'ssh-reconnected-auth-cache' + ghExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: + 'github.acme-corp.com\n ✓ Logged in to github.acme-corp.com account kelora (keyring)', + stderr: '' + }) + .mockResolvedValueOnce({ + stdout: 'github.com\n ✓ Logged in to github.com account kelora (keyring)', + stderr: '' + }) + + registerSshGitProvider(connectionId, {} as never) + try { + await expect( + isGitHubHostAuthenticated('github.acme-corp.com', '/repo', connectionId) + ).resolves.toBe(true) + await expect( + isGitHubHostAuthenticated('github.acme-corp.com', '/repo', connectionId) + ).resolves.toBe(true) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + + registerSshGitProvider(connectionId, {} as never) + await expect( + isGitHubHostAuthenticated('github.acme-corp.com', '/repo', connectionId) + ).resolves.toBe(false) + } finally { + unregisterSshGitProvider(connectionId) + } + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2) + }) + + it('does not join an in-flight auth probe from an older SSH provider generation', async () => { + const connectionId = 'ssh-reconnected-auth-in-flight' + let finishOldProbe: (() => void) | undefined + ghExecFileAsyncMock + .mockImplementationOnce( + () => + new Promise((resolve) => { + finishOldProbe = () => + resolve({ + stdout: + 'github.acme-corp.com\n ✓ Logged in to github.acme-corp.com account kelora (keyring)', + stderr: '' + }) + }) + ) + .mockResolvedValueOnce({ + stdout: 'github.com\n ✓ Logged in to github.com account kelora (keyring)', + stderr: '' + }) + + registerSshGitProvider(connectionId, {} as never) + try { + const oldProbe = isGitHubHostAuthenticated('github.acme-corp.com', '/repo', connectionId) + registerSshGitProvider(connectionId, {} as never) + const newProbe = isGitHubHostAuthenticated('github.acme-corp.com', '/repo', connectionId) + + await expect(newProbe).resolves.toBe(false) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2) + finishOldProbe?.() + await expect(oldProbe).resolves.toBe(true) + } finally { + unregisterSshGitProvider(connectionId) + } + }) + + // The probe answers differ by runtime: a local repo gets a cwd, a + // connection-backed repo does not. Sharing one entry attributes whichever + // resolved first to both hosts. + it.each([ + ['local resolves first', [null, 'ssh-1'] as const, [true, false]], + ['connection resolves first', ['ssh-1', null] as const, [false, true]] + ])( + 'does not cross-attribute auth between a local and a connection-backed repo at the same path (%s)', + async (_label, connectionIds, expected) => { + ghExecFileAsyncMock.mockImplementation( + async (_args: string[], options: { cwd?: string }) => ({ + stdout: options.cwd + ? 'github.acme-corp.com\n ✓ Logged in to github.acme-corp.com account kelora (keyring)' + : 'github.com\n ✓ Logged in to github.com account kelora (keyring)', + stderr: '' + }) + ) + + for (const [index, connectionId] of connectionIds.entries()) { + await expect( + isGitHubHostAuthenticated('github.acme-corp.com', '/repo', connectionId) + ).resolves.toBe(expected[index]) + } + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2) + } + ) + it('does not target an unconfigured remote host with ambient credentials', async () => { mockHostAuthenticated('github.acme-corp.com') diff --git a/src/main/github/github-enterprise-repository.ts b/src/main/github/github-enterprise-repository.ts index 0d244039145..d32dfadb3b9 100644 --- a/src/main/github/github-enterprise-repository.ts +++ b/src/main/github/github-enterprise-repository.ts @@ -20,7 +20,8 @@ import { resolveSshConfigHostname } from './github-ssh-host-alias-resolution' import { parseWslPath } from '../wsl' import { getSshGitProvider, - SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE + SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE, + getSshGitProviderGeneration } from '../providers/ssh-git-dispatch' import { isStableMissingGitRemoteError } from '../git/stable-missing-git-remote-error' @@ -41,9 +42,19 @@ type HostAuthCacheEntry = { const hostAuthCache = new Map() const hostAuthInFlight = new Map>() -// Why: connection-backed Git operations execute remotely, but gh intentionally -// executes on the native host. Only WSL selects a distinct gh config/runtime. -function runtimeCacheKey(repoPath: string, wslDistro?: string): string { +// Why: gh intentionally executes on the native host even for connection-backed +// repos, but the answer still describes that connection's runtime (it is probed +// without the repository cwd). Keying it per connection stops a local repo and +// an SSH-hosted repo at the same path from adopting each other's auth state. +// Provider generation fences stale answers when reconnect replaces the runtime. +function runtimeCacheKey( + repoPath: string, + connectionId?: string | null, + wslDistro?: string +): string { + if (connectionId) { + return `connection:${connectionId}:${getSshGitProviderGeneration(connectionId)}` + } const resolvedDistro = wslDistro ?? parseWslPath(repoPath)?.distro return `local:${resolvedDistro?.toLowerCase() ?? 'host'}` } @@ -134,7 +145,7 @@ async function resolveAuthenticatedGitHubHost( localGitOptions: LocalGitExecOptions = {} ): Promise { const normalizedHost = normalizeGitHubHost(host)?.authority ?? host.trim().toLowerCase() - const cacheKey = `${runtimeCacheKey(repoPath, localGitOptions.wslDistro)}\0${normalizedHost}` + const cacheKey = `${runtimeCacheKey(repoPath, connectionId, localGitOptions.wslDistro)}\0${normalizedHost}` const now = Date.now() pruneHostAuthCache(now) const cached = hostAuthCache.get(cacheKey)