fix(http): fail closed without auth config (adopted from #325) - #334
fix(http): fail closed without auth config (adopted from #325)#334K4bain wants to merge 1 commit into
Conversation
mount_http() previously registered MCP HTTP endpoints with no auth dependencies when no AuthConfig was configured — any unauthenticated client could list and call MCP tools. Register a fail-closed dependency that rejects requests with 401 + WWW-Authenticate: Bearer when auth is not configured. Adopted from tadata-org#325 (original patch by @Daniyal0100101).
|
Thanks for rebasing and carrying this forward on current main. I appreciate the credit and the verification. Please let me know if there is anything else I can help with on this change. |
|
Thanks @Daniyal0100101! One small thing blocking this: the CI workflow run on my branch ended with \�ction_required\ (first-time/external contributor approval gate), so the required checks can't complete. Could you approve the run here — https://github.com/tadata-org/fastapi_mcp/actions/runs/33091255091 ? Once CI is green, this PR should be ready to merge (and it can supersede #325, whose author is happy to have it closed in favor of this one). |
|
I checked this from my side. I only have read access to the repo, so I can’t approve Actions runs from fork PRs. @itay-tadata, could you approve the blocked workflow when you get a chance? I’ve closed #325 now that this PR supersedes it. |
Adopted from #325 (original patch by @Daniyal0100101) — rebased onto current main and verified.
Why this is needed
Today, calling mount_http() on a FastApiMCP instance without an AuthConfig registers the MCP HTTP endpoints with zero auth dependencies. Any unauthenticated client can list and invoke every MCP tool the server exposes — there is no way to accidentally be secure here, it is open by default.
This PR makes the HTTP transport fail closed:
The real-transport test server fixture now passes an explicit permissive AuthConfig so the transport tests keep exercising the authenticated path.
Note: mount_sse() keeps its existing behavior in this PR, faithful to the original patch scope — happy to extend the same fail-closed guard to SSE as a follow-up if maintainers want it.
Verification
Credit to @Daniyal0100101 for the original report and patch in #325.