From ae249fc1c3b859770c6bc898ea525c1fda914c11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 24 Apr 2025 15:02:19 +0200 Subject: [PATCH 1/2] leverage webhook secrets from github integrations too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/large-experts-sort.md | 5 +++ .../events-backend-module-github/package.json | 1 + .../createGithubSignatureValidator.test.ts | 28 ++++++++++++ .../http/createGithubSignatureValidator.ts | 44 +++++++++++++------ yarn.lock | 1 + 5 files changed, 65 insertions(+), 14 deletions(-) create mode 100644 .changeset/large-experts-sort.md diff --git a/.changeset/large-experts-sort.md b/.changeset/large-experts-sort.md new file mode 100644 index 0000000000..003be57430 --- /dev/null +++ b/.changeset/large-experts-sort.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-events-backend-module-github': patch +--- + +Support webhook validation based on `integrations.github.[].apps.[].webhookSecret` too diff --git a/plugins/events-backend-module-github/package.json b/plugins/events-backend-module-github/package.json index 13ab583c0c..54bee4bfa0 100644 --- a/plugins/events-backend-module-github/package.json +++ b/plugins/events-backend-module-github/package.json @@ -45,6 +45,7 @@ "dependencies": { "@backstage/backend-plugin-api": "workspace:^", "@backstage/config": "workspace:^", + "@backstage/integration": "workspace:^", "@backstage/plugin-events-node": "workspace:^", "@octokit/webhooks-methods": "^3.0.0" }, diff --git a/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.test.ts b/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.test.ts index bb604e88d8..c771e29d67 100644 --- a/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.test.ts +++ b/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.test.ts @@ -47,6 +47,24 @@ describe('createGithubSignatureValidator', () => { }, }, }); + const configWithAppSecret = new ConfigReader({ + integrations: { + github: [ + { + host: 'github.com', + apps: [ + { + appId: 1, + privateKey: 'a', + clientId: 'b', + clientSecret: 'c', + webhookSecret: secret, + }, + ], + }, + ], + }, + }); const payloadString = '{"test": "payload", "score": 5.0}'; const payload = JSON.parse(payloadString); const payloadBuffer = Buffer.from(payloadString); @@ -104,4 +122,14 @@ describe('createGithubSignatureValidator', () => { expect(context.details).toBeUndefined(); }); + + it('secret configured, accept request with valid signature defined in integrations', async () => { + const request = await requestWithSignature(await validSignature); + const context = new TestContext(); + + const validator = createGithubSignatureValidator(configWithAppSecret); + await validator!(request, context); + + expect(context.details).toBeUndefined(); + }); }); diff --git a/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.ts b/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.ts index f54c00b58a..137b2a08f3 100644 --- a/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.ts +++ b/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.ts @@ -15,6 +15,7 @@ */ import { Config } from '@backstage/config'; +import { ScmIntegrations } from '@backstage/integration'; import { RequestDetails, RequestValidationContext, @@ -36,10 +37,25 @@ import { verify } from '@octokit/webhooks-methods'; export function createGithubSignatureValidator( config: Config, ): RequestValidator | undefined { - const secret = config.getOptionalString( + const webhookSecrets = new Set(); + + const integrations = ScmIntegrations.fromConfig(config); + for (const integration of integrations.github.list()) { + for (const app of integration.config.apps ?? []) { + if (app.webhookSecret) { + webhookSecrets.add(app.webhookSecret); + } + } + } + + const moduleSecret = config.getOptionalString( 'events.modules.github.webhookSecret', ); - if (!secret) { + if (moduleSecret) { + webhookSecrets.add(moduleSecret); + } + + if (webhookSecrets.size === 0) { return undefined; } @@ -51,18 +67,18 @@ export function createGithubSignatureValidator( | string | undefined; - if ( - !signature || - !(await verify( - secret, - request.raw.body.toString(request.raw.encoding), - signature, - )) - ) { - context.reject({ - status: 403, - payload: { message: 'invalid signature' }, - }); + if (signature) { + const body = request.raw.body.toString(request.raw.encoding); + for (const secret of webhookSecrets) { + if (await verify(secret, body, signature)) { + return; // OK + } + } } + + context.reject({ + status: 403, + payload: { message: 'invalid signature' }, + }); }; } diff --git a/yarn.lock b/yarn.lock index fac71e8789..7acee57c6d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6576,6 +6576,7 @@ __metadata: "@backstage/backend-test-utils": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/config": "workspace:^" + "@backstage/integration": "workspace:^" "@backstage/plugin-events-backend-test-utils": "workspace:^" "@backstage/plugin-events-node": "workspace:^" "@octokit/webhooks-methods": "npm:^3.0.0" From 3dd708ff4d14bf62b27f602c27faaacf2f4e4949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sat, 26 Apr 2025 11:49:55 +0200 Subject: [PATCH 2/2] properly resolve app IDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/large-experts-sort.md | 6 +- .../events-backend-module-github/package.json | 6 +- .../report.api.md | 7 -- .../createGithubSignatureValidator.test.ts | 51 +++++++-- .../http/createGithubSignatureValidator.ts | 40 ++++--- .../events-backend-module-github/src/index.ts | 6 +- .../src/service/eventsModuleGithubWebhook.ts | 9 +- .../src/util/createAppIdResolver.ts | 66 +++++++++++ .../src/util/octokitProviderService.ts | 104 ++++++++++++++++++ yarn.lock | 11 ++ 10 files changed, 270 insertions(+), 36 deletions(-) create mode 100644 plugins/events-backend-module-github/src/util/createAppIdResolver.ts create mode 100644 plugins/events-backend-module-github/src/util/octokitProviderService.ts diff --git a/.changeset/large-experts-sort.md b/.changeset/large-experts-sort.md index 003be57430..6d58836db7 100644 --- a/.changeset/large-experts-sort.md +++ b/.changeset/large-experts-sort.md @@ -1,5 +1,7 @@ --- -'@backstage/plugin-events-backend-module-github': patch +'@backstage/plugin-events-backend-module-github': minor --- -Support webhook validation based on `integrations.github.[].apps.[].webhookSecret` too +**BREAKING**: Removed the `createGithubSignatureValidator` export. + +Added support webhook validation based on `integrations.github.[].apps.[].webhookSecret`. diff --git a/plugins/events-backend-module-github/package.json b/plugins/events-backend-module-github/package.json index 54bee4bfa0..f03ba438fb 100644 --- a/plugins/events-backend-module-github/package.json +++ b/plugins/events-backend-module-github/package.json @@ -47,7 +47,11 @@ "@backstage/config": "workspace:^", "@backstage/integration": "workspace:^", "@backstage/plugin-events-node": "workspace:^", - "@octokit/webhooks-methods": "^3.0.0" + "@backstage/types": "workspace:^", + "@octokit/auth-callback": "^5.0.0", + "@octokit/webhooks-methods": "^3.0.0", + "lodash": "^4.17.21", + "octokit": "^3.0.0" }, "devDependencies": { "@backstage/backend-test-utils": "workspace:^", diff --git a/plugins/events-backend-module-github/report.api.md b/plugins/events-backend-module-github/report.api.md index 8d1f4d939f..afecb7590b 100644 --- a/plugins/events-backend-module-github/report.api.md +++ b/plugins/events-backend-module-github/report.api.md @@ -4,17 +4,10 @@ ```ts import { BackendFeature } from '@backstage/backend-plugin-api'; -import { Config } from '@backstage/config'; import { EventParams } from '@backstage/plugin-events-node'; import { EventsService } from '@backstage/plugin-events-node'; -import { RequestValidator } from '@backstage/plugin-events-node'; import { SubTopicEventRouter } from '@backstage/plugin-events-node'; -// @public -export function createGithubSignatureValidator( - config: Config, -): RequestValidator | undefined; - // @public (undocumented) const _default: BackendFeature; export default _default; diff --git a/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.test.ts b/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.test.ts index c771e29d67..9cb2ed3c4c 100644 --- a/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.test.ts +++ b/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.test.ts @@ -22,6 +22,7 @@ import { } from '@backstage/plugin-events-node'; import { sign } from '@octokit/webhooks-methods'; import { createGithubSignatureValidator } from './createGithubSignatureValidator'; +import { OctokitProviderService } from '../util/octokitProviderService'; class TestContext implements RequestValidationContext { #details?: Partial; @@ -35,6 +36,10 @@ class TestContext implements RequestValidationContext { } } +const octokitProvider = { + getOctokit: jest.fn(), +} satisfies OctokitProviderService; + describe('createGithubSignatureValidator', () => { const secret = 'valid-secret'; const configWithoutSecret = new ConfigReader({}); @@ -54,7 +59,7 @@ describe('createGithubSignatureValidator', () => { host: 'github.com', apps: [ { - appId: 1, + appId: 7, privateKey: 'a', clientId: 'b', clientSecret: 'c', @@ -65,8 +70,13 @@ describe('createGithubSignatureValidator', () => { ], }, }); - const payloadString = '{"test": "payload", "score": 5.0}'; - const payload = JSON.parse(payloadString); + const payload = { + test: 'payload', + score: 5.0, + repository: { html_url: 'https://github.com/backstage/backstage' }, + installation: { id: 70 }, + }; + const payloadString = JSON.stringify(payload); const payloadBuffer = Buffer.from(payloadString); const validSignature = sign({ secret, algorithm: 'sha256' }, payloadString); @@ -84,16 +94,19 @@ describe('createGithubSignatureValidator', () => { }; it('should return undefined if no secret is configured', async () => { - expect(createGithubSignatureValidator(configWithoutSecret)).toEqual( - undefined, - ); + expect( + createGithubSignatureValidator(configWithoutSecret, octokitProvider), + ).toEqual(undefined); }); it('secret configured, reject request without signature', async () => { const request = await requestWithSignature(undefined); const context = new TestContext(); - const validator = createGithubSignatureValidator(configWithSecret); + const validator = createGithubSignatureValidator( + configWithSecret, + octokitProvider, + ); await validator!(request, context); expect(context.details).not.toBeUndefined(); @@ -105,7 +118,10 @@ describe('createGithubSignatureValidator', () => { const request = await requestWithSignature('invalid signature'); const context = new TestContext(); - const validator = createGithubSignatureValidator(configWithSecret); + const validator = createGithubSignatureValidator( + configWithSecret, + octokitProvider, + ); await validator!(request, context); expect(context.details).not.toBeUndefined(); @@ -117,7 +133,10 @@ describe('createGithubSignatureValidator', () => { const request = await requestWithSignature(await validSignature); const context = new TestContext(); - const validator = createGithubSignatureValidator(configWithSecret); + const validator = createGithubSignatureValidator( + configWithSecret, + octokitProvider, + ); await validator!(request, context); expect(context.details).toBeUndefined(); @@ -126,8 +145,20 @@ describe('createGithubSignatureValidator', () => { it('secret configured, accept request with valid signature defined in integrations', async () => { const request = await requestWithSignature(await validSignature); const context = new TestContext(); + octokitProvider.getOctokit.mockResolvedValue({ + rest: { + apps: { + getInstallation: async () => ({ + data: { app_id: 7 }, + }), + }, + }, + }); - const validator = createGithubSignatureValidator(configWithAppSecret); + const validator = createGithubSignatureValidator( + configWithAppSecret, + octokitProvider, + ); await validator!(request, context); expect(context.details).toBeUndefined(); diff --git a/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.ts b/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.ts index 137b2a08f3..6eb203b79b 100644 --- a/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.ts +++ b/plugins/events-backend-module-github/src/http/createGithubSignatureValidator.ts @@ -22,6 +22,8 @@ import { RequestValidator, } from '@backstage/plugin-events-node'; import { verify } from '@octokit/webhooks-methods'; +import { createAppIdResolver } from '../util/createAppIdResolver'; +import { OctokitProviderService } from '../util/octokitProviderService'; /** * Validates that the request received is the expected GitHub request @@ -36,29 +38,31 @@ import { verify } from '@octokit/webhooks-methods'; */ export function createGithubSignatureValidator( config: Config, + octokitProvider: OctokitProviderService, ): RequestValidator | undefined { - const webhookSecrets = new Set(); - const integrations = ScmIntegrations.fromConfig(config); + + // GitHub App installation ID to secret + const githubAppSecrets = new Map(); for (const integration of integrations.github.list()) { - for (const app of integration.config.apps ?? []) { - if (app.webhookSecret) { - webhookSecrets.add(app.webhookSecret); + for (const { appId, webhookSecret } of integration.config.apps ?? []) { + if (appId && webhookSecret) { + githubAppSecrets.set(appId, webhookSecret); } } } - const moduleSecret = config.getOptionalString( + // A single optional secret for all GitHub events + const genericSecret = config.getOptionalString( 'events.modules.github.webhookSecret', ); - if (moduleSecret) { - webhookSecrets.add(moduleSecret); - } - if (webhookSecrets.size === 0) { + if (!genericSecret && githubAppSecrets.size === 0) { return undefined; } + const appIdResolver = createAppIdResolver(octokitProvider); + return async ( request: RequestDetails, context: RequestValidationContext, @@ -69,9 +73,19 @@ export function createGithubSignatureValidator( if (signature) { const body = request.raw.body.toString(request.raw.encoding); - for (const secret of webhookSecrets) { - if (await verify(secret, body, signature)) { - return; // OK + + if (githubAppSecrets.size) { + const appId = await appIdResolver(request); + if (appId && githubAppSecrets.has(appId)) { + if (await verify(githubAppSecrets.get(appId)!, body, signature)) { + return; + } + } + } + + if (genericSecret) { + if (await verify(genericSecret, body, signature)) { + return; } } } diff --git a/plugins/events-backend-module-github/src/index.ts b/plugins/events-backend-module-github/src/index.ts index 72b55f892b..285d74bae4 100644 --- a/plugins/events-backend-module-github/src/index.ts +++ b/plugins/events-backend-module-github/src/index.ts @@ -20,6 +20,7 @@ * * @packageDocumentation */ + import { createBackendFeatureLoader } from '@backstage/backend-plugin-api'; export default createBackendFeatureLoader({ @@ -31,5 +32,8 @@ export default createBackendFeatureLoader({ }, }); -export { createGithubSignatureValidator } from './http/createGithubSignatureValidator'; +// TODO(freben): This is not exported at the moment since it depends on the octokit provider. +// Until we have made that a core thing in integrations, we can't export it +// export { createGithubSignatureValidator } from './http/createGithubSignatureValidator'; + export { GithubEventRouter } from './router/GithubEventRouter'; diff --git a/plugins/events-backend-module-github/src/service/eventsModuleGithubWebhook.ts b/plugins/events-backend-module-github/src/service/eventsModuleGithubWebhook.ts index 5cb9c88679..78def1f30a 100644 --- a/plugins/events-backend-module-github/src/service/eventsModuleGithubWebhook.ts +++ b/plugins/events-backend-module-github/src/service/eventsModuleGithubWebhook.ts @@ -20,6 +20,7 @@ import { } from '@backstage/backend-plugin-api'; import { eventsExtensionPoint } from '@backstage/plugin-events-node/alpha'; import { createGithubSignatureValidator } from '../http/createGithubSignatureValidator'; +import { octokitProviderServiceRef } from '../util/octokitProviderService'; /** * Module for the events-backend plugin, @@ -36,9 +37,13 @@ export default createBackendModule({ deps: { config: coreServices.rootConfig, events: eventsExtensionPoint, + octokitProvider: octokitProviderServiceRef, }, - async init({ config, events }) { - const validator = createGithubSignatureValidator(config); + async init({ config, events, octokitProvider }) { + const validator = createGithubSignatureValidator( + config, + octokitProvider, + ); if (validator) { events.addHttpPostIngress({ topic: 'github', diff --git a/plugins/events-backend-module-github/src/util/createAppIdResolver.ts b/plugins/events-backend-module-github/src/util/createAppIdResolver.ts new file mode 100644 index 0000000000..d5347ffd29 --- /dev/null +++ b/plugins/events-backend-module-github/src/util/createAppIdResolver.ts @@ -0,0 +1,66 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { RequestDetails } from '@backstage/plugin-events-node'; +import { OctokitProviderService } from './octokitProviderService'; +import lodash from 'lodash'; + +export type AppIdResolver = ( + request: RequestDetails, +) => Promise; + +/** + * Helps with resolving what app ID (if any) that sent a webhook event. + */ +export function createAppIdResolver( + octokitProvider: OctokitProviderService, +): AppIdResolver { + const installationIdToAppId = new Map>(); + + return async (request: RequestDetails) => { + const installationId = lodash.get( + request.body, + 'installation.id', + ) as unknown; + + if (!installationId || typeof installationId !== 'number') { + return undefined; + } + + let appIdPromsie = installationIdToAppId.get(installationId); + if (appIdPromsie) { + return await appIdPromsie; + } + + const repositoryUrl = lodash.get( + request.body, + 'repository.html_url', + ) as unknown; + + if (!repositoryUrl || typeof repositoryUrl !== 'string') { + return undefined; + } + + const octokit = await octokitProvider.getOctokit(repositoryUrl); + appIdPromsie = octokit.rest.apps + .getInstallation({ installation_id: installationId }) + .then(response => Number(response.data.app_id)) + .catch(() => undefined); + + installationIdToAppId.set(installationId, appIdPromsie); + return await appIdPromsie; + }; +} diff --git a/plugins/events-backend-module-github/src/util/octokitProviderService.ts b/plugins/events-backend-module-github/src/util/octokitProviderService.ts new file mode 100644 index 0000000000..9993b91000 --- /dev/null +++ b/plugins/events-backend-module-github/src/util/octokitProviderService.ts @@ -0,0 +1,104 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + coreServices, + createServiceFactory, + createServiceRef, + RootConfigService, +} from '@backstage/backend-plugin-api'; +import { + DefaultGithubCredentialsProvider, + GithubCredentialsProvider, + ScmIntegrationRegistry, + ScmIntegrations, +} from '@backstage/integration'; +import { durationToMilliseconds, HumanDuration } from '@backstage/types'; +import { Octokit } from 'octokit'; + +export interface OctokitProviderService { + getOctokit: (url: string) => Promise; +} + +class OctokitProviderImpl implements OctokitProviderService { + readonly #integrations: ScmIntegrationRegistry; + readonly #githubCredentials: GithubCredentialsProvider; + readonly #octokitCache: Map; + readonly #octokitCacheTtl: HumanDuration; + + constructor(config: RootConfigService) { + this.#integrations = ScmIntegrations.fromConfig(config); + this.#githubCredentials = DefaultGithubCredentialsProvider.fromIntegrations( + this.#integrations, + ); + this.#octokitCache = new Map(); + this.#octokitCacheTtl = { hours: 1 }; + } + + async getOctokit(url: string): Promise { + // TODO(freben): Be smart and cache these more granularly, e.g. by + // organization or even repo. + const integration = this.#integrations.github.byUrl(url); + if (!integration) { + throw new Error(`No integration found for url: ${url}`); + } + const key = integration.config.host; + + if (this.#octokitCache.has(key)) { + return this.#octokitCache.get(key)!; + } + + const { createCallbackAuth } = await import('@octokit/auth-callback'); + + const octokit = new Octokit({ + baseUrl: integration.config.apiBaseUrl, + authStrategy: createCallbackAuth, + auth: { + callback: async () => { + try { + const credentials = await this.#githubCredentials.getCredentials({ + url, + }); + return credentials.token; + } catch { + return undefined; + } + }, + }, + }); + + this.#octokitCache.set(key, octokit); + setTimeout(() => { + this.#octokitCache.delete(key); + }, durationToMilliseconds(this.#octokitCacheTtl)); + + return octokit; + } +} + +export const octokitProviderServiceRef = + createServiceRef({ + id: 'octokitProvider', + scope: 'root', + defaultFactory: async service => + createServiceFactory({ + service, + deps: { config: coreServices.rootConfig }, + async factory({ config }) { + return new OctokitProviderImpl(config); + }, + }), + }); diff --git a/yarn.lock b/yarn.lock index 7acee57c6d..876222dd6e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6579,7 +6579,11 @@ __metadata: "@backstage/integration": "workspace:^" "@backstage/plugin-events-backend-test-utils": "workspace:^" "@backstage/plugin-events-node": "workspace:^" + "@backstage/types": "workspace:^" + "@octokit/auth-callback": "npm:^5.0.0" "@octokit/webhooks-methods": "npm:^3.0.0" + lodash: "npm:^4.17.21" + octokit: "npm:^3.0.0" languageName: unknown linkType: soft @@ -12850,6 +12854,13 @@ __metadata: languageName: node linkType: hard +"@octokit/auth-callback@npm:^5.0.0": + version: 5.0.1 + resolution: "@octokit/auth-callback@npm:5.0.1" + checksum: 10/afa2fd71e1cc238c4fc09a1a8cc3b5c8d2f231aecdb8f8be2384857cde893aa4ac697f7c6aa0b61e4d5fa1f00d41fb349ceffb938977c02ee317d5432247f6ef + languageName: node + linkType: hard + "@octokit/auth-oauth-app@npm:^5.0.0": version: 5.0.1 resolution: "@octokit/auth-oauth-app@npm:5.0.1"