[PoC] Code Review in Sentry CLI #106434
[PoC] Code Review in Sentry CLI #106434
Conversation
…re flags Remove tests, feature flag logic, and rate limiting to keep only the core PoC functionality for the local code review feature. - Delete test files for code_review_local - Remove feature flag check and rate limiting from endpoint - Remove organizations:code-review-local feature flag registration - Remove CODE_REVIEW_LOCAL_ENABLED and rate limit settings from config
github-actions
bot
added
the
Scope: Backend
…string Instead of requiring separate owner, name, and provider fields, the serializer now accepts repository name in "owner/repo" format. The provider defaults to "github" for this PoC.
github-actions
bot
added
the
Scope: Frontend
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # Include the error message from Seer if available | ||
| error_msg = str(e) | ||
| if "Seer error" in error_msg: | ||
| return Response({"detail": error_msg}, status=502) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 6 hours ago
In general, to fix information exposure through exceptions, do not return raw exception messages or stack traces to clients. Instead, log detailed errors on the server side and send only generic, user-friendly error messages (optionally with a stable error code) to the client.
Here, the minimal, behavior-preserving fix is to stop returning error_msg directly in the HTTP response while still logging it. The existing logger.exception already logs the full error including str(e), so developers retain full diagnostics. We should replace the branch:
if "Seer error" in error_msg:
return Response({"detail": error_msg}, status=502)with a generic message that does not expose error_msg, e.g.:
if "Seer error" in error_msg:
return Response({"detail": "Upstream code review service returned an error"}, status=502)or similar. This preserves the status code and the control flow (the condition still distinguishes the “Seer error” case), but removes user exposure to the raw exception text.
Concretely, in src/sentry/seer/code_review/endpoints/code_review_local.py, within the except ValueError as e: block (lines 127–140), change only the if "Seer error" in error_msg: return statement to use a generic message, leaving logging and the fallback 502 response intact. No new imports or helper functions are required.
| @@ -133,10 +133,13 @@ | ||
| "error": str(e), | ||
| }, | ||
| ) | ||
| # Include the error message from Seer if available | ||
| # Include a specific but non-sensitive error message when Seer reports an error | ||
| error_msg = str(e) | ||
| if "Seer error" in error_msg: | ||
| return Response({"detail": error_msg}, status=502) | ||
| return Response( | ||
| {"detail": "Code review service reported an upstream error"}, | ||
| status=502, | ||
| ) | ||
| return Response({"detail": "Failed to start code review analysis"}, status=502) | ||
| except Exception as e: | ||
| # Catch-all for unexpected errors |
szokeasaurusrex
changed the title
Caution
This is an internal PoC, not intended to be merged to
master!This is a PR on top of #106360, with two distinct changes, in separate commits.
The second commit (9dfde14) contains some changes to how the request is serialized. This is necessary for compatibility with the Sentry CLI PR getsentry/sentry-cli#3084.
The first commit (d2eb27c) deletes everything that is not necessary for the PoC (feature flags, rate limiting, tests). We may or may not want to merge that during the PoC.