diff --git a/.changeset/sixty-pandas-switch.md b/.changeset/sixty-pandas-switch.md new file mode 100644 index 0000000000..91e767e154 --- /dev/null +++ b/.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). diff --git a/packages/backend/src/plugins/search.ts b/packages/backend/src/plugins/search.ts index 9a8db0f0f9..5659968e1d 100644 --- a/packages/backend/src/plugins/search.ts +++ b/packages/backend/src/plugins/search.ts @@ -96,5 +96,6 @@ export default async function createPlugin({ return await createRouter({ engine: indexBuilder.getSearchEngine(), logger, + discovery, }); } diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index 4b3cb30264..aeb5709379 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/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; + let mockSearchEngine: jest.Mocked; 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] }); + }); + }); + }); }); }); diff --git a/plugins/search-backend/src/service/router.ts b/plugins/search-backend/src/service/router.ts index 5bd99988a7..e13e0cc3bf 100644 --- a/plugins/search-backend/src/service/router.ts +++ b/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 { - 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}`,