fix(webhooks): validate organisation and integration URLs against SSRF

api/features/feature_external_resources/serializers.py

@@ -7,6 +7,10 @@
 
 class FeatureExternalResourceSerializer(serializers.ModelSerializer):  # type: ignore[type-arg]
     metadata = serializers.JSONField(required=False, allow_null=True, default=None)
+    # Not SSRF-relevant: never fetched server-side, only regex-matched and
+    # passed as GitHub API payload data. Overrides the SSRF-safe default so
+    # self-hosted GitHub/GitLab instances on internal hosts still work.
+    url = serializers.URLField()
 
     class Meta:
         model = FeatureExternalResource

api/integrations/webhook/serializers.py

@@ -10,11 +10,14 @@
 )
 from segments.models import Segment
 from util.mappers.engine import map_environment_to_evaluation_context
+from webhooks.fields import NoSSRFURLField
 
 from .models import WebhookConfiguration
 
 
 class WebhookConfigurationSerializer(BaseEnvironmentIntegrationModelSerializer):
+    url = NoSSRFURLField(max_length=200)
+
     class Meta:
         model = WebhookConfiguration
         fields = ("id", "url", "secret")

api/organisations/serializers.py

@@ -12,6 +12,7 @@
 )
 from organisations.invites.models import Invite
 from users.models import FFAdminUser, UserPermissionGroup
