diff --git a/apps/api/plane/api/views/asset.py b/apps/api/plane/api/views/asset.py index 9804a53bc9..81451acd88 100644 --- a/apps/api/plane/api/views/asset.py +++ b/apps/api/plane/api/views/asset.py @@ -587,6 +587,28 @@ class GenericAssetEndpoint(BaseAPIView): ).first() if existing_asset: + # The dedup lookup is scoped to the workspace only -- and when the + # body omits project_id the validation above is skipped entirely -- + # so the match may belong to a project the caller cannot see. + # Echoing it would hand over that asset's id, and asset_url + # additionally embeds the owning project and issue ids. Knowing the + # asset UUID is the precondition for every asset-scoped attack on + # this surface, so this branch must not supply it. + # + # 404 rather than 403 on purpose: a 403 would still confirm that + # some asset carries this external id pair in this workspace, which + # turns the pair into an existence oracle. The elsewhere-consistent + # 403 is fine on routes where the caller already named the asset id; + # here they named only an external id, so a match is new + # information. The cost is that a caller who guesses a pair held by + # a project they cannot see cannot create their own asset under it, + # which is the right trade -- real integrations mint ids per source + # and run as a member of the target project. + if not existing_asset.is_project_accessible_to(request.user): + return Response( + {"error": "Asset not found.", "status": False}, + status=status.HTTP_404_NOT_FOUND, + ) return Response( { "message": "Asset with same external id and source already exists", diff --git a/apps/api/plane/tests/contract/api/test_generic_asset_project_scope.py b/apps/api/plane/tests/contract/api/test_generic_asset_project_scope.py index d780016790..453e39292b 100644 --- a/apps/api/plane/tests/contract/api/test_generic_asset_project_scope.py +++ b/apps/api/plane/tests/contract/api/test_generic_asset_project_scope.py @@ -295,3 +295,94 @@ class TestGenericAssetPostProjectScope: f"Got {response.status_code}: {getattr(response, 'data', None)!r}" ) assert FileAsset.objects.count() == before + 1 + + +@pytest.mark.contract +class TestGenericAssetExternalIdDedupDisclosure: + """The 409 dedup echo must not hand out a foreign project's asset identifiers. + + The dedup lookup matches on workspace + external_source + external_id, with no + project scoping, and the 409 body carries ``asset_id`` and ``asset_url``. That + url embeds the owning project and issue ids for an attachment. Since knowing + the asset UUID is the precondition for every asset-scoped attack on this + surface, echoing a match the caller cannot access supplies exactly what the + other guards in this module exist to make useless. + """ + + EXTERNAL = {"external_id": "EXT-1", "external_source": "jira"} + + def _payload(self, project_id=None): + payload = {"name": "dedup.png", "type": "image/png", "size": 16, **self.EXTERNAL} + if project_id is not None: + payload["project_id"] = str(project_id) + return payload + + @pytest.mark.django_db + def test_dedup_does_not_disclose_foreign_asset_identifiers( + self, api_key_client, workspace, foreign_asset, joined_project + ): + """404, not 403: a 403 would still confirm the external id pair is taken.""" + foreign_asset.external_id = self.EXTERNAL["external_id"] + foreign_asset.external_source = self.EXTERNAL["external_source"] + foreign_asset.save(update_fields=["external_id", "external_source"]) + + response = api_key_client.post( + asset_list_url(workspace.slug), self._payload(joined_project.id), format="json" + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + body = str(getattr(response, "data", "")) + assert str(foreign_asset.id) not in body, "the foreign asset id was disclosed" + assert str(foreign_asset.project_id) not in body, "the foreign project id was disclosed" + + @pytest.mark.django_db + def test_dedup_does_not_disclose_when_project_id_is_omitted( + self, api_key_client, workspace, foreign_asset + ): + """Omitting project_id skips the create-path validation, so this branch is the only gate.""" + foreign_asset.external_id = self.EXTERNAL["external_id"] + foreign_asset.external_source = self.EXTERNAL["external_source"] + foreign_asset.save(update_fields=["external_id", "external_source"]) + + response = api_key_client.post(asset_list_url(workspace.slug), self._payload(), format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert str(foreign_asset.id) not in str(getattr(response, "data", "")) + + @pytest.mark.django_db + def test_dedup_still_echoes_an_accessible_asset( + self, api_key_client, workspace, joined_asset, joined_project + ): + """Dedup must keep working for a caller who is a member of the match's project.""" + joined_asset.external_id = self.EXTERNAL["external_id"] + joined_asset.external_source = self.EXTERNAL["external_source"] + joined_asset.save(update_fields=["external_id", "external_source"]) + + response = api_key_client.post( + asset_list_url(workspace.slug), self._payload(joined_project.id), format="json" + ) + + assert response.status_code == status.HTTP_409_CONFLICT, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert response.data["asset_id"] == str(joined_asset.id) + + @pytest.mark.django_db + def test_dedup_still_echoes_a_workspace_level_asset( + self, api_key_client, workspace, workspace_level_asset + ): + """project_id is NULL, so there is no project dimension to gate on.""" + workspace_level_asset.external_id = self.EXTERNAL["external_id"] + workspace_level_asset.external_source = self.EXTERNAL["external_source"] + workspace_level_asset.save(update_fields=["external_id", "external_source"]) + + response = api_key_client.post(asset_list_url(workspace.slug), self._payload(), format="json") + + assert response.status_code == status.HTTP_409_CONFLICT, ( + f"Got {response.status_code}: {getattr(response, 'data', None)!r}" + ) + assert response.data["asset_id"] == str(workspace_level_asset.id)