🛡️ Sentinel: [HIGH] Fix Information Exposure through Exceptions#123
🛡️ Sentinel: [HIGH] Fix Information Exposure through Exceptions#123daggerstuff wants to merge 1 commit intostagingfrom
Conversation
Co-authored-by: daggerstuff <261005129+daggerstuff@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 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. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSanitizes error responses for inference-related endpoints to prevent leaking internal exception details while preserving server-side logging, and documents the security pattern in the Sentinel playbook. Sequence diagram for sanitized inference error handlingsequenceDiagram
actor User
participant FastAPIApp
participant PixelInferenceService
participant InferenceEngine
participant Logger
User->>FastAPIApp: POST /infer
FastAPIApp->>PixelInferenceService: infer(request)
PixelInferenceService->>InferenceEngine: generate_response(request)
InferenceEngine-->>PixelInferenceService: Exception e
PixelInferenceService->>Logger: error(Inference error: e)
PixelInferenceService-->>FastAPIApp: raise HTTPException(500, Internal server error)
FastAPIApp-->>User: 500 Internal server error (generic message)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider centralizing the generic error message (e.g., a constant or helper) so that future changes to the wording or localization only need to be made in one place.
- For the
/batch-inferendpoint, you might want to include a simple machine-readable error code alongside the generic message to help clients distinguish between different failure modes without exposing internal details.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider centralizing the generic error message (e.g., a constant or helper) so that future changes to the wording or localization only need to be made in one place.
- For the `/batch-infer` endpoint, you might want to include a simple machine-readable error code alongside the generic message to help clients distinguish between different failure modes without exposing internal details.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR addresses information exposure risks by sanitizing error details returned from several FastAPI endpoints, ensuring internal exception strings aren’t surfaced to API clients.
Changes:
- Replaced user-facing
detail=str(e)/{"error": str(e)}with a generic"Internal server error". - Removed explicit exception chaining (
from e) in raisedHTTPExceptions. - Added a Sentinel learning entry documenting the pattern.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| api/pixel_inference_service.py | Sanitizes error responses for /infer, /batch-infer, and /reload-model to avoid leaking internal exception details. |
| .Jules/sentinel.md | Documents the “mask exception details” guidance for future prevention. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except Exception as e: | ||
| logger.error(f"Inference error: {e}") | ||
| raise HTTPException(status_code=500, detail=str(e)) from e | ||
| raise HTTPException(status_code=500, detail="Internal server error") |
There was a problem hiding this comment.
logger.error(f\"... {e}\") logs only the exception message and drops the stack trace, which makes production debugging (and incident response) significantly harder. Prefer logging with traceback (e.g., logger.exception(...) inside an except, or logger.error(..., exc_info=True)), while still returning the sanitized HTTPException detail to the client.
| except Exception as e: | ||
| logger.error(f"Batch inference error: {e}") | ||
| responses.append({"error": str(e)}) | ||
| responses.append({"error": "Internal server error"}) |
There was a problem hiding this comment.
Same logging issue here: the server log will not include a traceback, only str(e). Switching to a traceback-inclusive log (logger.exception(...) / exc_info=True) will preserve diagnostic detail server-side without reintroducing information exposure to clients.
| @@ -437,7 +437,7 @@ async def reload_model(): | |||
| raise HTTPException(status_code=500, detail="Failed to reload model") | |||
| except Exception as e: | |||
| logger.error(f"Reload error: {e}") | |||
There was a problem hiding this comment.
Consider logging the full traceback for reload failures as well. Model reload issues can be operationally critical, and having the stack trace in logs (without exposing it to clients) improves reliability and reduces time-to-mitigate.
| logger.error(f"Reload error: {e}") | |
| logger.exception(f"Reload error: {e}") |
| @@ -1,2 +1,3 @@ | |||
| ## 2026-03-27 - Prevent SQL Error Leakage | Vulnerability: Information Exposure through Error Messages | Learning: Database error traces bubbling up to users can leak schema and path information. | Prevention: Catch database exceptions and return generic error messages (e.g. 500 Internal Server Error) to the client while logging the detailed exception server-side. | |||
| ## 2026-03-29 - Server-Side Logging of Database Errors | Vulnerability: Insufficient Logging and Monitoring | Learning: Swallowing database errors without logging them hides potential malicious activity like SQL injection attempts. | Prevention: Always log database exception details using `logger.error` on the backend before returning a sanitized generic error to the client. | |||
| ## 2026-03-31 - Mask API Exception Details | Vulnerability: Information Exposure through Exceptions | Learning: Passing raw exception objects (`str(e)`) directly to FastAPI `HTTPException` detail fields leaks internal stack traces and error states to end-users. | Prevention: Always log exception objects server-side using `logger.error()` and raise HTTPExceptions with generic error details like 'Internal server error', removing the `from e` exception chaining clause. | |||
There was a problem hiding this comment.
This guidance claims removing from e prevents internal details from bubbling up, but raise HTTPException(...) inside an except still implicitly retains exception context (as __context__) unless explicitly suppressed (e.g., from None). If the intent is purely to avoid leaking details to clients, the key control is the sanitized detail=...; if the intent is to suppress chaining in tracebacks, the doc should mention from None (or clarify that chaining affects tracebacks/logs, not the HTTP response body).
| ## 2026-03-31 - Mask API Exception Details | Vulnerability: Information Exposure through Exceptions | Learning: Passing raw exception objects (`str(e)`) directly to FastAPI `HTTPException` detail fields leaks internal stack traces and error states to end-users. | Prevention: Always log exception objects server-side using `logger.error()` and raise HTTPExceptions with generic error details like 'Internal server error', removing the `from e` exception chaining clause. | |
| ## 2026-03-31 - Mask API Exception Details | Vulnerability: Information Exposure through Exceptions | Learning: Passing raw exception objects (`str(e)`) directly to FastAPI `HTTPException` detail fields leaks internal stack traces and error states to end-users. | Prevention: Always log exception objects server-side using `logger.error()` and raise `HTTPException` with a generic, sanitized `detail` (for example, 'Internal server error'). If you also want to suppress exception chaining in tracebacks, raise `HTTPException(...) from None`; otherwise, the original exception is only kept in server-side traceback context, not in the HTTP response body. |
🚨 Severity: HIGH
💡 Vulnerability: Information Exposure. The
/infer,/batch-infer, and/reload-modelendpoints inapi/pixel_inference_service.pywere catching exceptions and passing the raw string representation (str(e)) to the FastAPIHTTPExceptiondetail field, potentially leaking stack traces, system paths, and internal error states to end users.🔧 Fix: Replaced
detail=str(e)with a genericdetail="Internal server error"and removed thefrom eexception chaining clause to prevent internal details from bubbling up. The original detailed errors are still logged server-side vialogger.error().✅ Verification: Review the modified exception handlers in
api/pixel_inference_service.pyand verifypython3 -c 'import ast; ast.parse(open("api/pixel_inference_service.py").read())'passes without syntax errors.PR created automatically by Jules for task 6697975960274588231 started by @daggerstuff
Summary by Sourcery
Sanitize error responses in inference-related API endpoints to avoid leaking internal exception details to clients.
Bug Fixes:
Documentation:
Summary by cubic
Sanitized inference API error responses to prevent leaking stack traces or internal paths. Replaced raw exception details with "Internal server error", removed exception chaining, and logged full errors server-side for
/infer,/batch-infer, and/reload-modelinapi/pixel_inference_service.py.Written for commit b8d36e3. Summary will update on new commits.