feat(webhooks): Add payload validation during dual-write migration #116040

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Christinarlong merged 16 commits into master from crl/webhook-payload-validation-usage

May 26, 2026

src/sentry/notifications/notification_action/action_handler_registry/webhook_handler.py

-Original file line number
+Diff line change
@@ Expand Up / @@ -3,11 +3,15 @@ @@
     from sentry import features
     from sentry.notifications.notification_action.utils import execute_via_group_type_registry
+    from sentry.options.rollout import in_random_rollout
+    from sentry.plugins.sentry_webhooks.plugin import WebHooksPlugin
     from sentry.sentry_apps.services.legacy_webhook.service import (
+        build_legacy_webhook_payload,
         get_triggering_rule_name,
         send_legacy_webhooks_for_invocation,
         send_sentry_app_webhook,
     )
+    from sentry.sentry_apps.services.legacy_webhook.validation import validate_payload_equivalence
     from sentry.services.eventstore.models import GroupEvent
     from sentry.workflow_engine.models import Action
     from sentry.workflow_engine.registry import action_handler_registry
@@ Expand All / @@ -16,6 +20,22 @@ @@
     logger = logging.getLogger(__name__)
+    def _validate_webhook_payloads(invocation: ActionInvocation) -> None:
+        group = invocation.event_data.group
+        event = invocation.event_data.event
+        rule_name = get_triggering_rule_name(invocation)
+        old_payload = WebHooksPlugin().get_group_data(group, event, [rule_name])
+        new_payload = build_legacy_webhook_payload(invocation)
+        validate_payload_equivalence(
+            old_payload,
+            new_payload,
+            organization_id=invocation.detector.project.organization_id,
+            project_id=invocation.detector.project.id,
+        )
     @action_handler_registry.register(Action.Type.WEBHOOK)
     class WebhookActionHandler(ActionHandler):
         group = ActionHandler.Group.OTHER
@@ Expand Down Expand Up / @@ -63,6 +83,12 @@ def execute(invocation: ActionInvocation) -> None: @@
                 target_identifier = invocation.action.config.get("target_identifier")
                 if target_identifier == "webhooks":
                     send_legacy_webhooks_for_invocation(invocation)
+                    if in_random_rollout("sentry-apps.legacy-webhook-payload-validation.rate"):
+                        try:
+                            _validate_webhook_payloads(invocation)
+                        except Exception:
+                            logger.exception("webhook_action_handler.validation_error")
                 else:
                     send_sentry_app_webhook(
                         group_event=invocation.event_data.event,
@@ Expand Down @@

src/sentry/options/defaults.py

-Original file line number
+Diff line change
@@ Expand Up / @@ -4100,6 +4100,14 @@ @@
         flags=FLAG_AUTOMATOR_MODIFIABLE,
     )
