* fix(mcp): reject malformed Args on Add MCP Server instead of silently defaulting to []
* test(mcp): pass every Form param add_server reads past args validation
CI's pytest run showed test_add_server_still_accepts_valid_json_args and
test_add_server_still_defaults_empty_args_to_empty_list failing with
TypeError: the JSON object must be str, bytes or bytearray, not Form.
Calling the endpoint function directly bypasses FastAPI's dependency
resolution, so an unpassed Form(...) parameter (url, oauth_file,
oauth_config) arrives as the Form marker object itself rather than its
declared default, and add_server's later `if oauth_file:` check reads
that marker as truthy. The malformed-args test never hit this because it
raises before reaching that code. Not a production bug: a real HTTP
request resolves these through FastAPI before add_server ever runs.
* fix(mcp): reject non-list args and surface the new 400 in the Admin panel
o3LL's review on #6215 found two gaps in the args validation this PR adds:
the Admin panel posts to the same /api/mcp/servers endpoint but never
validates Args client-side, so the new 400 falls into the generic failure
branch and shows "Added but connection failed: unknown". Mirror the same
JSON.parse guard settings.js already has.
Also add an isinstance(list) check next to the existing JSON parse, since
valid-but-wrong-shaped JSON (args=5) reaches StdioServerParameters(args=5)
and 500s in the error formatter. Pre-existing on dev, same validation site
this PR already touches.
* fix(admin): surface the server's 400 detail instead of a generic connection-failed message
The Admin add-server handler read needs_oauth/connected/error but never
res.ok, so a request rejected by the isinstance(list) check added for
#6211 (args=5, a valid-JSON-but-non-list value the client-side JSON.parse
guard cannot catch) fell into the same-shape else branch as a successful
add whose connection attempt failed, and the form fields were cleared as
if the server had accepted it.
* 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.
Slice 2o of the route-domain reorganization (#4082/#4071). Moves
mcp_routes.py (697 lines) into routes/mcp/, leaving a backward-compat
sys.modules shim. Pure file reorganization, no behavior change.
The shim uses sys.modules replacement so sys.modules.pop + re-import,
monkeypatch.setattr(mcp_routes, "MCP_OAUTH_DIR", ...), and __file__
introspection in test_security_regressions.py all reach the canonical
module. One source-introspection path string repointed (line 1001).
Canonical module imports only from core/, src/, and stdlib (zero internal
routes/ coupling). Adds tests/test_mcp_routes_shim.py.
Verified: compileall clean; full suite 4804 passed, 3 skipped.
Amir Fathi