From 05a6e29c6d22282b1370706b729c705c07a121a2 Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Wed, 22 Jul 2026 13:04:05 -0400 Subject: [PATCH 1/4] fix(worker): classify permission sync failures by provider context Replace generic HTTP status fail-closed handling with typed upstream permission-sync errors so ambiguous forbidden and gone responses preserve cached permissions. Fixes SOU-1560 Fixes SOU-1177 --- .../src/ee/accountPermissionSyncer.test.ts | 38 ++++- .../backend/src/ee/accountPermissionSyncer.ts | 95 ++++++++---- .../src/ee/permissionSyncError.test.ts | 101 ++++++++++++ .../backend/src/ee/permissionSyncError.ts | 145 ++++++++++++++++++ packages/backend/src/errors.ts | 10 +- 5 files changed, 347 insertions(+), 42 deletions(-) create mode 100644 packages/backend/src/ee/permissionSyncError.test.ts create mode 100644 packages/backend/src/ee/permissionSyncError.ts diff --git a/packages/backend/src/ee/accountPermissionSyncer.test.ts b/packages/backend/src/ee/accountPermissionSyncer.test.ts index b2499e6e8..eee226e36 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.test.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.test.ts @@ -1,5 +1,9 @@ import { describe, expect, test } from 'vitest'; import { classifyPermissionSyncFailure } from './accountPermissionSyncer.js'; +import { + PermissionSyncUpstreamError, + type PermissionSyncUpstreamErrorKind, +} from './permissionSyncError.js'; import { TokenRefreshError, type TokenRefreshErrorKind } from './tokenRefresh.js'; const tokenRefreshError = ( @@ -10,6 +14,14 @@ const tokenRefreshError = ( status, }); +const upstreamError = ( + kind: PermissionSyncUpstreamErrorKind, +): PermissionSyncUpstreamError => new PermissionSyncUpstreamError(`Permission sync failed: ${kind}`, { + kind, + provider: 'github', + operation: 'list_accessible_repositories', +}); + describe('classifyPermissionSyncFailure', () => { test('fails closed for invalid_grant', () => { expect(classifyPermissionSyncFailure(tokenRefreshError('invalid_grant', 400))).toEqual({ @@ -36,19 +48,29 @@ describe('classifyPermissionSyncFailure', () => { }); test.each([ - [401, 'http_unauthorized'], - [403, 'http_forbidden'], - [410, 'http_gone'], - ] as const)('preserves fail-closed behavior for an API HTTP %s response', (status, reason) => { - const error = Object.assign(new Error(reason), { status }); - expect(classifyPermissionSyncFailure(error)).toEqual({ + ['credential_rejected', 'upstream_credential_rejected'], + ['insufficient_scope', 'upstream_insufficient_scope'], + ['permission_endpoint_removed', 'permission_endpoint_removed'], + ] as const)('fails closed for a classified %s upstream failure', (kind, reason) => { + expect(classifyPermissionSyncFailure(upstreamError(kind))).toEqual({ action: 'clear_permissions', reason, }); }); - test('keeps permissions for an unrelated API failure', () => { - const error = Object.assign(new Error('Internal Server Error'), { status: 500 }); + test.each([ + 'rate_limited', + 'upstream_unavailable', + 'forbidden', + 'unknown', + ] satisfies PermissionSyncUpstreamErrorKind[])('keeps permissions for a classified %s upstream failure', (kind) => { + expect(classifyPermissionSyncFailure(upstreamError(kind))).toEqual({ + action: 'preserve_permissions', + }); + }); + + test.each([401, 403, 410])('does not fail closed on an unclassified HTTP %s error', (status) => { + const error = Object.assign(new Error(`HTTP ${status}`), { status }); expect(classifyPermissionSyncFailure(error)).toEqual({ action: 'preserve_permissions', }); diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index 382d773fe..ffe622ba3 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -18,7 +18,7 @@ import { import { createBitbucketCloudClient, createBitbucketServerClient, getReposForAuthenticatedBitbucketCloudUser, getReposForAuthenticatedBitbucketServerUser } from "../bitbucket.js"; import { Settings } from "../types.js"; import { setIntervalAsync } from "../utils.js"; -import { isUnauthorized, isForbidden, isGone } from "../errors.js"; +import { PermissionSyncUpstreamError, withPermissionSyncUpstreamError } from "./permissionSyncError.js"; const LOG_TAG = 'user-permission-syncer'; const logger = createLogger(LOG_TAG); @@ -34,9 +34,9 @@ type AccountPermissionSyncJob = { export type PermissionCleanupReason = | 'oauth_invalid_grant' - | 'http_unauthorized' - | 'http_forbidden' - | 'http_gone'; + | 'upstream_credential_rejected' + | 'upstream_insufficient_scope' + | 'permission_endpoint_removed'; export type PermissionCleanupDecision = | { @@ -49,9 +49,9 @@ export type PermissionCleanupDecision = const PERMISSION_CLEANUP_REASON_MESSAGES: Record = { oauth_invalid_grant: 'OAuth invalid_grant', - http_unauthorized: 'HTTP 401 Unauthorized', - http_forbidden: 'HTTP 403 Forbidden', - http_gone: 'HTTP 410 Gone', + upstream_credential_rejected: 'upstream credential rejection', + upstream_insufficient_scope: 'insufficient OAuth scope', + permission_endpoint_removed: 'permission endpoint removed', }; export const classifyPermissionSyncFailure = (error: unknown): PermissionCleanupDecision => { @@ -64,14 +64,16 @@ export const classifyPermissionSyncFailure = (error: unknown): PermissionCleanup : { action: 'preserve_permissions' }; } - if (isUnauthorized(error)) { - return { action: 'clear_permissions', reason: 'http_unauthorized' }; - } - if (isForbidden(error)) { - return { action: 'clear_permissions', reason: 'http_forbidden' }; - } - if (isGone(error)) { - return { action: 'clear_permissions', reason: 'http_gone' }; + if (error instanceof PermissionSyncUpstreamError) { + if (error.kind === 'credential_rejected') { + return { action: 'clear_permissions', reason: 'upstream_credential_rejected' }; + } + if (error.kind === 'insufficient_scope') { + return { action: 'clear_permissions', reason: 'upstream_insufficient_scope' }; + } + if (error.kind === 'permission_endpoint_removed') { + return { action: 'clear_permissions', reason: 'permission_endpoint_removed' }; + } } return { action: 'preserve_permissions' }; @@ -235,12 +237,9 @@ 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) or that the endpoint we depend - // on is gone (e.g. Bitbucket Cloud's CHANGE-2770), clear the - // account's existing permission rows so the read-side filter stops - // matching through them. + // Clear cached permissions only for classified permanent failures. + // Ambiguous HTTP errors and transient upstream failures preserve the + // last successful permission state. const cleanupDecision = classifyPermissionSyncFailure(error); if (cleanupDecision.action === 'clear_permissions') { @@ -280,19 +279,34 @@ export class AccountPermissionSyncer { url: idpConfig.baseUrl, }); - const scopes = await getGitHubOAuthScopesForAuthenticatedUser(octokit, accessToken); + const scopes = await withPermissionSyncUpstreamError( + 'github', + 'inspect_token_scopes', + () => getGitHubOAuthScopesForAuthenticatedUser(octokit, accessToken), + ); // Token supports scope introspection (classic PAT or OAuth app token) if (scopes !== null) { if (!scopes.includes('repo')) { - throw new Error(`OAuth token with scopes [${scopes.join(', ')}] is missing the 'repo' scope required for permission syncing. Please re-authorize with GitHub to grant the required scope.`); + throw new PermissionSyncUpstreamError( + `OAuth token with scopes [${scopes.join(', ')}] is missing the 'repo' scope required for permission syncing. Please re-authorize with GitHub to grant the required scope.`, + { + kind: 'insufficient_scope', + provider: 'github', + operation: 'inspect_token_scopes', + }, + ); } } // @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 - const githubRepos = await getReposForAuthenticatedUser(/* visibility = */ 'private', octokit); + const githubRepos = await withPermissionSyncUpstreamError( + 'github', + 'list_accessible_repositories', + () => getReposForAuthenticatedUser(/* visibility = */ 'private', octokit), + ); const gitHubRepoIds = githubRepos.map(repo => repo.id.toString()); const repos = await this.db.repo.findMany({ @@ -314,9 +328,20 @@ export class AccountPermissionSyncer { url: idpConfig.baseUrl, }); - const scopes = await getGitLabOAuthScopesForAuthenticatedUser(api); + const scopes = await withPermissionSyncUpstreamError( + 'gitlab', + 'inspect_token_scopes', + () => getGitLabOAuthScopesForAuthenticatedUser(api), + ); if (!scopes.includes('read_api')) { - throw new Error(`OAuth token with scopes [${scopes.join(', ')}] is missing the 'read_api' scope required for permission syncing.`); + throw new PermissionSyncUpstreamError( + `OAuth token with scopes [${scopes.join(', ')}] is missing the 'read_api' scope required for permission syncing.`, + { + kind: 'insufficient_scope', + provider: 'gitlab', + operation: 'inspect_token_scopes', + }, + ); } // @note: we only care about the private repos since we don't need to build a @@ -326,7 +351,11 @@ export class AccountPermissionSyncer { // // @see: packages/web/src/prisma.ts const gitLabProjectIds = ( - await getProjectsForAuthenticatedUser('private', api) + await withPermissionSyncUpstreamError( + 'gitlab', + 'list_accessible_repositories', + () => getProjectsForAuthenticatedUser('private', api), + ) ).map(project => project.id.toString()); const repos = await this.db.repo.findMany({ @@ -346,7 +375,11 @@ export class AccountPermissionSyncer { // @note: we don't pass a user here since we want to use a bearer token // for authentication. const client = createBitbucketCloudClient(/* user = */ undefined, accessToken) - const bitbucketRepos = await getReposForAuthenticatedBitbucketCloudUser(client); + const bitbucketRepos = await withPermissionSyncUpstreamError( + 'bitbucket-cloud', + 'list_accessible_repositories', + () => getReposForAuthenticatedBitbucketCloudUser(client), + ); const bitbucketRepoUuids = bitbucketRepos.map(repo => repo.uuid); const repos = await this.db.repo.findMany({ @@ -364,7 +397,11 @@ export class AccountPermissionSyncer { repos.forEach(repo => aggregatedRepoIds.add(repo.id)); } else if (idpConfig.provider === 'bitbucket-server') { const client = createBitbucketServerClient(idpConfig.baseUrl, /* user = */ undefined, accessToken); - const serverRepos = await getReposForAuthenticatedBitbucketServerUser(client); + const serverRepos = await withPermissionSyncUpstreamError( + 'bitbucket-server', + 'list_accessible_repositories', + () => getReposForAuthenticatedBitbucketServerUser(client), + ); const serverRepoIds = serverRepos.map(r => r.id); const repos = await this.db.repo.findMany({ diff --git a/packages/backend/src/ee/permissionSyncError.test.ts b/packages/backend/src/ee/permissionSyncError.test.ts new file mode 100644 index 000000000..34a4a0a53 --- /dev/null +++ b/packages/backend/src/ee/permissionSyncError.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, test } from 'vitest'; +import { + classifyPermissionSyncUpstreamError, + PermissionSyncUpstreamError, + withPermissionSyncUpstreamError, +} from './permissionSyncError.js'; + +describe('classifyPermissionSyncUpstreamError', () => { + test('classifies a 401 from an authenticated operation as a credential rejection', () => { + const cause = Object.assign(new Error('Unauthorized'), { status: 401 }); + + expect(classifyPermissionSyncUpstreamError( + cause, + 'bitbucket-server', + 'list_accessible_repositories', + )).toMatchObject({ + kind: 'credential_rejected', + provider: 'bitbucket-server', + operation: 'list_accessible_repositories', + status: 401, + cause, + }); + }); + + test('classifies a GitHub rate-limit 403 separately from an ambiguous forbidden response', () => { + const rateLimitError = Object.assign(new Error('Rate limited'), { + status: 403, + response: { headers: { 'x-ratelimit-remaining': '0' } }, + }); + const forbiddenError = Object.assign(new Error('Forbidden'), { status: 403 }); + + expect(classifyPermissionSyncUpstreamError( + rateLimitError, + 'github', + 'list_accessible_repositories', + ).kind).toBe('rate_limited'); + expect(classifyPermissionSyncUpstreamError( + forbiddenError, + 'github', + 'list_accessible_repositories', + ).kind).toBe('forbidden'); + }); + + test('classifies HTTP 429 as rate limited', () => { + const cause = Object.assign(new Error('Too Many Requests'), { status: 429 }); + + expect(classifyPermissionSyncUpstreamError( + cause, + 'gitlab', + 'list_accessible_repositories', + ).kind).toBe('rate_limited'); + }); + + test('classifies Bitbucket Cloud 410 from the repository-list operation as a removed endpoint', () => { + const cause = Object.assign(new Error('Gone'), { status: 410 }); + + expect(classifyPermissionSyncUpstreamError( + cause, + 'bitbucket-cloud', + 'list_accessible_repositories', + ).kind).toBe('permission_endpoint_removed'); + }); + + test('does not generalize HTTP 410 from another provider to a removed permission endpoint', () => { + const cause = Object.assign(new Error('Gone'), { status: 410 }); + + expect(classifyPermissionSyncUpstreamError( + cause, + 'github', + 'list_accessible_repositories', + ).kind).toBe('unknown'); + }); + + test.each([ + Object.assign(new Error('Internal Server Error'), { status: 500 }), + new TypeError('fetch failed'), + Object.assign(new Error('request timed out'), { name: 'TimeoutError' }), + ])('classifies an unavailable provider as transient', (cause) => { + expect(classifyPermissionSyncUpstreamError( + cause, + 'bitbucket-server', + 'list_accessible_repositories', + ).kind).toBe('upstream_unavailable'); + }); + + test('does not reclassify an existing permission sync error', async () => { + const error = new PermissionSyncUpstreamError('Missing scope', { + kind: 'insufficient_scope', + provider: 'github', + operation: 'inspect_token_scopes', + }); + + const caught = await withPermissionSyncUpstreamError( + 'github', + 'inspect_token_scopes', + () => Promise.reject(error), + ).catch(error => error); + + expect(caught).toBe(error); + }); +}); diff --git a/packages/backend/src/ee/permissionSyncError.ts b/packages/backend/src/ee/permissionSyncError.ts new file mode 100644 index 000000000..191dba1b0 --- /dev/null +++ b/packages/backend/src/ee/permissionSyncError.ts @@ -0,0 +1,145 @@ +import type { IdentityProviderType } from '@sourcebot/shared'; +import { getErrorStatus } from '../errors.js'; + +export type PermissionSyncUpstreamErrorKind = + | 'credential_rejected' + | 'insufficient_scope' + | 'rate_limited' + | 'permission_endpoint_removed' + | 'upstream_unavailable' + | 'forbidden' + | 'unknown'; + +export type PermissionSyncOperation = + | 'inspect_token_scopes' + | 'list_accessible_repositories'; + +type PermissionSyncUpstreamErrorOptions = { + kind: PermissionSyncUpstreamErrorKind; + provider: IdentityProviderType; + operation: PermissionSyncOperation; + status?: number; + cause?: unknown; +}; + +export class PermissionSyncUpstreamError extends Error { + public readonly kind: PermissionSyncUpstreamErrorKind; + public readonly provider: IdentityProviderType; + public readonly operation: PermissionSyncOperation; + public readonly status?: number; + + constructor(message: string, options: PermissionSyncUpstreamErrorOptions) { + super(message, { cause: options.cause }); + this.name = 'PermissionSyncUpstreamError'; + this.kind = options.kind; + this.provider = options.provider; + this.operation = options.operation; + this.status = options.status; + } +} + +const getHeader = (error: unknown, name: string): string | undefined => { + if (error === null || typeof error !== 'object') { + return undefined; + } + + const directHeaders = (error as { response?: { headers?: unknown } }).response?.headers; + const nestedHeaders = (error as { cause?: { response?: { headers?: unknown } } }).cause?.response?.headers; + const headers = directHeaders ?? nestedHeaders; + + if (headers instanceof Headers) { + return headers.get(name) ?? undefined; + } + + if (headers !== null && typeof headers === 'object') { + const value = (headers as Record)[name.toLowerCase()]; + return typeof value === 'string' ? value : undefined; + } + + return undefined; +}; + +const isRateLimited = (error: unknown, status: number | null): boolean => + status === 429 || + getHeader(error, 'retry-after') !== undefined || + getHeader(error, 'x-ratelimit-remaining') === '0'; + +const isNetworkOrTimeoutError = (error: unknown): boolean => { + if (!(error instanceof Error)) { + return false; + } + + return error instanceof TypeError || error.name === 'AbortError' || error.name === 'TimeoutError'; +}; + +export const classifyPermissionSyncUpstreamError = ( + error: unknown, + provider: IdentityProviderType, + operation: PermissionSyncOperation, +): PermissionSyncUpstreamError => { + if (error instanceof PermissionSyncUpstreamError) { + return error; + } + + const status = getErrorStatus(error); + + if (status === 401) { + return new PermissionSyncUpstreamError( + `${provider} rejected the permission sync credential.`, + { kind: 'credential_rejected', provider, operation, status, cause: error }, + ); + } + + if (isRateLimited(error, status)) { + return new PermissionSyncUpstreamError( + `${provider} rate limited the permission sync request.`, + { kind: 'rate_limited', provider, operation, status: status ?? undefined, cause: error }, + ); + } + + if (status === 403) { + return new PermissionSyncUpstreamError( + `${provider} forbade the permission sync request.`, + { kind: 'forbidden', provider, operation, status, cause: error }, + ); + } + + if ( + status === 410 && + provider === 'bitbucket-cloud' && + operation === 'list_accessible_repositories' + ) { + return new PermissionSyncUpstreamError( + 'The Bitbucket Cloud permission endpoint is no longer available.', + { kind: 'permission_endpoint_removed', provider, operation, status, cause: error }, + ); + } + + if ( + status === 408 || + (status !== null && status >= 500 && status < 600) || + isNetworkOrTimeoutError(error) + ) { + return new PermissionSyncUpstreamError( + `${provider} is temporarily unavailable for permission syncing.`, + { kind: 'upstream_unavailable', provider, operation, status: status ?? undefined, cause: error }, + ); + } + + return new PermissionSyncUpstreamError( + `${provider} permission sync failed with an unclassified upstream error.`, + { kind: 'unknown', provider, operation, status: status ?? undefined, cause: error }, + ); +}; + +export const withPermissionSyncUpstreamError = async ( + provider: IdentityProviderType, + operation: PermissionSyncOperation, + callback: () => Promise, +): Promise => { + try { + return await callback(); + } catch (error) { + throw classifyPermissionSyncUpstreamError(error, provider, operation); + } +}; diff --git a/packages/backend/src/errors.ts b/packages/backend/src/errors.ts index e174f0b20..5b3b108ab 100644 --- a/packages/backend/src/errors.ts +++ b/packages/backend/src/errors.ts @@ -7,7 +7,7 @@ * or errors wrapped via Object.assign(new Error(...), { status }) * - gitbeaker (GitLab): { cause: { response: { status } } } */ -const getStatus = (err: unknown): number | null => { +export const getErrorStatus = (err: unknown): number | null => { if (err === null || typeof err !== 'object') { return null; } @@ -25,7 +25,7 @@ const getStatus = (err: unknown): number | null => { return null; }; -export const isUnauthorized = (err: unknown): boolean => getStatus(err) === 401; -export const isForbidden = (err: unknown): boolean => getStatus(err) === 403; -export const isNotFound = (err: unknown): boolean => getStatus(err) === 404; -export const isGone = (err: unknown): boolean => getStatus(err) === 410; +export const isUnauthorized = (err: unknown): boolean => getErrorStatus(err) === 401; +export const isForbidden = (err: unknown): boolean => getErrorStatus(err) === 403; +export const isNotFound = (err: unknown): boolean => getErrorStatus(err) === 404; +export const isGone = (err: unknown): boolean => getErrorStatus(err) === 410; From 2269ca062b19fc8d3e4c010ca7830c29914a94de Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Wed, 22 Jul 2026 13:04:43 -0400 Subject: [PATCH 2/4] chore: add changelog entry for permission sync classification --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2382fb1d..4678376f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - [EE] Preserved cached repository permissions when OAuth token refresh fails because of a transient code host outage. [#1481](https://github.com/sourcebot-dev/sourcebot/pull/1481) +- [EE] Classified code host permission sync failures by provider context before clearing cached repository permissions. [#1482](https://github.com/sourcebot-dev/sourcebot/pull/1482) ## [5.1.3] - 2026-07-20 From e66cf46dda8ad202791c93d480ec5f21f6a5e1f1 Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Wed, 22 Jul 2026 13:29:20 -0400 Subject: [PATCH 3/4] refactor(worker): normalize HTTP error metadata access --- .../backend/src/ee/permissionSyncError.ts | 27 +-------- packages/backend/src/errors.test.ts | 55 ++++++++++++++++- packages/backend/src/errors.ts | 60 ++++++++++++++----- 3 files changed, 101 insertions(+), 41 deletions(-) diff --git a/packages/backend/src/ee/permissionSyncError.ts b/packages/backend/src/ee/permissionSyncError.ts index 191dba1b0..c5694d947 100644 --- a/packages/backend/src/ee/permissionSyncError.ts +++ b/packages/backend/src/ee/permissionSyncError.ts @@ -1,5 +1,5 @@ import type { IdentityProviderType } from '@sourcebot/shared'; -import { getErrorStatus } from '../errors.js'; +import { getErrorHeader, getErrorStatus } from '../errors.js'; export type PermissionSyncUpstreamErrorKind = | 'credential_rejected' @@ -38,31 +38,10 @@ export class PermissionSyncUpstreamError extends Error { } } -const getHeader = (error: unknown, name: string): string | undefined => { - if (error === null || typeof error !== 'object') { - return undefined; - } - - const directHeaders = (error as { response?: { headers?: unknown } }).response?.headers; - const nestedHeaders = (error as { cause?: { response?: { headers?: unknown } } }).cause?.response?.headers; - const headers = directHeaders ?? nestedHeaders; - - if (headers instanceof Headers) { - return headers.get(name) ?? undefined; - } - - if (headers !== null && typeof headers === 'object') { - const value = (headers as Record)[name.toLowerCase()]; - return typeof value === 'string' ? value : undefined; - } - - return undefined; -}; - const isRateLimited = (error: unknown, status: number | null): boolean => status === 429 || - getHeader(error, 'retry-after') !== undefined || - getHeader(error, 'x-ratelimit-remaining') === '0'; + getErrorHeader(error, 'retry-after') !== undefined || + getErrorHeader(error, 'x-ratelimit-remaining') === '0'; const isNetworkOrTimeoutError = (error: unknown): boolean => { if (!(error instanceof Error)) { diff --git a/packages/backend/src/errors.test.ts b/packages/backend/src/errors.test.ts index c96b4f694..8e2dbbc90 100644 --- a/packages/backend/src/errors.test.ts +++ b/packages/backend/src/errors.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from 'vitest'; import { RequestError } from '@octokit/request-error'; import { GitbeakerRequestError } from '@gitbeaker/requester-utils'; -import { isForbidden, isGone, isUnauthorized } from './errors'; +import { getErrorHeader, getErrorStatus, isForbidden, isGone, isUnauthorized } from './errors'; import { throwOnHttpError } from './bitbucket'; // Helper: invoke the openapi-fetch middleware against a synthetic Response and @@ -23,6 +23,59 @@ const invokeMiddleware = async (response: Response): Promise => { } }; +describe('HTTP error metadata', () => { + test('reads status and case-insensitive headers from a direct response', () => { + const error = Object.assign(new Error('Rate limited'), { + response: { + status: 429, + headers: { + 'Retry-After': '30', + }, + }, + }); + + expect(getErrorStatus(error)).toBe(429); + expect(getErrorHeader(error, 'retry-after')).toBe('30'); + }); + + test('reads status and native Headers from a nested cause response', () => { + const error = Object.assign(new Error('Rate limited'), { + cause: { + response: new Response(null, { + status: 429, + headers: { + 'X-RateLimit-Remaining': '0', + }, + }), + }, + }); + + expect(getErrorStatus(error)).toBe(429); + expect(getErrorHeader(error, 'x-ratelimit-remaining')).toBe('0'); + }); + + test('combines a direct status with headers from the direct response', () => { + const error = Object.assign(new Error('Rate limited'), { + status: 403, + response: { + headers: { + 'x-ratelimit-remaining': '0', + }, + }, + }); + + expect(getErrorStatus(error)).toBe(403); + expect(getErrorHeader(error, 'X-RateLimit-Remaining')).toBe('0'); + }); + + test('returns no metadata for a plain error', () => { + const error = new Error('Not an HTTP error'); + + expect(getErrorStatus(error)).toBeNull(); + expect(getErrorHeader(error, 'retry-after')).toBeUndefined(); + }); +}); + describe('isUnauthorized', () => { test('Octokit RequestError with status 401', () => { const err = new RequestError('Unauthorized', 401, { diff --git a/packages/backend/src/errors.ts b/packages/backend/src/errors.ts index 5b3b108ab..56865e587 100644 --- a/packages/backend/src/errors.ts +++ b/packages/backend/src/errors.ts @@ -1,28 +1,56 @@ +type HttpErrorDetails = { + status: number | null; + headers?: unknown; +}; + /** - * 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 } } } + * Normalizes HTTP response metadata across the code-host client libraries: + * - Octokit RequestError: { status, response: { headers } } + * - openapi-fetch (Bitbucket Cloud / Server): { status } + * - gitbeaker (GitLab): { cause: { response: { status, headers } } } */ -export const getErrorStatus = (err: unknown): number | null => { - if (err === null || typeof err !== 'object') { - return null; +const getHttpErrorDetails = (error: unknown): HttpErrorDetails => { + if (error === null || typeof error !== 'object') { + return { status: null }; } - const direct = (err as { status?: unknown }).status; - if (typeof direct === 'number') { - return direct; + const directError = error as { + status?: unknown; + response?: { status?: unknown; headers?: unknown }; + cause?: { response?: { status?: unknown; headers?: unknown } }; + }; + const directResponse = directError.response; + const nestedResponse = directError.cause?.response; + const status = [ + directError.status, + directResponse?.status, + nestedResponse?.status, + ].find((value): value is number => typeof value === 'number') ?? null; + + return { + status, + headers: directResponse?.headers ?? nestedResponse?.headers, + }; +}; + +export const getErrorStatus = (error: unknown): number | null => + getHttpErrorDetails(error).status; + +export const getErrorHeader = (error: unknown, name: string): string | undefined => { + const { headers } = getHttpErrorDetails(error); + + if (headers instanceof Headers) { + return headers.get(name) ?? undefined; } - const nested = (err as { cause?: { response?: { status?: unknown } } }).cause?.response?.status; - if (typeof nested === 'number') { - return nested; + if (headers !== null && typeof headers === 'object') { + const normalizedName = name.toLowerCase(); + const entry = Object.entries(headers).find(([key]) => key.toLowerCase() === normalizedName); + return typeof entry?.[1] === 'string' ? entry[1] : undefined; } - return null; + return undefined; }; export const isUnauthorized = (err: unknown): boolean => getErrorStatus(err) === 401; From a985fac927ba1604be4a4709e152a4bd307cbb27 Mon Sep 17 00:00:00 2001 From: Brendan Kellam Date: Wed, 22 Jul 2026 14:31:41 -0400 Subject: [PATCH 4/4] fix(worker): stop treating Bitbucket Cloud 410 as removed endpoint --- .../backend/src/ee/accountPermissionSyncer.test.ts | 1 - packages/backend/src/ee/accountPermissionSyncer.ts | 7 +------ packages/backend/src/ee/permissionSyncError.test.ts | 12 +----------- packages/backend/src/ee/permissionSyncError.ts | 12 ------------ 4 files changed, 2 insertions(+), 30 deletions(-) diff --git a/packages/backend/src/ee/accountPermissionSyncer.test.ts b/packages/backend/src/ee/accountPermissionSyncer.test.ts index eee226e36..9636b34c9 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.test.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.test.ts @@ -50,7 +50,6 @@ describe('classifyPermissionSyncFailure', () => { test.each([ ['credential_rejected', 'upstream_credential_rejected'], ['insufficient_scope', 'upstream_insufficient_scope'], - ['permission_endpoint_removed', 'permission_endpoint_removed'], ] as const)('fails closed for a classified %s upstream failure', (kind, reason) => { expect(classifyPermissionSyncFailure(upstreamError(kind))).toEqual({ action: 'clear_permissions', diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index ffe622ba3..b507051d0 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -35,8 +35,7 @@ type AccountPermissionSyncJob = { export type PermissionCleanupReason = | 'oauth_invalid_grant' | 'upstream_credential_rejected' - | 'upstream_insufficient_scope' - | 'permission_endpoint_removed'; + | 'upstream_insufficient_scope'; export type PermissionCleanupDecision = | { @@ -51,7 +50,6 @@ const PERMISSION_CLEANUP_REASON_MESSAGES: Record { @@ -71,9 +69,6 @@ export const classifyPermissionSyncFailure = (error: unknown): PermissionCleanup if (error.kind === 'insufficient_scope') { return { action: 'clear_permissions', reason: 'upstream_insufficient_scope' }; } - if (error.kind === 'permission_endpoint_removed') { - return { action: 'clear_permissions', reason: 'permission_endpoint_removed' }; - } } return { action: 'preserve_permissions' }; diff --git a/packages/backend/src/ee/permissionSyncError.test.ts b/packages/backend/src/ee/permissionSyncError.test.ts index 34a4a0a53..5dffde4ac 100644 --- a/packages/backend/src/ee/permissionSyncError.test.ts +++ b/packages/backend/src/ee/permissionSyncError.test.ts @@ -51,23 +51,13 @@ describe('classifyPermissionSyncUpstreamError', () => { ).kind).toBe('rate_limited'); }); - test('classifies Bitbucket Cloud 410 from the repository-list operation as a removed endpoint', () => { + test('does not classify Bitbucket Cloud 410 as a removed permission endpoint', () => { const cause = Object.assign(new Error('Gone'), { status: 410 }); expect(classifyPermissionSyncUpstreamError( cause, 'bitbucket-cloud', 'list_accessible_repositories', - ).kind).toBe('permission_endpoint_removed'); - }); - - test('does not generalize HTTP 410 from another provider to a removed permission endpoint', () => { - const cause = Object.assign(new Error('Gone'), { status: 410 }); - - expect(classifyPermissionSyncUpstreamError( - cause, - 'github', - 'list_accessible_repositories', ).kind).toBe('unknown'); }); diff --git a/packages/backend/src/ee/permissionSyncError.ts b/packages/backend/src/ee/permissionSyncError.ts index c5694d947..cdebeb250 100644 --- a/packages/backend/src/ee/permissionSyncError.ts +++ b/packages/backend/src/ee/permissionSyncError.ts @@ -5,7 +5,6 @@ export type PermissionSyncUpstreamErrorKind = | 'credential_rejected' | 'insufficient_scope' | 'rate_limited' - | 'permission_endpoint_removed' | 'upstream_unavailable' | 'forbidden' | 'unknown'; @@ -83,17 +82,6 @@ export const classifyPermissionSyncUpstreamError = ( ); } - if ( - status === 410 && - provider === 'bitbucket-cloud' && - operation === 'list_accessible_repositories' - ) { - return new PermissionSyncUpstreamError( - 'The Bitbucket Cloud permission endpoint is no longer available.', - { kind: 'permission_endpoint_removed', provider, operation, status, cause: error }, - ); - } - if ( status === 408 || (status !== null && status >= 500 && status < 600) ||