From 8afeaee1f9ce6ca7298f11bad74cd92bc3bd776e Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 19 Nov 2024 12:15:58 +0000 Subject: [PATCH 1/7] feat: Send PII to Spotlight when no DSN is set Quick fix for getsentry/spotlight#543 until we implement a global scrubber that only scrubs events sent to the clound thorugh the DSN. --- sentry_sdk/client.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index b1e7868031..9084741ce2 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -451,7 +451,11 @@ def should_send_default_pii(self): Returns whether the client should send default PII (Personally Identifiable Information) data to Sentry. """ - return self.options.get("send_default_pii", False) + result = self.options.get("send_default_pii") + if result is None: + result = not self.options["dsn"] and self.spotlight is not None + + return result @property def dsn(self): From bd3b2bb13a58991cf7b5a12b7b4e34ba6a07157b Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 19 Nov 2024 20:10:39 +0000 Subject: [PATCH 2/7] add tests fix bugs --- sentry_sdk/consts.py | 3 ++- tests/test_scope.py | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index ae32294d05..bb2a73337e 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -489,6 +489,7 @@ class OP: # This type exists to trick mypy and PyCharm into thinking `init` and `Client` # take these arguments (even though they take opaque **kwargs) class ClientConstructor: + def __init__( self, dsn=None, # type: Optional[str] @@ -506,7 +507,7 @@ def __init__( transport=None, # type: Optional[Union[sentry_sdk.transport.Transport, Type[sentry_sdk.transport.Transport], Callable[[Event], None]]] transport_queue_size=DEFAULT_QUEUE_SIZE, # type: int sample_rate=1.0, # type: float - send_default_pii=False, # type: bool + send_default_pii=None, # type: Optional[bool] http_proxy=None, # type: Optional[str] https_proxy=None, # type: Optional[str] ignore_errors=[], # type: Sequence[Union[type, str]] # noqa: B006 diff --git a/tests/test_scope.py b/tests/test_scope.py index 0dfa155d11..3efd4c2483 100644 --- a/tests/test_scope.py +++ b/tests/test_scope.py @@ -811,6 +811,24 @@ def test_should_send_default_pii_false(sentry_init): assert should_send_default_pii() is False +def test_should_send_default_pii_default_fals(sentry_init): + sentry_init() + + assert should_send_default_pii() is False + + +def test_should_send_default_pii_false_with_dsn_and_spotlight(sentry_init): + sentry_init(dsn="http://key@localhost/1", spotlight=True) + + assert should_send_default_pii() is False + + +def test_should_send_default_pii_true_without_dsn_and_spotlight(sentry_init): + sentry_init(spotlight=True) + + assert should_send_default_pii() is True + + def test_set_tags(): scope = Scope() scope.set_tags({"tag1": "value1", "tag2": "value2"}) From 7911570ebff00fd0a7e1d0440ce1ea68060681b7 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 10:05:35 +0100 Subject: [PATCH 3/7] Make scrubber initialization more explicit --- sentry_sdk/client.py | 6 +++++- tests/test_scope.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 9084741ce2..db2cc19110 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -128,7 +128,11 @@ def _get_options(*args, **kwargs): rv["traces_sample_rate"] = 1.0 if rv["event_scrubber"] is None: - rv["event_scrubber"] = EventScrubber(send_default_pii=rv["send_default_pii"]) + rv["event_scrubber"] = EventScrubber( + send_default_pii=( + False if rv["send_default_pii"] is None else rv["send_default_pii"] + ) + ) if rv["socket_options"] and not isinstance(rv["socket_options"], list): logger.warning( diff --git a/tests/test_scope.py b/tests/test_scope.py index 3efd4c2483..374a354446 100644 --- a/tests/test_scope.py +++ b/tests/test_scope.py @@ -811,7 +811,7 @@ def test_should_send_default_pii_false(sentry_init): assert should_send_default_pii() is False -def test_should_send_default_pii_default_fals(sentry_init): +def test_should_send_default_pii_default_false(sentry_init): sentry_init() assert should_send_default_pii() is False From 15cf625859852b0a51c70f8126ad92af6d947d48 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 10:46:05 +0100 Subject: [PATCH 4/7] Refactored to not change the default value of send_default_pii --- sentry_sdk/client.py | 13 +++++-------- sentry_sdk/consts.py | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index db2cc19110..a858523cdd 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -128,11 +128,7 @@ def _get_options(*args, **kwargs): rv["traces_sample_rate"] = 1.0 if rv["event_scrubber"] is None: - rv["event_scrubber"] = EventScrubber( - send_default_pii=( - False if rv["send_default_pii"] is None else rv["send_default_pii"] - ) - ) + rv["event_scrubber"] = EventScrubber(send_default_pii=rv["send_default_pii"]) if rv["socket_options"] and not isinstance(rv["socket_options"], list): logger.warning( @@ -455,9 +451,10 @@ def should_send_default_pii(self): Returns whether the client should send default PII (Personally Identifiable Information) data to Sentry. """ - result = self.options.get("send_default_pii") - if result is None: - result = not self.options["dsn"] and self.spotlight is not None + result = self.options.get("send_default_pii", False) + + if not self.options["dsn"] and self.spotlight is not None: + result = True return result diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index bb2a73337e..0fb84cf052 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -507,7 +507,7 @@ def __init__( transport=None, # type: Optional[Union[sentry_sdk.transport.Transport, Type[sentry_sdk.transport.Transport], Callable[[Event], None]]] transport_queue_size=DEFAULT_QUEUE_SIZE, # type: int sample_rate=1.0, # type: float - send_default_pii=None, # type: Optional[bool] + send_default_pii=False, # type: bool http_proxy=None, # type: Optional[str] https_proxy=None, # type: Optional[str] ignore_errors=[], # type: Sequence[Union[type, str]] # noqa: B006 From de7f39818af78a1012a8fcea6bbd80f20c6b0eb3 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 11:06:52 +0100 Subject: [PATCH 5/7] Add test to show that there is now no way to opt out of sending PII to spotlight. --- tests/test_scope.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_scope.py b/tests/test_scope.py index 374a354446..2dc7375fdd 100644 --- a/tests/test_scope.py +++ b/tests/test_scope.py @@ -829,6 +829,13 @@ def test_should_send_default_pii_true_without_dsn_and_spotlight(sentry_init): assert should_send_default_pii() is True +def test_should_send_default_pii_false_without_dsn_and_spotlight(sentry_init): + sentry_init(spotlight=True, send_default_pii=False) + + # There is now now way to opt out of sending PII to spotlight + assert should_send_default_pii() is True + + def test_set_tags(): scope = Scope() scope.set_tags({"tag1": "value1", "tag2": "value2"}) From d33f1e5a0ceba821b7dd3d79b912f6c05f670f64 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 16:04:37 +0100 Subject: [PATCH 6/7] Revert "Refactored to not change the default value of send_default_pii" This reverts commit 15cf625859852b0a51c70f8126ad92af6d947d48. --- sentry_sdk/client.py | 13 ++++++++----- sentry_sdk/consts.py | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index a858523cdd..db2cc19110 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -128,7 +128,11 @@ def _get_options(*args, **kwargs): rv["traces_sample_rate"] = 1.0 if rv["event_scrubber"] is None: - rv["event_scrubber"] = EventScrubber(send_default_pii=rv["send_default_pii"]) + rv["event_scrubber"] = EventScrubber( + send_default_pii=( + False if rv["send_default_pii"] is None else rv["send_default_pii"] + ) + ) if rv["socket_options"] and not isinstance(rv["socket_options"], list): logger.warning( @@ -451,10 +455,9 @@ def should_send_default_pii(self): Returns whether the client should send default PII (Personally Identifiable Information) data to Sentry. """ - result = self.options.get("send_default_pii", False) - - if not self.options["dsn"] and self.spotlight is not None: - result = True + result = self.options.get("send_default_pii") + if result is None: + result = not self.options["dsn"] and self.spotlight is not None return result diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 0fb84cf052..bb2a73337e 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -507,7 +507,7 @@ def __init__( transport=None, # type: Optional[Union[sentry_sdk.transport.Transport, Type[sentry_sdk.transport.Transport], Callable[[Event], None]]] transport_queue_size=DEFAULT_QUEUE_SIZE, # type: int sample_rate=1.0, # type: float - send_default_pii=False, # type: bool + send_default_pii=None, # type: Optional[bool] http_proxy=None, # type: Optional[str] https_proxy=None, # type: Optional[str] ignore_errors=[], # type: Sequence[Union[type, str]] # noqa: B006 From e45e5810cebbc0702c61685403baa087c178dab4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 16:04:55 +0100 Subject: [PATCH 7/7] Revert "Add test to show that there is now no way to opt out of sending PII to spotlight." This reverts commit de7f39818af78a1012a8fcea6bbd80f20c6b0eb3. --- tests/test_scope.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/test_scope.py b/tests/test_scope.py index 2dc7375fdd..374a354446 100644 --- a/tests/test_scope.py +++ b/tests/test_scope.py @@ -829,13 +829,6 @@ def test_should_send_default_pii_true_without_dsn_and_spotlight(sentry_init): assert should_send_default_pii() is True -def test_should_send_default_pii_false_without_dsn_and_spotlight(sentry_init): - sentry_init(spotlight=True, send_default_pii=False) - - # There is now now way to opt out of sending PII to spotlight - assert should_send_default_pii() is True - - def test_set_tags(): scope = Scope() scope.set_tags({"tag1": "value1", "tag2": "value2"})