codeant-ai-for-open-source[bot] commented on code in PR #44800: URL: https://github.com/apache/superset/pull/44800#discussion_r4141144864
########## superset/mcp_service/dashboard_scope.py: ########## @@ -0,0 +1,1228 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Dashboard filter scope for MCP tool calls. + +A client answering questions about a dashboard the user is looking at (an AI +assistant embedded next to it, for example) sends that dashboard's active +filters in the ``X-Superset-Dashboard-Scope`` request header. The header rides +every HTTP request of the conversation turn, so the model driving the tool +calls can neither see nor remove it, and a retried call carries it again. + +Scope is a *correctness* control: answers must reflect the filtered view the +user sees. It is not an authorization boundary. RBAC, dataset permissions and +row-level security are applied by the normal execution paths exactly as +without a scope, and nothing here relaxes or replaces them. + +Enforcement happens once per tool call in ``mcp_auth_hook`` (see +``apply_call_dashboard_scope``), before the tool runs. Every tool is in exactly +one class while a scope is active: + +* rewritten: the tool's request is rewritten so the scope is AND-composed with + whatever the model supplied. The rewrite happens before the tool builds a + query, so every internal path and every cache key sees the scoped request. +* gated: allowed unless the particular call would return data the scope cannot + be applied to, in which case it is refused. +* neutral: returns no dataset rows (metadata, links, mutations), unchanged. +* anything else, including extension tools and tools added later: refused. + +A refusal is always explicit. Scope that cannot be applied is never dropped. +""" + +from __future__ import annotations + +import base64 +import binascii +import logging +import zlib +from collections.abc import Callable, Mapping, Sequence +from dataclasses import dataclass +from datetime import datetime +from typing import Any, TYPE_CHECKING, TypeGuard + +from fastmcp.exceptions import ToolError + +from superset.constants import ( + EXTRA_FORM_DATA_APPEND_KEYS, + EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS, + NO_TIME_RANGE, +) +from superset.utils import json + +if TYPE_CHECKING: + import inspect + +logger = logging.getLogger(__name__) + +HEADER_NAME = "X-Superset-Dashboard-Scope" +SCOPE_VERSION = 1 +# The header is compressed by the sender; these bound what a request can make +# the server allocate before any scope logic runs. +MAX_HEADER_LENGTH = 64 * 1024 +MAX_PAYLOAD_BYTES = 1024 * 1024 + +REFUSAL_PREFIX = "Dashboard filter scope refused this call:" +_NO_QUERY = "No query was run." + +# extra_form_data keys whose value decides which rows a query includes. A model +# may repeat the scope's value but never replace it. +ROW_OVERRIDE_KEYS = frozenset( + {"time_range", "granularity_sqla", "time_column", "relative_start", "relative_end"} +) +# Keys that choose the column a time range applies to. Setting one while the +# scope carries filters would move the dashboard window to another column, and +# Superset's granularity handling drops existing filters on the chosen column. +TIME_TARGET_KEYS = frozenset({"granularity_sqla", "time_column"}) +# Keys that only change presentation (bucketing, interactivity), never which +# rows are included. The model's value wins. +PRESENTATION_KEYS = frozenset( + set(EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS) - ROW_OVERRIDE_KEYS +) | (set(EXTRA_FORM_DATA_APPEND_KEYS) - {"filters", "adhoc_filters"}) + +# Filter operators the dataset and SQL paths know how to apply. Anything else +# a dashboard sends is refused on those paths rather than guessed at. +SUPPORTED_OPERATORS = frozenset( + { + "==", + "!=", + ">", + "<", + ">=", + "<=", + "IN", + "NOT IN", + "IS NULL", + "IS NOT NULL", + "LIKE", + "ILIKE", + } +) +TEMPORAL_RANGE = "TEMPORAL_RANGE" + +# Preview formats that carry query results rather than a link. +DATA_PREVIEW_FORMATS = frozenset({"ascii", "table", "vega_lite"}) + + +class MCPDashboardScopeError(ToolError): + """A tool call refused because the dashboard scope cannot be applied to it. + + Subclasses ``ToolError`` so the message reaches the caller verbatim and the + model can explain the limitation instead of retrying blindly. + """ + + def __init__(self, reason: str, guidance: str = "") -> None: + message = f"{REFUSAL_PREFIX} {reason} {_NO_QUERY}" + if guidance: + message = f"{message} {guidance}" + super().__init__(message) + + +_USE_CHART_TOOLS = ( + "Use get_chart_data or get_dashboard_data on the dashboard's charts, which " + "apply the filters each chart shows, or ask the user to change the " + "dashboard filters." +) +_ASK_USER = ( + "Explain that the active dashboard filters cannot be applied to this " + "request, and do not present unfiltered results as the filtered answer." +) + + +@dataclass(frozen=True) +class DashboardScope: + """The active filters of the dashboard a request is scoped to.""" + + dashboard_id: int + # Chart id -> the extra_form_data the dashboard applies to that chart. + # Charts on the dashboard without an entry have no active filters. + chart_filters: Mapping[int, Mapping[str, Any]] + + @property + def has_constraints(self) -> bool: + """True when any chart's filters restrict which rows are included.""" + return any(_restricts_rows(efd) for efd in self.chart_filters.values()) + + +@dataclass(frozen=True) +class DashboardConstraints: + """Dashboard-wide constraints for queries not tied to one dashboard chart. + + ``clauses`` are ``{"col", "op", "val"}`` dicts AND-ed into the query. + ``time_range`` is the dashboard time window; ``time_column`` names the + column it applies to, or None for the queried dataset's main datetime + column. + """ + + clauses: tuple[dict[str, Any], ...] + time_range: str | None + time_column: str | None + + @property + def is_empty(self) -> bool: + return not self.clauses and self.time_range is None + + @property + def columns(self) -> set[str]: + return {clause["col"] for clause in self.clauses} + + +def _restricts_rows(extra_form_data: Mapping[str, Any]) -> bool: + return bool( + extra_form_data.get("filters") + or extra_form_data.get("adhoc_filters") + or _time_range(extra_form_data) is not None + ) + + +def _time_range(extra_form_data: Mapping[str, Any]) -> str | None: + value = extra_form_data.get("time_range") + if value is None or value == NO_TIME_RANGE: + return None + return value + + +# --------------------------------------------------------------------------- +# Header decoding +# --------------------------------------------------------------------------- + + +def _malformed(detail: str) -> MCPDashboardScopeError: + return MCPDashboardScopeError( + f"the {HEADER_NAME} request header is malformed ({detail}).", + "This is a client integration error; the request cannot be answered " + "until the client sends a valid dashboard scope.", + ) + + +def decode_dashboard_scope(value: str) -> DashboardScope: + """Decode ``base64url(zlib(JSON))`` into a validated ``DashboardScope``. + + Any deviation is an error rather than "no scope": a caller that sent a + scope expects it applied, so a garbled one must not silently widen answers. + """ + if len(value) > MAX_HEADER_LENGTH: + raise _malformed("too large") + try: + compressed = base64.b64decode( + value.strip() + "=" * (-len(value.strip()) % 4), + altchars=b"-_", + validate=True, + ) + except (binascii.Error, ValueError) as ex: + raise _malformed("not base64url") from ex + + decompressor = zlib.decompressobj() + try: + raw = decompressor.decompress(compressed, MAX_PAYLOAD_BYTES + 1) + except zlib.error as ex: + raise _malformed("not zlib-compressed") from ex + if len(raw) > MAX_PAYLOAD_BYTES or decompressor.unconsumed_tail: + raise _malformed("payload too large") + if not decompressor.eof: + raise _malformed("truncated payload") + + try: + payload = json.loads(raw.decode("utf-8")) + except (UnicodeDecodeError, ValueError) as ex: + raise _malformed("not JSON") from ex + return _parse_payload(payload) + + +def _parse_payload(payload: Any) -> DashboardScope: + if not isinstance(payload, dict): + raise _malformed("payload must be a JSON object") + if payload.get("version") != SCOPE_VERSION: + raise _malformed(f"unsupported version {payload.get('version')!r}") + dashboard_id = payload.get("dashboard_id") + if not _is_positive_int(dashboard_id): + raise _malformed("dashboard_id must be a positive integer") + return DashboardScope( + dashboard_id=dashboard_id, + chart_filters=_parse_chart_filters(payload.get("chart_filters")), + ) + + +def _parse_chart_filters(raw_filters: Any) -> dict[int, Mapping[str, Any]]: + if raw_filters is None: + return {} + if not isinstance(raw_filters, dict): + raise _malformed("chart_filters must be an object") + chart_filters: dict[int, Mapping[str, Any]] = {} + for key, extra_form_data in raw_filters.items(): + chart_id = _chart_id(key) + if chart_id is None: + raise _malformed(f"chart id {key!r} is not a positive integer") + if not isinstance(extra_form_data, dict): + raise _malformed(f"filters for chart {chart_id} must be an object") + for list_key in ("filters", "adhoc_filters"): + if not isinstance(extra_form_data.get(list_key) or [], list): + raise _malformed(f"{list_key} for chart {chart_id} must be a list") Review Comment: ✅ **CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved** as of `acb10cb`. Added explicit list-or-null type validation for `filters` and `adhoc_filters`, so falsy non-list values now raise a malformed-scope error instead of being treated as empty. <sub>If that's not right, unresolve this thread and CodeAnt will leave it open.</sub> <!-- codeant-auto-resolve-reply --> -- 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]
