Skip to content

fix(wsgi): Gate url.full, url.path, and http.query behind send_default_pii - #6654

Merged
ericapisani merged 2 commits into
masterfrom
py-2552-wsgi-url-attr
Jun 25, 2026
Merged

fix(wsgi): Gate url.full, url.path, and http.query behind send_default_pii#6654
ericapisani merged 2 commits into
masterfrom
py-2552-wsgi-url-attr

Conversation

@ericapisani

@ericapisaniericapisani commented Jun 24, 2026

Copy link
Copy Markdown
Member

The WSGI integration now gates url.full, url.path, and http.query span attributes behind send_default_pii, consistent with how client.address is already handled. These attributes can contain user-provided query parameters and path segments that may include PII, so they should not be captured by default.

Fixes PY-2552
Fixes#6653

…t_pii
The url.full, url.path, and http.query span attributes can contain
user-provided query parameters and paths that may include PII. Gate
these behind the send_default_pii setting, consistent with how
client.address is handled.
Fixes PY-2552
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@linear-code

Copy link
Copy Markdown

PY-2552

@github-actions

github-actionsBot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

89867 passed | ⏭️ 6240 skipped | Total: 96107 | Pass Rate: 93.51% | Execution Time: 323m 58s

📊 Comparison with Base Branch

MetricChange
Total Tests📈 +30
Passed Tests📈 +30
Failed Tests
Skipped Tests

All tests are passing successfully.

✅ Patch coverage is 100.00%. Project has 2394 uncovered lines.
✅ Project coverage is 89.93%. Comparing base (base) to head (head).

Files with missing lines (1)
FilePatch %Lines
sentry_sdk/integrations/wsgi.py100.00%⚠️ 1 partials
Coverage diff
@@ Coverage Diff @@## main #PR +/-##
==========================================
+ Coverage 89.88% 89.93% +0.05%
==========================================
Files 192 192 —
Lines 23763 23766 +3
Branches 8206 8208 +2
==========================================
+ Hits 21358 21372 +14- Misses 2405 2394 -11- Partials 1345 1342 -3

Generated by Codecov Action

@ericapisani
ericapisani marked this pull request as ready for review June 24, 2026 18:19
@ericapisani
ericapisani requested a review from a team as a code ownerJune 24, 2026 18:19
Comment on lines 266 to +276
envelope = events[0]

assert envelope["type"] == "transaction"
assert envelope["transaction"] == "generic WSGI request"
assert envelope["contexts"]["trace"]["op"] == "http.server"
assert envelope["request"] == DictionaryContaining(
{"method": "GET", "url": "http://localhost/dogs/are/great/"}
{
"method": "GET",
"url": "http://localhost/dogs/are/great",
"query_string": "toy=tennisball",
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The WSGI integration leaks PII in non-streaming transaction events by unconditionally adding url and query_string, even when send_default_pii is False.
Severity: HIGH

Suggested Fix

In the _make_wsgi_event_processor function, wrap the assignments for request_info["url"] and request_info["query_string"] in a conditional check for should_send_default_pii(). This will ensure that PII is consistently redacted across both streaming and non-streaming code paths, respecting the user's setting.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: tests/integrations/wsgi/test_wsgi.py#L265-L276
Potential issue: The WSGI integration aims to prevent sending Personally Identifiable
Information (PII) when `send_default_pii` is `False`. While the change correctly redacts
PII for span attributes in the streaming path (`_get_request_attributes`), it fails to
do so for the non-streaming transaction event path. The `_make_wsgi_event_processor`
function unconditionally sets `request["url"]` and `request["query_string"]` on the
event. This results in an inconsistent behavior where users who set
`send_default_pii=False` will still have unredacted URLs and query strings sent in
transaction events if they are not using the span streaming path, leading to an
unintended PII leak.

Did we get this right? 👍 / 👎 to inform future reviews.

@alexander-alderman-webbalexander-alderman-webb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced that not gating URL attribute behind send_default_pii is considered a sufficiently severe bug that allows us to break existing behavior.
I'll stamp given precedent in #6501.

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6273bbd. Configure here.

if path:
attributes["url.path"] = path

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

url.path omits SCRIPT_NAME prefix

Medium Severity

When send_default_pii is enabled, url.path is taken from PATH_INFO only, while url.full is built with get_request_url, which merges SCRIPT_NAME and PATH_INFO. Mounted WSGI apps can therefore emit spans where url.path and url.full describe different paths.

Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 6273bbd. Configure here.

@ericapisani

Copy link
Copy Markdown
MemberAuthor

I'm not convinced that not gating URL attribute behind send_default_pii is considered a sufficiently severe bug that allows us to break existing behavior.

If we decide to talk about this more, we should make it a thread/convo with some other folks outside of this PR. One could argue that we were breaking the contract set out by our conventions/attributes by not doing so, and, in another case for a different attribute relatively recently, we considered it sufficiently severe that we called an incident over it to correct this.

@ericapisani
ericapisani merged commit 68e5ea7 into masterJun 25, 2026
144 checks passed
@ericapisani
ericapisani deleted the py-2552-wsgi-url-attr branch June 25, 2026 12:49
ericapisani added a commit that referenced this pull request Jun 26, 2026
…6664)
Gates `url.full`, `url.path`, and `url.query` span attributes in the
Tornado integration behind `send_default_pii`, consistent with the same
fix applied to the aiohttp (#6650) and wsgi (#6654) integrations.
Adds a `send_pii` parametrize dimension to the transactions test to
cover both the PII-on and PII-off paths explicitly.
Fixes PY-2556
Fixes#6661
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
mgaligniana pushed a commit to mgaligniana/sentry-python that referenced this pull request Aug 9, 2026
…t_pii (getsentry#6654)
The WSGI integration now gates `url.full`, `url.path`, and `http.query`
span attributes behind `send_default_pii`, consistent with how
`client.address` is already handled. These attributes can contain
user-provided query parameters and path segments that may include PII,
so they should not be captured by default.
Fixes PY-2552
Fixesgetsentry#6653
mgaligniana pushed a commit to mgaligniana/sentry-python that referenced this pull request Aug 9, 2026
…etsentry#6664)
Gates `url.full`, `url.path`, and `url.query` span attributes in the
Tornado integration behind `send_default_pii`, consistent with the same
fix applied to the aiohttp (getsentry#6650) and wsgi (getsentry#6654) integrations.
Adds a `send_pii` parametrize dimension to the transactions test to
cover both the PII-on and PII-off paths explicitly.
Fixes PY-2556
Fixesgetsentry#6661
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wsgi missing url.path attribute

2 participants

@ericapisani@alexander-alderman-webb