codeant-ai-for-open-source[bot] commented on code in PR #40869:
URL: https://github.com/apache/superset/pull/40869#discussion_r3375851419
##########
superset/mcp_service/jwt_verifier.py:
##########
@@ -447,6 +448,14 @@ async def load_access_token(self, token: str) ->
AccessToken | None: # noqa: C9
# Step 2: Get verification key (static or JWKS)
try:
verification_key = await self._get_verification_key(token)
+ except (httpx.HTTPError, OSError, TimeoutError) as e:
+ # Transient failure reaching or reading the JWKS endpoint.
+ # Treat it as an authentication failure (return None) instead
of
+ # letting the network error propagate as an unexpected
exception.
+ reason = "JWKS verification key unavailable"
+ _jwt_failure_reason.set(reason)
Review Comment:
**Suggestion:** This warning log includes raw exception text from
network/JWKS failures, which can leak endpoint details or untrusted message
content at WARNING level and breaks the module's "generic categories only at
WARNING" contract. Log only a generic category at WARNING and move sanitized
exception details to DEBUG. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ MCP JWT auth logs raw JWKS/network error text.
- ⚠️ Violates documented WARNING-vs-DEBUG logging contract.
- ⚠️ May disclose internal JWKS endpoints in auth logs.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Enable the MCP JWT auth path with debug logging so that
`DetailedJWTVerifier` is used:
`superset.mcp_service.server._create_auth_provider()` at `server.py:6-43`
calls
`create_default_mcp_auth_factory(flask_app)` in `mcp_config.py:344-362`,
which in turn
calls `_build_jwt_verifier()` at `mcp_config.py:98-119`. When
`MCP_AUTH_ENABLED=True`,
`MCP_JWKS_URI` is set, and `MCP_JWT_DEBUG_ERRORS=True`,
`_build_jwt_verifier()` returns a
`DetailedJWTVerifier` instance (see `mcp_config.py:105-117` and comment at
`mcp_config.py:33-40`).
2. Start the MCP server using `run_server()` in
`superset/mcp_service/server.py:35-56`. In
the default configuration branch (`server.py:77-99`), this obtains
`auth_provider =
_create_auth_provider(flask_app)` (`server.py:82-84`) and passes it to
`init_fastmcp_server(auth=auth_provider, ...)` (`server.py:95-98`), wiring
the
`DetailedJWTVerifier` (or a `CompositeTokenVerifier` wrapping it) into the
FastMCP
authentication pipeline for all MCP HTTP requests.
3. Configure JWT verification to use JWKS (set `MCP_JWKS_URI`) so that key
retrieval
happens over the network (`_build_jwt_verifier()` assigns `jwks_uri` in
`mcp_config.py:118-120`). Then cause the JWKS endpoint to be unreachable
(e.g., DNS or
connection failure). In the codebase this scenario is concretely exercised by
`test_jwks_network_error_is_handled` at
`tests/unit_tests/mcp_service/test_jwt_verifier.py:96-113`, which patches
`hs256_verifier._get_verification_key` to raise
`httpx.ConnectError("connection
refused")`, modeling a real network failure.
4. With that configuration, send any MCP HTTP request that includes an
`Authorization:
Bearer <token>` header so authentication runs. FastMCP's auth stack invokes
`DetailedJWTVerifier.load_access_token()` (`jwt_verifier.py:18-64`). In Step
2 of that
method (`jwt_verifier.py:49-64`), `verification_key = await
self._get_verification_key(token)` (`line 51`) raises an `httpx.HTTPError`
(or
`OSError`/`TimeoutError`), which is caught by the new `except
(httpx.HTTPError, OSError,
TimeoutError) as e:` block (`jwt_verifier.py:52-58`). That block sets the
generic failure
reason `"JWKS verification key unavailable"` in `_jwt_failure_reason` and
then executes
`logger.warning("Could not fetch JWKS verification key: %s", e)` at
`jwt_verifier.py:58`
(diff line 456). Because this WARNING-level log interpolates the raw
exception object `e`
without using `_sanitize_for_log` and without constraining it to the generic
reason
string, the log can include specific JWKS URL/hostnames or low-level network
error text.
This conflicts with the module's documented logging contract that WARNING
should carry
only generic failure categories (see `DetailedJWTVerifier` docstring at
`jwt_verifier.py:5-15` and the `_jwt_failure_reason` comment at
`jwt_verifier.py:62-67`),
where detailed exception text is supposed to be reserved for DEBUG-level
logging.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3b218f63c9c44b4485f579761193b88a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=3b218f63c9c44b4485f579761193b88a&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:** 456:456
**Comment:**
*Security: This warning log includes raw exception text from
network/JWKS failures, which can leak endpoint details or untrusted message
content at WARNING level and breaks the module's "generic categories only at
WARNING" contract. Log only a generic category at WARNING and move sanitized
exception details to DEBUG.
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%2F40869&comment_hash=fa0ddd994b98063815e00a6f76dfd7c4d826d0ae8b6c4bc656ad5ea8a8399b4a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40869&comment_hash=fa0ddd994b98063815e00a6f76dfd7c4d826d0ae8b6c4bc656ad5ea8a8399b4a&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]
