fix(node): do not trust commiter date (#758)

The date of a commit is provided by the user, and we should not trust it
blindly. Instead, we can check when was the last push event to the PR
base branch.

Author: aduh95

lib/pr_checker.js

@@ -548,6 +548,15 @@ export default class PRChecker {
     });
 
     const totalCommits = afterCommits.length;
+    if (totalCommits === 0 && this.pr.timelineItems.updatedAt > reviewDate) {
+      // Some commits were pushed, but all the commits have a commit date prior
+      // to the last review. It means that either that a long time elapsed
+      // between the commit and the push, or that the clock on the dev machine
+      // is wrong, or the commit date was forged.
+      cli.warn('Something was pushed to the Pull Request branch since the last approving review.');
+      return false;
+    }
+
     if (totalCommits > 0) {
       cli.warn('Commits were pushed since the last approving review:');
       const sliceLength = maxCommits === 0 ? totalCommits : -maxCommits;

lib/queries/PR.gql

@@ -23,6 +23,9 @@ query PR($prid: Int!, $owner: String!, $repo: String!) {
           path
         }
       },
+      timelineItems(itemTypes: [HEAD_REF_FORCE_PUSHED_EVENT, PULL_REQUEST_COMMIT]) {
+        updatedAt
+      },
       title,
       baseRefName,
       headRefName,

test/fixtures/semver_major_pr.json

@@ -16,6 +16,9 @@
       }
     ]
   },
+  "timelineItems": {
+    "updatedAt": "2017-10-24T11:13:43Z"
+  },
   "title": "lib: awesome changes",
   "baseRefName": "main",
   "headRefName": "awesome-changes"

test/unit/pr_checker.test.js

@@ -2152,6 +2152,31 @@ describe('PRChecker', () => {
       cli.assertCalledWith(expectedLogs);
     });
 
+    it('should return true if PR can be landed', () => {
+      const expectedLogs = {
+        warn: [
+          ['Something was pushed to the Pull Request branch since the last approving review.']
+        ],
+        info: [],
+        error: []
+      };
+
+      const checker = new PRChecker(cli, {
+        pr: { ...semverMajorPR, timelineItems: { updatedAt: new Date().toISOString() } },
+        reviewers: allGreenReviewers,
+        comments: commentsWithLGTM,
+        reviews: approvingReviews,
+        commits: simpleCommits,
+        collaborators,
+        authorIsNew: () => true,
+        getThread: PRData.prototype.getThread
+      }, {}, argv);
+
+      const status = checker.checkCommitsAfterReview();
+      assert.deepStrictEqual(status, false);
+      cli.assertCalledWith(expectedLogs);
+    });
+
     it('should skip the check if there are no reviews', () => {
       const { commits } = multipleCommitsAfterReview;
       const expectedLogs = {};