fix(mcp): require MCP_JWT_AUDIENCE when MCP JWT auth is enabled#41292
Conversation
Code Review Agent Run #7d21cfActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41292 +/- ##
==========================================
+ Coverage 55.71% 64.44% +8.72%
==========================================
Files 2655 2655
Lines 145486 145487 +1
Branches 33584 33583 -1
==========================================
+ Hits 81055 93753 +12698
+ Misses 63690 50031 -13659
- Partials 741 1703 +962
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sha174n
left a comment
There was a problem hiding this comment.
Verified: when JWT auth is enabled and the audience is unset, the verifier factory now raises and the bootstrap re-raises before the broad except, so the service fails closed at startup rather than starting with an unset audience. API-key-only deployments are unaffected. Tests cover all branches and UPDATING.md is documented. LGTM.
When MCP JWT auth is enabled (MCP_AUTH_ENABLED=True) but no audience is configured, the verifier was constructed with audience validation skipped, so any otherwise-valid same-issuer token was accepted regardless of which service it was minted for. Require MCP_JWT_AUDIENCE to be set when JWT auth is enabled. The default auth factory now raises a dedicated MCPAuthConfigError, and the server bootstrap re-raises it so the MCP service fails fast at startup with a clear message instead of silently coming up in a permissive (or unauthenticated) state. API-key-only deployments are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0475416 to
375bbb9
Compare
|
Bito Automatic Review Skipped – PR Already Merged |
SUMMARY
When the MCP service has JWT auth enabled (
MCP_AUTH_ENABLED=True), the audience claim was only validated ifMCP_JWT_AUDIENCEhappened to be set. With it unset, the verifier was built with audience validation skipped, so any otherwise-valid token from the same issuer was accepted regardless of which service it was minted for.This makes audience configuration a required precondition for MCP JWT auth so tokens are correctly bound to this service:
create_default_mcp_auth_factory) now raises a dedicatedMCPAuthConfigErrorwhenMCP_AUTH_ENABLEDis true butMCP_JWT_AUDIENCEis unset._create_auth_provider) re-raises that error rather than swallowing it. This matters because a swallowed build error returns aNoneprovider, and the start path treatsauth_provider is Noneas "auth disabled" — i.e. the service would otherwise come up unauthenticated. Failing closed makes the misconfiguration a fast, explicit startup error instead.The check is placed at the config/bootstrap layer so it covers both verifier variants (
MCPJWTVerifierandDetailedJWTVerifier) uniformly.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend configuration behavior.
TESTING INSTRUCTIONS
Unit tests added in
tests/unit_tests/mcp_service/test_mcp_config.pyandtests/unit_tests/mcp_service/test_mcp_server.py:MCP_AUTH_ENABLED=TruewithoutMCP_JWT_AUDIENCEraisesMCPAuthConfigError._create_auth_providerpropagatesMCPAuthConfigErrorinstead of returningNone.Manual: start the MCP service with
MCP_AUTH_ENABLED=TrueandMCP_JWT_AUDIENCEunset → startup fails with a clear message. SetMCP_JWT_AUDIENCE→ starts normally.ADDITIONAL INFORMATION