From 2e2bb5231e9f1b3295cf08781bb045fc2b27ae9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o?= Date: Sun, 16 Aug 2026 07:09:01 +0200 Subject: [PATCH] fix(mcp): stop assuming http://localhost:7000 for the OAuth callback (#6032) * fix(mcp): stop assuming http://localhost:7000 for the OAuth callback The MCP OAuth callback origin is wrong on any install not reached at http://localhost:7000, and on Docker it cannot be corrected at all. Three sites, one assumption: - The redirect base fell back to a fixed port 7000. The app binds APP_PORT natively (app.py, launcher.py) and the macOS launcher defaults to 7860, where 7000 is AirPlay Receiver, so the callback lands on another service entirely. The fallback now follows APP_PORT. The hostname stays localhost rather than internal_api_base()'s 127.0.0.1: this URI is registered with the authorization server, so changing the host would invalidate the registrations that already exist. - The paste-back form hardcoded an http:// action. Serving the page over HTTPS, Chrome raises its insecure-form interstitial, and overriding that posts plain HTTP at a TLS port, which fails too. Either way the authorization code never reaches Odysseus. The action now carries the scheme the request arrived on. - OAUTH_REDIRECT_BASE_URL is the only fix available to a Docker install, because the container listens on 7000 and cannot see the host port map, but compose never forwarded it and nothing documented it. Both fixed. * fix(mcp): make the paste-back form action relative and export APP_PORT Answers the review on #6032. Three of the fixes did not survive contact with the deployments they targeted. - The form action derived its scheme from request.url.scheme. uvicorn only honours X-Forwarded-Proto from a peer inside --forwarded-allow-ips, which defaults to 127.0.0.1; the Dockerfile CMD sets no override, so a proxy arriving over the Docker bridge is untrusted and the scheme stays http. That is mixed content on exactly the HTTPS installs paste-back exists for. A relative action is resolved by the browser against the origin the page came from, which is right under every proxy setup, and it drops the Host header from the page entirely. - The APP_PORT fallback never fired for the shipped launchers. start-macos.sh, the generated .app launcher and launch-windows.ps1 all pass --port to uvicorn without putting the value in the environment, so the motivating case, macOS on 7860, still registered localhost:7000. Each now exports it. internal_api_base() and companion pairing read APP_PORT too and were wrong in the same way. - .env.example pointed Google MCP servers at OAUTH_REDIRECT_BASE_URL. add_server writes Desktop App credentials, and Google only accepts loopback redirects for that client type, so a public origin comes back as redirect_uri_mismatch. The variable is for the DCR flow; Google stays on the loopback default and finishes remotely through paste-back. The Host header is no longer reflected into the page, so the escaping regression test asserts its absence instead of its escaping. --- .env.example | 15 ++++ build-macos-app.sh | 4 + docker-compose.gpu-amd.yml | 5 ++ docker-compose.gpu-nvidia.yml | 5 ++ docker-compose.yml | 5 ++ launch-windows.ps1 | 4 + routes/mcp/mcp_routes.py | 20 +++-- src/mcp_oauth.py | 34 +++++--- start-macos.sh | 4 + tests/test_mcp_oauth.py | 134 +++++++++++++++++++++++++++++ tests/test_security_regressions.py | 8 +- 11 files changed, 219 insertions(+), 19 deletions(-) diff --git a/.env.example b/.env.example index 6edea7c4c..2c6c2065a 100644 --- a/.env.example +++ b/.env.example @@ -159,6 +159,21 @@ SEARXNG_INSTANCE=http://localhost:8080 # Local HTTP setups may use the callback URL inferred by the application. # GOOGLE_OAUTH_REDIRECT_URI=https://your-domain.com/api/email/oauth/google/callback +# Origin the MCP OAuth callback is sent back to, for remote (Streamable HTTP) +# MCP servers that register it dynamically. Defaults to http://localhost:$APP_PORT, +# which is right only when you reach Odysseus directly on that port. Set it for +# HTTPS, reverse-proxy, hosted, and Docker installs — inside the container the +# app always listens on 7000 and cannot see the host port map, so the default is +# wrong there whenever APP_PORT is not 7000. +# +# Not for Google MCP servers. Those use Desktop App credentials, and Google only +# accepts loopback redirect URIs for that client type, so a public origin here is +# rejected with redirect_uri_mismatch. Leave it unset for a Google-only install: +# the loopback default is what Google wants, and remote users finish through the +# paste-back page, which never has to load the redirect. +# https://developers.google.com/identity/protocols/oauth2/native-app +# OAUTH_REDIRECT_BASE_URL=https://your-domain.com + # ============================================================ # Misc # ============================================================ diff --git a/build-macos-app.sh b/build-macos-app.sh index 1208a1dce..c76075cac 100755 --- a/build-macos-app.sh +++ b/build-macos-app.sh @@ -73,6 +73,10 @@ cat > "$APP/Contents/MacOS/$APP_NAME.tmpl" <<'LAUNCHER' INSTALL_DIR="__INSTALL_DIR__" PORT="__PORT__" URL="http://127.0.0.1:${PORT}" +# uvicorn is started with --port below, but APP_PORT is what the app itself +# reads when it needs to build a URL for this instance (internal_api_base(), +# companion pairing, the MCP OAuth callback), so export it as well. +export APP_PORT="$PORT" export PATH="/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:$PATH" UVICORN="$INSTALL_DIR/venv/bin/uvicorn" diff --git a/docker-compose.gpu-amd.yml b/docker-compose.gpu-amd.yml index 85d872b58..24f65e5cb 100644 --- a/docker-compose.gpu-amd.yml +++ b/docker-compose.gpu-amd.yml @@ -75,6 +75,11 @@ services: - GOOGLE_OAUTH_CLIENT_ID=${GOOGLE_OAUTH_CLIENT_ID:-} - GOOGLE_OAUTH_CLIENT_SECRET=${GOOGLE_OAUTH_CLIENT_SECRET:-} - GOOGLE_OAUTH_REDIRECT_URI=${GOOGLE_OAUTH_REDIRECT_URI:-} + # Externally reachable origin for MCP OAuth callbacks. The container + # always listens on 7000 and cannot see the host port map above, so + # remote MCP OAuth needs this set whenever the browser reaches + # Odysseus on anything other than http://localhost:7000. + - OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-} - TAVILY_API_KEY=${TAVILY_API_KEY:-} - SERPER_API_KEY=${SERPER_API_KEY:-} # PUID / PGID — the user/group the container drops to before diff --git a/docker-compose.gpu-nvidia.yml b/docker-compose.gpu-nvidia.yml index a367be001..fc66234db 100644 --- a/docker-compose.gpu-nvidia.yml +++ b/docker-compose.gpu-nvidia.yml @@ -74,6 +74,11 @@ services: - GOOGLE_OAUTH_CLIENT_ID=${GOOGLE_OAUTH_CLIENT_ID:-} - GOOGLE_OAUTH_CLIENT_SECRET=${GOOGLE_OAUTH_CLIENT_SECRET:-} - GOOGLE_OAUTH_REDIRECT_URI=${GOOGLE_OAUTH_REDIRECT_URI:-} + # Externally reachable origin for MCP OAuth callbacks. The container + # always listens on 7000 and cannot see the host port map above, so + # remote MCP OAuth needs this set whenever the browser reaches + # Odysseus on anything other than http://localhost:7000. + - OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-} - TAVILY_API_KEY=${TAVILY_API_KEY:-} - SERPER_API_KEY=${SERPER_API_KEY:-} # PUID / PGID — the user/group the container drops to before diff --git a/docker-compose.yml b/docker-compose.yml index 39774fb53..38cb654e5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -63,6 +63,11 @@ services: - GOOGLE_OAUTH_CLIENT_ID=${GOOGLE_OAUTH_CLIENT_ID:-} - GOOGLE_OAUTH_CLIENT_SECRET=${GOOGLE_OAUTH_CLIENT_SECRET:-} - GOOGLE_OAUTH_REDIRECT_URI=${GOOGLE_OAUTH_REDIRECT_URI:-} + # Externally reachable origin for MCP OAuth callbacks. The container + # always listens on 7000 and cannot see the host port map above, so + # remote MCP OAuth needs this set whenever the browser reaches + # Odysseus on anything other than http://localhost:7000. + - OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-} - TAVILY_API_KEY=${TAVILY_API_KEY:-} - SERPER_API_KEY=${SERPER_API_KEY:-} # PUID / PGID — the user/group the container drops to before diff --git a/launch-windows.ps1 b/launch-windows.ps1 index 263d95127..ab0e3542b 100644 --- a/launch-windows.ps1 +++ b/launch-windows.ps1 @@ -163,6 +163,10 @@ if (Test-Path $cudaBase) { } # 7. Start the server (use `python -m uvicorn` - bare `uvicorn` may not be on PATH) +# -Port only reaches uvicorn as a flag. Everything that builds a URL for this +# instance - internal_api_base(), companion pairing, the MCP OAuth callback - +# reads APP_PORT, so set it too or they all assume 7000. +$env:APP_PORT = $Port Write-Step ("Starting Odysseus at http://{0}:{1}" -f $BindHost, $Port) Write-Host "Press Ctrl+C to stop." Write-Host "" diff --git a/routes/mcp/mcp_routes.py b/routes/mcp/mcp_routes.py index a0ade88b6..94c83f8dd 100644 --- a/routes/mcp/mcp_routes.py +++ b/routes/mcp/mcp_routes.py @@ -475,7 +475,7 @@ def setup_mcp_routes(mcp_manager: McpManager): return RedirectResponse(auth_url) else: # Remote device — show paste-back page - return HTMLResponse(_oauth_authorize_page(auth_url, server_id, host, redirect_uri)) + return HTMLResponse(_oauth_authorize_page(auth_url, server_id, redirect_uri)) finally: db.close() @@ -612,15 +612,13 @@ def setup_mcp_routes(mcp_manager: McpManager): def _oauth_authorize_page( auth_url: str, server_id: str, - host: str, - redirect_uri: str = "http://localhost:7000/api/mcp/oauth/callback", + redirect_uri: str, ) -> str: """Page with Google sign-in link and URL paste-back form for remote access.""" - # Escape values interpolated into the page: `host` comes from the request - # Host header and `server_id` from the OAuth state — neither is trusted. + # Escape values interpolated into the page: `server_id` comes from the OAuth + # state and is not trusted. auth_url = html.escape(auth_url, quote=True) server_id = html.escape(server_id, quote=True) - host = html.escape(host, quote=True) redirect_uri = html.escape(redirect_uri, quote=True) return f""" @@ -664,7 +662,15 @@ def _oauth_authorize_page( Sign in with Google
-
+ +

