From 14ef70f3f71651978f1a290fa840419e1d78073b Mon Sep 17 00:00:00 2001 From: "kiloconnect[bot]" <240665456+kiloconnect[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 22:10:52 +0000 Subject: [PATCH] fix(github): complete install when app JWT getInstallation 404s Fixes KILOCODE-WEB-27TD --- .../github/callback/route.test.ts | 112 +++++++++-- .../api/integrations/github/callback/route.ts | 182 +++++++++++------- .../platforms/github/app-selector.test.ts | 42 ++++ .../platforms/github/app-selector.ts | 35 +++- 4 files changed, 271 insertions(+), 100 deletions(-) diff --git a/apps/web/src/app/api/integrations/github/callback/route.test.ts b/apps/web/src/app/api/integrations/github/callback/route.test.ts index e03f0cb94b..09b205d556 100644 --- a/apps/web/src/app/api/integrations/github/callback/route.test.ts +++ b/apps/web/src/app/api/integrations/github/callback/route.test.ts @@ -14,7 +14,7 @@ import { upsertPlatformIntegrationForOwner, } from '@/lib/integrations/db/platform-integrations'; import { isOrganizationMember } from '@/lib/organizations/organizations'; -import { assertUserAdministersInstallation } from '@/lib/integrations/platforms/github/app-selector'; +import { findAdministeredInstallation } from '@/lib/integrations/platforms/github/app-selector'; import { captureException, captureMessage } from '@sentry/nextjs'; import type { StateAdapter } from 'chat'; import { ensureOrganizationAccess } from '@/routers/organizations/utils'; @@ -45,6 +45,7 @@ jest.mock('@octokit/rest', () => ({ apps: { getInstallation: jest.fn(), listReposAccessibleToInstallation: jest.fn(), + listInstallationReposForAuthenticatedUser: jest.fn(), }, })), })); @@ -60,7 +61,14 @@ jest.mock('@/lib/integrations/platforms/github/app-selector', () => ({ appName: 'KiloConnect', webhookSecret: 'webhook-secret', })), - assertUserAdministersInstallation: jest.fn(async () => true), + findAdministeredInstallation: jest.fn(async () => ({ + id: 98765, + account: { id: 12_345, login: 'securexg' }, + created_at: '2026-07-09T19:00:00.000Z', + events: ['issues'], + permissions: { contents: 'write' }, + repository_selection: 'all', + })), })); jest.mock('@/routers/organizations/utils', () => ({ ensureOrganizationAccess: jest.fn(), @@ -93,7 +101,7 @@ const mockedOctokit = jest.mocked(Octokit); const mockedUpsertPlatformIntegrationForOwner = jest.mocked(upsertPlatformIntegrationForOwner); const mockedIsOrganizationMember = jest.mocked(isOrganizationMember); const mockedConsumeInstallState = jest.mocked(consumeInstallState); -const mockedAssertUserAdministersInstallation = jest.mocked(assertUserAdministersInstallation); +const mockedFindAdministeredInstallation = jest.mocked(findAdministeredInstallation); const mockedCaptureException = jest.mocked(captureException); const mockedCaptureMessage = jest.mocked(captureMessage); const mockedEnsureOrganizationAccess = jest.mocked(ensureOrganizationAccess); @@ -103,6 +111,14 @@ const OTHER_USER_ID = 'c00b91a1-6959-4b04-9ef8-e8d37b340f4a'; const GITHUB_USER_ID = '12345'; const INSTALLATION_ID = '98765'; const INSTALL_STATE_TOKEN = 'valid-database-token-for-callback-tests'; +const ADMINISTERED_INSTALLATION = { + id: 98765, + account: { id: 12_345, login: 'securexg' }, + created_at: '2026-07-09T19:00:00.000Z', + events: ['issues'], + permissions: { contents: 'write' }, + repository_selection: 'all', +}; beforeEach(() => { mockedExchangeGitHubOAuthCode.mockResolvedValue({ @@ -110,7 +126,7 @@ beforeEach(() => { login: 'octocat', accessToken: 'ghu_test-token', }); - mockedAssertUserAdministersInstallation.mockResolvedValue(true); + mockedFindAdministeredInstallation.mockResolvedValue(ADMINISTERED_INSTALLATION as never); }); function makeRequest(pathWithQuery: string) { @@ -733,7 +749,8 @@ describe('GET /api/integrations/github/callback database-backed install flow', ( expect(mockedUpsertPlatformIntegrationForOwner).not.toHaveBeenCalled(); }); - test('app-initiated installation_not_found redirects to /github-app fallback', async () => { + test('app-initiated install uses the user-token installation when app JWT getInstallation would 404', async () => { + mockedUpsertPlatformIntegrationForOwner.mockResolvedValue({ ok: true }); mockedConsumeInstallState.mockResolvedValue({ token: DB_TOKEN, kilo_user_id: USER_ID, @@ -765,9 +782,70 @@ describe('GET /api/integrations/github/callback database-backed install flow', ( ) as never ); + expect(response.status).toBe(307); + expectRedirectLocation(response, '/github-app?fromApp=1&github_install=success'); + expect(mockedFindAdministeredInstallation).toHaveBeenCalledWith({ + accessToken: 'ghu_test-token', + installationId: INSTALLATION_ID, + }); + expect(mockedUpsertPlatformIntegrationForOwner).toHaveBeenCalledWith( + { type: 'user', id: USER_ID }, + expect.objectContaining({ + platformInstallationId: INSTALLATION_ID, + platformAccountLogin: 'securexg', + }) + ); + expect(mockedCaptureException).not.toHaveBeenCalled(); + }); + + test('app-initiated installation_not_found redirects to /github-app fallback', async () => { + mockedConsumeInstallState.mockResolvedValue({ + token: DB_TOKEN, + kilo_user_id: USER_ID, + owner_type: 'user', + owner_id: USER_ID, + github_app_type: 'standard', + return_to: '/cloud/sessions', + expires_at: new Date(Date.now() + 300_000).toISOString(), + consumed_at: null, + created_at: new Date().toISOString(), + }); + mockedOctokit.mockImplementation( + () => + ({ + apps: { + getInstallation: jest.fn(async () => { + const err = Object.assign(new Error('Not Found'), { status: 404 }); + throw err; + }), + listReposAccessibleToInstallation: jest.fn(), + }, + }) as never + ); + + const { GET } = await import('./route'); + const response = await GET( + makeRequest( + `/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&state=${DB_TOKEN}` + ) as never + ); + expect(response.status).toBe(307); expectRedirectLocation(response, '/github-app?fromApp=1&error=installation_not_found'); expect(mockedUpsertPlatformIntegrationForOwner).not.toHaveBeenCalled(); + expect(mockedCaptureException).not.toHaveBeenCalled(); + expect(mockedCaptureMessage).toHaveBeenCalledWith( + 'GitHub installation not found for authenticated app', + expect.objectContaining({ + level: 'warning', + extra: expect.objectContaining({ + installationId: INSTALLATION_ID, + githubAppType: 'standard', + }), + }) + ); + const serializedMessage = JSON.stringify(mockedCaptureMessage.mock.calls); + expect(serializedMessage).not.toContain(USER_ID); }); test('ambiguous app-initiated pending request returns successful pending no-op', async () => { @@ -972,7 +1050,7 @@ describe('GET /api/integrations/github/callback admin proof', () => { }) as never ); mockedUpsertPlatformIntegrationForOwner.mockResolvedValue({ ok: true }); - mockedAssertUserAdministersInstallation.mockResolvedValue(true); + mockedFindAdministeredInstallation.mockResolvedValue(ADMINISTERED_INSTALLATION as never); mockedExchangeGitHubOAuthCode.mockResolvedValue({ id: GITHUB_USER_ID, login: 'octocat', @@ -992,7 +1070,7 @@ describe('GET /api/integrations/github/callback admin proof', () => { expectRedirectLocation(response, `/integrations/github?error=not_installation_admin`); expect(mockedUpsertPlatformIntegrationForOwner).not.toHaveBeenCalled(); expect(mockedExchangeGitHubOAuthCode).not.toHaveBeenCalled(); - expect(mockedAssertUserAdministersInstallation).not.toHaveBeenCalled(); + expect(mockedFindAdministeredInstallation).not.toHaveBeenCalled(); expect(mockedCreateAppAuth).not.toHaveBeenCalled(); }); @@ -1007,7 +1085,7 @@ describe('GET /api/integrations/github/callback admin proof', () => { expect(response.status).toBe(307); expectRedirectLocation(response, `/integrations/github?success=installed`); expect(mockedExchangeGitHubOAuthCode).toHaveBeenCalledWith('abc', 'standard'); - expect(mockedAssertUserAdministersInstallation).toHaveBeenCalledWith({ + expect(mockedFindAdministeredInstallation).toHaveBeenCalledWith({ accessToken: 'ghu_test-token', installationId: INSTALLATION_ID, }); @@ -1015,7 +1093,7 @@ describe('GET /api/integrations/github/callback admin proof', () => { }); test('rejects an install when admin check returns false', async () => { - mockedAssertUserAdministersInstallation.mockResolvedValue(false); + mockedFindAdministeredInstallation.mockResolvedValue(null); const { GET } = await import('./route'); const response = await GET( @@ -1027,7 +1105,7 @@ describe('GET /api/integrations/github/callback admin proof', () => { expect(response.status).toBe(307); expectRedirectLocation(response, `/integrations/github?error=not_installation_admin`); expect(mockedExchangeGitHubOAuthCode).toHaveBeenCalled(); - expect(mockedAssertUserAdministersInstallation).toHaveBeenCalled(); + expect(mockedFindAdministeredInstallation).toHaveBeenCalled(); expect(mockedUpsertPlatformIntegrationForOwner).not.toHaveBeenCalled(); expect(mockedCreateAppAuth).not.toHaveBeenCalled(); }); @@ -1119,7 +1197,7 @@ describe('GET /api/integrations/github/callback admin proof', () => { logSpy.mockClear(); // Case 2: code present but non-admin — should log fail_non_admin. - mockedAssertUserAdministersInstallation.mockResolvedValue(false); + mockedFindAdministeredInstallation.mockResolvedValue(null); await GET( makeRequest( `/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${INSTALL_STATE_TOKEN}&code=abc` @@ -1211,16 +1289,8 @@ describe('GET /api/integrations/github/callback Sentry redaction', () => { consumed_at: null, created_at: new Date().toISOString(), }); - mockedOctokit.mockImplementation( - () => - ({ - apps: { - getInstallation: jest.fn(async () => { - throw new Error('get installation failed'); - }), - listReposAccessibleToInstallation: jest.fn(), - }, - }) as never + mockedUpsertPlatformIntegrationForOwner.mockRejectedValue( + new Error('persist installation failed') ); const { GET } = await import('./route'); diff --git a/apps/web/src/app/api/integrations/github/callback/route.ts b/apps/web/src/app/api/integrations/github/callback/route.ts index 250351db00..735f194862 100644 --- a/apps/web/src/app/api/integrations/github/callback/route.ts +++ b/apps/web/src/app/api/integrations/github/callback/route.ts @@ -10,7 +10,7 @@ import { } from '@/lib/integrations/platforms/github/adapter'; import { getGitHubAppCredentials, - assertUserAdministersInstallation, + findAdministeredInstallation, type GitHubAppType, } from '@/lib/integrations/platforms/github/app-selector'; import { ensureOrganizationAccess } from '@/routers/organizations/utils'; @@ -467,7 +467,12 @@ async function handleCoreInstallFlow(params: { } // Require proof that the OAuth-authorized GitHub user administers the - // installation before using app credentials to fetch or persist it. + // installation before persisting it. The user-token installation list is the + // source of truth here: app-JWT GET /app/installations/{id} can 404 for a + // just-created installation (GitHub replication lag) even after the user + // token already sees it. + let installation; + let userAccessToken: string | undefined; if (setupAction === 'install' || setupAction === 'update') { const code = searchParams.get('code'); const rejectUnauthorizedInstallation = () => @@ -490,12 +495,12 @@ async function handleCoreInstallFlow(params: { try { const exchangeResult = await exchangeGitHubOAuthCode(code, githubAppType); - const isAdmin = await assertUserAdministersInstallation({ + const administeredInstallation = await findAdministeredInstallation({ accessToken: exchangeResult.accessToken, installationId, }); - if (!isAdmin) { + if (!administeredInstallation) { console.log('[github_admin_proof:fail_non_admin]', { github_user_id: exchangeResult.id, github_user_login: exchangeResult.login, @@ -509,6 +514,8 @@ async function handleCoreInstallFlow(params: { github_user_login: exchangeResult.login, installation_id: installationId, }); + installation = administeredInstallation; + userAccessToken = exchangeResult.accessToken; } catch (error) { console.error('[github_admin_proof:error]', { installation_id: installationId, @@ -523,89 +530,126 @@ async function handleCoreInstallFlow(params: { }); return rejectUnauthorizedInstallation(); } - } + } else { + const auth = createAppAuth({ + appId: credentials.appId, + privateKey: credentials.privateKey, + }); - // Fetch installation details from GitHub - const auth = createAppAuth({ - appId: credentials.appId, - privateKey: credentials.privateKey, - }); + const appAuth = await auth({ type: 'app' }); + const octokitApp = new Octokit({ + auth: appAuth.token, + }); - const appAuth = await auth({ type: 'app' }); - const octokitApp = new Octokit({ - auth: appAuth.token, - }); + try { + console.log('Fetching installation details for ID:', installationId); + const result = await octokitApp.apps.getInstallation({ + installation_id: parseInt(installationId, 10), + }); + installation = result.data; + } catch (error) { + const err = error as { message?: string; status?: number }; + + if (err.status === 404) { + captureMessage('GitHub installation not found for authenticated app', { + level: 'warning', + tags: { + endpoint: 'github/callback', + source: 'github_api_get_installation', + status: '404', + }, + extra: { + installationId, + ownerType: owner.type, + setupAction, + githubAppType, + }, + }); - let installation; - try { - console.log('Fetching installation details for ID:', installationId); - const result = await octokitApp.apps.getInstallation({ - installation_id: parseInt(installationId), - }); - installation = result.data; - } catch (error) { - const err = error as { message?: string; status?: number }; + const encodedInstallationId = encodeURIComponent(installationId); - captureException(error, { - tags: { - endpoint: 'github/callback', - source: 'github_api_get_installation', - status: err.status?.toString() || 'unknown', - }, - extra: { - installationId, - ownerId, - ownerType: owner.type, - setupAction, - errorStatus: err.status, - errorMessage: err.message, - }, - }); + if (isAppInitiated) { + return NextResponse.redirect( + new URL(appFallbackPath('error=installation_not_found'), APP_URL) + ); + } + return NextResponse.redirect( + new URL( + appendQueryParam( + redirectPath, + `error=installation_not_found&id=${encodedInstallationId}` + ), + APP_URL + ) + ); + } - if (err.status === 404) { - const encodedInstallationId = encodeURIComponent(installationId); + captureException(error, { + tags: { + endpoint: 'github/callback', + source: 'github_api_get_installation', + status: err.status?.toString() || 'unknown', + }, + extra: { + installationId, + ownerType: owner.type, + setupAction, + githubAppType, + errorStatus: err.status, + errorMessage: err.message, + }, + }); if (isAppInitiated) { return NextResponse.redirect( - new URL(appFallbackPath('error=installation_not_found'), APP_URL) + new URL(appFallbackPath('error=installation_failed'), APP_URL) ); } - return NextResponse.redirect( - new URL( - appendQueryParam( - redirectPath, - `error=installation_not_found&id=${encodedInstallationId}` - ), - APP_URL - ) - ); - } - - if (isAppInitiated) { - return NextResponse.redirect(new URL(appFallbackPath('error=installation_failed'), APP_URL)); + throw error; } - throw error; } // Get selected repositories let repositories: PlatformRepository[] | null = null; if (installation.repository_selection === 'selected') { console.log('Fetching repositories for installation:', installationId); - const installationAuth = await auth({ - type: 'installation', - installationId: parseInt(installationId), - }); - const octokitInstallation = new Octokit({ - auth: installationAuth.token, - }); + const numericInstallationId = parseInt(installationId, 10); - const { data: reposData } = await octokitInstallation.apps.listReposAccessibleToInstallation(); - repositories = reposData.repositories.map(repo => ({ - id: repo.id, - name: repo.name, - full_name: repo.full_name, - private: repo.private, - })); + if (userAccessToken) { + const octokitUser = new Octokit({ + auth: userAccessToken, + }); + const { data: reposData } = await octokitUser.apps.listInstallationReposForAuthenticatedUser({ + installation_id: numericInstallationId, + }); + repositories = reposData.repositories.map(repo => ({ + id: repo.id, + name: repo.name, + full_name: repo.full_name, + private: repo.private, + })); + } else { + const auth = createAppAuth({ + appId: credentials.appId, + privateKey: credentials.privateKey, + }); + const installationAuth = await auth({ + type: 'installation', + installationId: numericInstallationId, + }); + const octokitInstallation = new Octokit({ + auth: installationAuth.token, + }); + + const { data: reposData } = + await octokitInstallation.apps.listReposAccessibleToInstallation(); + repositories = reposData.repositories.map(repo => ({ + id: repo.id, + name: repo.name, + full_name: repo.full_name, + private: repo.private, + })); + } } // Store installation in database diff --git a/apps/web/src/lib/integrations/platforms/github/app-selector.test.ts b/apps/web/src/lib/integrations/platforms/github/app-selector.test.ts index 3ce4873f9f..849cdde8eb 100644 --- a/apps/web/src/lib/integrations/platforms/github/app-selector.test.ts +++ b/apps/web/src/lib/integrations/platforms/github/app-selector.test.ts @@ -22,6 +22,10 @@ jest.mock( }) as never ); +let findAdministeredInstallation: (params: { + accessToken: string; + installationId: number | string; +}) => Promise<{ id: number } | null>; let assertUserAdministersInstallation: (params: { accessToken: string; installationId: number | string; @@ -29,6 +33,7 @@ let assertUserAdministersInstallation: (params: { beforeAll(async () => { const mod = await import('./app-selector'); + findAdministeredInstallation = mod.findAdministeredInstallation; assertUserAdministersInstallation = mod.assertUserAdministersInstallation; }); @@ -43,6 +48,43 @@ function mockPage(installations: Array<{ id: number }>, totalCount: number) { }); } +describe('findAdministeredInstallation', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('returns the installation when it is on the first page', async () => { + const match = { + id: INSTALLATION_ID, + account: { id: 1, login: 'octocat' }, + repository_selection: 'all', + permissions: { contents: 'write' }, + events: ['issues'], + created_at: '2026-09-01T00:00:00.000Z', + }; + mockPage([match, { id: 11111 }], 2); + + const result = await findAdministeredInstallation({ + accessToken: 'test-token', + installationId: INSTALLATION_ID, + }); + + expect(result).toEqual(match); + expect(mockListInstallationsForAuthenticatedUser).toHaveBeenCalledTimes(1); + }); + + test('returns null when the installation is absent', async () => { + mockPage([{ id: 11111 }], 1); + + const result = await findAdministeredInstallation({ + accessToken: 'test-token', + installationId: INSTALLATION_ID, + }); + + expect(result).toBeNull(); + }); +}); + describe('assertUserAdministersInstallation', () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/apps/web/src/lib/integrations/platforms/github/app-selector.ts b/apps/web/src/lib/integrations/platforms/github/app-selector.ts index 56b21994ab..9faaccd77d 100644 --- a/apps/web/src/lib/integrations/platforms/github/app-selector.ts +++ b/apps/web/src/lib/integrations/platforms/github/app-selector.ts @@ -85,21 +85,23 @@ export function getGitHubAppName(appType: GitHubAppType): string { return process.env.NEXT_PUBLIC_GITHUB_APP_NAME || 'KiloConnect'; } +type AdministeredGitHubInstallation = Awaited< + ReturnType +>['data']['installations'][number]; + /** - * Asserts that the GitHub user administers the given installation. + * Finds a GitHub App installation the authenticated user administers. * * Calls GET /user/installations with the user access token through Octokit. - * Paginates through all results. Returns true when installationId appears. + * Paginates through all results. Returns the matching installation, or null + * when installationId is absent from the list. * - * @param params.accessToken - A user-scoped OAuth access token. - * @param params.installationId - The GitHub App installation ID to check. - * @returns true when the user administers the installation. - * @throws Error for network or API failures — never returns false for those. + * @throws Error for network or API failures — never returns null for those. */ -export async function assertUserAdministersInstallation(params: { +export async function findAdministeredInstallation(params: { accessToken: string; installationId: number | string; -}): Promise { +}): Promise { const { accessToken, installationId } = params; const targetId = typeof installationId === 'string' ? parseInt(installationId, 10) : installationId; @@ -117,7 +119,7 @@ export async function assertUserAdministersInstallation(params: { for (const installation of data.installations) { if (installation.id === targetId) { - return true; + return installation; } } @@ -125,5 +127,18 @@ export async function assertUserAdministersInstallation(params: { page++; } - return false; + return null; +} + +/** + * Asserts that the GitHub user administers the given installation. + * + * @returns true when the user administers the installation. + * @throws Error for network or API failures — never returns false for those. + */ +export async function assertUserAdministersInstallation(params: { + accessToken: string; + installationId: number | string; +}): Promise { + return (await findAdministeredInstallation(params)) !== null; }