Skip to content

Add error_description and hint for oauth flows#28471

Merged
mateo-berri merged 5 commits into
litellm_internal_stagingfrom
litellm_make_oauth_error_moredescriptive
May 22, 2026
Merged

Add error_description and hint for oauth flows#28471
mateo-berri merged 5 commits into
litellm_internal_stagingfrom
litellm_make_oauth_error_moredescriptive

Conversation

@Sameerlite

@Sameerlite Sameerlite commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Screenshots / Proof of Fix

Type

🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test

Changes

image

@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.72131% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ellm/proxy/_experimental/mcp_server/oauth_utils.py 96.72% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR improves MCP OAuth error responses by replacing plain-string "invalid_request" HTTP 400 details with structured RFC 6749 JSON objects containing error, error_description, and an actionable hint. It also refactors validate_trusted_redirect_uri into smaller, single-purpose helpers for parsing, shape validation, allowlist checking, rejection-message building, and raising.

  • _oauth_invalid_request is introduced as a -> NoReturn helper that centralises error construction; _build_trusted_redirect_rejection_message deliberately omits proxy-internal hostnames from the client-facing body while logging full diagnostic detail (proxy base, env var, forwarded headers) server-side.
  • Both validate_trusted_redirect_uri and validate_loopback_redirect_uri are updated to the new helper, making error shapes consistent across all OAuth paths.
  • The test is updated to assert the richer dict structure rather than the old string, which is a legitimate strengthening of coverage.

Confidence Score: 5/5

Safe to merge — the change only affects error response shape on the rejection path of an experimental OAuth endpoint, with all previously flagged concerns (NoReturn annotation, PROXY_BASE_URL leakage, loopback inconsistency) addressed in this revision.

The logic for accepting or rejecting redirect URIs is unchanged; only the error payload on the rejection path is modified. Internal proxy topology is kept out of HTTP responses by design, logged server-side instead. The test update reflects the genuine new contract and is strictly more specific than before. No authentication bypass, no data mutation, and no new network calls are introduced.

No files require special attention.

Important Files Changed

Filename Overview
litellm/proxy/_experimental/mcp_server/oauth_utils.py Refactors OAuth redirect URI validation into focused helper functions and replaces plain-string "invalid_request" errors with structured RFC 6749 JSON responses (error + error_description + hint). The _oauth_invalid_request helper is correctly annotated -> NoReturn; _build_trusted_redirect_rejection_message intentionally omits proxy internals from the HTTP body while logging full diagnostic detail server-side. Both validate_trusted_redirect_uri and validate_loopback_redirect_uri are updated consistently.
tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py Test updated to assert the new structured error dict shape (error, error_description, hint) instead of the old plain-string check. The new assertions are strictly more specific and correctly reflect the changed contract, not weakened coverage.

Reviews (3): Last reviewed commit: "fix(mcp-oauth): remove dead userinfo che..." | Re-trigger Greptile

Comment thread litellm/proxy/_experimental/mcp_server/oauth_utils.py Outdated
Comment thread litellm/proxy/_experimental/mcp_server/oauth_utils.py Outdated
…config

Use NoReturn on _oauth_invalid_request, structured errors for BYOK loopback
validation, and refactor validate_trusted_redirect_uri to satisfy PLR0915.
Keep PROXY_BASE_URL and raw proxy_base_url in server logs only, not in the
HTTP 400 body returned to unauthenticated callers.

Co-authored-by: Cursor <[email protected]>
Comment thread litellm/proxy/_experimental/mcp_server/oauth_utils.py Outdated
Comment thread litellm/proxy/_experimental/mcp_server/oauth_utils.py
…0 body

