-
Notifications
You must be signed in to change notification settings - Fork 630
fix(wsgi): Gate url.full, url.path, and http.query behind send_default_pii #6654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,20 +208,22 @@ def dogpark(environ, start_response): | |
| assert envelope["request"] == error_event["request"] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("send_pii", [True, False]) | ||
| @pytest.mark.parametrize("span_streaming", [True, False]) | ||
| def test_transaction_no_error( | ||
| sentry_init, | ||
| capture_events, | ||
| capture_items, | ||
| DictionaryContaining, # noqa:N803 | ||
| span_streaming, | ||
| send_pii, | ||
| ): | ||
| def dogpark(environ, start_response): | ||
| start_response("200 OK", []) | ||
| return ["Go get the ball! Good dog!"] | ||
|
|
||
| sentry_init( | ||
| send_default_pii=True, | ||
| send_default_pii=send_pii, | ||
| traces_sample_rate=1.0, | ||
| _experiments={ | ||
| "trace_lifecycle": "stream" if span_streaming else "static", | ||
|
|
@@ -235,7 +237,7 @@ def dogpark(environ, start_response): | |
| else: | ||
| events = capture_events() | ||
|
|
||
| client.get("/dogs/are/great/") | ||
| client.get("/dogs/are/great?toy=tennisball") | ||
|
|
||
| sentry_sdk.flush() | ||
|
|
||
|
|
@@ -248,17 +250,30 @@ def dogpark(environ, start_response): | |
| assert span["attributes"]["sentry.op"] == "http.server" | ||
| assert span["attributes"]["sentry.span.source"] == "route" | ||
| assert span["attributes"]["http.request.method"] == "GET" | ||
| assert span["attributes"]["url.full"] == "http://localhost/dogs/are/great/" | ||
| assert span["attributes"]["http.response.status_code"] == 200 | ||
| assert span["status"] == "ok" | ||
|
|
||
| if send_pii: | ||
| assert span["attributes"]["url.full"] == "http://localhost/dogs/are/great" | ||
| assert span["attributes"]["url.path"] == "/dogs/are/great" | ||
| assert span["attributes"]["http.query"] == "toy=tennisball" | ||
| else: | ||
| assert "url.path" not in span["attributes"] | ||
| assert "url.full" not in span["attributes"] | ||
| assert "http.query" not in span["attributes"] | ||
|
|
||
| else: | ||
| 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", | ||
| } | ||
|
Comment on lines
266
to
+276
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Suggested FixIn the Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
| ) | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
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_piiis enabled,url.pathis taken fromPATH_INFOonly, whileurl.fullis built withget_request_url, which mergesSCRIPT_NAMEandPATH_INFO. Mounted WSGI apps can therefore emit spans whereurl.pathandurl.fulldescribe different paths.Reviewed by Cursor Bugbot for commit 6273bbd. Configure here.