From 2c24b54779a58a8859309347398f99c94f3322fa Mon Sep 17 00:00:00 2001 From: hannahwestra25 Date: Mon, 20 Jul 2026 16:49:01 -0400 Subject: [PATCH 1/5] Sanitize externally visible error responses Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2040f373-2101-4e34-a067-70c15ac7211e --- frontend/src/auth/AuthProvider.test.tsx | 11 +- frontend/src/auth/AuthProvider.tsx | 4 +- .../src/components/ErrorBoundary.test.tsx | 7 +- frontend/src/components/ErrorBoundary.tsx | 4 +- frontend/src/services/errors.test.ts | 34 +++-- frontend/src/services/errors.ts | 24 +++- frontend/src/utils/messageMapper.test.ts | 32 +++++ frontend/src/utils/messageMapper.ts | 18 ++- pyrit/backend/exceptions.py | 8 ++ pyrit/backend/middleware/error_handlers.py | 12 +- pyrit/backend/models/attacks.py | 27 +++- pyrit/backend/routes/attacks.py | 11 +- pyrit/backend/routes/converters.py | 27 +++- pyrit/backend/routes/initializers.py | 21 ++- pyrit/backend/routes/scenarios.py | 7 +- pyrit/backend/routes/targets.py | 16 ++- pyrit/backend/services/attack_service.py | 25 ++-- pyrit/backend/services/converter_service.py | 7 +- .../backend/services/scenario_run_service.py | 111 ++++++++++++--- pyrit/backend/services/target_service.py | 7 +- tests/unit/backend/test_api_routes.py | 128 ++++++++++++++++-- tests/unit/backend/test_attack_service.py | 16 ++- tests/unit/backend/test_error_handlers.py | 31 ++++- .../unit/backend/test_initializer_service.py | 8 +- .../unit/backend/test_scenario_run_routes.py | 74 ++++++++-- .../unit/backend/test_scenario_run_service.py | 77 +++++++---- 26 files changed, 606 insertions(+), 141 deletions(-) create mode 100644 pyrit/backend/exceptions.py diff --git a/frontend/src/auth/AuthProvider.test.tsx b/frontend/src/auth/AuthProvider.test.tsx index e96e138087..406dabc8c5 100644 --- a/frontend/src/auth/AuthProvider.test.tsx +++ b/frontend/src/auth/AuthProvider.test.tsx @@ -146,9 +146,11 @@ describe("AuthProvider", () => { }); }); - // Test 13: fetchAuthConfig rejects with Error → err.message shown - it("shows error when initialization fails with Error", async () => { - mockFetchAuthConfig.mockRejectedValue(new Error("Config fetch failed")); + // Test 13: fetchAuthConfig rejects with Error → generic message shown + it("shows a generic error when initialization fails with Error", async () => { + mockFetchAuthConfig.mockRejectedValue( + new Error("token secret=sk-test failed at C:\\internal\\auth.ts") + ); render( @@ -158,7 +160,8 @@ describe("AuthProvider", () => { await waitFor(() => { expect(screen.getByText("Authentication Error")).toBeVisible(); - expect(screen.getByText("Config fetch failed")).toBeVisible(); + expect(screen.getByText("Failed to initialize authentication")).toBeVisible(); + expect(screen.queryByText(/sk-test|internal\\auth/i)).not.toBeInTheDocument(); }); }); diff --git a/frontend/src/auth/AuthProvider.tsx b/frontend/src/auth/AuthProvider.tsx index d619d03a48..a3291d5697 100644 --- a/frontend/src/auth/AuthProvider.tsx +++ b/frontend/src/auth/AuthProvider.tsx @@ -133,9 +133,9 @@ export function AuthProvider({ children }: AuthProviderProps) { setMsalInstance(instance) setAuthConfig(config) } - } catch (err) { + } catch { if (!cancelled) { - setError(err instanceof Error ? err.message : 'Failed to initialize authentication') + setError('Failed to initialize authentication') } } } diff --git a/frontend/src/components/ErrorBoundary.test.tsx b/frontend/src/components/ErrorBoundary.test.tsx index 5a99547486..dc8ee32b55 100644 --- a/frontend/src/components/ErrorBoundary.test.tsx +++ b/frontend/src/components/ErrorBoundary.test.tsx @@ -4,7 +4,7 @@ import { ErrorBoundary } from './ErrorBoundary' // Component that throws on render function ThrowingChild({ shouldThrow }: { shouldThrow: boolean }) { if (shouldThrow) { - throw new Error('Test crash') + throw new Error('secret=sk-test at C:\\internal\\component.tsx') } return
OK
} @@ -31,7 +31,7 @@ describe('ErrorBoundary', () => { expect(screen.queryByTestId('error-boundary-fallback')).toBeNull() }) - it('catches render error and shows fallback', () => { + it('catches render error without showing internal details', () => { render( @@ -39,7 +39,8 @@ describe('ErrorBoundary', () => { ) expect(screen.getByTestId('error-boundary-fallback')).toBeInTheDocument() - expect(screen.getByText(/test crash/i)).toBeInTheDocument() + expect(screen.getByText(/something went wrong\. please try again\./i)).toBeInTheDocument() + expect(screen.queryByText(/sk-test|internal\\component/i)).not.toBeInTheDocument() expect(screen.getByText('Try again')).toBeInTheDocument() }) diff --git a/frontend/src/components/ErrorBoundary.tsx b/frontend/src/components/ErrorBoundary.tsx index 56d54a5f31..edfd72bf41 100644 --- a/frontend/src/components/ErrorBoundary.tsx +++ b/frontend/src/components/ErrorBoundary.tsx @@ -4,7 +4,7 @@ import type { FallbackProps } from 'react-error-boundary' import type { ReactNode } from 'react' import { Button, MessageBar, MessageBarBody, tokens } from '@fluentui/react-components' -function ErrorFallback({ error, resetErrorBoundary }: FallbackProps) { +function ErrorFallback({ resetErrorBoundary }: FallbackProps) { const [crashCount, setCrashCount] = useState(1) const handleRetry = () => { @@ -31,7 +31,7 @@ function ErrorFallback({ error, resetErrorBoundary }: FallbackProps) { > - Something went wrong: {error instanceof Error ? error.message : 'Unknown error'} + Something went wrong. Please try again. diff --git a/frontend/src/services/errors.test.ts b/frontend/src/services/errors.test.ts index adec09d588..d1c34dc958 100644 --- a/frontend/src/services/errors.test.ts +++ b/frontend/src/services/errors.test.ts @@ -56,16 +56,18 @@ describe('toApiError', () => { }) // 2. Axios error with plain-string body (e.g. proxy HTML) - it('uses a plain-string response body as detail', () => { + it('sanitizes a plain-string server error response', () => { + const internalDetail = 'C:\\internal\\proxy.conf secret=sk-test' const err = makeAxiosError({ status: 502, - data: 'Bad Gateway', + data: internalDetail, }) const result = toApiError(err) expect(result.status).toBe(502) - expect(result.detail).toBe('Bad Gateway') + expect(result.detail).toBe('The server could not complete the request. Please try again.') + expect(result.detail).not.toContain(internalDetail) expect(result.type).toBeUndefined() expect(result.isNetworkError).toBe(false) }) @@ -80,7 +82,7 @@ describe('toApiError', () => { const result = toApiError(err) expect(result.status).toBe(500) - expect(result.detail).toBe('Server error (500)') + expect(result.detail).toBe('The server could not complete the request. Please try again.') expect(result.type).toBeUndefined() }) @@ -116,24 +118,26 @@ describe('toApiError', () => { }) // 6. Non-Axios Error instance - it('uses Error.message for non-Axios Error instances', () => { - const err = new Error('Something broke') + it('sanitizes non-Axios Error messages', () => { + const err = new Error('secret=sk-test at C:\\internal\\runtime.ts') const result = toApiError(err) expect(result.status).toBeNull() - expect(result.detail).toBe('Something broke') + expect(result.detail).toBe('An unexpected error occurred.') + expect(result.detail).not.toContain('sk-test') expect(result.isNetworkError).toBe(false) expect(result.isTimeout).toBe(false) expect(result.raw).toBe(err) }) // 7. String throw - it('uses the string directly for string throws', () => { - const result = toApiError('unexpected failure') + it('sanitizes string throws', () => { + const result = toApiError('provider deployment secret-model failed') expect(result.status).toBeNull() - expect(result.detail).toBe('unexpected failure') + expect(result.detail).toBe('An unexpected error occurred.') + expect(result.detail).not.toContain('secret-model') expect(result.isNetworkError).toBe(false) }) @@ -163,6 +167,16 @@ describe('toApiError', () => { expect(result.detail).toBe('Server error (403)') }) + it('does not expose plain-string client error bodies', () => { + const internalDetail = 'provider token secret=sk-test at C:\\internal\\proxy.conf' + const err = makeAxiosError({ status: 400, data: internalDetail }) + + const result = toApiError(err) + + expect(result.detail).toBe('Server error (400)') + expect(result.detail).not.toContain(internalDetail) + }) + // 11. Error with empty message it('falls back to generic message for Error with empty message', () => { const err = new Error('') diff --git a/frontend/src/services/errors.ts b/frontend/src/services/errors.ts index b328a4d202..ebc3965729 100644 --- a/frontend/src/services/errors.ts +++ b/frontend/src/services/errors.ts @@ -22,12 +22,15 @@ export interface ApiError { raw: unknown } +const UNEXPECTED_ERROR_DETAIL = 'An unexpected error occurred.' +const SERVER_ERROR_DETAIL = 'The server could not complete the request. Please try again.' + /** * Convert any caught value into a normalized {@link ApiError}. * * Handles: * - Axios errors with an RFC 7807 JSON body (`response.data.detail`) - * - Axios errors with a plain-string body (e.g. nginx 502 HTML) + * - Axios errors with a plain-string body (sanitized rather than displayed) * - Axios errors with no response at all (network / CORS) * - Axios timeout errors (`code === 'ECONNABORTED'`) * - Plain `Error` instances @@ -61,6 +64,15 @@ export function toApiError(err: unknown): ApiError { // We have an HTTP response — try to extract RFC 7807 detail const { status, data } = err.response + if (status >= 500) { + return { + status, + detail: SERVER_ERROR_DETAIL, + isNetworkError: false, + isTimeout: false, + raw: err, + } + } const { detail, type } = extractDetail(data) return { @@ -77,7 +89,7 @@ export function toApiError(err: unknown): ApiError { if (err instanceof Error) { return { status: null, - detail: err.message || 'An unexpected error occurred.', + detail: UNEXPECTED_ERROR_DETAIL, isNetworkError: false, isTimeout: false, raw: err, @@ -88,7 +100,7 @@ export function toApiError(err: unknown): ApiError { if (typeof err === 'string') { return { status: null, - detail: err, + detail: UNEXPECTED_ERROR_DETAIL, isNetworkError: false, isTimeout: false, raw: err, @@ -98,7 +110,7 @@ export function toApiError(err: unknown): ApiError { // Unknown throw (null, undefined, number, object, etc.) return { status: null, - detail: 'An unexpected error occurred.', + detail: UNEXPECTED_ERROR_DETAIL, isNetworkError: false, isTimeout: false, raw: err, @@ -119,12 +131,12 @@ function isAxiosError(err: unknown): err is AxiosError { * * The body may be: * - An RFC 7807 JSON object with `.detail` and optionally `.type` - * - A plain string (e.g. nginx HTML error page) + * - A plain string (ignored because proxy or server text is not trusted for display) * - Something else entirely (null, number, etc.) */ function extractDetail(data: unknown): { detail: string | undefined; type: string | undefined } { if (typeof data === 'string') { - return { detail: data, type: undefined } + return { detail: undefined, type: undefined } } if (typeof data === 'object' && data !== null) { const obj = data as Record diff --git a/frontend/src/utils/messageMapper.test.ts b/frontend/src/utils/messageMapper.test.ts index 93d480ecba..11d8dcef51 100644 --- a/frontend/src/utils/messageMapper.test.ts +++ b/frontend/src/utils/messageMapper.test.ts @@ -313,6 +313,38 @@ describe("messageMapper", () => { expect(result.error!.description).toBe("Content was filtered"); }); + it("should sanitize internal error response content", () => { + const internalDetail = "secret=sk-test at C:\\internal\\provider.py"; + const msg: BackendMessage = { + turn_number: 1, + role: "assistant", + message_pieces: [ + { + id: "p1", + original_value_data_type: "error", + converted_value_data_type: "error", + original_value: internalDetail, + converted_value: internalDetail, + scores: [], + response_error: "processing", + response_error_description: internalDetail, + }, + ], + created_at: "2026-02-15T00:00:00Z", + }; + + const result = backendMessageToFrontend(msg); + + expect(result.content).toBe( + "The target could not complete the request. Check server logs for details." + ); + expect(result.error?.description).toBe( + "The target could not complete the request. Check server logs for details." + ); + expect(JSON.stringify(result)).not.toContain("sk-test"); + expect(JSON.stringify(result)).not.toContain("internal\\\\provider.py"); + }); + it("should handle multi-piece message with text + image", () => { const msg: BackendMessage = { turn_number: 1, diff --git a/frontend/src/utils/messageMapper.ts b/frontend/src/utils/messageMapper.ts index 1868ca4ad0..2575c19b03 100644 --- a/frontend/src/utils/messageMapper.ts +++ b/frontend/src/utils/messageMapper.ts @@ -7,6 +7,8 @@ import type { MessagePieceRequest, } from '../types' +const INTERNAL_MESSAGE_ERROR = 'The target could not complete the request. Check server logs for details.' + /** * Read a File and return its contents as a base64-encoded string (no data URI prefix). */ @@ -177,12 +179,22 @@ function pieceToError(piece: BackendMessagePiece): MessageError | undefined { if (piece.response_error && piece.response_error !== 'none') { return { type: piece.response_error, - description: piece.response_error_description || undefined, + description: isInternalErrorPiece(piece) + ? INTERNAL_MESSAGE_ERROR + : piece.response_error_description || undefined, } } return undefined } +function isInternalErrorPiece(piece: BackendMessagePiece): boolean { + return ( + piece.response_error === 'processing' || + piece.response_error === 'unknown' || + piece.converted_value_data_type === 'error' + ) +} + /** * Convert a single backend Message DTO to a frontend Message for rendering. */ @@ -200,6 +212,10 @@ export function backendMessageToFrontend(msg: BackendMessage): Message { if (pieceError && !error) { error = pieceError } + if (isInternalErrorPiece(piece)) { + textParts.push(INTERNAL_MESSAGE_ERROR) + continue + } // Extract reasoning summaries from reasoning-type pieces if (isReasoningDataType(piece.converted_value_data_type)) { diff --git a/pyrit/backend/exceptions.py b/pyrit/backend/exceptions.py new file mode 100644 index 0000000000..4f47dab662 --- /dev/null +++ b/pyrit/backend/exceptions.py @@ -0,0 +1,8 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +"""Backend exceptions with client-safe messages.""" + + +class ClientRequestError(ValueError): + """A request error whose message is safe to return to API clients.""" diff --git a/pyrit/backend/middleware/error_handlers.py b/pyrit/backend/middleware/error_handlers.py index 44f4d340ed..bcdb072188 100644 --- a/pyrit/backend/middleware/error_handlers.py +++ b/pyrit/backend/middleware/error_handlers.py @@ -66,11 +66,12 @@ async def value_error_handler( # pyrit-async-suffix-exempt Returns: JSONResponse: RFC 7807 problem detail response with 400 status. """ + logger.warning("Bad request on %s %s: %s", request.method, request.url.path, exc) problem = ProblemDetail( type="/errors/bad-request", title="Bad Request", status=status.HTTP_400_BAD_REQUEST, - detail=str(exc), + detail="The request could not be processed", instance=str(request.url.path), ) @@ -90,11 +91,12 @@ async def not_found_handler( # pyrit-async-suffix-exempt Returns: JSONResponse: RFC 7807 problem detail response with 404 status. """ + logger.warning("Resource not found on %s %s: %s", request.method, request.url.path, exc) problem = ProblemDetail( type="/errors/not-found", title="Not Found", status=status.HTTP_404_NOT_FOUND, - detail=str(exc), + detail="The requested resource was not found", instance=str(request.url.path), ) @@ -114,11 +116,12 @@ async def permission_error_handler( # pyrit-async-suffix-exempt Returns: JSONResponse: RFC 7807 problem detail response with 403 status. """ + logger.warning("Permission denied on %s %s: %s", request.method, request.url.path, exc) problem = ProblemDetail( type="/errors/forbidden", title="Forbidden", status=status.HTTP_403_FORBIDDEN, - detail=str(exc), + detail="You are not authorized to access this resource", instance=str(request.url.path), ) @@ -138,11 +141,12 @@ async def not_implemented_handler( # pyrit-async-suffix-exempt Returns: JSONResponse: RFC 7807 problem detail response with 501 status. """ + logger.warning("Unimplemented operation on %s %s: %s", request.method, request.url.path, exc) problem = ProblemDetail( type="/errors/not-implemented", title="Not Implemented", status=status.HTTP_501_NOT_IMPLEMENTED, - detail=str(exc) or "This feature is not yet implemented", + detail="This feature is not yet implemented", instance=str(request.url.path), ) diff --git a/pyrit/backend/models/attacks.py b/pyrit/backend/models/attacks.py index 00464ebdcd..e17f2766b6 100644 --- a/pyrit/backend/models/attacks.py +++ b/pyrit/backend/models/attacks.py @@ -9,7 +9,7 @@ """ from datetime import datetime, timezone -from typing import Any, Literal, cast +from typing import Any, ClassVar, Literal, cast from pydantic import BaseModel, Field, computed_field, field_serializer @@ -109,6 +109,10 @@ class MessagePieceView(MessagePiece): default=None, description="Description of the error if response_error is not 'none'" ) + _INTERNAL_ERROR_MESSAGE: ClassVar[str] = ( + "The target could not complete the request. Check server logs for details." + ) + @classmethod def from_domain( cls, @@ -139,19 +143,32 @@ def from_domain( A ``MessagePieceView`` with derived MIME types, filenames, and views. """ data = {name: getattr(piece, name) for name in MessagePiece.model_fields} + if piece.response_error in ("processing", "unknown") or piece.converted_value_data_type == "error": + data.update( + original_value=cls._INTERNAL_ERROR_MESSAGE, + original_value_sha256=None, + converted_value=cls._INTERNAL_ERROR_MESSAGE, + converted_value_sha256=None, + prompt_metadata={}, + response_error_description=cls._INTERNAL_ERROR_MESSAGE, + ) + original_value = cast("str", data["original_value"]) + converted_value = cast("str", data["converted_value"]) + original_value_sha256 = cast("str | None", data["original_value_sha256"]) + converted_value_sha256 = cast("str | None", data["converted_value_sha256"]) orig_dtype = piece.original_value_data_type or "text" conv_dtype = piece.converted_value_data_type or "text" data.update( scores=[ScoreView.from_domain(score) for score in (scores or [])], original_value_url=original_value_url, converted_value_url=converted_value_url, - original_value_mime_type=infer_mime_type(value=piece.original_value, data_type=orig_dtype), - converted_value_mime_type=infer_mime_type(value=piece.converted_value, data_type=conv_dtype), + original_value_mime_type=infer_mime_type(value=original_value, data_type=orig_dtype), + converted_value_mime_type=infer_mime_type(value=converted_value, data_type=conv_dtype), original_filename=build_filename( - data_type=orig_dtype, sha256=piece.original_value_sha256, value=piece.original_value + data_type=orig_dtype, sha256=original_value_sha256, value=original_value ), converted_filename=build_filename( - data_type=conv_dtype, sha256=piece.converted_value_sha256, value=piece.converted_value + data_type=conv_dtype, sha256=converted_value_sha256, value=converted_value ), ) return cls.model_construct(**data) diff --git a/pyrit/backend/routes/attacks.py b/pyrit/backend/routes/attacks.py index 7f41ec4339..1fd97bd478 100644 --- a/pyrit/backend/routes/attacks.py +++ b/pyrit/backend/routes/attacks.py @@ -14,6 +14,7 @@ from fastapi import APIRouter, HTTPException, Query, status +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.models.attacks import ( AddMessageRequest, AddMessageResponse, @@ -205,7 +206,7 @@ async def create_attack(request: CreateAttackRequest) -> CreateAttackResponse: try: return await service.create_attack_async(request=request) - except ValueError as e: + except ClientRequestError as e: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail=str(e), @@ -298,7 +299,7 @@ async def get_conversation_messages( # pyrit-async-suffix-exempt attack_result_id=attack_result_id, conversation_id=conversation_id, ) - except ValueError as e: + except ClientRequestError as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=str(e), @@ -371,7 +372,7 @@ async def create_related_conversation( # pyrit-async-suffix-exempt attack_result_id=attack_result_id, request=request, ) - except ValueError as e: + except ClientRequestError as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=str(e), @@ -414,7 +415,7 @@ async def update_main_conversation( # pyrit-async-suffix-exempt attack_result_id=attack_result_id, request=request, ) - except ValueError as e: + except ClientRequestError as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=str(e), @@ -460,7 +461,7 @@ async def add_message( # pyrit-async-suffix-exempt try: return await service.add_message_async(attack_result_id=attack_result_id, request=request) - except ValueError as e: + except ClientRequestError as e: error_msg = str(e) if "not found" in error_msg.lower(): raise HTTPException( diff --git a/pyrit/backend/routes/converters.py b/pyrit/backend/routes/converters.py index c741353919..ef912b49a3 100644 --- a/pyrit/backend/routes/converters.py +++ b/pyrit/backend/routes/converters.py @@ -8,8 +8,11 @@ Converter types are set at app startup - you cannot add new types at runtime. """ +import logging + from fastapi import APIRouter, HTTPException, status +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.models.common import ProblemDetail from pyrit.backend.models.converters import ( ConverterCatalogResponse, @@ -22,6 +25,8 @@ ) from pyrit.backend.services.converter_service import get_converter_service +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/converters", tags=["converters"]) @@ -79,15 +84,22 @@ async def create_converter(request: CreateConverterRequest) -> CreateConverterRe try: return await service.create_converter_async(request=request) - except ValueError as e: + except ClientRequestError as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=str(e), ) from e + except ValueError as e: + logger.warning("Invalid converter configuration: %s", e) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Converter parameters are invalid.", + ) from e except Exception as e: + logger.exception("Failed to create converter") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Failed to create converter: {str(e)}", + detail="Failed to create converter. Check server logs for details.", ) from e @@ -138,13 +150,20 @@ async def preview_conversion(request: ConverterPreviewRequest) -> ConverterPrevi try: return await service.preview_conversion_async(request=request) - except ValueError as e: + except ClientRequestError as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=str(e), ) from e + except ValueError as e: + logger.warning("Invalid converter preview configuration: %s", e) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Converter preview parameters are invalid.", + ) from e except Exception as e: + logger.exception("Failed to preview converter") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Converter preview failed: {str(e)}", + detail="Converter preview failed. Check server logs for details.", ) from e diff --git a/pyrit/backend/routes/initializers.py b/pyrit/backend/routes/initializers.py index 9d2e4cbb1d..2c6ac0500f 100644 --- a/pyrit/backend/routes/initializers.py +++ b/pyrit/backend/routes/initializers.py @@ -13,6 +13,8 @@ DELETE /api/initializers/{name} — unregister an initializer """ +import logging + from fastapi import APIRouter, HTTPException, Query, Request, status from pyrit.backend.models.common import ProblemDetail @@ -23,6 +25,8 @@ from pyrit.backend.services.initializer_service import get_initializer_service from pyrit.models.catalog.initializer import RegisteredInitializer +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/initializers", tags=["initializers"]) @@ -130,9 +134,13 @@ async def register_initializer( # pyrit-async-suffix-exempt return await service.register_initializer_async(name=body.name, script_content=body.script_content) except ValueError as e: detail = str(e) - if "already registered" in detail: + if detail.startswith(f"Initializer '{body.name}' is already registered."): raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=detail) from None - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=detail) from None + logger.warning("Failed to register initializer '%s': %s", body.name, e) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Initializer script is invalid or could not be loaded.", + ) from None @router.delete( @@ -164,9 +172,16 @@ async def unregister_initializer( # pyrit-async-suffix-exempt try: await service.unregister_initializer_async(initializer_name=initializer_name) except ValueError as e: + detail = str(e) + if detail.startswith("Cannot remove built-in initializer"): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=detail, + ) from None + logger.warning("Failed to unregister initializer '%s': %s", initializer_name, e) raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=str(e), + detail="The initializer could not be removed.", ) from None except KeyError: raise HTTPException( diff --git a/pyrit/backend/routes/scenarios.py b/pyrit/backend/routes/scenarios.py index fa3a5635bb..1a1cad89a9 100644 --- a/pyrit/backend/routes/scenarios.py +++ b/pyrit/backend/routes/scenarios.py @@ -14,6 +14,7 @@ from fastapi import APIRouter, HTTPException, Query, status +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.models.common import ProblemDetail from pyrit.backend.models.scenarios import ( ListRegisteredScenariosResponse, @@ -114,7 +115,7 @@ async def start_scenario_run(request: RunScenarioRequest) -> ScenarioRunSummary: service = get_scenario_run_service() try: return await service.start_run_async(request=request) - except ValueError as e: + except ClientRequestError as e: raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) from None @@ -184,7 +185,7 @@ async def cancel_scenario_run(scenario_result_id: str) -> ScenarioRunSummary: # service = get_scenario_run_service() try: result = await service.cancel_run_async(scenario_result_id=scenario_result_id) - except ValueError as e: + except ClientRequestError as e: raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(e)) from None if result is None: @@ -216,7 +217,7 @@ async def get_scenario_run_results(scenario_result_id: str) -> ScenarioResult: service = get_scenario_run_service() try: result = service.get_run_results(scenario_result_id=scenario_result_id) - except ValueError as e: + except ClientRequestError as e: raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(e)) from None if result is None: diff --git a/pyrit/backend/routes/targets.py b/pyrit/backend/routes/targets.py index 3bac8a23b7..27da547220 100644 --- a/pyrit/backend/routes/targets.py +++ b/pyrit/backend/routes/targets.py @@ -8,8 +8,11 @@ Target types are set at app startup via initializers - you cannot add new types at runtime. """ +import logging + from fastapi import APIRouter, HTTPException, Query, status +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.models.common import ProblemDetail from pyrit.backend.models.targets import ( CreateTargetRequest, @@ -19,6 +22,8 @@ from pyrit.backend.services.target_service import get_target_service from pyrit.models.catalog.target import TargetInstance +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/targets", tags=["targets"]) @@ -92,15 +97,22 @@ async def create_target( try: return await service.create_target_async(request=request) - except ValueError as e: + except ClientRequestError as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=str(e), ) from e + except ValueError as e: + logger.warning("Invalid target configuration: %s", e) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Target parameters are invalid.", + ) from e except Exception as e: + logger.exception("Failed to create target") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Failed to create target: {str(e)}", + detail="Failed to create target. Check server logs for details.", ) from e diff --git a/pyrit/backend/services/attack_service.py b/pyrit/backend/services/attack_service.py index 5a2164b221..be30991584 100644 --- a/pyrit/backend/services/attack_service.py +++ b/pyrit/backend/services/attack_service.py @@ -25,6 +25,7 @@ from typing import Any, Literal, cast from urllib.parse import parse_qs, urlparse +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.mappers import ( attack_result_to_summary_async, format_last_message_preview, @@ -272,7 +273,7 @@ async def get_conversation_messages_async( # Verify the conversation belongs to this attack ar = results[0] if conversation_id not in ar.get_active_conversation_ids(): - raise ValueError(f"Conversation '{conversation_id}' is not part of attack '{attack_result_id}'") + raise ClientRequestError(f"Conversation '{conversation_id}' is not part of attack '{attack_result_id}'") # Get messages for this conversation pyrit_messages = self._memory.get_conversation_messages(conversation_id=conversation_id) @@ -302,7 +303,7 @@ async def create_attack_async(self, *, request: CreateAttackRequest) -> CreateAt target_service = get_target_service() target_instance = await target_service.get_target_async(target_registry_name=request.target_registry_name) if not target_instance: - raise ValueError(f"Target instance '{request.target_registry_name}' not found") + raise ClientRequestError(f"Target instance '{request.target_registry_name}' not found") # Get the actual target object so we can capture its ComponentIdentifier target_obj = target_service.get_target_object(target_registry_name=request.target_registry_name) @@ -484,11 +485,11 @@ async def create_related_conversation_async( # Validate that both or neither branching fields are provided if (request.source_conversation_id is None) != (request.cutoff_index is None): - raise ValueError("Both source_conversation_id and cutoff_index must be provided together") + raise ClientRequestError("Both source_conversation_id and cutoff_index must be provided together") # Validate source_conversation_id belongs to this attack if request.source_conversation_id is not None and not ar.includes_conversation(request.source_conversation_id): - raise ValueError( + raise ClientRequestError( f"Conversation '{request.source_conversation_id}' is not part of attack '{attack_result_id}'" ) @@ -550,7 +551,7 @@ async def update_main_conversation_async( # Verify the conversation belongs to this attack (main or related) if not ar.includes_conversation(target_conv_id): - raise ValueError(f"Conversation '{target_conv_id}' is not part of this attack") + raise ClientRequestError(f"Conversation '{target_conv_id}' is not part of this attack") # Build updated DB columns: remove target from its list, add old main # to pruned list (user-visible GUI conversations are PRUNED, not ADVERSARIAL). @@ -601,7 +602,7 @@ async def add_message_async(self, *, attack_result_id: str, request: AddMessageR """ results = self._memory.get_attack_results(attack_result_ids=[attack_result_id]) if not results: - raise ValueError(f"Attack '{attack_result_id}' not found") + raise ClientRequestError(f"Attack '{attack_result_id}' not found") ar = results[0] main_conversation_id = ar.conversation_id @@ -613,11 +614,13 @@ async def add_message_async(self, *, attack_result_id: str, request: AddMessageR # Validate the target conversation belongs to this attack (main + pruned only) if msg_conversation_id not in ar.get_active_conversation_ids(): - raise ValueError(f"Conversation '{msg_conversation_id}' is not part of attack '{attack_result_id}'") + raise ClientRequestError( + f"Conversation '{msg_conversation_id}' is not part of attack '{attack_result_id}'" + ) target_registry_name = request.target_registry_name if request.send and not target_registry_name: - raise ValueError("target_registry_name is required when send=True") + raise ClientRequestError("target_registry_name is required when send=True") # Get existing messages to determine sequence. # NOTE: This read-then-write is not atomic (TOCTOU). Fine for the @@ -703,7 +706,7 @@ def _validate_target_match( or (stored_target_id.params.get("endpoint") or "") != (request_target_id.params.get("endpoint") or "") or (stored_target_id.params.get("model_name") or "") != (request_target_id.params.get("model_name") or "") ): - raise ValueError( + raise ClientRequestError( f"Target mismatch: attack was created with " f"{stored_target_id.class_name}/{stored_target_id.params.get('model_name')} " f"but request uses " @@ -727,7 +730,7 @@ def _validate_operator_match(self, *, attack_result: AttackResult, request: AddM request_operator = request.labels.get("operator") if request_operator and request_operator != attack_operator: - raise ValueError( + raise ClientRequestError( f"Operator mismatch: attack belongs to operator '{attack_operator}' " f"but request is from '{request_operator}'. " f"Create a new attack to continue." @@ -1030,7 +1033,7 @@ async def _send_and_store_message_async( """Send message to target via normalizer and store response.""" target_obj = get_target_service().get_target_object(target_registry_name=target_registry_name) if not target_obj: - raise ValueError(f"Target object for '{target_registry_name}' not found") + raise ClientRequestError(f"Target object for '{target_registry_name}' not found") await self._persist_base64_pieces_async(request) diff --git a/pyrit/backend/services/converter_service.py b/pyrit/backend/services/converter_service.py index 7ec6aec799..59b20e8e75 100644 --- a/pyrit/backend/services/converter_service.py +++ b/pyrit/backend/services/converter_service.py @@ -20,6 +20,7 @@ from typing import Any from urllib.parse import parse_qs, urlparse +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.mappers.converter_mappers import converter_object_to_instance from pyrit.backend.models import DEFAULT_MEDIA_EXTENSIONS from pyrit.backend.models.converters import ( @@ -146,7 +147,7 @@ async def create_converter_async(self, *, request: CreateConverterRequest) -> Cr # construction (incl. param coercion and reference resolution) to the # converter registry. if request.type not in self._registry: - raise ValueError(f"Converter type '{request.type}' not found") + raise ClientRequestError(f"Converter type '{request.type}' not found") params = await self._persist_data_uri_params_async(converter_type=request.type, params=request.params) converter_obj = self._registry.create_instance(request.type, **params) self._registry.instances.register(converter_obj, name=converter_id) @@ -235,7 +236,7 @@ def get_converter_objects_for_ids(self, *, converter_ids: list[str]) -> list[Any for conv_id in converter_ids: conv_obj = self.get_converter_object(converter_id=conv_id) if conv_obj is None: - raise ValueError(f"Converter instance '{conv_id}' not found") + raise ClientRequestError(f"Converter instance '{conv_id}' not found") converters.append(conv_obj) return converters @@ -318,7 +319,7 @@ def _gather_converters(self, *, converter_ids: list[str]) -> list[tuple[str, str for conv_id in converter_ids: conv_obj = self.get_converter_object(converter_id=conv_id) if conv_obj is None: - raise ValueError(f"Converter instance '{conv_id}' not found") + raise ClientRequestError(f"Converter instance '{conv_id}' not found") conv_type = conv_obj.__class__.__name__ converters.append((conv_id, conv_type, conv_obj)) return converters diff --git a/pyrit/backend/services/scenario_run_service.py b/pyrit/backend/services/scenario_run_service.py index 840858adef..28d6d360c5 100644 --- a/pyrit/backend/services/scenario_run_service.py +++ b/pyrit/backend/services/scenario_run_service.py @@ -14,9 +14,10 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Any +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.models.scenarios import ScenarioRunListResponse from pyrit.memory import CentralMemory -from pyrit.models import AttackOutcome, ScenarioResult, ScenarioRunState +from pyrit.models import AttackOutcome, RetryEvent, ScenarioResult, ScenarioRunState from pyrit.models.catalog.scenario import ( AttackErrorSummary, AttackRetrySummary, @@ -41,6 +42,59 @@ _DEFAULT_MAX_CONCURRENT_RUNS = 3 _CONVERTER_MODIFIER_PREFIX = "converter." +_SCENARIO_ERROR_MESSAGE = "Scenario run failed. Check server logs for details." +_ATTACK_ERROR_MESSAGE = "Attack execution failed. Check server logs for details." +_RETRY_ERROR_MESSAGE = "Retryable operation failed. Check server logs for details." + + +def _remove_endpoint_fields(value: Any) -> Any: + """ + Remove endpoint fields from a serialized API payload. + + Returns: + A recursively copied value without endpoint fields. + """ + if isinstance(value, dict): + return { + key: _remove_endpoint_fields(item) + for key, item in value.items() + if str(key).lower() != "endpoint" + } + if isinstance(value, list): + return [_remove_endpoint_fields(item) for item in value] + return value + + +def _sanitize_scenario_result(scenario_result: ScenarioResult) -> ScenarioResult: + """ + Create an API-safe copy of a persisted scenario result. + + Returns: + A deep copy with exception-derived fields replaced by stable messages. + """ + sanitized = ScenarioResult.model_validate(_remove_endpoint_fields(scenario_result.model_dump())) + if sanitized.error_message or sanitized.error_type: + sanitized.error_message = _SCENARIO_ERROR_MESSAGE + sanitized.error_type = "ScenarioRunError" + + for attack_results in sanitized.attack_results.values(): + for attack_result in attack_results: + if attack_result.error_message or attack_result.error_type or attack_result.error_traceback: + attack_result.error_message = _ATTACK_ERROR_MESSAGE + attack_result.error_type = "AttackExecutionError" + attack_result.error_traceback = None + attack_result.retry_events = [ + event.model_copy( + update={ + "exception_type": "RetryableOperationError", + "exception_message": _RETRY_ERROR_MESSAGE, + "endpoint": None, + } + ) + for event in attack_result.retry_events + ] + + return sanitized @dataclass @@ -88,7 +142,7 @@ async def start_run_async(self, *, request: RunScenarioRequest) -> ScenarioRunSu or concurrent limit exceeded. """ if self._run_semaphore.locked(): - raise ValueError( + raise ClientRequestError( f"Maximum concurrent runs ({self._max_concurrent_runs}) reached. " "Wait for an existing run to complete or cancel one." ) @@ -176,7 +230,7 @@ async def cancel_run_async(self, *, scenario_result_id: str) -> ScenarioRunSumma db_status = ScenarioRunState(scenario_result.scenario_run_state) if db_status in (ScenarioRunState.COMPLETED, ScenarioRunState.FAILED, ScenarioRunState.CANCELLED): - raise ValueError(f"Cannot cancel run in '{db_status}' state.") + raise ClientRequestError(f"Cannot cancel run in '{db_status}' state.") # Cancel the asyncio task if active and wait for it to finish active = self._active_tasks.get(scenario_result_id) @@ -212,7 +266,7 @@ def _resolve_scenario_class(self, *, request: RunScenarioRequest) -> type[Scenar try: return scenario_registry.get_class(request.scenario_name) except KeyError as e: - raise ValueError(str(e)) from None + raise ClientRequestError(str(e)) from None async def _run_initializers_async(self, *, request: RunScenarioRequest) -> None: """ @@ -235,7 +289,7 @@ async def _run_initializers_async(self, *, request: RunScenarioRequest) -> None: initializer_name, initializer_params=initializer_params ) except KeyError as e: - raise ValueError(f"Initializer not found: {e}") from None + raise ClientRequestError(f"Initializer not found: {e}") from None await instance.initialize_async() def _resolve_target(self, *, request: RunScenarioRequest) -> "PromptTarget": @@ -256,12 +310,12 @@ def _resolve_target(self, *, request: RunScenarioRequest) -> "PromptTarget": if objective_target is None: available_names = target_registry.instances.get_names() if not available_names: - raise ValueError( + raise ClientRequestError( f"Target '{request.target_name}' not found. The target registry is empty. " "Make sure to include an initializer that registers targets " "(e.g., initializers: ['target'])." ) - raise ValueError( + raise ClientRequestError( f"Target '{request.target_name}' not found in registry. Available targets: {', '.join(available_names)}" ) return objective_target @@ -429,7 +483,7 @@ def _resolve_techniques_and_converters( technique_enum = technique_class(base_name) except ValueError: available_techniques = [s.value for s in technique_class] - raise ValueError( + raise ClientRequestError( f"Technique '{base_name}' not found for scenario '{scenario_name}'. " f"Available: {', '.join(available_techniques)}" ) from None @@ -466,7 +520,7 @@ def _resolve_converter_modifiers(self, *, modifiers: list[str], token: str) -> l converters: list[Converter] = [] for modifier in modifiers: if not modifier.startswith(_CONVERTER_MODIFIER_PREFIX): - raise ValueError( + raise ClientRequestError( f"Unknown technique modifier '{modifier}' in '{token}'. " f"Supported modifiers must use the '{_CONVERTER_MODIFIER_PREFIX}' prefix " f"(e.g. '{_CONVERTER_MODIFIER_PREFIX}translation_spanish')." @@ -476,7 +530,7 @@ def _resolve_converter_modifiers(self, *, modifiers: list[str], token: str) -> l if converter is None: available = instances.get_names() available_text = ", ".join(available) if available else "(none registered)" - raise ValueError( + raise ClientRequestError( f"Converter '{converter_name}' in '{token}' is not a registered converter " f"instance. Available converters: {available_text}" ) @@ -534,9 +588,9 @@ async def _execute_run_async(self, *, scenario_result_id: str) -> None: except asyncio.CancelledError: logger.info(f"Scenario run {scenario_result_id} was cancelled.") - except Exception as e: - active.error = str(e) - logger.exception(f"Scenario run {scenario_result_id} failed: {e}") + except Exception: + active.error = _SCENARIO_ERROR_MESSAGE + logger.exception("Scenario run %s failed", scenario_result_id) finally: self._run_semaphore.release() @@ -573,6 +627,8 @@ def _build_response_from_db(self, *, scenario_result: ScenarioResult) -> Scenari if active is not None and active.task is not None and active.task.done(): del self._active_tasks[scenario_result_id] + status = ScenarioRunState(scenario_result.scenario_run_state) + # Primary source: DB-persisted error fields error = scenario_result.error_message error_type = scenario_result.error_type @@ -592,7 +648,9 @@ def _build_response_from_db(self, *, scenario_result: ScenarioResult) -> Scenari if not error and active is not None: error = active.error - status = ScenarioRunState(scenario_result.scenario_run_state) + if error and status != ScenarioRunState.CANCELLED: + error = _SCENARIO_ERROR_MESSAGE + error_type = "ScenarioRunError" # Build result fields from DB (always computed so in-progress runs show progress) total_attacks = sum(len(results) for results in scenario_result.attack_results.values()) @@ -616,7 +674,20 @@ def _build_response_from_db(self, *, scenario_result: ScenarioResult) -> Scenari AttackRetrySummary( attack_result_id=str(attack_result.attack_result_id), atomic_attack_name=atomic_attack_name, - retries=retry_events, + retries=[ + RetryEvent( + timestamp=event.timestamp, + attempt_number=event.attempt_number, + function_name=event.function_name, + exception_type="RetryableOperationError", + exception_message=_RETRY_ERROR_MESSAGE, + component_role=event.component_role, + component_name=event.component_name, + endpoint=None, + elapsed_seconds=event.elapsed_seconds, + ) + for event in retry_events + ], ) ) @@ -625,8 +696,8 @@ def _build_response_from_db(self, *, scenario_result: ScenarioResult) -> Scenari AttackErrorSummary( atomic_attack_name=atomic_attack_name, objective=attack_result.objective, - error_type=attack_result.error_type, - error_message=attack_result.error_message, + error_type="AttackExecutionError", + error_message=_ATTACK_ERROR_MESSAGE, total_retries=retries if isinstance(retries, int) else 0, ) ) @@ -672,9 +743,11 @@ def get_run_results(self, *, scenario_result_id: str) -> ScenarioResult | None: run_response = self._build_response_from_db(scenario_result=scenario_result) if run_response.status != ScenarioRunState.COMPLETED: - raise ValueError(f"Results are only available for completed runs. Current status: '{run_response.status}'.") + raise ClientRequestError( + f"Results are only available for completed runs. Current status: '{run_response.status}'." + ) - return scenario_result + return _sanitize_scenario_result(scenario_result) _service_instance: ScenarioRunService | None = None diff --git a/pyrit/backend/services/target_service.py b/pyrit/backend/services/target_service.py index 4c3a6e7b8a..1a7e5c9b9a 100644 --- a/pyrit/backend/services/target_service.py +++ b/pyrit/backend/services/target_service.py @@ -16,6 +16,7 @@ from functools import lru_cache from typing import Any, Literal, cast +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.mappers.target_mappers import target_object_to_instance from pyrit.backend.models.common import PaginationInfo from pyrit.backend.models.targets import ( @@ -173,7 +174,7 @@ async def create_target_async(self, *, request: CreateTargetRequest) -> TargetIn endpoints) are raised by the registry / target classes. """ if request.type not in self._registry: - raise ValueError( + raise ClientRequestError( f"Target type '{request.type}' not found. Available types: {self._registry.get_class_names()}" ) @@ -182,7 +183,9 @@ async def create_target_async(self, *, request: CreateTargetRequest) -> TargetIn if request.auth_mode == "identity": if "identity" not in target_cls.supported_auth_modes: - raise ValueError(f"Target type '{request.type}' does not support identity-based authentication.") + raise ClientRequestError( + f"Target type '{request.type}' does not support identity-based authentication." + ) # Omit any api_key so the target validates its own endpoint and authenticates itself. params.pop("api_key", None) diff --git a/tests/unit/backend/test_api_routes.py b/tests/unit/backend/test_api_routes.py index de98617ecc..b9f55ca85c 100644 --- a/tests/unit/backend/test_api_routes.py +++ b/tests/unit/backend/test_api_routes.py @@ -15,6 +15,7 @@ from fastapi import status from fastapi.testclient import TestClient +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.main import app from pyrit.backend.models.attacks import ( AddMessageResponse, @@ -206,7 +207,7 @@ def test_create_attack_target_not_found(self, client: TestClient) -> None: """Test attack creation with non-existent target.""" with patch("pyrit.backend.routes.attacks.get_attack_service") as mock_get_service: mock_service = MagicMock() - mock_service.create_attack_async = AsyncMock(side_effect=ValueError("Target not found")) + mock_service.create_attack_async = AsyncMock(side_effect=ClientRequestError("Target not found")) mock_get_service.return_value = mock_service response = client.post( @@ -340,7 +341,9 @@ def test_add_message_attack_not_found(self, client: TestClient) -> None: """Test adding message to non-existent attack returns 404.""" with patch("pyrit.backend.routes.attacks.get_attack_service") as mock_get_service: mock_service = MagicMock() - mock_service.add_message_async = AsyncMock(side_effect=ValueError("Attack 'nonexistent' not found")) + mock_service.add_message_async = AsyncMock( + side_effect=ClientRequestError("Attack 'nonexistent' not found") + ) mock_get_service.return_value = mock_service response = client.post( @@ -354,7 +357,9 @@ def test_add_message_target_not_found(self, client: TestClient) -> None: """Test adding message when target object not found returns 404.""" with patch("pyrit.backend.routes.attacks.get_attack_service") as mock_get_service: mock_service = MagicMock() - mock_service.add_message_async = AsyncMock(side_effect=ValueError("Target object for 'target-1' not found")) + mock_service.add_message_async = AsyncMock( + side_effect=ClientRequestError("Target object for 'target-1' not found") + ) mock_get_service.return_value = mock_service response = client.post( @@ -368,7 +373,7 @@ def test_add_message_bad_request(self, client: TestClient) -> None: """Test adding message with invalid request returns 400.""" with patch("pyrit.backend.routes.attacks.get_attack_service") as mock_get_service: mock_service = MagicMock() - mock_service.add_message_async = AsyncMock(side_effect=ValueError("Invalid message format")) + mock_service.add_message_async = AsyncMock(side_effect=ClientRequestError("Invalid message format")) mock_get_service.return_value = mock_service response = client.post( @@ -380,9 +385,10 @@ def test_add_message_bad_request(self, client: TestClient) -> None: def test_add_message_internal_error(self, client: TestClient) -> None: """Test adding message when internal error occurs returns 500.""" + internal_detail = r"secret=sk-test C:\internal\provider.py" with patch("pyrit.backend.routes.attacks.get_attack_service") as mock_get_service: mock_service = MagicMock() - mock_service.add_message_async = AsyncMock(side_effect=RuntimeError("Unexpected internal error")) + mock_service.add_message_async = AsyncMock(side_effect=RuntimeError(internal_detail)) mock_get_service.return_value = mock_service response = client.post( @@ -391,6 +397,55 @@ def test_add_message_internal_error(self, client: TestClient) -> None: ) assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert internal_detail not in response.text + + def test_add_message_stored_error_does_not_leak_internal_details(self, client: TestClient) -> None: + """Test that stored target error pieces are sanitized in the HTTP response.""" + internal_detail = r"provider secret=sk-test at C:\internal\target.py" + now = datetime.now(timezone.utc) + error_piece = MessagePiece( + role="assistant", + original_value=internal_detail, + converted_value=internal_detail, + original_value_data_type="error", + converted_value_data_type="error", + response_error="processing", + conversation_id="attack-1", + sequence=1, + ) + response_data = AddMessageResponse( + attack=AttackSummary( + attack_result_id="ar-attack-1", + conversation_id="attack-1", + objective="test objective", + last_message_preview=None, + message_count=1, + created_at=now, + updated_at=now, + ), + messages=ConversationMessagesResponse( + conversation_id="attack-1", + messages=[ + MessageView.model_construct( + message_pieces=[MessagePieceView.from_domain(error_piece)], + ) + ], + ), + ) + + with patch("pyrit.backend.routes.attacks.get_attack_service") as mock_get_service: + mock_service = MagicMock() + mock_service.add_message_async = AsyncMock(return_value=response_data) + mock_get_service.return_value = mock_service + + response = client.post( + "/api/attacks/attack-1/messages", + json={"pieces": [{"original_value": "Hello"}], "target_conversation_id": "attack-1"}, + ) + + assert response.status_code == status.HTTP_200_OK + assert internal_detail not in response.text + assert "The target could not complete the request" in response.text def test_get_conversation_messages_success(self, client: TestClient) -> None: """Test getting attack messages.""" @@ -429,7 +484,7 @@ def test_get_conversation_messages_invalid_conversation_returns_400(self, client with patch("pyrit.backend.routes.attacks.get_attack_service") as mock_get_service: mock_service = MagicMock() mock_service.get_conversation_messages_async = AsyncMock( - side_effect=ValueError("conversation does not belong to this attack") + side_effect=ClientRequestError("conversation does not belong to this attack") ) mock_get_service.return_value = mock_service @@ -744,7 +799,9 @@ def test_update_main_conversation_bad_request(self, client: TestClient) -> None: """Test changing main conversation with invalid conversation returns 400.""" with patch("pyrit.backend.routes.attacks.get_attack_service") as mock_get_service: mock_service = MagicMock() - mock_service.update_main_conversation_async = AsyncMock(side_effect=ValueError("invalid conversation")) + mock_service.update_main_conversation_async = AsyncMock( + side_effect=ClientRequestError("invalid conversation") + ) mock_get_service.return_value = mock_service response = client.post( @@ -845,7 +902,7 @@ def test_create_target_invalid_type(self, client: TestClient) -> None: """Test target creation with invalid type.""" with patch("pyrit.backend.routes.targets.get_target_service") as mock_get_service: mock_service = MagicMock() - mock_service.create_target_async = AsyncMock(side_effect=ValueError("Target type not found")) + mock_service.create_target_async = AsyncMock(side_effect=ClientRequestError("Target type not found")) mock_get_service.return_value = mock_service response = client.post( @@ -857,9 +914,10 @@ def test_create_target_invalid_type(self, client: TestClient) -> None: def test_create_target_internal_error(self, client: TestClient) -> None: """Test target creation with internal error returns 500.""" + internal_detail = "provider deployment=my-secret-deployment" with patch("pyrit.backend.routes.targets.get_target_service") as mock_get_service: mock_service = MagicMock() - mock_service.create_target_async = AsyncMock(side_effect=RuntimeError("Unexpected error")) + mock_service.create_target_async = AsyncMock(side_effect=RuntimeError(internal_detail)) mock_get_service.return_value = mock_service response = client.post( @@ -868,6 +926,25 @@ def test_create_target_internal_error(self, client: TestClient) -> None: ) assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert response.json()["detail"] == "Failed to create target. Check server logs for details." + assert internal_detail not in response.text + + def test_create_target_invalid_parameters_do_not_leak_details(self, client: TestClient) -> None: + """Test that unexpected validation details are sanitized.""" + internal_detail = r"api_key=sk-test failed validation in C:\internal\target.py" + with patch("pyrit.backend.routes.targets.get_target_service") as mock_get_service: + mock_service = MagicMock() + mock_service.create_target_async = AsyncMock(side_effect=ValueError(internal_detail)) + mock_get_service.return_value = mock_service + + response = client.post( + "/api/targets", + json={"type": "TextTarget", "params": {}}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == "Target parameters are invalid." + assert internal_detail not in response.text def test_get_target_success(self, client: TestClient) -> None: """Test getting a target by ID.""" @@ -1038,7 +1115,9 @@ def test_create_converter_invalid_type(self, client: TestClient) -> None: """Test converter creation with invalid type.""" with patch("pyrit.backend.routes.converters.get_converter_service") as mock_get_service: mock_service = MagicMock() - mock_service.create_converter_async = AsyncMock(side_effect=ValueError("Converter type not found")) + mock_service.create_converter_async = AsyncMock( + side_effect=ClientRequestError("Converter type not found") + ) mock_get_service.return_value = mock_service response = client.post( @@ -1050,9 +1129,10 @@ def test_create_converter_invalid_type(self, client: TestClient) -> None: def test_create_converter_internal_error(self, client: TestClient) -> None: """Test converter creation with internal error returns 500.""" + internal_detail = r"secret=sk-test C:\internal\converter.py" with patch("pyrit.backend.routes.converters.get_converter_service") as mock_get_service: mock_service = MagicMock() - mock_service.create_converter_async = AsyncMock(side_effect=RuntimeError("Unexpected error")) + mock_service.create_converter_async = AsyncMock(side_effect=RuntimeError(internal_detail)) mock_get_service.return_value = mock_service response = client.post( @@ -1061,6 +1141,25 @@ def test_create_converter_internal_error(self, client: TestClient) -> None: ) assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert response.json()["detail"] == "Failed to create converter. Check server logs for details." + assert internal_detail not in response.text + + def test_create_converter_invalid_parameters_do_not_leak_details(self, client: TestClient) -> None: + """Test that unexpected converter validation details are sanitized.""" + internal_detail = r"secret=sk-test at C:\internal\converter.py" + with patch("pyrit.backend.routes.converters.get_converter_service") as mock_get_service: + mock_service = MagicMock() + mock_service.create_converter_async = AsyncMock(side_effect=ValueError(internal_detail)) + mock_get_service.return_value = mock_service + + response = client.post( + "/api/converters", + json={"type": "Base64Converter", "params": {}}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == "Converter parameters are invalid." + assert internal_detail not in response.text def test_get_converter_success(self, client: TestClient) -> None: """Test getting a converter instance by ID.""" @@ -1137,7 +1236,7 @@ def test_preview_conversion_bad_request(self, client: TestClient) -> None: with patch("pyrit.backend.routes.converters.get_converter_service") as mock_get_service: mock_service = MagicMock() mock_service.preview_conversion_async = AsyncMock( - side_effect=ValueError("Converter instance 'nonexistent' not found") + side_effect=ClientRequestError("Converter instance 'nonexistent' not found") ) mock_get_service.return_value = mock_service @@ -1154,9 +1253,10 @@ def test_preview_conversion_bad_request(self, client: TestClient) -> None: def test_preview_conversion_internal_error(self, client: TestClient) -> None: """Test preview conversion with internal error returns 500.""" + internal_detail = "provider response included secret=sk-test" with patch("pyrit.backend.routes.converters.get_converter_service") as mock_get_service: mock_service = MagicMock() - mock_service.preview_conversion_async = AsyncMock(side_effect=RuntimeError("Converter execution failed")) + mock_service.preview_conversion_async = AsyncMock(side_effect=RuntimeError(internal_detail)) mock_get_service.return_value = mock_service response = client.post( @@ -1169,6 +1269,8 @@ def test_preview_conversion_internal_error(self, client: TestClient) -> None: ) assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert response.json()["detail"] == "Converter preview failed. Check server logs for details." + assert internal_detail not in response.text # ============================================================================ diff --git a/tests/unit/backend/test_attack_service.py b/tests/unit/backend/test_attack_service.py index a4809b1078..9bd247f57e 100644 --- a/tests/unit/backend/test_attack_service.py +++ b/tests/unit/backend/test_attack_service.py @@ -1112,7 +1112,11 @@ async def test_add_message_surfaces_stored_error_piece_on_send_failure(self, att # The PromptNormalizer persists a full error piece before re-raising; model # that by flipping to return the stored error piece only after send fails. - traceback_text = "Connection error.\nAPIConnectionError('Connection error.')\nTraceback..." + traceback_text = ( + "Connection error.\n" + "APIConnectionError('provider secret=sk-test')\n" + r"Traceback at C:\internal\provider.py" + ) error_piece = MessagePiece( role="assistant", original_value=traceback_text, @@ -1160,12 +1164,16 @@ async def _raise_after_store(**_): mock_normalizer.send_prompt_async.assert_called_once() assert result.attack is not None - # The error turn (response_error="processing" + traceback) must come back in the response, - # so the send-time view matches the conversation-reload view. + # The error turn must come back in the response without its stored traceback, + # so the send-time view matches the sanitized conversation-reload view. returned_pieces = [piece for message in result.messages.messages for piece in message.message_pieces] error_views = [piece for piece in returned_pieces if piece.response_error == "processing"] assert len(error_views) == 1 - assert "APIConnectionError" in error_views[0].converted_value + assert error_views[0].converted_value == ( + "The target could not complete the request. Check server logs for details." + ) + assert "sk-test" not in error_views[0].model_dump_json() + assert "internal" not in error_views[0].model_dump_json() async def test_add_message_reraises_when_send_fails_without_stored_error_piece( self, attack_service, mock_memory diff --git a/tests/unit/backend/test_error_handlers.py b/tests/unit/backend/test_error_handlers.py index 8fb2be55a4..364cd79d80 100644 --- a/tests/unit/backend/test_error_handlers.py +++ b/tests/unit/backend/test_error_handlers.py @@ -93,7 +93,8 @@ async def test_endpoint() -> dict: assert data["type"] == "/errors/bad-request" assert data["title"] == "Bad Request" assert data["status"] == 400 - assert "Invalid input value" in data["detail"] + assert data["detail"] == "The request could not be processed" + assert "Invalid input value" not in response.text def test_file_not_found_error_returns_404(self, app: FastAPI, client: TestClient) -> None: """Test that FileNotFoundError returns 404 with RFC 7807 format.""" @@ -156,6 +157,34 @@ async def test_endpoint() -> dict: assert data["status"] == 500 # Should not leak internal error details assert "An unexpected error occurred" in data["detail"] + assert "Something went wrong" not in response.text + + @pytest.mark.parametrize( + ("exception", "expected_status"), + [ + (ValueError("secret=sk-test-value"), 400), + (FileNotFoundError(r"C:\internal\secrets.json"), 404), + (PermissionError("provider token verification failed"), 403), + (NotImplementedError("provider deployment detail"), 501), + ], + ) + def test_expected_exception_details_are_not_returned( + self, + app: FastAPI, + client: TestClient, + exception: Exception, + expected_status: int, + ) -> None: + """Test that exception handlers do not expose exception-derived details.""" + + @app.get("/test") + async def test_endpoint() -> dict: + raise exception + + response = client.get("/test") + + assert response.status_code == expected_status + assert str(exception) not in response.text def test_error_response_includes_instance(self, app: FastAPI, client: TestClient) -> None: """Test that error responses include the request path as instance.""" diff --git a/tests/unit/backend/test_initializer_service.py b/tests/unit/backend/test_initializer_service.py index e0e83e4176..775a5d4d7e 100644 --- a/tests/unit/backend/test_initializer_service.py +++ b/tests/unit/backend/test_initializer_service.py @@ -416,9 +416,13 @@ def test_post_returns_201_with_registered_initializer( assert data["initializer_name"] == "my_custom" def test_post_returns_400_for_invalid_script(self, client_with_custom_initializers_enabled: TestClient) -> None: + internal_detail = ( + r"Failed to load initializer: already registered; " + r"secret=sk-test at C:\internal\initializers\bad.py" + ) with patch("pyrit.backend.routes.initializers.get_initializer_service") as mock_get_service: mock_service = MagicMock() - mock_service.register_initializer_async = AsyncMock(side_effect=ValueError("no classes")) + mock_service.register_initializer_async = AsyncMock(side_effect=ValueError(internal_detail)) mock_get_service.return_value = mock_service response = client_with_custom_initializers_enabled.post( @@ -426,6 +430,8 @@ def test_post_returns_400_for_invalid_script(self, client_with_custom_initialize ) assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == "Initializer script is invalid or could not be loaded." + assert internal_detail not in response.text def test_post_forwards_name_and_content(self, client_with_custom_initializers_enabled: TestClient) -> None: summary = RegisteredInitializer( diff --git a/tests/unit/backend/test_scenario_run_routes.py b/tests/unit/backend/test_scenario_run_routes.py index dc41e698a3..7b82b3101a 100644 --- a/tests/unit/backend/test_scenario_run_routes.py +++ b/tests/unit/backend/test_scenario_run_routes.py @@ -13,9 +13,10 @@ from fastapi.testclient import TestClient import pyrit.backend.services.scenario_run_service as _svc_mod +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.main import app from pyrit.backend.models.scenarios import ScenarioRunListResponse -from pyrit.models import ScenarioRunState +from pyrit.models import AttackOutcome, AttackResult, RetryEvent, ScenarioRunState, TargetIdentifier from pyrit.models.catalog.scenario import ScenarioRunSummary from unit.mocks import make_scenario_result @@ -77,7 +78,9 @@ def test_start_run_invalid_scenario_returns_400(self, client: TestClient) -> Non """Test that an invalid scenario returns 400.""" with patch("pyrit.backend.routes.scenarios.get_scenario_run_service") as mock_get: mock_service = MagicMock() - mock_service.start_run_async = AsyncMock(side_effect=ValueError("'bad.scenario' not found in registry.")) + mock_service.start_run_async = AsyncMock( + side_effect=ClientRequestError("'bad.scenario' not found in registry.") + ) mock_get.return_value = mock_service response = client.post( @@ -216,7 +219,9 @@ def test_cancel_completed_run_returns_409(self, client: TestClient) -> None: """Test that cancelling a completed run returns 409 Conflict.""" with patch("pyrit.backend.routes.scenarios.get_scenario_run_service") as mock_get: mock_service = MagicMock() - mock_service.cancel_run_async = AsyncMock(side_effect=ValueError("Cannot cancel run in 'completed' state.")) + mock_service.cancel_run_async = AsyncMock( + side_effect=ClientRequestError("Cannot cancel run in 'completed' state.") + ) mock_get.return_value = mock_service response = client.post("/api/scenarios/runs/test-run-id/cancel") @@ -251,10 +256,13 @@ def test_get_results_returns_200(self, client: TestClient) -> None: scenario_run_state="COMPLETED", ) - with patch("pyrit.backend.routes.scenarios.get_scenario_run_service") as mock_get: - mock_service = MagicMock() - mock_service.get_run_results.return_value = scenario_result - mock_get.return_value = mock_service + service = _svc_mod.ScenarioRunService.__new__(_svc_mod.ScenarioRunService) + service._memory = MagicMock() + service._memory.get_scenario_results.return_value = [scenario_result] + service._memory.get_attack_results.return_value = [] + service._active_tasks = {} + + with patch("pyrit.backend.routes.scenarios.get_scenario_run_service", return_value=service): response = client.get("/api/scenarios/runs/test-run-id/results") @@ -278,7 +286,7 @@ def test_get_results_not_completed_returns_409(self, client: TestClient) -> None """Test that getting results of a non-completed run returns 409.""" with patch("pyrit.backend.routes.scenarios.get_scenario_run_service") as mock_get: mock_service = MagicMock() - mock_service.get_run_results.side_effect = ValueError( + mock_service.get_run_results.side_effect = ClientRequestError( "Results are only available for completed runs. Current status: 'running'." ) mock_get.return_value = mock_service @@ -287,3 +295,53 @@ def test_get_results_not_completed_returns_409(self, client: TestClient) -> None assert response.status_code == status.HTTP_409_CONFLICT assert "only available for completed runs" in response.json()["detail"] + + def test_get_results_sanitizes_persisted_attack_errors(self, client: TestClient) -> None: + """Test that detailed results do not expose persisted provider diagnostics.""" + internal_detail = r"provider secret=sk-test at C:\internal\provider.py" + attack = AttackResult( + conversation_id="conv-1", + objective="Extract sensitive info", + outcome=AttackOutcome.ERROR, + executed_turns=1, + execution_time_ms=100, + timestamp=datetime(2025, 1, 1, tzinfo=timezone.utc), + error_message=internal_detail, + error_type="ProviderConnectionError", + error_traceback=f"Traceback: {internal_detail}", + retry_events=[ + RetryEvent( + attempt_number=1, + exception_type="ProviderRetryError", + exception_message=internal_detail, + endpoint="https://provider.internal/?api_key=sk-test", + ) + ], + ) + scenario_result = make_scenario_result( + scenario_name="foundry.red_team_agent", + scenario_description="Foundry red-team agent", + objective_target_identifier=TargetIdentifier( + class_name="FakeTarget", + class_module="test.mod", + endpoint="https://provider.internal/?api_key=sk-test", + ), + objective_scorer_identifier=None, + attack_results={"base64_attack": [attack]}, + scenario_run_state="COMPLETED", + ) + + service = _svc_mod.ScenarioRunService.__new__(_svc_mod.ScenarioRunService) + service._memory = MagicMock() + service._memory.get_scenario_results.return_value = [scenario_result] + service._memory.get_attack_results.return_value = [] + service._active_tasks = {} + + with patch("pyrit.backend.routes.scenarios.get_scenario_run_service", return_value=service): + + response = client.get("/api/scenarios/runs/test-run-id/results") + + assert response.status_code == status.HTTP_200_OK + assert internal_detail not in response.text + assert "api_key=sk-test" not in response.text + assert "Attack execution failed. Check server logs for details." in response.text diff --git a/tests/unit/backend/test_scenario_run_service.py b/tests/unit/backend/test_scenario_run_service.py index b22b704aee..43e2a2c5cf 100644 --- a/tests/unit/backend/test_scenario_run_service.py +++ b/tests/unit/backend/test_scenario_run_service.py @@ -17,10 +17,11 @@ ScenarioRunService, ) from pyrit.converter import Converter -from pyrit.models import AttackOutcome, ScenarioRunState +from pyrit.models import AttackOutcome, AttackResult, ComponentIdentifier, ScenarioRunState from pyrit.models.catalog.scenario import RunScenarioRequest from pyrit.scenario.core import DatasetAttackConfiguration, DatasetConfiguration from pyrit.scenario.core.scenario_technique import ScenarioTechnique +from unit.mocks import make_scenario_result class _StubTechnique(ScenarioTechnique): @@ -576,8 +577,9 @@ def test_get_run_falls_back_to_persisted_error(self, mock_memory) -> None: # Mock the error AttackResult lookup error_ar = MagicMock() - error_ar.error_message = "Connection refused" - error_ar.error_type = "ConnectionError" + internal_detail = r"Connection refused: secret=sk-test C:\internal\provider.py" + error_ar.error_message = internal_detail + error_ar.error_type = "ProviderConnectionError" mock_memory.get_scenario_results.return_value = [db_result] mock_memory.get_attack_results.return_value = [error_ar] @@ -585,8 +587,9 @@ def test_get_run_falls_back_to_persisted_error(self, mock_memory) -> None: fetched = service.get_run(scenario_result_id="sr-fail") assert fetched is not None - assert fetched.error == "Connection refused" - assert fetched.error_type == "ConnectionError" + assert fetched.error == "Scenario run failed. Check server logs for details." + assert fetched.error_type == "ScenarioRunError" + assert internal_detail not in fetched.model_dump_json() mock_memory.get_attack_results.assert_called_once_with( scenario_result_id="sr-fail", outcome=AttackOutcome.ERROR, @@ -715,7 +718,8 @@ async def test_execute_run_fails_with_error(self, mock_all_registries) -> None: service = ScenarioRunService() mock_instance = mock_all_registries["scenario_instance"] - mock_instance.run_async = AsyncMock(side_effect=RuntimeError("scenario exploded")) + internal_detail = r"scenario exploded: secret=sk-test C:\internal\scenario.py" + mock_instance.run_async = AsyncMock(side_effect=RuntimeError(internal_detail)) response = await service.start_run_async(request=_make_request()) @@ -726,13 +730,14 @@ async def test_execute_run_fails_with_error(self, mock_all_registries) -> None: await active.task # Error is stored on the active task until get_run reads it - assert active.error == "scenario exploded" + assert active.error == "Scenario run failed. Check server logs for details." assert response.scenario_result_id in service._active_tasks # get_run should surface the error and clean up fetched = service.get_run(scenario_result_id=response.scenario_result_id) assert fetched is not None - assert fetched.error == "scenario exploded" + assert fetched.error == "Scenario run failed. Check server logs for details." + assert internal_detail not in fetched.model_dump_json() assert response.scenario_result_id not in service._active_tasks @@ -757,24 +762,31 @@ def test_get_results_raises_if_not_completed(self, mock_memory) -> None: def test_get_results_returns_details_for_completed_run(self, mock_memory) -> None: """Test that get_run_results returns the ScenarioResult for a completed run.""" - from pyrit.models import AttackOutcome - - mock_attack_result = MagicMock() - mock_attack_result.outcome = AttackOutcome.SUCCESS - mock_attack_result.objective = "Extract info" - - db_result = _make_db_scenario_result( - result_id="sr-123", - run_state="COMPLETED", - attack_results={"base64_attack": [mock_attack_result]}, + attack_result = AttackResult( + conversation_id="conv-1", + objective="Extract info", + outcome=AttackOutcome.SUCCESS, + executed_turns=1, + execution_time_ms=100, + timestamp=datetime(2025, 1, 1, tzinfo=timezone.utc), + ) + db_result = make_scenario_result( + scenario_name="foundry.red_team_agent", + scenario_description="Foundry red-team agent", + objective_target_identifier=ComponentIdentifier.model_validate( + {"__type__": "FakeTarget", "__module__": "test.mod", "params": {}} + ), + objective_scorer_identifier=None, + attack_results={"base64_attack": [attack_result]}, + scenario_run_state="COMPLETED", ) - db_result.objective_achieved_rate.return_value = 100 mock_memory.get_scenario_results.return_value = [db_result] service = ScenarioRunService() result = service.get_run_results(scenario_result_id="sr-123") - assert result is db_result + assert result is not db_result + assert result is not None assert result.attack_results["base64_attack"][0].outcome == AttackOutcome.SUCCESS @@ -863,7 +875,7 @@ class TestScenarioRunServiceFailedAttackReporting: """Tests that per-attack errors and retry pressure surface in the summary.""" def test_error_attacks_and_retries_are_surfaced(self, mock_memory) -> None: - from pyrit.models import AttackOutcome + from pyrit.models import AttackOutcome, RetryEvent success = MagicMock() success.outcome = AttackOutcome.SUCCESS @@ -873,8 +885,18 @@ def test_error_attacks_and_retries_are_surfaced(self, mock_memory) -> None: errored.outcome = AttackOutcome.ERROR errored.objective = "do the bad thing" errored.error_type = "RateLimitError" - errored.error_message = "429 Too Many Requests" + errored.error_message = "429 Too Many Requests from provider deployment secret-model" errored.total_retries = 4 + errored.attack_result_id = "ar-error" + errored.retry_events = [ + RetryEvent( + attempt_number=1, + function_name="send_prompt_async", + exception_type="ProviderRateLimitError", + exception_message="secret-model returned provider token sk-test", + component_role="objective_target", + ) + ] db_result = _make_db_scenario_result( result_id="sr-mixed", @@ -892,9 +914,14 @@ def test_error_attacks_and_retries_are_surfaced(self, mock_memory) -> None: assert len(fetched.failed_attacks) == 1 failed = fetched.failed_attacks[0] assert failed.atomic_attack_name == "baseline_airt_hate" - assert failed.error_type == "RateLimitError" - assert failed.error_message == "429 Too Many Requests" + assert failed.error_type == "AttackExecutionError" + assert failed.error_message == "Attack execution failed. Check server logs for details." assert failed.total_retries == 4 + assert fetched.attack_retries[0].retries[0].exception_type == "RetryableOperationError" + assert fetched.attack_retries[0].retries[0].exception_message == ( + "Retryable operation failed. Check server logs for details." + ) + assert "sk-test" not in fetched.model_dump_json() def test_no_failed_attacks_when_all_succeed(self, mock_memory) -> None: from pyrit.models import AttackOutcome @@ -953,7 +980,7 @@ def test_retry_events_surface_per_attack(self, mock_memory) -> None: summary = fetched.attack_retries[0] assert summary.attack_result_id == "ar-9" assert summary.atomic_attack_name == "baseline_airt_hate" - assert summary.retries[0].endpoint == "https://ep/" + assert summary.retries[0].endpoint is None assert summary.retries[0].component_role == "objective_scorer" From 92d090218ae365370e918adcaa7aa3ecccae3896 Mon Sep 17 00:00:00 2001 From: hannahwestra25 Date: Mon, 20 Jul 2026 17:51:54 -0400 Subject: [PATCH 2/5] Preserve safe validation error details Distinguish public registry validation from constructor failures, sanitize error outcome reasons, and retain blocked-content messages. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2040f373-2101-4e34-a067-70c15ac7211e --- frontend/src/utils/messageMapper.test.ts | 10 ++++-- frontend/src/utils/messageMapper.ts | 2 +- pyrit/backend/mappers/attack_mappers.py | 5 +++ pyrit/backend/models/attacks.py | 4 ++- pyrit/backend/services/converter_service.py | 6 +++- .../backend/services/scenario_run_service.py | 20 +++++++---- pyrit/backend/services/target_service.py | 7 ++-- pyrit/registry/__init__.py | 2 ++ .../components/initializer_registry.py | 8 +++-- pyrit/registry/exceptions.py | 8 +++++ pyrit/registry/registry.py | 26 +++++++++++---- tests/unit/backend/test_api_routes.py | 33 +++++++++++++++++++ tests/unit/backend/test_converter_service.py | 23 +++++++++++++ tests/unit/backend/test_mappers.py | 15 +++++++++ tests/unit/backend/test_response_contracts.py | 19 +++++++++++ .../unit/backend/test_scenario_run_routes.py | 18 ++++++++++ .../unit/backend/test_scenario_run_service.py | 13 ++++++++ tests/unit/backend/test_target_service.py | 17 +++++++++- .../registry/test_initializer_registry.py | 7 ++-- tests/unit/registry/test_registry.py | 24 ++++++++++++++ 20 files changed, 240 insertions(+), 27 deletions(-) create mode 100644 pyrit/registry/exceptions.py diff --git a/frontend/src/utils/messageMapper.test.ts b/frontend/src/utils/messageMapper.test.ts index 11d8dcef51..5127ca1099 100644 --- a/frontend/src/utils/messageMapper.test.ts +++ b/frontend/src/utils/messageMapper.test.ts @@ -289,15 +289,18 @@ describe("messageMapper", () => { }); it("should handle error response", () => { + const blockedMessage = + "The provider blocked this prompt because it matched a content filter"; const msg: BackendMessage = { turn_number: 1, role: "assistant", message_pieces: [ { id: "p1", - original_value_data_type: "text", - converted_value_data_type: "text", - converted_value: "", + original_value_data_type: "error", + converted_value_data_type: "error", + original_value: blockedMessage, + converted_value: blockedMessage, scores: [], response_error: "blocked", response_error_description: "Content was filtered", @@ -311,6 +314,7 @@ describe("messageMapper", () => { expect(result.error).toBeDefined(); expect(result.error!.type).toBe("blocked"); expect(result.error!.description).toBe("Content was filtered"); + expect(result.content).toBe(blockedMessage); }); it("should sanitize internal error response content", () => { diff --git a/frontend/src/utils/messageMapper.ts b/frontend/src/utils/messageMapper.ts index 2575c19b03..3b964ed5a4 100644 --- a/frontend/src/utils/messageMapper.ts +++ b/frontend/src/utils/messageMapper.ts @@ -191,7 +191,7 @@ function isInternalErrorPiece(piece: BackendMessagePiece): boolean { return ( piece.response_error === 'processing' || piece.response_error === 'unknown' || - piece.converted_value_data_type === 'error' + (piece.response_error !== 'blocked' && piece.converted_value_data_type === 'error') ) } diff --git a/pyrit/backend/mappers/attack_mappers.py b/pyrit/backend/mappers/attack_mappers.py index fd856791a9..a926c76b32 100644 --- a/pyrit/backend/mappers/attack_mappers.py +++ b/pyrit/backend/mappers/attack_mappers.py @@ -36,6 +36,7 @@ from pyrit.memory import CentralMemory from pyrit.models import ( MEDIA_PATH_DATA_TYPES, + AttackOutcome, AttackResult, ChatMessageRole, Message, @@ -46,6 +47,8 @@ logger = logging.getLogger(__name__) +_ATTACK_ERROR_MESSAGE = "Attack execution failed. Check server logs for details." + if TYPE_CHECKING: from pyrit.models.conversation_stats import ConversationStats @@ -212,6 +215,8 @@ async def attack_result_to_summary_async( created_at, updated_at = _resolve_summary_timestamps(ar) data = {name: getattr(ar, name) for name in AttackResult.model_fields} + if ar.outcome == AttackOutcome.ERROR: + data["outcome_reason"] = _ATTACK_ERROR_MESSAGE data.update( last_response=await _summary_last_response_async(ar.last_response), last_score=ScoreView.from_domain(ar.last_score) if ar.last_score else None, diff --git a/pyrit/backend/models/attacks.py b/pyrit/backend/models/attacks.py index e17f2766b6..029da658f5 100644 --- a/pyrit/backend/models/attacks.py +++ b/pyrit/backend/models/attacks.py @@ -143,7 +143,9 @@ def from_domain( A ``MessagePieceView`` with derived MIME types, filenames, and views. """ data = {name: getattr(piece, name) for name in MessagePiece.model_fields} - if piece.response_error in ("processing", "unknown") or piece.converted_value_data_type == "error": + if piece.response_error in ("processing", "unknown") or ( + piece.response_error != "blocked" and piece.converted_value_data_type == "error" + ): data.update( original_value=cls._INTERNAL_ERROR_MESSAGE, original_value_sha256=None, diff --git a/pyrit/backend/services/converter_service.py b/pyrit/backend/services/converter_service.py index 59b20e8e75..c934ca24ff 100644 --- a/pyrit/backend/services/converter_service.py +++ b/pyrit/backend/services/converter_service.py @@ -36,6 +36,7 @@ ) from pyrit.memory import data_serializer_factory from pyrit.models import PromptDataType +from pyrit.registry import RegistryValidationError from pyrit.registry.components import ConverterRegistry @@ -149,7 +150,10 @@ async def create_converter_async(self, *, request: CreateConverterRequest) -> Cr if request.type not in self._registry: raise ClientRequestError(f"Converter type '{request.type}' not found") params = await self._persist_data_uri_params_async(converter_type=request.type, params=request.params) - converter_obj = self._registry.create_instance(request.type, **params) + try: + converter_obj = self._registry.create_instance(request.type, **params) + except RegistryValidationError as exc: + raise ClientRequestError(str(exc)) from exc self._registry.instances.register(converter_obj, name=converter_id) return CreateConverterResponse( diff --git a/pyrit/backend/services/scenario_run_service.py b/pyrit/backend/services/scenario_run_service.py index 28d6d360c5..919e009431 100644 --- a/pyrit/backend/services/scenario_run_service.py +++ b/pyrit/backend/services/scenario_run_service.py @@ -27,6 +27,7 @@ from pyrit.registry import ( ConverterRegistry, InitializerRegistry, + RegistryValidationError, ScenarioRegistry, TargetRegistry, ) @@ -79,6 +80,8 @@ def _sanitize_scenario_result(scenario_result: ScenarioResult) -> ScenarioResult for attack_results in sanitized.attack_results.values(): for attack_result in attack_results: + if attack_result.outcome == AttackOutcome.ERROR: + attack_result.outcome_reason = _ATTACK_ERROR_MESSAGE if attack_result.error_message or attack_result.error_type or attack_result.error_traceback: attack_result.error_message = _ATTACK_ERROR_MESSAGE attack_result.error_type = "AttackExecutionError" @@ -290,6 +293,8 @@ async def _run_initializers_async(self, *, request: RunScenarioRequest) -> None: ) except KeyError as e: raise ClientRequestError(f"Initializer not found: {e}") from None + except RegistryValidationError as exc: + raise ClientRequestError(str(exc)) from exc await instance.initialize_async() def _resolve_target(self, *, request: RunScenarioRequest) -> "PromptTarget": @@ -557,12 +562,15 @@ async def _initialize_scenario_async(self, *, request: RunScenarioRequest, init_ The fully initialized Scenario instance ready for run_async. """ scenario_registry = ScenarioRegistry.get_registry_singleton() - return await scenario_registry.create_and_initialize_async( - request.scenario_name, - scenario_params=request.scenario_params or {}, - scenario_result_id=request.scenario_result_id or None, - **init_kwargs, - ) + try: + return await scenario_registry.create_and_initialize_async( + request.scenario_name, + scenario_params=request.scenario_params or {}, + scenario_result_id=request.scenario_result_id or None, + **init_kwargs, + ) + except RegistryValidationError as exc: + raise ClientRequestError(str(exc)) from exc async def _execute_run_async(self, *, scenario_result_id: str) -> None: """ diff --git a/pyrit/backend/services/target_service.py b/pyrit/backend/services/target_service.py index 1a7e5c9b9a..2cf5fac146 100644 --- a/pyrit/backend/services/target_service.py +++ b/pyrit/backend/services/target_service.py @@ -26,7 +26,7 @@ TargetListResponse, ) from pyrit.models.catalog.target import TargetInstance -from pyrit.registry import TargetRegistry +from pyrit.registry import RegistryValidationError, TargetRegistry logger = logging.getLogger(__name__) @@ -189,7 +189,10 @@ async def create_target_async(self, *, request: CreateTargetRequest) -> TargetIn # Omit any api_key so the target validates its own endpoint and authenticates itself. params.pop("api_key", None) - target_obj = self._registry.create_instance(request.type, **params) + try: + target_obj = self._registry.create_instance(request.type, **params) + except RegistryValidationError as exc: + raise ClientRequestError(str(exc)) from exc self._registry.instances.register(target_obj) diff --git a/pyrit/registry/__init__.py b/pyrit/registry/__init__.py index 7706953463..433343c22a 100644 --- a/pyrit/registry/__init__.py +++ b/pyrit/registry/__init__.py @@ -18,6 +18,7 @@ TargetRegistry, ) from pyrit.registry.discovery import discover_in_directory +from pyrit.registry.exceptions import RegistryValidationError from pyrit.registry.instance_registry import ( DefaultInstanceRegistry, InstanceRegistry, @@ -38,6 +39,7 @@ "ParamBagRegistry", "Registry", "RegistryMetadata", + "RegistryValidationError", "SupportsInstances", "discover_in_directory", "InitializerMetadata", diff --git a/pyrit/registry/components/initializer_registry.py b/pyrit/registry/components/initializer_registry.py index 1f152babb3..1b5f3aefbb 100644 --- a/pyrit/registry/components/initializer_registry.py +++ b/pyrit/registry/components/initializer_registry.py @@ -26,6 +26,7 @@ from pyrit.models import class_name_to_snake_case, validate_registry_name from pyrit.registry.discovery import discover_in_directory +from pyrit.registry.exceptions import RegistryValidationError from pyrit.registry.registry import ParamBagRegistry from pyrit.registry.registry_metadata import RegistryMetadata @@ -247,11 +248,14 @@ def create_and_configure(self, name: str, *, initializer_params: dict[str, Any] Raises: KeyError: If the name is not registered. - ValueError: If the configured parameters are invalid. + RegistryValidationError: If the configured parameters are invalid. """ instance = self._create_and_configure(name, params=initializer_params or None) if initializer_params: - instance.validate_params() + try: + instance.validate_params() + except ValueError as exc: + raise RegistryValidationError(str(exc)) from exc return instance @staticmethod diff --git a/pyrit/registry/exceptions.py b/pyrit/registry/exceptions.py new file mode 100644 index 0000000000..9ad9fc0968 --- /dev/null +++ b/pyrit/registry/exceptions.py @@ -0,0 +1,8 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +"""Exceptions raised by registry validation.""" + + +class RegistryValidationError(ValueError): + """Raised when registry-supplied arguments cannot be safely resolved or configured.""" diff --git a/pyrit/registry/registry.py b/pyrit/registry/registry.py index f79f1f20c7..55a5f155dd 100644 --- a/pyrit/registry/registry.py +++ b/pyrit/registry/registry.py @@ -31,6 +31,7 @@ from abc import ABC, abstractmethod from typing import TYPE_CHECKING, Any, Generic, Protocol, TypeVar +from pyrit.registry.exceptions import RegistryValidationError from pyrit.registry.registry_metadata import RegistryMetadata from pyrit.registry.resolution import ( derive_parameters, @@ -616,15 +617,19 @@ def create_instance(self, name: str, **kwargs: object) -> T: Raises: KeyError: If the name is not registered. - ValueError: If an argument is not a valid constructor parameter, a + RegistryValidationError: If an argument is not a valid constructor parameter, a registry reference cannot be resolved, or a value cannot be coerced. + ValueError: If the component constructor rejects the resolved arguments. """ cls = self.get_class(name) - resolved = resolve_constructor_args( - cls=cls, - raw_args=dict(kwargs), - identifier_type=self._identifier_type(), - ) + try: + resolved = resolve_constructor_args( + cls=cls, + raw_args=dict(kwargs), + identifier_type=self._identifier_type(), + ) + except ValueError as exc: + raise RegistryValidationError(str(exc)) from exc return cls(**resolved) def __contains__(self, name: str) -> bool: @@ -700,8 +705,15 @@ def _create_and_configure( Returns: ConfigurableT: The constructed, parameterized instance. The caller owns any further lifecycle steps (initialize / validate). + + Raises: + RegistryValidationError: If constructor arguments or parameter-bag values + cannot be resolved, coerced, or validated. """ instance = self.create_instance(name, **(constructor_kwargs or {})) if params is not None: - instance.set_params_from_args(args=params) + try: + instance.set_params_from_args(args=params) + except ValueError as exc: + raise RegistryValidationError(str(exc)) from exc return instance diff --git a/tests/unit/backend/test_api_routes.py b/tests/unit/backend/test_api_routes.py index b9f55ca85c..ff54964b10 100644 --- a/tests/unit/backend/test_api_routes.py +++ b/tests/unit/backend/test_api_routes.py @@ -911,6 +911,23 @@ def test_create_target_invalid_type(self, client: TestClient) -> None: ) assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == "Target type not found" + + def test_create_target_preserves_safe_parameter_validation(self, client: TestClient) -> None: + """Known registry parameter validation remains actionable.""" + validation_message = "Unknown parameter 'bogus'. Valid parameters: endpoint, model_name" + with patch("pyrit.backend.routes.targets.get_target_service") as mock_get_service: + mock_service = MagicMock() + mock_service.create_target_async = AsyncMock(side_effect=ClientRequestError(validation_message)) + mock_get_service.return_value = mock_service + + response = client.post( + "/api/targets", + json={"type": "TextTarget", "params": {"bogus": "value"}}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == validation_message def test_create_target_internal_error(self, client: TestClient) -> None: """Test target creation with internal error returns 500.""" @@ -1127,6 +1144,22 @@ def test_create_converter_invalid_type(self, client: TestClient) -> None: assert response.status_code == status.HTTP_400_BAD_REQUEST + def test_create_converter_preserves_safe_parameter_validation(self, client: TestClient) -> None: + """Known registry parameter validation remains actionable.""" + validation_message = "Could not coerce parameter 'caesar_offset' to int" + with patch("pyrit.backend.routes.converters.get_converter_service") as mock_get_service: + mock_service = MagicMock() + mock_service.create_converter_async = AsyncMock(side_effect=ClientRequestError(validation_message)) + mock_get_service.return_value = mock_service + + response = client.post( + "/api/converters", + json={"type": "CaesarConverter", "params": {"caesar_offset": "invalid"}}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == validation_message + def test_create_converter_internal_error(self, client: TestClient) -> None: """Test converter creation with internal error returns 500.""" internal_detail = r"secret=sk-test C:\internal\converter.py" diff --git a/tests/unit/backend/test_converter_service.py b/tests/unit/backend/test_converter_service.py index 06ce662bac..7fab7d83e2 100644 --- a/tests/unit/backend/test_converter_service.py +++ b/tests/unit/backend/test_converter_service.py @@ -12,6 +12,7 @@ import pytest from pyrit import converter +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.models.converters import ( ConverterPreviewRequest, CreateConverterRequest, @@ -240,6 +241,28 @@ async def test_create_converter_registers_in_registry(self) -> None: converter_obj = service.get_converter_object(converter_id=result.converter_id) assert converter_obj is not None + async def test_create_converter_exposes_registry_parameter_validation(self) -> None: + """Registry argument validation is explicitly marked safe for API clients.""" + service = ConverterService() + request = CreateConverterRequest(type="Base64Converter", params={"not_a_param": "value"}) + + with pytest.raises(ClientRequestError, match="Unknown parameter 'not_a_param'"): + await service.create_converter_async(request=request) + + async def test_create_converter_does_not_mark_constructor_value_error_safe(self) -> None: + """Constructor failures remain ordinary ValueErrors for route sanitization.""" + internal_detail = r"provider secret=sk-test at C:\internal\converter.py" + service = ConverterService() + request = CreateConverterRequest(type="Base64Converter", params={}) + + with ( + patch.object(service._registry, "create_instance", side_effect=ValueError(internal_detail)), + pytest.raises(ValueError, match="sk-test") as exc_info, + ): + await service.create_converter_async(request=request) + + assert type(exc_info.value) is ValueError + class TestPersistDataUriParams: """Tests for ConverterService._persist_data_uri_params_async (registry-metadata driven).""" diff --git a/tests/unit/backend/test_mappers.py b/tests/unit/backend/test_mappers.py index fd3961652d..9837dfe647 100644 --- a/tests/unit/backend/test_mappers.py +++ b/tests/unit/backend/test_mappers.py @@ -157,6 +157,21 @@ async def test_basic_mapping(self) -> None: assert summary.target is not None assert summary.target.target_type == "TextTarget" + async def test_error_outcome_reason_is_sanitized(self) -> None: + """Exception-derived outcome reasons are not exposed by attack APIs.""" + internal_detail = r"provider secret=sk-test at C:\internal\provider.py" + ar = _make_attack_result().model_copy( + update={ + "outcome": AttackOutcome.ERROR, + "outcome_reason": f"Exception: ProviderConnectionError: {internal_detail}", + } + ) + + summary = await attack_result_to_summary_async(ar, stats=ConversationStats(message_count=0)) + + assert summary.outcome_reason == "Attack execution failed. Check server logs for details." + assert internal_detail not in summary.model_dump_json() + async def test_empty_pieces_gives_zero_messages(self) -> None: """Test mapping with no message pieces.""" ar = _make_attack_result() diff --git a/tests/unit/backend/test_response_contracts.py b/tests/unit/backend/test_response_contracts.py index 282ee6ced2..ee8ffaccc6 100644 --- a/tests/unit/backend/test_response_contracts.py +++ b/tests/unit/backend/test_response_contracts.py @@ -109,6 +109,25 @@ def test_dump_has_canonical_and_presentation_fields(self) -> None: assert "response_error_description" in dumped assert dumped["scores"] == [] + def test_blocked_error_piece_preserves_public_message(self) -> None: + """Blocked error pieces retain their public content-filter explanation.""" + blocked_message = "The provider blocked this prompt because it matched a content filter." + piece = MessagePiece( + role="assistant", + original_value=blocked_message, + original_value_data_type="error", + converted_value=blocked_message, + converted_value_data_type="error", + conversation_id="conversation-id", + sequence=1, + response_error="blocked", + ) + + view = MessagePieceView.from_domain(piece) + + assert view.original_value == blocked_message + assert view.converted_value == blocked_message + def test_scores_are_score_views(self) -> None: """Test that nested scores serialize with the ScoreView computed field.""" piece = _make_piece() diff --git a/tests/unit/backend/test_scenario_run_routes.py b/tests/unit/backend/test_scenario_run_routes.py index 7b82b3101a..8b8cd37491 100644 --- a/tests/unit/backend/test_scenario_run_routes.py +++ b/tests/unit/backend/test_scenario_run_routes.py @@ -91,6 +91,22 @@ def test_start_run_invalid_scenario_returns_400(self, client: TestClient) -> Non assert response.status_code == status.HTTP_400_BAD_REQUEST assert "not found" in response.json()["detail"] + def test_start_run_preserves_safe_parameter_validation(self, client: TestClient) -> None: + """Known scenario parameter validation remains actionable.""" + validation_message = "Unknown parameter 'bogus'. Valid parameters: objective, max_turns" + with patch("pyrit.backend.routes.scenarios.get_scenario_run_service") as mock_get: + mock_service = MagicMock() + mock_service.start_run_async = AsyncMock(side_effect=ClientRequestError(validation_message)) + mock_get.return_value = mock_service + + response = client.post( + "/api/scenarios/runs", + json={"scenario_name": "test.scenario", "target_name": "my_target", "scenario_params": {"bogus": 1}}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == validation_message + def test_start_run_missing_required_fields_returns_422(self, client: TestClient) -> None: """Test that missing required fields returns 422.""" response = client.post("/api/scenarios/runs", json={}) @@ -303,6 +319,7 @@ def test_get_results_sanitizes_persisted_attack_errors(self, client: TestClient) conversation_id="conv-1", objective="Extract sensitive info", outcome=AttackOutcome.ERROR, + outcome_reason=f"Exception: ProviderConnectionError: {internal_detail}", executed_turns=1, execution_time_ms=100, timestamp=datetime(2025, 1, 1, tzinfo=timezone.utc), @@ -344,4 +361,5 @@ def test_get_results_sanitizes_persisted_attack_errors(self, client: TestClient) assert response.status_code == status.HTTP_200_OK assert internal_detail not in response.text assert "api_key=sk-test" not in response.text + assert f"Exception: ProviderConnectionError: {internal_detail}" not in response.text assert "Attack execution failed. Check server logs for details." in response.text diff --git a/tests/unit/backend/test_scenario_run_service.py b/tests/unit/backend/test_scenario_run_service.py index 43e2a2c5cf..2e74abc42d 100644 --- a/tests/unit/backend/test_scenario_run_service.py +++ b/tests/unit/backend/test_scenario_run_service.py @@ -12,6 +12,7 @@ import pytest import pyrit.backend.services.scenario_run_service as _svc_mod +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.services.scenario_run_service import ( _DEFAULT_MAX_CONCURRENT_RUNS, ScenarioRunService, @@ -19,6 +20,7 @@ from pyrit.converter import Converter from pyrit.models import AttackOutcome, AttackResult, ComponentIdentifier, ScenarioRunState from pyrit.models.catalog.scenario import RunScenarioRequest +from pyrit.registry import RegistryValidationError from pyrit.scenario.core import DatasetAttackConfiguration, DatasetConfiguration from pyrit.scenario.core.scenario_technique import ScenarioTechnique from unit.mocks import make_scenario_result @@ -530,6 +532,17 @@ async def test_start_run_passes_scenario_result_id_for_resume(self, mock_all_reg assert call.args[0] == "foundry.red_team_agent" assert call.kwargs["scenario_result_id"] == "existing-result-uuid" + async def test_start_run_exposes_registry_parameter_validation(self, mock_all_registries) -> None: + """Scenario parameter resolution failures are marked safe for API clients.""" + validation_message = "Unknown parameter 'bogus'. Valid parameters: objective, max_turns" + mock_all_registries["scenario_registry"].create_and_initialize_async.side_effect = RegistryValidationError( + validation_message + ) + service = ScenarioRunService() + + with pytest.raises(ClientRequestError, match="Unknown parameter 'bogus'"): + await service.start_run_async(request=_make_request()) + async def test_start_run_omits_scenario_result_id_when_none(self, mock_all_registries) -> None: """Test that scenario_result_id is None when not provided in the request.""" service = ScenarioRunService() diff --git a/tests/unit/backend/test_target_service.py b/tests/unit/backend/test_target_service.py index d6fd6df784..941c76bcc1 100644 --- a/tests/unit/backend/test_target_service.py +++ b/tests/unit/backend/test_target_service.py @@ -10,6 +10,7 @@ import pytest +from pyrit.backend.exceptions import ClientRequestError from pyrit.backend.models.targets import CreateTargetRequest from pyrit.backend.services.target_service import TargetService, get_target_service from pyrit.models import ComponentIdentifier @@ -368,9 +369,23 @@ async def test_create_gandalf_target_rejects_invalid_level(self, sqlite_instance params={"level": "unknown"}, ) - with pytest.raises(ValueError, match="Parameter 'level'.*expected one of"): + with pytest.raises(ClientRequestError, match="Parameter 'level'.*expected one of"): await service.create_target_async(request=request) + async def test_create_target_does_not_mark_constructor_value_error_safe(self) -> None: + """Constructor failures remain ordinary ValueErrors for route sanitization.""" + internal_detail = r"provider secret=sk-test at C:\internal\target.py" + service = TargetService() + request = CreateTargetRequest(type="TextTarget", params={}) + + with ( + patch.object(service._registry, "create_instance", side_effect=ValueError(internal_detail)), + pytest.raises(ValueError, match="sk-test") as exc_info, + ): + await service.create_target_async(request=request) + + assert type(exc_info.value) is ValueError + async def test_create_azure_blob_target_coerces_content_type_string(self, sqlite_instance) -> None: """An explicit blob content type from JSON is coerced before target construction.""" service = TargetService() diff --git a/tests/unit/registry/test_initializer_registry.py b/tests/unit/registry/test_initializer_registry.py index 395e8d12b2..de80409448 100644 --- a/tests/unit/registry/test_initializer_registry.py +++ b/tests/unit/registry/test_initializer_registry.py @@ -8,6 +8,7 @@ import pytest from pyrit.models.parameter import Parameter +from pyrit.registry import RegistryValidationError from pyrit.registry.components.initializer_registry import PYRIT_PATH, InitializerRegistry from pyrit.setup.pyrit_initializer import PyRITInitializer @@ -300,11 +301,11 @@ def test_create_and_configure_without_params_leaves_instance_unconfigured(lazy_r assert instance.params == {} -def test_create_and_configure_unknown_param_raises_value_error(lazy_registry): - """Test that an unknown parameter raises ValueError during configuration.""" +def test_create_and_configure_unknown_param_raises_registry_validation_error(lazy_registry): + """Test that an unknown parameter is identified as registry validation.""" lazy_registry.register_class(_ParamInitializer, name="param_init") - with pytest.raises(ValueError, match="unknown parameter"): + with pytest.raises(RegistryValidationError, match="unknown parameter"): lazy_registry.create_and_configure("param_init", initializer_params={"bogus": "x"}) diff --git a/tests/unit/registry/test_registry.py b/tests/unit/registry/test_registry.py index 8f0aef0388..3fb6a66a41 100644 --- a/tests/unit/registry/test_registry.py +++ b/tests/unit/registry/test_registry.py @@ -17,6 +17,7 @@ import pytest +from pyrit.registry import RegistryValidationError from pyrit.registry.registry import Registry, _get_metadata_value, _matches_filters from pyrit.registry.registry_metadata import RegistryMetadata @@ -36,6 +37,11 @@ def __init__(self, *, size: int = 1) -> None: self.size = size +class ExplodingWidget: + def __init__(self, *, detail: str) -> None: + raise ValueError(detail) + + class UnregisteredWidget: """An unregistered widget.""" @@ -117,6 +123,24 @@ def test_create_instance_builds_object(): assert widget.size == 3 +def test_create_instance_wraps_only_resolution_value_errors(): + registry = WidgetRegistry() + + with pytest.raises(RegistryValidationError, match="Unknown parameter 'unknown'"): + registry.create_instance("SampleWidget", unknown="value") + + +def test_create_instance_preserves_constructor_value_errors(): + registry = WidgetRegistry() + registry.register_class(ExplodingWidget) + internal_detail = r"provider secret=sk-test at C:\internal\widget.py" + + with pytest.raises(ValueError, match="sk-test") as exc_info: + registry.create_instance("ExplodingWidget", detail=internal_detail) + + assert type(exc_info.value) is ValueError + + def test_lazy_discovery_defers_until_access(): registry = WidgetRegistry(lazy_discovery=True) From 8c4c2b280c44b3cb42ba8fddb0bbf65ebd4fc364 Mon Sep 17 00:00:00 2001 From: hannahwestra25 Date: Tue, 21 Jul 2026 09:46:59 -0400 Subject: [PATCH 3/5] Fix error sanitization CI checks Update frontend expectations for generic runtime and server errors and apply Ruff formatting required by pre-commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2040f373-2101-4e34-a067-70c15ac7211e --- .../src/components/Chat/ChatWindow.test.tsx | 12 ++-- .../Chat/ConversationPanel.test.tsx | 6 +- .../Config/CreateTargetDialog.test.tsx | 6 +- .../components/Config/TargetConfig.test.tsx | 4 +- .../components/History/AttackHistory.test.tsx | 2 +- pyrit/backend/models/attacks.py | 8 +-- pyrit/backend/services/attack_service.py | 4 +- .../backend/services/scenario_run_service.py | 6 +- tests/unit/backend/test_api_routes.py | 60 +++++++++---------- .../unit/backend/test_scenario_run_routes.py | 2 - 10 files changed, 49 insertions(+), 61 deletions(-) diff --git a/frontend/src/components/Chat/ChatWindow.test.tsx b/frontend/src/components/Chat/ChatWindow.test.tsx index f705915964..1a8bfc3036 100644 --- a/frontend/src/components/Chat/ChatWindow.test.tsx +++ b/frontend/src/components/Chat/ChatWindow.test.tsx @@ -814,7 +814,7 @@ describe("ChatWindow Integration", () => { await user.click(screen.getByRole("button", { name: /send/i })); await waitFor(() => { - expect(screen.getByText(/Network error/)).toBeInTheDocument(); + expect(screen.getByText(/An unexpected error occurred\./)).toBeInTheDocument(); }); }); @@ -847,11 +847,11 @@ describe("ChatWindow Integration", () => { await user.click(screen.getByRole("button", { name: /send/i })); await waitFor(() => { - expect(screen.getByText(/Request failed with status code 404/)).toBeInTheDocument(); + expect(screen.getByText(/An unexpected error occurred\./)).toBeInTheDocument(); }); }); - it("should extract detail from axios-style error response", async () => { + it("should sanitize detail from a 500 axios-style error response", async () => { const user = userEvent.setup(); mockedMapper.buildMessagePieces.mockResolvedValue([ @@ -881,7 +881,7 @@ describe("ChatWindow Integration", () => { await user.click(screen.getByRole("button", { name: /send/i })); await waitFor(() => { - expect(screen.getByText(/Failed to add message/)).toBeInTheDocument(); + expect(screen.getByText(/The server could not complete the request\. Please try again\./)).toBeInTheDocument(); }); }); @@ -915,7 +915,7 @@ describe("ChatWindow Integration", () => { await user.click(screen.getByRole("button", { name: /send/i })); await waitFor(() => { - expect(screen.getByText(/Internal Server Error/)).toBeInTheDocument(); + expect(screen.getByText(/The server could not complete the request\. Please try again\./)).toBeInTheDocument(); }); }); @@ -941,7 +941,7 @@ describe("ChatWindow Integration", () => { await user.click(screen.getByRole("button", { name: /send/i })); await waitFor(() => { - expect(screen.getByText(/string error/)).toBeInTheDocument(); + expect(screen.getByText(/An unexpected error occurred\./)).toBeInTheDocument(); }); }); diff --git a/frontend/src/components/Chat/ConversationPanel.test.tsx b/frontend/src/components/Chat/ConversationPanel.test.tsx index 286a23ed63..a7da382559 100644 --- a/frontend/src/components/Chat/ConversationPanel.test.tsx +++ b/frontend/src/components/Chat/ConversationPanel.test.tsx @@ -366,7 +366,7 @@ describe("ConversationPanel", () => { await waitFor(() => { expect(screen.getByTestId("conversation-error")).toBeInTheDocument(); }); - expect(screen.getByText("Network error")).toBeInTheDocument(); + expect(screen.getByText("An unexpected error occurred.")).toBeInTheDocument(); }); // ----------------------------------------------------------------------- @@ -428,7 +428,9 @@ describe("ConversationPanel", () => { await waitFor(() => { expect(screen.getByTestId("conversation-error")).toBeInTheDocument(); }); - expect(screen.getByText("Server exploded")).toBeInTheDocument(); + expect( + screen.getByText("The server could not complete the request. Please try again.") + ).toBeInTheDocument(); expect(screen.getByTestId("conversation-retry-btn")).toBeInTheDocument(); }); diff --git a/frontend/src/components/Config/CreateTargetDialog.test.tsx b/frontend/src/components/Config/CreateTargetDialog.test.tsx index 62c2a7e8fe..686ef4fb47 100644 --- a/frontend/src/components/Config/CreateTargetDialog.test.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.test.tsx @@ -353,7 +353,7 @@ describe("CreateTargetDialog", () => { await user.click(screen.getByText("Create Target")); await waitFor(() => { - expect(screen.getByText("Invalid API key")).toBeInTheDocument(); + expect(screen.getByText("An unexpected error occurred.")).toBeInTheDocument(); }); }); @@ -449,7 +449,7 @@ describe("CreateTargetDialog", () => { }); }); - it("should surface string throws verbatim via toApiError", async () => { + it("should sanitize string throws via toApiError", async () => { const user = userEvent.setup(); mockedTargetsApi.createTarget.mockRejectedValue("string error"); @@ -469,7 +469,7 @@ describe("CreateTargetDialog", () => { await user.click(screen.getByText("Create Target")); await waitFor(() => { - expect(screen.getByText("string error")).toBeInTheDocument(); + expect(screen.getByText("An unexpected error occurred.")).toBeInTheDocument(); }); }); diff --git a/frontend/src/components/Config/TargetConfig.test.tsx b/frontend/src/components/Config/TargetConfig.test.tsx index 8aed790042..ac2203fdba 100644 --- a/frontend/src/components/Config/TargetConfig.test.tsx +++ b/frontend/src/components/Config/TargetConfig.test.tsx @@ -135,7 +135,9 @@ describe("TargetConfig", () => { await waitFor( () => { - expect(screen.getByText(/Connection refused/)).toBeInTheDocument(); + expect( + screen.getByText(/An unexpected error occurred\./) + ).toBeInTheDocument(); }, { timeout: 15000 } ); diff --git a/frontend/src/components/History/AttackHistory.test.tsx b/frontend/src/components/History/AttackHistory.test.tsx index 641431ea9d..20805b7ac1 100644 --- a/frontend/src/components/History/AttackHistory.test.tsx +++ b/frontend/src/components/History/AttackHistory.test.tsx @@ -425,7 +425,7 @@ describe('AttackHistory', () => { await waitFor(() => { expect(screen.getByTestId('error-state')).toBeInTheDocument() }) - expect(screen.getByText('Internal server error')).toBeInTheDocument() + expect(screen.getByText('The server could not complete the request. Please try again.')).toBeInTheDocument() expect(screen.getByTestId('retry-btn')).toBeInTheDocument() }) diff --git a/pyrit/backend/models/attacks.py b/pyrit/backend/models/attacks.py index 029da658f5..bbb13dd015 100644 --- a/pyrit/backend/models/attacks.py +++ b/pyrit/backend/models/attacks.py @@ -109,9 +109,7 @@ class MessagePieceView(MessagePiece): default=None, description="Description of the error if response_error is not 'none'" ) - _INTERNAL_ERROR_MESSAGE: ClassVar[str] = ( - "The target could not complete the request. Check server logs for details." - ) + _INTERNAL_ERROR_MESSAGE: ClassVar[str] = "The target could not complete the request. Check server logs for details." @classmethod def from_domain( @@ -166,9 +164,7 @@ def from_domain( converted_value_url=converted_value_url, original_value_mime_type=infer_mime_type(value=original_value, data_type=orig_dtype), converted_value_mime_type=infer_mime_type(value=converted_value, data_type=conv_dtype), - original_filename=build_filename( - data_type=orig_dtype, sha256=original_value_sha256, value=original_value - ), + original_filename=build_filename(data_type=orig_dtype, sha256=original_value_sha256, value=original_value), converted_filename=build_filename( data_type=conv_dtype, sha256=converted_value_sha256, value=converted_value ), diff --git a/pyrit/backend/services/attack_service.py b/pyrit/backend/services/attack_service.py index be30991584..11c111375c 100644 --- a/pyrit/backend/services/attack_service.py +++ b/pyrit/backend/services/attack_service.py @@ -614,9 +614,7 @@ async def add_message_async(self, *, attack_result_id: str, request: AddMessageR # Validate the target conversation belongs to this attack (main + pruned only) if msg_conversation_id not in ar.get_active_conversation_ids(): - raise ClientRequestError( - f"Conversation '{msg_conversation_id}' is not part of attack '{attack_result_id}'" - ) + raise ClientRequestError(f"Conversation '{msg_conversation_id}' is not part of attack '{attack_result_id}'") target_registry_name = request.target_registry_name if request.send and not target_registry_name: diff --git a/pyrit/backend/services/scenario_run_service.py b/pyrit/backend/services/scenario_run_service.py index 919e009431..9dbef611b1 100644 --- a/pyrit/backend/services/scenario_run_service.py +++ b/pyrit/backend/services/scenario_run_service.py @@ -56,11 +56,7 @@ def _remove_endpoint_fields(value: Any) -> Any: A recursively copied value without endpoint fields. """ if isinstance(value, dict): - return { - key: _remove_endpoint_fields(item) - for key, item in value.items() - if str(key).lower() != "endpoint" - } + return {key: _remove_endpoint_fields(item) for key, item in value.items() if str(key).lower() != "endpoint"} if isinstance(value, list): return [_remove_endpoint_fields(item) for item in value] return value diff --git a/tests/unit/backend/test_api_routes.py b/tests/unit/backend/test_api_routes.py index ff54964b10..ac37af7823 100644 --- a/tests/unit/backend/test_api_routes.py +++ b/tests/unit/backend/test_api_routes.py @@ -341,9 +341,7 @@ def test_add_message_attack_not_found(self, client: TestClient) -> None: """Test adding message to non-existent attack returns 404.""" with patch("pyrit.backend.routes.attacks.get_attack_service") as mock_get_service: mock_service = MagicMock() - mock_service.add_message_async = AsyncMock( - side_effect=ClientRequestError("Attack 'nonexistent' not found") - ) + mock_service.add_message_async = AsyncMock(side_effect=ClientRequestError("Attack 'nonexistent' not found")) mock_get_service.return_value = mock_service response = client.post( @@ -914,20 +912,20 @@ def test_create_target_invalid_type(self, client: TestClient) -> None: assert response.json()["detail"] == "Target type not found" def test_create_target_preserves_safe_parameter_validation(self, client: TestClient) -> None: - """Known registry parameter validation remains actionable.""" - validation_message = "Unknown parameter 'bogus'. Valid parameters: endpoint, model_name" - with patch("pyrit.backend.routes.targets.get_target_service") as mock_get_service: - mock_service = MagicMock() - mock_service.create_target_async = AsyncMock(side_effect=ClientRequestError(validation_message)) - mock_get_service.return_value = mock_service - - response = client.post( - "/api/targets", - json={"type": "TextTarget", "params": {"bogus": "value"}}, - ) + """Known registry parameter validation remains actionable.""" + validation_message = "Unknown parameter 'bogus'. Valid parameters: endpoint, model_name" + with patch("pyrit.backend.routes.targets.get_target_service") as mock_get_service: + mock_service = MagicMock() + mock_service.create_target_async = AsyncMock(side_effect=ClientRequestError(validation_message)) + mock_get_service.return_value = mock_service - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.json()["detail"] == validation_message + response = client.post( + "/api/targets", + json={"type": "TextTarget", "params": {"bogus": "value"}}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == validation_message def test_create_target_internal_error(self, client: TestClient) -> None: """Test target creation with internal error returns 500.""" @@ -1132,9 +1130,7 @@ def test_create_converter_invalid_type(self, client: TestClient) -> None: """Test converter creation with invalid type.""" with patch("pyrit.backend.routes.converters.get_converter_service") as mock_get_service: mock_service = MagicMock() - mock_service.create_converter_async = AsyncMock( - side_effect=ClientRequestError("Converter type not found") - ) + mock_service.create_converter_async = AsyncMock(side_effect=ClientRequestError("Converter type not found")) mock_get_service.return_value = mock_service response = client.post( @@ -1145,20 +1141,20 @@ def test_create_converter_invalid_type(self, client: TestClient) -> None: assert response.status_code == status.HTTP_400_BAD_REQUEST def test_create_converter_preserves_safe_parameter_validation(self, client: TestClient) -> None: - """Known registry parameter validation remains actionable.""" - validation_message = "Could not coerce parameter 'caesar_offset' to int" - with patch("pyrit.backend.routes.converters.get_converter_service") as mock_get_service: - mock_service = MagicMock() - mock_service.create_converter_async = AsyncMock(side_effect=ClientRequestError(validation_message)) - mock_get_service.return_value = mock_service - - response = client.post( - "/api/converters", - json={"type": "CaesarConverter", "params": {"caesar_offset": "invalid"}}, - ) + """Known registry parameter validation remains actionable.""" + validation_message = "Could not coerce parameter 'caesar_offset' to int" + with patch("pyrit.backend.routes.converters.get_converter_service") as mock_get_service: + mock_service = MagicMock() + mock_service.create_converter_async = AsyncMock(side_effect=ClientRequestError(validation_message)) + mock_get_service.return_value = mock_service - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.json()["detail"] == validation_message + response = client.post( + "/api/converters", + json={"type": "CaesarConverter", "params": {"caesar_offset": "invalid"}}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == validation_message def test_create_converter_internal_error(self, client: TestClient) -> None: """Test converter creation with internal error returns 500.""" diff --git a/tests/unit/backend/test_scenario_run_routes.py b/tests/unit/backend/test_scenario_run_routes.py index 8b8cd37491..12b308ba8e 100644 --- a/tests/unit/backend/test_scenario_run_routes.py +++ b/tests/unit/backend/test_scenario_run_routes.py @@ -279,7 +279,6 @@ def test_get_results_returns_200(self, client: TestClient) -> None: service._active_tasks = {} with patch("pyrit.backend.routes.scenarios.get_scenario_run_service", return_value=service): - response = client.get("/api/scenarios/runs/test-run-id/results") assert response.status_code == status.HTTP_200_OK @@ -355,7 +354,6 @@ def test_get_results_sanitizes_persisted_attack_errors(self, client: TestClient) service._active_tasks = {} with patch("pyrit.backend.routes.scenarios.get_scenario_run_service", return_value=service): - response = client.get("/api/scenarios/runs/test-run-id/results") assert response.status_code == status.HTTP_200_OK From 2712fcfd30ec24e0bc96f3b7999443353b2acd9b Mon Sep 17 00:00:00 2001 From: hannahwestra25 Date: Tue, 21 Jul 2026 12:11:00 -0400 Subject: [PATCH 4/5] Remove non-actionable server log guidance Keep public failure messages generic without directing users to operator-only logs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2040f373-2101-4e34-a067-70c15ac7211e --- frontend/src/utils/messageMapper.test.ts | 6 ++---- frontend/src/utils/messageMapper.ts | 2 +- pyrit/backend/mappers/attack_mappers.py | 2 +- pyrit/backend/models/attacks.py | 2 +- pyrit/backend/routes/attacks.py | 2 +- pyrit/backend/routes/converters.py | 4 ++-- pyrit/backend/routes/targets.py | 2 +- pyrit/backend/services/scenario_run_service.py | 6 +++--- tests/unit/backend/test_api_routes.py | 6 +++--- tests/unit/backend/test_attack_service.py | 4 +--- tests/unit/backend/test_mappers.py | 2 +- tests/unit/backend/test_scenario_run_routes.py | 2 +- tests/unit/backend/test_scenario_run_service.py | 12 +++++------- 13 files changed, 23 insertions(+), 29 deletions(-) diff --git a/frontend/src/utils/messageMapper.test.ts b/frontend/src/utils/messageMapper.test.ts index 5127ca1099..144eac325a 100644 --- a/frontend/src/utils/messageMapper.test.ts +++ b/frontend/src/utils/messageMapper.test.ts @@ -339,11 +339,9 @@ describe("messageMapper", () => { const result = backendMessageToFrontend(msg); - expect(result.content).toBe( - "The target could not complete the request. Check server logs for details." - ); + expect(result.content).toBe("The target could not complete the request."); expect(result.error?.description).toBe( - "The target could not complete the request. Check server logs for details." + "The target could not complete the request." ); expect(JSON.stringify(result)).not.toContain("sk-test"); expect(JSON.stringify(result)).not.toContain("internal\\\\provider.py"); diff --git a/frontend/src/utils/messageMapper.ts b/frontend/src/utils/messageMapper.ts index 3b964ed5a4..b6e8e461af 100644 --- a/frontend/src/utils/messageMapper.ts +++ b/frontend/src/utils/messageMapper.ts @@ -7,7 +7,7 @@ import type { MessagePieceRequest, } from '../types' -const INTERNAL_MESSAGE_ERROR = 'The target could not complete the request. Check server logs for details.' +const INTERNAL_MESSAGE_ERROR = 'The target could not complete the request.' /** * Read a File and return its contents as a base64-encoded string (no data URI prefix). diff --git a/pyrit/backend/mappers/attack_mappers.py b/pyrit/backend/mappers/attack_mappers.py index a926c76b32..e86f4111e5 100644 --- a/pyrit/backend/mappers/attack_mappers.py +++ b/pyrit/backend/mappers/attack_mappers.py @@ -47,7 +47,7 @@ logger = logging.getLogger(__name__) -_ATTACK_ERROR_MESSAGE = "Attack execution failed. Check server logs for details." +_ATTACK_ERROR_MESSAGE = "Attack execution failed." if TYPE_CHECKING: from pyrit.models.conversation_stats import ConversationStats diff --git a/pyrit/backend/models/attacks.py b/pyrit/backend/models/attacks.py index 03b6462227..fd79d7dd40 100644 --- a/pyrit/backend/models/attacks.py +++ b/pyrit/backend/models/attacks.py @@ -110,7 +110,7 @@ class MessagePieceView(MessagePiece): default=None, description="Description of the error if response_error is not 'none'" ) - _INTERNAL_ERROR_MESSAGE: ClassVar[str] = "The target could not complete the request. Check server logs for details." + _INTERNAL_ERROR_MESSAGE: ClassVar[str] = "The target could not complete the request." @classmethod def from_domain( diff --git a/pyrit/backend/routes/attacks.py b/pyrit/backend/routes/attacks.py index 1fd97bd478..b4b9bb3106 100644 --- a/pyrit/backend/routes/attacks.py +++ b/pyrit/backend/routes/attacks.py @@ -476,5 +476,5 @@ async def add_message( # pyrit-async-suffix-exempt logger.exception("Failed to add message to attack '%s'", attack_result_id) raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Internal server error. Check server logs for details.", + detail="Internal server error.", ) from e diff --git a/pyrit/backend/routes/converters.py b/pyrit/backend/routes/converters.py index ef912b49a3..3349a06619 100644 --- a/pyrit/backend/routes/converters.py +++ b/pyrit/backend/routes/converters.py @@ -99,7 +99,7 @@ async def create_converter(request: CreateConverterRequest) -> CreateConverterRe logger.exception("Failed to create converter") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Failed to create converter. Check server logs for details.", + detail="Failed to create converter.", ) from e @@ -165,5 +165,5 @@ async def preview_conversion(request: ConverterPreviewRequest) -> ConverterPrevi logger.exception("Failed to preview converter") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Converter preview failed. Check server logs for details.", + detail="Converter preview failed.", ) from e diff --git a/pyrit/backend/routes/targets.py b/pyrit/backend/routes/targets.py index 27da547220..108d2941e4 100644 --- a/pyrit/backend/routes/targets.py +++ b/pyrit/backend/routes/targets.py @@ -112,7 +112,7 @@ async def create_target( logger.exception("Failed to create target") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Failed to create target. Check server logs for details.", + detail="Failed to create target.", ) from e diff --git a/pyrit/backend/services/scenario_run_service.py b/pyrit/backend/services/scenario_run_service.py index 9dbef611b1..4ff54cbfd9 100644 --- a/pyrit/backend/services/scenario_run_service.py +++ b/pyrit/backend/services/scenario_run_service.py @@ -43,9 +43,9 @@ _DEFAULT_MAX_CONCURRENT_RUNS = 3 _CONVERTER_MODIFIER_PREFIX = "converter." -_SCENARIO_ERROR_MESSAGE = "Scenario run failed. Check server logs for details." -_ATTACK_ERROR_MESSAGE = "Attack execution failed. Check server logs for details." -_RETRY_ERROR_MESSAGE = "Retryable operation failed. Check server logs for details." +_SCENARIO_ERROR_MESSAGE = "Scenario run failed." +_ATTACK_ERROR_MESSAGE = "Attack execution failed." +_RETRY_ERROR_MESSAGE = "Retryable operation failed." def _remove_endpoint_fields(value: Any) -> Any: diff --git a/tests/unit/backend/test_api_routes.py b/tests/unit/backend/test_api_routes.py index ac37af7823..f15fbbf4f5 100644 --- a/tests/unit/backend/test_api_routes.py +++ b/tests/unit/backend/test_api_routes.py @@ -941,7 +941,7 @@ def test_create_target_internal_error(self, client: TestClient) -> None: ) assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR - assert response.json()["detail"] == "Failed to create target. Check server logs for details." + assert response.json()["detail"] == "Failed to create target." assert internal_detail not in response.text def test_create_target_invalid_parameters_do_not_leak_details(self, client: TestClient) -> None: @@ -1170,7 +1170,7 @@ def test_create_converter_internal_error(self, client: TestClient) -> None: ) assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR - assert response.json()["detail"] == "Failed to create converter. Check server logs for details." + assert response.json()["detail"] == "Failed to create converter." assert internal_detail not in response.text def test_create_converter_invalid_parameters_do_not_leak_details(self, client: TestClient) -> None: @@ -1298,7 +1298,7 @@ def test_preview_conversion_internal_error(self, client: TestClient) -> None: ) assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR - assert response.json()["detail"] == "Converter preview failed. Check server logs for details." + assert response.json()["detail"] == "Converter preview failed." assert internal_detail not in response.text diff --git a/tests/unit/backend/test_attack_service.py b/tests/unit/backend/test_attack_service.py index 6cba0af70e..dacb01eda7 100644 --- a/tests/unit/backend/test_attack_service.py +++ b/tests/unit/backend/test_attack_service.py @@ -1196,9 +1196,7 @@ async def _raise_after_store(**_): returned_pieces = [piece for message in result.messages.messages for piece in message.message_pieces] error_views = [piece for piece in returned_pieces if piece.response_error == "processing"] assert len(error_views) == 1 - assert error_views[0].converted_value == ( - "The target could not complete the request. Check server logs for details." - ) + assert error_views[0].converted_value == "The target could not complete the request." assert "sk-test" not in error_views[0].model_dump_json() assert "internal" not in error_views[0].model_dump_json() diff --git a/tests/unit/backend/test_mappers.py b/tests/unit/backend/test_mappers.py index 237e1b8275..ff7a1ce1b5 100644 --- a/tests/unit/backend/test_mappers.py +++ b/tests/unit/backend/test_mappers.py @@ -168,7 +168,7 @@ async def test_error_outcome_reason_is_sanitized(self) -> None: summary = await attack_result_to_summary_async(ar, stats=ConversationStats(message_count=0)) - assert summary.outcome_reason == "Attack execution failed. Check server logs for details." + assert summary.outcome_reason == "Attack execution failed." assert internal_detail not in summary.model_dump_json() async def test_round_robin_target_includes_canonical_identifier_hash(self) -> None: diff --git a/tests/unit/backend/test_scenario_run_routes.py b/tests/unit/backend/test_scenario_run_routes.py index 12b308ba8e..ed43a2228b 100644 --- a/tests/unit/backend/test_scenario_run_routes.py +++ b/tests/unit/backend/test_scenario_run_routes.py @@ -360,4 +360,4 @@ def test_get_results_sanitizes_persisted_attack_errors(self, client: TestClient) assert internal_detail not in response.text assert "api_key=sk-test" not in response.text assert f"Exception: ProviderConnectionError: {internal_detail}" not in response.text - assert "Attack execution failed. Check server logs for details." in response.text + assert "Attack execution failed." in response.text diff --git a/tests/unit/backend/test_scenario_run_service.py b/tests/unit/backend/test_scenario_run_service.py index 2e74abc42d..71b4deab80 100644 --- a/tests/unit/backend/test_scenario_run_service.py +++ b/tests/unit/backend/test_scenario_run_service.py @@ -600,7 +600,7 @@ def test_get_run_falls_back_to_persisted_error(self, mock_memory) -> None: fetched = service.get_run(scenario_result_id="sr-fail") assert fetched is not None - assert fetched.error == "Scenario run failed. Check server logs for details." + assert fetched.error == "Scenario run failed." assert fetched.error_type == "ScenarioRunError" assert internal_detail not in fetched.model_dump_json() mock_memory.get_attack_results.assert_called_once_with( @@ -743,13 +743,13 @@ async def test_execute_run_fails_with_error(self, mock_all_registries) -> None: await active.task # Error is stored on the active task until get_run reads it - assert active.error == "Scenario run failed. Check server logs for details." + assert active.error == "Scenario run failed." assert response.scenario_result_id in service._active_tasks # get_run should surface the error and clean up fetched = service.get_run(scenario_result_id=response.scenario_result_id) assert fetched is not None - assert fetched.error == "Scenario run failed. Check server logs for details." + assert fetched.error == "Scenario run failed." assert internal_detail not in fetched.model_dump_json() assert response.scenario_result_id not in service._active_tasks @@ -928,12 +928,10 @@ def test_error_attacks_and_retries_are_surfaced(self, mock_memory) -> None: failed = fetched.failed_attacks[0] assert failed.atomic_attack_name == "baseline_airt_hate" assert failed.error_type == "AttackExecutionError" - assert failed.error_message == "Attack execution failed. Check server logs for details." + assert failed.error_message == "Attack execution failed." assert failed.total_retries == 4 assert fetched.attack_retries[0].retries[0].exception_type == "RetryableOperationError" - assert fetched.attack_retries[0].retries[0].exception_message == ( - "Retryable operation failed. Check server logs for details." - ) + assert fetched.attack_retries[0].retries[0].exception_message == "Retryable operation failed." assert "sk-test" not in fetched.model_dump_json() def test_no_failed_attacks_when_all_succeed(self, mock_memory) -> None: From fb1223ff66e54c51af288cfd3e223af7db92889a Mon Sep 17 00:00:00 2001 From: hannahwestra25 Date: Tue, 21 Jul 2026 12:44:10 -0400 Subject: [PATCH 5/5] Update sanitized error E2E expectations Assert the generic client-safe 500 response instead of raw backend details. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2040f373-2101-4e34-a067-70c15ac7211e --- frontend/e2e/errors.spec.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/frontend/e2e/errors.spec.ts b/frontend/e2e/errors.spec.ts index 84afa9953c..19f99bcf4f 100644 --- a/frontend/e2e/errors.spec.ts +++ b/frontend/e2e/errors.spec.ts @@ -259,8 +259,9 @@ test.describe("Error: backend 500 on send message", () => { // Error message should appear in chat await expect( - page.getByText(/Internal server error/i), + page.getByText(/The server could not complete the request\. Please try again\./i), ).toBeVisible({ timeout: 10000 }); + await expect(page.getByText(/Internal server error/i)).toHaveCount(0); // The failed text should be restored in the input for easy re-send await expect(input).toHaveValue("This should fail", { timeout: 5000 }); @@ -282,9 +283,10 @@ test.describe("Error: backend 500 on send message", () => { await input.fill("First send fails"); await page.getByRole("button", { name: /send/i }).click(); - await expect(page.getByText(/Internal server error/i)).toBeVisible({ - timeout: 10000, - }); + await expect( + page.getByText(/The server could not complete the request\. Please try again\./i), + ).toBeVisible({ timeout: 10000 }); + await expect(page.getByText(/Internal server error/i)).toHaveCount(0); await expect(page.getByTestId("loading-state")).toHaveCount(0); await expect(input).toHaveValue("First send fails", { timeout: 5000 }); });