Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion sentry_sdk/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
get_sdk_name,
get_type_name,
handle_in_app,
has_data_collection_enabled,
has_logs_enabled,
has_metrics_enabled,
logger,
Expand Down Expand Up @@ -350,13 +351,21 @@
rv["traces_sample_rate"] = 1.0

rv["data_collection"] = _resolve_data_collection(rv)

if rv["event_scrubber"] is None:
# Do not add the event scrubber if data collection is enabled as it can remove data that's
# collected under data collection config
if not has_data_collection_enabled(rv) and 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"]
)
Comment thread
sentry[bot] marked this conversation as resolved.
elif has_data_collection_enabled(rv) and rv["event_scrubber"]:
warnings.warn(
"Event scrubbers are not enabled when data collection configuration is provided. Ignoring event_scrubber...",
stacklevel=2,
)
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Comment thread
sentry[bot] marked this conversation as resolved.
rv["event_scrubber"] = None

Check warning on line 368 in sentry_sdk/client.py

View check run for this annotation

@sentry/warden / warden: security-review

Enabling data_collection disables EventScrubber secret redaction

Configuring data_collection turns off the default and any explicit EventScrubber, so denylisted secrets (passwords, tokens, auth headers, session keys) in request bodies, extras, breadcrumbs, and frame vars are no longer redacted before events leave the process. data_collection only filters headers/cookies/query params—not body or extra fields—so it does not replace that guard.

