Merge pull request #167 from OpenPathfinder/feat/add-checklists

Author: UlisesGascon

.github/ISSUE_TEMPLATE/add-a-new-compliance-check.md

@@ -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.

.github/workflows/review-compliance-checks.yml

@@ -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" +

CONTRIBUTING.md

@@ -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`).
 

__tests__/checks/validators.test.js

@@ -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 = [
@@ -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', () => {
@@ -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 = [
@@ -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: []
-    })
-  })
 })

__tests__/utils.test.js

@@ -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
@@ -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')

src/checks/validators/githubOrgMFA.js

@@ -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')
 
@@ -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 }

src/checks/validators/softwareDesignTraining.js

@@ -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'
 
@@ -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 }

src/database/migrations/1733494222119_add_compliance_checks.js

@@ -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) => {
@@ -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()

src/database/migrations/1735041299955_alter_compliance_checks.js

@@ -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')
+  })
+}

src/database/migrations/1735041508837_add_compliance_checklists.js

@@ -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')
+}

src/database/migrations/1735041860349_populate_compliance_checklists.js

@@ -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()
+}

src/database/migrations/1735042950475_add_checklist_items.js

@@ -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')
+}