fix: Handle bad input before checking permission (#5502)

Author: emyller

.github/workflows/conventional-commit.yml

@@ -2,7 +2,9 @@ name: Conventional Commit
 
 on:
   pull_request:
-    types: [opened, synchronize, reopened, ready_for_review]
+    types:
+      - edited
+      - opened
 
 jobs:
   conventional-commit:

api/features/feature_segments/permissions.py

@@ -1,5 +1,3 @@
-from contextlib import suppress
-
 from common.environments.permissions import (
     MANAGE_SEGMENT_OVERRIDES,
 )
@@ -22,13 +20,20 @@ def has_permission(self, request, view):  # type: ignore[no-untyped-def]
             return True
 
         if view.action == "create":
-            with suppress(Environment.DoesNotExist, ValueError):
-                environment = request.data.get("environment")
-                environment = Environment.objects.get(id=int(environment))
-
-                return request.user.has_environment_permission(
-                    MANAGE_SEGMENT_OVERRIDES, environment
-                )
+            try:
+                # Validate here because this evaluates prior to the serializer layer
+                environment_pk = int(request.data.get("environment"))
+            except (TypeError, ValueError):
+                return False
+
+            try:
+                environment = Environment.objects.get(pk=environment_pk)
+            except Environment.DoesNotExist:
+                return False
+
+            return request.user.has_environment_permission(
+                MANAGE_SEGMENT_OVERRIDES, environment
+            )
 
         return False
 

api/tests/unit/features/feature_segments/test_unit_feature_segments_permissions.py

@@ -0,0 +1,37 @@
+from typing import Any, Callable
+
+import pytest
+from pytest_mock import MockerFixture
+
+from environments.models import Environment
+from features.feature_segments import permissions
+
+
+@pytest.mark.parametrize(
+    "get_payload, expected",
+    [
+        (lambda environment: {}, False),
+        (lambda environment: {"environment": "invalid"}, False),
+        (lambda environment: {"environment": 10_000_000}, False),  # Please don't exist
+        (lambda environment: {"environment": environment.pk}, True),
+    ],
+)
+def test_FeatureSegmentPermissions_has_permission__create_action__handles_environment_arg(
+    environment: Environment,
+    expected: bool,
+    mocker: MockerFixture,
+    get_payload: Callable[[Environment], dict[str, Any]],
+) -> None:
+    # Given
+    view = mocker.Mock(action="create", detail=False)
+    request = mocker.Mock(data=get_payload(environment))
+    request.user.has_environment_permission.return_value = True
+
+    # When
+    permission = mocker.Mock(spec=permissions.FeatureSegmentPermissions)
+    result = permissions.FeatureSegmentPermissions.has_permission(  # type: ignore[no-untyped-call]
+        permission, request, view
+    )
+
+    # Then
+    assert result is expected