backend-common: lock down UrlReader to only read from allowed URLs

.changeset/rich-turtles-roll.md

@@ -0,0 +1,19 @@
+---
+'@backstage/backend-common': minor
+---
+
+Remove fallback option from `UrlReaders.create` and `UrlReaders.default`, as well as the default fallback reader.
+
+To be able to read data from endpoints outside of the configured integrations, you now need to explicitly allow it by
+adding an entry in the `backend.reading.allow` list. For example:
+
+```yml
+backend:
+  baseUrl: ...
+  reading:
+    allow:
+      - host: example.com
+      - host: '*.examples.org'
+```
+
+Apart from adding the above configuration, most projects should not need to take any action to migrate existing code. If you do happen to have your own fallback reader configured, this needs to be replaced with a reader factory that selects a specific set of URLs to work with. If you where wrapping the existing fallback reader, the new one that handles the allow list is created using `FetchUrlReader.factory`.

app-config.yaml

@@ -16,6 +16,10 @@ backend:
     credentials: true
   csp:
     connect-src: ["'self'", 'http:', 'https:']
+  reading:
+    allow:
+      - host: example.com
+      - host: '*.mozilla.org'
   # workingDirectory: /tmp # Use this to configure a working directory for the scaffolder, defaults to the OS temp-dir
 
 # See README.md in the proxy-backend plugin for information on the configuration format

docs/features/software-catalog/descriptor-format.md

