fix(security-agent): handle invalid GitHub installations (#3440)

Author: jeanduplessis

apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts

@@ -265,6 +265,8 @@ function makeIntegration(overrides: Partial<PlatformIntegration> = {}): Platform
     repository_access: null,
     repositories: null,
     repositories_synced_at: null,
+    auth_invalid_at: null,
+    auth_invalid_reason: null,
     metadata: null,
     kilo_requester_user_id: null,
     platform_requester_account_id: null,

apps/web/src/lib/bot/platforms/github.test.ts

@@ -116,6 +116,8 @@ function createIntegration(overrides: Partial<PlatformIntegration> = {}): Platfo
     repository_access: 'all',
     repositories: null,
     repositories_synced_at: null,
+    auth_invalid_at: null,
+    auth_invalid_reason: null,
     metadata: null,
     kilo_requester_user_id: null,
     platform_requester_account_id: null,

apps/web/src/lib/integrations/db/platform-integrations.ts

@@ -127,6 +127,8 @@ export async function upsertPlatformIntegration(data: {
         repository_access: sql`EXCLUDED.repository_access`,
         integration_status: sql`EXCLUDED.integration_status`,
         repositories: sql`EXCLUDED.repositories`,
+        auth_invalid_at: null,
+        auth_invalid_reason: null,
         updated_at: sql`now()`,
       },
     });
