aminghadersohi opened a new pull request, #38521:
URL: https://github.com/apache/superset/pull/38521
### SUMMARY
MCP chart data tools called `ChartDataCommand.run()` without first calling
`ChartDataCommand.validate()`. The REST API (`charts/data/api.py`) correctly
calls `command.validate()` before `command.run()` in all its code paths.
`command.validate()` calls `QueryContext.raise_for_access()` →
`QueryContextProcessor.raise_for_access()` →
`SecurityManager.raise_for_access()`, which enforces:
- **Row-Level Security (RLS) filters**
- **Guest user payload modification guard** (`query_context_modified`)
- **Dashboard-scoped datasource access**
- **Schema/catalog-level permissions**
Without `validate()`, the MCP tools only relied on
`validate_chart_dataset()` — a weaker dataset-level-only check.
**Changes:**
- Added `command.validate()` before `command.run()` in all 6 locations that
use `ChartDataCommand`:
- `get_chart_data.py` — main chart data retrieval
- `get_chart_preview.py` — ASCII, table, and Vega-Lite preview strategies
(3 locations)
- `preview_utils.py` — preview generation from form data
- `generate_chart.py` — compile check during chart creation
- Existing `except (CommandException, SupersetException, ...)` handlers
already cover `SupersetSecurityException`
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — no UI changes
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/mcp_service/chart/ -x -q` — all 314 tests pass
- New tests in `TestChartDataCommandValidation` verify:
- `validate()` is called before `run()` (call ordering assertion)
- `SupersetSecurityException` from `validate()` blocks data access
- Security exceptions propagate correctly through `_compile_chart`
### 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
- [ ] 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]
