diff --git a/sentry_sdk/data_collection.py b/sentry_sdk/data_collection.py index e742f3c209..bde922f96e 100644 --- a/sentry_sdk/data_collection.py +++ b/sentry_sdk/data_collection.py @@ -23,7 +23,9 @@ """ import warnings -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING, List, Mapping, Optional, cast + +from sentry_sdk._types import SENSITIVE_DATA_SUBSTITUTE if TYPE_CHECKING: from typing import Any, Dict, Literal @@ -77,6 +79,62 @@ ] +def _is_sensitive_key(key: str, extra_terms: "Optional[List[str]]" = None) -> bool: + """ + Return whether ``key`` matches the sensitive denylist using a partial, + case-insensitive substring match. + + :param extra_terms: additional deny terms (e.g. user-provided) to consider + alongside the built-in `_SENSITIVE_DENYLIST`. + """ + lowered = key.lower() + for term in _SENSITIVE_DENYLIST: + if term in lowered: + return True + if extra_terms: + for term in extra_terms: + if term and term.lower() in lowered: + return True + return False + + +def _apply_key_value_collection_filtering( + items: "Mapping[str, Any]", + behaviour: "KeyValueCollectionBehaviour", + substitute: "Any" = SENSITIVE_DATA_SUBSTITUTE, +) -> "Dict[str, Any]": + + if behaviour["mode"] == "off": + return {} + + result: "Dict[str, Any]" = {} + + if behaviour["mode"] == "allowlist": + for key, value in items.items(): + is_allowed = False + if isinstance(key, str): + lowered = key.lower() + is_allowed = any( + term and term.lower() in lowered + for term in behaviour.get("terms", []) + ) + if is_allowed and not _is_sensitive_key(key): + result[key] = value + else: + result[key] = substitute + return result + + # denylist behaviour + for key, value in items.items(): + if isinstance(key, str) and _is_sensitive_key( + key=key, extra_terms=behaviour.get("terms", []) + ): + result[key] = substitute + else: + result[key] = value + return result + + def _map_from_send_default_pii( *, send_default_pii: bool, diff --git a/sentry_sdk/integrations/_asgi_common.py b/sentry_sdk/integrations/_asgi_common.py index 85f8f11d6d..1a3ced948e 100644 --- a/sentry_sdk/integrations/_asgi_common.py +++ b/sentry_sdk/integrations/_asgi_common.py @@ -148,7 +148,7 @@ def _get_request_attributes( if asgi_scope.get("method"): attributes["http.request.method"] = asgi_scope["method"].upper() - headers = _filter_headers(_get_headers(asgi_scope), use_annotated_value=False) + headers = _filter_headers(_get_headers(asgi_scope)) for header, value in 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 cf1a365209..8a931e2a38 100644 --- a/sentry_sdk/integrations/_wsgi_common.py +++ b/sentry_sdk/integrations/_wsgi_common.py @@ -4,6 +4,7 @@ import sentry_sdk from sentry_sdk._types import SENSITIVE_DATA_SUBSTITUTE +from sentry_sdk.data_collection import _apply_key_value_collection_filtering from sentry_sdk.scope import should_send_default_pii from sentry_sdk.utils import AnnotatedValue, logger @@ -23,22 +24,6 @@ from sentry_sdk._types import Event, HttpStatusCodeRange -SENSITIVE_ENV_KEYS = ( - "REMOTE_ADDR", - "HTTP_X_FORWARDED_FOR", - "HTTP_SET_COOKIE", - "HTTP_COOKIE", - "HTTP_AUTHORIZATION", - "HTTP_PROXY_AUTHORIZATION", - "HTTP_X_API_KEY", - "HTTP_X_FORWARDED_FOR", - "HTTP_X_REAL_IP", -) - -SENSITIVE_HEADERS = tuple( - x[len("HTTP_") :] for x in SENSITIVE_ENV_KEYS if x.startswith("HTTP_") -) - DEFAULT_HTTP_METHODS_TO_CAPTURE = ( "CONNECT", "DELETE", @@ -211,21 +196,19 @@ 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]]": - 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() +) -> "Mapping[str, str]": + data_collection_configuration = sentry_sdk.get_client().options["data_collection"] + + filtered = _apply_key_value_collection_filtering( + items=headers, + behaviour=data_collection_configuration["http_headers"]["request"], ) - return { - k: (v if k.upper().replace("-", "_") not in SENSITIVE_HEADERS else substitute) - for k, v in headers.items() - } + for key in filtered: + if isinstance(key, str) and key.lower() in ("cookie", "set-cookie"): + filtered[key] = SENSITIVE_DATA_SUBSTITUTE + + return filtered def _in_http_status_code_range( diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index d22f3a745b..e5c5769528 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -150,7 +150,7 @@ async def sentry_app_handle( header_attributes: "dict[str, Any]" = {} for header, header_value in _filter_headers( - headers, use_annotated_value=False + headers ).items(): header_attributes[ f"http.request.header.{header.lower()}" diff --git a/sentry_sdk/integrations/aws_lambda.py b/sentry_sdk/integrations/aws_lambda.py index c7fe77714a..d4dd778504 100644 --- a/sentry_sdk/integrations/aws_lambda.py +++ b/sentry_sdk/integrations/aws_lambda.py @@ -152,7 +152,7 @@ def sentry_handler( header_attributes: "dict[str, Any]" = {} for header, header_value in _filter_headers( - headers, use_annotated_value=False + 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 9baa0e5cdd..579a7eacf8 100644 --- a/sentry_sdk/integrations/chalice.py +++ b/sentry_sdk/integrations/chalice.py @@ -92,7 +92,7 @@ def wrapped_view_function(**function_args: "Any") -> "Any": header_attrs: "Dict[str, Any]" = {} for header, value in _filter_headers( - headers, use_annotated_value=False + headers ).items(): header_attrs[f"http.request.header.{header.lower()}"] = value diff --git a/sentry_sdk/integrations/gcp.py b/sentry_sdk/integrations/gcp.py index 91a62b3a81..d92f35e796 100644 --- a/sentry_sdk/integrations/gcp.py +++ b/sentry_sdk/integrations/gcp.py @@ -89,7 +89,7 @@ def sentry_func( if hasattr(gcp_event, "headers"): headers = gcp_event.headers for header, header_value in _filter_headers( - headers, use_annotated_value=False + 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 diff --git a/sentry_sdk/integrations/quart.py b/sentry_sdk/integrations/quart.py index 6a5603d825..37b2b80e49 100644 --- a/sentry_sdk/integrations/quart.py +++ b/sentry_sdk/integrations/quart.py @@ -195,7 +195,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 908fceb0cf..8ae313d282 100644 --- a/sentry_sdk/integrations/sanic.py +++ b/sentry_sdk/integrations/sanic.py @@ -374,7 +374,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 859b0d0870..845cab66d4 100644 --- a/sentry_sdk/integrations/tornado.py +++ b/sentry_sdk/integrations/tornado.py @@ -193,7 +193,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 e776ed915a..4cb2cf7303 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -390,7 +390,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 583e7a50b6..4732f53f7b 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -1258,14 +1258,128 @@ async def hello(request): ) +@pytest.mark.parametrize( + "options,expected", + [ + pytest.param( + { + "send_default_pii": True, + "data_collection": None, + }, + { + "authorization": "[Filtered]", + "custom": "foobar", + "cookie": "[Filtered]", + }, + id="enabled_send_default_pii_redacts_auth_header_due_to_data_collection_default_settings", + ), + pytest.param( + { + "send_default_pii": False, + "data_collection": None, + }, + { + "authorization": "[Filtered]", + "custom": "foobar", + "cookie": "[Filtered]", + }, + id="disabled_send_default_pii_redacts_auth_header_due_to_data_collection_default_settings", + ), + pytest.param( + { + "send_default_pii": False, + "data_collection": {"http_headers": {"request": {"mode": "off"}}}, + }, + None, + id="data_collection_off_does_not_add_headers", + ), + pytest.param( + { + "send_default_pii": False, + "data_collection": {"http_headers": {"request": {"mode": "allowlist"}}}, + }, + { + "authorization": "[Filtered]", + "custom": "[Filtered]", + "cookie": "[Filtered]", + }, + id="data_collection_allow_list_redacts_terms_that_do_not_appear", + ), + pytest.param( + { + "send_default_pii": False, + "data_collection": { + "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( + { + "send_default_pii": False, + "data_collection": { + "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( + { + "send_default_pii": False, + "data_collection": { + "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( + { + "send_default_pii": False, + "data_collection": { + "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_span_streaming( - sentry_init, aiohttp_client, capture_items + sentry_init, aiohttp_client, capture_items, options, expected, request ): sentry_init( integrations=[AioHttpIntegration()], traces_sample_rate=1.0, - send_default_pii=True, + send_default_pii=options["send_default_pii"], + data_collection=options["data_collection"], _experiments={"trace_lifecycle": "stream"}, ) @@ -1278,21 +1392,45 @@ async def hello(request): items = capture_items("span") client = await aiohttp_client(app) - await client.get("/", headers={"Authorization": "Bearer secret-token"}) + await client.get( + "/", + headers={ + "Authorization": "Bearer secret-token", + "x-custom-header": "foobar", + "Cookie": "sessionid=secret", + }, + ) sentry_sdk.flush() server_span, _client_segment = [item.payload for item in items] - # With send_default_pii=True, _filter_headers is a no-op and the original - # value reaches the span attribute. - assert ( - server_span["attributes"]["http.request.header.authorization"] - == "Bearer secret-token" - ) + if request.node.callspec.id == "data_collection_off_does_not_add_headers": + 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"] + ) + # client.address and user.ip_address is captured under send_default_pii=True. - assert server_span["attributes"]["client.address"] == "127.0.0.1" - assert server_span["attributes"]["user.ip_address"] == "127.0.0.1" + # TODO: This block will eventually need to be removed from this test into a separate + # test once data collection gating is introduced on these values + if options["send_default_pii"]: + assert server_span["attributes"]["client.address"] == "127.0.0.1" + assert server_span["attributes"]["user.ip_address"] == "127.0.0.1" + else: + assert "user.ip_address" not in server_span["attributes"] + assert "client.address" not in server_span["attributes"] @pytest.mark.asyncio diff --git a/tests/integrations/quart/test_quart.py b/tests/integrations/quart/test_quart.py index 730b24ef33..0e716698ca 100644 --- a/tests/integrations/quart/test_quart.py +++ b/tests/integrations/quart/test_quart.py @@ -14,7 +14,6 @@ set_tag, ) from sentry_sdk.integrations.logging import LoggingIntegration -from sentry_sdk.utils import SENSITIVE_DATA_SUBSTITUTE def quart_app_factory(): @@ -861,12 +860,128 @@ async def test_span_streaming_request_attributes_with_pii(sentry_init, capture_i assert "user.ip_address" in segment["attributes"] +@pytest.mark.parametrize( + "options,expected", + [ + pytest.param( + { + "send_default_pii": True, + "data_collection": None, + }, + { + "authorization": "[Filtered]", + "custom": "passthrough", + "cookie": "[Filtered]", + }, + id="enabled_send_default_pii_redacts_auth_header_due_to_data_collection_default_settings", + ), + pytest.param( + { + "send_default_pii": False, + "data_collection": None, + }, + { + "authorization": "[Filtered]", + "custom": "passthrough", + "cookie": "[Filtered]", + }, + id="disabled_send_default_pii_redacts_auth_header_due_to_data_collection_default_settings", + ), + pytest.param( + { + "send_default_pii": False, + "data_collection": {"http_headers": {"request": {"mode": "off"}}}, + }, + None, + id="data_collection_off_does_not_add_headers", + ), + pytest.param( + { + "send_default_pii": False, + "data_collection": {"http_headers": {"request": {"mode": "allowlist"}}}, + }, + { + "authorization": "[Filtered]", + "custom": "[Filtered]", + "cookie": "[Filtered]", + }, + id="data_collection_allow_list_redacts_terms_that_do_not_appear", + ), + pytest.param( + { + "send_default_pii": False, + "data_collection": { + "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( + { + "send_default_pii": False, + "data_collection": { + "http_headers": { + "request": {"mode": "allowlist", "terms": ["custom"]} + } + }, + }, + { + "authorization": "[Filtered]", + "custom": "passthrough", + "cookie": "[Filtered]", + }, + id="data_collection_allow_list_does_not_redact_provided_term", + ), + pytest.param( + { + "send_default_pii": False, + "data_collection": { + "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( + { + "send_default_pii": False, + "data_collection": { + "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_span_streaming_sensitive_header_scrubbing(sentry_init, capture_items): +async def test_span_streaming_sensitive_header_scrubbing( + sentry_init, capture_items, options, expected, request +): sentry_init( integrations=[quart_sentry.QuartIntegration()], traces_sample_rate=1.0, - send_default_pii=False, + send_default_pii=options["send_default_pii"], + data_collection=options["data_collection"], _experiments={"trace_lifecycle": "stream"}, ) items = capture_items("span") @@ -878,6 +993,7 @@ async def test_span_streaming_sensitive_header_scrubbing(sentry_init, capture_it headers={ "Authorization": "Bearer secret-token", "X-Custom-Header": "passthrough", + "Cookie": "sessionid=secret", }, ) assert response.status_code == 200 @@ -888,11 +1004,19 @@ async def test_span_streaming_sensitive_header_scrubbing(sentry_init, capture_it assert len(spans) == 1 segment = spans[0] - assert ( - segment["attributes"]["http.request.header.authorization"] - == SENSITIVE_DATA_SUBSTITUTE - ) - assert segment["attributes"]["http.request.header.x-custom-header"] == "passthrough" + if request.node.callspec.id == "data_collection_off_does_not_add_headers": + assert "http.request.header.authorization" not in segment["attributes"] + assert "http.request.header.cookie" not in segment["attributes"] + else: + assert ( + segment["attributes"]["http.request.header.authorization"] + == expected["authorization"] + ) + assert ( + segment["attributes"]["http.request.header.x-custom-header"] + == expected["custom"] + ) + assert segment["attributes"]["http.request.header.cookie"] == expected["cookie"] @pytest.mark.asyncio @@ -936,35 +1060,3 @@ async def login(): assert segment["attributes"]["user.id"] == user_id else: assert "user.id" not in segment.get("attributes", {}) - - -@pytest.mark.asyncio -async def test_span_streaming_sensitive_header_passthrough_with_pii( - sentry_init, capture_items -): - sentry_init( - integrations=[quart_sentry.QuartIntegration()], - traces_sample_rate=1.0, - send_default_pii=True, - _experiments={"trace_lifecycle": "stream"}, - ) - items = capture_items("span") - - app = quart_app_factory() - client = app.test_client() - response = await client.get( - "/message", - headers={"Authorization": "Bearer secret-token"}, - ) - assert response.status_code == 200 - - sentry_sdk.flush() - - spans = [item.payload for item in items] - assert len(spans) == 1 - - segment = spans[0] - assert ( - segment["attributes"]["http.request.header.authorization"] - == "Bearer secret-token" - )