fix(api): tighten published mutation scopes#116451
Draft
sentry-junior[bot] wants to merge 26 commits into
Draft
Conversation
Add a guardrail for published mutation endpoints that still accept readonly scopes. Previously, write methods could keep readonly scopes in scope_map without any explicit marker in code, which made the policy debt hard to audit and easy to expand accidentally. Require those endpoints to carry a readonly_mutation_scope_exceptions note, and fail the invariant test when a published mutation endpoint accepts readonly scopes without that note. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Tighten published mutation endpoints that already have an obvious existing write-capable scope. Previously, several write methods accepted readonly scopes like org:read, project:read, or event:read even though they mutate server-side state. Require the existing write scope for those surfaces instead. Keep session behavior intact, including team-scoped alert access, but stop exposing readonly token writes through these endpoints. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Drop the member-session surfaces that were not actually uncontroversial from this PR. Dashboards, code mappings, repo path parsing, key transactions, and org auth token updates still rely on broader session behavior today, so tightening them here broke existing member flows. Revert those paths in this branch and keep the safe mutation tightening for incidents, alerts, user issue creation, data export, and replay summary. Also keep replay delete in the project scope domain so delete remains consistent with the rest of the replay permission class. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Add an explicit owner for static/app/components/markdownTextArea.tsx so the branch clears the CODEOWNERS coverage check. The file was missing ownership coverage entirely, which caused the fresh PR run to fail even though the API changes on this branch are backend-only. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Keep the replay permission maps aligned with the current repo style of listing implied scopes explicitly. This does not change the effective permissions on these endpoints, but it removes a local inconsistency in this PR where replay endpoints relied on hierarchy expansion while nearby scope maps did not. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Replay delete is a public mutation, so it should not depend on a project read-style contract. Move the DELETE permission to event write/admin so member sessions keep working while token auth requires a real write scope. Add direct endpoint tests covering the token contract: event:read is denied and event:write is allowed for replay delete. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Remove the incident permission change from the existing-write-scope pass. Incident mutation has an older member-access contract tied to project access, so tightening it cleanly needs a dedicated scope decision instead of being folded into the uncontroversial cleanup. Also drop an unrelated CODEOWNERS hunk so this branch stays focused on backend mutation scope changes. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Keep replay summary POST on read-level replay scopes and mark it as an explicit readonly-mutation exception, since it derives summary data from existing replay/event data. Document the preview endpoints that intentionally keep write-aligned permissions because they are part of alert and monitor authoring flows, even though they use POST helper endpoints. Add replay summary token tests covering the read-scope contract. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Add endpoint-level regression tests for the scope changes on this branch so each permission update is covered directly. Cover the write-scope transitions for data export, alert rule creation, project user issues, replay delete, detector updates, and the alert helper endpoints. This keeps the branch from relying on indirect role coverage alone and makes the replay summary readonly exception explicit next to direct token tests. Refs getsentry/getsentry#19897 Co-Authored-By: OpenAI Codex <noreply@openai.com>
Use supported token auth for data export, give alert-rule token tests a valid member project setup, and return a real anomaly payload in the Seer regression test. These tests were asserting the new permission contracts, but two of them were exercising invalid success paths and one was using an auth shape that the endpoint does not serialize correctly. Co-Authored-By: Codex <noreply@openai.com>
Alert and monitor mutations now accept alerts:write at the endpoint level, but the nested project validators still required project:read. That let the request through permission checks and then failed token-based create and update flows with a 400 during validation. Allow those serializers to validate project selection against the same write-capable scopes the endpoints already advertise, and add direct token regression tests for alert rule updates and monitor creation so the contract stays enforced. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Limit org-scoped alert and detector mutations to either the target project's alerts:write access or an explicit endpoint opt-in for the older team-scoped fallback. This closes the cross-team mutation path on alert rule and detector details, and adds project-scoped checks for the other shared consumers that still relied on the broad organization permission. Align replay deletion with the rest of the event delete surface by requiring event:admin instead of event:write. Add regression coverage for the scope changes and the cross-project authorization cases. Co-Authored-By: Codex <noreply@openai.com>
Normalize RpcUserOrganizationContext to the underlying organization before resolving target projects for alert-rule and detector mutation permissions. Without that, the new project-scoped alerts:write path breaks during organization object-permission checks because those hooks run before convert_args swaps in the concrete organization. Add team-admin session regressions that force the project-scoped alerts:write authorization path for alert rule and detector updates and deletes, including the workflow-engine fake-detector alert-rule route. Co-Authored-By: Codex <noreply@openai.com>
Annotate the published mutation endpoints that intentionally accept readonly or member scopes so the permission-note audit can distinguish expected behavior from real scope gaps. Tighten the newer alert, detector, workflow, uptime, and anomaly helpers to use alerts:write, but keep temporary org:write compatibility on the deprecated metric alert and cron monitor mutation paths. Add regression coverage for both the temporary compat and the stricter deny cases. Refs getsentry/getsentry#19897 Co-Authored-By: Codex <codex@openai.com>
Unwrap RPC organization context before resolving anomaly preview projects and stop swallowing PermissionDenied from that lookup. The earlier helper could hide project-resolution failures and fall back to the broader any-team alerts:write check. Keep invalid project_id parsing local, but preserve the existing permission path for real lookup errors and add regression coverage for both cases. Refs getsentry/getsentry#19897 Co-Authored-By: Codex <codex@openai.com>
Centralize the RPC-aware organization id helper used by alert-rule and detector detail permission lookups. Detector detail mutations also need the same missing-object fallback as alert rules so project-scoped alerts:write callers still reach convert_args() and return 404 for stale detector ids instead of failing permission checks early. Refs getsentry/getsentry#19897 Co-Authored-By: Codex <codex@openai.com>
Remove duplicated handler-level permission checks and share the legacy alert project scope tuple between the alert-rule and monitor validators. Keep anomaly preview project validation and the original system-detector edit contract intact, add the missing detector-index delete regression, and annotate the prompts activity readonly mutation note that the permission audit now enforces. Refs getsentry/getsentry#19897 Co-Authored-By: Codex <codex@openai.com>
Restore legacy write-scope compatibility for published and experimental API mutations that this branch tightened, and add regression coverage for the token and session-role paths those endpoints rely on. Preserve current client behavior while keeping the temporary compat shims clearly marked for later removal once callers have migrated. Co-Authored-By: Codex <noreply@openai.com>
Keep the write-scope compatibility follow-up clean under our backend type checker. Annotate the workflow permission override to match the base permission contract and align the workflow API-key test helpers with the DRF client stubs so the touched endpoint coverage stays type-safe. Co-Authored-By: Codex <codex@openai.com>
Add the missing readonly mutation scope notes for the project user issue and replay delete endpoints so the published-mutation permission audit reflects their existing behavior. While touching the alert permission code, extract the shared alert-mutation permission logic used by alert rules and detectors to keep future scope changes from drifting between the two classes. Co-Authored-By: Codex <codex@openai.com>
Rename the shared alerting mutation permission to reflect the broader alerting surface and reuse it for workflow endpoints. This removes the remaining duplicated workflow permission logic without changing behavior, since workflow endpoints do not define the extra project or team fallbacks handled by the shared implementation. Co-Authored-By: Codex <codex@openai.com>
Restore the legacy org:* compatibility scopes for workflow detail mutations while keeping the shared alerting permission base in place. Without this, API keys that still rely on org:write or org:admin could create or bulk update workflows through the index endpoint but would get 403 on detail PUT and DELETE. Add detail endpoint regression tests for both legacy and alerts:write API key auth paths. Co-Authored-By: Codex <codex@openai.com>
Keep the detector details endpoint lint-clean after the rebase conflict resolution. Ruff removed the duplicate SnubaQuery import, so capture that hook-driven cleanup as a standalone follow-up instead of leaving the branch dirty. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Move alert authoring compatibility away from readonly scopes and align related preview, workflow, detector, and monitor endpoints on alerts:write. Keep only explicit legacy write/admin compatibility where needed. Require event write/admin for user-created issues and event admin for replay deletion so event read and project read scopes no longer authorize these published mutations. Refs getsentry/getsentry#19897 Co-Authored-By: OpenAI Codex <codex@openai.com>
Add a grepable TODO to personal preference and member-state mutation exceptions that still use readonly org or member scopes. These remain compatibility notes for this PR, but should move to a narrow personal write scope once the API scope taxonomy supports one. Refs getsentry/getsentry#19897 Co-Authored-By: OpenAI Codex <codex@openai.com>
…cope System-created detectors (error, performance, issue-stream types) should only be editable by users with org:write, not members with alerts:write. Separating SYSTEM_CREATED_DETECTOR_EDIT_SCOPES from USER_CREATED_DETECTOR_EDIT_SCOPES makes the existing branching in can_edit_detectors meaningful. Fixes test_update_detectors_member_permission_denied_for_non_user_created_detector and test_update_detectors_mixed_permissions. Co-authored-by: Greg Pstrucha <greg.pstrucha@sentry.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tighten published mutation scope enforcement and annotate the remaining published mutations that intentionally still accept readonly or member scopes.
This supersedes #113120 and #113194. The old stacked PR and the clean rebuild had diverged; this PR carries the validated branch state directly against
master, including the compatibility shims, the follow-up 404 fixes, and the published-mutation audit notes.Behavioral scope changes
OrganizationAlertRulePermissionandOrganizationDetectorPermissionnow route alert mutations throughOrganizationAlertingMutationPermission, whosePOST/PUT/DELETEscope map isalerts:write.alerts:writetokens work without regressing missing alert-rule or detector IDs from404to403.org:writeonly.org:read,org:admin, andproject:readare not retained for those mutation paths.alerts:writeconsistently. Workflow and detector endpoints still retain their previousorg:write/org:adminAPI-token compatibility where those scopes were already accepted.alerts:writesemantics. They keep temporaryorg:writecompatibility only; readonly organization scopes are no longer accepted.POSTnow requiresevent:writeorevent:admin;event:readno longer authorizes export creation.event:writeorevent:admin;event:readno longer authorizes creating a derived issue occurrence.event:admin;project:read,project:write, andevent:writeno longer authorize deletion. Replay summary generation stays read-like and is explicitly documented as such.org:writeto edit;alerts:writealone is not sufficient. User-created detectors retainalerts:writeauthorization.Published mutation note coverage
readonly_mutation_scope_exceptionsnotes for pinned searches, saved searches, recent searches, dashboards, dashboard starring, Discover and Explore saved queries, insights segment starring, and group search view preference endpoints.TODO(api-scope-personalization)comments. These are intentionally left as follow-up taxonomy work, not treated as semantically ideal scopes.Future legacy compat to remove
TODO(api-write-scope-compat)inOrganizationAlertRuleIndexEndpoint.POST: drop temporaryorg:write.TODO(api-write-scope-compat)inOrganizationAlertRuleDetailsEndpoint.PUTand.DELETE: drop temporaryorg:write.TODO(api-write-scope-compat)in alert-rule project validation: drop temporaryorg:writeproject-field fallback.TODO(api-write-scope-compat)in workflow and detector endpoints: drop temporaryorg:write/org:admincompatibility after public clients migrate toalerts:write.TODO(api-write-scope-compat)in uptime preview, uptime assertion suggestions, Seer anomaly preview, and cron monitor endpoints: drop temporaryorg:write.TODO(api-scope-personalization)on personal/member-state writes: introduce a narrow personal or member preference write scope, grant it to normal Sentry member RBAC roles, and move these endpoints off readonly scopes.The OpenAPI scope diff is expected on this PR because the documented security requirements intentionally change.
Refs getsentry/getsentry#19897
Action taken on behalf of David Cramer.
View Session in Sentry