Skip to content

Add initial support to checklists #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Dec 24, 2024
1 change: 0 additions & 1 deletion .github/ISSUE_TEMPLATE/add-a-new-compliance-check.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ You can find more details in [the contributing guide](/CONTRIBUTING.md#current-i
- [ ] **3. Implement the Business Logic [Validator Example](https://github.com/OpenPathfinder/visionBoard/commit/44c41d119f0daefb7b2e496ba35d5ab65bcc319b) and [Check Example](https://github.com/OpenPathfinder/visionBoard/commit/6f1e16129ee0d01a1b9b536cd2dc6090b048b71f)**
- [ ] Add the specific validator in `src/checks/validators/index.js`
- [ ] Add the check logic in `src/checks/complianceChecks`
- [ ] Ensure that the check is in scope for the organization (use `isCheckApplicableToProjectCategory`)
- [ ] Ensure that the `severity` value is well calculated (use `getSeverityFromPriorityGroup`)
- [ ] Add the alert row in the `compliance_checks_alerts` table when is needed.
- [ ] Add the task row in the `compliance_checks_tasks` table when is needed.
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/review-compliance-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:
"- [ ] Have you updated the compliance check in the `compliance_checks` table?\n" +
"- [ ] Have you included a specific validator (`src/checks/validators/`) for this check with unit tests (`__tests__/checks/`)?\n" +
"- [ ] Have you included a specific file in `src/checks/complianceChecks` with the integration tests (`__tests__/checks/`)?\n" +
"- [ ] Have you included severity validation (`getSeverityFromPriorityGroup`) and checked applicability (`isCheckApplicableToProjectCategory`)?\n" +
"- [ ] Have you included severity validation (`getSeverityFromPriorityGroup`)?\n" +
"- [ ] Have you included the tasks, alerts, and results in the database tables?\n" +
"- [ ] Have you tested the check with `check run --name {check_code_name}` using the seeded database (`npm run db:seed`)?\n" +
"- [ ] Have you created a PR in [the website](https://github.com/OpenPathfinder/website) with the calculation details?\n" +
Expand Down
1 change: 0 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ We are looking for contributors to implement compliance checks in the Dashboard.
- **3. Implement the Business Logic ([Validator Example](https://github.com/OpenPathfinder/visionBoard/commit/44c41d119f0daefb7b2e496ba35d5ab65bcc319b) and [Check Example](https://github.com/OpenPathfinder/visionBoard/commit/6f1e16129ee0d01a1b9b536cd2dc6090b048b71f)):**
- Add the specific validator in `src/checks/validators/index.js`.
- Add the check logic in `src/checks/complianceChecks`.
- Ensure the check is applicable to the organization (`isCheckApplicableToProjectCategory`).
- Calculate `severity` accurately (`getSeverityFromPriorityGroup`).
- Update relevant database tables (`compliance_checks_alerts`, `compliance_checks_tasks`, `compliance_checks_results`).

Expand Down
37 changes: 4 additions & 33 deletions __tests__/checks/validators.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,8 @@ describe('githubOrgMFA', () => {

check = {
id: 1,
priority_group: 'P1',
details_url: 'https://example.com',
level_incubating_status: 'expected',
level_active_status: 'expected',
level_retiring_status: 'expected'
default_priority_group: 'P1',
details_url: 'https://example.com'
}

projects = [
Expand Down Expand Up @@ -133,18 +130,6 @@ describe('githubOrgMFA', () => {
tasks: []
})
})

it('Should skip the check if it is not in scope for the project category', () => {
check.level_active_status = 'n/a'
check.level_incubating_status = 'n/a'
check.level_retiring_status = 'n/a'
const analysis = githubOrgMFA({ organizations, check, projects })
expect(analysis).toEqual({
alerts: [],
results: [],
tasks: []
})
})
})

describe('softwareDesignTraining', () => {
Expand All @@ -163,11 +148,8 @@ describe('softwareDesignTraining', () => {

check = {
id: 1,
priority_group: 'P1',
details_url: 'https://example.com',
level_incubating_status: 'expected',
level_active_status: 'expected',
level_retiring_status: 'expected'
default_priority_group: 'P1',
details_url: 'https://example.com'
}

projects = [
Expand Down Expand Up @@ -299,15 +281,4 @@ describe('softwareDesignTraining', () => {
]
})
})
it('Should skip the check if it is not in scope for the project category', () => {
check.level_active_status = 'n/a'
check.level_incubating_status = 'n/a'
check.level_retiring_status = 'n/a'
const analysis = softwareDesignTraining({ trainings, check, projects })
expect(analysis).toEqual({
alerts: [],
results: [],
tasks: []
})
})
})
27 changes: 1 addition & 26 deletions __tests__/utils.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { validateGithubUrl, ensureGithubToken, groupArrayItemsByCriteria, isCheckApplicableToProjectCategory, getSeverityFromPriorityGroup, isDateWithinPolicy, redactSensitiveData } = require('../src/utils/index')
const { validateGithubUrl, ensureGithubToken, groupArrayItemsByCriteria, getSeverityFromPriorityGroup, isDateWithinPolicy, redactSensitiveData } = require('../src/utils/index')

describe('ensureGithubToken', () => {
let originalGithubToken
Expand Down Expand Up @@ -66,31 +66,6 @@ describe('groupArrayItemsByCriteria', () => {
})
})

describe('isCheckApplicableToProjectCategory', () => {
const disabledCheck = {
level_active_status: 'n/a',
level_incubating_status: 'n/a',
level_retiring_status: 'n/a'
}

it('should return false if the check is not applicable to the project category', () => {
let project = { category: 'impact' }
expect(isCheckApplicableToProjectCategory(disabledCheck, project)).toBe(false)
project = { category: 'incubation' }
expect(isCheckApplicableToProjectCategory(disabledCheck, project)).toBe(false)
project = { category: 'emeritus' }
expect(isCheckApplicableToProjectCategory(disabledCheck, project)).toBe(false)
project = { category: 'at-large' }
expect(isCheckApplicableToProjectCategory(disabledCheck, project)).toBe(false)
})

it('should return true if the check is applicable to the project category', () => {
const project = { category: 'impact' }
const check = { ...disabledCheck, level_active_status: 'recommended' }
expect(isCheckApplicableToProjectCategory(check, project)).toBe(true)
})
})

describe('getSeverityFromPriorityGroup', () => {
it('should return the correct severity based on the priority group', () => {
expect(getSeverityFromPriorityGroup('P0')).toBe('critical')
Expand Down
10 changes: 2 additions & 8 deletions src/checks/validators/githubOrgMFA.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const debug = require('debug')('checks:validator:githubOrgMFA')
const { getSeverityFromPriorityGroup, isCheckApplicableToProjectCategory, groupArrayItemsByCriteria } = require('../../utils')
const { getSeverityFromPriorityGroup, groupArrayItemsByCriteria } = require('../../utils')

const groupByProject = groupArrayItemsByCriteria('project_id')

Expand All @@ -17,17 +17,11 @@ module.exports = ({ organizations = [], check, projects = [] }) => {
organizationsGroupedByProject.forEach((projectOrgs) => {
debug(`Processing project (${projectOrgs[0].project_id})`)
const project = projects.find(p => p.id === projectOrgs[0].project_id)
const isInScope = isCheckApplicableToProjectCategory(check, project)
// If the check is not in scope, skip it.
if (!isInScope) {
debug(`This check is not in scope for project (${project.id})`)
return
}

const baseData = {
project_id: projectOrgs[0].project_id,
compliance_check_id: check.id,
severity: getSeverityFromPriorityGroup(check.priority_group)
severity: getSeverityFromPriorityGroup(check.default_priority_group)
}

const result = { ...baseData }
Expand Down
11 changes: 2 additions & 9 deletions src/checks/validators/softwareDesignTraining.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const debug = require('debug')('checks:validator:softwareDesignTraining')
const { isCheckApplicableToProjectCategory, getSeverityFromPriorityGroup, isDateWithinPolicy } = require('../../utils')
const { getSeverityFromPriorityGroup, isDateWithinPolicy } = require('../../utils')

const expirationPolicy = '6m'

Expand All @@ -11,17 +11,10 @@ module.exports = ({ trainings = [], check, projects = [] }) => {

debug('Processing Projects...')
projects.forEach(project => {
const isInScope = isCheckApplicableToProjectCategory(check, project)
// If the check is not in scope, skip it.
if (!isInScope) {
debug(`This check is not in scope for project (${project.id})`)
return
}

const baseData = {
project_id: project.id,
compliance_check_id: check.id,
severity: getSeverityFromPriorityGroup(check.priority_group)
severity: getSeverityFromPriorityGroup(check.default_priority_group)
}

const result = { ...baseData }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
const statusLevels = ['n/a', 'deferrable', 'expected', 'recommended']
const priorityGroupOptions = [
'P0', 'P1', 'P2', 'P3', 'P4', 'P5', 'P6', 'P7', 'P8', 'P9', 'P10', 'P11', 'P12', 'P13', 'P14',
'R0', 'R1', 'R2', 'R3', 'R4', 'R5', 'R6', 'R7', 'R8', 'R9', 'R10', 'R11', 'R12', 'R13', 'R14'
]

exports.up = async (knex) => {
await knex.schema.createTable('compliance_checks', (table) => {
Expand All @@ -8,7 +12,7 @@ exports.up = async (knex) => {
table.string('section_number').notNullable()
table.string('section_name').notNullable()
table.string('code_name').unique().notNullable()
table.string('priority_group').notNullable()
table.enum('priority_group', priorityGroupOptions).notNullable()
table.boolean('is_c_scrm').notNullable().defaultTo(false)
table.enum('level_incubating_status', statusLevels).notNullable()
table.enum('level_active_status', statusLevels).notNullable()
Expand Down
29 changes: 29 additions & 0 deletions src/database/migrations/1735041299955_alter_compliance_checks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const statusLevels = ['n/a', 'deferrable', 'expected', 'recommended']

exports.up = async (knex) => {
await knex.schema.alterTable('compliance_checks', (table) => {
// Drop old fields
table.dropColumn('level_incubating_status')
table.dropColumn('level_active_status')
table.dropColumn('level_retiring_status')

// Rename fields
table.renameColumn('priority_group', 'default_priority_group')
table.renameColumn('section_number', 'default_section_number')
table.renameColumn('section_name', 'default_section_name')
})
}

exports.down = async (knex) => {
await knex.schema.alterTable('compliance_checks', (table) => {
// IMPORTANT: Re-add dropped fields but without the original values and nullable.
table.enum('level_incubating_status', statusLevels).nullable()
table.enum('level_active_status', statusLevels).nullable()
table.enum('level_retiring_status', statusLevels).nullable()

// Rename fields back
table.renameColumn('default_priority_group', 'priority_group')
table.renameColumn('default_section_number', 'section_number')
table.renameColumn('default_section_name', 'section_name')
})
}
29 changes: 29 additions & 0 deletions src/database/migrations/1735041508837_add_compliance_checklists.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
exports.up = async (knex) => {
await knex.schema.createTable('compliance_checklists', (table) => {
table.increments('id').primary() // Primary key
table.text('author').notNullable()
table.string('title').notNullable()
table.text('description').notNullable()
table.string('code_name').notNullable()
table.text('url').notNullable()

// Timestamps
table.timestamp('created_at').defaultTo(knex.fn.now()).notNullable()
table.timestamp('updated_at').defaultTo(knex.fn.now()).notNullable()
})

// Add trigger to automatically update the 'updated_at' column
await knex.raw(`
CREATE TRIGGER set_updated_at_compliance_checklists
BEFORE UPDATE ON compliance_checklists
FOR EACH ROW
EXECUTE FUNCTION update_updated_at_column();
`)
}

exports.down = async (knex) => {
// Drop trigger
await knex.raw('DROP TRIGGER IF EXISTS set_updated_at_compliance_checklists ON compliance_checklists;')
// Drop table
await knex.schema.dropTableIfExists('compliance_checklists')
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
const list = [
{
id: 1,
author: 'OpenJS Foundation',
title: 'Security Compliance Guide v1.0 - Incubating',
description: 'This checklist is for projects that are in the incubating phase and have multiple maintainers.',
url: 'https://openpathfinder.com/docs/checklists/openjsSCGv1.0-incubating',
code_name: 'OpenJS-SCGv1.0-incubating'
}, {
id: 2,
author: 'OpenJS Foundation',
title: 'Security Compliance Guide v1.0 - Active',
description: 'This checklist is for projects that are in the active phase and have multiple maintainers.',
url: 'https://openpathfinder.com/docs/checklists/openjsSCGv1.0-active',
code_name: 'OpenJS-SCGv1.0-active'
}, {
id: 3,
author: 'OpenJS Foundation',
title: 'Security Compliance Guide v1.0 - Retiring',
description: 'This checklist is for projects that are in the retiring phase and have multiple maintainers.',
url: 'https://openpathfinder.com/docs/checklists/openjsSCGv1.0-retiring',
code_name: 'OpenJS-SCGv1.0-retiring'
}, {
id: 4,
author: 'OpenJS Foundation',
title: 'Security Compliance Guide v1.0 - Solo Maintainers incubating',
description: 'This checklist is for projects that are in the incubating phase and have a solo maintainer.',
url: 'https://openpathfinder.com/docs/checklists/openjsSCGv1.0-solo-incubating',
code_name: 'OpenJS-SCGv1.0-solo-incubating'
}, {
id: 5,
author: 'OpenJS Foundation',
title: 'Security Compliance Guide v1.0 - Solo Maintainers Active',
description: 'This checklist is for projects that are in the active phase and have a solo maintainer.',
url: 'https://openpathfinder.com/docs/checklists/openjsSCGv1.0-solo-active',
code_name: 'OpenJS-SCGv1.0-solo-active'
}, {
id: 6,
author: 'OpenJS Foundation',
title: 'Security Compliance Guide v1.0 - Solo Maintainers Retiring',
description: 'This checklist is for projects that are in the retiring phase and have a solo maintainer.',
url: 'https://openpathfinder.com/docs/checklists/openjsSCGv1.0-solo-retiring',
code_name: 'OpenJS-SCGv1.0-solo-retiring'
}
]

exports.up = async (knex) => {
await knex('compliance_checklists').insert(list)
}

exports.down = async (knex) => {
await knex('compliance_checklists').del()
}
39 changes: 39 additions & 0 deletions src/database/migrations/1735042950475_add_checklist_items.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
const priorityGroupOptions = [
'P0', 'P1', 'P2', 'P3', 'P4', 'P5', 'P6', 'P7', 'P8', 'P9', 'P10', 'P11', 'P12', 'P13', 'P14',
'R0', 'R1', 'R2', 'R3', 'R4', 'R5', 'R6', 'R7', 'R8', 'R9', 'R10', 'R11', 'R12', 'R13', 'R14'
]

exports.up = async (knex) => {
await knex.schema.createTable('checklist_items', (table) => {
table.increments('id').primary() // Primary key
table.integer('checklist_id').unsigned().notNullable() // Foreign key to compliance_checklists
table.foreign('checklist_id').references('id').inTable('compliance_checklists').onDelete('CASCADE')

table.integer('compliance_check_id').unsigned().notNullable() // Foreign key to compliance_checks
table.foreign('compliance_check_id').references('id').inTable('compliance_checks').onDelete('CASCADE')

// Overrides for the association if needed
table.enum('priority_group', priorityGroupOptions).nullable()
table.string('section_number').nullable()
table.string('section_name').nullable()

// Timestamps
table.timestamp('created_at').defaultTo(knex.fn.now()).notNullable()
table.timestamp('updated_at').defaultTo(knex.fn.now()).notNullable()
})

// Add trigger to automatically update the 'updated_at' column
await knex.raw(`
CREATE TRIGGER set_updated_at_checklist_items
BEFORE UPDATE ON checklist_items
FOR EACH ROW
EXECUTE FUNCTION update_updated_at_column();
`)
}

exports.down = async (knex) => {
// Drop trigger
await knex.raw('DROP TRIGGER IF EXISTS set_updated_at_checklist_items ON checklist_items;')
// Drop table
await knex.schema.dropTableIfExists('checklist_items')
}
Loading
Loading