Refactor has_release_permission to protect mutations

src/sentry/api/bases/organization.py

@@ -706,14 +706,21 @@ def has_release_permission(
         organization: Organization | RpcOrganization,
         release: Release | None = None,
         project_ids: set[int] | None = None,
+        require_all_projects: bool = False,
     ) -> bool:
         """
         Does the given request have permission to access this release, based
         on the projects to which the release is attached?
 
-        If the given request has an actor (user or ApiKey), cache the results
-        for a minute on the unique combination of actor,org,release, and project
-        ids.
+        By default, access is granted if the user has access to *any* project
+        on the release (suitable for reads). When require_all_projects=True,
+        the user must have access to *all* projects on the release (use for
+        mutations). Without this, a user with access to one project on a
+        multi-project release could modify or delete it, affecting projects
+        they cannot access. The all-projects check respects Open Membership
+        via has_global_access.
+
+        Results are cached for 60s per actor/org/release/project-ids/mode.
         """
         actor_id = None
         has_perms = None
@@ -727,20 +734,23 @@ def has_release_permission(
             if requested_project_ids is None:
                 requested_project_ids = self.get_requested_project_ids_unchecked(request)
             key = "release_perms:1:%s" % hash_values(
-                [actor_id, organization.id, release.id if release is not None else 0]
+                [
+                    actor_id,
+                    organization.id,
+                    release.id if release is not None else 0,
+                    int(require_all_projects),
+                ]
                 + sorted(requested_project_ids)
             )
             has_perms = cache.get(key)
         if has_perms is None:
             projects = self.get_projects(request, organization, project_ids=project_ids)
-            # XXX(iambriccardo): The logic here is that you have access to this release if any of your projects
-            # associated with this release you have release permissions to.  This is a bit of
-            # a problem because anyone can add projects to a release, so this check is easy
-            # to defeat.
             if release is not None:
                 has_perms = ReleaseProject.objects.filter(
                     release=release, project__in=projects
                 ).exists()
+                if has_perms and require_all_projects:
+                    has_perms = request.access.has_projects_access(list(release.projects.all()))
             else:
                 has_perms = len(projects) > 0
 

src/sentry/releases/endpoints/organization_release_assemble.py

@@ -36,7 +36,9 @@ def post(self, request: Request, organization, version) -> Response:
         except Release.DoesNotExist:
             raise ResourceDoesNotExist
 
-        if not self.has_release_permission(request, organization, release):
+        if not self.has_release_permission(
+            request, organization, release, require_all_projects=True
+        ):
             raise ResourceDoesNotExist
 
         schema = {

src/sentry/releases/endpoints/organization_release_details.py

@@ -446,14 +446,12 @@ def put(self, request: Request, organization: Organization, version) -> Response
             scope.set_tag("failure_reason", "Release.DoesNotExist")
             raise ResourceDoesNotExist
 
-        if not self.has_release_permission(request, organization, release):
+        if not self.has_release_permission(
+            request, organization, release, require_all_projects=True
+        ):
             scope.set_tag("failure_reason", "no_release_permission")
             raise ResourceDoesNotExist
 
-        if not request.access.has_projects_access(list(projects)):
-            scope.set_tag("failure_reason", "no_access_to_all_projects")
-            raise ResourceDoesNotExist
-
         serializer = OrganizationReleaseSerializer(data=request.data)
 
         if not serializer.is_valid():
@@ -562,10 +560,9 @@ def delete(self, request: Request, organization, version) -> Response:
         except Release.DoesNotExist:
             raise ResourceDoesNotExist
 
-        if not self.has_release_permission(request, organization, release):
-            raise ResourceDoesNotExist
-
-        if not request.access.has_projects_access(list(release.projects.all())):
+        if not self.has_release_permission(
+            request, organization, release, require_all_projects=True
+        ):
             raise ResourceDoesNotExist
 
         try:

src/sentry/releases/endpoints/organization_release_file_details.py

@@ -76,7 +76,9 @@ def put(self, request: Request, organization: Organization, version, file_id) ->
         except Release.DoesNotExist:
             raise ResourceDoesNotExist
 
-        if not self.has_release_permission(request, organization, release):
+        if not self.has_release_permission(
+            request, organization, release, require_all_projects=True
+        ):
             raise ResourceDoesNotExist
 
         return self.update_releasefile(request, release, file_id)
@@ -101,7 +103,9 @@ def delete(self, request: Request, organization, version, file_id) -> Response:
         except Release.DoesNotExist:
             raise ResourceDoesNotExist
 
-        if not self.has_release_permission(request, organization, release):
+        if not self.has_release_permission(
+            request, organization, release, require_all_projects=True
+        ):
             raise ResourceDoesNotExist
 
         return self.delete_releasefile(release, file_id)

src/sentry/releases/endpoints/organization_release_files.py

@@ -103,7 +103,9 @@ def post(self, request: Request, organization, version) -> Response:
         logger = logging.getLogger("sentry.files")
         logger.info("organizationreleasefile.start")
 
-        if not self.has_release_permission(request, organization, release):
+        if not self.has_release_permission(
+            request, organization, release, require_all_projects=True
+        ):
             raise ResourceDoesNotExist
 
         return self.post_releasefile(request, release, logger)

src/sentry/releases/endpoints/release_deploys.py

@@ -207,9 +207,9 @@ def post(self, request: Request, organization, version) -> Response:
             )
             raise ResourceDoesNotExist
 
-        if not self.has_release_permission(request, organization, release):
-            # Logic here copied from `has_release_permission` (lightly edited for results to be more
-            # human-readable)
+        if not self.has_release_permission(
+            request, organization, release, require_all_projects=True
+        ):
             if request.user.is_authenticated:
                 auth = f"user.id: {request.user.id}"
             elif request.auth is not None:

tests/sentry/releases/endpoints/test_organization_release_assemble.py

@@ -26,6 +26,39 @@ def setUp(self) -> None:
             args=[self.organization.slug, self.release.version],
         )
 
