fix(api): Enforce write scopes on published mutations#113109
Closed
fix(api): Enforce write scopes on published mutations#113109
Conversation
Remove readonly scopes from published mutation endpoints and add dedicated write scopes where the API intentionally allows narrower writes. This keeps the public token contract explicit while preserving the existing session behavior for user-owned state and team-scoped workflows. Add a published-endpoint invariant test plus endpoint-level permission coverage for the new scope contracts, including user preferences, project creation, and codeowners flows. Refs getsentry/getsentry#19897 Co-Authored-By: OpenAI Codex <noreply@openai.com>
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| teams = Team.objects.filter(id__in=request.access.team_ids_with_membership) | ||
| return any( | ||
| any(request.access.has_team_scope(team, scope) for scope in scopes) for team in teams | ||
| ) |
Contributor
There was a problem hiding this comment.
Duplicate _has_any_team_scope with divergent signatures
Low Severity
Two _has_any_team_scope helper functions are introduced in separate files with different signatures — one in organization.py accepts a single scope: str, the other in discover_key_transactions.py accepts scopes: list[str]. This duplication with divergent APIs increases maintenance burden and invites confusion if a future developer imports or copies the wrong variant.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit df3aac6. Configure here.
Contributor
|
This PR has a migration; here is the generated SQL for for --
-- Alter field scopes on apiauthorization
--
-- (no-op)
--
-- Alter field scopes on apikey
--
-- (no-op)
--
-- Alter field scopes on apitoken
--
-- (no-op)
--
-- Alter field scopes on sentryapp
--
-- (no-op) |
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
| or request.access.has_scope("project:admin") | ||
| or request.access.has_any_project_scope(project, ["project:write", "project:admin"]) | ||
| ) | ||
| requested_fields = set(request.data.keys()) |
Contributor
There was a problem hiding this comment.
AttributeError if request body is a JSON array instead of object
The new code calls request.data.keys() at line 614 before serializer validation. If a client sends a JSON array ([]) instead of an object ({}), request.data will be a list and .keys() will raise AttributeError: 'list' object has no attribute 'keys', resulting in a 500 error. Other Sentry endpoints (e.g., organization_seer_explorer_update.py:51) explicitly guard against this with isinstance(request.data, dict) checks.
Verification
Traced the code path: line 614 calls set(request.data.keys()) before the serializer is instantiated at line 627. Confirmed other endpoints (src/sentry/seer/endpoints/organization_seer_explorer_update.py:51, src/sentry/seer/endpoints/group_autofix_update.py:40) explicitly check isinstance(request.data, dict) before accessing dict methods, indicating this is a recognized pattern. The old code used request.data.get(key) which would not crash on a list.
Suggested fix: Add a type check before accessing .keys() to handle malformed request bodies gracefully.
Suggested change
| requested_fields = set(request.data.keys()) | |
| if not isinstance(request.data, dict): | |
| return Response( | |
| {"detail": "Request body must be a JSON object."}, | |
| status=400, | |
| ) | |
| requested_fields = set(request.data.keys()) |
Identified by Warden sentry-backend-bugs · K2K-496
Contributor
There was a problem hiding this comment.
Fix attempt detected (commit 23b5b5e)
The commit introduced the problematic code set(request.data.keys()) at line 614 without the required isinstance(request.data, dict) check, making the endpoint vulnerable to AttributeError when clients send JSON arrays instead of objects.
The original issue appears unresolved. Please review and try again.
Evaluated by Warden
Contributor
Backend Test FailuresFailures on
|
Expand token scope hierarchies during permission evaluation so leaf scopes like org:searches, project:create, and project:codeowners do not need parent fallbacks in endpoint scope maps. Move clearly user-owned search and starring endpoints onto user:preferences, keep org:searches for real saved-search and custom-view resources, and reject user:preferences and flags:write when third-party API apps request scopes. This keeps personal state out of app-granted scopes while preserving the intended session and broad-token behavior for higher-level write scopes. Refs getsentry/getsentry#19897 Co-Authored-By: OpenAI Codex <noreply@openai.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 23b5b5e. Configure here.
| "project:read", | ||
| "project:write", | ||
| "project:admin", | ||
| "project:codeowners", |
Contributor
There was a problem hiding this comment.
Admin/manager/owner roles missing project:create scope breaks project creation
High Severity
The project:create scope is only added to the member role, but not to admin, manager, or owner roles. Since TeamProjectPermission and OrgProjectPermission scope_maps now exclusively require project:create for POST, and session-based auth uses raw role scopes without hierarchy expansion, admin/manager/owner users can no longer create projects through either endpoint. The project:write → project:create hierarchy expansion only applies to token scopes_upper_bound via _wrap_scopes, not to session-based access.scopes.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 23b5b5e. Configure here.
Contributor
Backend Test FailuresFailures on
|
romtsn
reviewed
Apr 15, 2026
| "GET": ["org:read", "org:write", "org:admin", "org:integrations", "org:ci"], | ||
| "POST": ["org:read", "org:write", "org:admin", "org:integrations"], | ||
| "PUT": ["org:read", "org:write", "org:admin", "org:integrations"], | ||
| "POST": ["org:write", "org:admin", "org:integrations"], |
Member
There was a problem hiding this comment.
unless I missed it, but this is going to restrict code mappings creation only to admins/owners. We might want to introduce a new scope org:codemappings or something like that
Member
Author
|
fyi i will dig through any feedback here but i broke this up into 4 somewhat-smaller changes (they're a little tightly coupled so its tough) |
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
2 participants
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.
Enforce the API contract that published mutation endpoints must require non-readonly scopes.
This removes readonly scopes from published POST/PUT/DELETE endpoints and adds dedicated write scopes where Sentry intentionally exposes narrower mutations:
project:createfor self-serve project creation,project:codeownersfor ownership and CODEOWNERS management,user:preferencesfor personal state like bookmarks and onboarding progress,org:searchesfor saved searches and custom views, andflags:writefor flag webhook signing secrets.The patch keeps session behavior working by granting those scopes to the appropriate roles instead of relying on readonly scopes plus later row or object ACL checks. It also preserves legacy token decoding by appending the new scope bits instead of reordering the existing bitfield.
Add a published-endpoint invariant test and endpoint-level permission coverage for the new mixed-scope cases, while leaving the more controversial POST-as-query surfaces for follow-up.
Refs getsentry/getsentry#19897