aminghadersohi opened a new pull request, #44146:
URL: https://github.com/apache/superset/pull/44146
### SUMMARY
Three related improvements to how MCP clients discover and are routed to
datasets.
**1. `list_datasets` now searches `description`.**
`search_columns` was `["schema", "sql", "table_name", "uuid"]`, so a query
term
could only ever match a table name, schema, raw SQL or UUID — even though
`description` is returned in the response. The practical failure mode: a
search
for a business term matches a similarly-named summary table while the
relevant
data sits in a differently-named one, and the assistant then states
confidently that the data does not exist.
This was inherited from the dataset REST API's `search_columns` in the
original
MCP implementation, not an intentional exclusion. `description` is already
returned by the tool, so it is not a disclosure change, and the DAO applies
`ilike` over `cast(column, Text)` with wildcard escaping just as it does for
the
existing columns. Cost is one additional OR'd text predicate on the same
scan —
no joins, no extra query — and it is strictly cheaper than the `sql` column
the
same scan already searches. It is a substring match, not an indexed full-text
search, which the docs now say explicitly.
The effect is that writing a good dataset description actually makes the
dataset
discoverable, which today it does not.
**2. Dataset provenance in responses.**
`query_dataset` already returns `dataset_id`/`dataset_name` and `get_table`
already returns `source` plus the corresponding dataset or external-view
identity, so no response schema changed. What was missing was any
instruction to
*use* them: the tool docstrings and server instructions now tell callers to
cite
the returned source identity, to treat search results as candidates rather
than a
relevance ranking, to surface the alternatives when the choice is ambiguous,
and
to state the scope limitation instead of concluding that data does not
exist. A
regression test pins `dataset_name` on the `get_table` built-in path.
**3. Optional per-role dataset allowlist (`MCP_DATASET_ROLE_ALLOWLIST`).**
Deployments that route assistants at a curated set of datasets can map role
names to dataset UUIDs. Effective roles contribute the **union** of their
lists,
which is then **intersected with the caller's existing dataset access** —
lookups
go through the ordinary access-filtered DAO and never `skip_base_filter`, so
an
allowlist entry cannot surface a dataset the caller could not already reach.
**This only narrows and is not a security boundary.** Dataset permissions,
query
validation and row-level security remain the actual boundary and are
unchanged
and undiminished; this controls routing and usability. `None` (the default)
disables it entirely and preserves the full tool surface; `{}` allows
nothing;
malformed config raises rather than silently disabling the restriction. Admin
gets no exemption.
In scoped mode, discovery filters before counting and pagination, and only
tools
that can be attributed to a registered dataset UUID are available. Paths
that can
read data without such an identity — SQL Lab, chart data and previews, cached
results, external semantic sources, mutations, extension tools — refuse
rather
than guess at SQL lineage. An out-of-scope request returns an explicit
refusal;
it never substitutes an allowed-but-different dataset, since silently
answering
from the wrong dataset is the exact failure this is meant to prevent.
Two follow-up fixes in the second commit: `MCP_DATASET_ROLE_ALLOWLIST` was
declared in `mcp_config.py` but omitted from `get_mcp_config()`'s defaults,
so
the standalone MCP app never carried it through its config overlay; and the
auth
hook bound every tool call's signature before consulting the scope, making
all
callers pay for — and be able to fail on — argument binding even where the
feature is off.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — server-side behavior only.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/mcp_service/test_dataset_scope.py \
tests/unit_tests/mcp_service/dataset \
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py
```
Manual, description search:
1. Give a dataset a description containing a term that appears nowhere in its
table name, schema or SQL.
2. Call `list_datasets` with that term as `search`. The dataset is returned.
Manual, routing scope:
1. In `superset_config.py`, set
`MCP_DATASET_ROLE_ALLOWLIST = {"<a role you hold>": ["<a dataset uuid>"]}`
(UUIDs come from `get_dataset_info`, or request `uuid` in
`list_datasets.select_columns`).
2. `list_datasets` returns only that dataset; counts and pagination reflect
the
filter.
3. `query_dataset` against a different dataset refuses without executing a
query.
4. `execute_sql` refuses with the same explanation.
5. Remove the setting (or set it to `None`) and confirm the full tool
surface and
unfiltered discovery return.
Note:
`tests/unit_tests/mcp_service/test_mcp_e2e_smoke.py::test_tools_call_health_check_over_real_asgi_transport`
fails on `master` independently of this branch.
### 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]