+    # Rollout rate for legacy webhook payload validation (0.0 to 1.0)
+    register(
+        "sentry-apps.legacy-webhook-payload-validation.rate",
+        type=Float,
+        default=0.0,
+        flags=FLAG_AUTOMATOR_MODIFIABLE,
+    )
     # Killswitch for web vital issue detection
     register(
         "issue-detection.web-vitals-detection.enabled",
@@ Expand Down @@

src/sentry/sentry_apps/services/legacy_webhook/tasks.py

-Original file line number
+Diff line change
@@ Expand Up @@
         organization = group.project.organization
         if features.has("organizations:legacy-webhook-dry-run", organization):
-            logger.info(
-                "legacy_webhook.dry_run",
-                extra={
-                    "url": url,
-                    "payload": payload,
-                },
-            )
             metrics.incr(
                 "legacy_webhook.task.result",
                 tags={"outcome": LegacyWebhookOutcome.DRY_RUN},
@@ Expand Down @@

src/sentry/sentry_apps/services/legacy_webhook/validation.py

-Original file line number
+Diff line change
@@ -0,0 +1,40 @@
+    from __future__ import annotations
+    import logging
+    from collections.abc import Mapping
+    from typing import Any
+    from sentry.utils.payload_comparison import ParityChecker, describe_value
+    logger = logging.getLogger("sentry.legacy_webhook")
+    def compare_payloads(old_payload: Mapping[str, Any], new_payload: Mapping[str, Any]) -> list[str]:
+        comparator = ParityChecker(format_value=describe_value)
+        comparator.compare(old_payload, new_payload, frozenset())
+        return comparator.mismatches
+    def validate_payload_equivalence(
+        old_payload: Mapping[str, Any],
+        new_payload: Mapping[str, Any],
+        organization_id: int,
+        project_id: int,
+    ) -> None:
+        logging_context = {"organization_id": organization_id, "project_id": project_id}
+        if old_payload == new_payload:
+            logger.info("legacy_webhook.validation.match", extra=logging_context)
+            return
+        try:
+            mismatches = compare_payloads(old_payload, new_payload)
+        except Exception:
+            logger.exception("legacy_webhook.validation.comparison_error", extra=logging_context)
+            return
+        if mismatches:
+            logger.warning(
+                "legacy_webhook.validation.payload_mismatch",
+                extra={**logging_context, "mismatches": mismatches},
+            )

.../sentry/notifications/notification_action/action_handler_registry/test_webhook_handler.py

-Original file line number
+Diff line change
@@ Expand Up / @@ -78,8 +78,7 @@ def test_new_path_disable_old_fires_new_only(self) -> None: @@
             assert body["id"] == str(self.group.id)
         @responses.activate
-        @mock.patch("sentry.sentry_apps.services.legacy_webhook.tasks.logger")
-        def test_new_path_dry_run_logs_instead_of_sending(self, mock_logger: mock.MagicMock) -> None:
+        def test_new_path_dry_run_does_not_send(self) -> None:
             responses.add(responses.POST, "http://example.com/hook")
             with (
@@ Expand All @@
                 WebhookActionHandler.execute(self.invocation)
             assert len(responses.calls) == 0
-            mock_logger.info.assert_called_once()
-            assert mock_logger.info.call_args[0][0] == "legacy_webhook.dry_run"
         @responses.activate
         def test_disable_old_without_new_path_fires_nothing(self) -> None:
@@ Expand Down @@

tests/sentry/sentry_apps/services/legacy_webhook/test_tasks.py

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -1,5 +1,4 @@
  
    import uuid

    from unittest import mock

    import responses

    @@ -46,17 +45,11 @@ def test_task_sends_webhook(self) -> None:
  
            assert body["message"] == self.group_event.message

        @responses.activate

        @mock.patch("sentry.sentry_apps.services.legacy_webhook.tasks.logger")

        @with_feature("organizations:legacy-webhook-dry-run")

        def test_task_dry_run_logs_instead_of_sending(self, mock_logger: mock.MagicMock) -> None:

        def test_task_dry_run_does_not_send(self) -> None:

            responses.add(responses.POST, "http://example.com/hook")

            payload = build_legacy_webhook_payload(self.invocation)

            send_legacy_webhook_task(url="http://example.com/hook", payload=payload)

            assert len(responses.calls) == 0

            mock_logger.info.assert_called_once()

            call_args = mock_logger.info.call_args

            assert call_args[0][0] == "legacy_webhook.dry_run"

            assert call_args[1]["extra"]["url"] == "http://example.com/hook"

            assert call_args[1]["extra"]["payload"] == payload

tests/sentry/sentry_apps/services/legacy_webhook/test_validation.py

Christinarlong marked this conversation as resolved.

-Original file line number
+Diff line change
@@ -0,0 +1,124 @@
+    from __future__ import annotations
+    import uuid
+    from typing import Any
+    from unittest import mock
+    from sentry.plugins.sentry_webhooks.plugin import WebHooksPlugin
+    from sentry.sentry_apps.services.legacy_webhook.service import (
+        build_legacy_webhook_payload,
+        get_triggering_rule_name,
+    )
+    from sentry.sentry_apps.services.legacy_webhook.validation import (
+        compare_payloads,
+        validate_payload_equivalence,
+    )
+    from sentry.workflow_engine.models import Action
+    from sentry.workflow_engine.types import ActionInvocation, WorkflowEventData
+    from tests.sentry.workflow_engine.test_base import BaseWorkflowTest
+    class TestWebhookPayloadValidation(BaseWorkflowTest):
+        def setUp(self) -> None:
+            super().setUp()
+            self.detector = self.create_detector(project=self.project)
+            self.workflow = self.create_workflow(environment=self.environment)
+            self.action = self.create_action(
+                type=Action.Type.WEBHOOK,
+                config={"target_identifier": "webhooks"},
+            )
+            self.group, self.event, self.group_event = self.create_group_event()
+            self.event_data = WorkflowEventData(
+                event=self.group_event, workflow_env=self.environment, group=self.group
+            )
+            self.invocation = ActionInvocation(
+                event_data=self.event_data,
+                action=self.action,
+                detector=self.detector,
+                notification_uuid=str(uuid.uuid4()),
+                workflow_id=self.workflow.id,
+            )
+        def _build_old_payload(self) -> dict[str, Any]:
+            rule_name = get_triggering_rule_name(self.invocation)
+            return WebHooksPlugin().get_group_data(self.group, self.group_event, [rule_name])
+        def _build_new_payload(self) -> dict[str, Any]:
+            return dict(build_legacy_webhook_payload(self.invocation))
+        def test_old_and_new_payloads_match(self) -> None:
+            old = self._build_old_payload()
+            new = self._build_new_payload()
+            mismatches = compare_payloads(old, new)
+            assert mismatches == []
+        def test_detects_extra_field_in_new_payload(self) -> None:
+            old = self._build_old_payload()
+            new = self._build_new_payload()
+            new["extra_field"] = "surprise"
+            mismatches = compare_payloads(old, new)
+            assert mismatches == ["Extra in new: extra_field"]
+        def test_detects_missing_field_in_new_payload(self) -> None:
+            old = self._build_old_payload()
+            new = self._build_new_payload()
+            del new["culprit"]
+            mismatches = compare_payloads(old, new)
+            assert mismatches == ["Missing from new: culprit"]
+        def test_detects_nested_event_field_difference(self) -> None:
+            old = self._build_old_payload()
+            new = self._build_new_payload()
+            new["event"]["event_id"] = "tampered"
+            mismatches = compare_payloads(old, new)
+            assert mismatches == ["event.event_id: old=str(len=32), new=str(len=8)"]
+        @mock.patch("sentry.sentry_apps.services.legacy_webhook.validation.logger")
+        def test_validate_logs_match_on_identical_payloads(self, mock_logger: mock.MagicMock) -> None:
+            old = self._build_old_payload()
+            new = self._build_new_payload()
+            validate_payload_equivalence(
+                old, new, organization_id=self.project.organization_id, project_id=self.project.id
+            )
+            mock_logger.info.assert_called_once_with(
+                "legacy_webhook.validation.match",
+                extra={
+                    "organization_id": self.project.organization_id,
+                    "project_id": self.project.id,
+                },
+            )
+        @mock.patch("sentry.sentry_apps.services.legacy_webhook.validation.logger")
+        def test_validate_logs_warnings_on_mismatch(self, mock_logger: mock.MagicMock) -> None:
+            old = self._build_old_payload()
+            new = self._build_new_payload()
+            new["extra_field"] = "surprise"
+            validate_payload_equivalence(
+                old, new, organization_id=self.project.organization_id, project_id=self.project.id
+            )
+            mock_logger.warning.assert_called_once()
+            assert mock_logger.warning.call_args[1]["extra"]["mismatches"] == [
+                "Extra in new: extra_field"
+            ]
+        @mock.patch("sentry.sentry_apps.services.legacy_webhook.validation.logger")
+        @mock.patch(
+            "sentry.sentry_apps.services.legacy_webhook.validation.compare_payloads",
+            side_effect=RuntimeError("boom"),
+        )
+        def test_validate_logs_exception_on_comparison_error(
+            self, _mock_compare: mock.MagicMock, mock_logger: mock.MagicMock
+        ) -> None:
+            validate_payload_equivalence(
+                {"a": 1},
+                {"b": 2},
+                organization_id=self.project.organization_id,
+                project_id=self.project.id,
+            )
+            mock_logger.exception.assert_called_once_with(
+                "legacy_webhook.validation.comparison_error",
+                extra={
+                    "organization_id": self.project.organization_id,
+                    "project_id": self.project.id,
+                },
+            )

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(webhooks): Add payload validation during dual-write migration #116040

Uh oh!

Diff view

Diff view

There are no files selected for viewing

GabeVillalobos May 21, 2026

Uh oh!

Christinarlong May 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feat(webhooks): Add payload validation during dual-write migration #116040

Uh oh!

feat(webhooks): Add payload validation during dual-write migration #116040

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

GabeVillalobos May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Christinarlong May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!