mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 16:03:17 +00:00
fix(github): preserve GHES non-default auth ports
This commit is contained in:
@@ -21,10 +21,10 @@ describe('applyGhHostToArgs', () => {
|
||||
'github.com',
|
||||
'rate_limit'
|
||||
])
|
||||
expect(applyGhHostToArgs(['api', 'repos/a/b/pulls'], 'github.acme-corp.com')).toEqual([
|
||||
expect(applyGhHostToArgs(['api', 'repos/a/b/pulls'], 'github.acme-corp.com:8443')).toEqual([
|
||||
'api',
|
||||
'--hostname',
|
||||
'github.acme-corp.com',
|
||||
'github.acme-corp.com:8443',
|
||||
'repos/a/b/pulls'
|
||||
])
|
||||
})
|
||||
|
||||
@@ -363,11 +363,11 @@ describe('GitHub PR local runtime routing', () => {
|
||||
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('host-qualifies SSH-backed GitHub Enterprise review reads and mutations', async () => {
|
||||
it('preserves a ported GHES host in SSH-backed review reads and mutations', async () => {
|
||||
const enterpriseRepo = {
|
||||
owner: 'team',
|
||||
repo: 'orca',
|
||||
host: 'github.acme-corp.com'
|
||||
host: 'github.acme-corp.com:8443'
|
||||
}
|
||||
getOwnerRepoMock.mockResolvedValue(null)
|
||||
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(enterpriseRepo)
|
||||
@@ -392,7 +392,7 @@ describe('GitHub PR local runtime routing', () => {
|
||||
number: 7,
|
||||
title: 'Enterprise PR',
|
||||
state: 'OPEN',
|
||||
url: 'https://github.acme-corp.com/team/orca/pull/7',
|
||||
url: 'https://github.acme-corp.com:8443/team/orca/pull/7',
|
||||
labels: [],
|
||||
updatedAt: '2026-07-16T00:00:00Z',
|
||||
author: { login: 'pr-author' },
|
||||
@@ -430,7 +430,7 @@ describe('GitHub PR local runtime routing', () => {
|
||||
name: 'lint',
|
||||
status: 'completed',
|
||||
conclusion: 'failure',
|
||||
details_url: 'https://github.acme-corp.com/team/orca/actions/runs/77/job/88'
|
||||
details_url: 'https://github.acme-corp.com:8443/team/orca/actions/runs/77/job/88'
|
||||
}
|
||||
]
|
||||
})
|
||||
@@ -449,7 +449,7 @@ describe('GitHub PR local runtime routing', () => {
|
||||
name: 'lint',
|
||||
status: 'completed',
|
||||
conclusion: 'failure',
|
||||
details_url: 'https://github.acme-corp.com/team/orca/actions/runs/77/job/88',
|
||||
details_url: 'https://github.acme-corp.com:8443/team/orca/actions/runs/77/job/88',
|
||||
output: { title: 'Lint failed', summary: 'One error' }
|
||||
})
|
||||
}
|
||||
@@ -557,7 +557,7 @@ describe('GitHub PR local runtime routing', () => {
|
||||
// The runner host-qualifies argv at spawn time from options.host, so the
|
||||
// mocked call sees the unqualified --repo plus the host in exec options.
|
||||
expect(prViewCall?.[0]).toEqual(expect.arrayContaining(['--repo', 'team/orca']))
|
||||
expect(prViewCall?.[1]).toEqual({ host: 'github.acme-corp.com' })
|
||||
expect(prViewCall?.[1]).toEqual({ host: 'github.acme-corp.com:8443' })
|
||||
const prCalls = ghExecFileAsyncMock.mock.calls.filter(([args]) => args[0] === 'pr')
|
||||
expect(
|
||||
prCalls.every(
|
||||
@@ -570,7 +570,7 @@ describe('GitHub PR local runtime routing', () => {
|
||||
apiCalls.every(
|
||||
([args, options]) =>
|
||||
!args.includes('--hostname') &&
|
||||
options.host === 'github.acme-corp.com' &&
|
||||
options.host === 'github.acme-corp.com:8443' &&
|
||||
options.cwd === undefined &&
|
||||
options.wslDistro === undefined
|
||||
)
|
||||
|
||||
@@ -61,7 +61,7 @@ describe('githubHostExecOptions', () => {
|
||||
|
||||
describe('resolveGitHubRepoExecution', () => {
|
||||
it('combines local repository and GitHub host execution options', async () => {
|
||||
const ownerRepo = { owner: 'acme', repo: 'widgets', host: 'github.acme-corp.com' }
|
||||
const ownerRepo = { owner: 'acme', repo: 'widgets', host: 'github.acme-corp.com:8443' }
|
||||
isGitHubHostAuthenticatedMock.mockResolvedValue(true)
|
||||
|
||||
await expect(
|
||||
@@ -71,11 +71,11 @@ describe('resolveGitHubRepoExecution', () => {
|
||||
ghOptions: {
|
||||
cwd: '/repo',
|
||||
wslDistro: 'Ubuntu',
|
||||
host: 'github.acme-corp.com'
|
||||
host: 'github.acme-corp.com:8443'
|
||||
}
|
||||
})
|
||||
expect(isGitHubHostAuthenticatedMock).toHaveBeenCalledWith(
|
||||
'github.acme-corp.com',
|
||||
'github.acme-corp.com:8443',
|
||||
'/repo',
|
||||
null,
|
||||
{ wslDistro: 'Ubuntu' }
|
||||
|
||||
@@ -28,10 +28,16 @@ function mockOriginRemote(url: string): void {
|
||||
})
|
||||
}
|
||||
|
||||
// gh exit 0 for `auth status --hostname <host>` means logged in to that host.
|
||||
// gh auth status inventory entries represent hosts with configured credentials.
|
||||
function mockHostAuthenticated(host = 'github.acme-corp.com'): void {
|
||||
mockAuthenticatedHosts([host])
|
||||
}
|
||||
|
||||
function mockAuthenticatedHosts(hosts: string[]): void {
|
||||
ghExecFileAsyncMock.mockResolvedValue({
|
||||
stdout: `${host}\n ✓ Logged in to ${host} account kelora (keyring)`,
|
||||
stdout: hosts
|
||||
.map((host) => `${host}\n ✓ Logged in to ${host} account kelora (keyring)`)
|
||||
.join('\n'),
|
||||
stderr: ''
|
||||
})
|
||||
}
|
||||
@@ -89,14 +95,67 @@ describe('getEnterpriseGitHubRepoSlug', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('uses the auth inventory host when an HTTPS remote includes a non-default port', async () => {
|
||||
mockOriginRemote('https://ghe.acme.com:8443/team/orca.git')
|
||||
it.each([
|
||||
[['ghe.acme.com', 'ghe.acme.com:9443', 'ghe.acme.com:8443']],
|
||||
[['ghe.acme.com:8443', 'ghe.acme.com:9443', 'ghe.acme.com']]
|
||||
])(
|
||||
'requires exact host:port authentication regardless of auth inventory order: %j',
|
||||
async (authenticatedHosts) => {
|
||||
mockOriginRemote('https://ghe.acme.com:8443/team/orca.git')
|
||||
mockAuthenticatedHosts(authenticatedHosts)
|
||||
|
||||
await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toEqual({
|
||||
owner: 'team',
|
||||
repo: 'orca',
|
||||
host: 'ghe.acme.com:8443'
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
it.each([
|
||||
'https://ghe.acme.com:8443/team/orca.git',
|
||||
'https://ghe.acme.com:80/team/orca.git',
|
||||
'http://ghe.acme.com:443/team/orca.git'
|
||||
])('rejects portless authentication for a non-default web endpoint: %s', async (remoteUrl) => {
|
||||
mockOriginRemote(remoteUrl)
|
||||
mockHostAuthenticated('ghe.acme.com')
|
||||
|
||||
await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toBeNull()
|
||||
})
|
||||
|
||||
it.each(['http://ghe.acme.com:80/team/orca.git', 'https://ghe.acme.com:443/team/orca.git'])(
|
||||
'matches a protocol-default port to the portless auth host: %s',
|
||||
async (remoteUrl) => {
|
||||
mockOriginRemote(remoteUrl)
|
||||
mockHostAuthenticated('ghe.acme.com')
|
||||
|
||||
await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toEqual({
|
||||
owner: 'team',
|
||||
repo: 'orca',
|
||||
host: 'ghe.acme.com'
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
it('preserves exact authentication for an HTTP endpoint on port 443', async () => {
|
||||
mockOriginRemote('http://ghe.acme.com:443/team/orca.git')
|
||||
mockHostAuthenticated('ghe.acme.com:443')
|
||||
|
||||
await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toEqual({
|
||||
owner: 'team',
|
||||
repo: 'orca',
|
||||
host: 'ghe.acme.com'
|
||||
host: 'ghe.acme.com:443'
|
||||
})
|
||||
})
|
||||
|
||||
it('preserves exact authentication for an HTTPS endpoint on port 80', async () => {
|
||||
mockOriginRemote('https://ghe.acme.com:80/team/orca.git')
|
||||
mockHostAuthenticated('ghe.acme.com:80')
|
||||
|
||||
await expect(getEnterpriseGitHubRepoSlug('/repo')).resolves.toEqual({
|
||||
owner: 'team',
|
||||
repo: 'orca',
|
||||
host: 'ghe.acme.com:80'
|
||||
})
|
||||
})
|
||||
|
||||
@@ -264,17 +323,38 @@ describe('isGitHubHostAuthenticated', () => {
|
||||
expect(ghExecFileAsyncMock.mock.calls.flat()).not.toContain('evil.example.test')
|
||||
})
|
||||
|
||||
it('does not guess between multiple ported auth hosts for one SSH hostname', async () => {
|
||||
ghExecFileAsyncMock.mockResolvedValue({
|
||||
stdout: `ghe.acme.com:8443
|
||||
✓ Logged in to ghe.acme.com:8443 account kelora (keyring)
|
||||
ghe.acme.com:9443
|
||||
✓ Logged in to ghe.acme.com:9443 account other (keyring)`,
|
||||
stderr: ''
|
||||
})
|
||||
it.each([
|
||||
[['ghe.acme.com:8443', 'ghe.acme.com:9443']],
|
||||
[['ghe.acme.com:9443', 'ghe.acme.com:8443']]
|
||||
])(
|
||||
'does not guess between multiple ported auth hosts for one SSH hostname: %j',
|
||||
async (authenticatedHosts) => {
|
||||
mockAuthenticatedHosts(authenticatedHosts)
|
||||
|
||||
await expect(isGitHubHostAuthenticated('ghe.acme.com', '/repo')).resolves.toBe(false)
|
||||
})
|
||||
await expect(isGitHubHostAuthenticated('ghe.acme.com', '/repo')).resolves.toBe(false)
|
||||
}
|
||||
)
|
||||
|
||||
it.each([
|
||||
[
|
||||
['ghe.acme.com', false],
|
||||
['ghe.acme.com:8443', true]
|
||||
],
|
||||
[
|
||||
['ghe.acme.com:8443', true],
|
||||
['ghe.acme.com', false]
|
||||
]
|
||||
] as const)(
|
||||
'keeps exact-port and ambiguous hostname auth cached independently: %j',
|
||||
async (...requests) => {
|
||||
mockAuthenticatedHosts(['ghe.acme.com:8443', 'ghe.acme.com:9443'])
|
||||
|
||||
for (const [host, expected] of requests) {
|
||||
await expect(isGitHubHostAuthenticated(host, '/repo')).resolves.toBe(expected)
|
||||
}
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
}
|
||||
)
|
||||
|
||||
it('treats default web ports as the same auth host', async () => {
|
||||
mockHostAuthenticated('ghe.acme.com:443')
|
||||
|
||||
@@ -90,10 +90,8 @@ function normalizeGitHubHost(host: string): NormalizedGitHubHost | null {
|
||||
return null
|
||||
}
|
||||
const hostname = match[1]
|
||||
const rawPort = match[2] ?? null
|
||||
// Why: default web ports do not distinguish a gh auth host from the same
|
||||
// hostname without an explicit port.
|
||||
const port = rawPort === '80' || rawPort === '443' ? null : rawPort
|
||||
// Why: remote URL parsing already removes protocol-default ports; any port left here identifies the endpoint.
|
||||
const port = match[2] ?? null
|
||||
return { hostname, port, authority: port ? `${hostname}:${port}` : hostname }
|
||||
}
|
||||
|
||||
@@ -114,10 +112,11 @@ function authenticatedHostFromInventory(host: string, output: string): string |
|
||||
if (exact) {
|
||||
return exact.authority
|
||||
}
|
||||
const compatible = inventory.filter(
|
||||
(candidate) =>
|
||||
candidate.hostname === requested.hostname && (!requested.port || candidate.port === null)
|
||||
)
|
||||
// Why: a non-default web port identifies the API endpoint; portless credentials target a different server.
|
||||
if (requested.port) {
|
||||
return null
|
||||
}
|
||||
const compatible = inventory.filter((candidate) => candidate.hostname === requested.hostname)
|
||||
// Why: an SSH remote has no API port. Only a unique auth-inventory host can
|
||||
// safely supply it; multiple ported endpoints on one hostname are ambiguous.
|
||||
return compatible.length === 1 ? compatible[0].authority : null
|
||||
|
||||
Reference in New Issue
Block a user