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

Author: brendan-kellam

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 when the refresh token is rejected', () => {
         expect(classifyPermissionSyncFailure(tokenRefreshError('refresh_token_rejected', 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',
         });

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_refresh_token_rejected'
-    | '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<PermissionCleanupReason, string> = {
     oauth_refresh_token_rejected: 'OAuth refresh token rejection',
-    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({

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);
+    });
+});