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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,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,
Expand Down
2 changes: 2 additions & 0 deletions apps/web/src/lib/bot/platforms/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 10 additions & 0 deletions apps/web/src/lib/integrations/db/platform-integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()`,
},
});
Expand All @@ -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(
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
6 changes: 6 additions & 0 deletions apps/web/src/lib/security-agent/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
};
Expand Down
66 changes: 66 additions & 0 deletions apps/web/src/lib/security-agent/github/dependabot-api.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
88 changes: 71 additions & 17 deletions apps/web/src/lib/security-agent/github/dependabot-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = [
Expand All @@ -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';
}
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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;
}
}

/**
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
14 changes: 12 additions & 2 deletions apps/web/src/lib/security-agent/router/shared-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment thread
jeanduplessis marked this conversation as resolved.

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,
};
},

Expand Down Expand Up @@ -475,6 +482,7 @@ export function createSecurityAgentHandlers<TExtra = {}>(deps: SecurityAgentDeps
syncResult: {
synced: syncResult.synced,
errors: syncResult.errors,
reauthRequired: syncResult.reauthRequired,
},
};
}
Expand Down Expand Up @@ -736,6 +744,7 @@ export function createSecurityAgentHandlers<TExtra = {}>(deps: SecurityAgentDeps
success: true,
synced: result.synced,
errors: result.errors,
reauthRequired: result.reauthRequired,
};
}

Expand Down Expand Up @@ -800,6 +809,7 @@ export function createSecurityAgentHandlers<TExtra = {}>(deps: SecurityAgentDeps
success: true,
synced: result.synced,
errors: result.errors,
reauthRequired: result.reauthRequired,
};
},
},
Expand Down
Loading