Merge pull request #7055 from SDA-SE/feat/placeholder-resolve

Use `ScmIntegrationRegistry#resolveUrl` in the placeholder processors instead of a custom implementation

.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.

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<ResolverRead> = 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(() => {

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<Buffer>;
+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<string, PlaceholderResolver>;
   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<string> {
   }
 }
 
-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();
 }

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(),
     ];
 

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(),
     ];