aminghadersohi opened a new pull request, #44800:
URL: https://github.com/apache/superset/pull/44800
### SUMMARY
An assistant answering questions about a filtered dashboard could report
numbers that ignore that dashboard's filters. For example, with a single-client
filter applied, it could return totals across all clients. There were three
gaps:
1. Only the chart tools received dashboard filters. `query_dataset`,
`get_table` and `execute_sql`, which open-ended questions usually go through,
had no notion of dashboard scope.
2. The filters travelled as a tool argument, so the model could drop them by
passing its own `extra_form_data`, even `{}`.
3. A retried call after a tool error carried no scope.
**Design**
- **Transport.** The client sends the dashboard's active filters in an
`X-Superset-Dashboard-Scope` request header (`base64url(zlib(JSON))`:
`{version, dashboard_id, chart_filters: {chart_id: extra_form_data}}`) on every
MCP request of the turn. The model never sees the header, so it cannot remove
it, and retries carry it again. Without the header nothing changes. A malformed
header refuses every call rather than being ignored.
- **One chokepoint.** `mcp_auth_hook` applies the scope to every protected
tool call after RBAC and dataset routing, and before the tool runs
(`apply_call_dashboard_scope`). `check_tool_permission` is not the right place:
it is a boolean RBAC probe that also backs `tools/list` visibility and never
sees call arguments. The tool-search `call_tool` proxy forwards through
`ctx.fastmcp.call_tool`, which reaches the same wrapper.
- **Request rewriting, AND-composed.** The tool's request is rewritten
before it builds a query, so every internal path, and every cache key derived
from the request, sees the scoped request:
- `get_chart_data`, `get_chart_preview` (data formats),
`get_dashboard_data`: each chart's dashboard filters are AND-ed with the
model's `extra_form_data`. Filter lists are concatenated. Row-constraining
overrides the dashboard sets (`time_range`, time column, relative anchors)
cannot be changed. Charts not on the scoped dashboard are refused.
- `get_chart_info`, `get_chart_sql`: composed the same way when the chart
is on the dashboard, otherwise unchanged (they return no rows).
- `query_dataset`, `get_table` (datasets): the dashboard-wide filters are
AND-ed into the query filters, and the model's time range is intersected with
the dashboard's.
- `execute_sql`: every table the SQL reads gets predicates injected
through the same AST rewrite RLS uses, so the filter applies at the read, not
the output. Predicates are built with the dataset's own column quoting, value
coercion (`filter_values_handler`), NULL-aware IN lists and `get_time_filter`,
then rendered with dialect literal binds, as RLS predicates are.
- Chart authoring tools refuse only data previews
(`ascii`/`table`/`vega_lite`). Tools that return no dataset rows pass through.
**Any other tool, including extension tools, is refused (default deny).** A
test asserts every registered tool is classified.
- **Refuse, never widen.** When the scope cannot be applied, the call fails
with a tool error starting `Dashboard filter scope refused this call:`, stating
that no query was run and what to do instead. Cases include: a column the
dataset lacks; a column the dashboard filters differently for different charts;
SQL reading an unregistered table, or a table without the filtered column (for
example a dimension-table join or a UNION branch); templated SQL; semantic
views; a clause Superset's granularity handling would drop.
- **Caches.** Superset's chart-data and SQL result caches key on the
rewritten query or SQL, so an unscoped entry cannot be hit. FastMCP's response
cache keys only on tool name and arguments, so it is bypassed for calls
carrying the header.
- **Not a security boundary.** RBAC, dataset permissions and RLS run exactly
as before and are neither relaxed nor relied on. `execute_sql` runs the normal
access check before describing any table in a refusal.
No config flag: the header is the opt-in, and coverage is complete by
construction (rewrite, refuse, or pass a tool that reads no rows).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (MCP server behaviour).
### TESTING INSTRUCTIONS
```
pytest tests/unit_tests/mcp_service/test_dashboard_scope.py \
tests/unit_tests/mcp_service/test_dashboard_scope_sql.py
```
- Model-supplied `extra_form_data` (`None`, `{}`, `{"filters": []}`) keeps
the scope. Overrides that would loosen it are refused. Tested at the unit level
and end to end through the MCP client.
- Each covered tool receives the scope: chart tools, `get_dashboard_data`,
`query_dataset` (end to end, the query dict handed to `QueryContextFactory`),
`get_table`, `execute_sql` (end to end, the SQL handed to `Database.execute`).
- Cache: a real `ResponseCachingMiddleware` serves unscoped calls from cache
but never serves or stores scoped ones. Scoped and unscoped query objects have
different cache keys.
- Unmappable paths refuse. The SQL tests run the rewritten SQL against a
real SQLite database and check the numbers: aggregates over filtered rows only,
CTEs, subqueries, UNION branches, a hostile `OR 1=1`, an injection-style value,
NULL-aware IN, and a time range.
- Full `tests/unit_tests/mcp_service` suite passes locally.
Manual check: run the MCP server over streamable HTTP and call
`query_dataset` with and without the header, e.g. `python -c "import
base64,zlib,json;print(base64.urlsafe_b64encode(zlib.compress(json.dumps({'version':1,'dashboard_id':<id>,'chart_filters':{'<chart_id>':{'filters':[{'col':'<col>','op':'IN','val':['<v>']}]}}}).encode())).decode())"`.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
