feat(ncu-ci): require --certify-safe flag

Or ensure the PR has received at least one approving review since last time it was pushed.
nodejs · Apr 24, 2024 · 55e4f43 · 55e4f43
1 parent 78ad337
commit 55e4f43
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 7 deletions.
diff --git a/bin/ncu-ci.js b/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;

diff --git a/lib/ci/run_ci.js b/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();
 
@@ -75,6 +85,7 @@ export class RunPRJob {
 
     try {
       cli.startSpinner('Starting PR CI job');
+      throw 'NOOO'
       const response = await this.request.fetch(CI_PR_URL, {
         method: 'POST',
         headers: {

diff --git a/lib/pr_checker.js b/lib/pr_checker.js
@@ -534,6 +534,7 @@ export default class PRChecker {
     );
 
     if (reviewIndex === -1) {
+      cli.warn('No approving reviews found');
       return false;
     }
 

diff --git a/test/unit/ci_start.test.js b/test/unit/ci_start.test.js
@@ -51,7 +51,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 +61,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 +89,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 +108,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.strictEqual(await jobRunner.start(), false);
   });
 });