diff --git a/sentry_sdk/integrations/_asgi_common.py b/sentry_sdk/integrations/_asgi_common.py index dbf02decdc..640412657d 100644 --- a/sentry_sdk/integrations/_asgi_common.py +++ b/sentry_sdk/integrations/_asgi_common.py @@ -121,10 +121,7 @@ def _get_request_data( headers = _get_headers(asgi_scope) - request_data["headers"] = _filter_headers( - headers, - use_annotated_value=False, - ) + request_data["headers"] = _filter_headers(headers) if has_data_collection_enabled(client_options): qs = _get_query(asgi_scope) @@ -175,7 +172,7 @@ def _get_request_attributes( headers = _get_headers(asgi_scope) - filtered_headers = _filter_headers(headers, use_annotated_value=False) + filtered_headers = _filter_headers(headers) for header, value in filtered_headers.items(): attributes[f"http.request.header.{header.lower()}"] = value diff --git a/sentry_sdk/integrations/_wsgi_common.py b/sentry_sdk/integrations/_wsgi_common.py index eaf764fb4b..6b3597ee39 100644 --- a/sentry_sdk/integrations/_wsgi_common.py +++ b/sentry_sdk/integrations/_wsgi_common.py @@ -221,38 +221,18 @@ def _is_json_content_type(ct: "Optional[str]") -> bool: def _filter_headers( headers: "Mapping[str, str]", - use_annotated_value: bool = True, -) -> "Mapping[str, Union[AnnotatedValue, str]]": +) -> "Mapping[str, str]": client_options = sentry_sdk.get_client().options - if has_data_collection_enabled(client_options): - data_collection_configuration = client_options["data_collection"] - - filtered = _apply_key_value_collection_filtering( - items=headers, - behaviour=data_collection_configuration["http_headers"]["request"], - ) - - for key in filtered: - if isinstance(key, str) and key.lower() in ("cookie", "set-cookie"): - filtered[key] = SENSITIVE_DATA_SUBSTITUTE - - return filtered - else: - if should_send_default_pii(): - return headers - - substitute: "Union[AnnotatedValue, str]" = ( - SENSITIVE_DATA_SUBSTITUTE - if not use_annotated_value - else AnnotatedValue.removed_because_over_size_limit() - ) - - return { - k: ( - v - if k.upper().replace("-", "_") not in SENSITIVE_HEADERS - else substitute - ) - for k, v in headers.items() - } + data_collection_configuration = client_options["data_collection"] + + filtered = _apply_key_value_collection_filtering( + items=headers, + behaviour=data_collection_configuration["http_headers"]["request"], + ) + + for key in filtered: + if isinstance(key, str) and key.lower() in ("cookie", "set-cookie"): + filtered[key] = SENSITIVE_DATA_SUBSTITUTE + + return filtered diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 5c05a5b08b..c743b40710 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -133,12 +133,8 @@ async def sentry_app_handle( Scope.set_custom_sampling_context({"aiohttp_request": request}) header_attributes: "dict[str, Any]" = {} - for header, header_value in _filter_headers( - headers, - use_annotated_value=False, - ).items(): + for header, header_value in _filter_headers(headers).items(): header_attributes[f"http.request.header.{header.lower()}"] = ( - # header_value will always be a string because we set `use_annotated_value` to false above header_value ) diff --git a/sentry_sdk/integrations/aws_lambda.py b/sentry_sdk/integrations/aws_lambda.py index 1eea18b299..aed1c95c8f 100644 --- a/sentry_sdk/integrations/aws_lambda.py +++ b/sentry_sdk/integrations/aws_lambda.py @@ -166,9 +166,7 @@ def sentry_handler( headers = {} header_attributes: "dict[str, Any]" = {} - for header, header_value in _filter_headers( - headers, use_annotated_value=False - ).items(): + for header, header_value in _filter_headers(headers).items(): header_attributes[f"http.request.header.{header.lower()}"] = ( header_value ) diff --git a/sentry_sdk/integrations/chalice.py b/sentry_sdk/integrations/chalice.py index cfe2c13634..453b245b41 100644 --- a/sentry_sdk/integrations/chalice.py +++ b/sentry_sdk/integrations/chalice.py @@ -94,9 +94,7 @@ def wrapped_view_function(**function_args: "Any") -> "Any": headers = request_dict.get("headers", {}) header_attrs: "Dict[str, Any]" = {} - for header, value in _filter_headers( - headers, use_annotated_value=False - ).items(): + for header, value in _filter_headers(headers).items(): header_attrs[f"http.request.header.{header.lower()}"] = value additional_attrs: "Dict[str, Any]" = {} diff --git a/sentry_sdk/integrations/gcp.py b/sentry_sdk/integrations/gcp.py index 1b53889a25..9862cb73fd 100644 --- a/sentry_sdk/integrations/gcp.py +++ b/sentry_sdk/integrations/gcp.py @@ -86,11 +86,8 @@ def sentry_func( header_attributes: "dict[str, Any]" = {} if hasattr(gcp_event, "headers"): headers = gcp_event.headers - for header, header_value in _filter_headers( - headers, use_annotated_value=False - ).items(): + for header, header_value in _filter_headers(headers).items(): header_attributes[f"http.request.header.{header.lower()}"] = ( - # header_value will always be a string because we set `use_annotated_value` to false above header_value ) diff --git a/sentry_sdk/integrations/quart.py b/sentry_sdk/integrations/quart.py index 344c42d36f..05083f7926 100644 --- a/sentry_sdk/integrations/quart.py +++ b/sentry_sdk/integrations/quart.py @@ -186,7 +186,7 @@ async def _request_websocket_started(app: "Quart", **kwargs: "Any") -> None: header_attributes: "dict[str, Any]" = {} for header, header_value in _filter_headers( - dict(request_websocket.headers), use_annotated_value=False + dict(request_websocket.headers) ).items(): header_attributes[f"http.request.header.{header.lower()}"] = header_value diff --git a/sentry_sdk/integrations/sanic.py b/sentry_sdk/integrations/sanic.py index 8b80eb7364..caa620ddd6 100644 --- a/sentry_sdk/integrations/sanic.py +++ b/sentry_sdk/integrations/sanic.py @@ -245,7 +245,7 @@ def _get_request_attributes(request: "Request") -> "Dict[str, Any]": if request.method: attributes[SPANDATA.HTTP_REQUEST_METHOD] = request.method.upper() - headers = _filter_headers(dict(request.headers), use_annotated_value=False) + headers = _filter_headers(dict(request.headers)) for header, value in headers.items(): attributes[f"{SPANDATA.HTTP_REQUEST_HEADER}.{header.lower()}"] = value diff --git a/sentry_sdk/integrations/tornado.py b/sentry_sdk/integrations/tornado.py index 5ccc3b19e2..cacee99caa 100644 --- a/sentry_sdk/integrations/tornado.py +++ b/sentry_sdk/integrations/tornado.py @@ -139,7 +139,7 @@ def _get_request_attributes(request: "Any") -> "Dict[str, Any]": if request.method: attributes[SPANDATA.HTTP_REQUEST_METHOD] = request.method.upper() - headers = _filter_headers(dict(request.headers), use_annotated_value=False) + headers = _filter_headers(dict(request.headers)) for header, value in headers.items(): attributes[f"{SPANDATA.HTTP_REQUEST_HEADER}.{header.lower()}"] = value diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index a4df7d1dc2..c589e802d1 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -398,7 +398,7 @@ def _get_request_attributes( if method: attributes["http.request.method"] = method.upper() - headers = _filter_headers(dict(_get_headers(environ)), use_annotated_value=False) + headers = _filter_headers(dict(_get_headers(environ))) for header, value in headers.items(): attributes[f"http.request.header.{header.lower()}"] = value diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index 074eb064b5..4ab52d30ea 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -1449,119 +1449,6 @@ async def hello(request): ) -@pytest.mark.parametrize( - "data_collection,expected", - [ - pytest.param( - { - "http_headers": {"request": {"mode": "off"}}, - }, - None, - id="data_collection_off_does_not_add_headers", - ), - pytest.param( - { - "http_headers": {"request": {"mode": "allowlist"}}, - }, - { - "authorization": "[Filtered]", - "custom": "[Filtered]", - "cookie": "[Filtered]", - }, - id="data_collection_allow_list_redacts_terms_that_do_not_appear", - ), - pytest.param( - { - "http_headers": { - "request": {"mode": "allowlist", "terms": ["Authorization"]} - } - }, - { - "authorization": "[Filtered]", - "custom": "[Filtered]", - "cookie": "[Filtered]", - }, - id="data_collection_allow_list_redacts_sensitive_terms_even_when_provided_by_user", - ), - pytest.param( - {"http_headers": {"request": {"mode": "allowlist", "terms": ["custom"]}}}, - { - "authorization": "[Filtered]", - "custom": "foobar", - "cookie": "[Filtered]", - }, - id="data_collection_allow_list_does_not_redact_provided_term", - ), - pytest.param( - {"http_headers": {"request": {"mode": "denylist", "terms": ["custom"]}}}, - { - "authorization": "[Filtered]", - "custom": "[Filtered]", - "cookie": "[Filtered]", - }, - id="data_collection_deny_list_redacts_sensitive_terms_when_provided_by_user", - ), - pytest.param( - {"http_headers": {"request": {"mode": "allowlist", "terms": ["cookie"]}}}, - { - "authorization": "[Filtered]", - "custom": "[Filtered]", - "cookie": "[Filtered]", - }, - id="data_collection_cookie_is_always_redacted_even_when_allow_listed", - ), - ], -) -@pytest.mark.asyncio -async def test_sensitive_header_passthrough_with_pii( - sentry_init, aiohttp_client, capture_items, data_collection, expected, request -): - sentry_init( - integrations=[AioHttpIntegration()], - traces_sample_rate=1.0, - data_collection=data_collection, - ) - - async def hello(request): - return web.Response(text="hello") - - app = web.Application() - app.router.add_get("/", hello) - - items = capture_items("span") - - client = await aiohttp_client(app) - await client.get( - "/", - headers={ - "Authorization": "Bearer secret-token", - "x-custom-header": "foobar", - "Cookie": "sessionid=secret", - }, - ) - - sentry_sdk.flush() - - (server_span,) = [item.payload for item in items] - - if expected is None: - assert "http.request.header.authorization" not in server_span["attributes"] - assert "http.request.header.cookie" not in server_span["attributes"] - else: - assert ( - server_span["attributes"]["http.request.header.authorization"] - == expected["authorization"] - ) - assert ( - server_span["attributes"]["http.request.header.x-custom-header"] - == expected["custom"] - ) - assert ( - server_span["attributes"]["http.request.header.cookie"] - == expected["cookie"] - ) - - @pytest.mark.asyncio @pytest.mark.parametrize( "url,transaction_style,expected_name,expected_source", diff --git a/tests/integrations/asgi/test_asgi.py b/tests/integrations/asgi/test_asgi.py index 64e67bef05..b8e1e20d5c 100644 --- a/tests/integrations/asgi/test_asgi.py +++ b/tests/integrations/asgi/test_asgi.py @@ -201,7 +201,6 @@ async def test_capture_transaction( assert span["attributes"]["network.protocol.name"] == "http" assert span["attributes"]["http.request.method"] == "GET" assert span["attributes"]["http.request.header.host"] == "localhost" - assert span["attributes"]["http.request.header.remote-addr"] == "127.0.0.1" assert span["attributes"]["http.request.header.user-agent"] == "ASGI-Test-Client" if should_send_pii: @@ -674,6 +673,108 @@ def test_get_headers(): } +@pytest.mark.asyncio +@pytest.mark.parametrize( + "data_collection, expected_headers", + [ + pytest.param( + {}, + { + "http.request.header.authorization": "[Filtered]", + "http.request.header.x-custom-header": "passthrough", + }, + id="default_redacts_sensitive_headers", + ), + pytest.param( + {"http_headers": {"request": {"mode": "off"}}}, + None, + id="mode_off_collects_no_headers", + ), + pytest.param( + {"http_headers": {"request": {"mode": "allowlist", "terms": ["custom"]}}}, + { + "http.request.header.x-custom-header": "passthrough", + "http.request.header.x-forwarded-for": "[Filtered]", + "http.request.header.host": "[Filtered]", + }, + id="allowlist_redacts_all_but_allowed_terms", + ), + pytest.param( + {"http_headers": {"request": {"mode": "denylist", "terms": ["custom"]}}}, + { + "http.request.header.x-custom-header": "[Filtered]", + "http.request.header.x-forwarded-for": "1.2.3.4", + "http.request.header.host": "localhost", + }, + id="denylist_redacts_only_matched_terms", + ), + ], +) +async def test_request_headers_data_collection( + sentry_init, asgi3_app, capture_items, data_collection, expected_headers +): + sentry_init( + traces_sample_rate=1.0, + data_collection=data_collection, + ) + app = SentryAsgiMiddleware(asgi3_app) + + items = capture_items("span") + async with TestClient(app) as client: + await client.get( + "/some_url", + headers={ + "Authorization": "Bearer secret-token", + "X-Forwarded-For": "1.2.3.4", + "X-Custom-Header": "passthrough", + }, + ) + + sentry_sdk.flush() + + (span,) = [item.payload for item in items] + attributes = span["attributes"] + + if expected_headers is None: + assert not any(key.startswith("http.request.header.") for key in attributes) + else: + for key, value in expected_headers.items(): + assert attributes[key] == value + + +@pytest.mark.asyncio +async def test_request_headers_data_collection_cookie_always_redacted( + sentry_init, asgi3_app, capture_items +): + sentry_init( + traces_sample_rate=1.0, + data_collection={ + "http_headers": { + "request": {"mode": "allowlist", "terms": ["cookie", "custom"]} + } + }, + ) + app = SentryAsgiMiddleware(asgi3_app) + + items = capture_items("span") + async with TestClient(app) as client: + await client.get( + "/some_url", + headers={ + "Cookie": "sessionid=secret", + "X-Custom-Header": "passthrough", + }, + ) + + sentry_sdk.flush() + + (span,) = [item.payload for item in items] + attributes = span["attributes"] + + assert attributes["http.request.header.cookie"] == "[Filtered]" + assert attributes["http.request.header.x-custom-header"] == "passthrough" + + @pytest.mark.asyncio async def test_get_request_attributes_url_with_filtered_host( sentry_init, capture_items, asgi3_app @@ -711,7 +812,6 @@ async def test_get_request_attributes_url_with_headers_off( # "off" mode in data collection captures no headers at all, but "url.full" must # still resolve via the (uncaptured) host header rather than being dropped. sentry_init( - send_default_pii=True, traces_sample_rate=1.0, data_collection={"http_headers": {"request": {"mode": "off"}}}, ) diff --git a/tests/integrations/aws_lambda/test_aws_lambda.py b/tests/integrations/aws_lambda/test_aws_lambda.py index 45aa6a8f20..da1d9bb60b 100644 --- a/tests/integrations/aws_lambda/test_aws_lambda.py +++ b/tests/integrations/aws_lambda/test_aws_lambda.py @@ -232,7 +232,7 @@ def test_non_dict_event( if has_request_data: request_data = { - "headers": {"Host": "x1.io", "X-Forwarded-Proto": "https"}, + "headers": {"Host": "x1.io", "X-Forwarded-Proto": "[Filtered]"}, "method": "GET", "url": "https://x1.io/1", "query_string": { diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py index c71a78c44e..3546aeeb9c 100644 --- a/tests/integrations/wsgi/test_wsgi.py +++ b/tests/integrations/wsgi/test_wsgi.py @@ -45,7 +45,7 @@ def next(self): def test_basic(sentry_init, crashing_app, capture_events): - sentry_init(send_default_pii=True) + sentry_init(data_collection={}) app = SentryWsgiMiddleware(crashing_app) client = Client(app) events = capture_events() @@ -60,7 +60,6 @@ def test_basic(sentry_init, crashing_app, capture_events): "env": {"SERVER_NAME": "localhost", "SERVER_PORT": "80"}, "headers": {"Host": "localhost"}, "method": "GET", - "query_string": "", "url": "http://localhost/", } @@ -687,175 +686,68 @@ def test_get_request_url_x_forwarded_proto(environ, use_x_forwarded_for, expecte assert get_request_url(environ, use_x_forwarded_for) == expected_url -@pytest.mark.parametrize("send_default_pii", [True, False]) -def test_request_headers_data_collection_default_redacts_sensitive( - sentry_init, crashing_app, capture_events, send_default_pii -): - """ - When ``data_collection`` is configured (here as ``{}``, i.e. spec - defaults), the WSGI event processor routes request headers through the - data-collection filtering path. Sensitive headers are redacted regardless - of ``send_default_pii`` -- the value of that legacy option must not change - the outcome. - """ - sentry_init( - send_default_pii=send_default_pii, - data_collection={}, - ) - app = SentryWsgiMiddleware(crashing_app) - client = Client(app) - events = capture_events() - - with pytest.raises(ZeroDivisionError): - client.get( - "/", - headers={ - "Authorization": "Bearer secret-token", - "X-Custom-Header": "passthrough", - }, - ) - - (event,) = events - headers = event["request"]["headers"] - - assert headers["Authorization"] == "[Filtered]" - assert headers["X-Custom-Header"] == "passthrough" - - -def test_request_headers_legacy_no_pii_redacts_sensitive( - sentry_init, crashing_app, capture_events -): - """ - With no ``data_collection`` configured, ``_filter_headers`` falls back to - the legacy ``send_default_pii`` behaviour. When PII is disabled, headers in - ``SENSITIVE_HEADERS`` are replaced with an ``AnnotatedValue`` (the default - ``use_annotated_value=True`` on the event-processor call site), which - serializes to an emptied value plus a ``_meta`` annotation. Non-sensitive - headers pass through untouched. - - ``X-Forwarded-For`` is used because it is in ``SENSITIVE_HEADERS`` but is - not scrubbed by the default ``EventScrubber``, so the substitution we are - asserting on can only come from ``_filter_headers``. - """ - sentry_init(send_default_pii=False) - app = SentryWsgiMiddleware(crashing_app) - client = Client(app) - events = capture_events() - - with pytest.raises(ZeroDivisionError): - client.get( - "/", - headers={ - "X-Forwarded-For": "1.2.3.4", +@pytest.mark.parametrize( + "data_collection, request_headers, expected_headers", + [ + pytest.param( + {}, + {"Authorization": "Bearer secret-token", "X-Custom-Header": "passthrough"}, + { + "Authorization": "[Filtered]", "X-Custom-Header": "passthrough", + "Host": "localhost", }, - ) - - (event,) = events - - assert event["request"]["headers"]["X-Forwarded-For"] == "" - assert event["request"]["headers"]["X-Custom-Header"] == "passthrough" - - # The emptied value is accompanied by a `_meta` annotation marking it as - # removed, confirming the substitution came from the AnnotatedValue path. - assert event["_meta"]["request"]["headers"]["X-Forwarded-For"] == { - "": {"rem": [["!config", "x"]]} - } - - -def test_request_headers_data_collection_off_collects_no_headers( - sentry_init, crashing_app, capture_events -): - """ - With ``http_headers.request`` mode set to ``off``, no request headers are - collected at all -- the filtering returns an empty mapping. - """ - sentry_init( - data_collection={"http_headers": {"request": {"mode": "off"}}}, - ) - app = SentryWsgiMiddleware(crashing_app) - client = Client(app) - events = capture_events() - - with pytest.raises(ZeroDivisionError): - client.get( - "/", - headers={ - "X-Forwarded-For": "1.2.3.4", + id="default_redacts_sensitive", + ), + pytest.param( + {"http_headers": {"request": {"mode": "off"}}}, + {"X-Forwarded-For": "1.2.3.4", "X-Custom-Header": "passthrough"}, + {}, + id="off_collects_no_headers", + ), + # Only headers matching an allowlist term (partial, case-insensitive) + # keep their value; every other key is kept but redacted. + pytest.param( + {"http_headers": {"request": {"mode": "allowlist", "terms": ["custom"]}}}, + {"X-Forwarded-For": "1.2.3.4", "X-Custom-Header": "passthrough"}, + { "X-Custom-Header": "passthrough", + "X-Forwarded-For": "[Filtered]", + "Host": "[Filtered]", }, - ) - - (event,) = events - - assert event["request"]["headers"] == {} - - -def test_request_headers_data_collection_allowlist_redacts_all_but_allowed_terms( - sentry_init, crashing_app, capture_events -): - """ - An ``allowlist`` allows through only headers matching a configured term - (partial, case-insensitive); every other header key is kept but its value - is redacted. - """ - sentry_init( - data_collection={ - "http_headers": {"request": {"mode": "allowlist", "terms": ["custom"]}} - }, - ) - app = SentryWsgiMiddleware(crashing_app) - client = Client(app) - events = capture_events() - - with pytest.raises(ZeroDivisionError): - client.get( - "/", - headers={ + id="allowlist_redacts_all_but_allowed_terms", + ), + pytest.param( + {"http_headers": {"request": {"mode": "denylist", "terms": ["custom"]}}}, + {"X-Forwarded-For": "1.2.3.4", "X-Custom-Header": "passthrough"}, + { + "X-Custom-Header": "[Filtered]", "X-Forwarded-For": "1.2.3.4", - "X-Custom-Header": "passthrough", + "Host": "localhost", }, - ) - - (event,) = events - headers = event["request"]["headers"] - - assert headers["X-Custom-Header"] == "passthrough" - assert headers["X-Forwarded-For"] == "[Filtered]" - assert headers["Host"] == "[Filtered]" - - -def test_request_headers_data_collection_denylist_redacts_only_matched_terms( - sentry_init, crashing_app, capture_events + id="denylist_redacts_only_matched_terms", + ), + ], +) +def test_request_headers_data_collection( + sentry_init, + crashing_app, + capture_events, + data_collection, + request_headers, + expected_headers, ): - """ - A ``denylist`` passes headers through by default, redacting only those - matching a configured term (partial, case-insensitive). - """ - sentry_init( - data_collection={ - "http_headers": {"request": {"mode": "denylist", "terms": ["custom"]}} - }, - ) + sentry_init(data_collection=data_collection) app = SentryWsgiMiddleware(crashing_app) client = Client(app) events = capture_events() with pytest.raises(ZeroDivisionError): - client.get( - "/", - headers={ - "X-Forwarded-For": "1.2.3.4", - "X-Custom-Header": "passthrough", - }, - ) + client.get("/", headers=request_headers) (event,) = events - headers = event["request"]["headers"] - assert headers["X-Custom-Header"] == "[Filtered]" - assert headers["X-Forwarded-For"] == "1.2.3.4" - assert headers["Host"] == "localhost" + assert event["request"]["headers"] == expected_headers def test_request_headers_data_collection_cookie_always_redacted( @@ -900,35 +792,6 @@ def test_request_headers_data_collection_cookie_always_redacted( assert headers["X-Custom-Header"] == "passthrough" -def test_request_headers_legacy_pii_passes_headers_through( - sentry_init, crashing_app, capture_events -): - """ - With no ``data_collection`` configured and ``send_default_pii`` enabled, - the legacy path returns all headers unchanged -- including those in - ``SENSITIVE_HEADERS``. - """ - sentry_init(send_default_pii=True) - app = SentryWsgiMiddleware(crashing_app) - client = Client(app) - events = capture_events() - - with pytest.raises(ZeroDivisionError): - client.get( - "/", - headers={ - "X-Forwarded-For": "1.2.3.4", - "X-Custom-Header": "passthrough", - }, - ) - - (event,) = events - headers = event["request"]["headers"] - - assert headers["X-Forwarded-For"] == "1.2.3.4" - assert headers["X-Custom-Header"] == "passthrough" - - @pytest.mark.parametrize( "init_kwargs, expected_query_string", [