Merge pull request #8487 from RoadieHQ/search-location-protocol-whitelist

Drop Search Results with potentially unsafe location values

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

@@ -18,6 +18,7 @@ import { getVoidLogger } from '@backstage/backend-common';
 import {
   IndexBuilder,
   LunrSearchEngine,
+  SearchEngine,
 } from '@backstage/plugin-search-backend-node';
 import express from 'express';
 import request from 'supertest';
@@ -26,11 +27,13 @@ import { createRouter } from './router';
 
 describe('createRouter', () => {
   let app: express.Express;
+  let mockSearchEngine: jest.Mocked<SearchEngine>;
 
   beforeAll(async () => {
     const logger = getVoidLogger();
     const searchEngine = new LunrSearchEngine({ logger });
     const indexBuilder = new IndexBuilder({ logger, searchEngine });
+
     const router = await createRouter({
       engine: indexBuilder.getSearchEngine(),
       logger,
@@ -49,5 +52,63 @@ describe('createRouter', () => {
       expect(response.status).toEqual(200);
       expect(response.body).toMatchObject({ results: [] });
     });
+
+    describe('search result filtering', () => {
+      beforeAll(async () => {
+        const logger = getVoidLogger();
+        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,
+        });
+        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

@@ -25,10 +25,28 @@ export type RouterOptions = {
   logger: Logger;
 };
 
+const allowedLocationProtocols = ['http:', 'https:'];
+
 export async function createRouter(
   options: RouterOptions,
 ): Promise<express.Router> {
   const { engine, logger } = options;
+
+  const filterResultSet = ({ results, ...resultSet }: SearchResultSet) => ({
+    ...resultSet,
+    results: results.filter(result => {
+      const protocol = new URL(result.document.location, 'https://example.com')
+        .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 +64,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}`,