getsentry / getsentry/sentry-python

DSN netloc substring matching misclassifies unrelated destinations as SDK-internal

未關閉
#7,332 4 則留言 0 個 reaction 已指派 1 人 已被 @YusefSyed 認領 在 GitHub 檢視
Python Traces
主要語言
Python
星號
2.2k
分支
669
平均合併
1 天 1 小時
30 天內合併 PR
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 摘要。