From 880509813f1966d74d5cd2ae63bd6e2c653b6387 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Fri, 24 Jul 2026 15:13:41 -0700 Subject: [PATCH 1/4] fix(integrations): Scope code mapping detail lookup to the org The config lookup only filtered on org integration ids, and the new project came straight out of `request.data` in `convert_args`. Two problems fell out of that. A PUT without `projectId` never set the `new_project` kwarg, so the handler blew up on a missing positional arg and returned a 500 instead of a validation error. And `get_project` was called with whatever string was in the payload, so a non-numeric `projectId` hit the db as garbage. Now the lookup filters on `organization_id` directly, and the new project is resolved from `serializer.validated_data` after validation runs. The serializer already marks `projectId` required and camelizes its error keys, so the missing-field case returns the normal 400 field errors with no special casing in the handler. Also catches ValueError on the config lookup since the url pattern for `config_id` is `[^/]+`, not digits. Co-Authored-By: Claude Opus 5 --- .../organization_code_mapping_details.py | 31 ++++++++++--------- .../test_organization_code_mapping_details.py | 26 ++++++++++++++++ 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py index 67770f09cfdc..6f83a53e61c2 100644 --- a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py +++ b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py @@ -42,19 +42,15 @@ def convert_args(self, request: Request, organization_id_or_slug, config_id, *ar "project_repository__project" ).get( id=config_id, + organization_id=kwargs["organization"].id, organization_integration_id__in=[oi.id for oi in ois], ) - except RepositoryProjectPathConfig.DoesNotExist: + except (RepositoryProjectPathConfig.DoesNotExist, ValueError): raise Http404 - if request.data.get("projectId"): - kwargs["new_project"] = super().get_project( - kwargs["organization"], request.data.get("projectId") - ) - return (args, kwargs) - def put(self, request: Request, config_id, organization, config, new_project) -> Response: + def put(self, request: Request, config_id, organization, config) -> Response: """ Update a repository project path config `````````````````` @@ -69,7 +65,7 @@ def put(self, request: Request, config_id, organization, config, new_project) -> :auth: required """ project = config.project_repository.project - if not request.access.has_projects_access([project, new_project]): + if not request.access.has_project_access(project): return self.respond(status=status.HTTP_403_FORBIDDEN) try: @@ -90,13 +86,18 @@ def put(self, request: Request, config_id, organization, config, new_project) -> instance=config, data=request.data, ) - if serializer.is_valid(): - repository_project_path_config = serializer.save() - return self.respond( - serialize(repository_project_path_config, request.user), - status=status.HTTP_200_OK, - ) - return self.respond(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + if not serializer.is_valid(): + return self.respond(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + new_project = self.get_project(organization, serializer.validated_data["project_id"]) + if not request.access.has_project_access(new_project): + return self.respond(status=status.HTTP_403_FORBIDDEN) + + repository_project_path_config = serializer.save() + return self.respond( + serialize(repository_project_path_config, request.user), + status=status.HTTP_200_OK, + ) def delete(self, request: Request, config_id, organization, config) -> Response: """ diff --git a/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py b/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py index ed8dc00b2165..eca71133c894 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py @@ -71,6 +71,9 @@ def test_non_project_member_permissions(self) -> None: non_member_om = self.create_member(organization=self.org, user=non_member) self.login_as(user=non_member) + response = self.client.put(self.url, {"sourceRoot": ""}) + assert response.status_code == status.HTTP_403_FORBIDDEN + response = self.make_put({"sourceRoot": "newRoot"}) assert response.status_code == status.HTTP_403_FORBIDDEN @@ -102,6 +105,16 @@ def test_basic_edit(self) -> None: assert resp.data["id"] == str(self.config.id) assert resp.data["sourceRoot"] == "newRoot" + def test_edit_rejects_missing_required_fields(self) -> None: + resp = self.client.put(self.url, {"sourceRoot": ""}) + + assert resp.status_code == status.HTTP_400_BAD_REQUEST + assert resp.data == { + "projectId": ["This field is required."], + "repositoryId": ["This field is required."], + "stackRoot": ["This field is required."], + } + def test_basic_edit_from_member_permissions(self) -> None: self.login_as(user=self.user2) resp = self.make_put({"sourceRoot": "newRoot"}) @@ -127,3 +140,16 @@ def test_delete_another_orgs_code_mapping(self) -> None: ) resp = self.client.delete(url) assert resp.status_code == 404 + + def test_edit_another_orgs_code_mapping(self) -> None: + invalid_user = self.create_user() + invalid_organization = self.create_organization(owner=invalid_user) + self.login_as(user=invalid_user) + url = reverse( + self.endpoint, + args=[invalid_organization.slug, self.config.id], + ) + + resp = self.client.put(url, {}) + + assert resp.status_code == status.HTTP_404_NOT_FOUND From 0b7cbc2b26897442ccc8e75ea06917151af01e7e Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Fri, 24 Jul 2026 15:18:59 -0700 Subject: [PATCH 2/4] ref(integrations): Drop redundant org filter from code mapping lookup Unrelated to the 500 fix, and `organization_integration_id__in` already scopes the lookup to the current org since those ids come from the org itself. Keeping the pr focused. Co-Authored-By: Claude Opus 5 --- .../endpoints/organization_code_mapping_details.py | 1 - .../test_organization_code_mapping_details.py | 13 ------------- 2 files changed, 14 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py index 6f83a53e61c2..2d2eef5ae3e1 100644 --- a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py +++ b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py @@ -42,7 +42,6 @@ def convert_args(self, request: Request, organization_id_or_slug, config_id, *ar "project_repository__project" ).get( id=config_id, - organization_id=kwargs["organization"].id, organization_integration_id__in=[oi.id for oi in ois], ) except (RepositoryProjectPathConfig.DoesNotExist, ValueError): diff --git a/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py b/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py index eca71133c894..a9c60386347a 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py @@ -140,16 +140,3 @@ def test_delete_another_orgs_code_mapping(self) -> None: ) resp = self.client.delete(url) assert resp.status_code == 404 - - def test_edit_another_orgs_code_mapping(self) -> None: - invalid_user = self.create_user() - invalid_organization = self.create_organization(owner=invalid_user) - self.login_as(user=invalid_user) - url = reverse( - self.endpoint, - args=[invalid_organization.slug, self.config.id], - ) - - resp = self.client.put(url, {}) - - assert resp.status_code == status.HTTP_404_NOT_FOUND From 28bcc5ba340e13d935c99a9fbed7fe7d129881e6 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Fri, 24 Jul 2026 15:28:03 -0700 Subject: [PATCH 3/4] ref(integrations): Keep new_project in convert_args, just make it optional Restores the original shape. `new_project` needs to exist so we access check the project a mapping is being moved to, otherwise you could move one into a project you can't see. The only thing wrong with it was that `put()` took it as required while `convert_args` set it conditionally. Now it defaults to None and the access check only includes it when the body actually asks to move the mapping. The project lookup moved into the existing try so a non-numeric `projectId` 404s like a foreign or unknown one already did, instead of blowing up on int(). Keeps every other status code the same as before, only the three 500s turn into 4xx. Co-Authored-By: Claude Opus 5 --- .../organization_code_mapping_details.py | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py index 2d2eef5ae3e1..442750fa95fd 100644 --- a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py +++ b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py @@ -44,12 +44,17 @@ def convert_args(self, request: Request, organization_id_or_slug, config_id, *ar id=config_id, organization_integration_id__in=[oi.id for oi in ois], ) + # Only set when the request wants to move the mapping to another project + if request.data.get("projectId"): + kwargs["new_project"] = self.get_project( + kwargs["organization"], request.data["projectId"] + ) except (RepositoryProjectPathConfig.DoesNotExist, ValueError): raise Http404 return (args, kwargs) - def put(self, request: Request, config_id, organization, config) -> Response: + def put(self, request: Request, config_id, organization, config, new_project=None) -> Response: """ Update a repository project path config `````````````````` @@ -63,8 +68,10 @@ def put(self, request: Request, config_id, organization, config) -> Response: :param string default_branch: :auth: required """ - project = config.project_repository.project - if not request.access.has_project_access(project): + projects = [config.project_repository.project] + if new_project is not None: + projects.append(new_project) + if not request.access.has_projects_access(projects): return self.respond(status=status.HTTP_403_FORBIDDEN) try: @@ -85,18 +92,13 @@ def put(self, request: Request, config_id, organization, config) -> Response: instance=config, data=request.data, ) - if not serializer.is_valid(): - return self.respond(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - - new_project = self.get_project(organization, serializer.validated_data["project_id"]) - if not request.access.has_project_access(new_project): - return self.respond(status=status.HTTP_403_FORBIDDEN) - - repository_project_path_config = serializer.save() - return self.respond( - serialize(repository_project_path_config, request.user), - status=status.HTTP_200_OK, - ) + if serializer.is_valid(): + repository_project_path_config = serializer.save() + return self.respond( + serialize(repository_project_path_config, request.user), + status=status.HTTP_200_OK, + ) + return self.respond(serializer.errors, status=status.HTTP_400_BAD_REQUEST) def delete(self, request: Request, config_id, organization, config) -> Response: """ From a63d3e3b88de93523f14a36f362898f6d8552286 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Fri, 24 Jul 2026 16:21:34 -0700 Subject: [PATCH 4/4] fix(integrations): Access check snake_case project_id on code mapping PUT Bugbot caught a real hole in the previous commit. Making `new_project` optional meant a body using `project_id` instead of `projectId` skipped the destination access check entirely, since `convert_args` looked for the literal camelCase key while the serializer normalizes both spellings and happily moves the mapping. A member could move a code mapping onto a project they can't see. It was a 500 before, so it wasn't reachable. Normalizes the body with the same helpers the serializer uses instead of matching one spelling, so `projectId`, `project_id` and `ProjectId` all resolve to the same lookup and get access checked. Test fails with 200 on the old code and 403 on this one. Co-Authored-By: Claude Opus 5 --- .../organization_code_mapping_details.py | 12 ++++++---- .../test_organization_code_mapping_details.py | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py index 442750fa95fd..a255031efc69 100644 --- a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py +++ b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py @@ -12,6 +12,7 @@ OrganizationIntegrationsLoosePermission, ) from sentry.api.serializers import serialize +from sentry.api.serializers.rest_framework.base import camel_to_snake_case, convert_dict_key_case from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig from sentry.integrations.services.integration import integration_service @@ -44,11 +45,12 @@ def convert_args(self, request: Request, organization_id_or_slug, config_id, *ar id=config_id, organization_integration_id__in=[oi.id for oi in ois], ) - # Only set when the request wants to move the mapping to another project - if request.data.get("projectId"): - kwargs["new_project"] = self.get_project( - kwargs["organization"], request.data["projectId"] - ) + # Only set when the request wants to move the mapping to another project. + # Normalize keys the way the serializer does first, otherwise a snake_case + # `project_id` skips the access check below but still moves the mapping. + data = convert_dict_key_case(request.data, camel_to_snake_case) + if data.get("project_id"): + kwargs["new_project"] = self.get_project(kwargs["organization"], data["project_id"]) except (RepositoryProjectPathConfig.DoesNotExist, ValueError): raise Http404 diff --git a/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py b/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py index a9c60386347a..921b206fea4e 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py @@ -115,6 +115,30 @@ def test_edit_rejects_missing_required_fields(self) -> None: "stackRoot": ["This field is required."], } + def test_edit_snake_case_project_id_is_access_checked(self) -> None: + member = self.create_user() + self.create_member( + organization=self.org, user=member, has_global_access=False, teams=[self.team] + ) + self.login_as(user=member) + + # every key is snake_case so `projectId` never appears in the body, but the + # serializer still reads it as project_id and would move the mapping + resp = self.client.put( + self.url, + { + "repository_id": self.repo.id, + "project_id": self.project2.id, + "stack_root": "/stack/root", + "source_root": "/source/root", + "default_branch": "master", + }, + ) + + assert resp.status_code == status.HTTP_403_FORBIDDEN + self.config.refresh_from_db() + assert self.config.project_repository.project_id == self.project.id + def test_basic_edit_from_member_permissions(self) -> None: self.login_as(user=self.user2) resp = self.make_put({"sourceRoot": "newRoot"})