From 6b10a852e607f472f13111a5e09ee504a5594b0d Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Thu, 30 Jul 2026 12:12:29 -0700 Subject: [PATCH 1/3] fix(backend): warn on missing GitHub App org installation --- packages/backend/src/ee/githubAppManager.ts | 9 ++- packages/backend/src/github.ts | 15 +++- packages/backend/src/githubAppAuth.test.ts | 83 +++++++++++++++++++-- 3 files changed, 97 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/ee/githubAppManager.ts b/packages/backend/src/ee/githubAppManager.ts index ec125cc10..fb7d8b388 100644 --- a/packages/backend/src/ee/githubAppManager.ts +++ b/packages/backend/src/ee/githubAppManager.ts @@ -16,6 +16,13 @@ type Installation = { }; }; +export class GithubAppInstallationNotFoundError extends Error { + constructor(owner: string, deploymentHostname: string) { + super(`GitHub App installation not found for ${deploymentHostname}/${owner}`); + this.name = 'GithubAppInstallationNotFoundError'; + } +} + export class GithubAppManager { private static instance: GithubAppManager | null = null; private octokitApps: Map; @@ -112,7 +119,7 @@ export class GithubAppManager { const key = this.generateMapKey(owner, deploymentHostname); const installation = this.installationMap.get(key) as Installation | undefined; if (!installation) { - throw new Error(`GitHub App Installation not found for ${key}`); + throw new GithubAppInstallationNotFoundError(owner, deploymentHostname); } const octokitApp = this.octokitApps.get(installation.appId) as App; diff --git a/packages/backend/src/github.ts b/packages/backend/src/github.ts index cb3b876cd..f1159cc43 100644 --- a/packages/backend/src/github.ts +++ b/packages/backend/src/github.ts @@ -9,7 +9,7 @@ import { hasEntitlement } from "./entitlements.js"; import micromatch from "micromatch"; import pLimit from "p-limit"; import { processPromiseResults, throwIfAnyFailed } from "./connectionUtils.js"; -import { GithubAppManager } from "./ee/githubAppManager.js"; +import { GithubAppInstallationNotFoundError, GithubAppManager } from "./ee/githubAppManager.js"; import { fetchWithRetry, measure } from "./utils.js"; export const GITHUB_CLOUD_HOSTNAME = "github.com"; @@ -145,6 +145,10 @@ export const getOctokitWithGithubApp = async ( }); return octokitFromToken; } catch (error) { + if (error instanceof GithubAppInstallationNotFoundError) { + throw error; + } + logger.error(`Error getting GitHub App token for ${context}.`, error); throw error; } @@ -403,6 +407,15 @@ const getReposForOrgs = async (orgs: string[], octokit: Octokit, signal: AbortSi data }; } catch (error) { + if (error instanceof GithubAppInstallationNotFoundError) { + const warning = error.message; + logger.warn(warning); + return { + type: 'warning' as const, + warning + }; + } + Sentry.captureException(error); logger.error(`Failed to fetch repositories for org ${org}.`, error); diff --git a/packages/backend/src/githubAppAuth.test.ts b/packages/backend/src/githubAppAuth.test.ts index b9e56072f..b5cd2461d 100644 --- a/packages/backend/src/githubAppAuth.test.ts +++ b/packages/backend/src/githubAppAuth.test.ts @@ -1,4 +1,3 @@ -import type { Octokit } from '@octokit/rest'; import { beforeEach, describe, expect, test, vi } from 'vitest'; const mocks = vi.hoisted(() => ({ @@ -6,6 +5,49 @@ const mocks = vi.hoisted(() => ({ ensureInitialized: vi.fn(), getInstallationToken: vi.fn(), hasEntitlement: vi.fn(), + logger: { + debug: vi.fn(), + error: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + }, + GithubAppInstallationNotFoundError: class GithubAppInstallationNotFoundError extends Error { + constructor(owner: string, deploymentHostname: string) { + super(`GitHub App installation not found for ${deploymentHostname}/${owner}`); + this.name = 'GithubAppInstallationNotFoundError'; + } + }, +})); + +vi.mock('@octokit/rest', () => ({ + Octokit: class { + public paginate = { + iterator: async function* (_request: unknown, options: { org: string }) { + yield { + data: [{ + clone_url: `https://github.com/${options.org}/repo.git`, + full_name: `${options.org}/repo`, + id: 1, + name: 'repo', + owner: { + avatar_url: '', + login: options.org, + }, + }], + }; + }, + }; + + public repos = { + listForOrg: vi.fn(), + }; + + public rest = { + users: { + getAuthenticated: vi.fn(), + }, + }; + }, })); vi.mock('@sentry/node', () => ({ @@ -13,12 +55,7 @@ vi.mock('@sentry/node', () => ({ })); vi.mock('@sourcebot/shared', () => ({ - createLogger: vi.fn(() => ({ - debug: vi.fn(), - error: vi.fn(), - info: vi.fn(), - warn: vi.fn(), - })), + createLogger: vi.fn(() => mocks.logger), env: { FALLBACK_GITHUB_CLOUD_TOKEN: undefined, }, @@ -30,6 +67,7 @@ vi.mock('./entitlements.js', () => ({ })); vi.mock('./ee/githubAppManager.js', () => ({ + GithubAppInstallationNotFoundError: mocks.GithubAppInstallationNotFoundError, GithubAppManager: { getInstance: () => ({ appsConfigured: mocks.appsConfigured, @@ -39,7 +77,8 @@ vi.mock('./ee/githubAppManager.js', () => ({ }, })); -import { getOctokitWithGithubApp } from './github.js'; +import type { Octokit } from '@octokit/rest'; +import { getGitHubReposFromConfig, getOctokitWithGithubApp } from './github.js'; describe('getOctokitWithGithubApp', () => { beforeEach(() => { @@ -47,6 +86,10 @@ describe('getOctokitWithGithubApp', () => { mocks.ensureInitialized.mockReset().mockResolvedValue(undefined); mocks.getInstallationToken.mockReset().mockResolvedValue('installation-token'); mocks.hasEntitlement.mockReset(); + mocks.logger.debug.mockReset(); + mocks.logger.error.mockReset(); + mocks.logger.info.mockReset(); + mocks.logger.warn.mockReset(); }); test('fails safely, then uses the GitHub App when the entitlement appears after startup', async () => { @@ -101,4 +144,28 @@ describe('getOctokitWithGithubApp', () => { 'org example', )).rejects.toBe(error); }); + + test('warns and continues when the GitHub App is not installed for one organization', async () => { + mocks.hasEntitlement.mockResolvedValue(true); + mocks.getInstallationToken.mockImplementation(async (owner: string) => { + if (owner === 'invalid-org') { + throw new mocks.GithubAppInstallationNotFoundError(owner, 'github.com'); + } + return 'installation-token'; + }); + + const result = await getGitHubReposFromConfig({ + type: 'github', + orgs: ['valid-org', 'invalid-org'], + }, new AbortController().signal); + + expect(result.repos.map(repo => repo.full_name)).toEqual(['valid-org/repo']); + expect(result.warnings).toEqual([ + 'GitHub App installation not found for github.com/invalid-org', + ]); + expect(mocks.logger.warn).toHaveBeenCalledWith( + 'GitHub App installation not found for github.com/invalid-org', + ); + expect(mocks.logger.error).not.toHaveBeenCalled(); + }); }); From a07413ad4bdfbe78deff980aed1554f89b98295c Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Thu, 30 Jul 2026 12:13:15 -0700 Subject: [PATCH 2/3] chore: update changelog for #1522 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bc9693cc..ad33046a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed vulnerability triage for reusable-workflow callers, repositories without CodeQL, and transient non-JSON Linear query responses. [#1515](https://github.com/sourcebot-dev/sourcebot/pull/1515) +- [EE] Continued syncing valid GitHub organizations when the GitHub App is not installed for another configured organization. [#1522](https://github.com/sourcebot-dev/sourcebot/pull/1522) ## [5.1.4] - 2026-07-24 From 25cc7753f8d720d6fde0f92b8dfbfbdff3fdca4d Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Thu, 30 Jul 2026 15:54:55 -0700 Subject: [PATCH 3/3] refactor(backend): centralize Octokit creation --- .../backend/src/ee/accountPermissionSyncer.ts | 4 +- .../backend/src/ee/repoPermissionSyncer.ts | 4 +- packages/backend/src/github.ts | 125 ++++++++++-------- packages/backend/src/githubAppAuth.test.ts | 71 +++++----- 4 files changed, 113 insertions(+), 91 deletions(-) diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index 7184a191e..22c2194f8 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -12,7 +12,7 @@ import { ensureFreshAccountToken, TokenRefreshError } from "./tokenRefresh.js"; import { DelayedError, Job, Queue, Worker } from "bullmq"; import { Redis } from "ioredis"; import { - createOctokitFromToken, + createOctokit, getOAuthScopesForAuthenticatedUser as getGitHubOAuthScopesForAuthenticatedUser, getReposForAuthenticatedUser, } from "../github.js"; @@ -296,7 +296,7 @@ export class AccountPermissionSyncer { } if (idpConfig.provider === 'github') { - const { octokit } = await createOctokitFromToken({ + const { octokit } = await createOctokit({ token: accessToken, url: idpConfig.baseUrl, }); diff --git a/packages/backend/src/ee/repoPermissionSyncer.ts b/packages/backend/src/ee/repoPermissionSyncer.ts index 536f48a09..fa4dc9ce0 100644 --- a/packages/backend/src/ee/repoPermissionSyncer.ts +++ b/packages/backend/src/ee/repoPermissionSyncer.ts @@ -5,7 +5,7 @@ import { env } from "@sourcebot/shared"; import { hasEntitlement } from "../entitlements.js"; import { DelayedError, Job, Queue, Worker } from 'bullmq'; import { Redis } from 'ioredis'; -import { createOctokitFromToken, getRepoCollaborators, GITHUB_CLOUD_HOSTNAME } from "../github.js"; +import { createOctokit, getRepoCollaborators, GITHUB_CLOUD_HOSTNAME } from "../github.js"; import { createGitLabFromPersonalAccessToken, getProjectMembers } from "../gitlab.js"; import { createBitbucketCloudClient, createBitbucketServerClient, getExplicitUserPermissionsForCloudRepo, getUserPermissionsForServerRepo } from "../bitbucket.js"; import { repoMetadataSchema } from "@sourcebot/shared"; @@ -209,7 +209,7 @@ export class RepoPermissionSyncer { }> => { if (repo.external_codeHostType === 'github') { const isGitHubCloud = credentials.hostUrl ? new URL(credentials.hostUrl).hostname === GITHUB_CLOUD_HOSTNAME : true; - const { octokit } = await createOctokitFromToken({ + const { octokit } = await createOctokit({ token: credentials.token, url: isGitHubCloud ? undefined : credentials.hostUrl, }); diff --git a/packages/backend/src/github.ts b/packages/backend/src/github.ts index f1159cc43..17d95821b 100644 --- a/packages/backend/src/github.ts +++ b/packages/backend/src/github.ts @@ -100,10 +100,49 @@ const isHttpError = (error: unknown, status: number): boolean => { && error.status === status; } -export const createOctokitFromToken = async ({ token, url }: { token?: string, url?: string }): Promise<{ octokit: Octokit, isAuthenticated: boolean }> => { +/** + * Creates an Octokit client using a GitHub App installation when one is + * configured for the requested owner, or falls back to the provided token. + */ +export const createOctokit = async ({ + token, + url, + owner, + context, +}: { + token?: string, + url?: string, + owner?: string, + context?: string, +}): Promise<{ octokit: Octokit, isAuthenticated: boolean }> => { + let resolvedToken = token; + + if (owner) { + const githubAppManager = GithubAppManager.getInstance(); + await githubAppManager.ensureInitialized(); + + if (githubAppManager.appsConfigured()) { + if (!await hasEntitlement('github-app')) { + throw new Error(`GitHub App authentication is not currently licensed for ${context ?? owner}.`); + } + + try { + const hostname = url ? new URL(url).hostname : GITHUB_CLOUD_HOSTNAME; + resolvedToken = await githubAppManager.getInstallationToken(owner, hostname); + } catch (error) { + if (error instanceof GithubAppInstallationNotFoundError) { + throw error; + } + + logger.error(`Error getting GitHub App token for ${context ?? owner}.`, error); + throw error; + } + } + } + const isGitHubCloud = url ? new URL(url).hostname === GITHUB_CLOUD_HOSTNAME : true; const octokit = new Octokit({ - auth: token, + auth: resolvedToken, ...(url && !isGitHubCloud ? { baseUrl: `${url}/api/v3` } : {}), @@ -111,49 +150,10 @@ export const createOctokitFromToken = async ({ token, url }: { token?: string, u return { octokit, - isAuthenticated: !!token, + isAuthenticated: !!resolvedToken, }; } -/** - * Uses GitHub App authentication when an app is configured. App initialization - * and token failures are propagated so callers cannot mistake a partial, - * unauthenticated response for an authoritative repository list. - */ -export const getOctokitWithGithubApp = async ( - octokit: Octokit, - owner: string, - url: string | undefined, - context: string -): Promise => { - const githubAppManager = GithubAppManager.getInstance(); - await githubAppManager.ensureInitialized(); - if (!githubAppManager.appsConfigured()) { - return octokit; - } - - if (!await hasEntitlement('github-app')) { - throw new Error(`GitHub App authentication is not currently licensed for ${context}.`); - } - - try { - const hostname = url ? new URL(url).hostname : GITHUB_CLOUD_HOSTNAME; - const token = await githubAppManager.getInstallationToken(owner, hostname); - const { octokit: octokitFromToken } = await createOctokitFromToken({ - token, - url, - }); - return octokitFromToken; - } catch (error) { - if (error instanceof GithubAppInstallationNotFoundError) { - throw error; - } - - logger.error(`Error getting GitHub App token for ${context}.`, error); - throw error; - } -} - export const getGitHubReposFromConfig = async (config: GithubConnectionConfig, signal: AbortSignal): Promise<{ repos: OctokitRepository[], warnings: string[] }> => { const hostname = config.url ? new URL(config.url).hostname : @@ -165,7 +165,7 @@ export const getGitHubReposFromConfig = async (config: GithubConnectionConfig, s env.FALLBACK_GITHUB_CLOUD_TOKEN : undefined; - const { octokit, isAuthenticated } = await createOctokitFromToken({ + const { octokit, isAuthenticated } = await createOctokit({ token, url: config.url, }); @@ -185,19 +185,19 @@ export const getGitHubReposFromConfig = async (config: GithubConnectionConfig, s let allWarnings: string[] = []; if (config.orgs) { - const { repos, warnings } = await getReposForOrgs(config.orgs, octokit, signal, config.url); + const { repos, warnings } = await getReposForOrgs(config.orgs, token, signal, config.url); allRepos = allRepos.concat(repos); allWarnings = allWarnings.concat(warnings); } if (config.repos) { - const { repos, warnings } = await getRepos(config.repos, octokit, signal, config.url); + const { repos, warnings } = await getRepos(config.repos, token, signal, config.url); allRepos = allRepos.concat(repos); allWarnings = allWarnings.concat(warnings); } if (config.users) { - const { repos, warnings } = await getReposOwnedByUsers(config.users, octokit, signal, config.url); + const { repos, warnings } = await getReposOwnedByUsers(config.users, token, signal, config.url); allRepos = allRepos.concat(repos); allWarnings = allWarnings.concat(warnings); } @@ -295,12 +295,17 @@ export const getOAuthScopesForAuthenticatedUser = async (octokit: Octokit, token } } -const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: AbortSignal, url?: string) => { +const getReposOwnedByUsers = async (users: string[], token: string | undefined, signal: AbortSignal, url?: string) => { const results = await Promise.allSettled(users.map((user) => githubQueryLimit(async () => { try { logger.debug(`Fetching repository info for user ${user}...`); - const octokitToUse = await getOctokitWithGithubApp(octokit, user, url, `user ${user}`); + const { octokit } = await createOctokit({ + token, + url, + owner: user, + context: `user ${user}`, + }); const { durationMs, data } = await measure(async () => { const fetchFn = async () => { let query = `user:${user}`; @@ -317,7 +322,7 @@ const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: A // signal.aborted between pages. paginate() only passes the signal to // individual fetch requests but doesn't check abort state between pages. const allRepos: OctokitRepository[] = []; - const iterator = octokitToUse.paginate.iterator(octokitToUse.rest.search.repos, { + const iterator = octokit.paginate.iterator(octokit.rest.search.repos, { q: query, per_page: 100, request: { @@ -368,19 +373,24 @@ const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: A }; } -const getReposForOrgs = async (orgs: string[], octokit: Octokit, signal: AbortSignal, url?: string) => { +const getReposForOrgs = async (orgs: string[], token: string | undefined, signal: AbortSignal, url?: string) => { const results = await Promise.allSettled(orgs.map((org) => githubQueryLimit(async () => { try { logger.debug(`Fetching repository info for org ${org}...`); - const octokitToUse = await getOctokitWithGithubApp(octokit, org, url, `org ${org}`); + const { octokit } = await createOctokit({ + token, + url, + owner: org, + context: `org ${org}`, + }); const { durationMs, data } = await measure(async () => { // @note: We use paginate.iterator() instead of paginate() to check // signal.aborted between pages. paginate() only passes the signal to // individual fetch requests but doesn't check abort state between pages. const fetchFn = async () => { const allRepos: OctokitRepository[] = []; - const iterator = octokitToUse.paginate.iterator(octokitToUse.repos.listForOrg, { + const iterator = octokit.paginate.iterator(octokit.repos.listForOrg, { org: org, per_page: 100, request: { @@ -440,15 +450,20 @@ const getReposForOrgs = async (orgs: string[], octokit: Octokit, signal: AbortSi }; } -const getRepos = async (repoList: string[], octokit: Octokit, signal: AbortSignal, url?: string) => { +const getRepos = async (repoList: string[], token: string | undefined, signal: AbortSignal, url?: string) => { const results = await Promise.allSettled(repoList.map((repo) => githubQueryLimit(async () => { try { const [owner, repoName] = repo.split('/'); logger.debug(`Fetching repository info for ${repo}...`); - const octokitToUse = await getOctokitWithGithubApp(octokit, owner, url, `repo ${repo}`); + const { octokit } = await createOctokit({ + token, + url, + owner, + context: `repo ${repo}`, + }); const { durationMs, data: result } = await measure(async () => { - const fetchFn = () => octokitToUse.repos.get({ + const fetchFn = () => octokit.repos.get({ owner, repo: repoName, request: { diff --git a/packages/backend/src/githubAppAuth.test.ts b/packages/backend/src/githubAppAuth.test.ts index b5cd2461d..7c8cbcce1 100644 --- a/packages/backend/src/githubAppAuth.test.ts +++ b/packages/backend/src/githubAppAuth.test.ts @@ -17,10 +17,15 @@ const mocks = vi.hoisted(() => ({ this.name = 'GithubAppInstallationNotFoundError'; } }, + octokitOptions: [] as Array<{ auth?: string, baseUrl?: string }>, })); vi.mock('@octokit/rest', () => ({ Octokit: class { + constructor(options: { auth?: string, baseUrl?: string }) { + mocks.octokitOptions.push(options); + } + public paginate = { iterator: async function* (_request: unknown, options: { org: string }) { yield { @@ -77,10 +82,9 @@ vi.mock('./ee/githubAppManager.js', () => ({ }, })); -import type { Octokit } from '@octokit/rest'; -import { getGitHubReposFromConfig, getOctokitWithGithubApp } from './github.js'; +import { createOctokit, getGitHubReposFromConfig } from './github.js'; -describe('getOctokitWithGithubApp', () => { +describe('createOctokit', () => { beforeEach(() => { mocks.appsConfigured.mockReset().mockReturnValue(true); mocks.ensureInitialized.mockReset().mockResolvedValue(undefined); @@ -90,45 +94,48 @@ describe('getOctokitWithGithubApp', () => { mocks.logger.error.mockReset(); mocks.logger.info.mockReset(); mocks.logger.warn.mockReset(); + mocks.octokitOptions.length = 0; }); test('fails safely, then uses the GitHub App when the entitlement appears after startup', async () => { - const fallbackOctokit = {} as Octokit; mocks.hasEntitlement .mockResolvedValueOnce(false) .mockResolvedValueOnce(true); - await expect(getOctokitWithGithubApp( - fallbackOctokit, - 'example', - undefined, - 'org example', - )).rejects.toThrow('GitHub App authentication is not currently licensed for org example.'); - - const entitledOctokit = await getOctokitWithGithubApp( - fallbackOctokit, - 'example', - undefined, - 'org example', - ); + await expect(createOctokit({ + token: 'legacy-token', + owner: 'example', + context: 'org example', + })).rejects.toThrow('GitHub App authentication is not currently licensed for org example.'); + + const result = await createOctokit({ + token: 'legacy-token', + owner: 'example', + context: 'org example', + }); expect(mocks.ensureInitialized).toHaveBeenCalledTimes(2); expect(mocks.getInstallationToken).toHaveBeenCalledWith('example', 'github.com'); - expect(entitledOctokit).not.toBe(fallbackOctokit); + expect(result.isAuthenticated).toBe(true); + expect(mocks.octokitOptions).toEqual([{ + auth: 'installation-token', + }]); }); - test('uses legacy authentication when no GitHub App is configured', async () => { - const fallbackOctokit = {} as Octokit; + test('falls back to token authentication when no GitHub App is configured', async () => { mocks.appsConfigured.mockReturnValue(false); mocks.hasEntitlement.mockResolvedValue(false); - await expect(getOctokitWithGithubApp( - fallbackOctokit, - 'example', - undefined, - 'org example', - )).resolves.toBe(fallbackOctokit); + const result = await createOctokit({ + token: 'legacy-token', + owner: 'example', + context: 'org example', + }); + expect(result.isAuthenticated).toBe(true); + expect(mocks.octokitOptions).toEqual([{ + auth: 'legacy-token', + }]); expect(mocks.hasEntitlement).not.toHaveBeenCalled(); }); @@ -137,12 +144,12 @@ describe('getOctokitWithGithubApp', () => { mocks.hasEntitlement.mockResolvedValue(true); mocks.getInstallationToken.mockRejectedValue(error); - await expect(getOctokitWithGithubApp( - {} as Octokit, - 'example', - undefined, - 'org example', - )).rejects.toBe(error); + await expect(createOctokit({ + token: 'legacy-token', + owner: 'example', + context: 'org example', + })).rejects.toBe(error); + expect(mocks.octokitOptions).toEqual([]); }); test('warns and continues when the GitHub App is not installed for one organization', async () => {