codeant-ai-for-open-source[bot] commented on code in PR #39645:
URL: https://github.com/apache/superset/pull/39645#discussion_r3465756393
##########
superset/mcp_service/jwt_verifier.py:
##########
@@ -703,10 +698,10 @@ async def load_access_token(self, token: str) ->
AccessToken | None: # noqa: C9
claims=dict(claims),
)
- except (ValueError, JoseError, KeyError, AttributeError, TypeError) as
e:
+ except (ValueError, JoseError, KeyError, AttributeError, TypeError):
Review Comment:
**Suggestion:** The top-level fallback handler now catches only a narrow
exception set and omits `OverflowError`. A JWT with a numerically valid but
non-finite `exp` (for example `1e309`, which decodes to `inf`) will pass the
`exp < time.time()` check and then raise `OverflowError` at `int(exp)`, causing
an unhandled 500 instead of a clean auth rejection. Include `OverflowError` in
this fail-closed handler (or validate `exp` as a finite integer before
casting). [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ MCP JWT auth can crash on oversized exp claims.
- ⚠️ Embedded guest token issuance may intermittently return HTTP 500.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure and start the MCP server so that JWT auth is enabled:
`run_server()` in
`superset/mcp_service/server.py:22-86` calls `_create_auth_provider()` which
in turn calls
`create_default_mcp_auth_factory()` in
`superset/mcp_service/mcp_config.py:344-48`. When
`MCP_AUTH_ENABLED=True` and keys are configured, `_build_jwt_verifier()` in
`mcp_config.py:84-117` constructs a `DetailedJWTVerifier` (when
`MCP_JWT_DEBUG_ERRORS=True`) whose `get_middleware()` in
`superset/mcp_service/jwt_verifier.py:162-170` installs a
`BearerAuthBackend`/`AuthenticationMiddleware` around all MCP HTTP endpoints.
2. Craft a signed JWT whose payload contains a syntactically valid but
extremely large
numeric `exp` value, e.g. `"exp": 1e309`, along with otherwise valid claims
(`iss`, `aud`,
`sub`) that satisfy the checks in `DetailedJWTVerifier.load_access_token()`
in
`superset/mcp_service/jwt_verifier.py:221-262`. When decoded by Authlib's
`jwt.decode()`,
this `exp` becomes `float("inf")` in the `claims` dict.
3. Call any MCP HTTP endpoint exposed by `mcp_instance.http_app(...)`
(created in
`run_server()` at `server.py:82-85`) with an `Authorization: Bearer
<crafted-token>`
header so that Starlette's `AuthenticationMiddleware` invokes
`BearerAuthBackend`, which
in turn calls `DetailedJWTVerifier.load_access_token(token)` (see the
comment at
`jwt_verifier.py:98-101` describing this flow).
4. Inside `DetailedJWTVerifier.load_access_token()`
(`jwt_verifier.py:221-262`), the code
reads `exp = claims.get("exp")` at global line ~591, then checks `if exp is
None:` and `if
exp < time.time():` at ~602; `float("inf")` passes both checks. On
successful validation,
the function reaches the return at ~693 and evaluates `expires_at = int(exp)`
(`jwt_verifier.py:697`). With `exp == float("inf")`, Python raises
`OverflowError: cannot
convert float infinity to integer`, which is *not* included in the enclosing
`except
(ValueError, JoseError, KeyError, AttributeError, TypeError):` block at
`jwt_verifier.py:701`, so the exception propagates out of
`load_access_token`, causing the
authentication middleware to raise an unhandled error and return a 500
instead of a clean
401-style auth failure.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c54ced5d376547f2a9175790b576c448&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c54ced5d376547f2a9175790b576c448&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/jwt_verifier.py
**Line:** 701:701
**Comment:**
*Possible Bug: The top-level fallback handler now catches only a narrow
exception set and omits `OverflowError`. A JWT with a numerically valid but
non-finite `exp` (for example `1e309`, which decodes to `inf`) will pass the
`exp < time.time()` check and then raise `OverflowError` at `int(exp)`, causing
an unhandled 500 instead of a clean auth rejection. Include `OverflowError` in
this fail-closed handler (or validate `exp` as a finite integer before casting).
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39645&comment_hash=1a59811e94b267f391faaf42c41d0469bd1ab97a8d946ba6b0b3eb5cae06fee4&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39645&comment_hash=1a59811e94b267f391faaf42c41d0469bd1ab97a8d946ba6b0b3eb5cae06fee4&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
