fix(event-endpoints): Validate event_ids being passed (#25031)

* fix(event-endpoints): Validate event_ids being passed

- This adds event_id validation to a few endpoints, when possible doing
  this via the URL so we just 404 before running any code
- This is to fix a bug on the eventid endpoint where a 32 character
  string was being treated as an event_id
    - Fixes SENTRY-PH0
- Also snuba already allows for ids to have an arbitrary number of
  dashes so something like "a"*32 + "-"*100 would actually work, but
  re-using 32-36 characters seems like a reasonable limit

src/sentry/api/endpoints/organization_event_details.py

@@ -8,6 +8,7 @@ from sentry.models.project import Project, ProjectStatus
 
 class OrganizationEventDetailsEndpoint(OrganizationEventsEndpointBase):
     def get(self, request, organization, project_slug, event_id):
+        """event_id is validated by a regex in the URL"""
         if not self.has_feature(organization, request):
             return Response(status=404)
 

src/sentry/api/endpoints/organization_eventid.py

@@ -19,13 +19,10 @@ class EventIdLookupEndpoint(OrganizationEndpoint):
 
         :pparam string organization_slug: the slug of the organization the
                                           event ID should be looked up in.
-        :param string event_id: the event ID to look up.
+        :param string event_id: the event ID to look up. validated by a
+                                regex in the URL.
         :auth: required
         """
-        # Largely copied from ProjectGroupIndexEndpoint
-        if len(event_id) != 32:
-            return Response({"detail": "Event ID must be 32 characters."}, status=400)
-
         project_slugs_by_id = dict(
             Project.objects.filter(organization=organization).values_list("id", "slug")
         )

src/sentry/api/endpoints/organization_events_trace.py

@@ -10,6 +10,7 @@ from sentry import eventstore, features
 from sentry.api.bases import NoProjects, OrganizationEventsV2EndpointBase
 from sentry.api.serializers.models.event import get_tags_with_meta
 from sentry.snuba import discover
+from sentry.utils.validators import is_event_id, INVALID_EVENT_DETAILS
 
 logger = logging.getLogger(__name__)
 MAX_TRACE_SIZE = 100
@@ -94,6 +95,10 @@ class OrganizationEventsTraceEndpointBase(OrganizationEventsV2EndpointBase):
         detailed = request.GET.get("detailed", "0") == "1"
         event_id = request.GET.get("event_id")
 
+        # Only need to validate event_id as trace_id is validated in the URL
+        if event_id and not is_event_id(event_id):
+            return Response({"detail": INVALID_EVENT_DETAILS.format("Event")}, status=400)
+
         # selected_columns is a set list, since we only want to include the minimum to render the trace
         selected_columns = [
             "id",

src/sentry/api/urls.py

@@ -796,7 +796,7 @@ urlpatterns = [
                     name="sentry-api-0-short-id-lookup",
                 ),
                 url(
-                    r"^(?P<organization_slug>[^\/]+)/eventids/(?P<event_id>[^\/]+)/$",
+                    r"^(?P<organization_slug>[^\/]+)/eventids/(?P<event_id>(?:\d+|[A-Fa-f0-9-]{32,36}))/$",
                     EventIdLookupEndpoint.as_view(),
                     name="sentry-api-0-event-id-lookup",
                 ),
@@ -942,12 +942,12 @@ urlpatterns = [
                     name="sentry-api-0-organization-event-baseline",
                 ),
                 url(
-                    r"^(?P<organization_slug>[^\/]+)/events-trace-light/(?P<trace_id>[^\/]+)/$",
+                    r"^(?P<organization_slug>[^\/]+)/events-trace-light/(?P<trace_id>(?:\d+|[A-Fa-f0-9-]{32,36}))/$",
                     OrganizationEventsTraceLightEndpoint.as_view(),
                     name="sentry-api-0-organization-events-trace-light",
                 ),
                 url(
-                    r"^(?P<organization_slug>[^\/]+)/events-trace/(?P<trace_id>[^\/]+)/$",
+                    r"^(?P<organization_slug>[^\/]+)/events-trace/(?P<trace_id>(?:\d+|[A-Fa-f0-9-]{32,36}))/$",
                     OrganizationEventsTraceEndpoint.as_view(),
                     name="sentry-api-0-organization-events-trace",
                 ),

src/sentry/utils/validators.py

@@ -4,6 +4,9 @@ import uuid
 from django.utils.encoding import force_text
 
 
+INVALID_EVENT_DETAILS = "{} ID must be a valid UUID hex (32-36 characters long, containing only digits, dashes, or a-f characters)"
+
+
 def validate_ip(value, required=True):
     if not required and not value:
         return

tests/snuba/api/endpoints/test_organization_event_details.py

@@ -1,6 +1,6 @@
 from datetime import timedelta
 
-from django.core.urlresolvers import reverse
+from django.core.urlresolvers import reverse, NoReverseMatch
 
 from sentry.models import Group
 from sentry.testutils import APITestCase, SnubaTestCase
@@ -175,6 +175,17 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
 
         assert response.status_code == 404, response.content
 
+    def test_invalid_event_id(self):
+        with self.assertRaises(NoReverseMatch):
+            reverse(
+                "sentry-api-0-organization-event-details",
+                kwargs={
+                    "organization_slug": self.project.organization.slug,
+                    "project_slug": self.project.slug,
+                    "event_id": "not-an-event",
+                },
+            )
+
     def test_long_trace_description(self):
         data = load_data("transaction")
         data["event_id"] = "d" * 32

tests/snuba/api/endpoints/test_organization_eventid.py

@@ -1,4 +1,4 @@
-from django.core.urlresolvers import reverse
+from django.core.urlresolvers import reverse, NoReverseMatch
 
 from sentry.testutils import APITestCase, SnubaTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
@@ -61,3 +61,13 @@ class EventIdLookupEndpointTest(APITestCase, SnubaTestCase):
         resp = self.client.get(url, format="json")
 
         assert resp.status_code == 429
+
+    def test_invalid_event_id(self):
+        with self.assertRaises(NoReverseMatch):
+            reverse(
+                "sentry-api-0-event-id-lookup",
+                kwargs={
+                    "organization_slug": self.org.slug,
+                    "event_id": "not-an-event",
+                },
+            )

tests/snuba/api/endpoints/test_organization_events_trace.py

@@ -1,7 +1,7 @@
 from datetime import timedelta
 from uuid import uuid4
 
-from django.core.urlresolvers import reverse
+from django.core.urlresolvers import reverse, NoReverseMatch
 
 from sentry.testutils import APITestCase, SnubaTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
@@ -192,6 +192,16 @@ class OrganizationEventsTraceLightEndpointTest(OrganizationEventsTraceEndpointBa
 
         assert response.status_code == 404, response.content
 
+        # Invalid event id
+        with self.feature(self.FEATURES):
+            response = self.client.get(
+                self.url,
+                data={"event_id": "not-a-event"},
+                format="json",
+            )
+
+        assert response.status_code == 400, response.content
+
         # Fake trace id
         self.url = reverse(
             "sentry-api-0-organization-events-trace-light",
@@ -207,6 +217,16 @@ class OrganizationEventsTraceLightEndpointTest(OrganizationEventsTraceEndpointBa
 
         assert response.status_code == 404, response.content
 
+        # Invalid trace id
+        with self.assertRaises(NoReverseMatch):
+            self.url = reverse(
+                "sentry-api-0-organization-events-trace-light",
+                kwargs={
+                    "organization_slug": self.project.organization.slug,
+                    "trace_id": "not-a-trace",
+                },
+            )
+
     def test_no_roots(self):
         """ Even when there's no root, we return the current event """
         no_root_trace = uuid4().hex