Skip to content

Commit c0f2100

Browse files
ericapisaniericapisani
authored
feat(wsgi): Apply data_collection filtering to URL query strings (getsentry#6827)
Filter WSGI request event and span query strings through the data_collection.url_query_params behaviour (denylist/allowlist/off) instead of gating query string capture solely on send_default_pii. This brings WSGI-based integrations in line with the data collection spec's handling of cookies and headers. Rename the data_collection field query_params to url_query_params to match the updated data collection spec, which distinguishes URL query params from other key-value data (cookies, headers). Update the DataCollection/DataCollectionUserOptions TypedDicts, resolution logic in data_collection.py, and all WSGI/Flask/Django test coverage accordingly. Refs PY-2583
1 parent dcedc3a commit c0f2100

7 files changed

Lines changed: 604 additions & 23 deletions

File tree

‎sentry_sdk/_types.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ class DataCollectionUserOptions(TypedDict, total=False):
183183
cookies: "KeyValueCollectionBehaviour"
184184
http_headers: "HttpHeadersCollectionUserOptions"
185185
http_bodies: "List[str]"
186-
query_params: "KeyValueCollectionBehaviour"
186+
url_query_params: "KeyValueCollectionBehaviour"
187187
graphql: "GraphQLCollectionUserOptions"
188188
gen_ai: "GenAICollectionUserOptions"
189189
database_query_data: bool
@@ -197,7 +197,7 @@ class DataCollection(TypedDict):
197197
cookies: "KeyValueCollectionBehaviour"
198198
http_headers: "HttpHeadersCollectionBehaviour"
199199
http_bodies: "List[str]"
200-
query_params: "KeyValueCollectionBehaviour"
200+
url_query_params: "KeyValueCollectionBehaviour"
201201
graphql: "GraphQLCollectionBehaviour"
202202
gen_ai: "GenAICollectionBehaviour"
203203
database_query_data: bool

‎sentry_sdk/data_collection.py‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
88
``data_collection`` supersedes the single ``send_default_pii`` boolean with a
99
structured configuration that lets users enable or restrict automatically
10-
collected data by category (user identity, cookies, HTTP headers, query params,
10+
collected data by category (user identity, cookies, HTTP headers, URL query params,
1111
HTTP bodies, generative AI inputs/outputs, stack frame variables, source
1212
context).
1313
@@ -23,12 +23,13 @@
2323
"""
2424

2525
import warnings
26-
from typing import TYPE_CHECKING, List, Mapping, Optional, cast
26+
from typing import TYPE_CHECKING, List, Mapping, Optional, Union, cast
27+
from urllib.parse import parse_qs, urlencode
2728

2829
from sentry_sdk._types import SENSITIVE_DATA_SUBSTITUTE
2930

3031
if TYPE_CHECKING:
31-
from typing import Any, Dict, Literal
32+
from typing import Any, Dict
3233

3334
from sentry_sdk._types import (
3435
DataCollection,
@@ -50,7 +51,7 @@
5051
# Default number of source lines captured above and below a stack frame.
5152
_DEFAULT_FRAME_CONTEXT_LINES = 5
5253

53-
# Collection modes for key-value data (cookies, headers, query params).
54+
# Collection modes for key-value data (cookies, headers, URL query params).
5455
# snake_case (Python-only deviation from the spec's camelCase); never
5556
# serialized to Sentry.
5657
_VALID_KEY_VALUE_COLLECTION_BEHAVIOUR_MODES = ("off", "denylist", "allowlist")
@@ -98,6 +99,21 @@ def _is_sensitive_key(key: str, extra_terms: "Optional[List[str]]" = None) -> bo
9899
return False
99100

100101

102+
def _apply_data_collection_filtering_to_query_string(
103+
query_string: str,
104+
behaviour: "KeyValueCollectionBehaviour",
105+
) -> "Union[str, None]":
106+
parsed_qs = parse_qs(query_string, keep_blank_values=True)
107+
filtered_qs = _apply_key_value_collection_filtering(
108+
items=parsed_qs, behaviour=behaviour
109+
)
110+
111+
if filtered_qs:
112+
return urlencode(filtered_qs, doseq=True)
113+
114+
return None
115+
116+
101117
def _apply_key_value_collection_filtering(
102118
items: "Mapping[str, Any]",
103119
behaviour: "KeyValueCollectionBehaviour",
@@ -146,13 +162,12 @@ def _map_from_send_default_pii(
146162
``send_default_pii`` collects today. Used when ``data_collection`` is not
147163
provided explicitly.
148164
"""
149-
kv_mode: "Literal['denylist', 'off']" = "denylist" if send_default_pii else "off"
150165
terms = [] if send_default_pii else ["forwarded", "-ip", "remote-", "via", "-user"]
151166

152167
return {
153168
"provided_by_user": False,
154169
"user_info": send_default_pii,
155-
"cookies": {"mode": kv_mode, "terms": terms},
170+
"cookies": {"mode": "denylist", "terms": terms},
156171
# Headers are collected in both PII modes today (sensitive ones filtered
157172
# when PII is off), so this never maps to "off".
158173
"http_headers": {
@@ -161,7 +176,7 @@ def _map_from_send_default_pii(
161176
# Bodies are collected regardless of PII today, bounded by
162177
# ``max_request_body_size``.
163178
"http_bodies": list(_ALL_HTTP_BODY_TYPES),
164-
"query_params": {"mode": kv_mode, "terms": terms},
179+
"url_query_params": {"mode": "denylist", "terms": terms},
165180
"graphql": {"document": send_default_pii, "variables": send_default_pii},
166181
"gen_ai": {"inputs": send_default_pii, "outputs": send_default_pii},
167182
"database_query_data": send_default_pii,
@@ -211,7 +226,7 @@ def _resolve_explicit(
211226
"cookies": _kvcb_from_value(d.get("cookies") or {}),
212227
"http_headers": _http_headers_from_value(d.get("http_headers") or {}),
213228
"http_bodies": http_bodies,
214-
"query_params": _kvcb_from_value(d.get("query_params") or {}),
229+
"url_query_params": _kvcb_from_value(d.get("url_query_params") or {}),
215230
"graphql": _graphql_from_value(d.get("graphql") or {}),
216231
"gen_ai": _gen_ai_from_value(d.get("gen_ai") or {}),
217232
"database_query_data": d.get("database_query_data", True),

‎sentry_sdk/integrations/wsgi.py‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from sentry_sdk._werkzeug import _get_headers, get_host
77
from sentry_sdk.api import continue_trace
88
from sentry_sdk.consts import OP, SPANDATA
9+
from sentry_sdk.data_collection import _apply_data_collection_filtering_to_query_string
910
from sentry_sdk.integrations._wsgi_common import (
1011
DEFAULT_HTTP_METHODS_TO_CAPTURE,
1112
_filter_headers,
@@ -19,6 +20,7 @@
1920
ContextVar,
2021
capture_internal_exceptions,
2122
event_from_exception,
23+
has_data_collection_enabled,
2224
nullcontext,
2325
reraise,
2426
)
@@ -355,6 +357,7 @@ def _make_wsgi_event_processor(
355357
method = environ.get("REQUEST_METHOD")
356358
env = dict(_get_environ(environ))
357359
headers = _filter_headers(dict(_get_headers(environ)))
360+
client_options = sentry_sdk.get_client().options
358361

359362
def event_processor(event: "Event", hint: "Dict[str, Any]") -> "Event":
360363
with capture_internal_exceptions():
@@ -367,11 +370,22 @@ def event_processor(event: "Event", hint: "Dict[str, Any]") -> "Event":
367370
user_info.setdefault("ip_address", client_ip)
368371

369372
request_info["url"] = request_url
370-
request_info["query_string"] = query_string
371373
request_info["method"] = method
372374
request_info["env"] = env
373375
request_info["headers"] = headers
374376

377+
if has_data_collection_enabled(client_options):
378+
if query_string:
379+
filtered_qs = _apply_data_collection_filtering_to_query_string(
380+
query_string=query_string,
381+
behaviour=client_options["data_collection"]["url_query_params"],
382+
)
383+
if filtered_qs:
384+
request_info["query_string"] = filtered_qs
385+
else:
386+
# This was not originally gated so if data collection is not enabled, leave as-is.
387+
request_info["query_string"] = query_string
388+
375389
return event
376390

377391
return event_processor
@@ -409,7 +423,26 @@ def _get_request_attributes(
409423
except ValueError:
410424
pass
411425

412-
if should_send_default_pii():
426+
client_options = sentry_sdk.get_client().options
427+
428+
if has_data_collection_enabled(client_options):
429+
query_string = environ.get("QUERY_STRING")
430+
if query_string:
431+
filtered_qs = _apply_data_collection_filtering_to_query_string(
432+
query_string=query_string,
433+
behaviour=client_options["data_collection"]["url_query_params"],
434+
)
435+
436+
if filtered_qs:
437+
attributes["http.query"] = filtered_qs
438+
439+
path = environ.get("PATH_INFO", "")
440+
if path:
441+
attributes["url.path"] = path
442+
443+
attributes["url.full"] = get_request_url(environ, use_x_forwarded_for)
444+
445+
elif should_send_default_pii():
413446
client_ip = get_client_ip(environ)
414447
if client_ip:
415448
attributes["client.address"] = client_ip

‎tests/integrations/django/test_data_scrubbing.py‎

Lines changed: 180 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import pytest
22
from werkzeug.test import Client
33

4+
import sentry_sdk
5+
from sentry_sdk.consts import SPANDATA
46
from sentry_sdk.integrations.django import DjangoIntegration
5-
from tests.conftest import werkzeug_set_cookie
7+
from tests.conftest import unpack_werkzeug_response, werkzeug_set_cookie
68
from tests.integrations.django.myapp.wsgi import application
79
from tests.integrations.django.utils import pytest_mark_django_db_decorator
810

@@ -12,6 +14,9 @@
1214
from django.core.urlresolvers import reverse
1315

1416

17+
NO_COOKIES = object()
18+
19+
1520
@pytest.fixture
1621
def client():
1722
return Client(application)
@@ -214,3 +219,177 @@ def test_data_collection_cookies_precedence_over_send_default_pii(
214219
"csrftoken": "[Filtered]",
215220
"foo": "bar",
216221
}
222+
223+
224+
# Query string used across the query-param filtering tests below. ``auth`` is a
225+
# built-in sensitive term, so it is redacted by the default denylist.
226+
QUERY_STRING = "toy=tennisball&color=red&auth=secret"
227+
228+
229+
@pytest.mark.forked
230+
@pytest_mark_django_db_decorator()
231+
@pytest.mark.parametrize(
232+
"init_kwargs, expected_query_string",
233+
[
234+
pytest.param(
235+
{"send_default_pii": True},
236+
"toy=tennisball&color=red&auth=secret",
237+
id="legacy_send_default_pii_true",
238+
),
239+
pytest.param(
240+
{"send_default_pii": False},
241+
"toy=tennisball&color=red&auth=secret",
242+
id="legacy_send_default_pii_false",
243+
),
244+
pytest.param(
245+
{"_experiments": {"data_collection": {}}},
246+
"toy=tennisball&color=red&auth=%5BFiltered%5D",
247+
id="data_collection_denylist_default",
248+
),
249+
pytest.param(
250+
{
251+
"_experiments": {
252+
"data_collection": {
253+
"url_query_params": {"mode": "allowlist", "terms": ["toy"]}
254+
}
255+
}
256+
},
257+
"toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D",
258+
id="data_collection_allowlist",
259+
),
260+
pytest.param(
261+
{
262+
"_experiments": {
263+
"data_collection": {"url_query_params": {"mode": "off"}}
264+
}
265+
},
266+
None,
267+
id="data_collection_off",
268+
),
269+
],
270+
)
271+
def test_query_string_data_collection(
272+
sentry_init,
273+
client,
274+
capture_events,
275+
init_kwargs,
276+
expected_query_string,
277+
):
278+
sentry_init(integrations=[DjangoIntegration()], **init_kwargs)
279+
events = capture_events()
280+
281+
client.get(reverse("view_exc") + "?" + QUERY_STRING)
282+
283+
(event,) = events
284+
285+
if expected_query_string is None:
286+
assert "query_string" not in event["request"]
287+
else:
288+
assert event["request"]["query_string"] == expected_query_string
289+
290+
291+
@pytest.mark.forked
292+
@pytest_mark_django_db_decorator()
293+
@pytest.mark.parametrize(
294+
"init_kwargs, expected_query",
295+
[
296+
pytest.param(
297+
{"send_default_pii": True},
298+
"toy=tennisball&color=red&auth=secret",
299+
id="legacy_send_default_pii_true",
300+
),
301+
pytest.param(
302+
{"send_default_pii": False},
303+
None,
304+
id="legacy_send_default_pii_false",
305+
),
306+
pytest.param(
307+
{"_experiments": {"data_collection": {}}},
308+
"toy=tennisball&color=red&auth=%5BFiltered%5D",
309+
id="data_collection_denylist_default",
310+
),
311+
pytest.param(
312+
{
313+
"_experiments": {
314+
"data_collection": {
315+
"url_query_params": {"mode": "allowlist", "terms": ["toy"]}
316+
}
317+
}
318+
},
319+
"toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D",
320+
id="data_collection_allowlist",
321+
),
322+
pytest.param(
323+
{
324+
"_experiments": {
325+
"data_collection": {"url_query_params": {"mode": "off"}}
326+
}
327+
},
328+
None,
329+
id="data_collection_off",
330+
),
331+
],
332+
)
333+
def test_span_http_query_data_collection(
334+
sentry_init,
335+
client,
336+
capture_items,
337+
init_kwargs,
338+
expected_query,
339+
):
340+
sentry_init(
341+
integrations=[DjangoIntegration()],
342+
traces_sample_rate=1.0,
343+
_experiments={
344+
"trace_lifecycle": "stream",
345+
**init_kwargs.pop("_experiments", {}),
346+
},
347+
**init_kwargs,
348+
)
349+
350+
items = capture_items("span")
351+
352+
unpack_werkzeug_response(client.get(reverse("message") + "?" + QUERY_STRING))
353+
354+
sentry_sdk.flush()
355+
356+
spans = [item.payload for item in items]
357+
(root_span,) = (span for span in spans if span["name"] == "/message")
358+
359+
if expected_query is None:
360+
assert SPANDATA.HTTP_QUERY not in root_span["attributes"]
361+
else:
362+
assert root_span["attributes"][SPANDATA.HTTP_QUERY] == expected_query
363+
364+
365+
@pytest.mark.forked
366+
@pytest_mark_django_db_decorator()
367+
def test_query_string_empty_legacy_emits_empty_string(
368+
sentry_init, client, capture_events
369+
):
370+
sentry_init(integrations=[DjangoIntegration()], send_default_pii=True)
371+
events = capture_events()
372+
373+
client.get(reverse("view_exc"))
374+
375+
(event,) = events
376+
assert event["request"]["query_string"] == ""
377+
378+
379+
@pytest.mark.forked
380+
@pytest_mark_django_db_decorator()
381+
def test_empty_query_string_is_dropped_with_data_collection(
382+
sentry_init, client, capture_events
383+
):
384+
# ``data_collection`` path: an empty query string is dropped entirely to
385+
# reduce envelope size, so the ``query_string`` key is absent.
386+
sentry_init(
387+
integrations=[DjangoIntegration()],
388+
_experiments={"data_collection": {}},
389+
)
390+
events = capture_events()
391+
392+
client.get(reverse("view_exc"))
393+
394+
(event,) = events
395+
assert "query_string" not in event["request"]

0 commit comments

Comments
 (0)