diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index e3c6370c9c..59f9018912 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -358,7 +358,8 @@ def _get_options(*args: "Optional[str]", **kwargs: "Any") -> "Dict[str, Any]": rv["event_scrubber"] = EventScrubber( send_default_pii=False if rv["send_default_pii"] is None - else rv["send_default_pii"] + else rv["send_default_pii"], + recursive=True, ) elif has_data_collection_enabled(rv) and rv["event_scrubber"]: warnings.warn( diff --git a/sentry_sdk/scrubber.py b/sentry_sdk/scrubber.py index 6794491325..c21f080f96 100644 --- a/sentry_sdk/scrubber.py +++ b/sentry_sdk/scrubber.py @@ -1,3 +1,4 @@ +import copy from typing import TYPE_CHECKING, Dict, List, cast from sentry_sdk.utils import ( @@ -111,12 +112,17 @@ def scrub_dict(self, d: object) -> None: if not isinstance(d, dict): return - for k, v in d.items(): + for k, v in list(d.items()): # The cast is needed because mypy is not smart enough to figure out that k must be a # string after the isinstance check. if isinstance(k, str) and k.lower() in self.denylist: d[k] = AnnotatedValue.substituted_because_contains_sensitive_data() elif self.recursive: + # Nested containers are often the caller's objects. Copy before + # walking them so scrubbing the event does not change that data. + if isinstance(v, (dict, list)): + v = copy.deepcopy(v) + d[k] = v self.scrub_dict(v) # no-op unless v is a dict self.scrub_list(v) # no-op unless v is a list @@ -164,8 +170,18 @@ def scrub_spans(self, event: "Event") -> None: with capture_internal_exceptions(): if "spans" in event: for span in cast(List[Dict[str, object]], event["spans"]): - if "data" in span: - self.scrub_dict(span["data"]) + data = span.get("data") + if not isinstance(data, dict): + continue + # PyMongo stores a logical session id here. "session" is on the + # denylist because of cookies, and this value is not a secret. + op_ids = data.get("operation_ids") + session = op_ids.get("session") if isinstance(op_ids, dict) else None + self.scrub_dict(data) + if session is not None: + restored = data.get("operation_ids") + if isinstance(restored, dict): + restored["session"] = session def scrub_event(self, event: "Event") -> None: self.scrub_request(event) diff --git a/tests/test_scrubber.py b/tests/test_scrubber.py index 211d6336ac..5181e4b83f 100644 --- a/tests/test_scrubber.py +++ b/tests/test_scrubber.py @@ -3,7 +3,7 @@ from sentry_sdk import capture_event, capture_exception, start_span, start_transaction from sentry_sdk.scrubber import EventScrubber -from sentry_sdk.utils import event_from_exception +from sentry_sdk.utils import AnnotatedValue, event_from_exception from tests.conftest import ApproxDict logger = logging.getLogger(__name__) @@ -283,3 +283,41 @@ def test_recursive_scrubber_does_not_override_original(sentry_init, capture_even (frame,) = frames assert data["csrf"] == "secret" assert frame["vars"]["data"]["csrf"] == "[Filtered]" + + +def test_default_scrubber_scrubs_nested_secret(sentry_init, capture_events): + sentry_init() + events = capture_events() + + try: + 1 / 0 + except ZeroDivisionError: + ev, _hint = event_from_exception(sys.exc_info()) + ev["extra"] = {"user": {"password": "secret", "name": "ada"}} + capture_event(ev) + + (event,) = events + assert event["extra"]["user"]["password"] == "[Filtered]" + assert event["extra"]["user"]["name"] == "ada" + + +def test_recursive_scrub_leaves_caller_dict_and_mongo_session(): + user = {"password": "secret", "name": "ada"} + event = { + "extra": {"user": user}, + "spans": [ + { + "data": { + "operation_ids": {"session": "abc", "operation": 1}, + "password": "secret", + } + } + ], + } + EventScrubber(recursive=True).scrub_event(event) + assert user["password"] == "secret" + assert isinstance(event["extra"]["user"]["password"], AnnotatedValue) + assert event["extra"]["user"]["name"] == "ada" + assert event["spans"][0]["data"]["operation_ids"]["session"] == "abc" + assert isinstance(event["spans"][0]["data"]["password"], AnnotatedValue) +