@@ -131,6 +131,19 @@ spec:
     $text: https://petstore.swagger.io/v2/swagger.json
 ```
 
+Note that to be able to read from targets that are outside of the normal
+integration points such as `github.com`, you'll need to explicitly allow it by
+adding an entry in the `backend.reading.allow` list. For example:
+
+```yml
+backend:
+  baseUrl: ...
+  reading:
+    allow:
+      - host: example.com
+      - host: '*.examples.org'
+```
+
 ## Common to All Kinds: The Envelope
 
 The root envelope object has the following structure.

packages/backend-common/config.d.ts

@@ -94,6 +94,25 @@ export interface Config {
       optionsSuccessStatus?: number;
     };
 
+    /**
+     * Configuration related to URL reading, used for example for reading catalog info
+     * files, scaffolder templates, and techdocs content.
+     */
+    reading?: {
+      /**
+       * A list of targets to allow outgoing requests to. Users will be able to make
+       * requests on behalf of the backend to the targets that are allowed by this list.
+       */
+      allow?: Array<{
+        /**
+         * A hostname to allow outgoing requests to, being either a full hostname or
+         * a subdomain wildcard pattern with a leading `*`. For example `example.com`
+         * and `*.example.com` are valid values, `prod.*.example.com` is not.
+         */
+        host: string;
+      }>;
+    };
+
     /**
      * Content Security Policy options.
      *

packages/backend-common/src/reading/FetchUrlReader.test.ts

@@ -0,0 +1,58 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * 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 { ConfigReader } from '@backstage/config';
+import { msw } from '@backstage/test-utils';
+import { setupServer } from 'msw/node';
+import { getVoidLogger } from '../logging';
+import { FetchUrlReader } from './FetchUrlReader';
+import { ReadTreeResponseFactory } from './tree';
+
+describe('FetchUrlReader', () => {
+  const worker = setupServer();
+
+  msw.setupDefaultHandlers(worker);
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('factory should create a single entry with a predicate that matches config', async () => {
+    const entries = FetchUrlReader.factory({
+      config: new ConfigReader({
+        backend: {
+          reading: {
+            allow: [{ host: 'example.com' }, { host: '*.examples.org' }],
+          },
+        },
+      }),
+      logger: getVoidLogger(),
+      treeResponseFactory: ReadTreeResponseFactory.create({
+        config: new ConfigReader({}),
+      }),
+    });
+
+    expect(entries.length).toBe(1);
+    const [{ predicate }] = entries;
+
+    expect(predicate(new URL('https://example.com/test'))).toBe(true);
+    expect(predicate(new URL('https://a.example.com/test'))).toBe(false);
+    expect(predicate(new URL('https://other.com/test'))).toBe(false);
+    expect(predicate(new URL('https://examples.org/test'))).toBe(false);
+    expect(predicate(new URL('https://a.examples.org/test'))).toBe(true);
+    expect(predicate(new URL('https://a.b.examples.org/test'))).toBe(true);
+  });
+});

packages/backend-common/src/reading/FetchUrlReader.ts

@@ -16,12 +16,39 @@
 
 import fetch from 'cross-fetch';
 import { NotFoundError } from '../errors';
-import { ReadTreeResponse, UrlReader } from './types';
+import { ReaderFactory, ReadTreeResponse, UrlReader } from './types';
 
 /**
  * A UrlReader that does a plain fetch of the URL.
  */
 export class FetchUrlReader implements UrlReader {
+  /**
+   * The factory creates a single reader that will be used for reading any URL that's listed
+   * in configuration at `backend.reading.allow`. The allow list contains a list of objects describing
+   * targets to allow, containing the following fields:
+   *
+   * `host`:
+   *   Either full hostnames to match, or subdomain wildcard matchers with a leading `*`.
+   *   For example `example.com` and `*.example.com` are valid values, `prod.*.example.com` is not.
+   */
+  static factory: ReaderFactory = ({ config }) => {
+    const predicates =
+      config
+        .getOptionalConfigArray('backend.reading.allow')
+        ?.map(allowConfig => {
+          const host = allowConfig.getString('host');
+          if (host.startsWith('*.')) {
+            const suffix = host.slice(1);
+            return (url: URL) => url.hostname.endsWith(suffix);
+          }
+          return (url: URL) => url.hostname === host;
+        }) ?? [];
+
+    const reader = new FetchUrlReader();
+    const predicate = (url: URL) => predicates.some(p => p(url));
+    return [{ reader, predicate }];
+  };
+
   async read(url: string): Promise<Buffer> {
     let response: Response;
     try {

packages/backend-common/src/reading/UrlReaderPredicateMux.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { NotAllowedError } from '../errors';
 import {
   ReadTreeOptions,
   ReadTreeResponse,
@@ -21,22 +22,12 @@ import {
   UrlReaderPredicateTuple,
 } from './types';
 
-type Options = {
-  // UrlReader to fall back to if no other reader is matched
-  fallback?: UrlReader;
-};
-
 /**
  * A UrlReader implementation that selects from a set of UrlReaders
  * based on a predicate tied to each reader.
  */
 export class UrlReaderPredicateMux implements UrlReader {
   private readonly readers: UrlReaderPredicateTuple[] = [];
-  private readonly fallback?: UrlReader;
-
-  constructor({ fallback }: Options) {
-    this.fallback = fallback;
-  }
 
   register(tuple: UrlReaderPredicateTuple): void {
     this.readers.push(tuple);
@@ -51,11 +42,7 @@ export class UrlReaderPredicateMux implements UrlReader {
       }
     }
 
-    if (this.fallback) {
-      return this.fallback.read(url);
-    }
-
-    throw new Error(`No reader found that could handle '${url}'`);
+    throw new NotAllowedError(`Reading from '${url}' is not allowed`);
   }
 
   readTree(url: string, options?: ReadTreeOptions): Promise<ReadTreeResponse> {
@@ -67,16 +54,10 @@ export class UrlReaderPredicateMux implements UrlReader {
       }
     }
 
-    if (this.fallback) {
-      return this.fallback.readTree(url, options);
-    }
-
-    throw new Error(`No reader found that could handle '${url}'`);
+    throw new NotAllowedError(`Reading from '${url}' is not allowed`);
   }
 
   toString() {
-    return `predicateMux{readers=${this.readers
-      .map(t => t.reader)
-      .join(',')},fallback=${this.fallback}}`;
+    return `predicateMux{readers=${this.readers.map(t => t.reader).join(',')}`;
   }
 }

packages/backend-common/src/reading/UrlReaders.ts

@@ -22,8 +22,8 @@ import { AzureUrlReader } from './AzureUrlReader';
 import { BitbucketUrlReader } from './BitbucketUrlReader';
 import { GithubUrlReader } from './GithubUrlReader';
 import { GitlabUrlReader } from './GitlabUrlReader';
-import { FetchUrlReader } from './FetchUrlReader';
 import { ReadTreeResponseFactory } from './tree';
+import { FetchUrlReader } from './FetchUrlReader';
 
 type CreateOptions = {
   /** Root config object */
@@ -32,8 +32,6 @@ type CreateOptions = {
   logger: Logger;
   /** A list of factories used to construct individual readers that match on URLs */
   factories?: ReaderFactory[];
-  /** Fallback reader to use if none of the readers created by the factories match */
-  fallback?: UrlReader;
 };
 
 /**
@@ -43,13 +41,8 @@ export class UrlReaders {
   /**
    * Creates a UrlReader without any known types.
    */
-  static create({
-    logger,
-    config,
-    factories,
-    fallback,
-  }: CreateOptions): UrlReader {
-    const mux = new UrlReaderPredicateMux({ fallback: fallback });
+  static create({ logger, config, factories }: CreateOptions): UrlReader {
+    const mux = new UrlReaderPredicateMux();
     const treeResponseFactory = ReadTreeResponseFactory.create({ config });
 
     for (const factory of factories ?? []) {
@@ -67,10 +60,8 @@ export class UrlReaders {
    * Creates a UrlReader that includes all the default factories from this package.
    *
    * Any additional factories passed will be loaded before the default ones.
-   *
-   * If no fallback reader is passed, a plain fetch reader will be used.
    */
-  static default({ logger, config, factories = [], fallback }: CreateOptions) {
+  static default({ logger, config, factories = [] }: CreateOptions) {
     return UrlReaders.create({
       logger,
       config,
@@ -79,8 +70,8 @@ export class UrlReaders {
         BitbucketUrlReader.factory,
         GithubUrlReader.factory,
         GitlabUrlReader.factory,
+        FetchUrlReader.factory,
       ]),
-      fallback: fallback ?? new FetchUrlReader(),
     });
   }
 }

plugins/catalog-backend/src/ingestion/processors/UrlReaderProcessor.test.ts

@@ -27,13 +27,18 @@ import {
 } from './types';
 
 describe('UrlReaderProcessor', () => {
-  const mockApiOrigin = 'http://localhost:23000';
+  const mockApiOrigin = 'http://localhost';
   const server = setupServer();
 
   msw.setupDefaultHandlers(server);
   it('should load from url', async () => {
     const logger = getVoidLogger();
-    const reader = UrlReaders.default({ logger, config: new ConfigReader({}) });
+    const reader = UrlReaders.default({
+      logger,
+      config: new ConfigReader({
+        backend: { reading: { allow: [{ host: 'localhost' }] } },
+      }),
+    });
     const processor = new UrlReaderProcessor({ reader, logger });
     const spec = {
       type: 'url',
@@ -57,7 +62,12 @@ describe('UrlReaderProcessor', () => {
 
   it('should fail load from url with error', async () => {
     const logger = getVoidLogger();
-    const reader = UrlReaders.default({ logger, config: new ConfigReader({}) });
+    const reader = UrlReaders.default({
+      logger,
+      config: new ConfigReader({
+        backend: { reading: { allow: [{ host: 'localhost' }] } },
+      }),
+    });
     const processor = new UrlReaderProcessor({ reader, logger });
     const spec = {
       type: 'url',