codeant-ai-for-open-source[bot] commented on code in PR #43388:
URL: https://github.com/apache/superset/pull/43388#discussion_r3831852220


##########
superset/mcp_service/__main__.py:
##########
@@ -157,8 +157,19 @@ def main() -> None:
                 sys.stderr.write(f"[MCP] Client disconnected: {e}\n")
                 sys.exit(0)
     else:
-        # For other transports, use normal initialization
-        init_fastmcp_server()
+        # For other transports (network listeners), install the same auth
+        # provider as the supported entry point (`superset mcp run` ->
+        # server.run_server()) instead of starting with no verifier at all.
+        # _create_auth_provider fails closed (raises MCPAuthConfigError) when
+        # auth is configured but a verifier could not be built, so letting
+        # that propagate here refuses to start rather than silently running
+        # this transport unauthenticated.
+        from superset.mcp_service.flask_singleton import get_flask_app
+        from superset.mcp_service.server import _create_auth_provider
+
+        flask_app = get_flask_app()
+        auth_provider = _create_auth_provider(flask_app)
+        init_fastmcp_server(auth=auth_provider)

Review Comment:
   **Suggestion:** The network entrypoint initializes FastMCP directly instead 
of using the complete `run_server()` setup, so configured response caching is 
never installed for `python -m superset.mcp_service` network transports. This 
causes the entrypoint to silently ignore `MCP_CACHE_CONFIG`; pass the optional 
caching middleware (and other required server setup) into initialization or 
delegate to the shared server startup path. [incomplete implementation]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Module network transports ignore configured MCP response caching.
   - ⚠️ Redis-backed cache configuration has no effect through this entrypoint.
   - ⚠️ Deployments lose configured response-performance optimizations.
   ```
   </details>
   
   [![Use CodeAnt 
Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/mcp_service/__main__.py
   **Line:** 172:172
   **Comment:**
        *Incomplete Implementation: The network entrypoint initializes FastMCP 
directly instead of using the complete `run_server()` setup, so configured 
response caching is never installed for `python -m superset.mcp_service` 
network transports. This causes the entrypoint to silently ignore 
`MCP_CACHE_CONFIG`; pass the optional caching middleware (and other required 
server setup) into initialization or delegate to the shared server startup path.
   
   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%2F43388&comment_hash=52303745377594f51304b3086645960e2bed03def56476175c969f03d5de826a&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43388&comment_hash=52303745377594f51304b3086645960e2bed03def56476175c969f03d5de826a&reaction=dislike'>👎</a>



##########
superset/mcp_service/middleware.py:
##########
@@ -219,15 +219,23 @@ def _is_user_error(error: Exception) -> bool:
 
 
 def _sanitize_params(params: dict[str, Any]) -> dict[str, Any]:
-    """Remove sensitive fields from params before logging."""
+    """Remove sensitive fields from params before logging.
+
+    Recurses into nested containers so sensitive keys are redacted no matter
+    which wrapper they arrive under (``arguments``, ``request``, etc.).
+    """
     if not isinstance(params, dict):
         return params
     result: dict[str, Any] = {}
     for k, v in params.items():
         if k.lower() in _SENSITIVE_PARAM_KEYS:
             result[k] = "[REDACTED]"
-        elif k == "arguments" and isinstance(v, dict):
+        elif isinstance(v, dict):
             result[k] = _sanitize_params(v)
+        elif isinstance(v, list):
+            result[k] = [
+                _sanitize_params(item) if isinstance(item, dict) else item for 
item in v

Review Comment:
   **Suggestion:** The list handling only sanitizes dictionary elements, so 
nested arrays are copied unchanged. A request such as a list containing another 
list containing a `password`, `token`, or other sensitive key will place the 
secret into the audit payload despite the recursive-sanitization contract. 
Recurse into every container element, not only dictionaries. [security]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Nested-array MCP arguments can leak passwords or tokens.
   - ❌ Sensitive values can enter curated MCP audit logs.
   - ⚠️ Affects logging paths in `LoggingMiddleware` and message auditing.
   ```
   </details>
   
   [![Use CodeAnt 
Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/mcp_service/middleware.py
   **Line:** 235:237
   **Comment:**
        *Security: The list handling only sanitizes dictionary elements, so 
nested arrays are copied unchanged. A request such as a list containing another 
list containing a `password`, `token`, or other sensitive key will place the 
secret into the audit payload despite the recursive-sanitization contract. 
Recurse into every container element, not only dictionaries.
   
   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%2F43388&comment_hash=ffa5cdce184dc4ecebf58c25b951e8afaa3913964b3a114f2b6cf31e44dcb6a3&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43388&comment_hash=ffa5cdce184dc4ecebf58c25b951e8afaa3913964b3a114f2b6cf31e44dcb6a3&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]

Reply via email to