From 5766fc71c62ef39bd469d81e3f03c66dad615625 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Wed, 9 Jul 2025 21:33:23 +0200 Subject: [PATCH] validate plugin and module ids Signed-off-by: Juan Pablo Garcia Ripa --- .changeset/itchy-sites-cheer.md | 8 ++++ .../architecture/08-naming-patterns.md | 16 +++---- .../src/wiring/BackendInitializer.test.ts | 47 +++++++++++++++++++ .../src/wiring/createBackendModule.ts | 7 +++ .../src/wiring/createBackendPlugin.ts | 7 +++ packages/config/report.api.md | 3 ++ packages/config/src/constants.ts | 27 +++++++++++ packages/config/src/index.ts | 1 + packages/config/src/reader.ts | 3 +- 9 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 .changeset/itchy-sites-cheer.md create mode 100644 packages/config/src/constants.ts diff --git a/.changeset/itchy-sites-cheer.md b/.changeset/itchy-sites-cheer.md new file mode 100644 index 0000000000..54f5f2fadf --- /dev/null +++ b/.changeset/itchy-sites-cheer.md @@ -0,0 +1,8 @@ +--- +'@backstage/backend-plugin-api': minor +'@backstage/backend-app-api': minor +'@backstage/config-loader': patch +'@backstage/config': patch +--- + +The backend will now throw an error if a plugin or a module doesn't have a valid ID diff --git a/docs/backend-system/architecture/08-naming-patterns.md b/docs/backend-system/architecture/08-naming-patterns.md index 585a9fe87c..990cc62b70 100644 --- a/docs/backend-system/architecture/08-naming-patterns.md +++ b/docs/backend-system/architecture/08-naming-patterns.md @@ -11,10 +11,10 @@ As a rule, all names should be camel case, with the exceptions of plugin and mod ### Plugins -| Description | Pattern | Examples | -| ----------- | ----------------- | ------------------------------------- | -| export | `Plugin` | `catalogPlugin`, `userSettingsPlugin` | -| ID | `''` | `'catalog'`, `'user-settings'` | +| Description | Pattern | Examples | Notes | +| ----------- | ----------------- | ------------------------------------- | --------------------------------------------------------------------- | +| export | `Plugin` | `catalogPlugin`, `userSettingsPlugin` | | +| ID | `''` | `'catalog'`, `'user-settings'` | letters, digits, dashes, and underscores only, starting with a letter | Example: @@ -27,10 +27,10 @@ export const userSettingsPlugin = createBackendPlugin({ ### Modules -| Description | Pattern | Examples | -| ----------- | ---------------------------- | ----------------------------------- | -| export | `Module` | `catalogModuleGithubEntityProvider` | -| ID | `''` | `'github-entity-provider'` | +| Description | Pattern | Examples | Notes | +| ----------- | ---------------------------- | ----------------------------------- | --------------------------------------------------------------------- | +| export | `Module` | `catalogModuleGithubEntityProvider` | | +| ID | `''` | `'github-entity-provider'` | letters, digits, dashes, and underscores only, starting with a letter | Example: diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index 1c0bf5a0ef..a64743233d 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts @@ -1011,6 +1011,53 @@ describe('BackendInitializer', () => { "Service or extension point dependencies of module 'test-mod' for plugin 'test' are missing for the following ref(s): serviceRef{a}", ); }); + it('should reject plugins with invalid pluginId', async () => { + const init = new BackendInitializer(baseFactories); + init.add( + createBackendPlugin({ + pluginId: 'test:invalid&id', + register(reg) { + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + }), + ); + await expect(init.start()).rejects.toThrow( + "Invalid pluginId 'test:invalid&id', must match the pattern /^[a-z][a-z0-9]*(?:[-_][a-z][a-z0-9]*)*$/i (letters, digits, dashes, and underscores only, starting with a letter)", + ); + }); + + it('should reject modules with invalid moduleId', async () => { + const init = new BackendInitializer(baseFactories); + init.add( + createBackendPlugin({ + pluginId: 'test', + register(reg) { + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + }), + ); + init.add( + createBackendModule({ + pluginId: 'test', + moduleId: 'invalid:module&id', + register(reg) { + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + }), + ); + await expect(init.start()).rejects.toThrow( + "Invalid moduleId 'invalid:module&id' for plugin 'test', must match the pattern /^[a-z][a-z0-9]*(?:[-_][a-z][a-z0-9]*)*$/i (letters, digits, dashes, and underscores only, starting with a letter)", + ); + }); it('should properly load double-default CJS modules', async () => { expect.assertions(3); diff --git a/packages/backend-plugin-api/src/wiring/createBackendModule.ts b/packages/backend-plugin-api/src/wiring/createBackendModule.ts index e2357a65d0..9d064d3b0d 100644 --- a/packages/backend-plugin-api/src/wiring/createBackendModule.ts +++ b/packages/backend-plugin-api/src/wiring/createBackendModule.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { CONFIG_KEY_PART_PATTERN } from '@backstage/config'; import { BackendFeature } from '../types'; import { BackendModuleRegistrationPoints, @@ -55,6 +56,12 @@ export function createBackendModule( options: CreateBackendModuleOptions, ): BackendFeature { function getRegistrations() { + if (!CONFIG_KEY_PART_PATTERN.test(options.moduleId)) { + throw new Error( + `Invalid moduleId '${options.moduleId}' for plugin '${options.pluginId}', must match the pattern ${CONFIG_KEY_PART_PATTERN} (letters, digits, dashes, and underscores only, starting with a letter)`, + ); + } + const extensionPoints: InternalBackendPluginRegistration['extensionPoints'] = []; let init: InternalBackendModuleRegistration['init'] | undefined = undefined; diff --git a/packages/backend-plugin-api/src/wiring/createBackendPlugin.ts b/packages/backend-plugin-api/src/wiring/createBackendPlugin.ts index 185500c495..e4d32197ff 100644 --- a/packages/backend-plugin-api/src/wiring/createBackendPlugin.ts +++ b/packages/backend-plugin-api/src/wiring/createBackendPlugin.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { CONFIG_KEY_PART_PATTERN } from '@backstage/config'; import { BackendFeature } from '../types'; import { BackendPluginRegistrationPoints, @@ -49,6 +50,12 @@ export function createBackendPlugin( options: CreateBackendPluginOptions, ): BackendFeature { function getRegistrations() { + if (!CONFIG_KEY_PART_PATTERN.test(options.pluginId)) { + throw new Error( + `Invalid pluginId '${options.pluginId}', must match the pattern ${CONFIG_KEY_PART_PATTERN} (letters, digits, dashes, and underscores only, starting with a letter)`, + ); + } + const extensionPoints: InternalBackendPluginRegistration['extensionPoints'] = []; let init: InternalBackendPluginRegistration['init'] | undefined = undefined; diff --git a/packages/config/report.api.md b/packages/config/report.api.md index 94c4652b44..2963960973 100644 --- a/packages/config/report.api.md +++ b/packages/config/report.api.md @@ -43,6 +43,9 @@ export type Config = { getOptionalStringArray(key: string): string[] | undefined; }; +// @public +export const CONFIG_KEY_PART_PATTERN: RegExp; + // @public export class ConfigReader implements Config { constructor( diff --git a/packages/config/src/constants.ts b/packages/config/src/constants.ts new file mode 100644 index 0000000000..acd5c50961 --- /dev/null +++ b/packages/config/src/constants.ts @@ -0,0 +1,27 @@ +/* + * 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. + */ + +/** + * The pattern that config keys must match. + * + * @remarks + * keys must only contain the letters `a` through `z` and digits, in groups separated by + * dashes or underscores. Additionally, the very first character of each such group + * must be a letter, not a digit + * + * @public + */ +export const CONFIG_KEY_PART_PATTERN = /^[a-z][a-z0-9]*(?:[-_][a-z0-9]+)*$/i; diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index c6254e97ba..2f3ade0189 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -29,3 +29,4 @@ export type { export { readDurationFromConfig } from './readDurationFromConfig'; export { ConfigReader } from './reader'; export type { AppConfig, Config } from './types'; +export { CONFIG_KEY_PART_PATTERN } from './constants'; diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 8186ee8faa..18f010daac 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -17,8 +17,7 @@ import { JsonObject, JsonValue } from '@backstage/types'; import { AppConfig, Config } from './types'; -// Update the same pattern in config-loader package if this is changed -const CONFIG_KEY_PART_PATTERN = /^[a-z][a-z0-9]*(?:[-_:][a-z0-9]+)*$/i; +import { CONFIG_KEY_PART_PATTERN } from './constants'; function isObject(value: JsonValue | undefined): value is JsonObject { return typeof value === 'object' && value !== null && !Array.isArray(value);