feat(wsgi): Apply data_collection filtering to URL query strings#6827
3 issues
find-bugs: Found 3 issues (2 medium, 1 low)
Medium
Filtered query string is reassembled without URL-encoding, corrupting values and re-revealing scrubbed data - `sentry_sdk/integrations/wsgi.py:379-382`
_apply_data_collection_filtering_to_query_string parses the query string with parse_qs, which percent-decodes keys and values and splits on &/=, then rebuilds the string with plain f"{key}={item}" joined by & without re-encoding. Decoded values containing &/= split into extra apparent params, so a sensitive value that was scrubbed to [Filtered] under one key can reappear intact when it was embedded (URL-encoded) inside another kept key's value. Benign percent-encoded values (e.g. q=hello%20world) are also silently corrupted in the reported query string.
Also found at:
sentry_sdk/data_collection.py:107-116
client.address span attribute never set when data_collection is enabled - `sentry_sdk/integrations/wsgi.py:428-446`
When data_collection is configured, the if has_data_collection_enabled(...) branch is taken and never adds client.address, so client IP is silently dropped on spans even when user_info is enabled — a regression from the should_send_default_pii() path which set it.
Low
test_span_http_query_data_collection mutates parametrize input dict via pop - `tests/integrations/wsgi/test_wsgi.py:1277-1281`
init_kwargs.pop("_experiments", {}) mutates the shared parametrize param dict, so a re-run of the same param (e.g. via pytest-rerunfailures/flaky) would no longer see _experiments and silently exercise the wrong configuration (the data_collection config would be dropped and the assertion would check a different code path than intended).
⏱ 6m 49s · 884.7k in / 50.8k out · $3.12
Annotations
Check warning on line 382 in sentry_sdk/integrations/wsgi.py
sentry-warden / warden: find-bugs
Filtered query string is reassembled without URL-encoding, corrupting values and re-revealing scrubbed data
`_apply_data_collection_filtering_to_query_string` parses the query string with `parse_qs`, which percent-decodes keys and values and splits on `&`/`=`, then rebuilds the string with plain `f"{key}={item}"` joined by `&` without re-encoding. Decoded values containing `&`/`=` split into extra apparent params, so a sensitive value that was scrubbed to `[Filtered]` under one key can reappear intact when it was embedded (URL-encoded) inside another kept key's value. Benign percent-encoded values (e.g. `q=hello%20world`) are also silently corrupted in the reported query string.
Check warning on line 116 in sentry_sdk/data_collection.py
sentry-warden / warden: find-bugs
[SYF-5NQ] Filtered query string is reassembled without URL-encoding, corrupting values and re-revealing scrubbed data (additional location)
`_apply_data_collection_filtering_to_query_string` parses the query string with `parse_qs`, which percent-decodes keys and values and splits on `&`/`=`, then rebuilds the string with plain `f"{key}={item}"` joined by `&` without re-encoding. Decoded values containing `&`/`=` split into extra apparent params, so a sensitive value that was scrubbed to `[Filtered]` under one key can reappear intact when it was embedded (URL-encoded) inside another kept key's value. Benign percent-encoded values (e.g. `q=hello%20world`) are also silently corrupted in the reported query string.
Check warning on line 446 in sentry_sdk/integrations/wsgi.py
sentry-warden / warden: find-bugs
client.address span attribute never set when data_collection is enabled
When `data_collection` is configured, the `if has_data_collection_enabled(...)` branch is taken and never adds `client.address`, so client IP is silently dropped on spans even when `user_info` is enabled — a regression from the `should_send_default_pii()` path which set it.