diff --git a/sentry_sdk/integrations/_asgi_common.py b/sentry_sdk/integrations/_asgi_common.py index 57ce4d54ca..dbf02decdc 100644 --- a/sentry_sdk/integrations/_asgi_common.py +++ b/sentry_sdk/integrations/_asgi_common.py @@ -115,6 +115,7 @@ def _get_request_data( """ request_data: "Dict[str, Any]" = {} ty = asgi_scope["type"] + client_options = sentry_sdk.get_client().options if ty in ("http", "websocket"): request_data["method"] = asgi_scope.get("method") @@ -125,7 +126,6 @@ def _get_request_data( use_annotated_value=False, ) - client_options = sentry_sdk.get_client().options if has_data_collection_enabled(client_options): qs = _get_query(asgi_scope) if qs: @@ -148,8 +148,12 @@ def _get_request_data( ) client = asgi_scope.get("client") - if client and should_send_default_pii(): - request_data["env"] = {"REMOTE_ADDR": _get_ip(asgi_scope)} + if client: + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + request_data["env"] = {"REMOTE_ADDR": _get_ip(asgi_scope)} + elif should_send_default_pii(): + request_data["env"] = {"REMOTE_ADDR": _get_ip(asgi_scope)} return request_data @@ -164,6 +168,7 @@ def _get_request_attributes( attributes: "dict[str, Any]" = {} ty = asgi_scope["type"] + client_options = sentry_sdk.get_client().options if ty in ("http", "websocket"): if asgi_scope.get("method"): attributes["http.request.method"] = asgi_scope["method"].upper() @@ -174,7 +179,6 @@ def _get_request_attributes( for header, value in filtered_headers.items(): attributes[f"http.request.header.{header.lower()}"] = value - client_options = sentry_sdk.get_client().options if has_data_collection_enabled(client_options): filtered_query_string = None query = _get_query(asgi_scope) @@ -226,9 +230,14 @@ def _get_request_attributes( else url_without_query_string ) - client = asgi_scope.get("client") - if client and should_send_default_pii(): - ip = _get_ip(asgi_scope) - attributes["client.address"] = ip + asgi_scope_client = asgi_scope.get("client") + if asgi_scope_client: + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + ip = _get_ip(asgi_scope) + attributes["client.address"] = ip + elif should_send_default_pii(): + ip = _get_ip(asgi_scope) + attributes["client.address"] = ip return attributes diff --git a/sentry_sdk/integrations/aws_lambda.py b/sentry_sdk/integrations/aws_lambda.py index 762a243735..84047a7581 100644 --- a/sentry_sdk/integrations/aws_lambda.py +++ b/sentry_sdk/integrations/aws_lambda.py @@ -11,6 +11,7 @@ import sentry_sdk from sentry_sdk.api import continue_trace from sentry_sdk.consts import OP +from sentry_sdk.data_collection import _apply_key_value_collection_filtering from sentry_sdk.integrations import Integration from sentry_sdk.integrations._wsgi_common import _filter_headers from sentry_sdk.integrations.cloud_resource_context import ( @@ -27,6 +28,7 @@ capture_internal_exceptions, ensure_integration_enabled, event_from_exception, + has_data_collection_enabled, logger, reraise, ) @@ -164,10 +166,20 @@ def sentry_handler( "httpMethod" ] - if should_send_default_pii() and "queryStringParameters" in request_data: + if "queryStringParameters" in request_data: qs = request_data["queryStringParameters"] if qs: - additional_attributes["url.query"] = urlencode(qs) + if has_data_collection_enabled(client.options): + filtered_qs = _apply_key_value_collection_filtering( + items=qs, + behaviour=client.options["data_collection"][ + "url_query_params" + ], + ) + if filtered_qs: + additional_attributes["url.query"] = urlencode(filtered_qs) + elif should_send_default_pii(): + additional_attributes["url.query"] = urlencode(qs) sampling_context = { "aws_event": aws_event, @@ -409,7 +421,18 @@ def event_processor( request["url"] = _get_url(aws_event, aws_context) if "queryStringParameters" in aws_event: - request["query_string"] = aws_event["queryStringParameters"] + query_string = aws_event["queryStringParameters"] + client_options = sentry_sdk.get_client().options + if has_data_collection_enabled(client_options): + if query_string: + filtered_qs = _apply_key_value_collection_filtering( + items=query_string, + behaviour=client_options["data_collection"]["url_query_params"], + ) + if filtered_qs: + request["query_string"] = filtered_qs + else: + request["query_string"] = query_string if "headers" in aws_event: request["headers"] = _filter_headers(aws_event["headers"]) diff --git a/sentry_sdk/integrations/gcp.py b/sentry_sdk/integrations/gcp.py index 29095343eb..cfd3344ab8 100644 --- a/sentry_sdk/integrations/gcp.py +++ b/sentry_sdk/integrations/gcp.py @@ -8,6 +8,7 @@ import sentry_sdk from sentry_sdk.api import continue_trace from sentry_sdk.consts import OP +from sentry_sdk.data_collection import _apply_data_collection_filtering_to_query_string from sentry_sdk.integrations import Integration from sentry_sdk.integrations._wsgi_common import _filter_headers from sentry_sdk.integrations.cloud_resource_context import CLOUD_PROVIDER @@ -20,6 +21,7 @@ TimeoutThread, capture_internal_exceptions, event_from_exception, + has_data_collection_enabled, logger, reraise, ) @@ -100,10 +102,20 @@ def sentry_func( if hasattr(gcp_event, "method"): additional_attributes["http.request.method"] = gcp_event.method - if should_send_default_pii() and hasattr(gcp_event, "query_string"): - additional_attributes["url.query"] = gcp_event.query_string.decode( - "utf-8", errors="replace" - ) + if hasattr(gcp_event, "query_string"): + query_string = gcp_event.query_string.decode("utf-8", errors="replace") + if query_string: + if has_data_collection_enabled(client.options): + filtered_qs = _apply_data_collection_filtering_to_query_string( + query_string=query_string, + behaviour=client.options["data_collection"][ + "url_query_params" + ], + ) + if filtered_qs: + additional_attributes["url.query"] = filtered_qs + elif should_send_default_pii(): + additional_attributes["url.query"] = query_string sampling_context = { "gcp_env": { @@ -235,9 +247,18 @@ def event_processor(event: "Event", hint: "Hint") -> "Optional[Event]": request["method"] = gcp_event.method if hasattr(gcp_event, "query_string"): - request["query_string"] = gcp_event.query_string.decode( - "utf-8", errors="replace" - ) + query_string = gcp_event.query_string.decode("utf-8", errors="replace") + client_options = sentry_sdk.get_client().options + if has_data_collection_enabled(client_options): + if query_string: + filtered_qs = _apply_data_collection_filtering_to_query_string( + query_string=query_string, + behaviour=client_options["data_collection"]["url_query_params"], + ) + if filtered_qs: + request["query_string"] = filtered_qs + else: + request["query_string"] = query_string if hasattr(gcp_event, "headers"): request["headers"] = _filter_headers(gcp_event.headers) diff --git a/sentry_sdk/integrations/quart.py b/sentry_sdk/integrations/quart.py index 6a5603d825..8c281f5874 100644 --- a/sentry_sdk/integrations/quart.py +++ b/sentry_sdk/integrations/quart.py @@ -5,6 +5,7 @@ from typing import TYPE_CHECKING import sentry_sdk +from sentry_sdk.data_collection import _apply_data_collection_filtering_to_query_string from sentry_sdk.integrations import DidNotEnable, Integration from sentry_sdk.integrations._wsgi_common import _filter_headers from sentry_sdk.integrations.asgi import SentryAsgiMiddleware @@ -17,6 +18,8 @@ capture_internal_exceptions, ensure_integration_enabled, event_from_exception, + has_data_collection_enabled, + parse_url, ) if TYPE_CHECKING: @@ -203,7 +206,39 @@ async def _request_websocket_started(app: "Quart", **kwargs: "Any") -> None: segment.set_attributes(header_attributes) - if should_send_default_pii(): + client_options = sentry_sdk.get_client().options + filtered_query_string = None + if has_data_collection_enabled(client_options): + query_string = request_websocket.query_string.decode( + "utf-8", errors="replace" + ) + if query_string: + filtered_query_string = ( + _apply_data_collection_filtering_to_query_string( + query_string=query_string, + behaviour=client_options["data_collection"][ + "url_query_params" + ], + ) + ) + if filtered_query_string: + segment.set_attribute( + "url.query", + filtered_query_string, + ) + + parsed_url = parse_url(request_websocket.url) + segment.set_attribute( + "url.full", + f"{parsed_url.url}?{filtered_query_string}" + if filtered_query_string + else parsed_url.url, + ) + + # TODO: Add the user properties that are seen in the branch below here once + # code is added to respect the `user_info` settings within the data collection + # configuration + elif should_send_default_pii(): segment.set_attribute("url.full", request_websocket.url) segment.set_attribute( "url.query", diff --git a/tests/integrations/asgi/test_asgi.py b/tests/integrations/asgi/test_asgi.py index 5957a3e763..321ac91d75 100644 --- a/tests/integrations/asgi/test_asgi.py +++ b/tests/integrations/asgi/test_asgi.py @@ -8,9 +8,6 @@ from sentry_sdk.integrations._asgi_common import ( _get_headers, _get_ip, - _get_request_attributes, - _get_request_data, - _RootPathInPath, ) from sentry_sdk.integrations.asgi import SentryAsgiMiddleware, _looks_like_asgi3 from sentry_sdk.tracing import TransactionSource @@ -837,82 +834,94 @@ def test_get_headers(): } -def test_get_request_data_url_with_filtered_host(sentry_init): - # allowlist mode in data collection that does not allow "host" scrubs the host header value, - # but the reported URL must still resolve via rather than embedding the substituted "[Filtered]" value. +@pytest.mark.asyncio +async def test_get_request_data_url_with_filtered_host( + sentry_init, capture_events, asgi3_app +): + # allowlist mode in data collection that does not allow "host" scrubs the host + # header value, but the reported URL must still resolve rather than embedding the + # substituted "[Filtered]" value. sentry_init( + traces_sample_rate=1.0, _experiments={ "data_collection": { "http_headers": {"request": {"mode": "allowlist", "terms": []}} } - } + }, ) + app = SentryAsgiMiddleware(asgi3_app) - scope = { - "type": "http", - "method": "GET", - "scheme": "http", - "server": ("example.com", 80), - "path": "/foo", - "query_string": b"", - "headers": [(b"host", b"example.com")], - } + events = capture_events() + scope = {"server": ("example.com", 80), "scheme": "http"} + async with TestClient(app, scope=scope) as client: + await client.get("/foo", headers={"host": "example.com"}) + + sentry_sdk.flush() - request_data = _get_request_data(scope, _RootPathInPath.EXCLUDED) + (transaction_event,) = events - assert request_data["headers"]["host"] == "[Filtered]" - assert request_data["url"] == "http://example.com/foo" + assert transaction_event["request"]["headers"]["host"] == "[Filtered]" + assert transaction_event["request"]["url"] == "http://example.com/foo" -def test_get_request_attributes_url_with_filtered_host(sentry_init): - # As with _get_request_data, an allowlist mode that does not allow "host" - # scrubs the host header value, but "url.full" must still resolve rather than embedding the substituted value. +@pytest.mark.asyncio +async def test_get_request_attributes_url_with_filtered_host( + sentry_init, capture_items, asgi3_app +): + # As with the request data, an allowlist mode that does not allow "host" scrubs + # the host header value, but "url.full" must still resolve rather than embedding + # the substituted value. sentry_init( send_default_pii=True, + traces_sample_rate=1.0, _experiments={ + "trace_lifecycle": "stream", "data_collection": { "http_headers": {"request": {"mode": "allowlist", "terms": []}} - } + }, }, ) + app = SentryAsgiMiddleware(asgi3_app) - scope = { - "type": "http", - "method": "GET", - "scheme": "http", - "server": ("example.com", 80), - "path": "/foo", - "query_string": b"somevalue=123", - "headers": [(b"host", b"example.com")], - } + items = capture_items("span") + scope = {"server": ("example.com", 80), "scheme": "http"} + async with TestClient(app, scope=scope) as client: + await client.get("/foo?somevalue=123", headers={"host": "example.com"}) + + sentry_sdk.flush() - attributes = _get_request_attributes(scope, _RootPathInPath.EXCLUDED) + assert len(items) == 1 + attributes = items[0].payload["attributes"] assert attributes["http.request.header.host"] == "[Filtered]" assert attributes["url.full"] == "http://example.com/foo?somevalue=123" -def test_get_request_attributes_url_with_headers_off(sentry_init): - # "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. +@pytest.mark.asyncio +async def test_get_request_attributes_url_with_headers_off( + sentry_init, capture_items, asgi3_app +): + # "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, _experiments={ - "data_collection": {"http_headers": {"request": {"mode": "off"}}} + "trace_lifecycle": "stream", + "data_collection": {"http_headers": {"request": {"mode": "off"}}}, }, ) + app = SentryAsgiMiddleware(asgi3_app) - scope = { - "type": "http", - "method": "GET", - "scheme": "http", - "server": ("example.com", 80), - "path": "/foo", - "query_string": b"somevalue=123", - "headers": [(b"host", b"example.com")], - } + items = capture_items("span") + scope = {"server": ("example.com", 80), "scheme": "http"} + async with TestClient(app, scope=scope) as client: + await client.get("/foo?somevalue=123", headers={"host": "example.com"}) + + sentry_sdk.flush() - attributes = _get_request_attributes(scope, _RootPathInPath.EXCLUDED) + assert len(items) == 1 + attributes = items[0].payload["attributes"] assert not any(key.startswith("http.request.header.") for key in attributes) assert attributes["url.full"] == "http://example.com/foo?somevalue=123" @@ -921,18 +930,11 @@ def test_get_request_attributes_url_with_headers_off(sentry_init): QUERY_STRING = "token=abc&theme=dark&lang=en&session=xyz" -def _http_scope(query_string=QUERY_STRING): - return { - "type": "http", - "method": "GET", - "scheme": "http", - "server": ("example.com", 80), - "path": "/foo", - "query_string": query_string.encode("latin-1"), - "headers": [(b"host", b"example.com")], - } +def _http_scope(): + return {"server": ("example.com", 80), "scheme": "http"} +@pytest.mark.asyncio @pytest.mark.parametrize( "init_kwargs, expected_query_string", [ @@ -1011,12 +1013,20 @@ def _http_scope(query_string=QUERY_STRING): ), ], ) -def test_get_request_data_query_string_data_collection( - sentry_init, init_kwargs, expected_query_string +async def test_get_request_data_query_string_data_collection( + sentry_init, capture_events, asgi3_app, init_kwargs, expected_query_string ): - sentry_init(**init_kwargs) + sentry_init(traces_sample_rate=1.0, **init_kwargs) + app = SentryAsgiMiddleware(asgi3_app) + + events = capture_events() + async with TestClient(app, scope=_http_scope()) as client: + await client.get(f"/foo?{QUERY_STRING}", headers={"host": "example.com"}) + + sentry_sdk.flush() - request_data = _get_request_data(_http_scope(), _RootPathInPath.EXCLUDED) + (transaction_event,) = events + request_data = transaction_event["request"] if expected_query_string is None: assert "query_string" not in request_data @@ -1024,27 +1034,42 @@ def test_get_request_data_query_string_data_collection( assert request_data["query_string"] == expected_query_string -def test_get_request_data_query_string_empty_legacy_is_none(sentry_init): +@pytest.mark.asyncio +async def test_get_request_data_query_string_empty_legacy_is_none( + sentry_init, capture_events, asgi3_app +): # Legacy path: the query string is always set even when empty (``None``). - sentry_init(send_default_pii=True) + sentry_init(send_default_pii=True, traces_sample_rate=1.0) + app = SentryAsgiMiddleware(asgi3_app) - request_data = _get_request_data( - _http_scope(query_string=""), _RootPathInPath.EXCLUDED - ) + events = capture_events() + async with TestClient(app, scope=_http_scope()) as client: + await client.get("/foo", headers={"host": "example.com"}) - assert request_data["query_string"] is None + sentry_sdk.flush() + (transaction_event,) = events + assert transaction_event["request"]["query_string"] is None -def test_get_request_data_empty_query_string_dropped_with_data_collection(sentry_init): - sentry_init(_experiments={"data_collection": {}}) - request_data = _get_request_data( - _http_scope(query_string=""), _RootPathInPath.EXCLUDED - ) +@pytest.mark.asyncio +async def test_get_request_data_empty_query_string_dropped_with_data_collection( + sentry_init, capture_events, asgi3_app +): + sentry_init(traces_sample_rate=1.0, _experiments={"data_collection": {}}) + app = SentryAsgiMiddleware(asgi3_app) + + events = capture_events() + async with TestClient(app, scope=_http_scope()) as client: + await client.get("/foo", headers={"host": "example.com"}) + + sentry_sdk.flush() - assert "query_string" not in request_data + (transaction_event,) = events + assert "query_string" not in transaction_event["request"] +@pytest.mark.asyncio @pytest.mark.parametrize( "init_kwargs, expected_query, expected_url_full", [ @@ -1107,12 +1132,27 @@ def test_get_request_data_empty_query_string_dropped_with_data_collection(sentry ), ], ) -def test_get_request_attributes_query_data_collection( - sentry_init, init_kwargs, expected_query, expected_url_full +async def test_get_request_attributes_query_data_collection( + sentry_init, + capture_items, + asgi3_app, + init_kwargs, + expected_query, + expected_url_full, ): - sentry_init(**init_kwargs) + kwargs = {k: v for k, v in init_kwargs.items() if k != "_experiments"} + experiments = {"trace_lifecycle": "stream", **init_kwargs.get("_experiments", {})} + sentry_init(traces_sample_rate=1.0, _experiments=experiments, **kwargs) + app = SentryAsgiMiddleware(asgi3_app) - attributes = _get_request_attributes(_http_scope(), _RootPathInPath.EXCLUDED) + items = capture_items("span") + async with TestClient(app, scope=_http_scope()) as client: + await client.get(f"/foo?{QUERY_STRING}", headers={"host": "example.com"}) + + sentry_sdk.flush() + + assert len(items) == 1 + attributes = items[0].payload["attributes"] if expected_query is None: assert "http.query" not in attributes @@ -1127,6 +1167,105 @@ def test_get_request_attributes_query_data_collection( assert attributes["url.path"] == "/foo" +USER_INFO_CASES = [ + pytest.param( + {"_experiments": {"data_collection": {"user_info": False}}}, + True, + False, + id="dc_user_info_false", + ), + pytest.param( + {"_experiments": {"data_collection": {}}}, + True, + True, + id="dc_default_user_info", + ), + pytest.param( + { + "send_default_pii": True, + "_experiments": {"data_collection": {"user_info": False}}, + }, + True, + False, + id="dc_wins_over_pii", + ), + pytest.param( + {"send_default_pii": True}, + True, + True, + id="legacy_pii_true", + ), + pytest.param( + {"send_default_pii": False}, + True, + False, + id="legacy_pii_false", + ), + pytest.param( + {"_experiments": {"data_collection": {}}}, + False, + False, + id="no_client", + ), +] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("init_kwargs, has_client, expect_ip", USER_INFO_CASES) +async def test_get_request_data_env_user_info( + sentry_init, capture_events, asgi3_app, init_kwargs, has_client, expect_ip +): + sentry_init(traces_sample_rate=1.0, **init_kwargs) + app = SentryAsgiMiddleware(asgi3_app) + + scope = _http_scope() + if has_client: + scope["client"] = ("127.0.0.1", 60457) + + events = capture_events() + async with TestClient(app, scope=scope) as client: + await client.get("/foo", headers={"host": "example.com"}) + + sentry_sdk.flush() + + (transaction_event,) = events + request_data = transaction_event["request"] + + if expect_ip: + assert request_data["env"] == {"REMOTE_ADDR": "127.0.0.1"} + else: + assert "env" not in request_data + + +@pytest.mark.asyncio +@pytest.mark.parametrize("init_kwargs, has_client, expect_ip", USER_INFO_CASES) +async def test_get_request_attributes_client_address_user_info( + sentry_init, capture_items, asgi3_app, init_kwargs, has_client, expect_ip +): + kwargs = {k: v for k, v in init_kwargs.items() if k != "_experiments"} + experiments = {"trace_lifecycle": "stream", **init_kwargs.get("_experiments", {})} + sentry_init(traces_sample_rate=1.0, _experiments=experiments, **kwargs) + app = SentryAsgiMiddleware(asgi3_app) + + scope = _http_scope() + if has_client: + scope["client"] = ("127.0.0.1", 60457) + + items = capture_items("span") + async with TestClient(app, scope=scope) as client: + await client.get("/foo", headers={"host": "example.com"}) + + sentry_sdk.flush() + + assert len(items) == 1 + attributes = items[0].payload["attributes"] + + if expect_ip: + assert attributes["client.address"] == "127.0.0.1" + else: + assert "client.address" not in attributes + + @pytest.mark.asyncio @pytest.mark.parametrize( "request_url,transaction_style,expected_transaction_name,expected_transaction_source", diff --git a/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryAllowlist/.gitignore b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryAllowlist/.gitignore new file mode 100644 index 0000000000..1c56884372 --- /dev/null +++ b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryAllowlist/.gitignore @@ -0,0 +1,11 @@ +# Need to add some ignore rules in this directory, because the unit tests will add the Sentry SDK and its dependencies +# into this directory to create a Lambda function package that contains everything needed to instrument a Lambda function using Sentry. + +# Ignore everything +* + +# But not index.py +!index.py + +# And not .gitignore itself +!.gitignore diff --git a/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryAllowlist/index.py b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryAllowlist/index.py new file mode 100644 index 0000000000..67ed8be6e3 --- /dev/null +++ b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryAllowlist/index.py @@ -0,0 +1,24 @@ +import os + +import sentry_sdk +from sentry_sdk.integrations.aws_lambda import AwsLambdaIntegration + +sentry_sdk.init( + dsn=os.environ.get("SENTRY_DSN"), + traces_sample_rate=1.0, + integrations=[AwsLambdaIntegration()], + _experiments={ + "data_collection": { + "url_query_params": { + "mode": "allowlist", + # "token" is allowlisted on purpose to show that an allowlist + # entry cannot override the built-in sensitive denylist. + "terms": ["page", "token"], + } + } + }, +) + + +def handler(event, context): + return {"event": event} diff --git a/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryDenylist/.gitignore b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryDenylist/.gitignore new file mode 100644 index 0000000000..1c56884372 --- /dev/null +++ b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryDenylist/.gitignore @@ -0,0 +1,11 @@ +# Need to add some ignore rules in this directory, because the unit tests will add the Sentry SDK and its dependencies +# into this directory to create a Lambda function package that contains everything needed to instrument a Lambda function using Sentry. + +# Ignore everything +* + +# But not index.py +!index.py + +# And not .gitignore itself +!.gitignore diff --git a/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryDenylist/index.py b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryDenylist/index.py new file mode 100644 index 0000000000..389d7890ea --- /dev/null +++ b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryDenylist/index.py @@ -0,0 +1,24 @@ +import os + +import sentry_sdk +from sentry_sdk.integrations.aws_lambda import AwsLambdaIntegration + +sentry_sdk.init( + dsn=os.environ.get("SENTRY_DSN"), + traces_sample_rate=1.0, + integrations=[AwsLambdaIntegration()], + _experiments={ + "data_collection": { + "url_query_params": { + "mode": "denylist", + # Custom terms deny otherwise non-sensitive query params on top + # of the built-in sensitive denylist. + "terms": ["tracking"], + } + } + }, +) + + +def handler(event, context): + return {"event": event} diff --git a/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryOff/.gitignore b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryOff/.gitignore new file mode 100644 index 0000000000..1c56884372 --- /dev/null +++ b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryOff/.gitignore @@ -0,0 +1,11 @@ +# Need to add some ignore rules in this directory, because the unit tests will add the Sentry SDK and its dependencies +# into this directory to create a Lambda function package that contains everything needed to instrument a Lambda function using Sentry. + +# Ignore everything +* + +# But not index.py +!index.py + +# And not .gitignore itself +!.gitignore diff --git a/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryOff/index.py b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryOff/index.py new file mode 100644 index 0000000000..02446562d9 --- /dev/null +++ b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkDataCollectionUrlQueryOff/index.py @@ -0,0 +1,19 @@ +import os + +import sentry_sdk +from sentry_sdk.integrations.aws_lambda import AwsLambdaIntegration + +sentry_sdk.init( + dsn=os.environ.get("SENTRY_DSN"), + traces_sample_rate=1.0, + integrations=[AwsLambdaIntegration()], + _experiments={ + "data_collection": { + "url_query_params": {"mode": "off"}, + } + }, +) + + +def handler(event, context): + return {"event": event} diff --git a/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkSpanStreamingDataCollection/.gitignore b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkSpanStreamingDataCollection/.gitignore new file mode 100644 index 0000000000..1c56884372 --- /dev/null +++ b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkSpanStreamingDataCollection/.gitignore @@ -0,0 +1,11 @@ +# Need to add some ignore rules in this directory, because the unit tests will add the Sentry SDK and its dependencies +# into this directory to create a Lambda function package that contains everything needed to instrument a Lambda function using Sentry. + +# Ignore everything +* + +# But not index.py +!index.py + +# And not .gitignore itself +!.gitignore diff --git a/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkSpanStreamingDataCollection/index.py b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkSpanStreamingDataCollection/index.py new file mode 100644 index 0000000000..b1ee2ec8a6 --- /dev/null +++ b/tests/integrations/aws_lambda/lambda_functions_with_embedded_sdk/BasicOkSpanStreamingDataCollection/index.py @@ -0,0 +1,23 @@ +import os + +import sentry_sdk +from sentry_sdk.integrations.aws_lambda import AwsLambdaIntegration + +sentry_sdk.init( + dsn=os.environ.get("SENTRY_DSN"), + traces_sample_rate=1.0, + integrations=[AwsLambdaIntegration()], + trace_lifecycle="stream", + _experiments={ + "data_collection": { + "url_query_params": { + "mode": "denylist", + "terms": ["tracking"], + } + }, + }, +) + + +def handler(event, context): + return {"event": event} diff --git a/tests/integrations/aws_lambda/test_aws_lambda.py b/tests/integrations/aws_lambda/test_aws_lambda.py index 2268d772cc..3c03e1510b 100644 --- a/tests/integrations/aws_lambda/test_aws_lambda.py +++ b/tests/integrations/aws_lambda/test_aws_lambda.py @@ -626,6 +626,142 @@ def test_request_data_with_data_collection_off(lambda_client, test_environment): } +def test_url_query_params_with_data_collection_denylist( + lambda_client, test_environment +): + payload = b""" + { + "resource": "/asd", + "path": "/asd", + "httpMethod": "GET", + "headers": { + "Host": "iwsz2c7uwi.execute-api.us-east-1.amazonaws.com", + "X-Forwarded-Proto": "https" + }, + "queryStringParameters": { + "page": "2", + "tracking": "campaign", + "token": "secret-token" + }, + "pathParameters": null, + "stageVariables": null, + "requestContext": { + "identity": { + "sourceIp": "213.47.147.207", + "userArn": "42" + } + }, + "body": null, + "isBase64Encoded": false + } + """ + + lambda_client.invoke( + FunctionName="BasicOkDataCollectionUrlQueryDenylist", + Payload=payload, + ) + envelopes = test_environment["server"].envelopes + + (transaction_event,) = envelopes + + assert transaction_event["request"]["query_string"] == { + # Not denied by any term -> pass through. + "page": "2", + # Denied by custom terms. + "tracking": "[Filtered]", + # Denied by the built-in sensitive denylist. + "token": "[Filtered]", + } + + +def test_url_query_params_with_data_collection_allowlist( + lambda_client, test_environment +): + payload = b""" + { + "resource": "/asd", + "path": "/asd", + "httpMethod": "GET", + "headers": { + "Host": "iwsz2c7uwi.execute-api.us-east-1.amazonaws.com", + "X-Forwarded-Proto": "https" + }, + "queryStringParameters": { + "page": "2", + "tracking": "campaign", + "token": "secret-token" + }, + "pathParameters": null, + "stageVariables": null, + "requestContext": { + "identity": { + "sourceIp": "213.47.147.207", + "userArn": "42" + } + }, + "body": null, + "isBase64Encoded": false + } + """ + + lambda_client.invoke( + FunctionName="BasicOkDataCollectionUrlQueryAllowlist", + Payload=payload, + ) + envelopes = test_environment["server"].envelopes + + (transaction_event,) = envelopes + + assert transaction_event["request"]["query_string"] == { + # Allowlisted, non-sensitive -> pass through. + "page": "2", + # Not allowlisted -> substituted. + "tracking": "[Filtered]", + # Allowlisted but sensitive -> still filtered; an allowlist entry + # cannot override the built-in sensitive denylist. + "token": "[Filtered]", + } + + +def test_url_query_params_with_data_collection_off(lambda_client, test_environment): + payload = b""" + { + "resource": "/asd", + "path": "/asd", + "httpMethod": "GET", + "headers": { + "Host": "iwsz2c7uwi.execute-api.us-east-1.amazonaws.com", + "X-Forwarded-Proto": "https" + }, + "queryStringParameters": { + "page": "2", + "tracking": "campaign" + }, + "pathParameters": null, + "stageVariables": null, + "requestContext": { + "identity": { + "sourceIp": "213.47.147.207", + "userArn": "42" + } + }, + "body": null, + "isBase64Encoded": false + } + """ + + lambda_client.invoke( + FunctionName="BasicOkDataCollectionUrlQueryOff", + Payload=payload, + ) + envelopes = test_environment["server"].envelopes + + (transaction_event,) = envelopes + + # With url_query_params collection turned off, no query string is collected. + assert "query_string" not in transaction_event["request"] + + def test_trace_continuation(lambda_client, test_environment): trace_id = "471a43a4192642f0b136d5159a501701" parent_span_id = "6e8f22c393e68f19" @@ -933,6 +1069,38 @@ def test_span_streaming_request_attributes(lambda_client, test_environment): assert _get_span_attr(attrs, "aws.log.stream.names") == ["$LATEST"] +def test_span_streaming_url_query_params_with_data_collection( + lambda_client, test_environment +): + payload = { + "httpMethod": "GET", + "queryStringParameters": { + "page": "2", + "tracking": "campaign", + "token": "secret-token", + }, + "path": "/test", + } + + lambda_client.invoke( + FunctionName="BasicOkSpanStreamingDataCollection", + Payload=json.dumps(payload), + ) + span_items = test_environment["server"].span_items + + segment_spans = [s for s in span_items if s["is_segment"]] + assert len(segment_spans) == 1 + segment_span = segment_spans[0] + attrs = segment_span["attributes"] + + # "page" passes through; "tracking" is denied by a custom term and "token" + # by the built-in sensitive denylist. + assert ( + _get_span_attr(attrs, "url.query") + == "page=2&tracking=%5BFiltered%5D&token=%5BFiltered%5D" + ) + + @pytest.mark.parametrize( "lambda_function_name", ["RaiseErrorPerformanceEnabled", "RaiseErrorPerformanceDisabled"], diff --git a/tests/integrations/gcp/test_gcp.py b/tests/integrations/gcp/test_gcp.py index daf1c6c129..b02518a413 100644 --- a/tests/integrations/gcp/test_gcp.py +++ b/tests/integrations/gcp/test_gcp.py @@ -786,3 +786,170 @@ def cloud_function(functionhandler, event): assert attrs["gcp.project.id"] == "serverless_project" assert attrs["faas.identity"] == "func_ID" assert attrs["faas.entry_point"] == "cloud_function" + + +# Each case is (send_default_pii, data_collection, expected_event, expected_span). +# ``send_default_pii`` / ``data_collection`` of None means the option is omitted. +# The two paths only diverge for the legacy (no ``data_collection``) rows: the +# event processor always records the raw query string, while the span-streaming +# path gates the ``url.query`` attribute on ``send_default_pii``. +_QUERY_STRING_DATA_COLLECTION_CASES = [ + pytest.param( + True, + None, + "toy=tennisball&color=red&auth=secret", + "toy=tennisball&color=red&auth=secret", + id="send_default_pii_true", + ), + pytest.param( + False, + None, + "toy=tennisball&color=red&auth=secret", + None, + id="send_default_pii_false", + ), + pytest.param( + None, + None, + "toy=tennisball&color=red&auth=secret", + None, + id="defaults", + ), + # data_collection configured: query string is routed through filtering. + # Spec defaults -> denylist: only the sensitive ``auth`` is redacted. + pytest.param( + None, + {}, + "toy=tennisball&color=red&auth=%5BFiltered%5D", + "toy=tennisball&color=red&auth=%5BFiltered%5D", + id="data_collection_denylist_default", + ), + pytest.param( + None, + {"url_query_params": {"mode": "denylist", "terms": ["toy"]}}, + "toy=%5BFiltered%5D&color=red&auth=%5BFiltered%5D", + "toy=%5BFiltered%5D&color=red&auth=%5BFiltered%5D", + id="data_collection_denylist_custom_terms", + ), + # allowlist with only ``toy`` allowed: ``color`` is redacted even though it + # is not sensitive, proving the redaction comes from the allowlist. + pytest.param( + None, + {"url_query_params": {"mode": "allowlist", "terms": ["toy"]}}, + "toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D", + "toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D", + id="data_collection_allowlist", + ), + pytest.param( + None, + {"url_query_params": {"mode": "off"}}, + None, + None, + id="data_collection_off", + ), +] + + +def _build_init_kwargs(send_default_pii, data_collection): + """Render the keyword-argument string passed to ``init_sdk`` in the + subprocess.""" + kwargs = [] + if send_default_pii is not None: + kwargs.append("send_default_pii=%r" % send_default_pii) + if data_collection is not None: + kwargs.append("_experiments=%r" % {"data_collection": data_collection}) + + return ", ".join(kwargs) + + +@pytest.mark.parametrize( + "send_default_pii, data_collection, expected_event, expected_span", + _QUERY_STRING_DATA_COLLECTION_CASES, +) +def test_query_string_data_collection_event_processor( + run_cloud_function, + send_default_pii, + data_collection, + expected_event, + expected_span, +): + init_kwargs = _build_init_kwargs(send_default_pii, data_collection) + envelope_items, _, _ = run_cloud_function( + dedent( + """ + functionhandler = None + + from collections import namedtuple + GCPEvent = namedtuple("GCPEvent", ["headers", "method", "query_string"]) + event = GCPEvent( + headers={}, + method="GET", + query_string=b"toy=tennisball&color=red&auth=secret", + ) + + def cloud_function(functionhandler, event): + raise Exception("something went wrong") + """ + ) + + FUNCTIONS_PRELUDE + + dedent( + """ + init_sdk(%s) + gcp_functions.worker_v1.FunctionHandler.invoke_user_function(functionhandler, event) + """ + % init_kwargs + ) + ) + + request = envelope_items[0]["request"] + if expected_event is None: + assert "query_string" not in request + else: + assert request["query_string"] == expected_event + + +@pytest.mark.parametrize( + "send_default_pii, data_collection, expected_event, expected_span", + _QUERY_STRING_DATA_COLLECTION_CASES, +) +def test_query_string_data_collection_span_streaming( + run_cloud_function, + send_default_pii, + data_collection, + expected_event, + expected_span, +): + init_kwargs = _build_init_kwargs(send_default_pii, data_collection) + _, _, span_items = run_cloud_function( + dedent( + """ + functionhandler = None + + from collections import namedtuple + GCPEvent = namedtuple("GCPEvent", ["headers", "method", "query_string"]) + event = GCPEvent( + headers={}, + method="GET", + query_string=b"toy=tennisball&color=red&auth=secret", + ) + + def cloud_function(functionhandler, event): + return "ok" + """ + ) + + FUNCTIONS_PRELUDE + + dedent( + """ + init_sdk(traces_sample_rate=1.0, trace_lifecycle="stream", %s) + gcp_functions.worker_v1.FunctionHandler.invoke_user_function(functionhandler, event) + """ + % init_kwargs + ) + ) + + assert len(span_items) == 1 + attrs = span_items[0]["attributes"] + if expected_span is None: + assert "url.query" not in attrs + else: + assert attrs["url.query"] == expected_span diff --git a/tests/integrations/quart/test_quart.py b/tests/integrations/quart/test_quart.py index b2172d80e8..286d6aeaff 100644 --- a/tests/integrations/quart/test_quart.py +++ b/tests/integrations/quart/test_quart.py @@ -1131,3 +1131,154 @@ async def test_span_streaming_sensitive_header_passthrough_with_pii_and_no_data_ segment["attributes"]["http.request.header.authorization"] == "Bearer secret-token" ) + + +_QUERY_PARAM_DATA_COLLECTION_CASES = [ + pytest.param( + {"send_default_pii": True}, + "toy=tennisball&color=red&auth=secret", + id="send_default_pii_true", + ), + pytest.param( + {"send_default_pii": False}, + None, + id="send_default_pii_false", + ), + pytest.param( + {}, + None, + id="defaults", + ), + pytest.param( + {"_experiments": {"data_collection": {}}}, + "toy=tennisball&color=red&auth=%5BFiltered%5D", + id="data_collection_denylist_default", + ), + pytest.param( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "denylist", "terms": ["toy"]} + } + } + }, + "toy=%5BFiltered%5D&color=red&auth=%5BFiltered%5D", + id="data_collection_denylist_custom_terms", + ), + pytest.param( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "allowlist", "terms": ["toy"]} + } + } + }, + "toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D", + id="data_collection_allowlist", + ), + pytest.param( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "allowlist", "terms": ["auth"]} + } + } + }, + "toy=%5BFiltered%5D&color=%5BFiltered%5D&auth=%5BFiltered%5D", + id="data_collection_allowlist_sensitive_term", + ), + pytest.param( + {"_experiments": {"data_collection": {"url_query_params": {"mode": "off"}}}}, + None, + id="data_collection_off", + ), + pytest.param( + { + "send_default_pii": True, + "_experiments": {"data_collection": {"url_query_params": {"mode": "off"}}}, + }, + None, + id="data_collection_wins_over_send_default_pii", + ), +] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "init_kwargs, expected_query", _QUERY_PARAM_DATA_COLLECTION_CASES +) +async def test_span_streaming_url_query_data_collection( + sentry_init, capture_items, init_kwargs, expected_query +): + init_kwargs = dict(init_kwargs) + experiments = {"trace_lifecycle": "stream"} + experiments.update(init_kwargs.pop("_experiments", {})) + sentry_init( + integrations=[quart_sentry.QuartIntegration()], + traces_sample_rate=1.0, + _experiments=experiments, + **init_kwargs, + ) + items = capture_items("span") + + app = quart_app_factory() + client = app.test_client() + response = await client.get("/message?toy=tennisball&color=red&auth=secret") + assert response.status_code == 200 + + sentry_sdk.flush() + + spans = [item.payload for item in items] + assert len(spans) == 1 + + segment = spans[0] + + data_collection_enabled = "data_collection" in experiments + url_attrs_expected = data_collection_enabled or init_kwargs.get( + "send_default_pii", False + ) + + if expected_query is None: + assert "url.query" not in segment["attributes"] + if url_attrs_expected: + # When the filtered query string is empty, url.full carries the + # base URL only (no query). + assert segment["attributes"]["url.full"] == "http://localhost/message" + else: + assert "url.full" not in segment["attributes"] + else: + assert segment["attributes"]["url.query"] == expected_query + assert segment["attributes"]["url.full"] == ( + f"http://localhost/message?{expected_query}" + ) + + +@pytest.mark.asyncio +async def test_span_streaming_url_query_multi_and_blank_values( + sentry_init, capture_items +): + sentry_init( + integrations=[quart_sentry.QuartIntegration()], + traces_sample_rate=1.0, + _experiments={"trace_lifecycle": "stream", "data_collection": {}}, + ) + items = capture_items("span") + + app = quart_app_factory() + client = app.test_client() + response = await client.get("/message?foo=1&foo=2&empty=") + assert response.status_code == 200 + + sentry_sdk.flush() + + spans = [item.payload for item in items] + assert len(spans) == 1 + + segment = spans[0] + query = segment["attributes"]["url.query"] + # Repeated keys are preserved and blank values are kept. + assert "foo=1" in query + assert "foo=2" in query + assert "empty=" in query + # url.full carries the same filtered query string. + assert segment["attributes"]["url.full"] == f"http://localhost/message?{query}" diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index 6ead9f12b5..bf0d7b0993 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -798,6 +798,106 @@ def test_span_http_query_data_collection( assert attributes["url.path"] == "/message" +# TestClient provides ("testclient", 50000) as the scope's client. +TESTCLIENT_IP = "testclient" +NO_USER_INFO = object() + +USER_INFO_CASES = [ + pytest.param( + {"send_default_pii": True}, + TESTCLIENT_IP, + id="legacy_send_default_pii_true", + ), + pytest.param( + {"send_default_pii": False}, + NO_USER_INFO, + id="legacy_send_default_pii_false", + ), + pytest.param( + {"_experiments": {"data_collection": {}}}, + TESTCLIENT_IP, + id="data_collection_default_user_info_true", + ), + pytest.param( + {"_experiments": {"data_collection": {"user_info": True}}}, + TESTCLIENT_IP, + id="data_collection_user_info_true", + ), + pytest.param( + {"_experiments": {"data_collection": {"user_info": False}}}, + NO_USER_INFO, + id="data_collection_user_info_false", + ), + pytest.param( + { + "send_default_pii": True, + "_experiments": {"data_collection": {"user_info": False}}, + }, + NO_USER_INFO, + id="data_collection_wins_over_send_default_pii", + ), +] + + +@pytest.mark.parametrize("init_kwargs, expected_ip", USER_INFO_CASES) +def test_user_info_data_collection( + sentry_init, capture_events, init_kwargs, expected_ip +): + sentry_init( + traces_sample_rate=1.0, + integrations=[StarletteIntegration()], + **init_kwargs, + ) + + starlette_app = starlette_app_factory() + events = capture_events() + + client = TestClient(starlette_app) + client.get("/message") + + (event, transaction_event) = events + + if expected_ip is NO_USER_INFO: + assert "env" not in event["request"] + assert "env" not in transaction_event["request"] + else: + assert event["request"]["env"] == {"REMOTE_ADDR": expected_ip} + assert transaction_event["request"]["env"] == {"REMOTE_ADDR": expected_ip} + + +@pytest.mark.parametrize("init_kwargs, expected_ip", USER_INFO_CASES) +def test_user_info_data_collection_with_streamed_spans( + sentry_init, capture_items, init_kwargs, expected_ip +): + kwargs = dict(init_kwargs) + sentry_init( + auto_enabling_integrations=False, + integrations=[StarletteIntegration()], + traces_sample_rate=1.0, + trace_lifecycle="stream", + _experiments={ + **kwargs.pop("_experiments", {}), + }, + **kwargs, + ) + + starlette_app = starlette_app_factory() + items = capture_items("span") + + client = TestClient(starlette_app) + client.get("/message") + + sentry_sdk.flush() + + assert len(items) == 1 + attributes = items[0].payload["attributes"] + + if expected_ip is NO_USER_INFO: + assert SPANDATA.CLIENT_ADDRESS not in attributes + else: + assert attributes[SPANDATA.CLIENT_ADDRESS] == expected_ip + + @pytest.mark.parametrize( "url,transaction_style,expected_transaction,expected_source", [