From 71aafde2e12f29a2113e8edd3a2bbf7828cedca6 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Wed, 18 Feb 2026 19:35:02 +0000 Subject: [PATCH] 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. --- src/mcp/server/auth/routes.py | 14 ++++------ tests/server/auth/test_routes.py | 47 ++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 tests/server/auth/test_routes.py diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index 08f735f36..9a10ac57f 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -31,19 +31,15 @@ def validate_issuer_url(url: AnyHttpUrl): ValueError: If the issuer URL is invalid """ - # RFC 8414 requires HTTPS, but we allow localhost HTTP for testing - if ( - url.scheme != "https" - and url.host != "localhost" - and (url.host is not None and not url.host.startswith("127.0.0.1")) - ): - raise ValueError("Issuer URL must be HTTPS") # pragma: no cover + # RFC 8414 requires HTTPS, but we allow loopback/localhost HTTP for testing + if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"): + raise ValueError("Issuer URL must be HTTPS") # No fragments or query parameters allowed if url.fragment: - raise ValueError("Issuer URL must not have a fragment") # pragma: no cover + raise ValueError("Issuer URL must not have a fragment") if url.query: - raise ValueError("Issuer URL must not have a query string") # pragma: no cover + raise ValueError("Issuer URL must not have a query string") AUTHORIZATION_PATH = "/authorize" diff --git a/tests/server/auth/test_routes.py b/tests/server/auth/test_routes.py new file mode 100644 index 000000000..3d13b5ba5 --- /dev/null +++ b/tests/server/auth/test_routes.py @@ -0,0 +1,47 @@ +import pytest +from pydantic import AnyHttpUrl + +from mcp.server.auth.routes import validate_issuer_url + + +def test_validate_issuer_url_https_allowed(): + validate_issuer_url(AnyHttpUrl("https://example.com/path")) + + +def test_validate_issuer_url_http_localhost_allowed(): + validate_issuer_url(AnyHttpUrl("http://localhost:8080/path")) + + +def test_validate_issuer_url_http_127_0_0_1_allowed(): + validate_issuer_url(AnyHttpUrl("http://127.0.0.1:8080/path")) + + +def test_validate_issuer_url_http_ipv6_loopback_allowed(): + validate_issuer_url(AnyHttpUrl("http://[::1]:8080/path")) + + +def test_validate_issuer_url_http_non_loopback_rejected(): + with pytest.raises(ValueError, match="Issuer URL must be HTTPS"): + validate_issuer_url(AnyHttpUrl("http://evil.com/path")) + + +def test_validate_issuer_url_http_127_prefix_domain_rejected(): + """A domain like 127.0.0.1.evil.com is not loopback.""" + with pytest.raises(ValueError, match="Issuer URL must be HTTPS"): + validate_issuer_url(AnyHttpUrl("http://127.0.0.1.evil.com/path")) + + +def test_validate_issuer_url_http_127_prefix_subdomain_rejected(): + """A domain like 127.0.0.1something.example.com is not loopback.""" + with pytest.raises(ValueError, match="Issuer URL must be HTTPS"): + validate_issuer_url(AnyHttpUrl("http://127.0.0.1something.example.com/path")) + + +def test_validate_issuer_url_fragment_rejected(): + with pytest.raises(ValueError, match="fragment"): + validate_issuer_url(AnyHttpUrl("https://example.com/path#frag")) + + +def test_validate_issuer_url_query_rejected(): + with pytest.raises(ValueError, match="query"): + validate_issuer_url(AnyHttpUrl("https://example.com/path?q=1"))