From 12082fcf64b7b4525498fd1877c63510f387ae09 Mon Sep 17 00:00:00 2001 From: Rajin Kichannagari Date: Thu, 4 Jun 2026 14:09:49 -0400 Subject: [PATCH 1/3] Security: Sanitize Lightspeed Core error responses Fixes information disclosure vulnerability where LCS error details were being forwarded directly to clients. Changes: - Add sanitizeLcsError() function to sanitize error responses - Update 4 endpoints to use sanitization - Enhance tests to verify internal details are not exposed --- .../.changeset/long-penguins-rhyme.md | 5 ++ .../src/service/router.test.ts | 26 +++++++- .../lightspeed-backend/src/service/router.ts | 63 ++++++++++++++----- 3 files changed, 77 insertions(+), 17 deletions(-) create mode 100644 workspaces/lightspeed/.changeset/long-penguins-rhyme.md diff --git a/workspaces/lightspeed/.changeset/long-penguins-rhyme.md b/workspaces/lightspeed/.changeset/long-penguins-rhyme.md new file mode 100644 index 0000000000..b1e9d0215d --- /dev/null +++ b/workspaces/lightspeed/.changeset/long-penguins-rhyme.md @@ -0,0 +1,5 @@ +--- +'@red-hat-developer-hub/backstage-plugin-lightspeed-backend': patch +--- + +Security: Sanitize LCS error responses to prevent information disclosure diff --git a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts index c181b733f8..ff9426fe7b 100644 --- a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts +++ b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts @@ -258,7 +258,13 @@ describe('lightspeed router tests', () => { return new HttpResponse( JSON.stringify({ error: { - message: 'Internal server error', + message: + 'Model gpt-4-0613 failed with OpenAI API error: rate limit exceeded for organization org-abc123', + }, + detail: { + cause: 'OpenAIError: Rate limit reached', + provider: 'openai', + model_id: 'gpt-4-0613', }, }), { @@ -279,6 +285,12 @@ describe('lightspeed router tests', () => { expect(response.body.error).toContain( 'Error from lightspeed-core server', ); + // Verify internal details are NOT exposed + expect(response.body.error).not.toContain('gpt-4'); + expect(response.body.error).not.toContain('OpenAI'); + expect(response.body.error).not.toContain('org-abc123'); + expect(response.body.error).not.toContain('openai'); + expect(response.body.error).not.toContain('rate limit'); }); }); @@ -407,7 +419,12 @@ describe('lightspeed router tests', () => { return new HttpResponse( JSON.stringify({ error: { - message: 'Internal server error', + message: + 'Database connection failed at /app/db/postgres.py:142', + }, + detail: { + cause: 'PostgreSQL connection timeout', + trace_id: 'req_xyz789', }, }), { @@ -432,6 +449,11 @@ describe('lightspeed router tests', () => { expect(response.body.error).toContain( 'Error from lightspeed-core server', ); + // Verify internal details are NOT exposed + expect(response.body.error).not.toContain('Database'); + expect(response.body.error).not.toContain('postgres.py'); + expect(response.body.error).not.toContain('PostgreSQL'); + expect(response.body.error).not.toContain('req_xyz789'); }); }); diff --git a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts index a6402d7026..78a56d0798 100644 --- a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts +++ b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts @@ -61,6 +61,30 @@ interface StaticMcpServer { token?: string; } +/** + * Sanitizes Lightspeed Core Service (LCS) error responses to prevent + * information disclosure. Logs full error details server-side for debugging + * while returning only generic messages to clients. + * + * @param errorBody - The error response body from LCS + * @param logger - Logger instance for server-side logging + * @param context - Context string describing the operation (e.g., "sending feedback") + * @returns Generic error message safe to return to clients + */ +function sanitizeLcsError( + errorBody: any, + logger: any, + context: string, +): string { + // Log full error details server-side for debugging + logger.error( + `Error from lightspeed-core server while ${context}: ${JSON.stringify(errorBody)}`, + ); + + // Return only generic message to client (no internal LCS details) + return `Error from lightspeed-core server while ${context}`; +} + /** * Build MCP-HEADERS for LCS. Format matches the LCS "client" auth model: * { "server-name": { "Authorization": "" } } @@ -557,13 +581,15 @@ export async function createRouter( ); if (!fetchResponse.ok) { - // Read the error body const errorBody = await fetchResponse.json(); - const errormsg = `Error from lightspeed-core server: ${errorBody.error?.message || errorBody?.detail?.cause || 'Unknown error'}`; - logger.error(errormsg); + const sanitizedError = sanitizeLcsError( + errorBody, + logger, + 'sending feedback', + ); response.status(fetchResponse.status).json({ - error: errormsg, + error: sanitizedError, }); return; @@ -606,9 +632,12 @@ export async function createRouter( ); if (!fetchResponse.ok) { const errorBody = await fetchResponse.json(); - const errormsg = `Error from lightspeed-core server: ${errorBody.error?.message || errorBody?.detail?.cause || 'Unknown error'}`; - logger.error(errormsg); - response.status(fetchResponse.status).json({ error: errormsg }); + const sanitizedError = sanitizeLcsError( + errorBody, + logger, + 'interrupting query', + ); + response.status(fetchResponse.status).json({ error: sanitizedError }); return; } response.status(fetchResponse.status).json(await fetchResponse.json()); @@ -683,13 +712,15 @@ export async function createRouter( ); if (!fetchResponse.ok) { - // Read the error body const errorBody = await fetchResponse.json(); - const errormsg = `Error from lightspeed-core server: ${errorBody.error?.message || errorBody?.detail?.cause || 'Unknown error'}`; - logger.error(errormsg); + const sanitizedError = sanitizeLcsError( + errorBody, + logger, + 'processing query', + ); response.status(fetchResponse.status).json({ - error: errormsg, + error: sanitizedError, }); return; @@ -739,13 +770,15 @@ export async function createRouter( }, ); if (!fetchResponse.ok) { - // Read the error body const errorBody = await fetchResponse.json(); - const errormsg = `Error from lightspeed-core server: ${errorBody.error?.message || errorBody?.detail?.cause || 'Unknown error'}`; - logger.error(errormsg); + const sanitizedError = sanitizeLcsError( + errorBody, + logger, + 'updating conversation', + ); response.status(fetchResponse.status).json({ - error: errormsg, + error: sanitizedError, }); return; } From b4cd2e8bc30322c4fd2b55daa1b366a6ae05b3ad Mon Sep 17 00:00:00 2001 From: Rajin Kichannagari Date: Thu, 4 Jun 2026 15:58:10 -0400 Subject: [PATCH 2/3] Address PR feedback - Move sanitizeLCSError to utils.ts for reusability - Fix TypeScript types (use LoggerService, add LCSErrorResponse interface) - Capitalize LCS in function name - Fix failing test for conversation not found - Add unit tests for utils to improve coverage --- .../src/service/router.test.ts | 4 +- .../lightspeed-backend/src/service/router.ts | 33 +----- .../src/service/utils.test.ts | 106 ++++++++++++++++++ .../lightspeed-backend/src/service/utils.ts | 53 +++++++++ 4 files changed, 167 insertions(+), 29 deletions(-) create mode 100644 workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.test.ts create mode 100644 workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.ts diff --git a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts index ff9426fe7b..4b47d5c2fd 100644 --- a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts +++ b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts @@ -247,7 +247,9 @@ describe('lightspeed router tests', () => { }); expect(response.statusCode).toEqual(404); - expect(response.body.error).toContain('not found'); + expect(response.body.error).toContain( + 'Error from lightspeed-core server', + ); }); it('should handle upstream server errors properly', async () => { diff --git a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts index 78a56d0798..d16852010c 100644 --- a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts +++ b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts @@ -52,6 +52,7 @@ import { QueryRequestBody, RouterOptions, } from './types'; +import { sanitizeLCSError } from './utils'; import { isAllowedProxyPath, validateCompletionsRequest } from './validation'; const SKIP_USER_ID_ENDPOINTS = new Set(['/v1/models', '/v1/shields']); @@ -61,30 +62,6 @@ interface StaticMcpServer { token?: string; } -/** - * Sanitizes Lightspeed Core Service (LCS) error responses to prevent - * information disclosure. Logs full error details server-side for debugging - * while returning only generic messages to clients. - * - * @param errorBody - The error response body from LCS - * @param logger - Logger instance for server-side logging - * @param context - Context string describing the operation (e.g., "sending feedback") - * @returns Generic error message safe to return to clients - */ -function sanitizeLcsError( - errorBody: any, - logger: any, - context: string, -): string { - // Log full error details server-side for debugging - logger.error( - `Error from lightspeed-core server while ${context}: ${JSON.stringify(errorBody)}`, - ); - - // Return only generic message to client (no internal LCS details) - return `Error from lightspeed-core server while ${context}`; -} - /** * Build MCP-HEADERS for LCS. Format matches the LCS "client" auth model: * { "server-name": { "Authorization": "" } } @@ -582,7 +559,7 @@ export async function createRouter( if (!fetchResponse.ok) { const errorBody = await fetchResponse.json(); - const sanitizedError = sanitizeLcsError( + const sanitizedError = sanitizeLCSError( errorBody, logger, 'sending feedback', @@ -632,7 +609,7 @@ export async function createRouter( ); if (!fetchResponse.ok) { const errorBody = await fetchResponse.json(); - const sanitizedError = sanitizeLcsError( + const sanitizedError = sanitizeLCSError( errorBody, logger, 'interrupting query', @@ -713,7 +690,7 @@ export async function createRouter( if (!fetchResponse.ok) { const errorBody = await fetchResponse.json(); - const sanitizedError = sanitizeLcsError( + const sanitizedError = sanitizeLCSError( errorBody, logger, 'processing query', @@ -771,7 +748,7 @@ export async function createRouter( ); if (!fetchResponse.ok) { const errorBody = await fetchResponse.json(); - const sanitizedError = sanitizeLcsError( + const sanitizedError = sanitizeLCSError( errorBody, logger, 'updating conversation', diff --git a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.test.ts b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.test.ts new file mode 100644 index 0000000000..3819ccafae --- /dev/null +++ b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.test.ts @@ -0,0 +1,106 @@ +/* + * Copyright Red Hat, Inc. + * + * 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 { sanitizeLCSError } from './utils'; + +describe('sanitizeLCSError', () => { + const mockLogger = { + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + child: jest.fn(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return generic error message to client', () => { + const errorBody = { + error: { + message: + 'Model gpt-4-0613 failed with OpenAI API error: rate limit exceeded', + }, + detail: { + cause: 'OpenAIError: Rate limit reached', + }, + }; + + const result = sanitizeLCSError(errorBody, mockLogger, 'processing query'); + + expect(result).toBe( + 'Error from lightspeed-core server while processing query', + ); + }); + + it('should log full error details server-side', () => { + const errorBody = { + error: { + message: 'Database connection failed', + }, + detail: { + cause: 'PostgreSQL connection timeout', + }, + }; + + sanitizeLCSError(errorBody, mockLogger, 'sending feedback'); + + expect(mockLogger.error).toHaveBeenCalledWith( + `Error from lightspeed-core server while sending feedback: ${JSON.stringify(errorBody)}`, + ); + }); + + it('should not expose internal details in return value', () => { + const errorBody = { + error: { + message: + 'Model gpt-4-0613 failed with organization org-abc123 rate limit', + }, + detail: { + cause: 'OpenAIError', + provider: 'openai', + model_id: 'gpt-4-0613', + }, + }; + + const result = sanitizeLCSError( + errorBody, + mockLogger, + 'interrupting query', + ); + + expect(result).not.toContain('gpt-4'); + expect(result).not.toContain('org-abc123'); + expect(result).not.toContain('openai'); + expect(result).not.toContain('OpenAIError'); + }); + + it('should handle empty error body', () => { + const errorBody = {}; + + const result = sanitizeLCSError( + errorBody, + mockLogger, + 'updating conversation', + ); + + expect(result).toBe( + 'Error from lightspeed-core server while updating conversation', + ); + expect(mockLogger.error).toHaveBeenCalled(); + }); +}); diff --git a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.ts b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.ts new file mode 100644 index 0000000000..26ff4a3e55 --- /dev/null +++ b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.ts @@ -0,0 +1,53 @@ +/* + * Copyright Red Hat, Inc. + * + * 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 { LoggerService } from '@backstage/backend-plugin-api'; + +/** + * Interface for Lightspeed Core Service error response structure + */ +export interface LCSErrorResponse { + error?: { + message?: string; + }; + detail?: { + cause?: string; + }; +} + +/** + * Sanitizes Lightspeed Core Service (LCS) error responses to prevent + * information disclosure. Logs full error details server-side for debugging + * while returning only generic messages to clients. + * + * @param errorBody - The error response body from LCS + * @param logger - Logger instance for server-side logging + * @param context - Context string describing the operation (e.g., "sending feedback") + * @returns Generic error message safe to return to clients + */ +export function sanitizeLCSError( + errorBody: LCSErrorResponse, + logger: LoggerService, + context: string, +): string { + // Log full error details server-side for debugging + logger.error( + `Error from lightspeed-core server while ${context}: ${JSON.stringify(errorBody)}`, + ); + + // Return only generic message to client (no internal LCS details) + return `Error from lightspeed-core server while ${context}`; +} From a9af3cd6d2062493a0985b7fc5d264e857ed0fac Mon Sep 17 00:00:00 2001 From: Rajin Kichannagari Date: Thu, 4 Jun 2026 16:19:46 -0400 Subject: [PATCH 3/3] Reduce code duplication with handleLCSFetchError helper Extract duplicated error handling pattern into a reusable helper function. This reduces duplication from 5.6% to under 3%. --- .../lightspeed-backend/src/service/router.ts | 41 ++++++------------- .../lightspeed-backend/src/service/utils.ts | 20 +++++++++ 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts index d16852010c..ef8359632c 100644 --- a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts +++ b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts @@ -52,7 +52,7 @@ import { QueryRequestBody, RouterOptions, } from './types'; -import { sanitizeLCSError } from './utils'; +import { handleLCSFetchError } from './utils'; import { isAllowedProxyPath, validateCompletionsRequest } from './validation'; const SKIP_USER_ID_ENDPOINTS = new Set(['/v1/models', '/v1/shields']); @@ -558,17 +558,12 @@ export async function createRouter( ); if (!fetchResponse.ok) { - const errorBody = await fetchResponse.json(); - const sanitizedError = sanitizeLCSError( - errorBody, + await handleLCSFetchError( + fetchResponse, logger, 'sending feedback', + response, ); - - response.status(fetchResponse.status).json({ - error: sanitizedError, - }); - return; } @@ -608,13 +603,12 @@ export async function createRouter( }, ); if (!fetchResponse.ok) { - const errorBody = await fetchResponse.json(); - const sanitizedError = sanitizeLCSError( - errorBody, + await handleLCSFetchError( + fetchResponse, logger, 'interrupting query', + response, ); - response.status(fetchResponse.status).json({ error: sanitizedError }); return; } response.status(fetchResponse.status).json(await fetchResponse.json()); @@ -689,17 +683,12 @@ export async function createRouter( ); if (!fetchResponse.ok) { - const errorBody = await fetchResponse.json(); - const sanitizedError = sanitizeLCSError( - errorBody, + await handleLCSFetchError( + fetchResponse, logger, 'processing query', + response, ); - - response.status(fetchResponse.status).json({ - error: sanitizedError, - }); - return; } @@ -747,16 +736,12 @@ export async function createRouter( }, ); if (!fetchResponse.ok) { - const errorBody = await fetchResponse.json(); - const sanitizedError = sanitizeLCSError( - errorBody, + await handleLCSFetchError( + fetchResponse, logger, 'updating conversation', + response, ); - - response.status(fetchResponse.status).json({ - error: sanitizedError, - }); return; } diff --git a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.ts b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.ts index 26ff4a3e55..f13707c446 100644 --- a/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.ts +++ b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.ts @@ -51,3 +51,23 @@ export function sanitizeLCSError( // Return only generic message to client (no internal LCS details) return `Error from lightspeed-core server while ${context}`; } + +/** + * Handles LCS fetch errors by sanitizing the error response and sending it to the client. + * This helper eliminates code duplication across multiple endpoints. + * + * @param fetchResponse - The failed fetch response from LCS + * @param logger - Logger instance for server-side logging + * @param context - Context string describing the operation + * @param response - Express response object + */ +export async function handleLCSFetchError( + fetchResponse: Response, + logger: LoggerService, + context: string, + response: any, +): Promise { + const errorBody = await fetchResponse.json(); + const sanitizedError = sanitizeLCSError(errorBody, logger, context); + response.status(fetchResponse.status).json({ error: sanitizedError }); +}