Skip to content
Merged
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 @@ -20,12 +20,14 @@ export type RepositoryMultiSelectProps<TId extends RepositoryId = number> = {
repositories: Repository<TId>[];
selectedIds: TId[];
onSelectionChange: (selectedIds: TId[]) => void;
renderRepositoryAccessory?: (repository: Repository<TId>) => React.ReactNode;
};

export function RepositoryMultiSelect<TId extends RepositoryId = number>({
repositories,
selectedIds,
onSelectionChange,
renderRepositoryAccessory,
}: RepositoryMultiSelectProps<TId>) {
const [searchQuery, setSearchQuery] = useState('');

Expand Down Expand Up @@ -116,14 +118,15 @@ export function RepositoryMultiSelect<TId extends RepositoryId = number>({
/>
<label
htmlFor={`repo-${repo.id}`}
className="flex flex-1 cursor-pointer items-center gap-2 text-sm"
className="flex min-w-0 flex-1 cursor-pointer items-center gap-2 text-sm"
>
{repo.private ? (
<Lock className="text-primary h-3.5 w-3.5 shrink-0" />
) : (
<Unlock className="text-muted-foreground h-3.5 w-3.5 shrink-0" />
)}
<span className="truncate font-mono">{repo.full_name}</span>
{renderRepositoryAccessory?.(repo)}
</label>
</div>
);
Expand Down
17 changes: 15 additions & 2 deletions apps/web/src/components/security-agent/SecurityAgentContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type { SecurityFinding } from '@kilocode/db/schema';
import type { SecurityRemediationAdmissionRejectionReason } from '@kilocode/worker-utils/security-remediation-policy';
import { getSecurityCommandFailureMessage } from '@kilocode/app-shared/security-agent';
import type { SecurityAgentUiInteraction } from '@/lib/security-agent/core/schemas';
import type { DependabotAlertsAvailability } from '@/lib/security-agent/core/types';
import { isGitHubIntegrationError } from '@/lib/security-agent/core/error-display';
import type { DismissReason } from './DismissFindingDialog';
import { getRemediationUnavailableCopy } from './remediation-unavailable-copy';
Expand Down Expand Up @@ -76,8 +77,20 @@ type SecurityAgentContextValue = {
refetchConfig: () => Promise<unknown>;

// Repositories
allRepositories: Array<{ id: number; fullName: string; name: string; private: boolean }>;
filteredRepositories: Array<{ id: number; fullName: string; name: string; private: boolean }>;
allRepositories: Array<{
id: number;
fullName: string;
name: string;
private: boolean;
dependabotAlerts: DependabotAlertsAvailability;
}>;
filteredRepositories: Array<{
id: number;
fullName: string;
name: string;
private: boolean;
dependabotAlerts: DependabotAlertsAvailability;
}>;

// Mutation handlers
trackUiInteraction: (interaction: SecurityAgentUiInteraction) => void;
Expand Down
52 changes: 52 additions & 0 deletions apps/web/src/components/security-agent/SecurityConfigSections.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
} from 'lucide-react';
import { RepositoryMultiSelect } from '@/components/code-reviews/RepositoryMultiSelect';
import { ModelCombobox } from '@/components/shared/ModelCombobox';
import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert';
import { Badge } from '@/components/ui/badge';
import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card';
import { Input } from '@/components/ui/input';
import { Label } from '@/components/ui/label';
Expand Down Expand Up @@ -334,6 +336,21 @@ export function RepositorySection({
repositories: SecurityRepository[];
isLoading?: boolean;
}) {
const dependabotAlertsByRepositoryId = new Map(
repositories.map(repository => [repository.id, repository.dependabotAlerts])
);
const disabledRepositories = repositories.filter(
repository => repository.dependabotAlerts === 'disabled'
);
const selectedRepositoryIds = new Set(state.selectedRepositoryIds);
const selectedDisabledRepositories =
state.repositorySelectionMode === 'all'
? disabledRepositories
: disabledRepositories.filter(repository => selectedRepositoryIds.has(repository.id));
const displayedDisabledRepositories = selectedDisabledRepositories.slice(0, 5);
const additionalDisabledRepositoryCount =
selectedDisabledRepositories.length - displayedDisabledRepositories.length;

return (
<Card>
<SectionHeader
Expand Down Expand Up @@ -375,6 +392,27 @@ export function RepositorySection({
</Label>
</div>
</RadioGroup>
{selectedDisabledRepositories.length > 0 && (
<Alert variant="warning">
<AlertTriangle aria-hidden="true" />
<AlertTitle>
Dependabot alerts disabled for {selectedDisabledRepositories.length}{' '}
{selectedDisabledRepositories.length === 1 ? 'repository' : 'repositories'}
</AlertTitle>
<AlertDescription>
<p>
Security Agent cannot import findings from{' '}
Comment thread
jeanduplessis marked this conversation as resolved.
Resolved
{displayedDisabledRepositories
.map(repository => repository.fullName)
.join(', ')}
{additionalDisabledRepositoryCount > 0
? `, and ${additionalDisabledRepositoryCount} more`
: ''}{' '}
until Dependabot alerts are enabled in GitHub repository settings.
</p>
</AlertDescription>
</Alert>
)}
{state.repositorySelectionMode === 'selected' && (
<div className="space-y-2">
<Label>Repositories</Label>
Expand All @@ -384,6 +422,20 @@ export function RepositorySection({
onSelectionChange={selectedRepositoryIds =>
setState(current => ({ ...current, selectedRepositoryIds }))
}
renderRepositoryAccessory={repository => {
if (dependabotAlertsByRepositoryId.get(repository.id) !== 'disabled') {
return null;
}

return (
<Badge
variant="outline"
className="border-status-warning-border bg-status-warning-surface text-status-warning ml-auto shrink-0"
>
Dependabot alerts off
</Badge>
);
}}
/>
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Repository } from '@/components/code-reviews/RepositoryMultiSelect';
import type { DependabotAlertsAvailability } from '@/lib/security-agent/core/types';

export type SlaConfig = {
critical: number;
Expand All @@ -19,6 +20,7 @@ export type SecurityRepository = {
fullName: string;
name: string;
private: boolean;
dependabotAlerts: DependabotAlertsAvailability;
};

export type SecurityConfigFormState = {
Expand Down
1 change: 1 addition & 0 deletions apps/web/src/lib/security-agent/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export type SecurityFindingAnalysisStatus =
(typeof SecurityFindingAnalysisStatus)[keyof typeof SecurityFindingAnalysisStatus];

export type AnalysisMode = 'auto' | 'shallow' | 'deep';
export type DependabotAlertsAvailability = 'enabled' | 'disabled' | 'unknown';

export type AutoAnalysisMinSeverity = 'critical' | 'high' | 'medium' | 'all';
export type AutoRemediationMinSeverity = 'critical' | 'high' | 'medium' | 'all';
Expand Down
81 changes: 79 additions & 2 deletions apps/web/src/lib/security-agent/github/dependabot-api.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { beforeEach, describe, expect, it, jest } from '@jest/globals';

const mockPaginate = jest.fn() as jest.MockedFunction<(...args: unknown[]) => Promise<unknown[]>>;
const mockListAlertsForRepo = jest.fn() as jest.MockedFunction<
(...args: unknown[]) => Promise<unknown>
>;
const mockGenerateGitHubInstallationToken = jest.fn(async () => ({ token: 'token' }));
const mockWarnExceptInTest = jest.fn();
const mockErrorExceptInTest = jest.fn();
const mockSentryLog = jest.fn();
Expand All @@ -11,14 +15,14 @@ jest.mock('@octokit/rest', () => ({
paginate: mockPaginate,
rest: {
dependabot: {
listAlertsForRepo: jest.fn(),
listAlertsForRepo: mockListAlertsForRepo,
},
},
})),
}));

jest.mock('@/lib/integrations/platforms/github/adapter', () => ({
generateGitHubInstallationToken: jest.fn(async () => ({ token: 'token' })),
generateGitHubInstallationToken: mockGenerateGitHubInstallationToken,
}));

jest.mock('@/lib/utils.server', () => ({
Expand Down Expand Up @@ -63,4 +67,77 @@ describe('dependabot-api', () => {
});
expect(mockSentryLog).toHaveBeenCalledTimes(1);
});

it('reports enabled only when the Dependabot alerts endpoint succeeds', async () => {
const { checkDependabotAlertsAvailability } = await import('./dependabot-api');
mockListAlertsForRepo.mockResolvedValueOnce({ data: [] });
mockListAlertsForRepo.mockRejectedValueOnce({
status: 422,
message: 'Dependabot alerts are disabled for this repository',
});
mockListAlertsForRepo.mockRejectedValueOnce({ status: 403, message: 'Forbidden' });

await expect(
checkDependabotAlertsAvailability('inst-1', 'standard', [
{ id: 1, fullName: 'acme/enabled' },
{ id: 2, fullName: 'acme/disabled' },
{ id: 3, fullName: 'acme/unknown' },
])
).resolves.toEqual([
{ id: 1, status: 'enabled' },
{ id: 2, status: 'disabled' },
{ id: 3, status: 'unknown' },
]);

expect(mockListAlertsForRepo).toHaveBeenCalledWith({
owner: 'acme',
repo: 'enabled',
state: 'open',
per_page: 1,
});
});

it('reuses a recent repository result without another GitHub request', async () => {
const { checkDependabotAlertsAvailability } = await import('./dependabot-api');
mockListAlertsForRepo.mockResolvedValue({ data: [] });
const repositories = [{ id: 1, fullName: 'cache-test/widgets' }];

await expect(
checkDependabotAlertsAvailability('cache-installation', 'standard', repositories)
).resolves.toEqual([{ id: 1, status: 'enabled' }]);
await expect(
checkDependabotAlertsAvailability('cache-installation', 'standard', repositories)
).resolves.toEqual([{ id: 1, status: 'enabled' }]);

expect(mockGenerateGitHubInstallationToken).toHaveBeenCalledTimes(1);
expect(mockListAlertsForRepo).toHaveBeenCalledTimes(1);
});

it('keeps every repository unknown when GitHub authentication fails', async () => {
const { checkDependabotAlertsAvailability } = await import('./dependabot-api');
mockGenerateGitHubInstallationToken.mockRejectedValueOnce(new Error('auth unavailable'));

await expect(
checkDependabotAlertsAvailability('auth-failure-installation', 'standard', [
{ id: 1, fullName: 'auth-failure/widgets' },
])
).resolves.toEqual([{ id: 1, status: 'unknown' }]);

expect(mockListAlertsForRepo).not.toHaveBeenCalled();
});

it('isolates malformed repository names instead of failing the whole check', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: This test does not cover the new try/catch it appears to target

With fullName: undefined, dependabotAvailabilityCacheKey returns null, so checkRepositoryDependabotAlertsAvailability returns 'unknown' at the guard (dependabot-api.ts:282) and never reaches fetchRepositoryDependabotAlertsAvailability. Nothing throws, so the new catch block that maps an unexpected rejection to 'unknown' (dependabot-api.ts:249) stays uncovered.

Adding a case where the per-repository check rejects unexpectedly (for example mockListAlertsForRepo.mockImplementationOnce(() => { throw new Error('boom'); }) with a well-formed name, or an Octokit constructor that throws) would exercise the isolation path this commit adds.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

const { checkDependabotAlertsAvailability } = await import('./dependabot-api');
mockListAlertsForRepo.mockResolvedValueOnce({ data: [] });

await expect(
checkDependabotAlertsAvailability('malformed-installation', 'standard', [
{ id: 1, fullName: undefined as never },
{ id: 2, fullName: 'acme/valid' },
])
).resolves.toEqual([
{ id: 1, status: 'unknown' },
{ id: 2, status: 'enabled' },
]);
});
});
Loading