From f7c31be2b17c0ae8a1e5b4bf9cb80b8afd33a0c8 Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Wed, 20 May 2026 18:20:17 -0700 Subject: [PATCH 1/4] fix --- packages/backend/src/bitbucket.ts | 150 +++++--------- .../backend/src/ee/accountPermissionSyncer.ts | 46 ++++- packages/backend/src/errors.test.ts | 187 ++++++++++++++++++ packages/backend/src/errors.ts | 29 +++ 4 files changed, 306 insertions(+), 106 deletions(-) create mode 100644 packages/backend/src/errors.test.ts create mode 100644 packages/backend/src/errors.ts diff --git a/packages/backend/src/bitbucket.ts b/packages/backend/src/bitbucket.ts index 2e10f57d0..5d3a7fa04 100644 --- a/packages/backend/src/bitbucket.ts +++ b/packages/backend/src/bitbucket.ts @@ -1,7 +1,7 @@ import { createBitbucketCloudClient as createBitbucketCloudClientBase } from "@coderabbitai/bitbucket/cloud"; import { createBitbucketServerClient as createBitbucketServerClientBase } from "@coderabbitai/bitbucket/server"; import { BitbucketConnectionConfig } from "@sourcebot/schemas/v3/bitbucket.type"; -import type { ClientOptions, ClientPathsWithMethod } from "openapi-fetch"; +import type { ClientOptions, ClientPathsWithMethod, Middleware } from "openapi-fetch"; import { createLogger } from "@sourcebot/shared"; import { measure, fetchWithRetry } from "./utils.js"; import * as Sentry from "@sentry/node"; @@ -42,6 +42,25 @@ type CloudGetRequestPath = ClientPathsWithMethod; type ServerAPI = ReturnType; type ServerGetRequestPath = ClientPathsWithMethod; +/** + * openapi-fetch middleware: convert any non-2xx response into a thrown Error + * with `.status` attached. Without this, every call site has to destructure + * `{ data, error }` and re-throw — with it, callers can rely on success + * meaning `data` is defined, and predicates like `isUnauthorized` see `.status` + * directly on the thrown Error. + */ +export const throwOnHttpError: Middleware = { + async onResponse({ response }) { + if (!response.ok) { + const body = await response.clone().text(); + throw Object.assign( + new Error(`Bitbucket API ${response.status}: ${body}`), + { status: response.status }, + ); + } + }, +}; + type CloudPaginatedResponse = { readonly next?: string; readonly page?: number; @@ -133,6 +152,7 @@ export function createBitbucketCloudClient(user: string | undefined, token: stri }; const apiClient = createBitbucketCloudClientBase(clientOptions); + apiClient.use(throwOnHttpError); var client: BitbucketClient = { deploymentType: BITBUCKET_CLOUD, token: token, @@ -199,7 +219,7 @@ async function cloudGetReposForWorkspace(client: BitbucketClient, workspaces: st const { durationMs, data } = await measure(async () => { const fetchFn = () => getPaginatedCloud(`/repositories/${workspace}` as CloudGetRequestPath, async (path, query) => { - const response = await client.apiClient.GET(path, { + const { data } = await client.apiClient.GET(path, { params: { path: { workspace, @@ -207,10 +227,6 @@ async function cloudGetReposForWorkspace(client: BitbucketClient, workspaces: st query: query, } }); - const { data, error } = response; - if (error) { - throw new Error(`Failed to fetch projects for workspace ${workspace}: ${JSON.stringify(error)}`); - } return data; }); return fetchWithRetry(fetchFn, `workspace ${workspace}`, logger); @@ -225,8 +241,7 @@ async function cloudGetReposForWorkspace(client: BitbucketClient, workspaces: st Sentry.captureException(e); logger.error(`Failed to get repos for workspace ${workspace}: ${e}`); - const status = e?.cause?.response?.status; - if (status == 404) { + if (e?.status === 404) { const warning = `Workspace ${workspace} not found or invalid access`; logger.warn(warning); return { @@ -262,7 +277,7 @@ async function cloudGetReposForProjects(client: BitbucketClient, projects: strin try { const { durationMs, data: repos } = await measure(async () => { const fetchFn = () => getPaginatedCloud(`/repositories/${workspace}` as CloudGetRequestPath, async (path, query) => { - const response = await client.apiClient.GET(path, { + const { data } = await client.apiClient.GET(path, { params: { path: { workspace, @@ -273,10 +288,6 @@ async function cloudGetReposForProjects(client: BitbucketClient, projects: strin } } }); - const { data, error } = response; - if (error) { - throw new Error(`Failed to fetch projects for workspace ${workspace}: ${JSON.stringify(error)}`); - } return data; }); return fetchWithRetry(fetchFn, `project ${project_name} in workspace ${workspace}`, logger); @@ -291,8 +302,7 @@ async function cloudGetReposForProjects(client: BitbucketClient, projects: strin Sentry.captureException(e); logger.error(`Failed to fetch repos for project ${project_name}: ${e}`); - const status = e?.cause?.response?.status; - if (status == 404) { + if (e?.status === 404) { const warning = `Project ${project_name} not found in ${workspace} or invalid access`; logger.warn(warning); return { @@ -328,11 +338,7 @@ async function cloudGetRepos(client: BitbucketClient, repoList: string[]): Promi try { const path = `/repositories/${workspace}/${repo_slug}` as CloudGetRequestPath; const data = await fetchWithRetry(async () => { - const response = await client.apiClient.GET(path); - const { data, error } = response; - if (error) { - throw new Error(`Failed to fetch repo ${repo}: ${JSON.stringify(error)}`); - } + const { data } = await client.apiClient.GET(path); return data; }, `repo ${repo}`, logger); return { @@ -343,8 +349,7 @@ async function cloudGetRepos(client: BitbucketClient, repoList: string[]): Promi Sentry.captureException(e); logger.error(`Failed to fetch repo ${repo}: ${e}`); - const status = e?.cause?.response?.status; - if (status === 404) { + if (e?.status === 404) { const warning = `Repo ${repo} not found in ${workspace} or invalid access`; logger.warn(warning); return { @@ -420,6 +425,7 @@ export function createBitbucketServerClient(url: string, user: string | undefine }; const apiClient = createBitbucketServerClientBase(clientOptions); + apiClient.use(throwOnHttpError); var client: BitbucketClient = { deploymentType: BITBUCKET_SERVER, token: token, @@ -477,7 +483,7 @@ async function serverGetReposForProjects(client: BitbucketClient, projects: stri const path = `/rest/api/1.0/projects/${project}/repos` as ServerGetRequestPath; const { durationMs, data } = await measure(async () => { const fetchFn = () => getPaginatedServer(path, async (url, start) => { - const response = await client.apiClient.GET(url, { + const { data } = await client.apiClient.GET(url, { params: { query: { limit: 1000, @@ -485,10 +491,6 @@ async function serverGetReposForProjects(client: BitbucketClient, projects: stri } } }); - const { data, error } = response; - if (error) { - throw new Error(`Failed to fetch repos for project ${project}: ${JSON.stringify(error)}`); - } return data; }); return fetchWithRetry(fetchFn, `project ${project}`, logger); @@ -503,8 +505,7 @@ async function serverGetReposForProjects(client: BitbucketClient, projects: stri Sentry.captureException(e); logger.error(`Failed to get repos for project ${project}: ${e}`); - const status = e?.cause?.response?.status; - if (status == 404) { + if (e?.status === 404) { const warning = `Project ${project} not found or invalid access`; logger.warn(warning); return { @@ -540,11 +541,7 @@ async function serverGetRepos(client: BitbucketClient, repoList: string[]): Prom try { const path = `/rest/api/1.0/projects/${project}/repos/${repo_slug}` as ServerGetRequestPath; const data = await fetchWithRetry(async () => { - const response = await client.apiClient.GET(path); - const { data, error } = response; - if (error) { - throw new Error(`Failed to fetch repo ${repo}: ${JSON.stringify(error)}`); - } + const { data } = await client.apiClient.GET(path); return data; }, `repo ${repo}`, logger); return { @@ -555,8 +552,7 @@ async function serverGetRepos(client: BitbucketClient, repoList: string[]): Prom Sentry.captureException(e); logger.error(`Failed to fetch repo ${repo}: ${e}`); - const status = e?.cause?.response?.status; - if (status === 404) { + if (e?.status === 404) { const warning = `Repo ${repo} not found in project ${project} or invalid access`; logger.warn(warning); return { @@ -581,13 +577,9 @@ async function serverGetAllRepos(client: BitbucketClient): Promise<{repos: Serve const path = `/rest/api/1.0/repos` as ServerGetRequestPath; const { durationMs, data } = await measure(async () => { const fetchFn = () => getPaginatedServer(path, async (url, start) => { - const response = await client.apiClient.GET(url, { + const { data } = await client.apiClient.GET(url, { params: { query: { limit: 1000, start } } }); - const { data, error } = response; - if (error) { - throw new Error(`Failed to fetch all repos: ${JSON.stringify(error)}`); - } return data; }); return fetchWithRetry(fetchFn, `all repos`, logger); @@ -652,16 +644,12 @@ export const getExplicitUserPermissionsForCloudRepo = async ( const path = `/repositories/${workspace}/${repoSlug}/permissions-config/users` as CloudGetRequestPath; const users = await fetchWithRetry(() => getPaginatedCloud(path, async (p, query) => { - const response = await client.apiClient.GET(p, { + const { data } = await client.apiClient.GET(p, { params: { path: { workspace, repo_slug: repoSlug }, query, }, }); - const { data, error } = response; - if (error) { - throw new Error(`Failed to get explicit user permissions for ${workspace}/${repoSlug}: ${JSON.stringify(error)}`); - } return data; }), `permissions for ${workspace}/${repoSlug}`, logger); @@ -682,13 +670,9 @@ export const getReposForAuthenticatedBitbucketCloudUser = async ( const path = `/user/permissions/repositories` as CloudGetRequestPath; const permissions = await fetchWithRetry(() => getPaginatedCloud(path, async (p, query) => { - const response = await client.apiClient.GET(p, { + const { data } = await client.apiClient.GET(p, { params: { query }, }); - const { data, error } = response; - if (error) { - throw new Error(`Failed to get user repository permissions: ${JSON.stringify(error)}`); - } return data; }), 'user repository permissions', logger); @@ -707,28 +691,21 @@ export const getReposForAuthenticatedBitbucketServerUser = async ( client: BitbucketClient, ): Promise> => { - /** - * @note We need to explicitly check if the user is authenticated here because - * /rest/api/1.0/repos?permission=REPO_READ will return an empty list if the - * following conditions are met: - * 1. Anonymous access is enabled via `feature.public.access` - * 2. The token is expired or invalid. - * - * This check ensures we will not hit this condition and instead fail with a - * explicit error. - * - * @see https://developer.atlassian.com/server/bitbucket/rest/v906/api-group-repository/#api-api-latest-repos-get - * @see https://confluence.atlassian.com/bitbucketserver/configuration-properties-776640155.html - */ - const isAuthenticated = await isBitbucketServerUserAuthenticated(client); - if (!isAuthenticated) { - throw new Error(`Bitbucket Server authentication check failed. The OAuth token may be expired and the server may be treating the request as anonymous. Please re-authenticate with Bitbucket Server.`); - } + // Probe an auth-required endpoint first. When `feature.public.access` is + // enabled on the BBS instance and the token is expired/invalid, the call + // to /rest/api/1.0/repos?permission=REPO_READ below returns 200 with an + // empty list instead of 401 — silently masking an unauthorized state. + // /profile/recent/repos does return 401 in that case, so the middleware's + // throw-on-error propagates with a real status code that isUnauthorized() + // can catch downstream. + // @see https://developer.atlassian.com/server/bitbucket/rest/v906/api-group-repository/#api-api-latest-repos-get + // @see https://confluence.atlassian.com/bitbucketserver/configuration-properties-776640155.html + await client.apiClient.GET(`/rest/api/1.0/profile/recent/repos` as ServerGetRequestPath, {}); const repos = await fetchWithRetry(() => getPaginatedServer<{ id: number }>( `/rest/api/1.0/repos` as ServerGetRequestPath, async (url, start) => { - const response = await client.apiClient.GET(url, { + const { data } = await client.apiClient.GET(url, { params: { query: { permission: 'REPO_READ', @@ -737,10 +714,6 @@ export const getReposForAuthenticatedBitbucketServerUser = async ( }, }, }); - const { data, error } = response; - if (error) { - throw new Error(`Failed to fetch Bitbucket Server repos for authenticated user: ${JSON.stringify(error)}`); - } return data; } ), 'repos for authenticated Bitbucket Server user', logger); @@ -766,13 +739,9 @@ export const getUserPermissionsForServerRepo = async ( const repoUsers = await fetchWithRetry(() => getPaginatedServer<{ user: { id: number } }>( `/rest/api/1.0/projects/${projectKey}/repos/${repoSlug}/permissions/users` as ServerGetRequestPath, async (url, start) => { - const response = await client.apiClient.GET(url, { + const { data } = await client.apiClient.GET(url, { params: { query: { limit: 1000, start } }, }); - const { data, error } = response; - if (error) { - throw new Error(`Failed to fetch repo-level permissions for ${projectKey}/${repoSlug}: ${JSON.stringify(error)}`); - } return data; } ), `repo-level permissions for ${projectKey}/${repoSlug}`, logger); @@ -808,28 +777,3 @@ export const isBitbucketServerPublicAccessEnabled = async ( return false; } }; - -/** - * Returns true if the Bitbucket Server client is authenticated as a real user, - * false if the token is expired, invalid, or the request is being treated as anonymous. - */ -export const isBitbucketServerUserAuthenticated = async ( - client: BitbucketClient, -): Promise => { - try { - const { error, response } = await client.apiClient.GET(`/rest/api/1.0/profile/recent/repos` as ServerGetRequestPath, {}); - if (error) { - if (response.status === 401 || response.status === 403) { - return false; - } - throw new Error(`Unexpected error when verifying Bitbucket Server authentication status: ${JSON.stringify(error)}`); - } - return true; - } catch (e: any) { - // Handle the case where openapi-fetch throws directly for auth errors - if (e?.status === 401 || e?.status === 403) { - return false; - } - throw e; - } -}; \ No newline at end of file diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index f8e32b745..b3878a8c1 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -1,5 +1,5 @@ import * as Sentry from "@sentry/node"; -import { PrismaClient, AccountPermissionSyncJobStatus, Account, PermissionSyncSource} from "@sourcebot/db"; +import { PrismaClient, AccountPermissionSyncJobStatus, Account, PermissionSyncSource } from "@sourcebot/db"; import { env, hasEntitlement, createLogger, loadConfig, PERMISSION_SYNC_SUPPORTED_IDENTITY_PROVIDERS } from "@sourcebot/shared"; import { ensureFreshAccountToken } from "./tokenRefresh.js"; import { Job, Queue, Worker } from "bullmq"; @@ -17,6 +17,7 @@ import { import { createBitbucketCloudClient, createBitbucketServerClient, getReposForAuthenticatedBitbucketCloudUser, getReposForAuthenticatedBitbucketServerUser } from "../bitbucket.js"; import { Settings } from "../types.js"; import { setIntervalAsync } from "../utils.js"; +import { isUnauthorized, isForbidden } from "../errors.js"; const LOG_TAG = 'user-permission-syncer'; const logger = createLogger(LOG_TAG); @@ -28,6 +29,11 @@ const POLLING_INTERVAL_MS = 1000; type AccountPermissionSyncJob = { jobId: string; } +class RefreshTokenError extends Error { + constructor(message: string) { + super(message); + } +} export class AccountPermissionSyncer { private queue: Queue; @@ -179,13 +185,47 @@ export class AccountPermissionSyncer { } }); + try { + await this.syncAccountPermissions(account, logger); + } catch (error) { + // Fail-closed: when the code-host layer signals that the upstream + // account is permanently unauthorized (token revoked, user + // deprovisioned, OAuth grant dead), clear the account's existing + // permission rows so the read-side filter stops matching through + // them. Re-throw so the job is marked FAILED via onJobFailed. + if ( + isUnauthorized(error) || + isForbidden(error) || + error instanceof RefreshTokenError + ) { + await this.db.account.update({ + where: { id: account.id }, + data: { + accessibleRepos: { + deleteMany: {}, + }, + }, + }); + throw error; + } + } + } + + private async syncAccountPermissions( + account: Account & { user: { email: string | null } }, + logger: ReturnType, + ) { const config = await loadConfig(env.CONFIG_PATH); logger.debug(`Syncing permissions for ${account.provider} account (id: ${account.id}) for user ${account.user.email}...`); // Ensure the OAuth token is fresh, refreshing it if it is expired or near expiry. - // Throws and sets Account.tokenRefreshErrorMessage if the refresh fails. - const accessToken = await ensureFreshAccountToken(account, this.db); + let accessToken; + try { + accessToken = await ensureFreshAccountToken(account, this.db); + } catch (error) { + throw new RefreshTokenError(error instanceof Error ? error.message : 'Failed to refresh token with unknown error.'); + } // Get a list of all repos that the user has access to from all connected accounts. const repoIds = await (async () => { diff --git a/packages/backend/src/errors.test.ts b/packages/backend/src/errors.test.ts new file mode 100644 index 000000000..ac6a00dd4 --- /dev/null +++ b/packages/backend/src/errors.test.ts @@ -0,0 +1,187 @@ +import { describe, expect, test } from 'vitest'; +import { RequestError } from '@octokit/request-error'; +import { GitbeakerRequestError } from '@gitbeaker/requester-utils'; +import { isForbidden, isUnauthorized } from './errors'; +import { throwOnHttpError } from './bitbucket'; + +// Helper: invoke the openapi-fetch middleware against a synthetic Response and +// return the thrown error (or null if it didn't throw). Validates the actual +// contract — not a hand-rolled mock of what we hope the middleware produces. +const invokeMiddleware = async (response: Response): Promise => { + try { + await throwOnHttpError.onResponse!({ + request: new Request('https://example.test'), + response, + schemaPath: '/path', + params: {}, + options: {} as never, + id: 'test', + }); + return null; + } catch (e) { + return e; + } +}; + +describe('isUnauthorized', () => { + test('Octokit RequestError with status 401', () => { + const err = new RequestError('Unauthorized', 401, { + request: { method: 'GET', url: 'https://api.github.com/user/repos', headers: {} }, + }); + expect(isUnauthorized(err)).toBe(true); + }); + + test('Octokit RequestError with status 403 is NOT unauthorized', () => { + const err = new RequestError('Forbidden', 403, { + request: { method: 'GET', url: 'https://api.github.com/user/repos', headers: {} }, + }); + expect(isUnauthorized(err)).toBe(false); + }); + + test('real GitbeakerRequestError with response status 401', () => { + const err = new GitbeakerRequestError('Unauthorized', { + cause: { + description: 'Unauthorized', + request: new Request('https://gitlab.com/api/v4/projects'), + response: new Response(null, { status: 401 }), + }, + }); + expect(isUnauthorized(err)).toBe(true); + }); + + test('Bitbucket middleware throws an isUnauthorized error on 401 Response', async () => { + const err = await invokeMiddleware(new Response('unauthorized body', { status: 401 })); + expect(err).toBeInstanceOf(Error); + expect(isUnauthorized(err)).toBe(true); + }); + + test('tokenRefresh invalid_grant synthetic 401', () => { + // Shape produced by ensureFreshAccountToken when invalid_grant is returned. + const err = Object.assign(new Error('OAuth invalid_grant'), { status: 401 }); + expect(isUnauthorized(err)).toBe(true); + }); + + test('plain Error without status is NOT unauthorized', () => { + // e.g. "missing scope" errors, decryption failures. + expect(isUnauthorized(new Error('Missing required scope'))).toBe(false); + }); + + test('TypeError from fetch (network failure) is NOT unauthorized', () => { + // Node fetch network errors have no .status. + const err = new TypeError('fetch failed'); + expect(isUnauthorized(err)).toBe(false); + }); + + test('null is NOT unauthorized', () => { + expect(isUnauthorized(null)).toBe(false); + }); + + test('undefined is NOT unauthorized', () => { + expect(isUnauthorized(undefined)).toBe(false); + }); + + test('non-Error throwable (string) is NOT unauthorized', () => { + expect(isUnauthorized('boom')).toBe(false); + }); + + test('object with non-numeric status is NOT unauthorized', () => { + expect(isUnauthorized({ status: '401' })).toBe(false); + }); + + test('Octokit RequestError with status 500 is NOT unauthorized', () => { + const err = new RequestError('Internal Server Error', 500, { + request: { method: 'GET', url: 'https://api.github.com/user/repos', headers: {} }, + }); + expect(isUnauthorized(err)).toBe(false); + }); + + test('object missing both direct and nested status is NOT unauthorized', () => { + expect(isUnauthorized({ message: 'something' })).toBe(false); + }); +}); + +describe('isForbidden', () => { + test('Octokit RequestError with status 403', () => { + const err = new RequestError('Forbidden', 403, { + request: { method: 'GET', url: 'https://api.github.com/user/repos', headers: {} }, + }); + expect(isForbidden(err)).toBe(true); + }); + + test('Octokit RequestError with status 401 is NOT forbidden', () => { + const err = new RequestError('Unauthorized', 401, { + request: { method: 'GET', url: 'https://api.github.com/user/repos', headers: {} }, + }); + expect(isForbidden(err)).toBe(false); + }); + + test('real GitbeakerRequestError with response status 403', () => { + const err = new GitbeakerRequestError('Forbidden', { + cause: { + description: 'Forbidden', + request: new Request('https://gitlab.com/api/v4/projects'), + response: new Response(null, { status: 403 }), + }, + }); + expect(isForbidden(err)).toBe(true); + }); + + test('Bitbucket middleware throws an isForbidden error on 403 Response', async () => { + const err = await invokeMiddleware(new Response('forbidden body', { status: 403 })); + expect(err).toBeInstanceOf(Error); + expect(isForbidden(err)).toBe(true); + }); + + test('plain Error without status is NOT forbidden', () => { + expect(isForbidden(new Error('Missing required scope'))).toBe(false); + }); + + test('null is NOT forbidden', () => { + expect(isForbidden(null)).toBe(false); + }); + + test('Octokit RequestError with status 500 is NOT forbidden', () => { + const err = new RequestError('Internal Server Error', 500, { + request: { method: 'GET', url: 'https://api.github.com/user/repos', headers: {} }, + }); + expect(isForbidden(err)).toBe(false); + }); +}); + +describe('throwOnHttpError middleware contract', () => { + test('does not throw on 2xx Response', async () => { + const err = await invokeMiddleware(new Response('ok', { status: 200 })); + expect(err).toBeNull(); + }); + + test('does not throw on 204 No Content', async () => { + // 204 is 2xx, so middleware should pass through. (Downstream code uses + // `data!` assertions that would misbehave on undefined — but that's a + // bitbucket.ts call-site concern, not a middleware-contract concern.) + const err = await invokeMiddleware(new Response(null, { status: 204 })); + expect(err).toBeNull(); + }); + + test('throws on 500 Response with status attached', async () => { + const err = await invokeMiddleware(new Response('boom', { status: 500 })); + expect(err).toBeInstanceOf(Error); + expect((err as { status?: number }).status).toBe(500); + }); + + test('throws on 404 Response with status attached', async () => { + const err = await invokeMiddleware(new Response('not found', { status: 404 })); + expect(err).toBeInstanceOf(Error); + expect((err as { status?: number }).status).toBe(404); + }); +}); + +describe('precedence: direct .status wins over nested .cause.response.status', () => { + test('direct status 0 returns 0, ignoring nested 401', () => { + // Latent precedence: if an error somehow has BOTH, direct wins. + const err = Object.assign(new Error('weird'), { + status: 0, + cause: { response: { status: 401 } }, + }); + expect(isUnauthorized(err)).toBe(false); + }); +}); diff --git a/packages/backend/src/errors.ts b/packages/backend/src/errors.ts new file mode 100644 index 000000000..341c76736 --- /dev/null +++ b/packages/backend/src/errors.ts @@ -0,0 +1,29 @@ + +/** + * Extract an HTTP status code from a thrown error across the libraries used by + * the code-host clients: + * - Octokit RequestError: { status } + * - openapi-fetch (Bitbucket Cloud / Server): direct throws with { status } + * or errors wrapped via Object.assign(new Error(...), { status }) + * - gitbeaker (GitLab): { cause: { response: { status } } } + */ +const getStatus = (err: unknown): number | null => { + if (err === null || typeof err !== 'object') { + return null; + } + + const direct = (err as { status?: unknown }).status; + if (typeof direct === 'number') { + return direct; + } + + const nested = (err as { cause?: { response?: { status?: unknown } } }).cause?.response?.status; + if (typeof nested === 'number') { + return nested; + } + + return null; +}; + +export const isUnauthorized = (err: unknown): boolean => getStatus(err) === 401; +export const isForbidden = (err: unknown): boolean => getStatus(err) === 403; From e35e8911d5f59afb671d164da7dca7e820cbc9c3 Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Wed, 20 May 2026 19:12:29 -0700 Subject: [PATCH 2/4] feedback --- .../backend/src/ee/accountPermissionSyncer.ts | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index b3878a8c1..ac25dbdb6 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -206,8 +206,8 @@ export class AccountPermissionSyncer { }, }, }); - throw error; } + throw error; } } @@ -254,21 +254,7 @@ export class AccountPermissionSyncer { // @note: we only care about the private repos since we don't need to build a mapping // for public repos. // @see: packages/web/src/prisma.ts - let githubRepos; - try { - githubRepos = await getReposForAuthenticatedUser(/* visibility = */ 'private', octokit); - } catch (error) { - if (error && typeof error === 'object' && 'status' in error) { - const status = (error as { status: number }).status; - if (status === 401 || status === 403) { - throw new Error( - `GitHub API returned ${status} error. Your token may have expired or lacks the required permissions. ` + - `Please re-authorize with GitHub to grant the necessary access.` - ); - } - } - throw error; - } + const githubRepos = await getReposForAuthenticatedUser(/* visibility = */ 'private', octokit); const gitHubRepoIds = githubRepos.map(repo => repo.id.toString()); const repos = await this.db.repo.findMany({ From a6e39a723d09b815bb416abef1d3f7fec96b78f3 Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Wed, 20 May 2026 19:16:11 -0700 Subject: [PATCH 3/4] feedback --- packages/backend/src/bitbucket.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/backend/src/bitbucket.ts b/packages/backend/src/bitbucket.ts index 5d3a7fa04..241a79a8d 100644 --- a/packages/backend/src/bitbucket.ts +++ b/packages/backend/src/bitbucket.ts @@ -52,9 +52,8 @@ type ServerGetRequestPath = ClientPathsWithMethod; export const throwOnHttpError: Middleware = { async onResponse({ response }) { if (!response.ok) { - const body = await response.clone().text(); throw Object.assign( - new Error(`Bitbucket API ${response.status}: ${body}`), + new Error(`Bitbucket API ${response.status}: ${response.statusText}`), { status: response.status }, ); } From f5cc2447b526fbb1f8d4fc0d92ba413e5f2db0c8 Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Wed, 20 May 2026 19:24:44 -0700 Subject: [PATCH 4/4] feedback --- CHANGELOG.md | 3 +++ packages/backend/src/ee/accountPermissionSyncer.ts | 11 ++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a71130d57..55683269b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Fixed issue where repo permissions could go stale when authentication or token refresh related errors occured. [#1215](https://github.com/sourcebot-dev/sourcebot/pull/1215) + ## [4.17.2] - 2026-05-16 ### Added diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index ac25dbdb6..5f524d829 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -29,6 +29,7 @@ const POLLING_INTERVAL_MS = 1000; type AccountPermissionSyncJob = { jobId: string; } + class RefreshTokenError extends Error { constructor(message: string) { super(message); @@ -192,7 +193,7 @@ export class AccountPermissionSyncer { // account is permanently unauthorized (token revoked, user // deprovisioned, OAuth grant dead), clear the account's existing // permission rows so the read-side filter stops matching through - // them. Re-throw so the job is marked FAILED via onJobFailed. + // them. if ( isUnauthorized(error) || isForbidden(error) || @@ -220,6 +221,14 @@ export class AccountPermissionSyncer { logger.debug(`Syncing permissions for ${account.provider} account (id: ${account.id}) for user ${account.user.email}...`); // Ensure the OAuth token is fresh, refreshing it if it is expired or near expiry. + // + // @note(SOU-1177) re-throwing as a RefreshTokenError here is required to flag to the caller + // (runJob) that the account's permissions should be cleared. The side-effect with this + // approach is that permissions will be cleared for any error thrown in the + // ensureFreshAccountToken path. A better approach would be to look at the response + // from the oauth call and determining if the host returned a invalid_grant. + // + // @see: https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 let accessToken; try { accessToken = await ensureFreshAccountToken(account, this.db);