Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- [EE] Fixed worker startup races that could disable GitHub App authentication and permission syncing until restart after an online license refresh. [#1454](https://github.com/sourcebot-dev/sourcebot/pull/1454)
- [EE] Fixed GitHub connection sync jobs to fail safely when GitHub App authentication is configured without the required entitlement. [#1454](https://github.com/sourcebot-dev/sourcebot/pull/1454)

## [5.1.1] - 2026-07-14

- Add book a call button to sidebar. [#1441](https://github.com/sourcebot-dev/sourcebot/pull/1441)
Expand Down
18 changes: 12 additions & 6 deletions packages/backend/src/ee/accountPermissionSyncer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { PrismaClient, AccountPermissionSyncJobStatus, Account, PermissionSyncSo
import { env, createLogger, getIdentityProviderConfig, PERMISSION_SYNC_SUPPORTED_IDENTITY_PROVIDERS } from "@sourcebot/shared";
import { hasEntitlement } from "../entitlements.js";
import { ensureFreshAccountToken } from "./tokenRefresh.js";
import { Job, Queue, Worker } from "bullmq";
import { DelayedError, Job, Queue, Worker } from "bullmq";
import { Redis } from "ioredis";
import {
createOctokitFromToken,
Expand All @@ -26,6 +26,7 @@ const createJobLogger = (jobId: string) => createLogger(`${LOG_TAG}:job:${jobId}

const QUEUE_NAME = 'accountPermissionSyncQueue';
const POLLING_INTERVAL_MS = 1000;
const ENTITLEMENT_RETRY_DELAY_MS = 30 * 1000;

type AccountPermissionSyncJob = {
jobId: string;
Expand Down Expand Up @@ -59,13 +60,13 @@ export class AccountPermissionSyncer {
}

public async startScheduler() {
if (!await hasEntitlement('permission-syncing')) {
throw new Error('Permission syncing is not supported in current plan.');
}

logger.debug('Starting scheduler');

this.interval = setIntervalAsync(async () => {
if (!await hasEntitlement('permission-syncing')) {
return;
}

const thresholdDate = new Date(Date.now() - this.settings.userDrivenPermissionSyncIntervalMs);

const accounts = await this.db.account.findMany({
Expand Down Expand Up @@ -168,6 +169,11 @@ export class AccountPermissionSyncer {
}

private async runJob(job: Job<AccountPermissionSyncJob>) {
if (!await hasEntitlement('permission-syncing')) {
await job.moveToDelayed(Date.now() + ENTITLEMENT_RETRY_DELAY_MS, job.token);
throw new DelayedError('Permission syncing entitlement is not currently available.');
}

const id = job.data.jobId;
const logger = createJobLogger(id);

Expand Down Expand Up @@ -443,4 +449,4 @@ export class AccountPermissionSyncer {
logger.error(errorMessage('unknown account (id not found)', 'unknown user (id not found)'));
}
}
}
}
57 changes: 57 additions & 0 deletions packages/backend/src/ee/githubAppManager.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { beforeEach, describe, expect, test, vi } from 'vitest';

const mocks = vi.hoisted(() => ({
loadConfig: vi.fn(),
}));

vi.mock('@sourcebot/shared', () => ({
createLogger: vi.fn(() => ({
debug: vi.fn(),
error: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
})),
env: { CONFIG_PATH: '/tmp/config.json' },
getTokenFromConfig: vi.fn(),
loadConfig: mocks.loadConfig,
}));

vi.mock('@octokit/app', () => ({
App: vi.fn(),
}));

const getManager = async () => {
const { GithubAppManager } = await import('./githubAppManager.js');
return GithubAppManager.getInstance();
};

describe('GithubAppManager.ensureInitialized', () => {
beforeEach(() => {
vi.resetModules();
mocks.loadConfig.mockReset();
});

test('shares initialization across concurrent callers', async () => {
mocks.loadConfig.mockResolvedValue({});
const manager = await getManager();

await Promise.all([
manager.ensureInitialized(),
manager.ensureInitialized(),
]);

expect(mocks.loadConfig).toHaveBeenCalledTimes(1);
});

test('retries initialization after a transient failure', async () => {
mocks.loadConfig
.mockRejectedValueOnce(new Error('GitHub unavailable'))
.mockResolvedValueOnce({});
const manager = await getManager();

await expect(manager.ensureInitialized()).rejects.toThrow('GitHub unavailable');
await expect(manager.ensureInitialized()).resolves.toBeUndefined();

expect(mocks.loadConfig).toHaveBeenCalledTimes(2);
});
});
34 changes: 25 additions & 9 deletions packages/backend/src/ee/githubAppManager.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { App } from "@octokit/app";
import { getTokenFromConfig } from "@sourcebot/shared";
import { PrismaClient } from "@sourcebot/db";
import { createLogger } from "@sourcebot/shared";
import { GitHubAppConfig } from "@sourcebot/schemas/v3/index.type";
import { env, loadConfig } from "@sourcebot/shared";
Expand All @@ -21,8 +20,8 @@ export class GithubAppManager {
private static instance: GithubAppManager | null = null;
private octokitApps: Map<number, App>;
private installationMap: Map<string, Installation>;
private db: PrismaClient | null = null;
private initialized: boolean = false;
private initializationPromise: Promise<void> | null = null;

private constructor() {
this.octokitApps = new Map<number, App>();
Expand All @@ -36,16 +35,33 @@ export class GithubAppManager {
return GithubAppManager.instance;
}

private ensureInitialized(): void {
private assertInitialized(): void {
if (!this.initialized) {
throw new Error('GithubAppManager must be initialized before use. Call init() first.');
throw new Error('GithubAppManager must be initialized before use. Call ensureInitialized() first.');
}
}

public async init(db: PrismaClient) {
this.db = db;
public async ensureInitialized(): Promise<void> {
if (this.initialized) {
return;
}

if (!this.initializationPromise) {
this.initializationPromise = this.init().catch((error) => {
// Allow a later operation to retry after a transient GitHub or
// secret-resolution failure.
this.initializationPromise = null;
throw error;
});
}

await this.initializationPromise;
}

private async init(): Promise<void> {
const config = await loadConfig(env.CONFIG_PATH);
if (!config.apps) {
this.initialized = true;
return;
}

Expand Down Expand Up @@ -86,12 +102,12 @@ export class GithubAppManager {
this.installationMap.set(this.generateMapKey(owner, deploymentHostname), installation);
}
}

this.initialized = true;
}

public async getInstallationToken(owner: string, deploymentHostname: string = GITHUB_DEFAULT_DEPLOYMENT_HOSTNAME): Promise<string> {
this.ensureInitialized();
this.assertInitialized();

const key = this.generateMapKey(owner, deploymentHostname);
const installation = this.installationMap.get(key) as Installation | undefined;
Expand All @@ -112,4 +128,4 @@ export class GithubAppManager {
private generateMapKey(owner: string, deploymentHostname: string): string {
return `${deploymentHostname}/${owner}`;
}
}
}
16 changes: 11 additions & 5 deletions packages/backend/src/ee/repoPermissionSyncer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { PermissionSyncSource, PrismaClient, Repo, RepoPermissionSyncJobStatus }
import { createLogger, PERMISSION_SYNC_SUPPORTED_CODE_HOST_TYPES } from "@sourcebot/shared";
import { env } from "@sourcebot/shared";
import { hasEntitlement } from "../entitlements.js";
import { Job, Queue, Worker } from 'bullmq';
import { DelayedError, Job, Queue, Worker } from 'bullmq';
import { Redis } from 'ioredis';
import { createOctokitFromToken, getRepoCollaborators, GITHUB_CLOUD_HOSTNAME } from "../github.js";
import { createGitLabFromPersonalAccessToken, getProjectMembers } from "../gitlab.js";
Expand All @@ -19,6 +19,7 @@ type RepoPermissionSyncJob = {

const QUEUE_NAME = 'repoPermissionSyncQueue';
const POLLING_INTERVAL_MS = 1000;
const ENTITLEMENT_RETRY_DELAY_MS = 30 * 1000;
const LOG_TAG = 'repo-permission-syncer';

const logger = createLogger(LOG_TAG);
Expand Down Expand Up @@ -46,13 +47,13 @@ export class RepoPermissionSyncer {
}

public async startScheduler() {
if (!await hasEntitlement('permission-syncing')) {
throw new Error('Permission syncing is not supported in current plan.');
}

logger.debug('Starting scheduler');

this.interval = setIntervalAsync(async () => {
if (!await hasEntitlement('permission-syncing')) {
return;
}

// @todo: make this configurable
const thresholdDate = new Date(Date.now() - this.settings.repoDrivenPermissionSyncIntervalMs);

Expand Down Expand Up @@ -160,6 +161,11 @@ export class RepoPermissionSyncer {
}

private async runJob(job: Job<RepoPermissionSyncJob>) {
if (!await hasEntitlement('permission-syncing')) {
await job.moveToDelayed(Date.now() + ENTITLEMENT_RETRY_DELAY_MS, job.token);
throw new DelayedError('Permission syncing entitlement is not currently available.');
}

const id = job.data.jobId;
const logger = createJobLogger(id);

Expand Down
31 changes: 16 additions & 15 deletions packages/backend/src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,36 +116,37 @@ export const createOctokitFromToken = async ({ token, url }: { token?: string, u
}

/**
* Helper function to get an authenticated Octokit instance using GitHub App if available,
* otherwise falls back to the provided octokit instance.
* 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.
*/
const getOctokitWithGithubApp = async (
export const getOctokitWithGithubApp = async (
octokit: Octokit,
owner: string,
url: string | undefined,
context: string
): Promise<Octokit> => {
if (!await hasEntitlement('github-app') || !GithubAppManager.getInstance().appsConfigured()) {
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.getInstance().getInstallationToken(owner, hostname);
const { octokit: octokitFromToken, isAuthenticated } = await createOctokitFromToken({
const token = await githubAppManager.getInstallationToken(owner, hostname);
const { octokit: octokitFromToken } = await createOctokitFromToken({
token,
url,
});

if (isAuthenticated) {
return octokitFromToken;
} else {
logger.error(`Failed to authenticate with GitHub App for ${context}. Falling back to legacy token resolution.`);
return octokit;
}
return octokitFromToken;
} catch (error) {
logger.error(`Error getting GitHub App token for ${context}. Falling back to legacy token resolution.`, error);
return octokit;
logger.error(`Error getting GitHub App token for ${context}.`, error);
throw error;
}
}

Expand Down
Loading
Loading