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..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 () => { @@ -258,7 +260,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 +287,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 +421,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 +451,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..ef8359632c 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 { handleLCSFetchError } from './utils'; import { isAllowedProxyPath, validateCompletionsRequest } from './validation'; const SKIP_USER_ID_ENDPOINTS = new Set(['/v1/models', '/v1/shields']); @@ -557,15 +558,12 @@ 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); - - response.status(fetchResponse.status).json({ - error: errormsg, - }); - + await handleLCSFetchError( + fetchResponse, + logger, + 'sending feedback', + response, + ); return; } @@ -605,10 +603,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 }); + await handleLCSFetchError( + fetchResponse, + logger, + 'interrupting query', + response, + ); return; } response.status(fetchResponse.status).json(await fetchResponse.json()); @@ -683,15 +683,12 @@ 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); - - response.status(fetchResponse.status).json({ - error: errormsg, - }); - + await handleLCSFetchError( + fetchResponse, + logger, + 'processing query', + response, + ); return; } @@ -739,14 +736,12 @@ 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); - - response.status(fetchResponse.status).json({ - error: errormsg, - }); + await handleLCSFetchError( + fetchResponse, + logger, + 'updating conversation', + response, + ); return; } 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..f13707c446 --- /dev/null +++ b/workspaces/lightspeed/plugins/lightspeed-backend/src/service/utils.ts @@ -0,0 +1,73 @@ +/* + * 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}`; +} + +/** + * 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 }); +}