From 19f774c450eaeaac82ed8401fe892671574c4fd4 Mon Sep 17 00:00:00 2001 From: Madhu Kanoor Date: Wed, 17 Sep 2025 21:03:38 -0400 Subject: [PATCH] fix: handling of HTTP headers in EventStreams To include all headers you have to explicitly set the additional_data_headers = * * If the header key is used for Authentication it will be redacted * To pick and choose which headers get included, list them as a comma delimited string in additional_data_headers e.g Host,Origin,X-Request-ID * Any header that starts with X-Envoy will be dropped --- .../api/views/external_event_stream.py | 51 ++++-- .../api/test_event_stream_token.py | 173 +++++++++++++++--- 2 files changed, 183 insertions(+), 41 deletions(-) diff --git a/src/aap_eda/api/views/external_event_stream.py b/src/aap_eda/api/views/external_event_stream.py index 7e397b9ac..25e597add 100644 --- a/src/aap_eda/api/views/external_event_stream.py +++ b/src/aap_eda/api/views/external_event_stream.py @@ -16,6 +16,7 @@ import datetime import logging import urllib.parse +from typing import Any import yaml from django.conf import settings @@ -46,6 +47,8 @@ from aap_eda.services.pg_notify import PGNotify logger = logging.getLogger(__name__) +UNSAFE_HEADER_KEYS = {"X-Trusted-Proxy", "X-Forwarded-For", "X-Real-IP"} +REDACTED_STRING = "********" class ExternalEventStreamViewSet(viewsets.GenericViewSet): @@ -107,20 +110,34 @@ def _parse_body(self, content_type: str, body: bytes) -> dict: raise ParseError(message) from exc return data - def _create_payload( - self, headers: HttpHeaders, data: dict, header_key: str, endpoint: str - ) -> dict: + def _redacted_headers( + self, headers: HttpHeaders, header_key: str + ) -> dict[str, Any]: event_headers = {} if self.event_stream.additional_data_headers: - for key in self.event_stream.additional_data_headers.split(","): - value = headers.get(key) - if value: - event_headers[key] = value - else: - event_headers = dict(headers) - if header_key in event_headers: - event_headers.pop(header_key) + if self.event_stream.additional_data_headers == "*": + event_headers = dict(headers) + if header_key in event_headers: + event_headers[header_key] = REDACTED_STRING + event_headers = { + key: value + for key, value in event_headers.items() + if not key.startswith("X-Envoy") + and key not in UNSAFE_HEADER_KEYS + } + else: + for key in self.event_stream.additional_data_headers.split( + "," + ): + key = key.strip() + value = headers.get(key) + if value: + event_headers[key] = value + return event_headers + def _create_payload( + self, event_headers: dict, data: dict, endpoint: str + ) -> dict: return { "payload": data, "meta": { @@ -226,7 +243,6 @@ def post(self, request, *_args, **kwargs): except (EventStream.DoesNotExist, ValidationError) as exc: raise ParseError("bad uuid specified") from exc - logger.debug("Headers %s", request.headers) logger.debug("Body %s", request.body) try: @@ -235,13 +251,17 @@ def post(self, request, *_args, **kwargs): logger.warning("Error fetching external secrets %s", str(err)) return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR) + event_headers = self._redacted_headers( + request.headers, inputs["http_header_key"] + ) + if inputs["http_header_key"] not in request.headers: message = f"{inputs['http_header_key']} header is missing" logger.error(message) if self.event_stream.test_mode: self._update_test_data( error_message=message, - headers=yaml.dump(dict(request.headers)), + headers=yaml.dump(event_headers), ) raise ParseError(message) @@ -260,9 +280,8 @@ def post(self, request, *_args, **kwargs): logger.debug("Data: %s", data) payload = self._create_payload( - request.headers, + event_headers, data, - inputs["http_header_key"], request.get_full_path(), ) self._update_stats() @@ -270,7 +289,7 @@ def post(self, request, *_args, **kwargs): self._update_test_data( content=yaml.dump(body), content_type=request.headers.get("Content-Type", "unknown"), - headers=yaml.dump(dict(request.headers)), + headers=yaml.dump(event_headers), ) else: try: diff --git a/tests/integration/api/test_event_stream_token.py b/tests/integration/api/test_event_stream_token.py index 55fdcc856..d5d9a614e 100644 --- a/tests/integration/api/test_event_stream_token.py +++ b/tests/integration/api/test_event_stream_token.py @@ -19,6 +19,10 @@ from rest_framework import status from rest_framework.test import APIClient +from aap_eda.api.views.external_event_stream import ( + REDACTED_STRING, + UNSAFE_HEADER_KEYS, +) from aap_eda.core import enums from tests.integration.api.test_event_stream import ( create_event_stream, @@ -80,47 +84,160 @@ def test_post_event_stream_with_token( assert response.status_code == auth_status +BASE_HEADERS = { + "X-Gitlab-Event-Uuid": "c2675c66-7e6e-4fe2-9ac3-288534ef34b9", + "X-Gitlab-Instance": "https://gitlab.com", + "X-Gitlab-Token": secrets.token_hex(32), + "X-Gitlab-Uuid": "b697868f-3b59-4a1f-985d-47f79e2b05ff", + "X-Gitlab-Event": "Push Hook", + "X-Envoy-abc": "abc", + "X-Trusted-Proxy": "gobbledegook", + "X-Forwarded-For": "fred", + "X-Real-IP": "barney", +} + + +@pytest.mark.parametrize( + ("test_args"), + [ + ( + { + "auth_header": "X-Gitlab-Token", + "additional_data_headers": ( + "x-gitlab-event, x-gitlab-event-uuid , x-gitlab-uuid" + ), + "headers": BASE_HEADERS, + "required_header_keys": [ + "X-Gitlab-Event", + "X-Gitlab-Event-Uuid", + "X-Gitlab-Uuid", + ], + "keys_should_not_exist": list(UNSAFE_HEADER_KEYS) + + [ + "X-Envoy-abc", + "X-Gitlab-Instance", + "X-Gitlab-Token", + ], + "redacted": True, + "key_remap": { + "X-Gitlab-Event": "x-gitlab-event", + "X-Gitlab-Event-Uuid": "x-gitlab-event-uuid", + "X-Gitlab-Uuid": "x-gitlab-uuid", + }, + "test_name": "lowercase data headers with extra spaces", + } + ), + ( + { + "auth_header": "X-Gitlab-Token", + "additional_data_headers": ( + "X-Gitlab-Event, X-Gitlab-Event-Uuid, X-Gitlab-Uuid" + ), + "headers": BASE_HEADERS, + "required_header_keys": [ + "X-Gitlab-Event", + "X-Gitlab-Event-Uuid", + "X-Gitlab-Uuid", + ], + "keys_should_not_exist": list(UNSAFE_HEADER_KEYS) + + [ + "X-Envoy-abc", + "X-Gitlab-Instance", + "X-Gitlab-Token", + ], + "redacted": True, + "key_remap": {}, + "test_name": "data headers with extra spaces", + } + ), + ( + { + "auth_header": "X-Gitlab-Token", + "additional_data_headers": " X-Gitlab-Event ", + "headers": BASE_HEADERS, + "required_header_keys": ["X-Gitlab-Event"], + "keys_should_not_exist": list(UNSAFE_HEADER_KEYS) + + [ + "X-Gitlab-Event-Uuid", + "X-Gitlab-Uuid", + "X-Gitlab-Token", + "X-Gitlab-Instance", + ], + "redacted": True, + "key_remap": {}, + "test_name": "single data headers with surrounding spaces", + } + ), + ( + { + "auth_header": "X-Gitlab-Token", + "additional_data_headers": " X-Gitlab-Token ", + "headers": BASE_HEADERS, + "required_header_keys": ["X-Gitlab-Token"], + "keys_should_not_exist": list(UNSAFE_HEADER_KEYS) + + [ + "X-Envoy-abc", + "X-Gitlab-Event-Uuid", + "X-Gitlab-Uuid", + "X-Gitlab-Instance", + "X-Gitlab-Event", + ], + "redacted": False, + "key_remap": {}, + "test_name": "single data header with exposed auth_header", + } + ), + ( + { + "auth_header": "X-Gitlab-Token", + "additional_data_headers": "*", + "headers": BASE_HEADERS, + "required_header_keys": [ + "X-Gitlab-Event", + "X-Gitlab-Event-Uuid", + "X-Gitlab-Instance", + "X-Gitlab-Uuid", + "X-Gitlab-Token", + ], + "keys_should_not_exist": list(UNSAFE_HEADER_KEYS) + + ["X-Envoy-abc"], + "redacted": True, + "key_remap": {}, + "test_name": "wild card data header", + } + ), + ], +) @pytest.mark.django_db def test_post_event_stream_with_test_mode_extra_headers( admin_client: APIClient, preseed_credential_types, + test_args, ): - secret = secrets.token_hex(32) - signature_header_name = "X-Gitlab-Token" + auth_header = test_args["auth_header"] inputs = { "auth_type": "token", - "token": secret, - "http_header_key": signature_header_name, + "token": test_args["headers"][auth_header], + "http_header_key": auth_header, } obj = create_event_stream_credential( admin_client, enums.EventStreamCredentialType.TOKEN.value, inputs ) - additional_data_headers = ( - "X-Gitlab-Event,X-Gitlab-Event-Uuid,X-Gitlab-Uuid" - ) data_in = { "name": "test-es-1", "eda_credential_id": obj["id"], "event_stream_type": obj["credential_type"]["kind"], "organization_id": get_default_test_org().id, "test_mode": True, - "additional_data_headers": additional_data_headers, + "additional_data_headers": test_args["additional_data_headers"], } event_stream = create_event_stream(admin_client, data_in) data = {"a": 1, "b": 2} - headers = { - "X-Gitlab-Event-Uuid": "c2675c66-7e6e-4fe2-9ac3-288534ef34b9", - "X-Gitlab-Instance": "https://gitlab.com", - signature_header_name: secret, - "X-Gitlab-Uuid": "b697868f-3b59-4a1f-985d-47f79e2b05ff", - "X-Gitlab-Event": "Push Hook", - } - response = admin_client.post( event_stream_post_url(event_stream.uuid), - headers=headers, + headers=test_args["headers"], data=data, ) assert response.status_code == status.HTTP_200_OK @@ -129,15 +246,21 @@ def test_post_event_stream_with_test_mode_extra_headers( test_data = yaml.safe_load(event_stream.test_content) assert test_data["a"] == 1 assert test_data["b"] == 2 + test_headers = yaml.safe_load(event_stream.test_headers) - assert ( - test_headers["X-Gitlab-Event-Uuid"] - == "c2675c66-7e6e-4fe2-9ac3-288534ef34b9" - ) - assert ( - test_headers["X-Gitlab-Uuid"] == "b697868f-3b59-4a1f-985d-47f79e2b05ff" - ) - assert test_headers["X-Gitlab-Event"] == "Push Hook" + + for key in test_args["required_header_keys"]: + if key == auth_header and test_args["redacted"]: + assert test_headers[key] == REDACTED_STRING + else: + assert ( + test_headers[test_args["key_remap"].get(key, key)] + == test_args["headers"][key] + ) + + for key in test_args["keys_should_not_exist"]: + assert key not in test_headers + assert event_stream.test_content_type == "application/json" assert event_stream.events_received == 1 assert event_stream.last_event_received_at is not None