@@ -145,6 +147,8 @@ export async function updateIntegrationRepositories(
     .set({
       repositories,
       repositories_synced_at: new Date().toISOString(),
+      auth_invalid_at: null,
+      auth_invalid_reason: null,
       updated_at: new Date().toISOString(),
     })
     .where(
@@ -167,6 +171,8 @@ export async function updateRepositoriesForIntegration(
     .set({
       repositories,
       repositories_synced_at: new Date().toISOString(),
+      auth_invalid_at: null,
+      auth_invalid_reason: null,
       updated_at: new Date().toISOString(),
     })
     .where(eq(platform_integrations.id, integrationId));
@@ -416,6 +422,8 @@ export async function autoCompleteInstallation({
       scopes: installationData.events,
       installed_at: new Date(installationData.created_at).toISOString(),
       metadata: completedMetadata,
+      auth_invalid_at: null,
+      auth_invalid_reason: null,
       updated_at: new Date().toISOString(),
     })
     .where(eq(platform_integrations.id, integrationId));
@@ -578,6 +586,8 @@ export async function upsertPlatformIntegrationForOwner(
         integration_status: INTEGRATION_STATUS.ACTIVE,
         repositories: data.repositories || null,
         github_app_type: data.githubAppType || existing.github_app_type,
+        auth_invalid_at: null,
+        auth_invalid_reason: null,
         updated_at: new Date().toISOString(),
       })
       .where(eq(platform_integrations.id, existing.id));

apps/web/src/lib/security-agent/core/types.ts

@@ -146,6 +146,12 @@ export type SyncResult = {
   errors: number;
   /** Repos where Dependabot alerts are permanently disabled (safe to skip) */
   skipped: number;
+  /** Repos where the GitHub App installation auth is invalid and needs reauthorization */
+  authInvalid: number;
+  /** Names of repos skipped because the installation needs reauthorization */
+  authInvalidRepos: string[];
+  /** True when the GitHub App installation needs user reauthorization */
+  reauthRequired: boolean;
   /** Repos that returned 404 or are access-blocked (deleted/transferred/inaccessible) */
   staleRepos: string[];
 };

apps/web/src/lib/security-agent/github/dependabot-api.test.ts

@@ -0,0 +1,66 @@
+import { beforeEach, describe, expect, it, jest } from '@jest/globals';
+
+const mockPaginate = jest.fn() as jest.MockedFunction<(...args: unknown[]) => Promise<unknown[]>>;
+const mockWarnExceptInTest = jest.fn();
+const mockErrorExceptInTest = jest.fn();
+const mockSentryLog = jest.fn();
+const mockSentryLogger = jest.fn(() => mockSentryLog);
+
+jest.mock('@octokit/rest', () => ({
+  Octokit: jest.fn().mockImplementation(() => ({
+    paginate: mockPaginate,
+    rest: {
+      dependabot: {
+        listAlertsForRepo: jest.fn(),
+      },
+    },
+  })),
+}));
+
+jest.mock('@/lib/integrations/platforms/github/adapter', () => ({
+  generateGitHubInstallationToken: jest.fn(async () => ({ token: 'token' })),
+}));
+
+jest.mock('@/lib/utils.server', () => ({
+  sentryLogger: mockSentryLogger,
+  warnExceptInTest: mockWarnExceptInTest,
+  errorExceptInTest: mockErrorExceptInTest,
+}));
+
+describe('dependabot-api', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('classifies 401 as auth_invalid and preserves existing skip classifications', async () => {
+    const { classifyFetchAlertsError } = await import('./dependabot-api');
+
+    expect(classifyFetchAlertsError(401, 'Bad credentials')).toBe('auth_invalid');
+    expect(classifyFetchAlertsError(404, 'Not Found')).toBe('repo_not_found');
+    expect(classifyFetchAlertsError(451, 'Unavailable For Legal Reasons')).toBe('access_blocked');
+    expect(classifyFetchAlertsError(403, 'Repository access blocked')).toBe('access_blocked');
+    expect(classifyFetchAlertsError(422, 'Dependabot alerts are disabled')).toBe('alerts_disabled');
+    expect(classifyFetchAlertsError(403, 'Dependabot alerts are not available')).toBe(
+      'alerts_disabled'
+    );
+  });
+
+  it('returns auth_invalid for 401 without throwing or Sentry logging', async () => {
+    const { fetchAllDependabotAlerts } = await import('./dependabot-api');
+    mockPaginate.mockRejectedValueOnce({ status: 401, message: 'Bad credentials' });
+
+    await expect(fetchAllDependabotAlerts('inst-1', 'acme', 'widgets')).resolves.toEqual({
+      status: 'auth_invalid',
+    });
+
+    expect(mockWarnExceptInTest).toHaveBeenCalledWith(
+      'GitHub App installation auth invalid for acme/widgets, skipping',
+      expect.objectContaining({ status: 401, message: 'Bad credentials' })
+    );
+    expect(mockErrorExceptInTest).not.toHaveBeenCalled();
+    expect(mockSentryLog).toHaveBeenCalledWith('Fetching alerts for acme/widgets', {
+      installationId: 'inst-1',
+    });
+    expect(mockSentryLog).toHaveBeenCalledTimes(1);
+  });
+});

apps/web/src/lib/security-agent/github/dependabot-api.ts

@@ -7,11 +7,10 @@
 import { Octokit } from '@octokit/rest';
 import { generateGitHubInstallationToken } from '@/lib/integrations/platforms/github/adapter';
 import type { DependabotAlertRaw, DependabotAlertState } from '../core/types';
-import { sentryLogger } from '@/lib/utils.server';
+import { errorExceptInTest, sentryLogger, warnExceptInTest } from '@/lib/utils.server';
 
 const log = sentryLogger('security-agent:dependabot-api', 'info');
 const warn = sentryLogger('security-agent:dependabot-api', 'warning');
-const logError = sentryLogger('security-agent:dependabot-api', 'error');
 
 /**
  * Dependabot alert from GitHub API
@@ -109,9 +108,14 @@ export type FetchAlertsResult =
   | { status: 'success'; alerts: DependabotAlertRaw[] }
   | { status: 'repo_not_found' }
   | { status: 'alerts_disabled' }
-  | { status: 'access_blocked' };
+  | { status: 'access_blocked' }
+  | { status: 'auth_invalid' };
 
-type FetchAlertsSkipStatus = 'repo_not_found' | 'alerts_disabled' | 'access_blocked';
+type FetchAlertsSkipStatus =
+  | 'repo_not_found'
+  | 'alerts_disabled'
+  | 'access_blocked'
+  | 'auth_invalid';
 
 // Permanent repo-level settings — safe to skip without blocking freshness.
 const DEPENDABOT_DISABLED_HINTS = [
@@ -134,10 +138,14 @@ function matchesAnyHint(message: string | undefined, hints: readonly string[]):
   return hints.some(hint => normalized.includes(hint));
 }
 
-function classifyFetchAlertsError(
+export function classifyFetchAlertsError(
   httpStatus?: number,
   message?: string
 ): FetchAlertsSkipStatus | null {
+  if (httpStatus === 401) {
+    return 'auth_invalid';
+  }
+
   if (httpStatus === 404) {
     return 'repo_not_found';
   }
@@ -217,6 +225,14 @@ export async function fetchAllDependabotAlerts(
     const message = (error as { message?: string }).message;
     const skipStatus = classifyFetchAlertsError(httpStatus, message);
 
+    if (skipStatus === 'auth_invalid') {
+      warnExceptInTest(`GitHub App installation auth invalid for ${owner}/${repo}, skipping`, {
+        status: httpStatus,
+        message,
+      });
+      return { status: 'auth_invalid' };
+    }
+
     if (skipStatus === 'alerts_disabled') {
       warn(`Dependabot alerts are disabled for ${owner}/${repo}, skipping`, {
         status: httpStatus,
@@ -240,7 +256,7 @@ export async function fetchAllDependabotAlerts(
       return { status: 'repo_not_found' };
     }
 
-    logError(`Error fetching alerts for ${owner}/${repo}`, {
+    errorExceptInTest(`Error fetching alerts for ${owner}/${repo}`, {
       status: httpStatus,
       message,
       durationMs: apiDurationMs,
@@ -258,19 +274,42 @@ export async function fetchOpenDependabotAlerts(
   installationId: string,
   owner: string,
   repo: string
-): Promise<DependabotAlertRaw[]> {
+): Promise<FetchAlertsResult> {
   const tokenData = await generateGitHubInstallationToken(installationId);
   const octokit = new Octokit({ auth: tokenData.token });
 
-  // Use Octokit's paginate helper which handles cursor-based pagination automatically
-  const data = await octokit.paginate(octokit.rest.dependabot.listAlertsForRepo, {
-    owner,
-    repo,
-    state: 'open',
-    per_page: 100,
-  });
+  try {
+    // Use Octokit's paginate helper which handles cursor-based pagination automatically
+    const data = await octokit.paginate(octokit.rest.dependabot.listAlertsForRepo, {
+      owner,
+      repo,
+      state: 'open',
+      per_page: 100,
+    });
 
-  return data.map(alert => toInternalAlert(alert as unknown as GitHubDependabotAlert));
+    return {
+      status: 'success',
+      alerts: data.map(alert => toInternalAlert(alert as unknown as GitHubDependabotAlert)),
+    };
+  } catch (error) {
+    const httpStatus = (error as { status?: number }).status;
+    const message = (error as { message?: string }).message;
+    const skipStatus = classifyFetchAlertsError(httpStatus, message);
+
+    if (skipStatus === 'auth_invalid') {
+      warnExceptInTest(`GitHub App installation auth invalid for ${owner}/${repo}, skipping`, {
+        status: httpStatus,
+        message,
+      });
+      return { status: 'auth_invalid' };
+    }
+
+    if (skipStatus) {
+      return { status: skipStatus };
+    }
+
+    throw error;
+  }
 }
 
 /**
@@ -294,8 +333,16 @@ export async function fetchDependabotAlert(
 
     return toInternalAlert(data as unknown as GitHubDependabotAlert);
   } catch (error) {
+    const status = (error as { status?: number }).status;
+    if (status === 401) {
+      warnExceptInTest(`GitHub App installation auth invalid for ${owner}/${repo}, skipping`, {
+        status,
+      });
+      return null;
+    }
+
     // Return null if alert not found
-    if ((error as { status?: number }).status === 404) {
+    if (status === 404) {
       return null;
     }
     throw error;
@@ -365,8 +412,15 @@ export async function isDependabotEnabled(
     });
     return true;
   } catch (error) {
-    // 403 or 404 typically means Dependabot is not enabled or no access
+    // 401 means the GitHub App installation needs reauthorization.
+    // 403 or 404 typically means Dependabot is not enabled or no access.
     const status = (error as { status?: number }).status;
+    if (status === 401) {
+      warnExceptInTest(`GitHub App installation auth invalid for ${owner}/${repo}, skipping`, {
+        status,
+      });
+      return false;
+    }
     if (status === 403 || status === 404) {
       return false;
     }

apps/web/src/lib/security-agent/router/shared-handlers.ts

@@ -128,19 +128,26 @@ export function createSecurityAgentHandlers<TExtra = {}>(deps: SecurityAgentDeps
           hasIntegration: false,
           hasPermissions: false,
           reauthorizeUrl: null,
+          authInvalidAt: integration?.auth_invalid_at ?? null,
+          authInvalidReason: integration?.auth_invalid_reason ?? null,
         };
       }
 
       const hasPermissions = hasSecurityReviewPermissions(integration);
+      // UI reauthorization state is intentionally time-invariant: once GitHub returns
+      // auth-invalid, keep prompting until a sync or install-refresh path clears the flag.
+      const hasEffectivePermissions = hasPermissions && !integration.auth_invalid_at;
 
       return {
         hasIntegration: true,
-        hasPermissions,
-        reauthorizeUrl: hasPermissions
+        hasPermissions: hasEffectivePermissions,
+        reauthorizeUrl: hasEffectivePermissions
           ? null
           : integration.platform_installation_id
             ? getReauthorizeUrl(integration.platform_installation_id)
             : null,
+        authInvalidAt: integration.auth_invalid_at,
+        authInvalidReason: integration.auth_invalid_reason,
       };
     },
 
@@ -475,6 +482,7 @@ export function createSecurityAgentHandlers<TExtra = {}>(deps: SecurityAgentDeps
                 syncResult: {
                   synced: syncResult.synced,
                   errors: syncResult.errors,
+                  reauthRequired: syncResult.reauthRequired,
                 },
               };
             }
@@ -736,6 +744,7 @@ export function createSecurityAgentHandlers<TExtra = {}>(deps: SecurityAgentDeps
             success: true,
             synced: result.synced,
             errors: result.errors,
+            reauthRequired: result.reauthRequired,
           };
         }
 
@@ -800,6 +809,7 @@ export function createSecurityAgentHandlers<TExtra = {}>(deps: SecurityAgentDeps
           success: true,
           synced: result.synced,
           errors: result.errors,
+          reauthRequired: result.reauthRequired,
         };
       },
     },