if rv["socket_options"] and not isinstance(rv["socket_options"], list):
logger.warning(
Expand Down
32 changes: 27 additions & 5 deletions sentry_sdk/integrations/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,14 @@ def __call__(
)
Scope.set_custom_sampling_context({"wsgi_environ": environ})

if should_send_default_pii():
if has_data_collection_enabled(client.options):
if client.options["data_collection"]["user_info"]:
client_ip = get_client_ip(environ)
if client_ip:
scope.set_attribute(
SPANDATA.USER_IP_ADDRESS, client_ip
)
elif should_send_default_pii():
client_ip = get_client_ip(environ)
if client_ip:
scope.set_attribute(
Expand Down Expand Up @@ -243,9 +250,14 @@ def _get_environ(environ: "Dict[str, str]") -> "Iterator[Tuple[str, str]]":
capture (server name, port and remote addr if pii is enabled).
"""
keys = ["SERVER_NAME", "SERVER_PORT"]
if should_send_default_pii():
# make debugging of proxy setup easier. Proxy headers are
# in headers.
client_options = sentry_sdk.get_client().options

# make debugging of proxy setup easier. Proxy headers are
# in headers.
if has_data_collection_enabled(client_options):
if client_options["data_collection"]["user_info"]:
keys += ["REMOTE_ADDR"]
elif should_send_default_pii():
keys += ["REMOTE_ADDR"]

for key in keys:
Expand Down Expand Up @@ -364,7 +376,12 @@ def event_processor(event: "Event", hint: "Dict[str, Any]") -> "Event":
# if the code below fails halfway through we at least have some data
request_info = event.setdefault("request", {})

if should_send_default_pii():
if has_data_collection_enabled(client_options):
if client_options["data_collection"]["user_info"]:
user_info = event.setdefault("user", {})
if client_ip:
user_info.setdefault("ip_address", client_ip)
elif should_send_default_pii():
user_info = event.setdefault("user", {})
if client_ip:
user_info.setdefault("ip_address", client_ip)
Expand Down Expand Up @@ -442,6 +459,11 @@ def _get_request_attributes(

attributes["url.full"] = get_request_url(environ, use_x_forwarded_for)

if client_options["data_collection"]["user_info"]:
client_ip = get_client_ip(environ)
if client_ip:
attributes["client.address"] = client_ip

elif should_send_default_pii():
client_ip = get_client_ip(environ)
if client_ip:
Expand Down
109 changes: 109 additions & 0 deletions tests/integrations/django/test_data_scrubbing.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,3 +396,112 @@ def test_empty_query_string_is_dropped_with_data_collection(

(event,) = events
assert "query_string" not in event["request"]


USER_INFO_INIT_KWARGS = [
pytest.param({"send_default_pii": True}, True, id="legacy_send_default_pii_true"),
pytest.param(
{"send_default_pii": False}, False, id="legacy_send_default_pii_false"
),
pytest.param(
{"_experiments": {"data_collection": {"user_info": True}}},
True,
id="data_collection_user_info_true",
),
pytest.param(
{"_experiments": {"data_collection": {"user_info": False}}},
False,
id="data_collection_user_info_false",
),
pytest.param(
{
"send_default_pii": True,
"_experiments": {"data_collection": {"user_info": False}},
},
False,
id="data_collection_wins_over_send_default_pii_true",
),
pytest.param(
{
"send_default_pii": False,
"_experiments": {"data_collection": {"user_info": True}},
},
True,
id="data_collection_wins_over_send_default_pii_false",
),
]


@pytest.mark.forked
@pytest_mark_django_db_decorator()
@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS)
def test_user_info_span_attributes_data_collection(
sentry_init, client, capture_items, init_kwargs, expect_ip
):
init_kwargs = dict(init_kwargs) # shallow copy so we can mutate
experiments = init_kwargs.pop("_experiments", {})

sentry_init(
integrations=[DjangoIntegration()],
traces_sample_rate=1.0,
trace_lifecycle="stream",
_experiments=experiments,
**init_kwargs,
)

items = capture_items("span")

unpack_werkzeug_response(
client.get(reverse("message"), environ_base={"REMOTE_ADDR": "127.0.0.1"})
)

sentry_sdk.flush()

spans = [item.payload for item in items]
root_span = spans[-1]

if expect_ip:
assert root_span["attributes"][SPANDATA.USER_IP_ADDRESS] == "127.0.0.1"
assert root_span["attributes"]["client.address"] == "127.0.0.1"
else:
assert SPANDATA.USER_IP_ADDRESS not in root_span["attributes"]
assert "client.address" not in root_span["attributes"]


@pytest.mark.forked
@pytest_mark_django_db_decorator()
@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS)
def test_user_info_error_event_data_collection(
sentry_init, client, capture_events, init_kwargs, expect_ip
):
sentry_init(integrations=[DjangoIntegration()], **init_kwargs)
events = capture_events()

client.get(reverse("view_exc"), environ_base={"REMOTE_ADDR": "127.0.0.1"})

(event,) = events

if expect_ip:
assert event["user"]["ip_address"] == "127.0.0.1"
assert event["request"]["env"]["REMOTE_ADDR"] == "127.0.0.1"
else:
assert "ip_address" not in event.get("user", {})
assert "REMOTE_ADDR" not in event["request"]["env"]


@pytest.mark.forked
@pytest_mark_django_db_decorator()
def test_error_event_no_user_ip_address_without_remote_addr(
sentry_init, client, capture_events
):
sentry_init(
integrations=[DjangoIntegration()],
_experiments={"data_collection": {"user_info": True}},
)
events = capture_events()

client.get(reverse("view_exc"))

(event,) = events

assert "ip_address" not in event.get("user", {})
125 changes: 125 additions & 0 deletions tests/integrations/flask/test_flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -1398,3 +1398,128 @@ def test_empty_query_string_is_dropped_with_data_collection(

(event,) = events
assert "query_string" not in event["request"]


# Parametrization shared by the user_info tests below. ``expect_ip`` is
# whether the client IP may be collected under the given init kwargs.
USER_INFO_INIT_KWARGS = [
pytest.param({"send_default_pii": True}, True, id="legacy_send_default_pii_true"),
pytest.param(
{"send_default_pii": False}, False, id="legacy_send_default_pii_false"
),
pytest.param(
{"_experiments": {"data_collection": {"user_info": True}}},
True,
id="data_collection_user_info_true",
),
pytest.param(
{"_experiments": {"data_collection": {"user_info": False}}},
False,
id="data_collection_user_info_false",
),
# ``data_collection`` is the single source of truth: it must win over
# ``send_default_pii`` when both are configured.
pytest.param(
{
"send_default_pii": True,
"_experiments": {"data_collection": {"user_info": False}},
},
False,
id="data_collection_wins_over_send_default_pii_true",
),
pytest.param(
{
"send_default_pii": False,
"_experiments": {"data_collection": {"user_info": True}},
},
True,
id="data_collection_wins_over_send_default_pii_false",
),
]


@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS)
def test_user_info_span_attributes_data_collection(
sentry_init, app, capture_items, monkeypatch, init_kwargs, expect_ip
):
init_kwargs = dict(init_kwargs) # shallow copy so we can mutate
experiments = init_kwargs.pop("_experiments", {})

sentry_init(
integrations=[flask_sentry.FlaskIntegration()],
traces_sample_rate=1.0,
trace_lifecycle="stream",
_experiments=experiments,
**init_kwargs,
)
# This test is about user IP collection, not flask_login. Disable
# flask_login so the module-level login manager does not interfere.
monkeypatch.setattr(flask_sentry, "flask_login", None)

items = capture_items("span")

client = app.test_client()
client.get("/message", environ_overrides={"REMOTE_ADDR": "127.0.0.1"})

sentry_sdk.flush()

spans = [item.payload for item in items if item.type == "span"]
(segment,) = spans

if expect_ip:
assert segment["attributes"][SPANDATA.USER_IP_ADDRESS] == "127.0.0.1"
assert segment["attributes"]["client.address"] == "127.0.0.1"
else:
assert SPANDATA.USER_IP_ADDRESS not in segment["attributes"]
assert "client.address" not in segment["attributes"]


@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS)
def test_user_info_error_event_data_collection(
sentry_init, app, capture_events, monkeypatch, init_kwargs, expect_ip
):
sentry_init(integrations=[flask_sentry.FlaskIntegration()], **init_kwargs)
monkeypatch.setattr(flask_sentry, "flask_login", None)

@app.route("/crash")
def crash():
1 / 0

events = capture_events()

client = app.test_client()
with pytest.raises(ZeroDivisionError):
client.get("/crash", environ_overrides={"REMOTE_ADDR": "127.0.0.1"})

(event,) = events

if expect_ip:
assert event["user"]["ip_address"] == "127.0.0.1"
assert event["request"]["env"]["REMOTE_ADDR"] == "127.0.0.1"
else:
assert "ip_address" not in event.get("user", {})
assert "REMOTE_ADDR" not in event["request"]["env"]


def test_error_event_no_user_ip_address_without_remote_addr(
sentry_init, app, capture_events, monkeypatch
):
sentry_init(
integrations=[flask_sentry.FlaskIntegration()],
_experiments={"data_collection": {"user_info": True}},
)
monkeypatch.setattr(flask_sentry, "flask_login", None)

@app.route("/crash")
def crash():
1 / 0

events = capture_events()

client = app.test_client()
with pytest.raises(ZeroDivisionError):
client.get("/crash", environ_overrides={"REMOTE_ADDR": ""})

(event,) = events

assert "ip_address" not in event.get("user", {})
Loading
Loading