Paste the URL from your browser after signing in:


diff --git a/src/mcp_oauth.py b/src/mcp_oauth.py index 27a30383e..8c69717eb 100644 --- a/src/mcp_oauth.py +++ b/src/mcp_oauth.py @@ -15,18 +15,32 @@ from urllib.parse import urlparse, parse_qs logger = logging.getLogger(__name__) + +def _resolve_redirect_base() -> str: + """Origin the browser is sent back to after authorizing. + + Falls back to the port the app binds natively (APP_PORT, read the same way + by app.py and launcher.py) rather than a fixed 7000: the macOS launcher + defaults to 7860, and a callback on the wrong port reaches nothing. The + hostname stays `localhost` rather than internal_api_base()'s 127.0.0.1 — + this URI is registered with the authorization server (via DCR, or by hand + for Google clients), so changing the host invalidates registrations that + already exist. + """ + return ( + os.environ.get("OAUTH_REDIRECT_BASE_URL") + or os.environ.get("APP_PUBLIC_URL") + or f"http://localhost:{os.environ.get('APP_PORT', '7000')}" + ).rstrip("/") + + # OAuth redirect URI registered with every authorization server via DCR. Loopback # is allowed for native/desktop clients (RFC 8252); remote users finish via the -# paste-back flow. Deployments not reachable at http://localhost:7000 (custom -# port, reverse proxy, or public domain) must set OAUTH_REDIRECT_BASE_URL (or -# APP_PUBLIC_URL) to their externally reachable origin so the redirect lands back -# on Odysseus. APP_PORT is intentionally not used: it is only the Docker host -# port-map; the app always listens on 7000 inside the container. -_REDIRECT_BASE = ( - os.environ.get("OAUTH_REDIRECT_BASE_URL") - or os.environ.get("APP_PUBLIC_URL") - or "http://localhost:7000" -).rstrip("/") +# paste-back flow. Deployments whose externally reachable origin differs from the +# port Odysseus binds — reverse proxy, public domain, or Docker, whose host port +# map is invisible inside the container — must set OAUTH_REDIRECT_BASE_URL (or +# APP_PUBLIC_URL), otherwise the redirect never lands back on Odysseus. +_REDIRECT_BASE = _resolve_redirect_base() REDIRECT_URI = f"{_REDIRECT_BASE}/api/mcp/oauth/callback" # How long the background connect waits for the user to authorize before giving up. diff --git a/start-macos.sh b/start-macos.sh index 2aa15d261..3e9048547 100755 --- a/start-macos.sh +++ b/start-macos.sh @@ -34,6 +34,10 @@ fi # values (APP_PORT / APP_BIND), then built-in defaults. PORT="${ODYSSEUS_PORT:-${APP_PORT:-7860}}" # 7860, not 7000 — macOS AirPlay Receiver holds 7000. HOST="${ODYSSEUS_HOST:-${APP_BIND:-127.0.0.1}}" # Set APP_BIND=0.0.0.0 in .env for LAN/Tailscale access. +# The port only reaches uvicorn as a flag, so export it too: everything that +# builds a URL for this instance — internal_api_base(), the companion pairing +# code, the MCP OAuth callback — reads APP_PORT and would otherwise assume 7000. +export APP_PORT="$PORT" PROBE_HOST="$HOST" if [ "$PROBE_HOST" = "0.0.0.0" ] || [ "$PROBE_HOST" = "::" ]; then PROBE_HOST="127.0.0.1" diff --git a/tests/test_mcp_oauth.py b/tests/test_mcp_oauth.py index 6fb6f43b9..6a8e21876 100644 --- a/tests/test_mcp_oauth.py +++ b/tests/test_mcp_oauth.py @@ -130,3 +130,137 @@ def test_update_recovers_from_non_dict_oauth_tokens(): srv, storage = _fake_storage('["stale", "data"]') storage._update("tokens", {"access_token": "new"}) assert json.loads(srv.oauth_tokens) == {"tokens": {"access_token": "new"}} + + +# ── Callback origin ─────────────────────────────────────────────── +# +# The redirect URI is registered with the authorization server (dynamically for +# remote MCP servers, by hand for Google ones) and the browser is sent to it +# after authorizing. It is resolved once, outside any request, so it cannot be +# derived from the request the way the email OAuth routes derive theirs — an +# operator-supplied origin is the only thing that can be right behind a proxy. +# What the default can get right is the port, which the app knows. + +_REDIRECT_ENV = ("OAUTH_REDIRECT_BASE_URL", "APP_PUBLIC_URL", "APP_PORT") + + +def _resolve_base(monkeypatch, **env): + for key in _REDIRECT_ENV: + monkeypatch.delenv(key, raising=False) + for key, value in env.items(): + monkeypatch.setenv(key, value) + return mcp_oauth._resolve_redirect_base() + + +def test_redirect_base_defaults_to_the_bound_port(monkeypatch): + # The macOS launcher serves on 7860 because AirPlay Receiver holds 7000; a + # callback pinned to 7000 lands on AirPlay instead of Odysseus. + assert _resolve_base(monkeypatch, APP_PORT="7860") == "http://localhost:7860" + + +def test_redirect_base_keeps_7000_when_app_port_is_unset(monkeypatch): + assert _resolve_base(monkeypatch) == "http://localhost:7000" + + +def test_redirect_base_prefers_the_explicit_origin(monkeypatch): + # Only an operator-supplied origin can be right behind a TLS proxy, so it + # outranks the derived default — and its trailing slash is trimmed. + resolved = _resolve_base( + monkeypatch, OAUTH_REDIRECT_BASE_URL="https://odysseus.example/", APP_PORT="7860" + ) + assert resolved == "https://odysseus.example" + + +def test_redirect_base_accepts_app_public_url_as_the_alias(monkeypatch): + assert ( + _resolve_base(monkeypatch, APP_PUBLIC_URL="https://public.example", APP_PORT="7860") + == "https://public.example" + ) + + +# ── Paste-back form origin ──────────────────────────────────────── + +def _authorize_page(): + from routes.mcp.mcp_routes import _oauth_authorize_page + + return _oauth_authorize_page( + "https://accounts.google.com/o/oauth2/v2/auth?state=srv-1", + "srv-1", + "https://odysseus.example.com/api/mcp/oauth/callback", + ) + + +def test_paste_back_form_action_is_relative(): + # Remote users finish the flow by pasting the callback URL into this form, + # so it has to post back to the origin they are on. An absolute action + # cannot: an http:// one is mixed content on an HTTPS page and gets blocked, + # and the app cannot reliably tell that it is behind TLS, because uvicorn + # only honours X-Forwarded-Proto from a peer inside --forwarded-allow-ips + # (default 127.0.0.1, which a proxy on the Docker bridge is not). A relative + # action is resolved by the browser and is right in every one of those cases. + page = _authorize_page() + assert 'action="/api/mcp/oauth/exchange/srv-1"' in page + assert 'action="http' not in page + + +# ── Docker configurability ──────────────────────────────────────── +# +# The override above is the only fix available to a Docker install: the +# container always listens on 7000 and cannot see the host port map, so the +# derived default cannot be right there. Compose has to forward the variable +# or the escape hatch does not exist. + +_COMPOSE_FILES = ( + "docker-compose.yml", + "docker-compose.gpu-nvidia.yml", + "docker-compose.gpu-amd.yml", +) + + +def _repo_root(): + from pathlib import Path + + return Path(__file__).resolve().parent.parent + + +def test_redirect_base_override_is_forwarded_into_the_container(): + import yaml + + for name in _COMPOSE_FILES: + path = _repo_root() / name + compose = yaml.safe_load(path.read_text(encoding="utf-8")) + environment = set(compose["services"]["odysseus"]["environment"]) + assert "OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-}" in environment, name + + +def test_redirect_base_override_is_documented(): + import pytest + + env_example = _repo_root() / ".env.example" + if not env_example.exists(): + pytest.skip("this checkout does not include the optional .env.example file") + assert "# OAUTH_REDIRECT_BASE_URL=" in env_example.read_text(encoding="utf-8") + + +# ── Launcher port propagation ───────────────────────────────────── +# +# The derived default is only as good as APP_PORT, and every launcher hands the +# port to uvicorn as a command-line flag, which the app cannot read back. Each +# one has to put the same value in the environment or the callback falls back to +# 7000 — which is the macOS-launcher-on-7860 case this whole change is about. +# internal_api_base() and companion pairing read APP_PORT too, so they go wrong +# in the same way. + +_LAUNCHERS = ( + # file, the export, the uvicorn flag it has to agree with + ("start-macos.sh", 'export APP_PORT="$PORT"', '--port "$PORT"'), + ("build-macos-app.sh", 'export APP_PORT="$PORT"', '--port "$PORT"'), + ("launch-windows.ps1", "$env:APP_PORT = $Port", "--port $Port"), +) + + +def test_launchers_export_the_port_they_serve_on(): + for name, export, uvicorn_flag in _LAUNCHERS: + text = (_repo_root() / name).read_text(encoding="utf-8") + assert uvicorn_flag in text, f"{name}: launcher no longer passes {uvicorn_flag}" + assert export in text, f"{name}: serves on a port the app cannot read back" diff --git a/tests/test_security_regressions.py b/tests/test_security_regressions.py index cf899f7c0..be8d3b8a3 100644 --- a/tests/test_security_regressions.py +++ b/tests/test_security_regressions.py @@ -1027,9 +1027,13 @@ def test_session_html_export_escapes_name(): def test_mcp_oauth_page_escapes_reflected_values(): src = Path(__file__).resolve().parents[1] / "routes" / "mcp" / "mcp_routes.py" text = src.read_text() - body = text.split("def _oauth_authorize_page(", 1)[1].split("return f", 1)[0] - for var in ("auth_url", "server_id", "host", "redirect_uri"): + page = text.split("def _oauth_authorize_page(", 1)[1].split("def _oauth_result_page", 1)[0] + body = page.split("return f", 1)[0] + for var in ("auth_url", "server_id", "redirect_uri"): assert f"{var} = html.escape({var}" in body, var + # The Host header is no longer reflected at all: the paste-back form posts to + # a relative action, so there is nothing to escape and nothing to smuggle. + assert "{host}" not in page def _import_mcp_routes():