feat(trace): Add debug param to trace item details endpoint#116151
Conversation
|
@cursor review |
|
@sentry review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1b11b23. Configure here.
| return Response(serializer.errors, status=400) | ||
|
|
||
| serialized = serializer.validated_data | ||
| debug = request.user.is_superuser and request.GET.get("debug", False) |
There was a problem hiding this comment.
Bug: The superuser check uses request.user.is_superuser instead of the more secure is_active_superuser(request), bypassing active session validation for accessing debug info.
Severity: MEDIUM
Suggested Fix
Replace the request.user.is_superuser check with a call to is_active_superuser(request). This ensures that access to debug information is consistently protected by requiring an active and validated superuser session.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/api/endpoints/project_trace_item_details.py#L366
Potential issue: The debug flag is enabled by checking `request.user.is_superuser`,
which only verifies a database flag. This check is insufficient as it does not validate
if the superuser has an active, elevated session. A more secure function,
`is_active_superuser(request)`, is used elsewhere in the same file for similar checks
and validates session cookies, expiry, and IP restrictions. By using the weaker check, a
user whose superuser session has expired or is otherwise invalid can still access
sensitive `debug_info` containing raw RPC request and response data.
Did we get this right? 👍 / 👎 to inform future reviews.
In general, but especially during our span streaming rollout, it's useful for debugging to be able to see the underlying data for a particular span before it is transformed by the Sentry API and frontend. This PR adds a superuser-only
debugflag onProjectTraceItemDetailsEndpointthat adds the raw request and result from EAP'sTraceItemDetailsRPC into the API response'smetadictionary.Follows the established pattern from
OrganizationEventsEndpointBase:sentry/src/sentry/api/bases/organization_events.py
Line 208 in 49fb050
sentry/src/sentry/api/bases/organization_events.py
Lines 446 to 448 in 49fb050
This will be hooked up to a superuser-only button on the span details pane in the trace waterfall, similar to how we have the link to see raw transaction JSON for transactions (except limited to superusers) - see #116131.
This PR replaces the approach in #116127.
🤖: Claude Code (Opus 4.6) used to generate the code, with human editing + review. All words my own.