fix: prevent exception details from leaking in API error responses#2007
fix: prevent exception details from leaking in API error responses#2007Vchen7629 wants to merge 4 commits into
Conversation
Signed-off-by: vchen7629 <[email protected]>
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
|
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)
WalkthroughFour settings/onboarding exception handlers now log tracebacks server-side and return generic 500 JSON error messages instead of exposing raw exception text. ChangesException message redaction
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/settings/endpoints.py (1)
1000-1005: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSame CodeQL exception-leak pattern remains unfixed in adjacent
onboardinghandlers.Within this same
onboardingfunction, two otherexceptblocks still returnstr(e)directly to the client: the provider-validation failure at line 1003 (JSONResponse({"error": str(e)}, status_code=400)) and the outer catch-all at lines 1259-1263 (logger.error(..., error=str(e))plusJSONResponse({"error": str(e)}, status_code=500)). These weren't in the four CodeQL-flagged lines this PR targets, but they leak the same class of internal exception details (e.g. provider auth errors) to clients.Also applies to: 1258-1264
🤖 Prompt for 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. In `@src/api/settings/endpoints.py` around lines 1000 - 1005, The onboarding handler still leaks raw exception text in adjacent exception paths; update the provider-validation and outer catch-all branches in onboarding to avoid returning or logging str(e) directly to clients. Use the same error-handling pattern as the fixed CodeQL spots by replacing the JSONResponse({"error": str(e)}) and logger.error(..., error=str(e)) usage with a generic client-facing message while preserving the detailed exception only in internal logs, and locate the affected code in the onboarding function’s except blocks.
🤖 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 1129-1133: The OpenSearch onboarding failure path in the exception
handler is returning a default HTTP 200 because the JSONResponse in the
onboarding initialization block omits an explicit status code. Update the except
Exception branch around the OpenSearch initialization logic to return the same
error payload from JSONResponse with status_code=500 so the client sees a real
server error; use the existing logger.exception message and the JSONResponse
call in that handler as the place to fix it.
---
Outside diff comments:
In `@src/api/settings/endpoints.py`:
- Around line 1000-1005: The onboarding handler still leaks raw exception text
in adjacent exception paths; update the provider-validation and outer catch-all
branches in onboarding to avoid returning or logging str(e) directly to clients.
Use the same error-handling pattern as the fixed CodeQL spots by replacing the
JSONResponse({"error": str(e)}) and logger.error(..., error=str(e)) usage with a
generic client-facing message while preserving the detailed exception only in
internal logs, and locate the affected code in the onboarding function’s except
blocks.
🪄 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: a301591e-49a2-4709-a07f-3bc6cdc80932
📒 Files selected for processing (1)
src/api/settings/endpoints.py
Signed-off-by: vchen7629 <[email protected]>
Summary
Changes
Summary by CodeRabbit