fix(3225): organize the return values of _parseHook (#238)

screwdriver-cd · Oct 21, 2024 · d96b38a · d96b38a
1 parent 720ac28
commit d96b38a
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 19 deletions.
diff --git a/index.js b/index.js
@@ -1522,25 +1522,21 @@ class GithubScm extends Scm {
         const regexMatchArray = checkoutUrl.match(CHECKOUT_URL_REGEX);
 
         if (!regexMatchArray || regexMatchArray[1] !== checkoutSshHost) {
-            logger.info(`Incorrect checkout SshHost: ${checkoutUrl}`);
-
-            return null;
+            throwError(`Incorrect checkout SshHost: ${checkoutUrl}`, 400);
         }
 
         // additional check for github enterprise cloud hooks
         if (this.config.gheCloud) {
             const enterpriseSlug = hoek.reach(webhookPayload, 'enterprise.slug');
 
             if (this.config.gheCloudSlug !== enterpriseSlug) {
-                logger.info(`Skipping incorrect scm context for hook parsing, ${checkoutUrl}, ${scmContext}`);
-
-                return null;
+                throwError(`Skipping incorrect scm context for hook parsing, ${checkoutUrl}, ${scmContext}`, 400);
             }
         }
 
         // eslint-disable-next-line no-underscore-dangle
         if (!(await verify(this.config.secret, JSON.stringify(webhookPayload), signature))) {
-            throwError('Invalid x-hub-signature');
+            throwError('Invalid x-hub-signature', 400);
         }
 
         switch (type) {
@@ -1900,9 +1896,9 @@ class GithubScm extends Scm {
      */
     async _canHandleWebhook(headers, payload) {
         try {
-            const result = await this._parseHook(headers, payload);
+            await this._parseHook(headers, payload);
 
-            return result !== null;
+            return true;
         } catch (err) {
             logger.error('Failed to run canHandleWebhook', err);
 

diff --git a/test/index.test.js b/test/index.test.js
@@ -1879,7 +1879,7 @@ jobs:
             });
         });
 
-        it('resolves null for a pull request payload from incorrect ghe enterprise cloud', () => {
+        it('rejects for a pull request payload from incorrect ghe enterprise cloud', () => {
             const scmGHEC = new GithubScm({
                 oauthClientId: 'abcdefg',
                 oauthClientSecret: 'defghijk',
@@ -1897,9 +1897,15 @@ jobs:
                 }
             });
 
-            return scmGHEC.parseHook(testHeaders, testPayloadPrBadSlug).then(result => {
-                assert.isNull(result);
-            });
+            return scmGHEC
+                .parseHook(testHeaders, testPayloadPrBadSlug)
+                .then(() => {
+                    assert.fail('This should not fail the tests');
+                })
+                .catch(err => {
+                    assert.include(err.message, 'Skipping incorrect scm context for hook parsing');
+                    assert.strictEqual(err.statusCode, 400);
+                });
         });
 
         it('parses a payload for a pull request event payload', () => {
@@ -1951,9 +1957,15 @@ jobs:
         it('rejects when ssh host is not valid', () => {
             testHeaders['x-hub-signature'] = 'sha1=1b51a3f9f548fdacab52c0e83f9a63f8cbb4b591';
 
-            return scm.parseHook(testHeaders, testPayloadPingBadSshHost).then(result => {
-                assert.isNull(result);
-            });
+            return scm
+                .parseHook(testHeaders, testPayloadPingBadSshHost)
+                .then(() => {
+                    assert.fail('This should not fail the tests');
+                })
+                .catch(err => {
+                    assert.include(err.message, 'Incorrect checkout SshHost');
+                    assert.strictEqual(err.statusCode, 400);
+                });
         });
 
         it('rejects when signature is not valid', () => {
@@ -1966,7 +1978,7 @@ jobs:
                 })
                 .catch(err => {
                     assert.equal(err.message, 'Invalid x-hub-signature');
-                    assert.strictEqual(err.statusCode, 500);
+                    assert.strictEqual(err.statusCode, 400);
                 });
         });
     });
@@ -3524,11 +3536,11 @@ jobs:
             });
         });
 
-        it('returns false when the github event is not valid', () => {
+        it('returns true when the github event is not valid', () => {
             testHeaders['x-github-event'] = 'REEEEEEEE';
 
             return scm.canHandleWebhook(testHeaders, testPayloadPush).then(result => {
-                assert.strictEqual(result, false);
+                assert.strictEqual(result, true);
             });
         });