diff --git a/src/sentry/api/bases/organization_request_change.py b/src/sentry/api/bases/organization_request_change.py index 905325f6ec1626..d52ed444ce694e 100644 --- a/src/sentry/api/bases/organization_request_change.py +++ b/src/sentry/api/bases/organization_request_change.py @@ -7,6 +7,9 @@ class OrganizationRequestChangeEndpointPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "This endpoint only files a request for an organization change; members use it without organization write access.", + } # just requesting so read permission is enough scope_map = { "POST": ["org:read"], diff --git a/src/sentry/codecov/endpoints/repository_token_regenerate/repository_token_regenerate.py b/src/sentry/codecov/endpoints/repository_token_regenerate/repository_token_regenerate.py index fa1c10855f459a..070044325d81f0 100644 --- a/src/sentry/codecov/endpoints/repository_token_regenerate/repository_token_regenerate.py +++ b/src/sentry/codecov/endpoints/repository_token_regenerate/repository_token_regenerate.py @@ -18,6 +18,9 @@ class RepositoryTokenRegeneratePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Codecov token regeneration preserves read-only token access for now.", + } scope_map = { "GET": ["org:read", "org:write", "org:admin"], "POST": ["org:read", "org:write", "org:admin"], diff --git a/src/sentry/codecov/endpoints/sync_repos/sync_repos.py b/src/sentry/codecov/endpoints/sync_repos/sync_repos.py index a00af3014addd5..1ef5e9cfa93c6f 100644 --- a/src/sentry/codecov/endpoints/sync_repos/sync_repos.py +++ b/src/sentry/codecov/endpoints/sync_repos/sync_repos.py @@ -18,6 +18,9 @@ class SyncReposPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Codecov sync preserves read-only token access pending integration-scope cleanup.", + } scope_map = { "GET": ["org:read", "org:write", "org:admin"], "POST": ["org:read", "org:write", "org:admin"], diff --git a/src/sentry/conduit/endpoints/organization_conduit_demo.py b/src/sentry/conduit/endpoints/organization_conduit_demo.py index 60ad92cb22c860..7ad015ff5223f9 100644 --- a/src/sentry/conduit/endpoints/organization_conduit_demo.py +++ b/src/sentry/conduit/endpoints/organization_conduit_demo.py @@ -31,6 +31,9 @@ class OrganizationConduitDemoPermission(OrganizationPermission): This is a demo-only feature and doesn't modify organization state. """ + readonly_mutation_scope_exceptions = { + "POST": "Demo credential generation preserves read-only token access for now.", + } scope_map = { "POST": ["org:read", "org:write", "org:admin"], } diff --git a/src/sentry/core/endpoints/organization_member_invite/index.py b/src/sentry/core/endpoints/organization_member_invite/index.py index 999c77af08ec80..ff40ad0542f9a5 100644 --- a/src/sentry/core/endpoints/organization_member_invite/index.py +++ b/src/sentry/core/endpoints/organization_member_invite/index.py @@ -38,6 +38,9 @@ class MemberInvitePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Members can create invite requests on this endpoint even when they cannot send invites directly.", + } scope_map = { "GET": ["member:read", "member:write", "member:admin"], # We will do an additional check to see if a user can invite members. If diff --git a/src/sentry/core/endpoints/organization_member_invite/utils.py b/src/sentry/core/endpoints/organization_member_invite/utils.py index 6be047608e3795..67f6b36d2457d0 100644 --- a/src/sentry/core/endpoints/organization_member_invite/utils.py +++ b/src/sentry/core/endpoints/organization_member_invite/utils.py @@ -6,6 +6,9 @@ class MemberInviteDetailsPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "DELETE": "Invite deletion keeps current member-read token semantics for now.", + } scope_map = { "GET": ["member:read", "member:write", "member:admin"], "PUT": ["member:write", "member:admin"], diff --git a/src/sentry/core/endpoints/organization_member_requests_invite_index.py b/src/sentry/core/endpoints/organization_member_requests_invite_index.py index 4fd8a274c763d3..2a0113bd22b163 100644 --- a/src/sentry/core/endpoints/organization_member_requests_invite_index.py +++ b/src/sentry/core/endpoints/organization_member_requests_invite_index.py @@ -20,6 +20,9 @@ class InviteRequestPermissions(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Invite request creation keeps member-read token access for now.", + } scope_map = { "GET": ["member:read", "member:write", "member:admin"], "POST": ["member:read", "member:write", "member:admin"], diff --git a/src/sentry/core/endpoints/organization_member_team_details.py b/src/sentry/core/endpoints/organization_member_team_details.py index a569d7f7cc575a..bbb1e9e61293ac 100644 --- a/src/sentry/core/endpoints/organization_member_team_details.py +++ b/src/sentry/core/endpoints/organization_member_team_details.py @@ -72,6 +72,11 @@ def serialize( class OrganizationTeamMemberPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Team membership writes keep current mixed org/member/team token semantics for now.", + "PUT": "Team membership writes keep current mixed org/member/team token semantics for now.", + "DELETE": "Team membership writes keep current mixed org/member/team token semantics for now.", + } scope_map = { "GET": [ "org:read", diff --git a/src/sentry/core/endpoints/organization_member_utils.py b/src/sentry/core/endpoints/organization_member_utils.py index 9993a68d563cbd..346f5d2e709744 100644 --- a/src/sentry/core/endpoints/organization_member_utils.py +++ b/src/sentry/core/endpoints/organization_member_utils.py @@ -61,6 +61,9 @@ class MemberConflictValidationError(serializers.ValidationError): class RelaxedMemberPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "DELETE": "Member deletion keeps self-service and role-comparison semantics for now.", + } scope_map = { "GET": ["member:read", "member:write", "member:admin"], "POST": ["member:write", "member:admin"], diff --git a/src/sentry/dashboards/endpoints/organization_dashboard_generate.py b/src/sentry/dashboards/endpoints/organization_dashboard_generate.py index 66b90b0d6f3742..cd36350241d18f 100644 --- a/src/sentry/dashboards/endpoints/organization_dashboard_generate.py +++ b/src/sentry/dashboards/endpoints/organization_dashboard_generate.py @@ -49,6 +49,9 @@ class DashboardGenerateSerializer(serializers.Serializer[dict[str, Any]]): class OrganizationDashboardGeneratePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Dashboard generation is a POST helper/action and needs separate contract cleanup.", + } scope_map = { "POST": ["org:read"], } diff --git a/src/sentry/notifications/api/endpoints/notification_actions_index.py b/src/sentry/notifications/api/endpoints/notification_actions_index.py index f1bcfee6590675..1e1a27feedad64 100644 --- a/src/sentry/notifications/api/endpoints/notification_actions_index.py +++ b/src/sentry/notifications/api/endpoints/notification_actions_index.py @@ -30,6 +30,11 @@ class NotificationActionsPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Notification action writes also rely on project-scoped checks; cleanup is deferred.", + "PUT": "Notification action writes also rely on project-scoped checks; cleanup is deferred.", + "DELETE": "Notification action writes also rely on project-scoped checks; cleanup is deferred.", + } scope_map = { "GET": ["org:read", "org:write", "org:admin"], "POST": ["org:read", "org:write", "org:admin"], diff --git a/src/sentry/preprod/api/bases/preprod_artifact_endpoint.py b/src/sentry/preprod/api/bases/preprod_artifact_endpoint.py index 2e328ce4818d57..bc60e21c71c159 100644 --- a/src/sentry/preprod/api/bases/preprod_artifact_endpoint.py +++ b/src/sentry/preprod/api/bases/preprod_artifact_endpoint.py @@ -33,6 +33,10 @@ class BasePreprodArtifactResourceDoesNotExist(APIException): # This is not a general permission. It specifically for triggering comparisons. class ProjectPreprodArtifactPermission(OrganizationEventPermission): + readonly_mutation_scope_exceptions = { + "POST": "Preprod comparison triggers preserve read-only token access for now.", + "PUT": "Preprod comparison triggers preserve read-only token access for now.", + } scope_map = { "GET": ["event:read", "event:write", "event:admin"], # Some simple actions, like triggering comparisons, should be allowed diff --git a/src/sentry/seer/endpoints/issue_view_title_generate.py b/src/sentry/seer/endpoints/issue_view_title_generate.py index 1e02445b93e525..bef34aa8b64db9 100644 --- a/src/sentry/seer/endpoints/issue_view_title_generate.py +++ b/src/sentry/seer/endpoints/issue_view_title_generate.py @@ -29,6 +29,9 @@ class IssueViewTitleGeneratePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Issue title generation is a read-like POST helper and needs separate cleanup.", + } scope_map = { "POST": ["org:read"], } diff --git a/src/sentry/seer/endpoints/organization_seer_explorer_chat.py b/src/sentry/seer/endpoints/organization_seer_explorer_chat.py index a0ba5be568ce1d..a38a7d2ceb7efb 100644 --- a/src/sentry/seer/endpoints/organization_seer_explorer_chat.py +++ b/src/sentry/seer/endpoints/organization_seer_explorer_chat.py @@ -58,6 +58,9 @@ class SeerExplorerChatSerializer(serializers.Serializer): class OrganizationSeerExplorerChatPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Seer explorer chat is a read-like POST helper and needs separate cleanup.", + } scope_map = { "GET": ["org:read"], "POST": ["org:read"], diff --git a/src/sentry/seer/endpoints/organization_seer_explorer_update.py b/src/sentry/seer/endpoints/organization_seer_explorer_update.py index 722fedc69ac669..24752d9741720a 100644 --- a/src/sentry/seer/endpoints/organization_seer_explorer_update.py +++ b/src/sentry/seer/endpoints/organization_seer_explorer_update.py @@ -24,6 +24,9 @@ class OrganizationSeerExplorerUpdatePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Seer explorer updates are POST helpers and need separate contract cleanup.", + } scope_map = { "POST": ["org:read"], } diff --git a/src/sentry/seer/endpoints/organization_seer_rpc.py b/src/sentry/seer/endpoints/organization_seer_rpc.py index e46abba59b58c8..8526893983f494 100644 --- a/src/sentry/seer/endpoints/organization_seer_rpc.py +++ b/src/sentry/seer/endpoints/organization_seer_rpc.py @@ -147,6 +147,9 @@ class SeerRpcPermission(OrganizationPermission): # Seer RPCs uses POST requests but is actually read only # So relax the permissions here. + readonly_mutation_scope_exceptions = { + "POST": "Seer RPC POST is intentionally read-only and tracked separately.", + } scope_map = { "POST": ["org:read", "org:write", "org:admin"], } diff --git a/src/sentry/seer/endpoints/organization_trace_summary.py b/src/sentry/seer/endpoints/organization_trace_summary.py index e0b9b766a24c21..8eb28b0dd89a5c 100644 --- a/src/sentry/seer/endpoints/organization_trace_summary.py +++ b/src/sentry/seer/endpoints/organization_trace_summary.py @@ -21,6 +21,9 @@ class OrganizationTraceSummaryPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Trace summary is a read-like POST helper and needs separate contract cleanup.", + } scope_map = { "POST": ["org:read"], } diff --git a/src/sentry/seer/endpoints/trace_explorer_ai_setup.py b/src/sentry/seer/endpoints/trace_explorer_ai_setup.py index 35016ca955ab86..ef4a9b05b9458f 100644 --- a/src/sentry/seer/endpoints/trace_explorer_ai_setup.py +++ b/src/sentry/seer/endpoints/trace_explorer_ai_setup.py @@ -27,6 +27,9 @@ class OrganizationTraceExplorerAIPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Trace explorer POST helpers are read-like actions and need separate cleanup.", + } scope_map = { "GET": ["org:read"], "POST": ["org:read"], diff --git a/src/sentry/sentry_apps/api/bases/sentryapps.py b/src/sentry/sentry_apps/api/bases/sentryapps.py index 637d0a27d64515..7fcf6f57afc30f 100644 --- a/src/sentry/sentry_apps/api/bases/sentryapps.py +++ b/src/sentry/sentry_apps/api/bases/sentryapps.py @@ -432,6 +432,9 @@ def convert_args(self, request: Request, uuid, *args, **kwargs): class SentryAppInstallationExternalIssuePermission(SentryAppInstallationPermission): + readonly_mutation_scope_exceptions = { + "POST": "This endpoint creates an external issue link for an issue the caller can already read.", + } scope_map = { "POST": ("event:read", "event:write", "event:admin"), "DELETE": ("event:admin",), diff --git a/tests/sentry/api/test_permissions.py b/tests/sentry/api/test_permissions.py index 1d2286204b35e1..faef4de77ea996 100644 --- a/tests/sentry/api/test_permissions.py +++ b/tests/sentry/api/test_permissions.py @@ -1,5 +1,11 @@ +from collections.abc import Generator + +from django.test import SimpleTestCase +from django.urls import URLPattern, URLResolver +from django.urls.resolvers import get_resolver from rest_framework.views import APIView +from sentry.api.base import Endpoint from sentry.api.permissions import ( DemoSafePermission, DisallowImpersonatedTokenCreation, @@ -8,10 +14,34 @@ SuperuserOrStaffFeatureFlaggedPermission, SuperuserPermission, ) +from sentry.conf.server import SENTRY_READONLY_SCOPES from sentry.demo_mode.utils import READONLY_SCOPES from sentry.organizations.services.organization import organization_service from sentry.testutils.cases import DRFPermissionTestCase from sentry.testutils.helpers.options import override_options +from sentry.testutils.silo import no_silo_test + +MUTATION_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"}) + + +def _iter_endpoint_view_classes(urlpatterns, base: str = "") -> Generator[type[Endpoint]]: + for pattern in urlpatterns: + if isinstance(pattern, URLResolver): + yield from _iter_endpoint_view_classes( + pattern.url_patterns, base + str(pattern.pattern) + ) + elif isinstance(pattern, URLPattern): + callback = pattern.callback + if hasattr(callback, "view_class") and issubclass(callback.view_class, Endpoint): + yield callback.view_class + + +def _get_class_path(cls: type[object]) -> str: + return f"{cls.__module__}.{cls.__name__}" + + +def _get_readonly_mutation_scope_exceptions(cls: type[object]) -> dict[str, str]: + return getattr(cls, "readonly_mutation_scope_exceptions", {}) or {} class PermissionsTest(DRFPermissionTestCase): @@ -223,3 +253,62 @@ def test_determine_access_no_demo_users(self) -> None: ) assert readonly_rpc_context.member.scopes == list(self.org_member_scopes) + + +@no_silo_test +class PublishedMutationScopeTest(SimpleTestCase): + def test_readonly_mutation_scope_exceptions_are_notes(self) -> None: + for view_cls in sorted( + set(_iter_endpoint_view_classes(get_resolver().url_patterns)), key=_get_class_path + ): + for cls in (view_cls, *getattr(view_cls, "permission_classes", ())): + exceptions = getattr(cls, "readonly_mutation_scope_exceptions", None) + if exceptions is None: + continue + + assert isinstance(exceptions, dict), ( + f"{_get_class_path(cls)} readonly_mutation_scope_exceptions must be a dict" + ) + + for method, note in exceptions.items(): + assert method in MUTATION_METHODS, ( + f"{_get_class_path(cls)} readonly_mutation_scope_exceptions[{method!r}] " + "must target a mutation method" + ) + assert isinstance(note, str) and note.strip(), ( + f"{_get_class_path(cls)} readonly_mutation_scope_exceptions[{method!r}] " + "must be a non-empty note" + ) + + def test_published_mutation_endpoints_require_readonly_scope_notes(self) -> None: + missing_notes = [] + + for view_cls in sorted( + set(_iter_endpoint_view_classes(get_resolver().url_patterns)), key=_get_class_path + ): + publish_status = getattr(view_cls, "publish_status", {}) or {} + permission_classes = getattr(view_cls, "permission_classes", ()) or () + view_exceptions = _get_readonly_mutation_scope_exceptions(view_cls) + + for method in MUTATION_METHODS & set(publish_status): + for permission_cls in permission_classes: + readonly_scopes = ( + set(getattr(permission_cls, "scope_map", {}).get(method, ())) + & SENTRY_READONLY_SCOPES + ) + if not readonly_scopes: + continue + + if view_exceptions.get(method) or _get_readonly_mutation_scope_exceptions( + permission_cls + ).get(method): + continue + + missing_notes.append( + f"{_get_class_path(view_cls)} {method} accepts readonly scopes " + f"{sorted(readonly_scopes)} via {_get_class_path(permission_cls)}. " + "Remove the readonly scopes or add " + "readonly_mutation_scope_exceptions[method] with a justification note." + ) + + assert not missing_notes, "\n".join(missing_notes)