The trusted-redirect-uri rejection helper included the proxy's
resolved scheme/host/port (e.g. http://litellm-internal:4000) in both
the error_description and as a top-level proxy_origin field. Since
the OAuth /authorize endpoint is unauthenticated, any caller could
probe with a crafted redirect_uri and enumerate the internal network
topology behind a reverse proxy.

Keep full diagnostic detail in the server-side warning log
(including the computed proxy base) but omit proxy-side values from
the HTTP 400 body. Also drop the duplicated origin computation in
_raise_trusted_redirect_uri_rejected now that those values are no
longer needed by the response.

Co-authored-by: Yassin Kortam <[email protected]>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Sameerlite
❌ cursoragent
You have signed the CLA already but the status is still pending? Let us recheck it.

@Sameerlite

Copy link
Copy Markdown
Collaborator Author

@greptileai rereview

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high mode and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Unreachable dead code with misleading error message
    • Split the combined netloc+userinfo check into two distinct checks so the userinfo error path is reachable and each message reflects the actual failure mode.
Preview (a31111e8d7)
diff --git a/litellm/proxy/_experimental/mcp_server/oauth_utils.py b/litellm/proxy/_experimental/mcp_server/oauth_utils.py
--- a/litellm/proxy/_experimental/mcp_server/oauth_utils.py
+++ b/litellm/proxy/_experimental/mcp_server/oauth_utils.py
@@ -3,8 +3,8 @@
 
 import os
 from ipaddress import ip_address
-from typing import List, Optional
-from urllib.parse import urlparse, urlunparse
+from typing import Any, Dict, List, NoReturn, Optional
+from urllib.parse import ParseResult, urlparse, urlunparse
 
 from fastapi import HTTPException, Request
 
@@ -43,6 +43,33 @@
 _warned_invalid_proxy_base_url: Optional[str] = None
 
 
+def _oauth_invalid_request(
+    error_description: str,
+    *,
+    hint: Optional[str] = None,
+    **extra: Any,
+) -> NoReturn:
+    """Raise ``invalid_request`` (RFC 6749) with a debuggable description.
+
+    FastAPI serializes ``detail`` as JSON. Callers still see ``error``:
+    ``invalid_request``; ``error_description`` and ``hint`` explain what
+    failed and how to fix it (e.g. reverse-proxy / PROXY_BASE_URL issues).
+    """
+    detail: Dict[str, Any] = {
+        "error": "invalid_request",
+        "error_description": error_description,
+    }
+    if hint:
+        detail["hint"] = hint
+    detail.update(extra)
+    raise HTTPException(status_code=400, detail=detail)
+
+
+def _origin_label(scheme: str, netloc: str) -> str:
+    """Human-readable origin for error messages (scheme + host[:port])."""
+    return f"{scheme}://{netloc}" if netloc else f"{scheme}://"
+
+
 def _resolve_proxy_base_url_env() -> Optional[str]:
     global _warned_invalid_proxy_base_url
     configured = os.environ.get("PROXY_BASE_URL", "").strip()
@@ -118,17 +145,15 @@
     ``"127.0.0.1"`` alone would miss ``127.0.0.2`` and the full-form
     IPv6 loopback ``0:0:0:0:0:0:0:1``.
     """
-    try:
-        parsed = urlparse(redirect_uri)
-    except ValueError:
-        raise HTTPException(status_code=400, detail="invalid_request")
+    parsed = _parse_redirect_uri_for_validation(redirect_uri)
     if parsed.scheme not in ("http", "https"):
-        raise HTTPException(status_code=400, detail="invalid_request")
-    # Fragments are not allowed in OAuth redirect URIs (RFC 6749 §3.1.2)
-    # — rejecting them prevents a ``http://127.0.0.1/cb#frag?code=...``
-    # from silently eating the authorization code.
+        _oauth_invalid_request(
+            f"redirect_uri scheme {parsed.scheme!r} is not allowed; use http or https.",
+        )
     if parsed.fragment:
-        raise HTTPException(status_code=400, detail="invalid_request")
+        _oauth_invalid_request(
+            "redirect_uri must not contain a URL fragment (#...).",
+        )
     host = (parsed.hostname or "").lower()
     if host == "localhost":
         return
@@ -139,7 +164,10 @@
         # Unparseable host (malformed IPv6, etc.) — treat as invalid,
         # don't let it bubble up as a 500.
         pass
-    raise HTTPException(status_code=400, detail="invalid_request")
+    _oauth_invalid_request(
+        "redirect_uri must use a loopback host (localhost or 127.0.0.0/8).",
+        hint="Native MCP clients should register a callback on http://127.0.0.1:<port>/...",
+    )
 
 
 def _strip_default_port(scheme: str, netloc: str) -> str:
@@ -293,76 +321,62 @@
     return False
 
 
-def validate_trusted_redirect_uri(request: Request, redirect_uri: str) -> None:
-    """Accept ``redirect_uri`` when it is (a) same-origin with the
-    proxy's own request origin, (b) loopback, (c) listed in the
-    ``MCP_TRUSTED_REDIRECT_ORIGINS`` ops allowlist, or (d) a built-in /
-    env-configured native MCP client callback (e.g. ``cursor://``).
+def _parse_redirect_uri_for_validation(redirect_uri: str) -> ParseResult:
+    try:
+        return urlparse(redirect_uri)
+    except ValueError:
+        _oauth_invalid_request(
+            "redirect_uri is not a valid URL.",
+            hint="Use a full absolute URL for redirect_uri (e.g. https://your-host/ui/mcp/oauth/callback).",
+        )
 
-    Same-origin is VERIA-57's threat-model-safe equivalent of loopback:
-    an attacker who can host content on the proxy's own HTTPS origin
-    has already compromised the proxy, so the open-redirect + code-
-    theft primitive that motivated the loopback-only rule does not
-    apply. The same reasoning extends to ops-trusted first-party
-    hosts (e.g. an internal web app registering as an OAuth client of
-    the proxy on a sister domain).
 
-    Allowlisted non-loopback hosts are accepted only when the
-    redirect_uri scheme is ``https`` — an attacker on the network
-    cannot elevate to https without controlling the host's TLS key.
-
-    Use this in the discoverable OAuth proxy endpoints that serve both
-    native clients and the proxy's UI / cross-origin web clients. The
-    BYOK endpoints, which only serve native MCP clients, retain
-    :func:`validate_loopback_redirect_uri`.
-    """
-    try:
-        parsed = urlparse(redirect_uri)
-    except ValueError:
-        raise HTTPException(status_code=400, detail="invalid_request")
+def _validate_trusted_http_redirect_shape(parsed: ParseResult) -> bool:
+    """Return True when ``parsed`` is an allowlisted native callback (caller may return)."""
     if parsed.scheme not in ("http", "https"):
         if _matches_trusted_native_redirect_uri(parsed):
-            return
-        raise HTTPException(status_code=400, detail="invalid_request")
+            return True
+        _oauth_invalid_request(
+            f"redirect_uri scheme {parsed.scheme!r} is not allowed; use http/https "
+            "or a registered native callback (e.g. cursor://).",
+            hint="Add the full URI to MCP_TRUSTED_NATIVE_REDIRECT_URIS for custom native clients.",
+        )
     if parsed.fragment:
-        raise HTTPException(status_code=400, detail="invalid_request")
-    if not parsed.netloc or parsed.username is not None or parsed.password is not None:
-        raise HTTPException(status_code=400, detail="invalid_request")
-    # Reject userinfo (``user:pass@host``) outright: OAuth redirect_uris
-    # have no legitimate reason to carry credentials, and allowing them
-    # opens a host-confusion attack where the netloc *looks* allowlisted
-    # (``app.example.com:[email protected]``) but the browser navigates
-    # to the post-``@`` host and hands the authorization code to the
-    # attacker. We compare against ``hostname`` after this, but defense in
-    # depth keeps malformed netloc strings from reaching the wildcard
-    # splitter.
+        _oauth_invalid_request(
+            "redirect_uri must not contain a URL fragment (#...).",
+        )
+    if not parsed.netloc:
+        _oauth_invalid_request(
+            "redirect_uri must include a host (e.g. https://your-host/path).",
+        )
     if parsed.username is not None or parsed.password is not None:
-        raise HTTPException(status_code=400, detail="invalid_request")
-    # Reject backslash in netloc: urlparse keeps ``\`` as part of netloc,
-    # but browsers normalize ``\`` to ``/`` for http(s) URLs and treat it
-    # as the start of the path. An attacker can exploit that split by
-    # crafting ``https://attacker.net\app.example.com/cb`` — urlparse sees
-    # ``attacker.net\app.example.com`` (matches ``*.example.com``) while
-    # the browser navigates to ``attacker.net`` with the auth code.
+        _oauth_invalid_request(
+            "redirect_uri must not contain userinfo (user:pass@host).",
+        )
     if "\\" in parsed.netloc:
-        raise HTTPException(status_code=400, detail="invalid_request")
+        _oauth_invalid_request(
+            "redirect_uri host must not contain backslashes.",
+        )
+    return False
 
-    redirect_netloc = _strip_default_port(parsed.scheme, parsed.netloc)
 
-    # (a) Same-origin. Swallow ``get_request_base_url`` failures so the
-    # loopback + allowlist paths remain reachable when the origin can't
-    # be determined (e.g. request came from an untrusted proxy and
-    # ``get_request_base_url`` raised).
-    proxy_base: Optional[str] = None
+def _resolve_proxy_base_for_redirect(request: Request) -> Optional[str]:
     try:
-        proxy_base = get_request_base_url(request)
+        return get_request_base_url(request)
     except Exception as exc:
         verbose_logger.warning(
             "validate_trusted_redirect_uri: could not determine proxy origin, "
             "falling back to loopback + allowlist. error=%s",
             exc,
         )
-        proxy_base = None
+        return None
+
+
+def _trusted_redirect_uri_is_allowed(
+    parsed: ParseResult,
+    redirect_netloc: str,
+    proxy_base: Optional[str],
+) -> bool:
     if proxy_base:
         proxy_parsed = urlparse(proxy_base)
         if (
@@ -370,37 +384,100 @@
             and redirect_netloc
             == _strip_default_port(proxy_parsed.scheme, proxy_parsed.netloc)
         ):
-            return
+            return True
 
-    # (b) Loopback — same rule as validate_loopback_redirect_uri.
     host = (parsed.hostname or "").lower()
     if host == "localhost":
-        return
+        return True
     try:
         if ip_address(host).is_loopback:
-            return
+            return True
     except ValueError:
         pass
 
-    # (c) Ops allowlist. https only.
     if parsed.scheme == "https":
         for entry in _parse_trusted_redirect_origins():
             if _matches_trusted_origin_entry(redirect_netloc, entry):
-                return
+                return True
+    return False
 
+
+def _build_trusted_redirect_rejection_message(
+    redirect_uri: str,
+    parsed: ParseResult,
+    redirect_netloc: str,
+    proxy_base: Optional[str],
+) -> str:
+    """Build a client-facing rejection message.
+
+    Intentionally omits the proxy's resolved scheme / host / port to avoid
+    leaking internal network topology (e.g. ``http://litellm-internal:4000``)
+    through an unauthenticated endpoint. Full diagnostic detail — including
+    the computed proxy base — is logged server-side by the caller.
+    """
+    redirect_origin = _origin_label(parsed.scheme, redirect_netloc)
+    proxy_parsed = urlparse(proxy_base) if proxy_base else None
+    proxy_netloc_norm = (
+        _strip_default_port(proxy_parsed.scheme, proxy_parsed.netloc)
+        if proxy_parsed and proxy_parsed.netloc
+        else ""
+    )
+
+    mismatch_parts: List[str] = []
+    if proxy_parsed and proxy_parsed.netloc:
+        if parsed.scheme != proxy_parsed.scheme:
+            mismatch_parts.append(
+                f"scheme: redirect_uri uses {parsed.scheme!r}, but the proxy "
+                "resolved a different scheme "
+                "(TLS often terminates at ingress — set PROXY_BASE_URL to https://… "
+                "or trust X-Forwarded-Proto from your ingress)"
+            )
+        if redirect_netloc != proxy_netloc_norm:
+            mismatch_parts.append(
+                f"host/port: redirect_uri {redirect_netloc!r} does not match "
+                "the proxy origin"
+            )
+
+    if mismatch_parts:
+        return (
+            f"redirect_uri origin ({redirect_origin}) does not match the proxy "
+            "origin. " + "; ".join(mismatch_parts)
+        )
+    return (
+        f"redirect_uri ({redirect_uri!r}) is not allowed: not same-origin with "
+        f"the proxy origin, not loopback, and not listed in "
+        f"{_TRUSTED_REDIRECT_ORIGINS_ENV}."
+    )
+
+
+def _raise_trusted_redirect_uri_rejected(
+    request: Request,
+    redirect_uri: str,
+    parsed: ParseResult,
+    redirect_netloc: str,
+    proxy_base: Optional[str],
+) -> NoReturn:
+    description = _build_trusted_redirect_rejection_message(
+        redirect_uri, parsed, redirect_netloc, proxy_base
+    )
+
+    hint = (
+        "Align the proxy public URL with the browser URL. Set PROXY_BASE_URL to your "
+        "HTTPS origin (e.g. https://litellm.example.com), or enable "
+        "general_settings.use_x_forwarded_for with mcp_trusted_proxy_ranges for your "
+        "ingress. Verify: curl https://<host>/.well-known/oauth-authorization-server "
+        "| jq .issuer — issuer must match window.location.origin in the UI."
+    )
+
     verbose_logger.warning(
-        "MCP OAuth: rejecting redirect_uri %r as invalid_request. "
+        "MCP OAuth: rejecting redirect_uri %r. %s "
         "Computed proxy base=%r (PROXY_BASE_URL=%r). "
         "Inbound headers: X-Forwarded-Proto=%r X-Forwarded-Host=%r "
         "X-Forwarded-Port=%r Host=%r. "
         "Trusted-redirect-origins env=%r. "
-        "Trusted-native-redirect-uris env=%r. "
-        "If this should be accepted, either align ingress X-Forwarded-* "
-        "with the browser URL, set PROXY_BASE_URL to your public origin, "
-        "add the redirect_uri host to MCP_TRUSTED_REDIRECT_ORIGINS, or "
-        "for native MCP clients (cursor://, etc.) add the full redirect_uri "
-        "to MCP_TRUSTED_NATIVE_REDIRECT_URIS.",
+        "Trusted-native-redirect-uris env=%r.",
         redirect_uri,
+        description,
         proxy_base,
         os.environ.get("PROXY_BASE_URL"),
         request.headers.get("X-Forwarded-Proto"),
@@ -410,4 +487,44 @@
         os.environ.get(_TRUSTED_REDIRECT_ORIGINS_ENV),
         os.environ.get(_TRUSTED_NATIVE_REDIRECT_URIS_ENV),
     )
-    raise HTTPException(status_code=400, detail="invalid_request")
+
+    _oauth_invalid_request(
+        description,
+        hint=hint,
+        redirect_uri=redirect_uri,
+    )
+
+
+def validate_trusted_redirect_uri(request: Request, redirect_uri: str) -> None:
+    """Accept ``redirect_uri`` when it is (a) same-origin with the
+    proxy's own request origin, (b) loopback, (c) listed in the
+    ``MCP_TRUSTED_REDIRECT_ORIGINS`` ops allowlist, or (d) a built-in /
+    env-configured native MCP client callback (e.g. ``cursor://``).
+
+    Same-origin is VERIA-57's threat-model-safe equivalent of loopback:
+    an attacker who can host content on the proxy's own HTTPS origin
+    has already compromised the proxy, so the open-redirect + code-
+    theft primitive that motivated the loopback-only rule does not
+    apply. The same reasoning extends to ops-trusted first-party
+    hosts (e.g. an internal web app registering as an OAuth client of
+    the proxy on a sister domain).
+
+    Allowlisted non-loopback hosts are accepted only when the
+    redirect_uri scheme is ``https`` — an attacker on the network
+    cannot elevate to https without controlling the host's TLS key.
+
+    Use this in the discoverable OAuth proxy endpoints that serve both
+    native clients and the proxy's UI / cross-origin web clients. The
+    BYOK endpoints, which only serve native MCP clients, retain
+    :func:`validate_loopback_redirect_uri`.
+    """
+    parsed = _parse_redirect_uri_for_validation(redirect_uri)
+    if _validate_trusted_http_redirect_shape(parsed):
+        return
+    redirect_netloc = _strip_default_port(parsed.scheme, parsed.netloc)
+    proxy_base = _resolve_proxy_base_for_redirect(request)
+    if _trusted_redirect_uri_is_allowed(parsed, redirect_netloc, proxy_base):
+        return
+    _raise_trusted_redirect_uri_rejected(
+        request, redirect_uri, parsed, redirect_netloc, proxy_base
+    )

diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py
--- a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py
+++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py
@@ -1345,7 +1345,13 @@
                 "https://litellm.example.com/ui/mcp/oauth/callback",
             )
         assert exc_info.value.status_code == 400
-        assert exc_info.value.detail == "invalid_request"
+        detail = exc_info.value.detail
+        assert isinstance(detail, dict)
+        assert detail.get("error") == "invalid_request"
+        assert "error_description" in detail
+        assert "redirect_uri origin" in detail["error_description"]
+        assert "proxy origin" in detail["error_description"]
+        assert "hint" in detail
 
     matching = [r for r in caplog.records if "rejecting redirect_uri" in r.getMessage()]
     assert len(matching) == 1, (

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit bb800ab. Configure here.

Comment thread litellm/proxy/_experimental/mcp_server/oauth_utils.py
The first check combined missing netloc with userinfo presence, making
the second userinfo-only check unreachable. Split into two distinct
checks so each error message reflects the actual failure mode.

Co-authored-by: Yassin Kortam <[email protected]>
@Sameerlite

Copy link
Copy Markdown
Collaborator Author

@greptileai rereview

@krrish-berri-2

Copy link
Copy Markdown
Contributor

@Sameerlite — could you add a screenshot or short video showing that this change works as expected? It really helps reviewers verify the fix quickly. Thanks!

@Sameerlite

Copy link
Copy Markdown
Collaborator Author

Added

@mateo-berri mateo-berri self-requested a review May 22, 2026 16:00

@mateo-berri mateo-berri left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; thanks!

@mateo-berri mateo-berri merged commit 7a93cce into litellm_internal_staging May 22, 2026
115 of 117 checks passed
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
* Add error_description and hint for oauth flows

* Fix tests

* fix(mcp-oauth): improve redirect_uri errors without leaking internal config

Use NoReturn on _oauth_invalid_request, structured errors for BYOK loopback
validation, and refactor validate_trusted_redirect_uri to satisfy PLR0915.
Keep PROXY_BASE_URL and raw proxy_base_url in server logs only, not in the
HTTP 400 body returned to unauthenticated callers.

Co-authored-by: Cursor <[email protected]>

* fix(mcp-oauth): stop leaking internal proxy origin in redirect_uri 400 body

The trusted-redirect-uri rejection helper included the proxy's
resolved scheme/host/port (e.g. http://litellm-internal:4000) in both
the error_description and as a top-level proxy_origin field. Since
the OAuth /authorize endpoint is unauthenticated, any caller could
probe with a crafted redirect_uri and enumerate the internal network
topology behind a reverse proxy.

Keep full diagnostic detail in the server-side warning log
(including the computed proxy base) but omit proxy-side values from
the HTTP 400 body. Also drop the duplicated origin computation in
_raise_trusted_redirect_uri_rejected now that those values are no
longer needed by the response.

Co-authored-by: Yassin Kortam <[email protected]>

* fix(mcp-oauth): remove dead userinfo check in redirect_uri validation

The first check combined missing netloc with userinfo presence, making
the second userinfo-only check unreachable. Split into two distinct
checks so each error message reflects the actual failure mode.

Co-authored-by: Yassin Kortam <[email protected]>

---------

Co-authored-by: Cursor <[email protected]>
Co-authored-by: Yassin Kortam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants