getsentry / getsentry/sentry-python

DSN netloc substring matching misclassifies unrelated destinations as SDK-internal

Open
#7,332 4 comments 0 reactions 1 assignee Claimed by @YusefSyed View on GitHub
Python Traces
Dominant language
Python
Stars
2.2k
Forks
669
Avg merge
1d 1h
Merged PRs (30d)
213

Description

### 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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.