From bae04ff810c6df306685d0458d20d70bc1c15369 Mon Sep 17 00:00:00 2001 From: Cory Bullinger Date: Thu, 4 Jun 2026 10:11:58 -0400 Subject: [PATCH 1/4] fix(python-fastapi): stop exposing exception details in API errors Add server_error_response() to log failures server-side and return generic messages. Resolves CodeQL py/stack-trace-exposure alerts (#7-22). Co-authored-by: Cursor --- mflix/server/python-fastapi/main.py | 1 - .../python-fastapi/src/routers/movies.py | 229 ++++++++---------- .../python-fastapi/src/utils/errorResponse.py | 22 ++ 3 files changed, 119 insertions(+), 133 deletions(-) diff --git a/mflix/server/python-fastapi/main.py b/mflix/server/python-fastapi/main.py index 81048f9..87b6271 100644 --- a/mflix/server/python-fastapi/main.py +++ b/mflix/server/python-fastapi/main.py @@ -162,7 +162,6 @@ async def voyage_api_error_handler(request: Request, exc: VoyageAPIError): content=create_error_response( message="Vector search service unavailable", code="VOYAGE_API_ERROR", - details=exc.message ) ) diff --git a/mflix/server/python-fastapi/src/routers/movies.py b/mflix/server/python-fastapi/src/routers/movies.py index 50170eb..b46e8cc 100644 --- a/mflix/server/python-fastapi/src/routers/movies.py +++ b/mflix/server/python-fastapi/src/routers/movies.py @@ -4,7 +4,7 @@ from src.models.models import VectorSearchResult, CreateMovieRequest, Movie, SuccessResponse, UpdateMovieRequest, SearchMoviesResponse from typing import Any, List, Optional from src.utils.successResponse import create_success_response -from src.utils.errorResponse import create_error_response +from src.utils.errorResponse import create_error_response, server_error_response from src.utils.response_docs import ( VECTOR_SEARCH_RESPONSES, OBJECTID_VALIDATION_RESPONSES, @@ -14,6 +14,7 @@ CRUD_WITH_OBJECTID_RESPONSES ) from src.utils.exceptions import VoyageAuthError, VoyageAPIError +from src.utils.logger import logger from bson import ObjectId, errors import re from bson.errors import InvalidId @@ -276,13 +277,11 @@ async def search_movies( # Execute the aggregation pipeline using the helper function try: results = await execute_aggregation(aggregation_pipeline) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"An error occurred while performing the search: {str(e)}", - code="SEARCH_ERROR" - ) + except Exception: + return server_error_response( + "An error occurred while performing the search.", + "SEARCH_ERROR", + log_context="search_movies", ) @@ -443,17 +442,11 @@ async def vector_search_movies( except VoyageAPIError: # Re-raise custom exceptions to be handled by the exception handlers raise - except Exception as e: - # Log the error for debugging - print(f"Vector search error: {str(e)}") - - # Handle generic errors - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Error performing vector search: {str(e)}", - code="VECTOR_SEARCH_ERROR" - ) + except Exception: + return server_error_response( + "Error performing vector search.", + "VECTOR_SEARCH_ERROR", + log_context="vector_search_movies", ) """ @@ -480,13 +473,11 @@ async def get_distinct_genres(): # Use distinct() to get all unique values from the genres array field # MongoDB automatically flattens array fields when using distinct() genres = await movies_collection.distinct("genres") - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred.", + "DATABASE_ERROR", + log_context="get_distinct_genres", ) # Filter out null/empty values and sort alphabetically @@ -529,13 +520,11 @@ async def get_movie_by_id(id: str): movies_collection = get_collection("movies") try: movie = await movies_collection.find_one({"_id": object_id}) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred.", + "DATABASE_ERROR", + log_context="get_movie_by_id", ) @@ -619,13 +608,11 @@ async def get_all_movies( try: result = movies_collection.find(filter_dict).sort(sort).skip(skip).limit(limit) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"An error occurred while fetching movies. {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "An error occurred while fetching movies.", + "DATABASE_ERROR", + log_context="get_all_movies", ) movies = [] @@ -671,13 +658,11 @@ async def create_movie(movie: CreateMovieRequest): movies_collection = get_collection("movies") try: result = await movies_collection.insert_one(movie_data) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred.", + "DATABASE_ERROR", + log_context="create_movie_insert", ) # Verify that the document was created before querying it @@ -693,13 +678,11 @@ async def create_movie(movie: CreateMovieRequest): try: # Retrieve the created document to return complete data created_movie = await movies_collection.find_one({"_id": result.inserted_id}) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred.", + "DATABASE_ERROR", + log_context="create_movie_fetch", ) if created_movie is None: @@ -777,13 +760,11 @@ async def create_movies_batch(movies: List[CreateMovieRequest]) ->SuccessRespons }, f"Successfully created {len(result.inserted_ids)} movies." ) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred.", + "DATABASE_ERROR", + log_context="create_movies_batch", ) """ @@ -843,13 +824,11 @@ async def update_movie( {"_id": movie_id}, {"$set":update_dict} ) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"An error occurred while updating the movie: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "An error occurred while updating the movie.", + "DATABASE_ERROR", + log_context="update_movie", ) if result.matched_count == 0: @@ -920,13 +899,11 @@ async def update_movies_batch( try: result = await movies_collection.update_many(filter_data, {"$set": update_data}) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"An error occurred while updating movies: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "An error occurred while updating movies.", + "DATABASE_ERROR", + log_context="update_movies_batch", ) return create_success_response({ @@ -968,13 +945,11 @@ async def delete_movie_by_id(id: str): try: # Use deleteOne() to remove a single document result = await movies_collection.delete_one({"_id": object_id}) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred.", + "DATABASE_ERROR", + log_context="delete_movie_by_id", ) if result.deleted_count == 0: @@ -1043,13 +1018,11 @@ async def delete_movies_batch(request_body: dict = Body(...)) -> SuccessResponse try: result = await movies_collection.delete_many(filter_data) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"An error occurred while deleting movies: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "An error occurred while deleting movies.", + "DATABASE_ERROR", + log_context="delete_movies_batch", ) return create_success_response( @@ -1092,13 +1065,11 @@ async def find_and_delete_movie(id: str): # or ensure the document exists before deletion try: deleted_movie = await movies_collection.find_one_and_delete({"_id": object_id}) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred.", + "DATABASE_ERROR", + log_context="find_and_delete_movie", ) if deleted_movie is None: @@ -1251,13 +1222,11 @@ async def aggregate_movies_recent_commented( # Execute the aggregation try: results = await execute_aggregation(pipeline) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred during aggregation: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred during aggregation.", + "DATABASE_ERROR", + log_context="aggregate_movies_recent_commented", ) # Convert ObjectId to string for response @@ -1387,13 +1356,11 @@ async def aggregate_movies_by_year(): # Execute the aggregation try: results = await execute_aggregation(pipeline) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred during aggregation: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred during aggregation.", + "DATABASE_ERROR", + log_context="aggregate_movies_by_year", ) return create_success_response( @@ -1491,13 +1458,11 @@ async def aggregate_directors_most_movies( # Execute the aggregation try: results = await execute_aggregation(pipeline) - except Exception as e: - return JSONResponse( - status_code=500, - content=create_error_response( - message=f"Database error occurred during aggregation: {str(e)}", - code="DATABASE_ERROR" - ) + except Exception: + return server_error_response( + "Database error occurred during aggregation.", + "DATABASE_ERROR", + log_context="aggregate_directors_most_movies", ) return create_success_response( @@ -1584,22 +1549,22 @@ def get_embedding(data, input_type = "document", client=None): data, model = model, output_dimension = outputDimension, input_type = input_type ).embeddings return embeddings[0] - except voyage_error.AuthenticationError as e: + except voyage_error.AuthenticationError: # Handle authentication errors (401) from Voyage AI SDK raise VoyageAuthError("Invalid Voyage AI API key. Please check your VOYAGE_API_KEY in the .env file") - except voyage_error.InvalidRequestError as e: - # Handle invalid request errors (400) - often due to malformed API key - raise VoyageAPIError(f"Invalid request to Voyage AI API: {str(e)}", 400) - except voyage_error.RateLimitError as e: - # Handle rate limiting errors (429) - raise VoyageAPIError(f"Voyage AI API rate limit exceeded: {str(e)}", 429) - except voyage_error.ServiceUnavailableError as e: - # Handle service unavailable errors (502, 503, 504) - raise VoyageAPIError(f"Voyage AI service unavailable: {str(e)}", 503) + except voyage_error.InvalidRequestError: + logger.exception("Voyage AI invalid request") + raise VoyageAPIError("Invalid request to Voyage AI API.", 400) + except voyage_error.RateLimitError: + logger.exception("Voyage AI rate limit") + raise VoyageAPIError("Voyage AI API rate limit exceeded.", 429) + except voyage_error.ServiceUnavailableError: + logger.exception("Voyage AI service unavailable") + raise VoyageAPIError("Voyage AI service unavailable.", 503) except voyage_error.VoyageError as e: - # Handle any other Voyage AI SDK errors - raise VoyageAPIError(f"Voyage AI API error: {str(e)}", getattr(e, 'http_status', 500) or 500) - except Exception as e: - # Handle unexpected errors - raise VoyageAPIError(f"Failed to generate embedding: {str(e)}", 500) + logger.exception("Voyage AI API error") + raise VoyageAPIError("Voyage AI API error.", getattr(e, "http_status", 500) or 500) + except Exception: + logger.exception("Failed to generate embedding") + raise VoyageAPIError("Failed to generate embedding.", 500) diff --git a/mflix/server/python-fastapi/src/utils/errorResponse.py b/mflix/server/python-fastapi/src/utils/errorResponse.py index d82e144..30e4baa 100644 --- a/mflix/server/python-fastapi/src/utils/errorResponse.py +++ b/mflix/server/python-fastapi/src/utils/errorResponse.py @@ -8,6 +8,10 @@ from datetime import datetime, timezone from typing import Optional, Any +from fastapi.responses import JSONResponse + +from src.utils.logger import logger + def create_error_response( message: str, @@ -36,3 +40,21 @@ def create_error_response( "timestamp": datetime.now(timezone.utc).isoformat().replace('+00:00', 'Z') } + +def server_error_response( + message: str, + code: str, + *, + log_context: str, + status_code: int = 500, +) -> JSONResponse: + """ + Log the current exception and return a generic error payload (no stack traces). + Call only from an except block. + """ + logger.exception("%s failed", log_context) + return JSONResponse( + status_code=status_code, + content=create_error_response(message=message, code=code), + ) + From c58e42754821c1d17a7753e8bc7d081653b42eb7 Mon Sep 17 00:00:00 2001 From: Cory Bullinger Date: Fri, 5 Jun 2026 09:10:16 -0400 Subject: [PATCH 2/4] fix(python-fastapi): address Copilot review on PR #117 Use VoyageAPIError.status_code in the global handler and wrap cursor iteration in get_all_movies so database errors are logged consistently. Co-authored-by: Cursor --- mflix/server/python-fastapi/main.py | 11 +++++-- .../python-fastapi/src/routers/movies.py | 30 +++++++++---------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/mflix/server/python-fastapi/main.py b/mflix/server/python-fastapi/main.py index 87b6271..096f8b1 100644 --- a/mflix/server/python-fastapi/main.py +++ b/mflix/server/python-fastapi/main.py @@ -156,11 +156,16 @@ async def voyage_auth_error_handler(request: Request, exc: VoyageAuthError): @app.exception_handler(VoyageAPIError) async def voyage_api_error_handler(request: Request, exc: VoyageAPIError): - """Handle Voyage AI API errors with 503 status.""" + """Handle Voyage AI API errors using the exception's HTTP status code.""" + client_messages = { + 400: "Invalid vector search request.", + 429: "Vector search rate limit exceeded. Please try again later.", + 503: "Vector search service unavailable.", + } return JSONResponse( - status_code=503, + status_code=exc.status_code, content=create_error_response( - message="Vector search service unavailable", + message=client_messages.get(exc.status_code, "Vector search failed."), code="VOYAGE_API_ERROR", ) ) diff --git a/mflix/server/python-fastapi/src/routers/movies.py b/mflix/server/python-fastapi/src/routers/movies.py index b46e8cc..bb9d773 100644 --- a/mflix/server/python-fastapi/src/routers/movies.py +++ b/mflix/server/python-fastapi/src/routers/movies.py @@ -608,6 +608,21 @@ async def get_all_movies( try: result = movies_collection.find(filter_dict).sort(sort).skip(skip).limit(limit) + + movies = [] + + async for movie in result: + if "title" in movie: + movie["_id"] = str(movie["_id"]) # Convert ObjectId to string + # Ensure that the year field contains int value. + if "year" in movie and not isinstance(movie["year"], int): + cleaned_year = re.sub(r"\D", "", str(movie["year"])) + try: + movie["year"] = int(cleaned_year) if cleaned_year else None + except ValueError: + movie["year"] = None + + movies.append(movie) except Exception: return server_error_response( "An error occurred while fetching movies.", @@ -615,21 +630,6 @@ async def get_all_movies( log_context="get_all_movies", ) - movies = [] - - async for movie in result: - if "title" in movie: - movie["_id"] = str(movie["_id"]) # Convert ObjectId to string - # Ensure that the year field contains int value. - if "year" in movie and not isinstance(movie["year"], int): - cleaned_year = re.sub(r"\D", "", str(movie["year"])) - try: - movie["year"] = int(cleaned_year) if cleaned_year else None - except ValueError: - movie["year"] = None - - movies.append(movie) - # Return the results wrapped in a SuccessResponse message = f"Found {len(movies)} movies." return create_success_response(movies, message) From 1dd77ef83ed81aa0adee8590bbdb0fd9ea376496 Mon Sep 17 00:00:00 2001 From: Cory Bullinger Date: Fri, 5 Jun 2026 09:11:38 -0400 Subject: [PATCH 3/4] test(python-fastapi): cover get_all_movies cursor iteration errors Co-authored-by: Cursor --- .../python-fastapi/tests/test_movie_routes.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/mflix/server/python-fastapi/tests/test_movie_routes.py b/mflix/server/python-fastapi/tests/test_movie_routes.py index b9f12f8..e4bedc2 100644 --- a/mflix/server/python-fastapi/tests/test_movie_routes.py +++ b/mflix/server/python-fastapi/tests/test_movie_routes.py @@ -422,6 +422,27 @@ async def test_get_all_movies_database_error(self, mock_get_collection): assert body["success"] is False assert body["error"]["code"] == "DATABASE_ERROR" + @patch('src.routers.movies.get_collection') + async def test_get_all_movies_cursor_iteration_error(self, mock_get_collection): + """Should return error when cursor iteration fails.""" + mock_collection = MagicMock() + mock_cursor = MagicMock() + mock_cursor.sort.return_value = mock_cursor + mock_cursor.skip.return_value = mock_cursor + mock_cursor.limit.return_value = mock_cursor + mock_cursor.__aiter__.side_effect = Exception("Cursor iteration failed") + mock_collection.find.return_value = mock_cursor + mock_get_collection.return_value = mock_collection + + from src.routers.movies import get_all_movies + response = await get_all_movies() + + assert isinstance(response, JSONResponse) + assert response.status_code == 500 + body = json.loads(response.body.decode()) + assert body["success"] is False + assert body["error"]["code"] == "DATABASE_ERROR" + @pytest.mark.unit @pytest.mark.asyncio From 48c91c112cd16ecc990946b339836bfd666e3f79 Mon Sep 17 00:00:00 2001 From: Cory Bullinger Date: Mon, 8 Jun 2026 07:11:37 -0400 Subject: [PATCH 4/4] fix(python-fastapi): log Voyage AI authentication failures server-side Add logger.exception before raising VoyageAuthError so revoked or invalid API keys leave a trace in server logs. Addresses PR #117 review. Co-authored-by: Cursor --- mflix/server/python-fastapi/src/routers/movies.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mflix/server/python-fastapi/src/routers/movies.py b/mflix/server/python-fastapi/src/routers/movies.py index bb9d773..f745437 100644 --- a/mflix/server/python-fastapi/src/routers/movies.py +++ b/mflix/server/python-fastapi/src/routers/movies.py @@ -1551,6 +1551,7 @@ def get_embedding(data, input_type = "document", client=None): return embeddings[0] except voyage_error.AuthenticationError: # Handle authentication errors (401) from Voyage AI SDK + logger.exception("Voyage AI authentication failed") raise VoyageAuthError("Invalid Voyage AI API key. Please check your VOYAGE_API_KEY in the .env file") except voyage_error.InvalidRequestError: logger.exception("Voyage AI invalid request")