feat(ncu-ci)!: require --certify-safe flag (#798)

Or ensure the PR has received at least one approving review since last
time it was pushed.

Author: aduh95

bin/ncu-ci.js

@@ -113,6 +113,11 @@ const args = yargs(hideBin(process.argv))
           describe: 'ID of the PR',
           type: 'number'
         })
+        .positional('certify-safe', {
+          describe: 'If not provided, the command will reject PRs that have ' +
+                    'been pushed since the last review',
+          type: 'boolean'
+        })
         .option('owner', {
           default: '',
           describe: 'GitHub repository owner'
@@ -291,7 +296,7 @@ class RunPRJobCommand {
       this.cli.setExitCode(1);
       return;
     }
-    const jobRunner = new RunPRJob(cli, request, owner, repo, prid);
+    const jobRunner = new RunPRJob(cli, request, owner, repo, prid, this.argv.certifySafe);
     if (!(await jobRunner.start())) {
       this.cli.setExitCode(1);
       process.exitCode = 1;

lib/ci/run_ci.js

@@ -7,6 +7,7 @@ import {
 } from './ci_type_parser.js';
 import PRData from '../pr_data.js';
 import { debuglog } from '../verbosity.js';
+import PRChecker from '../pr_checker.js';
 
 export const CI_CRUMB_URL = `https://${CI_DOMAIN}/crumbIssuer/api/json`;
 const CI_PR_NAME = CI_TYPES.get(CI_TYPES_KEYS.PR).jobName;
@@ -16,13 +17,16 @@ const CI_V8_NAME = CI_TYPES.get(CI_TYPES_KEYS.V8).jobName;
 export const CI_V8_URL = `https://${CI_DOMAIN}/job/${CI_V8_NAME}/build`;
 
 export class RunPRJob {
-  constructor(cli, request, owner, repo, prid) {
+  constructor(cli, request, owner, repo, prid, certifySafe) {
     this.cli = cli;
     this.request = request;
     this.owner = owner;
     this.repo = repo;
     this.prid = prid;
     this.prData = new PRData({ prid, owner, repo }, cli, request);
+    this.certifySafe =
+      certifySafe ||
+      new PRChecker(cli, this.prData, request, {}).checkCommitsAfterReview();
   }
 
   async getCrumb() {
@@ -62,7 +66,13 @@ export class RunPRJob {
   }
 
   async start() {
-    const { cli } = this;
+    const { cli, certifySafe } = this;
+
+    if (!certifySafe) {
+      cli.error('Refusing to run CI on potentially unsafe PR');
+      return false;
+    }
+
     cli.startSpinner('Validating Jenkins credentials');
     const crumb = await this.getCrumb();
 

lib/pr_checker.js

@@ -534,6 +534,7 @@ export default class PRChecker {
     );
 
     if (reviewIndex === -1) {
+      cli.warn('No approving reviews found');
       return false;
     }
 

test/unit/ci_start.test.js

@@ -1,4 +1,4 @@
-import { describe, it, before } from 'node:test';
+import { describe, it, before, afterEach } from 'node:test';
 import assert from 'assert';
 
 import sinon from 'sinon';
@@ -9,6 +9,7 @@ import {
   CI_CRUMB_URL,
   CI_PR_URL
 } from '../../lib/ci/run_ci.js';
+import PRChecker from '../../lib/pr_checker.js';
 
 import TestCLI from '../fixtures/test_cli.js';
 
@@ -51,7 +52,7 @@ describe('Jenkins', () => {
         .returns(Promise.resolve({ crumb }))
     };
 
-    const jobRunner = new RunPRJob(cli, request, owner, repo, prid);
+    const jobRunner = new RunPRJob(cli, request, owner, repo, prid, true);
     assert.strictEqual(await jobRunner.start(), false);
   });
 
@@ -61,7 +62,7 @@ describe('Jenkins', () => {
       json: sinon.stub().throws()
     };
 
-    const jobRunner = new RunPRJob(cli, request, owner, repo, prid);
+    const jobRunner = new RunPRJob(cli, request, owner, repo, prid, true);
     assert.strictEqual(await jobRunner.start(), false);
   });
 
@@ -89,7 +90,7 @@ describe('Jenkins', () => {
       json: sinon.stub().withArgs(CI_CRUMB_URL)
         .returns(Promise.resolve({ crumb }))
     };
-    const jobRunner = new RunPRJob(cli, request, owner, repo, prid);
+    const jobRunner = new RunPRJob(cli, request, owner, repo, prid, true);
     assert.ok(await jobRunner.start());
   });
 
@@ -108,7 +109,48 @@ describe('Jenkins', () => {
       json: sinon.stub().withArgs(CI_CRUMB_URL)
         .returns(Promise.resolve({ crumb }))
     };
-    const jobRunner = new RunPRJob(cli, request, owner, repo, prid);
+    const jobRunner = new RunPRJob(cli, request, owner, repo, prid, true);
     assert.strictEqual(await jobRunner.start(), false);
   });
+
+  describe('without --certify-safe flag', { concurrency: false }, () => {
+    afterEach(() => {
+      sinon.restore();
+    });
+    for (const certifySafe of [true, false]) {
+      it(`should return ${certifySafe} if PR checker reports it as ${
+        certifySafe ? '' : 'potentially un'
+      }safe`, async() => {
+        const cli = new TestCLI();
+
+        sinon.replace(PRChecker.prototype, 'checkCommitsAfterReview',
+          sinon.fake.returns(certifySafe));
+
+        const request = {
+          gql: sinon.stub().returns({
+            repository: {
+              pullRequest: {
+                labels: {
+                  nodes: []
+                }
+              }
+            }
+          }),
+          fetch: sinon.stub()
+            .callsFake((url, { method, headers, body }) => {
+              assert.strictEqual(url, CI_PR_URL);
+              assert.strictEqual(method, 'POST');
+              assert.deepStrictEqual(headers, { 'Jenkins-Crumb': crumb });
+              assert.ok(body._validated);
+              return Promise.resolve({ status: 201 });
+            }),
+          json: sinon.stub().withArgs(CI_CRUMB_URL)
+            .returns(Promise.resolve({ crumb }))
+        };
+
+        const jobRunner = new RunPRJob(cli, request, owner, repo, prid, false);
+        assert.strictEqual(await jobRunner.start(), certifySafe);
+      });
+    }
+  });
 });

test/unit/pr_checker.test.js

@@ -2179,7 +2179,9 @@ describe('PRChecker', () => {
 
     it('should skip the check if there are no reviews', () => {
       const { commits } = multipleCommitsAfterReview;
-      const expectedLogs = {};
+      const expectedLogs = {
+        warn: [['No approving reviews found']]
+      };
 
       const data = {
         pr: firstTimerPR,