Skip to content

🛡️ Sentinel: [HIGH] Fix Information Exposure through Exceptions #123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daggerstuff wants to merge 1 commit into
base: staging
Choose a base branch
from
+4 −3
Open

🛡️ Sentinel: [HIGH] Fix Information Exposure through Exceptions #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .Jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
## 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.

Copilot uses AI. Check for mistakes.
6 changes: 3 additions & 3 deletions api/pixel_inference_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ async def infer(request: PixelInferenceRequest, background_tasks: BackgroundTask
return await inference_engine.generate_response(request)
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")
Comment on lines 407 to +409
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.


@app.post("/batch-infer")
Expand All @@ -422,7 +422,7 @@ async def batch_infer(requests: list[PixelInferenceRequest]):
responses.append(response)
except Exception as e:
logger.error(f"Batch inference error: {e}")
responses.append({"error": str(e)})
responses.append({"error": "Internal server error"})
Comment on lines 423 to +425
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

return {"results": responses}

Expand All @@ -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}")
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
logger.error(f"Reload error: {e}")
logger.exception(f"Reload error: {e}")

Copilot uses AI. Check for mistakes.
raise HTTPException(status_code=500, detail=str(e)) from e
raise HTTPException(status_code=500, detail="Internal server error")


if __name__ == "__main__":
Expand Down
Loading