chore(slug): Prevent numeric sentry function slugs (#59669)

Prevent numeric slugs in SentryFunction model by changing `slugify` call
to `sentry_slugify`.

Add a test for this called `test_generated_slug_not_entirely_numeric`.

Also refactor test file and fix a bug where we weren't correctly testing
the passed in env variables in the test file. Explanation in comments
below

Author: schew2381

src/sentry/api/endpoints/organization_sentry_function.py

@@ -1,22 +1,22 @@
 from uuid import uuid4
 
-from django.utils.text import slugify
 from rest_framework import serializers
 from rest_framework.response import Response
 
 from sentry import features
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases import OrganizationEndpoint
+from sentry.api.helpers.slugs import sentry_slugify
 from sentry.api.serializers import serialize
 from sentry.api.serializers.rest_framework import CamelSnakeSerializer
 from sentry.models.sentryfunction import SentryFunction
 from sentry.utils.cloudfunctions import create_function
 
 
 class EnvVariableSerializer(CamelSnakeSerializer):
-    value = serializers.CharField(required=True)
-    name = serializers.CharField(required=True)
+    value = serializers.CharField(required=True, allow_blank=False, allow_null=False)
+    name = serializers.CharField(required=True, allow_blank=False, allow_null=False)
 
 
 class SentryFunctionSerializer(CamelSnakeSerializer):
@@ -28,19 +28,20 @@ class SentryFunctionSerializer(CamelSnakeSerializer):
     env_variables = serializers.ListField(child=EnvVariableSerializer())
 
     def validate_env_variables(self, env_variables):
+        """
+        Convert env_variables from a list of dicts to a dict of key-value pairs
+        """
         output = {}
         for env_variable in env_variables:
-            # double checking for blanks, but also checked on frontend
-            if env_variable.get("name", None) and env_variable.get("value", None):
-                output[env_variable["name"]] = env_variable["value"]
+            output[env_variable["name"]] = env_variable["value"]
         return output
 
 
 @region_silo_endpoint
 class OrganizationSentryFunctionEndpoint(OrganizationEndpoint):
     publish_status = {
-        "GET": ApiPublishStatus.UNKNOWN,
-        "POST": ApiPublishStatus.UNKNOWN,
+        "GET": ApiPublishStatus.PRIVATE,
+        "POST": ApiPublishStatus.PRIVATE,
     }
 
     # Creating a new sentry function
@@ -53,7 +54,9 @@ def post(self, request, organization):
             return Response(serializer.errors, status=400)
 
         data = serializer.validated_data
-        data["slug"] = slugify(data["name"])
+        # sentry_slugify ensures the slug is not entirely numeric
+        data["slug"] = sentry_slugify(data["name"])
+
         # TODO: Make sure the slug is unique
         # Currently slug unique within organization
         # In future, may add "global_slug" so users can publish their functions

src/sentry/api/endpoints/organization_sentry_function_details.py

@@ -16,9 +16,9 @@
 @region_silo_endpoint
 class OrganizationSentryFunctionDetailsEndpoint(OrganizationEndpoint):
     publish_status = {
-        "DELETE": ApiPublishStatus.UNKNOWN,
-        "GET": ApiPublishStatus.UNKNOWN,
-        "PUT": ApiPublishStatus.UNKNOWN,
+        "DELETE": ApiPublishStatus.PRIVATE,
+        "GET": ApiPublishStatus.PRIVATE,
+        "PUT": ApiPublishStatus.PRIVATE,
     }
 
     def convert_args(self, request, organization_slug, function_slug, *args, **kwargs):

src/sentry/api/serializers/models/sentry_function.py

@@ -6,7 +6,12 @@
 class SentryFunctionSerializer(Serializer):
     def serialize(self, obj, attrs, user):
         events = [event for event in obj.events]
-        env_variables = map(lambda x: {"name": x[0], "value": x[1]}, obj.env_variables.items())
+        env_variables = list(
+            map(
+                lambda env_variable: {"name": env_variable[0], "value": env_variable[1]},
+                obj.env_variables.items(),
+            )
+        )
         data = {
             "name": obj.name,
             "slug": obj.slug,

tests/sentry/api/endpoints/test_organization_sentry_functions.py

@@ -2,80 +2,137 @@
 
 from typing import Any
 from unittest.mock import patch
+from uuid import uuid4
 
-from django.urls import reverse
-
+from sentry.models.sentryfunction import SentryFunction
 from sentry.testutils.cases import APITestCase
-from sentry.testutils.helpers import Feature
+from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.silo import region_silo_test
 
 
-@region_silo_test(stable=True)
-class OrganizationSentryFunctions(APITestCase):
+class OrganizationSentryFunctionBase(APITestCase):
     endpoint = "sentry-api-0-organization-sentry-functions"
 
     def setUp(self):
         super().setUp()
-        self.create_organization(owner=self.user, name="RowdyTiger")
-        self.url = reverse(self.endpoint, args=[self.organization.slug])
         self.login_as(user=self.user)
+        self.code = "exports.yourFunction = (req, res) => {\n\tlet message = req.query.message || req.body.message || 'Hello World!';\n\tconsole.log('Query: ' + req.query);\n\tconsole.log('Body: ' + req.body);\n\tres.status(200).send(message);\n};"
+        self.data: dict[str, Any] = {
+            "name": "foo",
+            "author": "bar",
+            "code": self.code,
+            "overview": "qux",
+        }
+
+
+@region_silo_test(stable=True)
+class OrganizationSentryFunctionsPost(OrganizationSentryFunctionBase):
+    method = "POST"
+
+    def setUp(self):
+        super().setUp()
+        self.login_as(user=self.user)
+        self.data["env_variables"] = [{"name": "foo", "value": "bar"}]
 
+    @with_feature("organizations:sentry-functions")
     @patch("sentry.api.endpoints.organization_sentry_function.create_function")
     def test_post_feature_true(self, mock_func):
-        defaultCode = "exports.yourFunction = (req, res) => {\n\tlet message = req.query.message || req.body.message || 'Hello World!';\n\tconsole.log('Query: ' + req.query);\n\tconsole.log('Body: ' + req.body);\n\tres.status(200).send(message);\n};"
-        data = {
+        response = self.get_success_response(self.organization.slug, status_code=201, **self.data)
+
+        assert response.data == {
             "name": "foo",
+            "slug": "foo",
             "author": "bar",
-            "code": defaultCode,
+            "code": self.code,
             "overview": "qux",
-            "envVariables": [{"name": "foo", "value": "bar"}],
+            # skip checking external id because it has a random suffix
+            "external_id": response.data["external_id"],
+            "events": [],
+            "env_variables": [{"name": "foo", "value": "bar"}],
         }
-        with Feature("organizations:sentry-functions"):
-            response = self.client.post(self.url, data)
-            assert response.status_code == 201
-            assert response.data["name"] == "foo"
-            assert response.data["author"] == "bar"
-            assert response.data["code"] == defaultCode
-            assert response.data["overview"] == "qux"
-            mock_func.assert_called_once_with(
-                defaultCode, response.data["external_id"], "qux", {"foo": "bar"}
-            )
 
+        mock_func.assert_called_once_with(
+            self.code, response.data["external_id"], "qux", {"foo": "bar"}
+        )
+
+    @with_feature("organizations:sentry-functions")
+    @patch("sentry.api.endpoints.organization_sentry_function.create_function")
+    def test_generated_slug_not_entirely_numeric(self, mock_func):
+        data = {**self.data, "name": "123"}
+        response = self.get_success_response(self.organization.slug, status_code=201, **data)
+
+        assert response.data["name"] == "123"
+        assert response.data["author"] == "bar"
+        assert response.data["code"] == self.code
+        assert response.data["overview"] == "qux"
+
+        slug = response.data["slug"]
+        assert not slug.isdecimal()
+        assert slug.startswith("123-")
+
+        mock_func.assert_called_once_with(
+            self.code, response.data["external_id"], "qux", {"foo": "bar"}
+        )
+
+    @with_feature("organizations:sentry-functions")
     def test_post_missing_params(self):
-        data: dict[str, Any] = {"name": "foo", "overview": "qux"}
-        with Feature("organizations:sentry-functions"):
-            response = self.client.post(self.url, **data)
-            assert response.status_code == 400
+        data = {"name": "foo", "overview": "qux"}
+        self.get_error_response(self.organization.slug, status_code=400, **data)
 
     def test_post_feature_false(self):
-        data: dict[str, Any] = {"name": "foo", "author": "bar"}
-        response = self.client.post(self.url, **data)
-        assert response.status_code == 404
+        data = {"name": "foo", "author": "bar"}
+        response = self.get_error_response(self.organization.slug, status_code=404, **data)
+        assert response.data == "organizations:sentry-functions flag set to false"
 
-    def test_get(self):
-        with Feature("organizations:sentry-functions"):
-            response = self.client.get(self.url)
-            assert response.status_code == 200
-            assert response.data == []
 
-    @patch("sentry.api.endpoints.organization_sentry_function.create_function")
-    def test_get_with_function(self, mock_func):
-        defaultCode = "exports.yourFunction = (req, res) => {\n\tlet message = req.query.message || req.body.message || 'Hello World!';\n\tconsole.log('Query: ' + req.query);\n\tconsole.log('Body: ' + req.body);\n\tres.status(200).send(message);\n};"
-        data = {
+@region_silo_test(stable=True)
+class OrganizationSentryFunctionsGet(OrganizationSentryFunctionBase):
+    endpoint = "sentry-api-0-organization-sentry-functions"
+    method = "GET"
+
+    def setUp(self):
+        super().setUp()
+        self.login_as(user=self.user)
+        self.post_data = {
+            **self.data,
+            "slug": "foo",
+            "organization_id": self.organization.id,
+            "external_id": "foo-" + uuid4().hex,
+        }
+
+    @with_feature("organizations:sentry-functions")
+    def test_get_empty(self):
+        response = self.get_success_response(self.organization.slug, status_code=200)
+        assert response.data == []
+
+    @with_feature("organizations:sentry-functions")
+    def test_get_with_function(self):
+        SentryFunction.objects.create(**self.post_data)
+        response = self.get_success_response(self.organization.slug, status_code=200)
+        assert response.data[0] == {
+            "name": "foo",
+            "slug": "foo",
+            "author": "bar",
+            "code": self.code,
+            "overview": "qux",
+            "external_id": self.post_data["external_id"],
+            "events": [],
+            "env_variables": [],
+        }
+
+    @with_feature("organizations:sentry-functions")
+    def test_get_with_function_and_env_variables(self):
+        # env_variables is expected to be a single dict of key-value pairs if
+        # you're directly creating a SentryFunction object using .create()
+        SentryFunction.objects.create(**self.post_data, env_variables={"foo": "bar", "baz": "qux"})
+        response = self.get_success_response(self.organization.slug, status_code=200)
+        assert response.data[0] == {
             "name": "foo",
+            "slug": "foo",
             "author": "bar",
-            "code": defaultCode,
+            "code": self.code,
             "overview": "qux",
-            "envVariables": [{"name": "foo", "value": "bar"}],
+            "external_id": self.post_data["external_id"],
+            "events": [],
+            "env_variables": [{"name": "foo", "value": "bar"}, {"name": "baz", "value": "qux"}],
         }
-        with Feature("organizations:sentry-functions"):
-            self.client.post(self.url, data)
-            response = self.client.get(self.url)
-            assert response.status_code == 200
-            assert response.data[0]["name"] == "foo"
-            assert response.data[0]["author"] == "bar"
-            assert response.data[0]["code"] == defaultCode
-            assert response.data[0]["overview"] == "qux"
-            mock_func.assert_called_once_with(
-                defaultCode, response.data[0]["external_id"], "qux", {"foo": "bar"}
-            )