Skip to content

fix(http): fail closed without auth config (adopted from #325) - #334

Open
K4bain wants to merge 1 commit into
tadata-org:mainfrom
K4bain:fix/http-fail-closed
Open

fix(http): fail closed without auth config (adopted from #325)#334
K4bain wants to merge 1 commit into
tadata-org:mainfrom
K4bain:fix/http-fail-closed

Conversation

@K4bain

@K4bain K4bain commented Aug 27, 2026

Copy link
Copy Markdown

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:

  1. When no AuthConfig is configured, a fail-closed dependency is registered that rejects requests with 401 + WWW-Authenticate: Bearer and a clear message telling the operator to configure AuthConfig.
  2. When an AuthConfig IS configured, behavior is unchanged (its dependencies are used as before).

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

  • New tests: test_http_fails_closed_without_auth_config (route registers auth dependency) and test_http_rejects_unauthenticated_request (unauthenticated POST /mcp gets 401 + Bearer challenge). Both fail on current main and pass with this fix.
  • Full unit suite: 69 passed with no regressions.

Credit to @Daniyal0100101 for the original report and patch in #325.

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).
@Daniyal0100101

Copy link
Copy Markdown

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.

@K4bain

K4bain commented Sep 2, 2026

Copy link
Copy Markdown
Author

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).

Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants