gabotorresruiz opened a new pull request, #43066: URL: https://github.com/apache/superset/pull/43066
### SUMMARY Follow-up to the embedded-guest MCP data-read work (#41753). The max-effort review of that PR fixed the high-severity findings in-line and deferred a batch of lower-severity correctness, robustness, and coverage gaps; this PR clears that batch. Two things up front, because these are intentionally small: - **None is a security bypass.** Guest chart data stays gated by `raise_for_access` on every path. These are about *not returning a raw 500/opaque error* and about internal consistency, not about what a guest can access. - **None changes behavior for non-guest principals.** Every change is either guest-only or a no-op for existing callers. Grouped by area: **Guest dashboard scoping (`utils/filters.py`, `dashboards/filters.py`)** - A dashboard **slug** is a valid guest-token resource id (`Dashboard.get` accepts it), but it was routed to `Dashboard.id.in_([slug])`, which raises a cast error/500 on strict backends. It now routes to `Dashboard.slug`. - The dashboard-list guest filter had its own copy of the scoping logic that used an either/or (uuid *or* int), so a token mixing uuid and int ids silently dropped the int-id dashboards — disagreeing with the chart-list filter. Both now use the shared `guest_embedded_dashboard_filter`, so visibility is consistent and the duplicate branch is gone. **`get_chart_data` / `get_chart_preview` robustness** - `row_limit` can arrive as a string (cached form_data). The no-context path already coerced it; the `using_unsaved_state` and `_query_from_form_data` paths did not, and a string could 500 in `apply_max_row_limit`. Now coerced on all paths. - The `form_data_key`-only path had no guest handling: the unsaved-chart cache is not scoped to a guest's dashboards, so a guest failed opaquely downstream. Guests are now denied cleanly there. - The dataset pre-check was skipped entirely for guests, so a *deleted* dataset surfaced a raw error instead of the clean `DatasetNotAccessible` contract. It now runs for guests with `check_access=False` — existence check kept (clean error), RBAC access check skipped (guests are authorized via the dashboard context, not dataset RBAC). **MCP config / cleanup (`mcp_config.py`, `auth.py`)** - `get_mcp_config` did not forward `MCP_RESTRICTED_TOOL_POLICY`, so a policy set at the `mcp_config` layer was dropped in standalone MCP mode. Now forwarded. - The 7-tool guest allow-list was duplicated as two literals kept in sync by a comment (and a test). `auth.py` now derives it from the single `mcp_config` source; the now-redundant sync test is removed. **Tests** - Added tool-level guest tests for `get_chart_data` (the highest-value guest tool): the data query is pinned to the token's dashboard, the dataset check runs existence-only for guests, an out-of-scope chart is denied, and the `form_data_key`-only path is denied. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (backend/MCP correctness). ### TESTING INSTRUCTIONS - `pytest tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py tests/unit_tests/mcp_service/chart/tool/test_get_chart_preview.py tests/unit_tests/mcp_service/test_guest_token_auth.py tests/unit_tests/mcp_service/test_guest_scope.py` - `pytest tests/unit_tests/utils/filters_test.py tests/unit_tests/dashboards/` - All green locally; `ruff` and `ruff format` clean. ### 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)) - [ ] 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]
