🛡️ Sentinel: [MEDIUM] Add global exception handler to prevent stack trace leaks#81
🛡️ Sentinel: [MEDIUM] Add global exception handler to prevent stack trace leaks#81madara88645 wants to merge 2 commits intomainfrom
Conversation
…race leaks - Add a catch-all exception handler in serve.py for unhandled exceptions. - Ensure generic 500 Internal server error response is returned instead of stack traces. - Logs the traceback internally using the logging module.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…race leaks - Add a catch-all exception handler in serve.py for unhandled exceptions. - Ensure generic 500 Internal server error response is returned instead of stack traces. - Logs the traceback internally using the logging module. - Formatted `serve.py` using ruff format to satisfy CI requirements.
There was a problem hiding this comment.
Pull request overview
Adds a global FastAPI exception handler in serve.py to prevent unhandled exceptions from leaking stack traces/internal details to clients, while logging full tracebacks server-side.
Changes:
- Register a catch-all
@app.exception_handler(Exception)handler. - Log unhandled exceptions (with traceback) and return a generic 500 JSON response.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @app.exception_handler(Exception) | ||
| async def global_exception_handler(request: Request, exc: Exception): | ||
| """ | ||
| Catch-all exception handler to prevent leaking stack traces and internals | ||
| to the client. Logs the actual error and returns a generic 500 JSON response. | ||
| """ | ||
| logger.error( | ||
| f"Unhandled exception during {request.method} {request.url.path}: {exc}", | ||
| exc_info=True, | ||
| ) |
There was a problem hiding this comment.
Add automated test coverage for the new global exception handler (e.g., using TestClient(..., raise_server_exceptions=False) and forcing an endpoint to raise an unhandled Exception) to assert the response is a generic 500 with no traceback content. This prevents regressions where debug/stack traces could leak again.
| """ | ||
| Catch-all exception handler to prevent leaking stack traces and internals | ||
| to the client. Logs the actual error and returns a generic 500 JSON response. | ||
| """ |
There was a problem hiding this comment.
Use lazy logging / exception helpers here to avoid eagerly formatting the f-string and to standardize traceback logging (e.g., prefer logger.exception(...) or logger.error("... %s ...", exc, exc_info=True)). This keeps logging efficient when the error level is filtered and reduces chances of log injection via raw path formatting.
| """ | |
| logger.exception("Unhandled exception during %s %s", request.method, request.url.path) |
🚨 Severity: MEDIUM
💡 Vulnerability: Unhandled exceptions in the FastAPI application could potentially leak stack traces or internal implementation details to the client in certain environments.
🎯 Impact: An attacker could use stack trace information to understand the internal structure, dependencies, or paths of the server, facilitating further targeted attacks.
🔧 Fix: Implemented a global exception handler in
serve.pythat intercepts all unhandledExceptioninstances, securely logs the traceback internally using standard librarylogging, and returns a generic 500 "Internal server error" JSON response to the client.✅ Verification: Verified via manual test utilizing
raise_server_exceptions=FalseonTestClientthat simulated exceptions are caught and hidden from the response body. Full backend unit tests suite passing.PR created automatically by Jules for task 11055907245249008918 started by @madara88645