Fix kai-api 421 behind ingress: pin fastmcp <3 and build image from lockfile (#934)#935
Conversation
…ockfile (konveyor#934) The MCP solution server began returning HTTP 421 Misdirected Request for every request routed through an nginx ingress (breaking editor-extensions @requires-minikube CI) after the konveyor#932 merge triggered a `latest` image rebuild. Root cause is not the dependency bump itself but an unbounded fastmcp constraint combined with an image build that ignores the pins: - tools/deploy/Containerfile installed deps via `pip install .`, resolving fresh from pyproject.toml's unbounded `fastmcp>=2.8.0`. The pinned requirements.txt/uv.lock were never even copied into the image. - The rebuild floated fastmcp to 3.x, whose mcp SDK enables DNS-rebinding protection by default and rejects any request whose Host/Origin header does not match the bind host (0.0.0.0) with 421. - On the pinned fastmcp 2.14.3 / mcp 1.25.0, TransportSecurityMiddleware receives settings=None and leaves that protection off, which is why the repo's own tests (stdio, or HTTP to localhost) never reproduced it. Fixes: - Bound `fastmcp>=2.8.0,<3` in pyproject.toml (uv.lock regenerated) so an image rebuild can no longer jump a major version incidentally. - Build the image from the pinned requirements.txt, then install the project with --no-deps, so the deployed image uses exactly the versions CI tests. - Document at the FastMCP construction site why DNS-rebinding protection must stay disabled for ingress deployments, and how to configure it explicitly if fastmcp is ever bumped to 3.x. - Add a regression test that drives the streamable-http transport with a mismatched Host/Origin header and asserts the response is not 421. Signed-off-by: Fabian von Feilitzsch <fabian@fabianism.us> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR fixes an issue where fastmcp 3.x's default DNS-rebinding host protection caused HTTP 421 responses behind proxies. It pins fastmcp to <3, documents the rationale in server.py, updates the Containerfile to install from pinned requirements, and adds a regression test for mismatched Host headers. Changesfastmcp version pin and 421 fix
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Test as TestHostHeaderValidation
participant Server as MCP Server (subprocess)
Test->>Server: launch via subprocess.Popen (--host/--port)
Test->>Server: POST initialize (matching Host)
Server-->>Test: 200 OK (ready)
Test->>Server: POST initialize (mismatched Host/Origin)
Server-->>Test: response (status asserted != 421, == 200)
Test->>Server: terminate process (tearDown)
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
kai_mcp_solution_server/tests/test_host_header.py (1)
95-122: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReadiness check accepts any HTTP response as "ready".
_wait_until_readyreturns as soon ashttpx.postdoesn't raise, without checking the status code. A 500 or other error response would be treated as "ready", leading to a confusing assertion failure in the test itself rather than a clear "server not ready" message. Consider checking for a 200 status in the readiness probe.♻️ Proposed refactor for readiness check
try: - httpx.post(url, json=body, headers=headers, timeout=2.0) - return + resp = httpx.post(url, json=body, headers=headers, timeout=2.0) + if resp.status_code == 200: + return except httpx.HTTPError: time.sleep(0.5)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kai_mcp_solution_server/tests/test_host_header.py` around lines 95 - 122, The readiness probe in _wait_until_ready currently treats any successful httpx.post call as ready even if the server returns an error status. Update the probe to verify the response indicates success, specifically requiring a 200 OK before returning, and treat non-2xx responses as not ready so the loop keeps polling until the server is actually healthy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@kai_mcp_solution_server/tests/test_host_header.py`:
- Around line 95-122: The readiness probe in _wait_until_ready currently treats
any successful httpx.post call as ready even if the server returns an error
status. Update the probe to verify the response indicates success, specifically
requiring a 200 OK before returning, and treat non-2xx responses as not ready so
the loop keeps polling until the server is actually healthy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6711c130-cf3a-4daa-a6a1-42bda005f647
⛔ Files ignored due to path filters (1)
kai_mcp_solution_server/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
kai_mcp_solution_server/pyproject.tomlkai_mcp_solution_server/src/kai_mcp_solution_server/server.pykai_mcp_solution_server/tests/test_host_header.pykai_mcp_solution_server/tools/deploy/Containerfile
…ssion Every existing test runs the server from source against the pinned lockfile, so none of them exercised the deployed container image — which resolves dependencies at build time and is where issue konveyor#934 actually manifested. Add a workflow that builds tools/deploy/Containerfile, asserts the image resolved a 2.x fastmcp (a 3.x jump re-enables DNS-rebinding protection by default), starts the container on streamable-http, and POSTs an `initialize` request with a mismatched Host/Origin header (the nginx-ingress case from the issue). The build must not return HTTP 421. This would have caught the original regression, which the source-level tests could not. Signed-off-by: Fabian von Feilitzsch <fabian@fabianism.us> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Summary
Fixes #934. The
kai-apiMCP solution server started returning HTTP 421 Misdirected Request for every request routed through an nginx ingress (breaking alleditor-extensions@requires-minikubeCI) after the #932 merge triggered alatestimage rebuild.The dependency bump in #932 is not the cause. The real cause is a latent packaging bug that the rebuild exposed:
tools/deploy/Containerfileinstalled dependencies withpip install ., resolving fresh frompyproject.toml's unboundedfastmcp>=2.8.0. The pinnedrequirements.txt/uv.lockwere never even copied into the image.mcpSDK enables DNS-rebinding protection by default and rejects any request whoseHost/Originheader doesn't match the bind host (0.0.0.0) with 421.TransportSecurityMiddlewarereceivessettings=Noneand leaves that protection off (per the SDK's backwards-compat default) — which is exactly why this repo's own tests never reproduced it: they use stdio, or connect HTTP directly tolocalhost(matching Host), and CI installs from the lock while the image resolved fresh.Changes
fastmcp>=2.8.0,<3inpyproject.toml(uv.lockregenerated) so an image rebuild can no longer jump a major version incidentally.requirements.txt, then install the project with--no-deps, so the deployed image uses exactly the versions CI tests.FastMCP(...)construction site why DNS-rebinding protection must stay disabled for ingress deployments, and how to configure it explicitly if fastmcp is ever bumped to 3.x.tests/test_host_header.py) that drives the streamable-http transport with a mismatchedHost: 192.168.49.2/Originheader (mirroring the Unbounded fastmcp constraint: image rebuilds pick up FastMCP 3.4.3 host-origin protection, kai-api returns 421 for all proxied requests #934 repro) and asserts the response is not 421.Verification
7 passed, 5 skipped, including the new regression test.192.168.49.2,evil.example.com) → 200, not 421.pip install --no-deps .builds the package via theuv_buildbackend.requirements.txtis unchanged (adding<3doesn't alter resolution), soverify-requirements-txtstays green.Note:
release-0.9produces the same broken image and needs the same fix; will backport after this merges.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Chores