From 9ade861762643c15a1de72ba8b74152806ec92e4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 17 Jan 2023 14:44:06 +0100 Subject: [PATCH 1/8] Always remove Django session related cookies. --- sentry_sdk/consts.py | 2 ++ sentry_sdk/integrations/django/__init__.py | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index eeca4cbaf4..b823eb4a95 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -43,6 +43,8 @@ DEFAULT_QUEUE_SIZE = 100 DEFAULT_MAX_BREADCRUMBS = 100 +SENSITIVE_DATA_SUBSTITUTE = "[Filtered]" + class INSTRUMENTER: SENTRY = "sentry" diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 67a0bf3844..0a1e5f0594 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -6,7 +6,7 @@ import weakref from sentry_sdk._types import MYPY -from sentry_sdk.consts import OP +from sentry_sdk.consts import OP, SENSITIVE_DATA_SUBSTITUTE from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.scope import add_global_event_processor from sentry_sdk.serializer import add_global_repr_processor @@ -28,6 +28,7 @@ try: from django import VERSION as DJANGO_VERSION + from django.conf import settings as django_settings from django.core import signals try: @@ -477,7 +478,19 @@ def env(self): def cookies(self): # type: () -> Dict[str, str] - return self.request.COOKIES + privacy_cookies = [ + django_settings.CSRF_COOKIE_NAME, + django_settings.SESSION_COOKIE_NAME, + ] + + clean_cookies = {} + for (key, val) in self.request.COOKIES.items(): + if key in privacy_cookies: + clean_cookies[key] = SENSITIVE_DATA_SUBSTITUTE + else: + clean_cookies[key] = val + + return clean_cookies def raw_data(self): # type: () -> bytes From dc8595e1016f5c6b8e59d26e7836a63ae0fe0e23 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 17 Jan 2023 15:10:32 +0100 Subject: [PATCH 2/8] Added tests --- .../django/test_data_scrubbing.py | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/integrations/django/test_data_scrubbing.py diff --git a/tests/integrations/django/test_data_scrubbing.py b/tests/integrations/django/test_data_scrubbing.py new file mode 100644 index 0000000000..13b470a7a4 --- /dev/null +++ b/tests/integrations/django/test_data_scrubbing.py @@ -0,0 +1,77 @@ +import pytest + +from werkzeug.test import Client + +from sentry_sdk.integrations.django import DjangoIntegration + +from tests.integrations.django.myapp.wsgi import application + +try: + from django.urls import reverse +except ImportError: + from django.core.urlresolvers import reverse + + +@pytest.fixture +def client(): + return Client(application) + + +def test_scrub_django_session_cookies_removed( + sentry_init, + client, + capture_events, +): + sentry_init(integrations=[DjangoIntegration()], send_default_pii=False) + events = capture_events() + client.set_cookie("localhost", "sessionid", "123") + client.set_cookie("localhost", "csrftoken", "456") + client.set_cookie("localhost", "foo", "bar") + client.get(reverse("view_exc")) + + (event,) = events + assert "cookies" not in event["request"] + + +def test_scrub_django_session_cookies_filtered( + sentry_init, + client, + capture_events, +): + sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) + events = capture_events() + client.set_cookie("localhost", "sessionid", "123") + client.set_cookie("localhost", "csrftoken", "456") + client.set_cookie("localhost", "foo", "bar") + client.get(reverse("view_exc")) + + (event,) = events + assert event["request"]["cookies"] == { + "sessionid": "[Filtered]", + "csrftoken": "[Filtered]", + "foo": "bar", + } + + +def test_scrub_django_custom_session_cookies_filtered( + sentry_init, + client, + capture_events, + settings, +): + settings.SESSION_COOKIE_NAME = "my_sess" + settings.CSRF_COOKIE_NAME = "csrf_secret" + + sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) + events = capture_events() + client.set_cookie("localhost", "my_sess", "123") + client.set_cookie("localhost", "csrf_secret", "456") + client.set_cookie("localhost", "foo", "bar") + client.get(reverse("view_exc")) + + (event,) = events + assert event["request"]["cookies"] == { + "my_sess": "[Filtered]", + "csrf_secret": "[Filtered]", + "foo": "bar", + } From e55e48e0780f023af627b1a85f36f8a6272eb75e Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 17 Jan 2023 15:16:47 +0100 Subject: [PATCH 3/8] Fixed tests --- .../django/test_data_scrubbing.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/integrations/django/test_data_scrubbing.py b/tests/integrations/django/test_data_scrubbing.py index 13b470a7a4..c0ab14ae63 100644 --- a/tests/integrations/django/test_data_scrubbing.py +++ b/tests/integrations/django/test_data_scrubbing.py @@ -1,4 +1,6 @@ +from functools import partial import pytest +import pytest_django from werkzeug.test import Client @@ -12,11 +14,31 @@ from django.core.urlresolvers import reverse +# Hack to prevent from experimental feature introduced in version `4.3.0` in `pytest-django` that +# requires explicit database allow from failing the test +pytest_mark_django_db_decorator = partial(pytest.mark.django_db) +try: + pytest_version = tuple(map(int, pytest_django.__version__.split("."))) + if pytest_version > (4, 2, 0): + pytest_mark_django_db_decorator = partial( + pytest.mark.django_db, databases="__all__" + ) +except ValueError: + if "dev" in pytest_django.__version__: + pytest_mark_django_db_decorator = partial( + pytest.mark.django_db, databases="__all__" + ) +except AttributeError: + pass + + @pytest.fixture def client(): return Client(application) +@pytest.mark.forked +@pytest_mark_django_db_decorator() def test_scrub_django_session_cookies_removed( sentry_init, client, @@ -33,6 +55,8 @@ def test_scrub_django_session_cookies_removed( assert "cookies" not in event["request"] +@pytest.mark.forked +@pytest_mark_django_db_decorator() def test_scrub_django_session_cookies_filtered( sentry_init, client, @@ -53,6 +77,8 @@ def test_scrub_django_session_cookies_filtered( } +@pytest.mark.forked +@pytest_mark_django_db_decorator() def test_scrub_django_custom_session_cookies_filtered( sentry_init, client, From 31ca6f60ea59cb491d594b6b14cffba6708a4e73 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 18 Jan 2023 12:20:39 +0100 Subject: [PATCH 4/8] Annotate cookies with why they were removed. --- sentry_sdk/integrations/django/__init__.py | 7 +++++-- sentry_sdk/utils.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 0a1e5f0594..0461ffbe45 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -6,13 +6,14 @@ import weakref from sentry_sdk._types import MYPY -from sentry_sdk.consts import OP, SENSITIVE_DATA_SUBSTITUTE +from sentry_sdk.consts import OP from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.scope import add_global_event_processor from sentry_sdk.serializer import add_global_repr_processor from sentry_sdk.tracing import SOURCE_FOR_STYLE, TRANSACTION_SOURCE_URL from sentry_sdk.tracing_utils import record_sql_queries from sentry_sdk.utils import ( + AnnotatedValue, HAS_REAL_CONTEXTVARS, CONTEXTVARS_ERROR_MESSAGE, logger, @@ -486,7 +487,9 @@ def cookies(self): clean_cookies = {} for (key, val) in self.request.COOKIES.items(): if key in privacy_cookies: - clean_cookies[key] = SENSITIVE_DATA_SUBSTITUTE + clean_cookies[ + key + ] = AnnotatedValue.removed_because_contains_sensitive_data() else: clean_cookies[key] = val diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 4d6a091398..4e602cbd46 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -21,6 +21,7 @@ import sentry_sdk from sentry_sdk._compat import PY2, PY33, PY37, implements_str, text_type, urlparse from sentry_sdk._types import MYPY +from sentry_sdk.consts import SENSITIVE_DATA_SUBSTITUTE if MYPY: from types import FrameType, TracebackType @@ -370,6 +371,22 @@ def removed_because_over_size_limit(cls): }, ) + @classmethod + def removed_because_contains_sensitive_data(cls): + # type: () -> AnnotatedValue + """The actual value was removed because it contained sensitive information.""" + return AnnotatedValue( + value=SENSITIVE_DATA_SUBSTITUTE, + metadata={ + "rem": [ # Remark + [ + "!config", # Because of SDK configuration (in this case the config is the hard coded removal of certain django cookies) + "s", # The fields original value was substituted + ] + ] + }, + ) + if MYPY: from typing import TypeVar From df6a835b92e0a531040b48dfa58da4735d1e6567 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 18 Jan 2023 13:02:14 +0100 Subject: [PATCH 5/8] Fixed typing --- sentry_sdk/integrations/django/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 0461ffbe45..f31fa1a3c4 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -478,13 +478,13 @@ def env(self): return self.request.META def cookies(self): - # type: () -> Dict[str, str] + # type: () -> Dict[str, Union[str, AnnotatedValue]] privacy_cookies = [ django_settings.CSRF_COOKIE_NAME, django_settings.SESSION_COOKIE_NAME, ] - clean_cookies = {} + clean_cookies = {} # type: Dict[str, Union[str, AnnotatedValue]] for (key, val) in self.request.COOKIES.items(): if key in privacy_cookies: clean_cookies[ From e043b3a5dce1081bec8a52b1762aba24d6b354f8 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 18 Jan 2023 13:03:48 +0100 Subject: [PATCH 6/8] Fixed import --- sentry_sdk/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 4e602cbd46..39096263b1 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -21,7 +21,6 @@ import sentry_sdk from sentry_sdk._compat import PY2, PY33, PY37, implements_str, text_type, urlparse from sentry_sdk._types import MYPY -from sentry_sdk.consts import SENSITIVE_DATA_SUBSTITUTE if MYPY: from types import FrameType, TracebackType @@ -375,6 +374,8 @@ def removed_because_over_size_limit(cls): def removed_because_contains_sensitive_data(cls): # type: () -> AnnotatedValue """The actual value was removed because it contained sensitive information.""" + from sentry_sdk.consts import SENSITIVE_DATA_SUBSTITUTE + return AnnotatedValue( value=SENSITIVE_DATA_SUBSTITUTE, metadata={ From 4e0a8b26e5059961f8a0396f25e52faae33a9657 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 19 Jan 2023 11:13:45 +0100 Subject: [PATCH 7/8] Better naming --- sentry_sdk/integrations/django/__init__.py | 2 +- sentry_sdk/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index f31fa1a3c4..bbcd91e4b0 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -489,7 +489,7 @@ def cookies(self): if key in privacy_cookies: clean_cookies[ key - ] = AnnotatedValue.removed_because_contains_sensitive_data() + ] = AnnotatedValue.substituted_because_contains_sensitive_data() else: clean_cookies[key] = val diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 39096263b1..3f573171a6 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -371,7 +371,7 @@ def removed_because_over_size_limit(cls): ) @classmethod - def removed_because_contains_sensitive_data(cls): + def substituted_because_contains_sensitive_data(cls): # type: () -> AnnotatedValue """The actual value was removed because it contained sensitive information.""" from sentry_sdk.consts import SENSITIVE_DATA_SUBSTITUTE From ed00adf7e21e2d4810d6e5d27090bda247a3bc9c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 19 Jan 2023 11:15:09 +0100 Subject: [PATCH 8/8] Going with the simple version now, until frontend has fixed the problem with the tooltip. --- sentry_sdk/integrations/django/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index bbcd91e4b0..697ab484e3 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -6,7 +6,7 @@ import weakref from sentry_sdk._types import MYPY -from sentry_sdk.consts import OP +from sentry_sdk.consts import OP, SENSITIVE_DATA_SUBSTITUTE from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.scope import add_global_event_processor from sentry_sdk.serializer import add_global_repr_processor @@ -487,9 +487,7 @@ def cookies(self): clean_cookies = {} # type: Dict[str, Union[str, AnnotatedValue]] for (key, val) in self.request.COOKIES.items(): if key in privacy_cookies: - clean_cookies[ - key - ] = AnnotatedValue.substituted_because_contains_sensitive_data() + clean_cookies[key] = SENSITIVE_DATA_SUBSTITUTE else: clean_cookies[key] = val