-
Notifications
You must be signed in to change notification settings - Fork 642
feat(wsgi): Apply data_collection filtering to URL query strings #6827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7541460
5e9f610
a590c8e
16ed0c8
486f0bc
7d14a81
bb98334
61d1618
951c408
a19be39
620e222
ec6f520
4dd3ba6
d0bae90
a6c4688
d7b378b
52c2da1
aa645d7
8b6e3ad
d717172
c7f590f
293a291
530e83c
326d796
e9f0e20
48f7c7b
621d79b
3d7bddb
fad2292
40d9f38
e6e7148
1f9bce7
fad0c9a
5372ea6
7f12491
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
|
|
||
| ``data_collection`` supersedes the single ``send_default_pii`` boolean with a | ||
| structured configuration that lets users enable or restrict automatically | ||
| collected data by category (user identity, cookies, HTTP headers, query params, | ||
| collected data by category (user identity, cookies, HTTP headers, URL query params, | ||
| HTTP bodies, generative AI inputs/outputs, stack frame variables, source | ||
| context). | ||
|
|
||
|
|
@@ -23,12 +23,13 @@ | |
| """ | ||
|
|
||
| import warnings | ||
| from typing import TYPE_CHECKING, List, Mapping, Optional, cast | ||
| from typing import TYPE_CHECKING, List, Mapping, Optional, Union, cast | ||
| from urllib.parse import parse_qs, urlencode | ||
|
|
||
| from sentry_sdk._types import SENSITIVE_DATA_SUBSTITUTE | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing import Any, Dict, Literal | ||
| from typing import Any, Dict | ||
|
|
||
| from sentry_sdk._types import ( | ||
| DataCollection, | ||
|
|
@@ -50,7 +51,7 @@ | |
| # Default number of source lines captured above and below a stack frame. | ||
| _DEFAULT_FRAME_CONTEXT_LINES = 5 | ||
|
|
||
| # Collection modes for key-value data (cookies, headers, query params). | ||
| # Collection modes for key-value data (cookies, headers, URL query params). | ||
| # snake_case (Python-only deviation from the spec's camelCase); never | ||
| # serialized to Sentry. | ||
| _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 | |
| return False | ||
|
|
||
|
|
||
| def _apply_data_collection_filtering_to_query_string( | ||
| query_string: str, | ||
| behaviour: "KeyValueCollectionBehaviour", | ||
| ) -> "Union[str, None]": | ||
| parsed_qs = parse_qs(query_string, keep_blank_values=True) | ||
| filtered_qs = _apply_key_value_collection_filtering( | ||
| items=parsed_qs, behaviour=behaviour | ||
| ) | ||
|
|
||
| if filtered_qs: | ||
| return urlencode(filtered_qs, doseq=True) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Query string left URL-encodedHigh Severity
Reviewed by Cursor Bugbot for commit 7f12491. Configure here.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was done to address #6827 (comment) . I'll be following up with the wider team to understand if the data collection spec needs to be updated. |
||
|
|
||
| return None | ||
|
ericapisani marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def _apply_key_value_collection_filtering( | ||
| items: "Mapping[str, Any]", | ||
| behaviour: "KeyValueCollectionBehaviour", | ||
|
|
@@ -146,13 +162,12 @@ def _map_from_send_default_pii( | |
| ``send_default_pii`` collects today. Used when ``data_collection`` is not | ||
| provided explicitly. | ||
| """ | ||
| kv_mode: "Literal['denylist', 'off']" = "denylist" if send_default_pii else "off" | ||
| terms = [] if send_default_pii else ["forwarded", "-ip", "remote-", "via", "-user"] | ||
|
|
||
| return { | ||
| "provided_by_user": False, | ||
| "user_info": send_default_pii, | ||
| "cookies": {"mode": kv_mode, "terms": terms}, | ||
| "cookies": {"mode": "denylist", "terms": terms}, | ||
| # Headers are collected in both PII modes today (sensitive ones filtered | ||
| # when PII is off), so this never maps to "off". | ||
| "http_headers": { | ||
|
|
@@ -161,7 +176,7 @@ def _map_from_send_default_pii( | |
| # Bodies are collected regardless of PII today, bounded by | ||
| # ``max_request_body_size``. | ||
| "http_bodies": list(_ALL_HTTP_BODY_TYPES), | ||
| "query_params": {"mode": kv_mode, "terms": terms}, | ||
| "url_query_params": {"mode": "denylist", "terms": terms}, | ||
|
ericapisani marked this conversation as resolved.
|
||
| "graphql": {"document": send_default_pii, "variables": send_default_pii}, | ||
| "gen_ai": {"inputs": send_default_pii, "outputs": send_default_pii}, | ||
| "database_query_data": send_default_pii, | ||
|
|
@@ -211,7 +226,7 @@ def _resolve_explicit( | |
| "cookies": _kvcb_from_value(d.get("cookies") or {}), | ||
| "http_headers": _http_headers_from_value(d.get("http_headers") or {}), | ||
| "http_bodies": http_bodies, | ||
| "query_params": _kvcb_from_value(d.get("query_params") or {}), | ||
| "url_query_params": _kvcb_from_value(d.get("url_query_params") or {}), | ||
| "graphql": _graphql_from_value(d.get("graphql") or {}), | ||
| "gen_ai": _gen_ai_from_value(d.get("gen_ai") or {}), | ||
| "database_query_data": d.get("database_query_data", True), | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably use
urlencodefor the reassembly. Query strings can be finicky so it'd be safer to rely on the stdlib here.