getsentry / getsentry/sentry-python
DSN netloc substring matching misclassifies unrelated destinations as SDK-internal
- Ngôn ngữ chính
- Python
- Star
- 2.2k
- Fork
- 669
- Merge trung bình
- 1 ngày 1 giờ
- Pull request đã merge (30 ngày)
- 213
Mô tả
### How do you use Sentry?
Sentry Saas (sentry.io)
### Version
2.68.1 (latest release; also reproduced on current `master`)
### Steps to Reproduce
`sentry_sdk.utils.is_sentry_url()` currently classifies a candidate using:
```python
client.transport.parsed_dsn.netloc in url
```
This reproduction uses the installed SDK and makes no network requests:
```python
from http.client import HTTPConnection
from sentry_sdk.client import Client
from sentry_sdk.tracing_utils import should_propagate_trace
from sentry_sdk.utils import is_sentry_url
client = Client(
dsn="https://key@abcd1234.ingest.sentry.io/1",
trace_propagation_targets=[".*"],
default_integrations=False,
)
cases = [
(
"hostname suffix",
"https://abcd1234.ingest.sentry.io.evil.test/api/1",
False,
),
(
"userinfo",
"https://abcd1234.ingest.sentry.io@attacker.test/api/1",
False,
),
(
"query",
"https://attacker.test/?next=abcd1234.ingest.sentry.io",
False,
),
(
"case-only real host",
"https://ABCD1234.INGEST.SENTRY.IO/api/1/envelope/",
True,
),
]
for label, url, expected in cases:
print(
label,
expected,
is_sentry_url(client, url),
should_propagate_trace(client, url),
)
custom_port_client = Client(
dsn="http://key@localhost:9000/1",
default_integrations=False,
)
connection = HTTPConnection("localhost", 9000)
print(
"custom-port raw host",
True,
is_sentry_url(custom_port_client, connection.host),
)
```
Current output (`expected`, `actual`, `should_propagate_trace`) is:
```text
hostname suffix False True False
userinfo False True False
query False True False
case-only real host True False True
custom-port raw host True False
```
The same behavior reproduces in `sentry-sdk==2.68.1` and current `master` at
`0aa3f2bd4a27f40c7e828cb0f6f92bcf677df852`. The focused existing suites remain
green (`3` classifier tests and `16` trace-target tests), but do not cover these
boundaries.
### Expected Result
Internal-request classification should depend on the candidate's actual
hostname, not an arbitrary occurrence of DSN text.
Proposed narrow contract for maintainer confirmation:
- compare the candidate hostname with the DSN hostname using case-insensitive
exact equality;
- support both absolute URLs and raw-host input;
- preserve the current scheme-independent behavior;
- exclude port from identity for this focused change;
- do not include child subdomains by default; and
- return `False`, without raising, when a hostname cannot be safely obtained.
This deliberately leaves same-host/different-port requests classified as
internal. Would this focused contract be acceptable? I will wait for a
maintainer response before implementing anything and will follow a different
port or subdomain contract if requested.
### Actual Result
The substring comparison produces seven false positives and two false negatives
in the extended standalone matrix. The additional cases cover an embedded
hostname, path text, a raw-host suffix, and a neighboring IPv6 literal.
The common verified downstream effect is suppression of otherwise configured
`sentry-trace` and `baggage` headers for an unrelated destination. In the
stdlib raw-host path, a lookalike host can also take the early return that skips
the outgoing HTTP span and breadcrumb path. Conversely, a case-only real host
or the raw host of a custom-port DSN is not recognized as internal.
This is an instrumentation-correctness report. I have not demonstrated traffic
redirection, credential disclosure, account takeover, or another security
exploit. I found no matching public Sentry Python issue or open pull request in
bounded symbol and behavior searches.
Hướng dẫn đóng góp
Đánh giá
Issue này chưa được đánh giá.