fix(endpoints): normalize URL handling (#4338)

src/endpoint_resolver.py

@@ -161,6 +161,32 @@ def normalize_base(url: str) -> str:
     return url
 
 
+def _validated_endpoint_base(url: str) -> str:
+    """Return a base URL that is safe for endpoint path appends."""
+    base = (url or "").strip().rstrip("/")
+    if "?" in base or "#" in base:
+        raise ValueError("Endpoint base URL must not include query or fragment")
+    return urlunparse(urlparse(base)._replace(query="", fragment="")).rstrip("/")
+
+
+def _prepare_endpoint_base(base: str) -> str:
+    base = _validated_endpoint_base(normalize_base(base))
+    return _validated_endpoint_base(normalize_base(resolve_url(base)))
+
+
+def _append_endpoint_path(base: str, suffix: str) -> str:
+    parsed = urlparse(base)
+    current = (parsed.path or "").rstrip("/")
+    extra = "/" + suffix.lstrip("/")
+    path = f"{current}{extra}" if current else extra
+    return urlunparse(parsed._replace(path=path, query="", fragment=""))
+
+
+def _pathless_host(base: str, host: str) -> bool:
+    parsed = urlparse(base)
+    return (parsed.hostname or "").lower() == host and not (parsed.path or "").strip("/")
+
+
 def _anthropic_api_root(base: str) -> str:
     """Return Anthropic's API root, preserving /v1 for OpenAI-compatible APIs elsewhere."""
     base = (base or "").strip().rstrip("/")
@@ -171,15 +197,17 @@ def _anthropic_api_root(base: str) -> str:
 
 def build_chat_url(base: str) -> str:
     """Return the correct chat endpoint URL for a given base."""
-    base = resolve_url(base)
+    base = _prepare_endpoint_base(base)
     provider = _detect_provider(base)
     if provider == "anthropic":
-        return _anthropic_api_root(base) + "/v1/messages"
+        return _append_endpoint_path(_anthropic_api_root(base), "/v1/messages")
     if provider == "ollama":
-        return _ollama_api_root(base) + "/chat"
+        return _append_endpoint_path(_ollama_api_root(base), "/chat")
     if provider == "chatgpt-subscription":
-        return base.rstrip("/") + "/responses"
-    return base + "/chat/completions"
+        return _append_endpoint_path(base, "/responses")
+    if _pathless_host(base, "api.openai.com"):
+        base = _append_endpoint_path(base, "/v1")
+    return _append_endpoint_path(base, "/chat/completions")
 
 
 def build_models_url(base: str) -> Optional[str]:
@@ -193,12 +221,12 @@ def build_models_url(base: str) -> Optional[str]:
     untouched (so custom prefixes like ``/openai`` or ``/api/openai/v1`` keep
     their semantics).
     """
-    base = normalize_base(resolve_url(base))
+    base = _prepare_endpoint_base(base)
     provider = _detect_provider(base)
     if provider == "anthropic":
-        return _anthropic_api_root(base) + "/v1/models"
+        return _append_endpoint_path(_anthropic_api_root(base), "/v1/models")
     if provider == "ollama":
-        return _ollama_api_root(base) + "/tags"
+        return _append_endpoint_path(_ollama_api_root(base), "/tags")
     if provider == "chatgpt-subscription":
         return None
     # Generic OpenAI-compatible fallback: local model servers with no explicit
@@ -208,10 +236,10 @@ def build_models_url(base: str) -> Optional[str]:
     parsed = urlparse(base)
     host = (parsed.hostname or "").lower()
     is_local = host in {"localhost", "127.0.0.1", "::1", "host.docker.internal"}
-    uses_v1_models_by_default = is_local or host in {"api.deepseek.com"}
+    uses_v1_models_by_default = is_local or host in {"api.deepseek.com", "api.openai.com"}
     if not parsed.path and uses_v1_models_by_default:
-        base = base + "/v1"
-    return base + "/models"
+        base = _append_endpoint_path(base, "/v1")
+    return _append_endpoint_path(base, "/models")
 
 
 def build_headers(api_key: Optional[str], base: str) -> Dict[str, str]:

src/integrations.py

@@ -4,6 +4,7 @@ import uuid
 import logging
 import re
 from typing import Dict, List, Optional, Any
+from urllib.parse import urljoin, urlparse, urlunparse
 
 import httpx
 from fastapi import HTTPException
@@ -202,6 +203,22 @@ def mask_integration_secret(integration: Dict[str, Any]) -> Dict[str, Any]:
     return safe
 
 
+def _normalize_integration_base_url(base_url: Any) -> str:
+    if not isinstance(base_url, str) or not base_url.strip():
+        raise ValueError("Integration base URL is required")
+    cleaned = base_url.strip().rstrip("/")
+    if "?" in cleaned or "#" in cleaned:
+        raise ValueError("Integration base URL must not include query or fragment")
+    parsed = urlparse(cleaned)
+    if parsed.scheme.lower() not in ("http", "https") or not parsed.hostname:
+        raise ValueError("Integration base URL must be an HTTP(S) URL")
+    return urlunparse(parsed._replace(scheme=parsed.scheme.lower(), query="", fragment="")).rstrip("/")
+
+
+def _join_integration_url(base_url: str, path: str) -> str:
+    return urljoin(base_url.rstrip("/") + "/", path.lstrip("/"))
+
+
 def load_integrations() -> List[Dict[str, Any]]:
     """Load all integrations from disk with secrets decrypted for runtime use."""
     if not os.path.exists(DATA_FILE):
@@ -261,8 +278,10 @@ def add_integration(data: Dict[str, Any]) -> Dict[str, Any]:
 
     if not isinstance(integration.get("name"), str) or not integration["name"].strip():
         raise HTTPException(400, "Integration name is required")
-    if not isinstance(integration.get("base_url"), str) or not integration["base_url"].strip():
-        raise HTTPException(400, "Integration base URL is required")
+    try:
+        integration["base_url"] = _normalize_integration_base_url(integration.get("base_url"))
+    except ValueError as exc:
+        raise HTTPException(400, str(exc)) from exc
 
     integrations = load_integrations()
     integrations.append(integration)
@@ -272,10 +291,14 @@ def add_integration(data: Dict[str, Any]) -> Dict[str, Any]:
 
 def update_integration(integration_id: str, data: Dict[str, Any]) -> Optional[Dict[str, Any]]:
     """Update fields on an existing integration. Returns updated integration or None."""
+    data = dict(data)
     if "name" in data and (not isinstance(data["name"], str) or not data["name"].strip()):
         raise HTTPException(400, "Integration name is required")
-    if "base_url" in data and (not isinstance(data["base_url"], str) or not data["base_url"].strip()):
-        raise HTTPException(400, "Integration base URL is required")
+    if "base_url" in data:
+        try:
+            data["base_url"] = _normalize_integration_base_url(data["base_url"])
+        except ValueError as exc:
+            raise HTTPException(400, str(exc)) from exc
 
     integrations = load_integrations()
     for item in integrations:
@@ -341,9 +364,10 @@ async def execute_api_call(
     if not integration.get("enabled", True):
         return {"error": f"Integration '{integration.get('name')}' is disabled", "exit_code": 1}
 
-    base_url = integration.get("base_url", "").rstrip("/")
-    if not base_url:
-        return {"error": "Integration has no base_url configured", "exit_code": 1}
+    try:
+        base_url = _normalize_integration_base_url(integration.get("base_url", ""))
+    except ValueError as exc:
+        return {"error": str(exc), "exit_code": 1}
 
     # Strip common API path suffixes users might accidentally include
     # (e.g. "http://host/v1/" → "http://host"). The integration's preset
@@ -366,7 +390,10 @@ async def execute_api_call(
     if re.search(r"^https?://", path) or "://" in path:
         return {"error": "Path must not contain a protocol scheme", "exit_code": 1}
 
-    url = base_url + path
+    if "#" in path:
+        return {"error": "Path must not contain a fragment", "exit_code": 1}
+
+    url = _join_integration_url(base_url, path)
     method = method.upper()
 
     # Build headers

src/model_context.py

@@ -17,10 +17,11 @@ import httpx
 logger = logging.getLogger(__name__)
 
 _LOCAL_HOSTS = {"localhost", "127.0.0.1", "0.0.0.0", "::1", "host.docker.internal"}
-_PRIVATE_PREFIXES = ("10.", "172.16.", "172.17.", "172.18.", "172.19.",
-                     "172.20.", "172.21.", "172.22.", "172.23.", "172.24.",
-                     "172.25.", "172.26.", "172.27.", "172.28.", "172.29.",
-                     "172.30.", "172.31.", "192.168.")
+_PRIVATE_NETWORKS = (
+    ipaddress.ip_network("10.0.0.0/8"),
+    ipaddress.ip_network("172.16.0.0/12"),
+    ipaddress.ip_network("192.168.0.0/16"),
+)
 
 # Tailscale uses the CGNAT range 100.64.0.0/10, NOT all of 100.0.0.0/8.
 # A bare "100." prefix would classify public addresses (e.g. AWS ranges
@@ -36,6 +37,14 @@ def _in_tailscale_range(host: str) -> bool:
         return False
 
 
+def _is_private_ip_literal(host: str) -> bool:
+    try:
+        ip = ipaddress.ip_address(host)
+    except ValueError:
+        return False
+    return any(ip in network for network in _PRIVATE_NETWORKS)
+
+
 def _normalize_base_for_compare(url: str) -> str:
     url = (url or "").strip().rstrip("/")
     for suffix in ("/chat/completions", "/models", "/completions", "/v1/messages"):
@@ -87,7 +96,7 @@ def is_local_endpoint(url: str) -> bool:
         return True
     try:
         host = urlparse(url).hostname or ""
-        return host in _LOCAL_HOSTS or host.startswith(_PRIVATE_PREFIXES) or _in_tailscale_range(host)
+        return host in _LOCAL_HOSTS or _is_private_ip_literal(host) or _in_tailscale_range(host)
     except Exception:
         return False