diff --git a/packages/repocop/src/index.ts b/packages/repocop/src/index.ts index 1c1ba3e18..aedf2b3b4 100644 --- a/packages/repocop/src/index.ts +++ b/packages/repocop/src/index.ts @@ -26,9 +26,12 @@ import { sendPotentialInteractives } from './remediations/topics/topic-monitor-i import { applyProductionTopicAndMessageTeams } from './remediations/topics/topic-monitor-production'; import { evaluateRepositories, + getAlertsForRepo, + hasOldAlerts, testExperimentalRepocopFeatures, } from './rules/repository'; -import type { AwsCloudFormationStack } from './types'; +import type { AwsCloudFormationStack, RepoAndAlerts } from './types'; +import { isProduction } from './utils'; async function writeEvaluationTable( evaluatedRepos: repocop_github_repository_rules[], @@ -63,8 +66,33 @@ export async function main() { await getStacks(prisma) ).filter((s) => s.tags.Stack !== 'playground'); const snykProjects = await getSnykProjects(prisma); + + const prodRepos = unarchivedRepos.filter((repo) => isProduction(repo)); + const alerts: RepoAndAlerts[] = ( + await Promise.all( + prodRepos.map(async (repo) => { + return { + shortName: repo.full_name, + alerts: await getAlertsForRepo(octokit, repo.name), + }; + }), + ) + ).filter((x) => !!x.alerts); + + alerts.forEach((alert) => { + if (alert.alerts && alert.alerts.length > 0) { + console.log( + `Found ${alert.alerts.length} alerts for ${alert.shortName}: `, + ); + hasOldAlerts(alert.alerts, alert.shortName); + } + }); + + console.log(`Found ${alerts.length} repos with alerts`); + const evaluatedRepos: repocop_github_repository_rules[] = evaluateRepositories( + alerts, unarchivedRepos, branches, repoTeams, @@ -77,8 +105,7 @@ export async function main() { const cloudwatch = new CloudWatchClient(awsConfig); await sendToCloudwatch(evaluatedRepos, cloudwatch, config); - await testExperimentalRepocopFeatures( - octokit, + testExperimentalRepocopFeatures( evaluatedRepos, unarchivedRepos, archivedRepos, diff --git a/packages/repocop/src/rules/repository.test.ts b/packages/repocop/src/rules/repository.test.ts index b90d47186..12d9f4f98 100644 --- a/packages/repocop/src/rules/repository.test.ts +++ b/packages/repocop/src/rules/repository.test.ts @@ -6,6 +6,7 @@ import type { } from '@prisma/client'; import type { AwsCloudFormationStack, + PartialAlert, Repository, TeamRepository, } from '../types'; @@ -13,6 +14,7 @@ import { evaluateOneRepo, findStacks, hasDependencyTracking, + hasOldAlerts, parseSnykTags, } from './repository'; @@ -23,8 +25,10 @@ function evaluateRepoTestHelper( languages: github_languages[] = [], snykProjects: snyk_projects[] = [], githubWorkflows: github_workflows[] = [], + alerts: PartialAlert[] = [], ) { return evaluateOneRepo( + alerts, repo, branches, teams, @@ -69,7 +73,7 @@ const thePerfectRepo: Repository = { default_branch: 'main', }; -describe('default_branch_name should be false when the default branch is not main', () => { +describe('REPOSITORY_01 - default_branch_name should be false when the default branch is not main', () => { test('branch is not main', () => { const badRepo = { ...thePerfectRepo, default_branch: 'notMain' }; const repos: Repository[] = [thePerfectRepo, badRepo]; @@ -82,7 +86,7 @@ describe('default_branch_name should be false when the default branch is not mai }); }); -describe('Repositories should have branch protection', () => { +describe('REPOSITORY_02 - Repositories should have branch protection', () => { const unprotectedMainBranch: github_repository_branches = { ...nullBranch, repository_id: BigInt(1), @@ -134,7 +138,7 @@ describe('Repositories should have branch protection', () => { }); }); -describe('Repository admin access', () => { +describe('REPOSITORY_04 - Repository admin access', () => { test('Should return false when there is no admin team', () => { const repo: Repository = { ...nullRepo, @@ -228,7 +232,7 @@ describe('Repository admin access', () => { }); }); -describe('Repository topics', () => { +describe('REPOSITORY_06 - Repository topics', () => { test('Should return true when there is a single recognised topic', () => { const repo: Repository = { ...nullRepo, @@ -294,7 +298,8 @@ describe('Repository topics', () => { }); }); -describe('Repository maintenance', () => { +// No rule for this evaluation yet +describe('NO RULE - Repository maintenance', () => { test('should have happened at some point in the last two years', () => { const recentRepo: Repository = { ...nullRepo, @@ -334,7 +339,7 @@ describe('Repository maintenance', () => { }); }); -describe('Repositories with related stacks on AWS', () => { +describe('REPOSITORY_08 - Repositories with related stacks on AWS', () => { test('should be findable if a stack has a matching tag', () => { const full_name = 'guardian/repo1'; const tags = { @@ -372,7 +377,7 @@ describe('Repositories with related stacks on AWS', () => { }); }); -describe('Repositories without any related stacks on AWS', () => { +describe('REPOSITORY_08 - Repositories without any related stacks on AWS', () => { test('should not be findable', () => { const repo: Repository = { ...nullRepo, @@ -407,7 +412,7 @@ describe('Repositories without any related stacks on AWS', () => { }); }); -describe('Snyk tags', () => { +describe('REPOSITORY_09 - Snyk tags', () => { const nullSnykProject: snyk_projects = { cq_source_name: null, cq_sync_time: null, @@ -447,7 +452,7 @@ describe('Snyk tags', () => { }); }); -describe('Dependency tracking', () => { +describe('REPOSITORY_09 - Dependency tracking', () => { const nullSnykProject: snyk_projects = { cq_source_name: null, cq_sync_time: null, @@ -643,3 +648,71 @@ describe('Dependency tracking', () => { expect(actual).toEqual(false); }); }); + +function createAlert( + severity: 'critical' | 'high' | 'medium', + createdAt: Date, + state: 'open' | 'auto_dismissed' | 'dismissed' | 'fixed', +): PartialAlert { + const alert = { + state, + created_at: createdAt.toISOString(), + security_vulnerability: { + severity, + vulnerable_version_range: '', + first_patched_version: { + identifier: '', + }, + package: { + name: '', + ecosystem: '', + }, + }, + dependency: { + package: { + name: '', + ecosystem: '', + }, + }, + }; + return alert; +} + +describe('NO RULE - Repository alerts', () => { + test('should be flagged if there are critical alerts older than one day', () => { + console.log(new Date('2021-01-01').toISOString()); + const alerts: PartialAlert[] = [ + createAlert('critical', new Date('2021-01-01'), 'open'), + ]; + + expect(hasOldAlerts(alerts, 'test')).toBe(true); + }); + test('should not be flagged if a critical alert was raised today', () => { + const alerts: PartialAlert[] = [ + createAlert('critical', new Date(), 'open'), + ]; + + expect(hasOldAlerts(alerts, 'test')).toBe(false); + }); + test('should be flagged if there are high alerts older than 14 days', () => { + const alerts: PartialAlert[] = [ + createAlert('high', new Date('2021-01-01'), 'open'), + ]; + + expect(hasOldAlerts(alerts, 'test')).toBe(true); + }); + test('should not be flagged if a high alert was raised today', () => { + const alerts: PartialAlert[] = [createAlert('high', new Date(), 'open')]; + + expect(hasOldAlerts(alerts, 'test')).toBe(false); + }); + test('should not be flagged if a high alert was raised 13 days ago', () => { + const thirteenDaysAgo = new Date(); + thirteenDaysAgo.setDate(thirteenDaysAgo.getDate() - 13); + const alerts: PartialAlert[] = [ + createAlert('high', thirteenDaysAgo, 'open'), + ]; + + expect(hasOldAlerts(alerts, 'test')).toBe(false); + }); +}); diff --git a/packages/repocop/src/rules/repository.ts b/packages/repocop/src/rules/repository.ts index 58328cafc..1e8649f68 100644 --- a/packages/repocop/src/rules/repository.ts +++ b/packages/repocop/src/rules/repository.ts @@ -5,15 +5,16 @@ import type { repocop_github_repository_rules, snyk_projects, } from '@prisma/client'; -import { shuffle } from 'common/src/functions'; import type { Octokit } from 'octokit'; import { supportedDependabotLanguages, supportedSnykLanguages, } from '../languages'; import type { - Alert, AwsCloudFormationStack, + DependabotVulnResponse, + PartialAlert, + RepoAndAlerts, RepoAndStack, Repository, TeamRepository, @@ -269,49 +270,78 @@ function findArchivedReposWithStacks( export async function getAlertsForRepo( octokit: Octokit, name: string, -): Promise { +): Promise { if (name.startsWith('guardian/')) { name = name.replace('guardian/', ''); } try { - const alert = await octokit.rest.dependabot.listAlertsForRepo({ - owner: 'guardian', - repo: name, - per_page: 100, - severity: 'critical', //eventually this should be "critical,high" - state: 'open', - }); - - return alert.data; + const alert: DependabotVulnResponse = + await octokit.rest.dependabot.listAlertsForRepo({ + owner: 'guardian', + repo: name, + per_page: 100, + severity: 'critical,high', + state: 'open', + sort: 'created', + direction: 'asc', //retrieve oldest vulnerabilities first + }); + + return alert.data as PartialAlert[]; } catch (error) { - console.error(`Error: could not get alerts for ${name}`); - console.error(error); return undefined; } } -export async function testExperimentalRepocopFeatures( - octokit: Octokit, +function isOldForSeverity( + date: Date, + severity: 'critical' | 'high', + alert: PartialAlert, +) { + const alertDate = new Date(alert.created_at); + return alertDate < date && alert.security_vulnerability.severity === severity; +} + +export function hasOldAlerts(alerts: PartialAlert[], repo: string): boolean { + const highDayCount = 14; + const criticalDayCount = 1; + + const highVulnCutOff = new Date(); + highVulnCutOff.setDate(highVulnCutOff.getDate() - highDayCount); + highVulnCutOff.setHours(0, 0, 0, 0); + + const criticalVulnCutOff = new Date(); + criticalVulnCutOff.setDate(criticalVulnCutOff.getDate() - criticalDayCount); + criticalVulnCutOff.setHours(0, 0, 0, 0); + const oldHighAlerts = alerts.filter((alert) => + isOldForSeverity(highVulnCutOff, 'high', alert), + ); + const oldCriticalAlerts = alerts.filter((alert) => + isOldForSeverity(criticalVulnCutOff, 'critical', alert), + ); + if (oldCriticalAlerts.length > 0) { + console.log( + `Dependabot - ${repo}: has ${oldCriticalAlerts.length} critical alerts older than ${criticalDayCount} days`, + ); + } + if (oldHighAlerts.length > 0) { + console.log( + `Dependabot - ${repo}: has ${oldHighAlerts.length} high alerts older than ${highDayCount} weeks`, + ); + } + if (oldCriticalAlerts.length === 0 && oldHighAlerts.length === 0) { + console.log(`Dependabot - ${repo}: has no old alerts`); + } + + return oldHighAlerts.length > 0 || oldCriticalAlerts.length > 0; +} + +export function testExperimentalRepocopFeatures( evaluatedRepos: repocop_github_repository_rules[], unarchivedRepos: Repository[], archivedRepos: Repository[], nonPlaygroundStacks: AwsCloudFormationStack[], ) { - const prodRepos = unarchivedRepos.filter((repo) => - repo.topics.includes('production'), - ); - - await Promise.all( - shuffle(prodRepos) - .slice(0, 10) - .map(async (repo) => { - console.log(`Getting alerts for ${repo.full_name}`); - const alerts = await getAlertsForRepo(octokit, repo.full_name); - console.log(repo.full_name, alerts); - }), - ); - const unmaintinedReposCount = evaluatedRepos.filter( (repo) => repo.archiving === false, ).length; @@ -329,8 +359,8 @@ export async function testExperimentalRepocopFeatures( console.log(`Found ${archivedWithStacks.length} archived repos with stacks.`); console.log( - 'Archived repos with live stacks, first 10 results:', - archivedWithStacks.slice(0, 10), + 'Archived repos with live stacks, first 3 results:', + archivedWithStacks.slice(0, 3), ); } @@ -338,6 +368,7 @@ export async function testExperimentalRepocopFeatures( * Apply rules to a repository as defined in https://github.com/guardian/recommendations/blob/main/best-practices.md. */ export function evaluateOneRepo( + alerts: PartialAlert[] | undefined, repo: Repository, allBranches: github_repository_branches[], teams: TeamRepository[], @@ -345,14 +376,10 @@ export function evaluateOneRepo( snykProjects: snyk_projects[], workflowFiles: github_workflows[], ): repocop_github_repository_rules { - /* - Either the fullname, or the org and name, or the org and 'unknown'. - The latter should never happen, it's just how the types have been defined. - */ - const fullName = repo.full_name; + alerts = undefined; return { - full_name: fullName, + full_name: repo.full_name, default_branch_name: hasDefaultBranchNameMain(repo), branch_protection: hasBranchProtection(repo, allBranches), team_based_access: false, @@ -371,6 +398,7 @@ export function evaluateOneRepo( } export function evaluateRepositories( + alerts: RepoAndAlerts[], repositories: Repository[], branches: github_repository_branches[], teams: TeamRepository[], @@ -378,10 +406,12 @@ export function evaluateRepositories( snykProjects: snyk_projects[], workflowFiles: github_workflows[], ): repocop_github_repository_rules[] { - return repositories.map((r) => { + const evaluatedRepos = repositories.map((r) => { const teamsForRepo = teams.filter((t) => t.id === r.id); const branchesForRepo = branches.filter((b) => b.repository_id === r.id); + const alertsForRepo = alerts.find((a) => a.shortName === r.name); return evaluateOneRepo( + alertsForRepo?.alerts, r, branchesForRepo, teamsForRepo, @@ -390,4 +420,5 @@ export function evaluateRepositories( workflowFiles, ); }); + return evaluatedRepos; } diff --git a/packages/repocop/src/types.ts b/packages/repocop/src/types.ts index 3e05e1e0b..1aaa07f74 100644 --- a/packages/repocop/src/types.ts +++ b/packages/repocop/src/types.ts @@ -63,7 +63,20 @@ export interface TeamRepository extends TeamRepositoryFields { role_name: NonNullable; } -type DependabotVulnResponse = +export type DependabotVulnResponse = Endpoints['GET /repos/{owner}/{repo}/dependabot/alerts']['response']; export type Alert = DependabotVulnResponse['data'][number]; + +export type PartialAlert = Pick< + Alert, + 'state' | 'dependency' | 'security_vulnerability' | 'created_at' +>; + +export interface RepoAndAlerts { + shortName: string; + /* + ** alerts is undefined if we catch an error, typically because dependabot is not enabled + */ + alerts: PartialAlert[] | undefined; +} diff --git a/packages/repocop/src/utils.test.ts b/packages/repocop/src/utils.test.ts new file mode 100644 index 000000000..e5b0bb1d1 --- /dev/null +++ b/packages/repocop/src/utils.test.ts @@ -0,0 +1,25 @@ +import type { Repository } from './types'; +import { isProduction } from './utils'; + +describe('isProduction', () => { + test('should return correct values for prod and non-prod repos', () => { + const prodRepo: Repository = { + archived: false, + full_name: 'test', + id: 1n, + name: 'test', + topics: ['production'], + default_branch: 'main', + created_at: new Date(), + pushed_at: new Date(), + updated_at: new Date(), + }; + const nonProdRepo: Repository = { + ...prodRepo, + topics: [], + }; + + expect(isProduction(prodRepo)).toBe(true); + expect(isProduction(nonProdRepo)).toBe(false); + }); +}); diff --git a/packages/repocop/src/utils.ts b/packages/repocop/src/utils.ts new file mode 100644 index 000000000..4fde3344f --- /dev/null +++ b/packages/repocop/src/utils.ts @@ -0,0 +1,5 @@ +import type { Repository } from './types'; + +export function isProduction(repo: Repository) { + return repo.topics.includes('production') && !repo.archived; +}