Skip to content

ref(explore): Narrow request.user in saved queries and dashboards - #129415

Merged
gricha merged 1 commit into
masterfrom
ref/sa-narrow-explore
Nov 3, 2026
Merged

gricha merged 1 commit into
masterfrom
ref/sa-narrow-explore

Conversation

@gricha

@gricha gricha commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

Explore and Discover saved queries, the homepage query, stars and dashboards read the caller through sentry.auth.principal, following the issue-views pattern from #129401: shared rows for everyone, personal state (stars, last visited, "created by me") only for a user.

  • A service account can list saved queries and Discover's myqueries (shared rows, nothing starred), the homepage GET returns 204, visits count without a last-visited row, and a query created with starred is created unstarred. Stars, favorites and homepage PUT are 403 user-required. Lists and creates skip personal state up front instead of reporting a caught UserRequired to Sentry on every call. New tests cover each; they fail on the old code.
  • Token callers: Discover sortBy=myqueries no longer sorts creator-less queries first, and Explore's visit endpoint returns 204 instead of a 500 (NOT NULL write): the visit counts, with no last-visited row, as Discover and dashboard visits already did.
  • Users and agents see no change.

Left: organization_dashboards.py calls has_edit_permissions(request.user.id), which needs to take the caller; and DashboardListSerializer still reads user.id, so a service account listing a non-empty set of dashboards still gets 403. Both are in the serializer follow-up.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 31, 2026
@gricha
gricha added this pull request to stack #129364 October 31, 2026 02:27
@gricha
gricha removed this pull request from stack #129364 November 1, 2026 04:24
@gricha
gricha force-pushed the ref/sa-narrow-explore branch from 6d91bbc to 51071bb Compare November 1, 2026 04:24
@gricha
gricha added this pull request to stack #129440 November 1, 2026 04:24
@gricha
gricha force-pushed the ref/sa-narrow-explore branch from 51071bb to e09a478 Compare November 1, 2026 04:57
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

decision-model-linter

api

src/sentry/discover/endpoints/discover_saved_queries.py

  • warn readsRequestBodyOutsideSerializer = TRUE (0.90): Request body values should be read from a serializer's validated_data. This file reads a value directly from request.data or request.POST.

src/sentry/explore/endpoints/explore_saved_queries.py

  • warn readsRequestBodyOutsideSerializer = TRUE (0.94): Request body values should be read from a serializer's validated_data. This file reads a value directly from request.data or request.POST.

decision-model-linter: 36 calls on 12 files, 80,211 input tokens, $0.0034, model typesafe/jev-1.13-20260917

@gricha
gricha force-pushed the ref/sa-narrow-explore branch 2 times, most recently from fd663da to c2ba565 Compare November 2, 2026 20:14
@gricha
gricha marked this pull request as ready for review November 2, 2026 20:22
@gricha
gricha requested review from a team as code owners November 2, 2026 20:22
@gricha
gricha requested review from azulus and skaasten November 2, 2026 20:25

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c2ba565. Configure here.

@gricha
gricha removed this pull request from stack #129440 November 2, 2026 21:45
@gricha
gricha added this pull request to stack #129572 November 2, 2026 21:45
@gricha
gricha force-pushed the ref/sa-narrow-explore branch 2 times, most recently from 35ea666 to c8c5fd6 Compare November 2, 2026 22:36
@gricha
gricha force-pushed the ref/sa-narrow-explore branch from c8c5fd6 to d9bc2ae Compare November 2, 2026 23:08
@sentry

sentry Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Sentry Snapshot Testing

Name Added Removed Changed Renamed Unchanged Skipped Status
sentry-frontend
sentry-frontend
0 0 0 0 780 0 ✅ Unchanged

⚙️ sentry-frontend Snapshot Settings

@gricha
gricha force-pushed the ref/sa-narrow-explore branch from d9bc2ae to 528918a Compare November 2, 2026 23:18
@gricha
gricha force-pushed the ref/sa-narrow-explore branch from 528918a to 20d28dc Compare November 2, 2026 23:31
@gricha
gricha requested a review from cvxluo November 2, 2026 23:34
Base automatically changed from ref/sa-narrow-api-a to master November 3, 2026 00:27
@gricha
gricha requested review from a team as code owners November 3, 2026 00:27
@gricha
gricha force-pushed the ref/sa-narrow-explore branch from 20d28dc to 4e4ca14 Compare November 3, 2026 00:27
@gricha
gricha removed this pull request from stack #129572 November 3, 2026 06:52
@gricha
gricha added this pull request to stack #129650 November 3, 2026 06:52

@cvxluo cvxluo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i saw #129422 first and left this comment: #129422 (review)

otherwise everything seems fine to me

@gricha
gricha force-pushed the ref/sa-narrow-explore branch from 4e4ca14 to 0b9fd59 Compare November 3, 2026 19:53
@gricha
gricha removed this pull request from stack #129650 November 3, 2026 19:53
@gricha
gricha added this pull request to stack #129747 November 3, 2026 19:54
@gricha
gricha force-pushed the ref/sa-narrow-explore branch from 0b9fd59 to f027c98 Compare November 3, 2026 20:09
Explore and Discover saved queries, the homepage query, stars and dashboards read the caller through `sentry.auth.principal`, following the issue-views pattern from #129401: shared rows for everyone, personal state (stars, last visited, "created by me") only for a user.

- A service account can list saved queries and Discover's `myqueries` (shared rows, nothing starred), the homepage GET returns 204, visits count without a last-visited row, and a query created with `starred` is created unstarred. Stars, favorites and homepage PUT are 403 `user-required`. Lists and creates skip personal state up front instead of reporting a caught `UserRequired` to Sentry on every call. New tests cover each; they fail on the old code.
- Token callers: Discover `sortBy=myqueries` no longer sorts creator-less queries first, and Explore's visit endpoint returns 204 instead of a 500 (NOT NULL write): the visit counts, with no last-visited row, as Discover and dashboard visits already did.
- Users and agents see no change.

Left: `organization_dashboards.py` calls `has_edit_permissions(request.user.id)`, which needs to take the caller; and `DashboardListSerializer` still reads `user.id`, so a service account listing a non-empty set of dashboards still gets 403. Both are in the serializer follow-up.
@gricha
gricha force-pushed the ref/sa-narrow-explore branch from f027c98 to 1ee0740 Compare November 3, 2026 20:51
@gricha
gricha merged commit 81b50dd into master Nov 3, 2026
67 checks passed
@gricha
gricha deleted the ref/sa-narrow-explore branch November 3, 2026 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants