fix(eco): Fixes prevent webhook forwarding body format (#103119)

Author: GabeVillalobos

src/sentry/integrations/github/webhook_types.py

@@ -1,6 +1,8 @@
 from enum import StrEnum
 
 GITHUB_WEBHOOK_TYPE_HEADER = "HTTP_X_GITHUB_EVENT"
+GITHUB_WEBHOOK_TYPE_HEADER_KEY = "X-GITHUB-EVENT"
+GITHUB_INSTALLATION_TARGET_ID_HEADER = "X-GITHUB-HOOK-INSTALLATION-TARGET-ID"
 
 
 class GithubWebhookType(StrEnum):

src/sentry/middleware/integrations/parsers/github.py

@@ -106,7 +106,7 @@ def get_response(self) -> HttpResponseBase:
 
         # The overwatch forwarder implements its own region-based checks
         OverwatchGithubWebhookForwarder(integration=integration).forward_if_applicable(
-            event=event, headers=self.request.META
+            event=event, headers=self.request.headers
         )
 
         return response

src/sentry/overwatch_webhooks/webhook_forwarder.py

@@ -6,7 +6,11 @@
 
 from sentry import options
 from sentry.constants import ObjectStatus
-from sentry.integrations.github.webhook_types import GITHUB_WEBHOOK_TYPE_HEADER, GithubWebhookType
+from sentry.integrations.github.webhook_types import (
+    GITHUB_INSTALLATION_TARGET_ID_HEADER,
+    GITHUB_WEBHOOK_TYPE_HEADER_KEY,
+    GithubWebhookType,
+)
 from sentry.integrations.models.integration import Integration
 from sentry.integrations.models.organization_integration import OrganizationIntegration
 from sentry.models.organizationmapping import OrganizationMapping
@@ -26,10 +30,6 @@
 }
 
 
-GITHUB_INSTALLATION_TARGET_ID_HEADER = "X-GitHub-Hook-Installation-Target-ID"
-DJANGO_HTTP_GITHUB_INSTALLATION_TARGET_ID_HEADER = "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID"
-
-
 @dataclass(frozen=True)
 class OverwatchOrganizationContext:
     organization_integration: OrganizationIntegration
@@ -51,7 +51,7 @@ def __init__(self, integration: Integration):
         self.integration = integration
 
     def should_forward_to_overwatch(self, headers: Mapping[str, str]) -> bool:
-        event_type = headers.get(GITHUB_WEBHOOK_TYPE_HEADER)
+        event_type = headers.get(GITHUB_WEBHOOK_TYPE_HEADER_KEY)
         verbose_log(
             "overwatch.debug.should_forward_to_overwatch.checked",
             extra={
@@ -163,7 +163,7 @@ def forward_if_applicable(self, event: Mapping[str, Any], headers: Mapping[str,
 
                 raw_app_id = headers.get(
                     GITHUB_INSTALLATION_TARGET_ID_HEADER,
-                ) or headers.get(DJANGO_HTTP_GITHUB_INSTALLATION_TARGET_ID_HEADER)
+                )
                 verbose_log(
                     "overwatch.debug.raw_app_id",
                     extra={"region_name": region_name, "raw_app_id": raw_app_id},
@@ -179,10 +179,12 @@ def forward_if_applicable(self, event: Mapping[str, Any], headers: Mapping[str,
                         )
                         app_id = None
 
+                formatted_headers = {k: v for k, v in headers.items()}
+
                 webhook_detail = WebhookDetails(
                     organizations=org_summaries,
                     webhook_body=dict(event),
-                    webhook_headers=headers,
+                    webhook_headers=formatted_headers,
                     integration_provider=self.integration.provider,
                     region=region_name,
                     app_id=app_id,

tests/sentry/middleware/integrations/parsers/test_github.py

@@ -1,3 +1,4 @@
+import orjson
 import pytest
 import responses
 from django.db import router, transaction
@@ -17,7 +18,7 @@
 from sentry.testutils.helpers.options import override_options
 from sentry.testutils.outbox import assert_no_webhook_payloads, assert_webhook_payloads_for_mailbox
 from sentry.testutils.region import override_regions
-from sentry.testutils.silo import control_silo_test
+from sentry.testutils.silo import control_silo_test, create_test_regions
 from sentry.types.region import Region, RegionCategory
 
 region = Region("us", 1, "https://us.testserver", RegionCategory.MULTI_TENANT)
@@ -304,3 +305,115 @@ class GithubRequestParserTypeRoutingTest(GithubRequestParserTest):
     def setup(self):
         with override_options({"github.webhook-type-routing.enabled": True}):
             yield
+
+
+@control_silo_test(regions=create_test_regions("us"))
+class GithubRequestParserOverwatchForwarderTest(TestCase):
+    factory = RequestFactory()
+    path = reverse("sentry-integration-github-webhook")
+
+    @pytest.fixture(autouse=True)
+    def setup(self):
+        with override_options({"github.webhook-type-routing.enabled": True}):
+            yield
+
+    def setUp(self) -> None:
+        super().setUp()
+        self.organization = self.create_organization(
+            name="Test Org",
+            slug="test-org",
+            region="us",
+            owner=self.create_user(email="test@example.com"),
+        )
+        self.integration = self.create_integration(
+            provider="github",
+            external_id="1",
+            name="Test Integration",
+            organization=self.organization,
+        )
+
+    @responses.activate
+    def test_overwatch_forwarder(self) -> None:
+        with (
+            override_options({"overwatch.enabled-regions": ["us"]}),
+            override_settings(
+                OVERWATCH_REGION_URLS={"us": "https://us.example.com/api"},
+                OVERWATCH_WEBHOOK_SECRET="test-secret",
+            ),
+        ):
+            responses.add(
+                responses.POST,
+                "https://us.example.com/api/webhooks/sentry",
+                status=200,
+            )
+
+            request = self.factory.post(
+                self.path,
+                data={"installation": {"id": "1"}, "action": "created"},
+                content_type="application/json",
+                headers={
+                    "x-github-event": GithubWebhookType.PULL_REQUEST.value,
+                    "x-github-hook-installation-target-id": "123",
+                },
+            )
+            parser = GithubRequestParser(
+                request=request,
+                response_handler=lambda _: HttpResponse(status=200, content="passthrough"),
+            )
+
+            response = parser.get_response()
+            assert isinstance(response, HttpResponse)
+            assert response.status_code == status.HTTP_202_ACCEPTED
+            assert response.content == b""
+
+            assert len(responses.calls) == 1
+            assert responses.calls[0].request.url == "https://us.example.com/api/webhooks/sentry"
+            assert responses.calls[0].request.method == "POST"
+            json_body = orjson.loads(responses.calls[0].request.body)
+
+            assert json_body["organizations"] == [
+                {
+                    "name": "Test Org",
+                    "slug": "test-org",
+                    "id": self.organization.id,
+                    "region": "us",
+                    "github_integration_id": self.integration.id,
+                    "organization_integration_id": self.organization_integration.id,
+                }
+            ]
+            assert json_body["webhook_body"] == {"installation": {"id": "1"}, "action": "created"}
+            assert json_body["app_id"] == 123
+            assert json_body["webhook_headers"]["X-Github-Event"] == "pull_request"
+            assert json_body["integration_provider"] == "github"
+            assert json_body["region"] == "us"
+            assert json_body["event_type"] == "github"
+
+    @responses.activate
+    def test_overwatch_forwarder_missing_region_config(self) -> None:
+        with (
+            override_options({"overwatch.enabled-regions": ["us"]}),
+            override_settings(
+                OVERWATCH_REGION_URLS={"de": "https://de.example.com/api"},
+                OVERWATCH_WEBHOOK_SECRET="test-secret",
+            ),
+        ):
+            request = self.factory.post(
+                self.path,
+                data={"installation": {"id": "1"}, "action": "created"},
+                content_type="application/json",
+                headers={
+                    "x-github-event": GithubWebhookType.PULL_REQUEST.value,
+                    "x-github-hook-installation-target-id": "1",
+                },
+            )
+            parser = GithubRequestParser(
+                request=request,
+                response_handler=lambda _: HttpResponse(status=200, content="passthrough"),
+            )
+
+            response = parser.get_response()
+            assert isinstance(response, HttpResponse)
+            assert response.status_code == status.HTTP_202_ACCEPTED
+            assert response.content == b""
+
+            assert len(responses.calls) == 0

tests/sentry/overwatch_webhooks/test_webhook_forwarder.py

@@ -6,6 +6,10 @@
 from django.test.utils import override_settings
 
 from sentry.hybridcloud.models.outbox import outbox_context
+from sentry.integrations.github.webhook_types import (
+    GITHUB_INSTALLATION_TARGET_ID_HEADER,
+    GITHUB_WEBHOOK_TYPE_HEADER_KEY,
+)
 from sentry.integrations.models.organization_integration import OrganizationIntegration
 from sentry.overwatch_webhooks.types import (
     DEFAULT_REQUEST_TYPE,
@@ -46,15 +50,15 @@ def test_init_creates_publisher_with_correct_provider(self):
 
     def test_should_forward_to_overwatch_with_valid_events(self):
         for event_action in GITHUB_EVENTS_TO_FORWARD_OVERWATCH:
-            headers = {"HTTP_X_GITHUB_EVENT": event_action}
+            headers = {GITHUB_WEBHOOK_TYPE_HEADER_KEY: event_action}
             assert self.forwarder.should_forward_to_overwatch(headers) is True
 
     def test_should_forward_to_overwatch_with_invalid_events(self):
         invalid_events = [
-            {"HTTP_X_GITHUB_EVENT": "invalid_action"},
-            {"HTTP_X_GITHUB_EVENT": "create"},
-            {"HTTP_X_GITHUB_EVENT": "delete"},
-            {"HTTP_X_GITHUB_EVENT": "some_other_action"},
+            {GITHUB_WEBHOOK_TYPE_HEADER_KEY: "invalid_action"},
+            {GITHUB_WEBHOOK_TYPE_HEADER_KEY: "create"},
+            {GITHUB_WEBHOOK_TYPE_HEADER_KEY: "delete"},
+            {GITHUB_WEBHOOK_TYPE_HEADER_KEY: "some_other_action"},
             {},
         ]
 
@@ -128,7 +132,9 @@ def test_forward_if_applicable_no_organizations(self):
         event = {"action": "push", "data": "test"}
 
         with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
-            self.forwarder.forward_if_applicable(event, headers={"HTTP_X_GITHUB_EVENT": "push"})
+            self.forwarder.forward_if_applicable(
+                event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "push"}
+            )
             mock_enqueue.assert_not_called()
 
     @override_options({"overwatch.enabled-regions": ["us"]})
@@ -143,7 +149,7 @@ def test_forward_if_applicable_event_not_eligible_for_forwarding(self):
 
         with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
             self.forwarder.forward_if_applicable(
-                event, headers={"HTTP_X_GITHUB_EVENT": "invalid_action"}
+                event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "invalid_action"}
             )
             mock_enqueue.assert_not_called()
 
@@ -161,8 +167,8 @@ def test_forward_if_applicable_successful_forwarding(self):
             self.forwarder.forward_if_applicable(
                 event,
                 headers={
-                    "HTTP_X_GITHUB_EVENT": "pull_request",
-                    "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
+                    GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
+                    GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
                 },
             )
 
@@ -188,7 +194,7 @@ def test_forward_if_applicable_multiple_organizations(self):
 
         with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
             self.forwarder.forward_if_applicable(
-                event, headers={"HTTP_X_GITHUB_EVENT": "pull_request"}
+                event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request"}
             )
 
             mock_enqueue.assert_called_once()
@@ -213,7 +219,7 @@ def test_forward_if_applicable_all_valid_event_actions(self):
 
             with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
                 self.forwarder.forward_if_applicable(
-                    event, headers={"HTTP_X_GITHUB_EVENT": event_type}
+                    event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: event_type}
                 )
                 mock_enqueue.assert_called_once()
 
@@ -246,7 +252,7 @@ def test_forward_if_applicable_preserves_webhook_body_data(self):
 
         with patch.object(OverwatchWebhookPublisher, "enqueue_webhook") as mock_enqueue:
             self.forwarder.forward_if_applicable(
-                complex_event, headers={"HTTP_X_GITHUB_EVENT": "pull_request"}
+                complex_event, headers={GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request"}
             )
 
             mock_enqueue.assert_called_once()
@@ -294,8 +300,8 @@ def test_forwards_to_correct_regions(self):
         self.forwarder.forward_if_applicable(
             event,
             headers={
-                "HTTP_X_GITHUB_EVENT": "pull_request",
-                "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
+                GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
+                GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
             },
         )
 
@@ -319,8 +325,8 @@ def test_forwards_to_correct_regions(self):
             ],
             "webhook_body": event,
             "webhook_headers": {
-                "HTTP_X_GITHUB_EVENT": "pull_request",
-                "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
+                GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
+                GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
             },
             "integration_provider": "github",
             "region": "us",
@@ -342,8 +348,8 @@ def test_forwards_to_correct_regions(self):
             ],
             "webhook_body": event,
             "webhook_headers": {
-                "HTTP_X_GITHUB_EVENT": "pull_request",
-                "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
+                GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
+                GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
             },
             "integration_provider": "github",
             "region": "de",
@@ -385,8 +391,8 @@ def test_forwards_conditionally_to_some_regions(self):
         self.forwarder.forward_if_applicable(
             event,
             headers={
-                "HTTP_X_GITHUB_EVENT": "pull_request",
-                "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
+                GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
+                GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
             },
         )
 
@@ -407,8 +413,8 @@ def test_forwards_conditionally_to_some_regions(self):
             ],
             "webhook_body": event,
             "webhook_headers": {
-                "HTTP_X_GITHUB_EVENT": "pull_request",
-                "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID": "987654",
+                GITHUB_WEBHOOK_TYPE_HEADER_KEY: "pull_request",
+                GITHUB_INSTALLATION_TARGET_ID_HEADER: "987654",
             },
             "integration_provider": "github",
             "region": "us",