From 57a10c6c69cc0bfb6033626f8c8513eb58347ca5 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 12 Aug 2023 13:58:08 +0200 Subject: [PATCH] backend-app-api: validate extension point deps Signed-off-by: Patrik Oldsberg --- .changeset/quick-cups-float.md | 5 +++ .../src/wiring/BackendInitializer.test.ts | 32 +++++++++++++++++++ .../src/wiring/BackendInitializer.ts | 23 +++++++++---- 3 files changed, 53 insertions(+), 7 deletions(-) create mode 100644 .changeset/quick-cups-float.md diff --git a/.changeset/quick-cups-float.md b/.changeset/quick-cups-float.md new file mode 100644 index 0000000000..21a2f856ec --- /dev/null +++ b/.changeset/quick-cups-float.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Add validation to make sure that extension points do not cross plugin boundaries. diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index ba637b7b5b..b90bcf6691 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts @@ -294,4 +294,36 @@ describe('BackendInitializer', () => { "Circular dependency detected for modules of plugin 'test', 'modA' -> 'modB' -> 'modA'", ); }); + + it('should reject modules that depend on extension points other plugins', async () => { + const init = new BackendInitializer(new ServiceRegistry(baseFactories)); + const extA = createExtensionPoint({ id: 'a' }); + init.add( + createBackendPlugin({ + pluginId: 'testA', + register(reg) { + reg.registerExtensionPoint(extA, 'a'); + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + })(), + ); + init.add( + createBackendModule({ + pluginId: 'testB', + moduleId: 'mod', + register(reg) { + reg.registerInit({ + deps: { ext: extA }, + async init() {}, + }); + }, + })(), + ); + await expect(init.start()).rejects.toThrow( + "Extension point registered for plugin 'testA' may not be used by module for plugin 'testB'", + ); + }); }); diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index 06201dfb2b..f71256d04e 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -42,7 +42,10 @@ export interface BackendRegisterInit { export class BackendInitializer { #startPromise?: Promise; #features = new Array(); - #extensionPoints = new Map, unknown>(); + #extensionPoints = new Map< + ExtensionPoint, + { impl: unknown; pluginId: string } + >(); #serviceHolder: EnumerableServiceHolder; constructor(serviceHolder: EnumerableServiceHolder) { @@ -57,11 +60,14 @@ export class BackendInitializer { const missingRefs = new Set(); for (const [name, ref] of Object.entries(deps)) { - const extensionPoint = this.#extensionPoints.get( - ref as ExtensionPoint, - ); - if (extensionPoint) { - result.set(name, extensionPoint); + const ep = this.#extensionPoints.get(ref as ExtensionPoint); + if (ep) { + if (ep.pluginId !== pluginId) { + throw new Error( + `Extension point registered for plugin '${ep.pluginId}' may not be used by module for plugin '${pluginId}'`, + ); + } + result.set(name, ep.impl); } else { const impl = await this.#serviceHolder.get( ref as ServiceRef, @@ -169,7 +175,10 @@ export class BackendInitializer { `ExtensionPoint with ID '${extRef.id}' is already registered`, ); } - this.#extensionPoints.set(extRef, extImpl); + this.#extensionPoints.set(extRef, { + impl: extImpl, + pluginId: r.pluginId, + }); provides.add(extRef); } }