Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);

Expand Down
32 changes: 22 additions & 10 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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);
});
});
});
Expand Down Expand Up @@ -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);
});
});

Expand Down