+    def test_no_access_to_all_projects(self) -> None:
+        user = self.create_user(is_staff=False, is_superuser=False)
+        org = self.organization
+        org.flags.allow_joinleave = False
+        org.save()
+
+        team1 = self.create_team(organization=org)
+        team2 = self.create_team(organization=org)
+        project1 = self.create_project(teams=[team1], organization=org)
+        project2 = self.create_project(teams=[team2], organization=org)
+
+        release = self.create_release(
+            version="restricted-release.1",
+            project=project1,
+            additional_projects=[project2],
+        )
+
+        self.create_member(teams=[team1], user=user, organization=org)
+        self.login_as(user=user)
+
+        url = reverse(
+            "sentry-api-0-organization-release-assemble",
+            args=[org.slug, release.version],
+        )
+
+        checksum = sha1(b"1").hexdigest()
+        response = self.client.post(
+            url,
+            data={"checksum": checksum, "chunks": [checksum]},
+        )
+
+        assert response.status_code == 404
+
     def test_assemble_json_schema(self) -> None:
         response = self.client.post(
             self.url, data={"lol": "test"}, HTTP_AUTHORIZATION=f"Bearer {self.token.token}"

tests/sentry/releases/endpoints/test_organization_release_file_details.py

@@ -120,6 +120,80 @@ def test_archived_with_dist(self) -> None:
 
 
 class ReleaseFileUpdateTest(APITestCase):
+    def test_no_access_to_all_projects(self) -> None:
+        user = self.create_user(is_staff=False, is_superuser=False)
+        org = self.organization
+        org.flags.allow_joinleave = False
+        org.save()
+
+        team1 = self.create_team(organization=org)
+        team2 = self.create_team(organization=org)
+        project1 = self.create_project(teams=[team1], organization=org)
+        project2 = self.create_project(teams=[team2], organization=org)
+
+        release = Release.objects.create(organization_id=org.id, version="1")
+        release.add_project(project1)
+        release.add_project(project2)
+
+        releasefile = ReleaseFile.objects.create(
+            organization_id=org.id,
+            release_id=release.id,
+            file=File.objects.create(name="application.js", type="release.file"),
+            name="http://example.com/application.js",
+        )
+
+        self.create_member(teams=[team1], user=user, organization=org)
+        self.login_as(user=user)
+
+        url = reverse(
+            "sentry-api-0-organization-release-file-details",
+            kwargs={
+                "organization_id_or_slug": org.slug,
+                "version": release.version,
+                "file_id": releasefile.id,
+            },
+        )
+        response = self.client.put(url, {"name": "foobar"})
+
+        assert response.status_code == 404
+
+    def test_no_access_to_all_projects_open_membership(self) -> None:
+        user = self.create_user(is_staff=False, is_superuser=False)
+        org = self.organization
+        org.flags.allow_joinleave = True
+        org.save()
+
+        team1 = self.create_team(organization=org)
+        team2 = self.create_team(organization=org)
+        project1 = self.create_project(teams=[team1], organization=org)
+        project2 = self.create_project(teams=[team2], organization=org)
+
+        release = Release.objects.create(organization_id=org.id, version="1")
+        release.add_project(project1)
+        release.add_project(project2)
+
+        releasefile = ReleaseFile.objects.create(
+            organization_id=org.id,
+            release_id=release.id,
+            file=File.objects.create(name="application.js", type="release.file"),
+            name="http://example.com/application.js",
+        )
+
+        self.create_member(teams=[team1], user=user, organization=org)
+        self.login_as(user=user)
+
+        url = reverse(
+            "sentry-api-0-organization-release-file-details",
+            kwargs={
+                "organization_id_or_slug": org.slug,
+                "version": release.version,
+                "file_id": releasefile.id,
+            },
+        )
+        response = self.client.put(url, {"name": "foobar"})
+
+        assert response.status_code == 200
+
     def test_simple(self) -> None:
         self.login_as(user=self.user)
 
@@ -155,6 +229,44 @@ def test_simple(self) -> None:
 
 
 class ReleaseFileDeleteTest(APITestCase):
+    def test_no_access_to_all_projects(self) -> None:
+        user = self.create_user(is_staff=False, is_superuser=False)
+        org = self.organization
+        org.flags.allow_joinleave = False
+        org.save()
+
+        team1 = self.create_team(organization=org)
+        team2 = self.create_team(organization=org)
+        project1 = self.create_project(teams=[team1], organization=org)
+        project2 = self.create_project(teams=[team2], organization=org)
+
+        release = Release.objects.create(organization_id=org.id, version="1")
+        release.add_project(project1)
+        release.add_project(project2)
+
+        releasefile = ReleaseFile.objects.create(
+            organization_id=org.id,
+            release_id=release.id,
+            file=File.objects.create(name="application.js", type="release.file"),
+            name="http://example.com/application.js",
+        )
+
+        self.create_member(teams=[team1], user=user, organization=org)
+        self.login_as(user=user)
+
+        url = reverse(
+            "sentry-api-0-organization-release-file-details",
+            kwargs={
+                "organization_id_or_slug": org.slug,
+                "version": release.version,
+                "file_id": releasefile.id,
+            },
+        )
+        response = self.client.delete(url)
+
+        assert response.status_code == 404
+        assert ReleaseFile.objects.filter(id=releasefile.id).exists()
+
     def test_simple(self) -> None:
         self.login_as(user=self.user)
 

tests/sentry/releases/endpoints/test_organization_release_files.py

@@ -206,6 +206,44 @@ def test_queries_should_be_narrowing_search(self) -> None:
 
 
 class ReleaseFileCreateTest(APITestCase):
+    def test_no_access_to_all_projects(self) -> None:
+        user = self.create_user(is_staff=False, is_superuser=False)
+        org = self.organization
+        org.flags.allow_joinleave = False
+        org.save()
+
+        team1 = self.create_team(organization=org)
+        team2 = self.create_team(organization=org)
+        project1 = self.create_project(teams=[team1], organization=org)
+        project2 = self.create_project(teams=[team2], organization=org)
+
+        release = Release.objects.create(organization_id=org.id, version="1")
+        release.add_project(project1)
+        release.add_project(project2)
+
+        self.create_member(teams=[team1], user=user, organization=org)
+        self.login_as(user=user)
+
+        url = reverse(
+            "sentry-api-0-organization-release-files",
+            kwargs={
+                "organization_id_or_slug": org.slug,
+                "version": release.version,
+            },
+        )
+        response = self.client.post(
+            url,
+            {
+                "name": "http://example.com/application.js",
+                "file": SimpleUploadedFile(
+                    "application.js", b"function() { }", content_type="application/javascript"
+                ),
+            },
+            format="multipart",
+        )
+
+        assert response.status_code == 404
+
     def test_simple(self) -> None:
         project = self.create_project(name="foo")