fix(pr): verified the PR sender rather than the event sender

Author: travi

src/handler.js

@@ -85,7 +85,7 @@ async function processStatusEvent(payload, settings, request, responseToolkit, l
 }
 
 async function processCheckRunEvent(request, responseToolkit, settings, log) {
-  const {repository, check_run: checkRun, sender} = request.payload;
+  const {repository, check_run: checkRun} = request.payload;
 
   if (checkRunEventIsSuccessfulAndCouldBeForGreenkeeperPR(checkRun, log)) {
     const {check_suite: {pull_requests: pullRequests}} = checkRun;
@@ -94,18 +94,18 @@ async function processCheckRunEvent(request, responseToolkit, settings, log) {
     if (!pullRequests.length) return responseToolkit.response('no PRs for this commit').code(BAD_REQUEST);
     if (1 < pullRequests.length) return responseToolkit.response(boom.internal('too many PRs exist for this commit'));
 
-    const senderUrl = sender.html_url;
-    if (!openedByGreenkeeperBot(senderUrl)) {
-      return responseToolkit.response(`PR is not from greenkeeper, but from ${senderUrl}`).code(BAD_REQUEST);
-    }
-
     let pullRequest;
     try {
       pullRequest = await getPullRequest(repository, pullRequests[0].number);
     } catch (err) {
       throw boom.internal('failed to fetch PRs', err);
     }
 
+    const senderUrl = pullRequest.user.html_url;
+    if (!openedByGreenkeeperBot(senderUrl)) {
+      return responseToolkit.response(`PR is not from greenkeeper, but from ${senderUrl}`).code(BAD_REQUEST);
+    }
+
     process(pullRequest, settings, log);
     return responseToolkit.response('check_run event will be processed').code(ACCEPTED);
   }

test/integration/features/step_definitions/common-steps.js

@@ -18,7 +18,6 @@ When(/^the webhook is received$/, function () {
       status: this.checkRunEventStatus,
       conclusion: this.checkRunEventConclusion,
       repoOwner: this.repoOwner,
-      sender: this.prSender,
       prLink: this.prLink,
       prNumber: this.prNumber,
       ...this.commitBranches && {branch: this.commitBranches[0]}

test/integration/features/support/world.js

@@ -36,8 +36,7 @@ function buildWebhookPayload(event, {statusEventDetails, checkRunEventDetails, r
         owner: {
           login: checkRunEventDetails.repoOwner
         }
-      },
-      sender: {html_url: checkRunEventDetails.sender}
+      }
     };
   }
 

test/unit/handler-test.js

@@ -188,7 +188,7 @@ suite('handler', () => {
       const prNumber = any.integer();
       const sha = any.string();
       const partialPullRequest = {user: {html_url: greenkeeperSender}, number: prNumber};
-      const fullPullRequest = any.simpleObject();
+      const fullPullRequest = {...any.simpleObject(), user: {html_url: greenkeeperSender}};
       const request = {
         payload: {
           action: 'completed',
@@ -201,8 +201,7 @@ suite('handler', () => {
               pull_requests: [partialPullRequest]
             }
           },
-          repository,
-          sender: {html_url: greenkeeperSender}
+          repository
         },
         headers: {'x-github-event': 'check_run'},
         log: () => undefined
@@ -323,6 +322,10 @@ suite('handler', () => {
 
     test('that the response is bad-request if the PR is not from greenkeeper', () => {
       const senderUrl = any.url();
+      const prNumber = any.integer();
+      const repository = any.simpleObject();
+      const partialPullRequest = {user: {html_url: greenkeeperSender}, number: prNumber};
+      const fullPullRequest = {...any.simpleObject(), user: {html_url: senderUrl}};
       const request = {
         payload: {
           action: 'completed',
@@ -331,15 +334,16 @@ suite('handler', () => {
             conclusion: 'success',
             check_suite: {
               head_branch: any.word(),
-              pull_requests: [any.simpleObject()]
+              pull_requests: [partialPullRequest]
             }
           },
-          sender: {html_url: senderUrl}
+          repository
         },
         headers: {'x-github-event': 'check_run'},
         log: () => undefined
       };
       response.withArgs(`PR is not from greenkeeper, but from ${senderUrl}`).returns({code});
+      getPullRequest.withArgs(repository, prNumber).resolves(fullPullRequest);
 
       return handler(request, {response}, settings).then(() => assert.calledWith(code, BAD_REQUEST));
     });