From 69fb975e2161a7b132235103a48d47f80786549b Mon Sep 17 00:00:00 2001 From: abinavsridhar-wk Date: Mon, 19 May 2025 13:09:56 +0200 Subject: [PATCH 1/3] Fix: 29961: Do not propogate SQL query to API response Signed-off-by: abinavsridhar-wk --- .changeset/wicked-socks-share.md | 5 +++++ plugins/search-backend/src/service/router.ts | 11 +++++++++++ 2 files changed, 16 insertions(+) create mode 100644 .changeset/wicked-socks-share.md diff --git a/.changeset/wicked-socks-share.md b/.changeset/wicked-socks-share.md new file mode 100644 index 0000000000..d3e1275732 --- /dev/null +++ b/.changeset/wicked-socks-share.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-search-backend': patch +--- + +Error messages should not contain backend SQL query strings in the API response, this change will ensure that messages are logged and empty response is returned to the user diff --git a/plugins/search-backend/src/service/router.ts b/plugins/search-backend/src/service/router.ts index 0ba5eede81..51d32e9e95 100644 --- a/plugins/search-backend/src/service/router.ts +++ b/plugins/search-backend/src/service/router.ts @@ -201,6 +201,17 @@ export async function createRouter( // re-throw and let the default error handler middleware captures it and serializes it with the right response code on the standard form throw error; } + if (query.term.indexOf('<') !== -1) { + // Search queries that contain '<' are likely to be a SQL injection attack, log the error and return an empty response + const message = (error as any)?.message || ''; + if ( + typeof message === 'string' && + message.indexOf('syntax error in tsquery') !== -1 + ) { + logger.info('Search query skipped due to tsquery syntax error.'); + return; // Do not throw an error and just skip the search + } + } throw new Error( `There was a problem performing the search query: ${error.message}`, From 5ea47166b28d5e0a7ea6667e602fd098364515c0 Mon Sep 17 00:00:00 2001 From: abinavsridhar-wk Date: Wed, 21 May 2025 16:20:21 +0200 Subject: [PATCH 2/3] Address comments Signed-off-by: abinavsridhar-wk --- plugins/search-backend/src/service/router.ts | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/plugins/search-backend/src/service/router.ts b/plugins/search-backend/src/service/router.ts index 51d32e9e95..eedd84105a 100644 --- a/plugins/search-backend/src/service/router.ts +++ b/plugins/search-backend/src/service/router.ts @@ -197,25 +197,17 @@ export async function createRouter( res.json(filterResultSet(toSearchResults(resultSet))); } catch (error) { + // Log the error message here, but don't expose it to the user in the response + logger.error( + `There was a problem performing the search query: ${error.message}`, + ); if (error.name === 'MissingIndexError') { // re-throw and let the default error handler middleware captures it and serializes it with the right response code on the standard form throw error; } - if (query.term.indexOf('<') !== -1) { - // Search queries that contain '<' are likely to be a SQL injection attack, log the error and return an empty response - const message = (error as any)?.message || ''; - if ( - typeof message === 'string' && - message.indexOf('syntax error in tsquery') !== -1 - ) { - logger.info('Search query skipped due to tsquery syntax error.'); - return; // Do not throw an error and just skip the search - } - } - throw new Error( - `There was a problem performing the search query: ${error.message}`, - ); + // If the error is not a MissingIndexError, we want to throw a generic error without the error message as it may leak internal information + throw new Error(`There was a problem performing the search query`); } }); From a365cbacf289fccc0f356bf4a9cdc7f61a9bccc5 Mon Sep 17 00:00:00 2001 From: abinavsridhar-wk Date: Sat, 7 Jun 2025 19:57:22 +0200 Subject: [PATCH 3/3] Fix test cases Signed-off-by: abinavsridhar-wk --- plugins/search-backend/src/service/router.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index 5aea71b7c1..623b78016a 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/plugins/search-backend/src/service/router.test.ts @@ -106,7 +106,7 @@ describe('createRouter', () => { expect.objectContaining({ error: { name: 'Error', - message: `There was a problem performing the search query: ${error.message}`, + message: `There was a problem performing the search query`, }, }), );