diff --git a/.changeset/perfect-elephants-do.md b/.changeset/perfect-elephants-do.md new file mode 100644 index 0000000000..d5e4c864d7 --- /dev/null +++ b/.changeset/perfect-elephants-do.md @@ -0,0 +1,21 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Use `ScmIntegrationRegistry#resolveUrl` in the placeholder processors instead of a custom implementation. + +If you manually instantiate the `PlaceholderProcessor` (you most probably don't), add the new required constructor parameter: + +```diff ++ import { ScmIntegrations } from '@backstage/integration'; + // ... ++ const integrations = ScmIntegrations.fromConfig(config); + // ... + new PlaceholderProcessor({ + resolvers: placeholderResolvers, + reader, ++ integrations, + }); +``` + +All custom `PlaceholderResolver` can use the new `resolveUrl` parameter to resolve relative URLs. diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts index 0254e854e5..c60b01d53d 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts @@ -15,6 +15,8 @@ */ import { UrlReader } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; +import { ConfigReader } from '@backstage/config'; +import { ScmIntegrations } from '@backstage/integration'; import { jsonPlaceholderResolver, PlaceholderProcessor, @@ -25,6 +27,8 @@ import { yamlPlaceholderResolver, } from './PlaceholderProcessor'; +const integrations = ScmIntegrations.fromConfig(new ConfigReader({})); + describe('PlaceholderProcessor', () => { const read: jest.MockedFunction = jest.fn(); const reader: UrlReader = { read, readTree: jest.fn(), search: jest.fn() }; @@ -44,6 +48,7 @@ describe('PlaceholderProcessor', () => { foo: async () => 'replaced', }, reader, + integrations, }); await expect( processor.preProcessEntity(input, { type: 't', target: 'l' }), @@ -59,6 +64,7 @@ describe('PlaceholderProcessor', () => { upper: upperResolver, }, reader, + integrations, }); await expect( @@ -95,6 +101,7 @@ describe('PlaceholderProcessor', () => { bar: jest.fn(), }, reader, + integrations, }); const entity: Entity = { apiVersion: 'a', @@ -115,6 +122,7 @@ describe('PlaceholderProcessor', () => { bar: jest.fn(), }, reader, + integrations, }); const entity: Entity = { apiVersion: 'a', @@ -134,6 +142,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { text: textPlaceholderResolver }, reader, + integrations, }); await expect( @@ -169,6 +178,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { json: jsonPlaceholderResolver }, reader, + integrations, }); await expect( @@ -202,6 +212,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { yaml: yamlPlaceholderResolver }, reader, + integrations, }); await expect( @@ -235,6 +246,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { text: textPlaceholderResolver }, reader, + integrations, }); await expect( @@ -272,6 +284,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { text: textPlaceholderResolver }, reader, + integrations, }); await expect( @@ -311,6 +324,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { text: textPlaceholderResolver }, reader, + integrations, }); await expect( @@ -331,7 +345,7 @@ describe('PlaceholderProcessor', () => { }, ), ).rejects.toThrow( - 'Placeholder $text could not form a URL out of ./a/b/catalog-info.yaml and ../c/catalog-info.yaml', + 'Placeholder $text could not form a URL out of ./a/b/catalog-info.yaml and ../c/catalog-info.yaml, TypeError: Invalid base URL: ./a/b/catalog-info.yaml', ); expect(read).not.toBeCalled(); @@ -345,6 +359,7 @@ describe('yamlPlaceholderResolver', () => { value: './file.yaml', baseUrl: 'https://github.com/backstage/backstage/a/b/catalog-info.yaml', read, + resolveUrl: (url, base) => integrations.resolveUrl({ url, base }), }; beforeEach(() => { @@ -389,6 +404,7 @@ describe('jsonPlaceholderResolver', () => { value: './file.json', baseUrl: 'https://github.com/backstage/backstage/a/b/catalog-info.yaml', read, + resolveUrl: (url, base) => integrations.resolveUrl({ url, base }), }; beforeEach(() => { diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts index 1b015d7d93..611e166605 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts @@ -17,16 +17,19 @@ import { UrlReader } from '@backstage/backend-common'; import { Entity, LocationSpec } from '@backstage/catalog-model'; import { JsonValue } from '@backstage/config'; +import { ScmIntegrationRegistry } from '@backstage/integration'; import yaml from 'yaml'; import { CatalogProcessor } from './types'; export type ResolverRead = (url: string) => Promise; +export type ResolverResolveUrl = (url: string, base: string) => string; export type ResolverParams = { key: string; value: JsonValue; baseUrl: string; read: ResolverRead; + resolveUrl: ResolverResolveUrl; }; export type PlaceholderResolver = ( @@ -36,6 +39,7 @@ export type PlaceholderResolver = ( type Options = { resolvers: Record; reader: UrlReader; + integrations: ScmIntegrationRegistry; }; /** @@ -103,12 +107,19 @@ export class PlaceholderProcessor implements CatalogProcessor { return this.options.reader.read(url); }; + const resolveUrl = (url: string, base: string): string => + this.options.integrations.resolveUrl({ + url, + base, + }); + return [ await resolver({ key: resolverKey, value: resolverValue, baseUrl: location.target, read, + resolveUrl, }), true, ]; @@ -191,31 +202,27 @@ async function readTextLocation(params: ResolverParams): Promise { } } -function relativeUrl({ key, value, baseUrl }: ResolverParams): string { +function relativeUrl({ + key, + value, + baseUrl, + resolveUrl, +}: ResolverParams): string { if (typeof value !== 'string') { throw new Error( `Placeholder \$${key} expected a string value parameter, in the form of an absolute URL or a relative path`, ); } - let url: URL; try { - // The two-value form of the URL constructor handles relative paths for us - url = new URL(value, baseUrl); - } catch { - try { - // Check whether value is a valid absolute URL on it's own, if not fail. - url = new URL(value); - } catch { - // The only remaining case that isn't support is a relative file path that should be - // resolved using a relative file location. Accessing local file paths can lead to - // path traversal attacks and access to any file on the host system. Implementing this - // would require additional security measures. - throw new Error( - `Placeholder \$${key} could not form a URL out of ${baseUrl} and ${value}`, - ); - } + return resolveUrl(value, baseUrl); + } catch (e) { + // The only remaining case that isn't support is a relative file path that should be + // resolved using a relative file location. Accessing local file paths can lead to + // path traversal attacks and access to any file on the host system. Implementing this + // would require additional security measures. + throw new Error( + `Placeholder \$${key} could not form a URL out of ${baseUrl} and ${value}, ${e}`, + ); } - - return url.toString(); } diff --git a/plugins/catalog-backend/src/next/NextCatalogBuilder.ts b/plugins/catalog-backend/src/next/NextCatalogBuilder.ts index ed690d07b6..262df7525e 100644 --- a/plugins/catalog-backend/src/next/NextCatalogBuilder.ts +++ b/plugins/catalog-backend/src/next/NextCatalogBuilder.ts @@ -385,7 +385,11 @@ export class NextCatalogBuilder { // These are always there no matter what const processors: CatalogProcessor[] = [ - new PlaceholderProcessor({ resolvers: placeholderResolvers, reader }), + new PlaceholderProcessor({ + resolvers: placeholderResolvers, + reader, + integrations, + }), new BuiltinKindsEntityProcessor(), ]; diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 70c1890fbf..e32e071ebc 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -306,7 +306,11 @@ export class CatalogBuilder { // These are always there no matter what const processors: CatalogProcessor[] = [ StaticLocationProcessor.fromConfig(config), - new PlaceholderProcessor({ resolvers: placeholderResolvers, reader }), + new PlaceholderProcessor({ + resolvers: placeholderResolvers, + reader, + integrations, + }), new BuiltinKindsEntityProcessor(), ];