Copilot commented on code in PR #40471:
URL: https://github.com/apache/superset/pull/40471#discussion_r3312732704
##########
superset/mcp_service/jwt_verifier.py:
##########
@@ -61,21 +64,243 @@
"_jwt_failure_reason", default=None
)
+_HTML_STYLES = """
+ *, *::before, *::after { box-sizing: border-box; }
+ body {
+ font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto,
sans-serif;
+ background: #f5f5f5;
+ color: #1a1a1a;
+ margin: 0;
+ padding: 40px 16px;
+ line-height: 1.6;
+ }
+ .card {
+ max-width: 640px;
+ margin: 0 auto;
+ background: #ffffff;
+ border-radius: 8px;
+ box-shadow: 0 1px 4px rgba(0,0,0,.12);
+ padding: 40px 40px 32px;
+ }
+ h1 { font-size: 1.4rem; margin: 0 0 8px; }
+ .badge {
+ display: inline-block;
+ background: #e8f4fd;
+ color: #0070c0;
+ font-size: .75rem;
+ font-weight: 600;
+ padding: 2px 8px;
+ border-radius: 4px;
+ margin-bottom: 20px;
+ letter-spacing: .04em;
+ text-transform: uppercase;
+ }
+ p { margin: 0 0 20px; color: #444; }
+ h2 { font-size: 1rem; margin: 24px 0 8px; color: #1a1a1a; }
+ pre {
+ background: #f0f0f0;
+ border-radius: 6px;
+ padding: 16px;
+ font-size: .85rem;
+ overflow-x: auto;
+ margin: 0 0 24px;
+ }
+ code {
+ font-family: "SFMono-Regular", Consolas, "Liberation Mono", Menlo,
monospace;
+ }
+ .note {
+ font-size: .85rem;
+ color: #666;
+ border-left: 3px solid #ddd;
+ padding-left: 12px;
+ margin-top: 24px;
+ }
+ .logo {
+ max-height: 48px; max-width: 200px; margin-bottom: 20px; display: block;
+ }"""
+
+_DEFAULT_CLIENTS = [
+ "Claude Desktop",
+ "Claude Code (CLI)",
+ "Cursor",
+]
+
+_DEFAULT_HELLO_PAGE_CONFIG: dict[str, Any] = {
+ # Page heading and browser tab title
+ "title": "Superset MCP Server",
+ # Key name used in the mcpServers config snippet (e.g. "superset",
"my-company")
+ "server_key": "superset",
+ # Include "transport": "streamable-http" in the config snippet.
+ # Recommended: Claude Desktop defaults to SSE so the transport must be
explicit.
+ "show_transport": True,
+ # Supported MCP clients listed on the page
+ "clients": _DEFAULT_CLIENTS,
+}
+
+
+def _build_config_snippet(
+ auth_enabled: bool, server_key: str, show_transport: bool
+) -> str:
+ # superset.utils.json.dumps() ensures the key is a valid JSON string.
+ from superset.utils import json as superset_json
+
+ key_json = superset_json.dumps(server_key)
+ inner_parts = [' "url": "<this-url>"']
+ if show_transport:
+ inner_parts.append(' "transport": "streamable-http"')
+ if auth_enabled:
+ inner_parts.append(
+ ' "headers": {\n'
+ ' "Authorization": "Bearer <your-api-key>"\n'
+ " }"
+ )
+ inner = ",\n".join(inner_parts)
+ return f'{{\n "mcpServers": {{\n {key_json}: {{\n{inner}\n }}\n
}}\n}}'
+
+
+def _build_browser_hello_html(
+ auth_enabled: bool,
+ page_config: dict[str, Any] | None = None,
+) -> str:
+ cfg = {**_DEFAULT_HELLO_PAGE_CONFIG, **(page_config or {})}
+ title: str = html_module.escape(str(cfg["title"]))
+ server_key: str = cfg["server_key"]
+ show_transport: bool = cfg["show_transport"]
+ clients: list[str] = [html_module.escape(str(c)) for c in cfg["clients"]]
+ app_name: str = html_module.escape(str(cfg.get("app_name", "Apache
Superset")))
+ logo_url: str | None = None
+ if logo_url_raw := cfg.get("logo_url"):
+ logo_url_stripped = str(logo_url_raw).strip()
+ if logo_url_stripped.startswith(("http://";, "https://";)):
+ logo_url = html_module.escape(logo_url_stripped)
+
+ config_block = _build_config_snippet(auth_enabled, server_key,
show_transport)
+
Review Comment:
The hello page injects `config_block` into HTML without escaping. Since
`server_key` is config-driven and is interpolated via `json.dumps()` (which
does not HTML-escape `<`/`&`), a malicious/accidental value could break out of
the `<code>` block and inject HTML/JS. Safer approach: build the JSON snippet
with raw placeholders and then `html.escape()` the *entire* snippet right
before embedding it in HTML.
##########
superset/mcp_service/server.py:
##########
@@ -717,6 +718,47 @@ def build_middleware_list() -> list[Middleware]:
]
+def _build_starlette_middleware(
+ flask_app: Any | None = None, auth_provider: Any | None = None
+) -> list[Any]:
+ from starlette.middleware import Middleware as StarletteMiddleware
+
+ if flask_app is None:
+ from superset.mcp_service.flask_singleton import get_flask_app
+
+ flask_app = get_flask_app()
+ # Auth is active only when an instantiated provider was passed in.
+ # Config-flag presence is not sufficient — MCP_AUTH_FACTORY may return
+ # None, and use_factory_config auth lives outside Flask config entirely.
+ auth_enabled = auth_provider is not None
+ app_name: str = flask_app.config.get("APP_NAME", "Superset")
+ app_icon: str = flask_app.config.get("APP_ICON", "")
+ base_page_config: dict[str, Any] = {
+ "title": f"{app_name} MCP Server",
+ "server_key": app_name.lower().replace(" ", "-"),
+ "app_name": app_name,
+ }
+ if app_icon:
+ if app_icon.startswith(("http://";, "https://";)):
+ base_page_config["logo_url"] = app_icon
+ elif app_icon.startswith("/"):
+ # Relative path — combine with Superset webserver address if
configured
+ superset_addr = flask_app.config.get(
+ "SUPERSET_WEBSERVER_ADDRESS", ""
+ ).rstrip("/")
+ if superset_addr:
+ base_page_config["logo_url"] = f"{superset_addr}{app_icon}"
+ mcp_hello_page: dict[str, Any] | None =
flask_app.config.get("MCP_HELLO_PAGE", None)
+ page_config: dict[str, Any] = {**base_page_config, **(mcp_hello_page or
{})}
+ return [
Review Comment:
`MCP_HELLO_PAGE` is assumed to be a dict, but `flask_app.config.get()` can
return any type. If it’s set to a non-mapping (e.g. a string from env parsing),
`{**base_page_config, **(mcp_hello_page or {})}` will raise `TypeError` and
prevent the MCP server from starting. Consider validating the type (and
optionally logging a warning) before merging.
--
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]
