Search result location filtering

Introduces filtering of unsafe search result locations.

Signed-off-by: Iain Billett <iain@roadie.io>

.changeset/sixty-pandas-switch.md

@@ -0,0 +1,8 @@
+---
+'@backstage/plugin-search-backend': minor
+---
+
+Search result location filtering
+
+This change introduces a filter for search results based on their location protocol. The intention is to filter out unsafe or
+malicious values before they can be consumed by the frontend. By default locations must be http/https URLs (or paths).

packages/backend/src/plugins/search.ts

@@ -96,5 +96,6 @@ export default async function createPlugin({
   return await createRouter({
     engine: indexBuilder.getSearchEngine(),
     logger,
+    discovery,
   });
 }

plugins/search-backend/src/service/router.test.ts

@@ -14,10 +14,14 @@
  * limitations under the License.
  */
 
-import { getVoidLogger } from '@backstage/backend-common';
+import {
+  getVoidLogger,
+  PluginEndpointDiscovery,
+} from '@backstage/backend-common';
 import {
   IndexBuilder,
   LunrSearchEngine,
+  SearchEngine,
 } from '@backstage/plugin-search-backend-node';
 import express from 'express';
 import request from 'supertest';
@@ -26,14 +30,22 @@ import { createRouter } from './router';
 
 describe('createRouter', () => {
   let app: express.Express;
+  let mockDiscoveryApi: jest.Mocked<PluginEndpointDiscovery>;
+  let mockSearchEngine: jest.Mocked<SearchEngine>;
 
   beforeAll(async () => {
     const logger = getVoidLogger();
     const searchEngine = new LunrSearchEngine({ logger });
     const indexBuilder = new IndexBuilder({ logger, searchEngine });
+    mockDiscoveryApi = {
+      getBaseUrl: jest.fn(),
+      getExternalBaseUrl: jest.fn().mockResolvedValue('http://localhost:3000/'),
+    };
+
     const router = await createRouter({
       engine: indexBuilder.getSearchEngine(),
       logger,
+      discovery: mockDiscoveryApi,
     });
     app = express().use(router);
   });
@@ -49,5 +61,70 @@ describe('createRouter', () => {
       expect(response.status).toEqual(200);
       expect(response.body).toMatchObject({ results: [] });
     });
+
+    describe('search result filtering', () => {
+      beforeAll(async () => {
+        const logger = getVoidLogger();
+        mockDiscoveryApi = {
+          getBaseUrl: jest.fn(),
+          getExternalBaseUrl: jest
+            .fn()
+            .mockResolvedValue('http://localhost:3000/'),
+        };
+        mockSearchEngine = {
+          index: jest.fn(),
+          setTranslator: jest.fn(),
+          query: jest.fn(),
+        };
+        const indexBuilder = new IndexBuilder({
+          logger,
+          searchEngine: mockSearchEngine,
+        });
+
+        const router = await createRouter({
+          engine: indexBuilder.getSearchEngine(),
+          logger,
+          discovery: mockDiscoveryApi,
+        });
+        app = express().use(router);
+      });
+
+      describe('where the search result set includes unsafe results', () => {
+        const safeResult = {
+          type: 'software-catalog',
+          document: {
+            text: 'safe',
+            title: 'safe-location',
+            // eslint-disable-next-line no-script-url
+            location: '/catalog/default/component/safe',
+          },
+        };
+        beforeEach(() => {
+          mockSearchEngine.query.mockResolvedValue({
+            results: [
+              {
+                type: 'software-catalog',
+                document: {
+                  text: 'unsafe',
+                  title: 'unsafe-location',
+                  // eslint-disable-next-line no-script-url
+                  location: 'javascript:alert("unsafe")',
+                },
+              },
+              safeResult,
+            ],
+            nextPageCursor: '',
+            previousPageCursor: '',
+          });
+        });
+
+        it('removes the unsafe results', async () => {
+          const response = await request(app).get('/query');
+
+          expect(response.status).toEqual(200);
+          expect(response.body).toMatchObject({ results: [safeResult] });
+        });
+      });
+    });
   });
 });

plugins/search-backend/src/service/router.ts

@@ -19,16 +19,42 @@ import Router from 'express-promise-router';
 import { Logger } from 'winston';
 import { SearchQuery, SearchResultSet } from '@backstage/search-common';
 import { SearchEngine } from '@backstage/plugin-search-backend-node';
+import { PluginEndpointDiscovery } from '@backstage/backend-common';
 
 export type RouterOptions = {
   engine: SearchEngine;
   logger: Logger;
+  discovery: PluginEndpointDiscovery;
+  allowedLocationProtocols?: string[];
 };
 
+const defaultAllowedLocationProtocols = ['http:', 'https:'];
+
 export async function createRouter(
   options: RouterOptions,
 ): Promise<express.Router> {
-  const { engine, logger } = options;
+  const {
+    engine,
+    logger,
+    discovery,
+    allowedLocationProtocols = defaultAllowedLocationProtocols,
+  } = options;
+  const baseUrl = await discovery.getExternalBaseUrl('');
+
+  const filterResultSet = ({ results, ...resultSet }: SearchResultSet) => ({
+    ...resultSet,
+    results: results.filter(result => {
+      const protocol = new URL(result.document.location, baseUrl).protocol;
+      const isAllowed = allowedLocationProtocols.includes(protocol);
+      if (!isAllowed) {
+        logger.info(
+          `Rejected search result for "${result.document.title}" as location protocol "${protocol}" is unsafe`,
+        );
+      }
+      return isAllowed;
+    }),
+  });
+
   const router = Router();
   router.get(
     '/query',
@@ -46,8 +72,8 @@ export async function createRouter(
       );
 
       try {
-        const results = await engine?.query(req.query);
-        res.send(results);
+        const resultSet = await engine?.query(req.query);
+        res.send(filterResultSet(resultSet));
       } catch (err) {
         throw new Error(
           `There was a problem performing the search query. ${err}`,