getsentry / getsentry/sentry-python

DSN netloc substring matching misclassifies unrelated destinations as SDK-internal

オープン
#7,332 コメント 4 件 リアクション 0 件 担当者 1 名 @YusefSyed が担当を希望しています GitHub で見る
Python Traces
主要言語
Python
スター
2.2k
フォーク
669
平均マージ
1日 1時間
マージ済み PR(30日)
213

説明

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

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。