security: prevent exception details from leaking in API error responses#1816
security: prevent exception details from leaking in API error responses#1816Vchen7629 wants to merge 3 commits into
Conversation
…n the JSONResponse
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR hardens error handling in four FastAPI endpoints in ChangesError handling and error message redaction
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/settings/endpoints.py`:
- Around line 785-789: The current exception handler that logs via
logger.exception(...) and returns JSONResponse({"error": "Failed to update
settings"}, status_code=500) swallows raised HTTPException instances; modify the
exception handling in the update-settings route (the block that calls
TelemetryClient.send_event and returns the 500 JSONResponse) to add an explicit
"except HTTPException: raise" before the generic "except Exception" so any
previously raised HTTPException (e.g., 403/422) is re-raised and preserved; keep
the existing logger.exception, TelemetryClient.send_event, and JSONResponse
behavior only in the generic exception branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b340f8ca-1878-4fa5-ba3b-94d2881c9ce2
📒 Files selected for processing (1)
src/api/settings/endpoints.py
|
While fixing the flagged lines, I noticed 5 additional instances of the same pattern in this file (lines 1200, 1283, 1319, 1625, 1663). Would you like me to include those in this PR, or handle them separately to keep the diff small? |
Summary
Summary by CodeRabbit