Skip to content

fix(aiohttp): Gate url.full, url.path, url.query on send_default_pii#6650

Merged
ericapisani merged 2 commits into
masterfrom
py-2545-aiohttp-url-attr
Jun 25, 2026
Merged

fix(aiohttp): Gate url.full, url.path, url.query on send_default_pii#6650
ericapisani merged 2 commits into
masterfrom
py-2545-aiohttp-url-attr

forgot to move setting of a url.query behind the pii check

ae3ad30
Select commit
Loading
Failed to load commit list.
@sentry/warden / warden: code-review completed Jun 24, 2026

1 issue

code-review: Found 1 issue (1 medium)

Medium

Parametrized `send_pii=False` path asserts nothing, leaving PII-leakage regression undetected - `tests/integrations/aiohttp/test_aiohttp.py:1510-1520`

The send_pii=False branch of this parametrized test only skips the positive assertions — it never asserts that url.full, url.path, and url.query are absent from inner_client_span["attributes"], so a regression that leaks these attributes when PII is off would silently pass. Add an else clause mirroring the pattern used in test_url_query_attribute_span_streaming (e.g. assert "url.full" not in inner_client_span["attributes"]).

Also found at:

  • tests/integrations/aiohttp/test_aiohttp.py:1139
  • tests/integrations/aiohttp/test_aiohttp.py:1564-1576

⏱ 4m 51s · 602.8k in / 30.8k out · $1.19

Annotations

Check warning on line 1520 in tests/integrations/aiohttp/test_aiohttp.py

See this annotation in the file changed.

@sentry-warden sentry-warden / warden: code-review

Parametrized `send_pii=False` path asserts nothing, leaving PII-leakage regression undetected

The `send_pii=False` branch of this parametrized test only skips the positive assertions — it never asserts that `url.full`, `url.path`, and `url.query` are **absent** from `inner_client_span["attributes"]`, so a regression that leaks these attributes when PII is off would silently pass. Add an `else` clause mirroring the pattern used in `test_url_query_attribute_span_streaming` (e.g. `assert "url.full" not in inner_client_span["attributes"]`).

Check warning on line 1139 in tests/integrations/aiohttp/test_aiohttp.py

See this annotation in the file changed.

@sentry-warden sentry-warden / warden: code-review

[7HK-BDM] Parametrized `send_pii=False` path asserts nothing, leaving PII-leakage regression undetected (additional location)

The `send_pii=False` branch of this parametrized test only skips the positive assertions — it never asserts that `url.full`, `url.path`, and `url.query` are **absent** from `inner_client_span["attributes"]`, so a regression that leaks these attributes when PII is off would silently pass. Add an `else` clause mirroring the pattern used in `test_url_query_attribute_span_streaming` (e.g. `assert "url.full" not in inner_client_span["attributes"]`).

Check warning on line 1576 in tests/integrations/aiohttp/test_aiohttp.py

See this annotation in the file changed.

@sentry-warden sentry-warden / warden: code-review

[7HK-BDM] Parametrized `send_pii=False` path asserts nothing, leaving PII-leakage regression undetected (additional location)

The `send_pii=False` branch of this parametrized test only skips the positive assertions — it never asserts that `url.full`, `url.path`, and `url.query` are **absent** from `inner_client_span["attributes"]`, so a regression that leaks these attributes when PII is off would silently pass. Add an `else` clause mirroring the pattern used in `test_url_query_attribute_span_streaming` (e.g. `assert "url.full" not in inner_client_span["attributes"]`).