From 21ccd499700aaea0e2cdbd3a3ca4d255ae72fdea Mon Sep 17 00:00:00 2001 From: "@pawelmitka" Date: Mon, 6 Sep 2021 13:00:39 +0200 Subject: [PATCH 1/2] feat(plugin-scaffolder-backend): Validate webhook event names Webhook event names are validated against Octokit Webhooks `emitterEventNames`. Note `emitterEventNames` contains event/action combinations as well that are comma delimited. Therefore, extracting strings without '.' provides list of all events. "@octokit/webhooks" version 9.14.0 is the minimum requirement because this is a version where I introduced exported `emitterEventNames` to be used here. Signed-off-by: @pawelmitka --- .changeset/long-walls-tie.md | 5 ++ plugins/scaffolder-backend/api-report.md | 2 +- plugins/scaffolder-backend/package.json | 1 + .../builtin/github/githubWebhook.test.ts | 58 +++++++++++++++++++ .../actions/builtin/github/githubWebhook.ts | 19 +++++- yarn.lock | 22 ++++++- 6 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 .changeset/long-walls-tie.md diff --git a/.changeset/long-walls-tie.md b/.changeset/long-walls-tie.md new file mode 100644 index 0000000000..c23c34b4ce --- /dev/null +++ b/.changeset/long-walls-tie.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +GitHub Webhook action in Scaffolder Backend has been improved to validate event names against Octokit Webhook event names list. diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index b43adc189a..6f73c55f51 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -179,7 +179,7 @@ export function createRouter(options: RouterOptions): Promise; // @public (undocumented) export const createTemplateAction: < Input extends Partial<{ - [name: string]: JsonValue | Partial | undefined; + [name: string]: Partial | JsonValue | undefined; }>, >( templateAction: TemplateAction, diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 93cee90937..902860679d 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -39,6 +39,7 @@ "@gitbeaker/core": "^30.2.0", "@gitbeaker/node": "^30.2.0", "@octokit/rest": "^18.5.3", + "@octokit/webhooks": "^9.14.1", "@types/express": "^4.17.6", "azure-devops-node-api": "^11.0.1", "command-exists": "^1.2.9", diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.test.ts index 7d5da3e7b8..0da08bbf9d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.test.ts @@ -205,4 +205,62 @@ describe('github:repository:webhook:create', () => { }, }); }); + + it('should validate input', async () => { + const Validator = require('jsonschema').Validator; + const v = new Validator(); + + // validate default input without events specified + expect(v.validate(mockContext.input, action.schema?.input).valid).toBe( + true, + ); + + const inputWithValidEvent = { + ...mockContext.input, + events: ['push'], + }; + expect(v.validate(inputWithValidEvent, action.schema?.input).valid).toBe( + true, + ); + + const inputWithMultipleValidEvents = { + ...mockContext.input, + events: ['push', 'pull_request'], + }; + expect( + v.validate(inputWithMultipleValidEvents, action.schema?.input).valid, + ).toBe(true); + + const inputWithInvalidEvent = { + ...mockContext.input, + events: ['unexpected_event'], + }; + expect(v.validate(inputWithInvalidEvent, action.schema?.input).valid).toBe( + false, + ); + + const inputWithOneInvalidEvent = { + ...mockContext.input, + events: ['push', 'unexpected_event'], + }; + expect( + v.validate(inputWithOneInvalidEvent, action.schema?.input).valid, + ).toBe(false); + + const inputWithAllEvents = { + ...mockContext.input, + events: ['*'], + }; + expect(v.validate(inputWithAllEvents, action.schema?.input).valid).toBe( + true, + ); + + const inputWithAllEventsAndMore = { + ...mockContext.input, + events: ['*', 'push'], + }; + expect( + v.validate(inputWithAllEventsAndMore, action.schema?.input).valid, + ).toBe(false); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts index 9aefc37034..6d8324768e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts @@ -16,6 +16,7 @@ import { ScmIntegrationRegistry } from '@backstage/integration'; import { createTemplateAction } from '../../createTemplateAction'; import { OctokitProvider } from './OctokitProvider'; +import { emitterEventNames } from '@octokit/webhooks'; type ContentType = 'form' | 'json'; @@ -25,6 +26,7 @@ export function createGithubWebhookAction(options: { }) { const { integrations, defaultWebhookSecret } = options; const octokitProvider = new OctokitProvider(integrations); + const eventNames = emitterEventNames.filter(event => !event.includes('.')); return createTemplateAction<{ repoUrl: string; @@ -63,9 +65,20 @@ export function createGithubWebhookAction(options: { description: 'Determines what events the hook is triggered for. Default: push', type: 'array', - items: { - type: 'string', - }, + oneOf: [ + { + items: { + type: 'string', + enum: eventNames, + }, + }, + { + items: { + type: 'string', + const: '*', + }, + }, + ], }, active: { title: 'Active', diff --git a/yarn.lock b/yarn.lock index 7b2a14c310..c7c260e1bf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4861,7 +4861,7 @@ "@octokit/types" "^6.16.2" deprecation "^2.3.1" -"@octokit/request-error@^2.0.0", "@octokit/request-error@^2.0.5", "@octokit/request-error@^2.1.0": +"@octokit/request-error@^2.0.0", "@octokit/request-error@^2.0.2", "@octokit/request-error@^2.0.5", "@octokit/request-error@^2.1.0": version "2.1.0" resolved "https://registry.npmjs.org/@octokit/request-error/-/request-error-2.1.0.tgz#9e150357831bfc788d13a4fd4b1913d60c74d677" integrity sha512-1VIvgXxs9WHSjicsRwq8PlR2LR2x6DwsJAaFgzdi0JfJoGSO8mYI/cHJQ+9FbN21aa+DrgNLnwObmyeSC8Rmpg== @@ -4906,6 +4906,26 @@ dependencies: "@octokit/openapi-types" "^7.3.2" +"@octokit/webhooks-methods@^2.0.0": + version "2.0.0" + resolved "https://registry.npmjs.org/@octokit/webhooks-methods/-/webhooks-methods-2.0.0.tgz#1108b9ea661ca6c81e4a8bfa63a09eb27d5bc2db" + integrity sha512-35cfQ4YWlnZnmZKmIxlGPUPLtbkF8lr/A/1Sk1eC0ddLMwQN06dOuLc+dI3YLQS+T+MoNt3DIQ0NynwgKPilig== + +"@octokit/webhooks-types@4.4.0": + version "4.4.0" + resolved "https://registry.npmjs.org/@octokit/webhooks-types/-/webhooks-types-4.4.0.tgz#0985964b72d577221574cca614770e0743fe0830" + integrity sha512-gK9L2pWLZ1CWWlf9zwzFBQeA3dgyWdRe6gqAedR5T7adFZpoHg4De6GeXIFySV796ooQtKhW4eFfQLFQF5ydAA== + +"@octokit/webhooks@^9.14.1": + version "9.14.1" + resolved "https://registry.npmjs.org/@octokit/webhooks/-/webhooks-9.14.1.tgz#d1b8fcc4c1c18ee46ec30cef9e84043b57c8d2dd" + integrity sha512-INsqtHKQJys5NbuE71kq6uR8BMSSkZ2L9dLanAc1jylGroQ80SW1TYXLuJlcYNakNrHCCe4c5qvNKfR/KJXrJA== + dependencies: + "@octokit/request-error" "^2.0.2" + "@octokit/webhooks-methods" "^2.0.0" + "@octokit/webhooks-types" "4.4.0" + aggregate-error "^3.1.0" + "@open-draft/until@^1.0.3": version "1.0.3" resolved "https://registry.npmjs.org/@open-draft/until/-/until-1.0.3.tgz#db9cc719191a62e7d9200f6e7bab21c5b848adca" From cd8da26d21e65726ecd4f7da35b6830354a88c34 Mon Sep 17 00:00:00 2001 From: "@pawelmitka" Date: Mon, 6 Sep 2021 13:24:45 +0200 Subject: [PATCH 2/2] fix: revert api-report.md changes Signed-off-by: @pawelmitka --- plugins/scaffolder-backend/api-report.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 6f73c55f51..b43adc189a 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -179,7 +179,7 @@ export function createRouter(options: RouterOptions): Promise; // @public (undocumented) export const createTemplateAction: < Input extends Partial<{ - [name: string]: Partial | JsonValue | undefined; + [name: string]: JsonValue | Partial | undefined; }>, >( templateAction: TemplateAction,