Merge pull request #4128 from backstage/rugvip/fetch

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

packages/backend-common/config.d.ts

@@ -79,6 +79,26 @@ 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 host to allow outgoing requests to, being either a full host or
+         * a subdomain wildcard pattern with a leading `*`. For example `example.com`
+         * and `*.example.com` are valid values, `prod.*.example.com` is not.
+         * The host may also contain a port, for example `example.com:8080`.
+         */
+        host: string;
+      }>;
+    };
+
     /**
      * Content Security Policy options.
      *

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

@@ -0,0 +1,73 @@
+/*
+ * 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: 'example.com:700' },
+              { host: '*.examples.org' },
+              { host: '*.examples.org:700' },
+            ],
+          },
+        },
+      }),
+      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://example.com:600/test'))).toBe(false);
+    expect(predicate(new URL('https://a.example.com:600/test'))).toBe(false);
+    expect(predicate(new URL('https://example.com:700/test'))).toBe(true);
+    expect(predicate(new URL('https://a.example.com:700/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);
+    expect(predicate(new URL('https://examples.org:600/test'))).toBe(false);
+    expect(predicate(new URL('https://a.examples.org:600/test'))).toBe(false);
+    expect(predicate(new URL('https://a.b.examples.org:600/test'))).toBe(false);
+    expect(predicate(new URL('https://examples.org:700/test'))).toBe(false);
+    expect(predicate(new URL('https://a.examples.org:700/test'))).toBe(true);
+    expect(predicate(new URL('https://a.b.examples.org:700/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.host.endsWith(suffix);
+          }
+          return (url: URL) => url.host === 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(),
     });
   }
 }