+from webhooks.fields import NoSSRFURLField
 
 from .models import (
     Organisation,
@@ -234,6 +235,8 @@ class PortalUrlSerializer(serializers.Serializer):  # type: ignore[type-arg]
 
 
 class OrganisationWebhookSerializer(serializers.ModelSerializer):  # type: ignore[type-arg]
+    url = NoSSRFURLField()
+
     class Meta:
         model = OrganisationWebhook
         fields = ("id", "url", "enabled", "secret", "created_at", "updated_at")

api/tests/unit/integrations/datadog/test_unit_datadog_views.py

@@ -177,3 +177,27 @@ def test_datadog_project_view__no_permissions__return_expected(
     # Then
     assert response.status_code == status.HTTP_403_FORBIDDEN
     assert not DataDogConfiguration.objects.filter(project=project).exists()
+
+
+def test_datadog_config__private_ip_base_url__returns_bad_request(
+    admin_client: APIClient,
+    project: Project,
+) -> None:
+    # Given
+    data = {
+        "base_url": "http://169.254.169.254/",
+        "api_key": "abc-123",
+        "use_custom_source": True,
+    }
+    url = reverse("api-v1:projects:integrations-datadog-list", args=[project.id])
+
+    # When
+    response = admin_client.post(
+        url,
+        data=json.dumps(data),
+        content_type="application/json",
+    )
+
+    # Then
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert not DataDogConfiguration.objects.filter(project=project).exists()

api/tests/unit/integrations/dynatrace/test_unit_dynatrace_views.py

@@ -171,3 +171,30 @@ def test_dynatrace_environment_view__no_permissions__return_expected(
     # Then
     assert response.status_code == status.HTTP_403_FORBIDDEN
     assert not DynatraceConfiguration.objects.filter(environment=environment).exists()
+
+
+def test_create_dynatrace_config__private_ip_base_url__returns_bad_request(
+    admin_client: APIClient,
+    environment: Environment,
+) -> None:
+    # Given
+    data = {
+        "base_url": "http://127.0.0.1/",
+        "api_key": "abc-123",
+        "entity_selector": "type(APPLICATION),entityName(docs)",
+    }
+    url = reverse(
+        "api-v1:environments:integrations-dynatrace-list",
+        args=[environment.api_key],
+    )
+
+    # When
+    response = admin_client.post(
+        url,
+        data=json.dumps(data),
+        content_type="application/json",
+    )
+
+    # Then
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert not DynatraceConfiguration.objects.filter(environment=environment).exists()

api/tests/unit/integrations/gitlab/test_configuration.py

@@ -315,3 +315,22 @@ def test_delete_configuration__non_admin__returns_403(
 
     # Then
     assert response.status_code == status.HTTP_403_FORBIDDEN
+
+
+def test_create_configuration__private_ip_instance_url__returns_400(
+    admin_client_new: APIClient,
+    project: Project,
+) -> None:
+    # Given / When
+    response = admin_client_new.post(
+        f"/api/v1/projects/{project.id}/integrations/gitlab/",
+        data={
+            "gitlab_instance_url": "http://127.0.0.1/",
+            "access_token": "glpat-xxxxxxxxxxxxxxxxxxxx",
+        },
+        format="json",
+    )
+
+    # Then
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert not GitLabConfiguration.objects.filter(project=project).exists()

api/tests/unit/integrations/new_relic/test_unit_new_relic_views.py

@@ -173,3 +173,27 @@ def test_new_relic_config__project_with_deleted_config__creates_new_configuratio
     assert response_json["api_key"] == api_key
     assert response_json["base_url"] == base_url
     assert response_json["app_id"] == app_id
+
+
+def test_new_relic_config__private_ip_base_url__returns_bad_request(
+    admin_client: APIClient,
+    project: Project,
+) -> None:
+    # Given
+    data = {
+        "base_url": "http://127.0.0.1/",
+        "api_key": "key-123",
+        "app_id": "app-123",
+    }
+    url = reverse("api-v1:projects:integrations-new-relic-list", args=[project.id])
+
+    # When
+    response = admin_client.post(
+        url,
+        data=json.dumps(data),
+        content_type="application/json",
+    )
+
+    # Then
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert not NewRelicConfiguration.objects.filter(project=project).exists()

api/tests/unit/integrations/sentry/test_unit_sentry_views.py

@@ -106,3 +106,24 @@ def test_sentry_change_tracking_setup__invalid_payload__rejects_with_400(
     # Then
     assert response.status_code == 400
     assert response.json() == errors
+
+
+def test_sentry_change_tracking_setup__private_ip_webhook_url__rejects_with_400(
+    admin_client: APIClient,
+    environment: Environment,
+) -> None:
+    # Given
+    url = f"/api/v1/environments/{environment.api_key}/integrations/sentry/"
+    payload = {
+        "webhook_url": "http://127.0.0.1/webhook",
+        "secret": "hush hush!",
+    }
+
+    # When
+    response = admin_client.post(url, payload, format="json")
+
+    # Then
+    assert response.status_code == 400
+    assert not SentryChangeTrackingConfiguration.objects.filter(
+        environment=environment
+    ).exists()

api/tests/unit/integrations/webhook/test_unit_webhook_views.py

@@ -116,3 +116,25 @@ def test_delete_webhook_config__existing_config__returns_204(  # type: ignore[no
     # Then
     assert res.status_code == status.HTTP_204_NO_CONTENT
     assert not WebhookConfiguration.objects.filter(environment=environment).exists()
+
+
+def test_create_webhook_config__private_ip_url__returns_400(  # type: ignore[no-untyped-def]
+    admin_client, organisation, environment
+):
+    # Given
+    url = reverse(
+        "api-v1:environments:integrations-webhook-list",
+        args=[environment.api_key],
+    )
+    data = {"url": "http://127.0.0.1/webhooks", "secret": "random_secret"}
+
+    # When
+    response = admin_client.post(
+        url,
+        data=json.dumps(data),
+        content_type="application/json",
+    )
+
+    # Then
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert not WebhookConfiguration.objects.filter(environment=environment).exists()

api/tests/unit/organisations/test_unit_organisations_serializers.py

@@ -1,8 +1,15 @@
+import socket
+from unittest import mock
+
+import pytest
 from pytest_django.fixtures import SettingsWrapper
 from pytest_mock import MockerFixture
 
 from organisations.models import Organisation
-from organisations.serializers import UpdateSubscriptionSerializer
+from organisations.serializers import (
+    OrganisationWebhookSerializer,
+    UpdateSubscriptionSerializer,
+)
 
 
 def test_update_subscription_serializer__create__updates_subscription(
@@ -39,3 +46,62 @@ def test_update_subscription_serializer__create__updates_subscription(
     organisation.subscription.refresh_from_db()
     assert organisation.subscription.subscription_id == "new-sub-id"
     assert organisation.subscription.plan == "startup-v2"
+
+
+def test_organisation_webhook_serializer__private_ip__is_invalid() -> None:
+    # Given
+    serializer = OrganisationWebhookSerializer(data={"url": "http://127.0.0.1/hook"})
+
+    # When
+    is_valid = serializer.is_valid()
+
+    # Then
+    assert is_valid is False
+    assert "internal_address" in str(serializer.errors["url"])
+
+
+def test_organisation_webhook_serializer__hostname_resolving_to_private_ip__is_invalid() -> (  # noqa: E501
+    None
+):
+    # Given — a hostname that resolves to an RFC1918 address
+    serializer = OrganisationWebhookSerializer(
+        data={"url": "http://internal.example.com/hook"}
+    )
+
+    # When
+    with mock.patch(
+        "core.network.socket.getaddrinfo",
+        return_value=[(socket.AF_INET, None, None, None, ("10.0.0.5", 0))],
+    ):
+        is_valid = serializer.is_valid()
+
+    # Then
+    assert is_valid is False
+    assert "internal_address" in str(serializer.errors["url"])
+
+
+def test_organisation_webhook_serializer__non_http_scheme__is_invalid() -> None:
+    # Given
+    serializer = OrganisationWebhookSerializer(data={"url": "ftp://example.com/hook"})
+
+    # When
+    is_valid = serializer.is_valid()
+
+    # Then
+    assert is_valid is False
+    assert "url" in serializer.errors
+
+
+@pytest.mark.parametrize(
+    "url",
+    ["https://example.com/hook", "http://8.8.8.8/hook"],
+)
+def test_organisation_webhook_serializer__public_url__is_valid(url: str) -> None:
+    # Given
+    serializer = OrganisationWebhookSerializer(data={"url": url})
+
+    # When
+    is_valid = serializer.is_valid()
+
+    # Then
+    assert is_valid is True

api/tests/unit/webhooks/test_webhooks_fields.py

@@ -2,11 +2,38 @@
 from unittest import mock
 
 import pytest
+from django.db import models
 from rest_framework.exceptions import ValidationError
+from rest_framework.serializers import ModelSerializer
 
 from webhooks.fields import NoSSRFURLField
 
 
+def test_serializer_field_mapping__model_url_field__maps_to_no_ssrf_url_field() -> None:
+    # Given — registered in `WebhooksAppConfig.ready()`, so any
+    # `ModelSerializer` built from a `models.URLField` gets this field
+    # automatically, without declaring it on each serializer.
+
+    # When / Then
+    assert ModelSerializer.serializer_field_mapping[models.URLField] is NoSSRFURLField
+
+
+def test_serializer_field_mapping__gitlab_instance_url__builds_no_ssrf_url_field() -> (
+    None
+):
+    # Given — `GitLabConfigurationSerializer` relies on the mapping instead
+    # of declaring `gitlab_instance_url` explicitly.
+    from integrations.gitlab.serializers import GitLabConfigurationSerializer
+
+    # When
+    built_field = GitLabConfigurationSerializer().fields["gitlab_instance_url"]
+
+    # Then
+    assert type(built_field) is NoSSRFURLField
+    with pytest.raises(ValidationError):
+        built_field.run_validation("http://127.0.0.1/")
+
+
 @pytest.fixture()
 def field() -> NoSSRFURLField:
     return NoSSRFURLField()
@@ -116,3 +143,24 @@ def test_no_ssrf_url_field__unresolvable_hostname__returns_value(
 
     # Then
     assert result == "https://unresolvable.example.com/hook"
+
+
+@pytest.mark.parametrize(
+    "url,label",
+    [
+        ("ftp://example.com/hook", "ftp"),
+        ("ftps://example.com/hook", "ftps"),
+        ("file:///etc/passwd", "file"),
+        ("gopher://example.com/hook", "gopher"),
+    ],
+)
+def test_no_ssrf_url_field__non_http_scheme__raises_validation_error(  # noqa: FT004
+    field: NoSSRFURLField,
+    url: str,
+    label: str,
+) -> None:
+    # Given / When / Then
+    with pytest.raises(ValidationError) as exc_info:
+        field.run_validation(url)
+
+    assert "invalid" in str(exc_info.value.detail)

api/webhooks/apps.py

@@ -3,3 +3,14 @@
 
 class WebhooksAppConfig(AppConfig):
     name = "webhooks"
+
+    def ready(self) -> None:
+        from django.db import models
+        from rest_framework.serializers import ModelSerializer
+
+        from webhooks.fields import NoSSRFURLField
+
+        # Any `ModelSerializer` field built from a `models.URLField` gets the
+        # SSRF-safe field instead, for every current and future serializer,
+        # without each one having to opt in.
+        ModelSerializer.serializer_field_mapping[models.URLField] = NoSSRFURLField