Skip to content

Commit 71aafde

Browse files
committed
fix: use exact match for loopback hosts in issuer URL validation
validate_issuer_url() used startswith("127.0.0.1") to exempt loopback addresses from the HTTPS requirement. This prefix match incorrectly allowed non-loopback hostnames like 127.0.0.1.evil.com or 127.0.0.1something.example.com to bypass the HTTPS check. Replace with an exact match against the set of loopback hosts (localhost, 127.0.0.1, [::1]), consistent with the DNS rebinding protection elsewhere in the codebase. This also adds the missing IPv6 loopback (::1) exemption. Remove pragma: no cover from validation branches now that they have dedicated test coverage.
1 parent b9431d4 commit 71aafde

File tree

2 files changed

+52
-9
lines changed

2 files changed

+52
-9
lines changed

src/mcp/server/auth/routes.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,15 @@ def validate_issuer_url(url: AnyHttpUrl):
3131
ValueError: If the issuer URL is invalid
3232
"""
3333

34-
# RFC 8414 requires HTTPS, but we allow localhost HTTP for testing
35-
if (
36-
url.scheme != "https"
37-
and url.host != "localhost"
38-
and (url.host is not None and not url.host.startswith("127.0.0.1"))
39-
):
40-
raise ValueError("Issuer URL must be HTTPS") # pragma: no cover
34+
# RFC 8414 requires HTTPS, but we allow loopback/localhost HTTP for testing
35+
if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"):
36+
raise ValueError("Issuer URL must be HTTPS")
4137

4238
# No fragments or query parameters allowed
4339
if url.fragment:
44-
raise ValueError("Issuer URL must not have a fragment") # pragma: no cover
40+
raise ValueError("Issuer URL must not have a fragment")
4541
if url.query:
46-
raise ValueError("Issuer URL must not have a query string") # pragma: no cover
42+
raise ValueError("Issuer URL must not have a query string")
4743

4844

4945
AUTHORIZATION_PATH = "/authorize"

tests/server/auth/test_routes.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import pytest
2+
from pydantic import AnyHttpUrl
3+
4+
from mcp.server.auth.routes import validate_issuer_url
5+
6+
7+
def test_validate_issuer_url_https_allowed():
8+
validate_issuer_url(AnyHttpUrl("https://example.com/path"))
9+
10+
11+
def test_validate_issuer_url_http_localhost_allowed():
12+
validate_issuer_url(AnyHttpUrl("http://localhost:8080/path"))
13+
14+
15+
def test_validate_issuer_url_http_127_0_0_1_allowed():
16+
validate_issuer_url(AnyHttpUrl("http://127.0.0.1:8080/path"))
17+
18+
19+
def test_validate_issuer_url_http_ipv6_loopback_allowed():
20+
validate_issuer_url(AnyHttpUrl("http://[::1]:8080/path"))
21+
22+
23+
def test_validate_issuer_url_http_non_loopback_rejected():
24+
with pytest.raises(ValueError, match="Issuer URL must be HTTPS"):
25+
validate_issuer_url(AnyHttpUrl("http://evil.com/path"))
26+
27+
28+
def test_validate_issuer_url_http_127_prefix_domain_rejected():
29+
"""A domain like 127.0.0.1.evil.com is not loopback."""
30+
with pytest.raises(ValueError, match="Issuer URL must be HTTPS"):
31+
validate_issuer_url(AnyHttpUrl("http://127.0.0.1.evil.com/path"))
32+
33+
34+
def test_validate_issuer_url_http_127_prefix_subdomain_rejected():
35+
"""A domain like 127.0.0.1something.example.com is not loopback."""
36+
with pytest.raises(ValueError, match="Issuer URL must be HTTPS"):
37+
validate_issuer_url(AnyHttpUrl("http://127.0.0.1something.example.com/path"))
38+
39+
40+
def test_validate_issuer_url_fragment_rejected():
41+
with pytest.raises(ValueError, match="fragment"):
42+
validate_issuer_url(AnyHttpUrl("https://example.com/path#frag"))
43+
44+
45+
def test_validate_issuer_url_query_rejected():
46+
with pytest.raises(ValueError, match="query"):
47+
validate_issuer_url(AnyHttpUrl("https://example.com/path?q=1"))

0 commit comments

Comments
 (0)