You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2025/10/25 00:18:36 UTC
[PR] feat(mcp): PR3 - add dashboard and dataset listing and info tools [superset]
aminghadersohi opened a new pull request, #35836:
URL: https://github.com/apache/superset/pull/35836
### SUMMARY
This PR adds 6 new MCP tools for dashboard and dataset management, expanding the MCP service to provide comprehensive read access to Superset's core resources.
**Built on top of:** PR #6 (feat/mcp_service_pr2_chart_listing_and_info) - Chart listing and info tools
**What's Included:**
## Dashboard Tools (3 tools)
**list_dashboards** - Paginated dashboard listing
- Filter by: dashboard_title, published, favorite
- Search across dashboard fields
- Sort by: id, dashboard_title, slug, published, changed_on, created_on
- Returns: DashboardList with UUID, slug, charts, owners
- 1-based pagination (page, page_size parameters)
**get_dashboard_info** - Retrieve dashboard details
- Lookup by integer ID, UUID string, or slug
- Returns: Complete dashboard metadata (title, position_json, charts, owners, roles)
- Supports all three identifier types (ID/UUID/slug)
- Consistent error responses with DashboardError schema
**get_dashboard_available_filters** - List filterable columns
- Returns available filter fields and operators
- Helps LLMs discover what dashboard fields can be filtered
- Returns: DashboardAvailableFilters with column/operator metadata
## Dataset Tools (3 tools)
**list_datasets** - Paginated dataset listing
- Filter by: table_name, schema, owner, favorite
- Search across dataset fields
- Sort by: id, table_name, schema, changed_on, created_on
- Returns: DatasetList with columns and metrics for each dataset
- 1-based pagination (page, page_size parameters)
**get_dataset_info** - Retrieve dataset details
- Lookup by integer ID or UUID string
- Returns: Complete dataset metadata including:
- Column definitions with types and expressions
- Metrics with SQL expressions
- Schema, database, table information
- Consistent error responses with DatasetError schema
**get_dataset_available_filters** - List filterable columns
- Returns available filter fields and operators
- Helps LLMs discover what dataset fields can be filtered
- Returns: DatasetAvailableFilters with column/operator metadata
## Core Infrastructure (579 lines)
**mcp_core.py** (expanded to 506 lines, +209 lines):
- **ModelGetAvailableFiltersCore**: Generic base class for get_*_available_filters tools
- Type-safe with `Generic[T]` for proper type inference
- Works with any Superset DAO (DashboardDAO, DatasetDAO, etc.)
- Returns filterable columns and supported operators
**utils/retry_utils.py** (340 lines):
- RetryableOperation class for database resilience
- Exponential backoff for transient failures
- Used by DAO operations throughout MCP service
**CLAUDE.md** (431 lines):
- Comprehensive guide for LLM agents working on MCP service
- Architecture overview and directory structure
- Tool development patterns and conventions
- Critical pitfalls to avoid
- Quick checklist for adding new tools
## Schemas (1,107 lines)
**dashboard/schemas.py** (418 lines):
- `DashboardInfo`: Detailed dashboard metadata model
- `DashboardList`: Paginated list response with PaginationInfo
- `DashboardError`: Consistent error responses
- `DashboardFilter`: Filter specification (col, opr, value)
- `ListDashboardsRequest`, `GetDashboardInfoRequest`: Tool request models
- `DashboardAvailableFilters`, `GetDashboardAvailableFiltersRequest`: Filter metadata
**dataset/schemas.py** (349 lines):
- `DatasetInfo`: Detailed dataset metadata model with columns/metrics
- `DatasetList`: Paginated list response with PaginationInfo
- `DatasetError`: Consistent error responses
- `DatasetFilter`: Filter specification (col, opr, value)
- `ListDatasetsRequest`, `GetDatasetInfoRequest`: Tool request models
- `DatasetAvailableFilters`, `GetDatasetAvailableFiltersRequest`: Filter metadata
**chart/schemas.py** (expanded to 793 lines, +510 lines):
- Added `serialize_chart_object()` helper for dashboard chart serialization
- Enhanced chart metadata extraction
- Used by dashboard tools to serialize embedded charts
**system/schemas.py** (expanded to 111 lines, +24 lines):
- Enhanced type safety with TYPE_CHECKING imports
- Additional shared schemas used across tools
## Testing (1,804 lines)
**test_dashboard_tools.py** (573 lines, 15 tests):
- Schema validation tests for DashboardInfo, DashboardList, DashboardFilter
- Request/response model validation
- list_dashboards: Basic, filters, search, custom columns, sorting tests
- get_dashboard_info: ID lookup, UUID lookup, slug lookup, not found cases
- Filter specification tests
**test_dataset_tools.py** (1,231 lines, 20 tests):
- Schema validation tests for DatasetInfo, DatasetList, DatasetFilter
- Request/response model validation
- list_datasets: Basic, filters, search, custom columns, sorting tests
- get_dataset_info: ID lookup, UUID lookup, not found cases, columns/metrics
- Filter specification tests
- Default ordering tests
**test_chart_schemas.py** (160 lines, 11 tests):
- XYChartConfig validation tests (label uniqueness, duplicate detection)
- TableChartConfig validation tests
- Chart configuration edge cases
**Total: 62 MCP tests passing** (11 chart + 15 dashboard + 20 dataset + 16 core)
## Integration
**app.py** (13 new lines):
```python
from superset.mcp_service.dashboard.tool import ( # noqa: F401, E402
get_dashboard_available_filters,
get_dashboard_info,
list_dashboards,
)
from superset.mcp_service.dataset.tool import ( # noqa: F401, E402
get_dataset_available_filters,
get_dataset_info,
list_datasets,
)
```
This follows the established pattern: tool imports in app.py auto-register via `@mcp.tool` decorators.
## Architecture & Patterns
All 6 tools follow the same reusable patterns established in PR #6:
1. **Use core classes**: ModelListCore for listing, ModelGetInfoCore for retrieval, ModelGetAvailableFiltersCore for filter metadata
2. **Pydantic schemas**: Type-safe requests/responses with Field descriptions for LLMs
3. **DAO pattern**: DashboardDAO and DatasetDAO for database access
4. **Authentication**: All tools use @mcp_auth_hook for security
5. **Type safety**: Modern Python 3.10+ type hints (`T | None` instead of `Optional[T]`)
6. **Consistent errors**: Structured error responses with timestamps
**Reusability:**
- Core classes work with any Superset entity (charts, dashboards, datasets, etc.)
- Just provide the DAO class and schema - the rest is handled automatically
- Same pattern will be used for upcoming chart creation and SQL Lab tools
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - Backend MCP service infrastructure only, no UI changes.
**Example tool usage via Claude Desktop:**
```
User: "List all published dashboards"
Claude calls: list_dashboards({
"filters": [{"col": "published", "opr": "eq", "value": true}]
})
User: "Show me the Sales Dashboard details"
Claude calls: get_dashboard_info({"identifier": "sales-dashboard"})
User: "List datasets in the analytics schema"
Claude calls: list_datasets({
"filters": [{"col": "schema", "opr": "eq", "value": "analytics"}]
})
User: "What columns are in dataset 42?"
Claude calls: get_dataset_info({"identifier": 42})
```
### TESTING INSTRUCTIONS
**Prerequisites:**
- Superset running with PR #6 (chart tools) merged
- Python 3.10 or 3.11
- fastmcp installed (`pip install -e .[development]`)
**Setup:**
```bash
# 1. Ensure database is initialized
export FLASK_APP=superset
superset db upgrade
superset init
# 2. Create admin user (if not already done)
superset fab create-admin \
--username admin \
--firstname Admin \
--lastname Admin \
--email admin@localhost \
--password admin
# 3. Load example data (for testing)
superset load-examples
```
**Run Tests:**
```bash
# Run all MCP service unit tests
pytest tests/unit_tests/mcp_service/ -v
# Should see:
# - test_chart_schemas.py: 11 chart schema tests passing
# - test_list_charts.py: Chart tool tests passing
# - test_dashboard_tools.py: 15 dashboard tests passing
# - test_dataset_tools.py: 20 dataset tests passing
# - test_mcp_core.py: Core infrastructure tests passing
# Total: 62 tests passing
```
**Test with MCP Server:**
```bash
# Terminal 1: Start Superset
superset run -p 9001
# Terminal 2: Start MCP service
superset mcp run --port 5008 --debug
# Terminal 3: Test with curl
curl http://localhost:5008/health
# Expected: {"status": "ok", "timestamp": "...", ...}
```
**Test Tools via Claude Desktop:**
Configure Claude Desktop with:
```json
{
"mcpServers": {
"superset": {
"command": "superset",
"args": ["mcp", "run", "--port", "5008"]
}
}
}
```
Then test queries:
- "List all dashboards" → Should call list_dashboards tool
- "Show me dashboard 1" → Should call get_dashboard_info tool
- "List all datasets" → Should call list_datasets tool
- "What columns are in dataset 5?" → Should call get_dataset_info tool
- "What can I filter dashboards by?" → Should call get_dashboard_available_filters
**Verify Error Handling:**
```python
# Test in Python shell with Flask app context
from superset.mcp_service.dashboard.tool import list_dashboards, get_dashboard_info
from superset.mcp_service.dataset.tool import list_datasets, get_dataset_info
# Test pagination
result = list_dashboards.fn(page=1, page_size=10)
assert result.pagination.page == 1
# Test filtering
result = list_dashboards.fn(filters=[{"col": "published", "opr": "eq", "value": True}])
assert all(d.published for d in result.dashboards)
# Test not found
result = get_dashboard_info.fn(identifier=99999)
assert result.error is not None
# Test dataset with columns
result = get_dataset_info.fn(identifier=1)
assert result.columns is not None
assert len(result.columns) > 0
```
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags: None
- [ ] Changes UI: No
- [ ] Includes DB Migration: No
- [x] Introduces new feature or API: Yes - MCP dashboard and dataset tools
- [ ] Removes existing feature or API: No
**Stats:**
- 24 files changed
- 5,294 insertions, 19 deletions
- 1,804 lines of tests (35 test cases: 15 dashboard + 20 dataset)
- 100% mypy compliant
- All pre-commit hooks passing
**Future PRs will add:**
- Chart creation tools (generate_chart, update_chart, generate_chart_preview)
- Dashboard creation tools (generate_dashboard, add_chart_to_dashboard)
- SQL Lab integration tools (execute_sql, open_sql_lab_with_context)
- Advanced authentication (JWT validation, user impersonation)
- Field-level permissions and audit logging
**Notes:**
- Builds cleanly on PR #6 (chart tools)
- Minimal changes to existing code (only app.py imports)
- No database migrations
- No UI changes
- Optional dependency (fastmcp in development requirements only)
- All tools follow established patterns from PR #6
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): PR3 - add dashboard and dataset listing and info tools [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #35836:
URL: https://github.com/apache/superset/pull/35836#issuecomment-3470884297
closing as this stack is too hard to maintain. i am just gonna keep the last one. PR10
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): PR3 - add dashboard and dataset listing and info tools [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #35836:
URL: https://github.com/apache/superset/pull/35836#issuecomment-3445303444
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): PR3 - add dashboard and dataset listing and info tools [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi closed pull request #35836: feat(mcp): PR3 - add dashboard and dataset listing and info tools
URL: https://github.com/apache/superset/pull/35836
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): PR3 - add dashboard and dataset listing and info tools [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #35836:
URL: https://github.com/apache/superset/pull/35836#issuecomment-3445367491
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/35836?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `432 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 49.83%. Comparing base ([`76d897e`](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`c6c8bdf`](https://app.codecov.io/gh/apache/superset/commit/c6c8bdf6270bd8b659f9d4d497262ec05a50afe9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2790 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/35836?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/mcp\_service/mcp\_core.py](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fmcp_core.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvbWNwX2NvcmUucHk=) | 0.00% | [193 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/utils/retry\_utils.py](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&filepath=superset%2Fmcp_service%2Futils%2Fretry_utils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvdXRpbHMvcmV0cnlfdXRpbHMucHk=) | 0.00% | [112 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/system/schemas.py](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fsystem%2Fschemas.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2Uvc3lzdGVtL3NjaGVtYXMucHk=) | 0.00% | [63 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/auth.py](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fauth.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvYXV0aC5weQ==) | 0.00% | [22 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/common/cache\_schemas.py](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fcommon%2Fcache_schemas.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvY29tbW9uL2NhY2hlX3NjaGVtYXMucHk=) | 0.00% | [16 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/middleware.py](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fmiddleware.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvbWlkZGxld2FyZS5weQ==) | 0.00% | [15 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/utils/\_\_init\_\_.py](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&filepath=superset%2Fmcp_service%2Futils%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvdXRpbHMvX19pbml0X18ucHk=) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/app.py](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fapp.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvYXBwLnB5) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/35836?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #35836 +/- ##
===========================================
- Coverage 60.48% 49.83% -10.66%
===========================================
Files 1931 606 -1325
Lines 76236 44440 -31796
Branches 8568 4799 -3769
===========================================
- Hits 46114 22147 -23967
+ Misses 28017 21669 -6348
+ Partials 2105 624 -1481
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/35836/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/35836/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `45.46% <0.00%> (-3.70%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/35836/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/35836/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `49.09% <0.00%> (-4.71%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/35836/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `49.83% <0.00%> (-13.68%)` | :arrow_down: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/35836/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (+42.36%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/35836?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): PR3 - add dashboard and dataset listing and info tools [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #35836:
URL: https://github.com/apache/superset/pull/35836#issuecomment-3445303371
Korbit doesn't automatically review large (3000+ lines changed) pull requests such as this one. If you want me to review anyway, use `/korbit-review`.
[](6552c25b-2764-4327-9fa4-cb2560195d59)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): PR3 - add dashboard and dataset listing and info tools [superset]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #35836:
URL: https://github.com/apache/superset/pull/35836#discussion_r2462284135
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -0,0 +1,1074 @@
+# 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.
+
+"""
+Pydantic schemas for chart-related responses
+"""
+
+from __future__ import annotations
+
+import html
+import re
+from datetime import datetime, timezone
+from typing import Annotated, Any, Dict, List, Literal, Protocol
+
+from pydantic import (
+ BaseModel,
+ ConfigDict,
+ Field,
+ field_validator,
+ model_validator,
+ PositiveInt,
+)
+
+from superset.daos.base import ColumnOperator, ColumnOperatorEnum
+from superset.mcp_service.common.cache_schemas import (
+ CacheStatus,
+ FormDataCacheControl,
+ MetadataCacheControl,
+ QueryCacheControl,
+)
+from superset.mcp_service.system.schemas import (
+ PaginationInfo,
+ TagInfo,
+ UserInfo,
+)
+
+
+class ChartLike(Protocol):
+ """Protocol for chart-like objects with expected attributes."""
+
+ id: int
+ slice_name: str | None
+ viz_type: str | None
+ datasource_name: str | None
+ datasource_type: str | None
+ url: str | None
+ description: str | None
+ cache_timeout: int | None
+ form_data: Dict[str, Any] | None
+ query_context: Any | None
+ changed_by: Any | None # User object
+ changed_by_name: str | None
+ changed_on: str | datetime | None
+ changed_on_humanized: str | None
+ created_by: Any | None # User object
+ created_by_name: str | None
+ created_on: str | datetime | None
+ created_on_humanized: str | None
+ uuid: str | None
+ tags: List[Any] | None
+ owners: List[Any] | None
+
+
+class ChartInfo(BaseModel):
+ """Full chart model with all possible attributes."""
+
+ id: int = Field(..., description="Chart ID")
+ slice_name: str = Field(..., description="Chart name")
+ viz_type: str | None = Field(None, description="Visualization type")
+ datasource_name: str | None = Field(None, description="Datasource name")
+ datasource_type: str | None = Field(None, description="Datasource type")
+ url: str | None = Field(None, description="Chart URL")
+ description: str | None = Field(None, description="Chart description")
+ cache_timeout: int | None = Field(None, description="Cache timeout")
+ form_data: Dict[str, Any] | None = Field(None, description="Chart form data")
+ query_context: Any | None = Field(None, description="Query context")
+ changed_by: str | None = Field(None, description="Last modifier (username)")
+ changed_by_name: str | None = Field(
+ None, description="Last modifier (display name)"
+ )
+ changed_on: str | datetime | None = Field(
+ None, description="Last modification timestamp"
+ )
+ changed_on_humanized: str | None = Field(
+ None, description="Humanized modification time"
+ )
+ created_by: str | None = Field(None, description="Chart creator (username)")
+ created_on: str | datetime | None = Field(None, description="Creation timestamp")
+ created_on_humanized: str | None = Field(
+ None, description="Humanized creation time"
+ )
+ uuid: str | None = Field(None, description="Chart UUID")
+ tags: List[TagInfo] = Field(default_factory=list, description="Chart tags")
+ owners: List[UserInfo] = Field(default_factory=list, description="Chart owners")
+ model_config = ConfigDict(from_attributes=True, ser_json_timedelta="iso8601")
+
+
+class GetChartAvailableFiltersRequest(BaseModel):
+ """
+ Request schema for get_chart_available_filters tool.
+
+ Currently has no parameters but provides consistent API for future extensibility.
+ """
+
+ model_config = ConfigDict(
+ extra="forbid",
+ str_strip_whitespace=True,
+ )
+
+
+class ChartAvailableFiltersResponse(BaseModel):
+ column_operators: Dict[str, Any] = Field(
+ ..., description="Available filter operators and metadata for each column"
+ )
+
+
+class ChartError(BaseModel):
+ error: str = Field(..., description="Error message")
+ error_type: str = Field(..., description="Type of error")
+ timestamp: datetime = Field(
+ default_factory=lambda: datetime.now(timezone.utc),
+ description="Error timestamp",
+ )
+ model_config = ConfigDict(ser_json_timedelta="iso8601")
+
+
+class ChartCapabilities(BaseModel):
+ """Describes what the chart can do for LLM understanding."""
+
+ supports_interaction: bool = Field(description="Chart supports user interaction")
+ supports_real_time: bool = Field(description="Chart supports live data updates")
+ supports_drill_down: bool = Field(
+ description="Chart supports drill-down navigation"
+ )
+ supports_export: bool = Field(description="Chart can be exported to other formats")
+ optimal_formats: List[str] = Field(description="Recommended preview formats")
+ data_types: List[str] = Field(
+ description="Types of data shown (time_series, categorical, etc)"
+ )
+
+
+class ChartSemantics(BaseModel):
+ """Semantic information for LLM reasoning."""
+
+ primary_insight: str = Field(
+ description="Main insight or pattern the chart reveals"
+ )
+ data_story: str = Field(description="Narrative description of what the data shows")
+ recommended_actions: List[str] = Field(
+ description="Suggested next steps based on data"
+ )
+ anomalies: List[str] = Field(description="Notable outliers or unusual patterns")
+ statistical_summary: Dict[str, Any] = Field(
+ description="Key statistics (mean, median, trends)"
+ )
+
+
+class PerformanceMetadata(BaseModel):
+ """Performance information for LLM cost understanding."""
+
+ query_duration_ms: int = Field(description="Query execution time")
+ estimated_cost: str | None = Field(None, description="Resource cost estimate")
+ cache_status: str = Field(description="Cache hit/miss status")
+ optimization_suggestions: List[str] = Field(
+ default_factory=list, description="Performance improvement tips"
+ )
+
+
+class AccessibilityMetadata(BaseModel):
+ """Accessibility information for inclusive visualization."""
+
+ color_blind_safe: bool = Field(description="Uses colorblind-safe palette")
+ alt_text: str = Field(description="Screen reader description")
+ high_contrast_available: bool = Field(description="High contrast version available")
+
+
+class VersionedResponse(BaseModel):
+ """Base class for versioned API responses."""
+
+ schema_version: str = Field("2.0", description="Response schema version")
+ api_version: str = Field("v1", description="MCP API version")
+
+
+class GetChartInfoRequest(BaseModel):
+ """Request schema for get_chart_info with support for ID or UUID."""
+
+ identifier: Annotated[
+ int | str,
+ Field(description="Chart identifier - can be numeric ID or UUID string"),
+ ]
+
+
+def serialize_chart_object(chart: ChartLike | None) -> ChartInfo | None:
+ if not chart:
+ return None
+
+ # TODO (Phase 3): Generate MCP service screenshot URL
+ # For now, use chart's native URL instead of screenshot URL
+ # Screenshot functionality will be added in Phase 3 PR
+
+ chart_id = getattr(chart, "id", None)
+ chart_url = getattr(chart, "url", None)
+
+ return ChartInfo(
+ id=chart_id,
+ slice_name=getattr(chart, "slice_name", None),
+ viz_type=getattr(chart, "viz_type", None),
+ datasource_name=getattr(chart, "datasource_name", None),
+ datasource_type=getattr(chart, "datasource_type", None),
+ url=chart_url,
+ description=getattr(chart, "description", None),
+ cache_timeout=getattr(chart, "cache_timeout", None),
+ form_data=getattr(chart, "form_data", None),
+ query_context=getattr(chart, "query_context", None),
+ changed_by=getattr(chart, "changed_by_name", None)
+ or (str(chart.changed_by) if getattr(chart, "changed_by", None) else None),
+ changed_by_name=getattr(chart, "changed_by_name", None),
+ changed_on=getattr(chart, "changed_on", None),
+ changed_on_humanized=getattr(chart, "changed_on_humanized", None),
+ created_by=getattr(chart, "created_by_name", None)
+ or (str(chart.created_by) if getattr(chart, "created_by", None) else None),
+ created_on=getattr(chart, "created_on", None),
+ created_on_humanized=getattr(chart, "created_on_humanized", None),
+ uuid=str(getattr(chart, "uuid", "")) if getattr(chart, "uuid", None) else None,
+ tags=[
+ TagInfo.model_validate(tag, from_attributes=True)
+ for tag in getattr(chart, "tags", [])
+ ]
+ if getattr(chart, "tags", None)
+ else [],
+ owners=[
+ UserInfo.model_validate(owner, from_attributes=True)
+ for owner in getattr(chart, "owners", [])
+ ]
+ if getattr(chart, "owners", None)
+ else [],
+ )
+
+
+class GenerateChartResponse(BaseModel):
+ """Comprehensive chart creation response with rich metadata."""
+
+ # Core chart information
+ chart: ChartInfo | None = Field(None, description="Complete chart metadata")
+
+ # Multiple preview formats available
+ previews: Dict[str, "ChartPreviewContent"] = Field(
+ default_factory=dict,
+ description="Available preview formats keyed by format type",
+ )
+
+ # LLM-friendly capabilities
+ capabilities: ChartCapabilities | None = Field(
+ None, description="Chart interaction capabilities"
+ )
+ semantics: ChartSemantics | None = Field(
+ None, description="Semantic chart understanding"
+ )
+
+ # Navigation and context
+ explore_url: str | None = Field(None, description="Edit chart in Superset")
+ embed_code: str | None = Field(None, description="HTML embed snippet")
+ api_endpoints: Dict[str, str] = Field(
+ default_factory=dict, description="Related API endpoints for data/updates"
+ )
+
+ # Performance and accessibility
+ performance: PerformanceMetadata | None = Field(
+ None, description="Performance metrics"
+ )
+ accessibility: AccessibilityMetadata | None = Field(
+ None, description="Accessibility info"
+ )
+
+ # Success/error handling
+ success: bool = Field(True, description="Whether chart creation succeeded")
+ error: ChartError | None = Field(
+ None, description="Error details if creation failed"
+ )
+ warnings: List[str] = Field(default_factory=list, description="Non-fatal warnings")
+
+ # Inherit versioning
+ schema_version: str = Field("2.0", description="Response schema version")
+ api_version: str = Field("v1", description="MCP API version")
+
+
+class ChartFilter(ColumnOperator):
+ """
+ Filter object for chart listing.
+ col: The column to filter on. Must be one of the allowed filter fields.
+ opr: The operator to use. Must be one of the supported operators.
+ value: The value to filter by (type depends on col and opr).
+ """
+
+ col: Literal[
+ "slice_name",
+ "viz_type",
+ "datasource_name",
+ ] = Field(
+ ...,
+ description="Column to filter on. See get_chart_available_filters for "
+ "allowed values.",
+ )
+ opr: ColumnOperatorEnum = Field(
+ ...,
+ description="Operator to use. See get_chart_available_filters for "
+ "allowed values.",
+ )
+ value: str | int | float | bool | List[str | int | float | bool] = Field(
+ ..., description="Value to filter by (type depends on col and opr)"
+ )
+
+
+class ChartList(BaseModel):
+ charts: List[ChartInfo]
+ count: int
+ total_count: int
+ page: int
+ page_size: int
+ total_pages: int
+ has_previous: bool
+ has_next: bool
+ columns_requested: List[str] | None = None
+ columns_loaded: List[str] | None = None
+ filters_applied: List[ChartFilter] = Field(
+ default_factory=list,
+ description="List of advanced filter dicts applied to the query.",
+ )
+ pagination: PaginationInfo | None = None
+ timestamp: datetime | None = None
+ model_config = ConfigDict(ser_json_timedelta="iso8601")
+
+
+# --- Simplified schemas for generate_chart tool ---
+
+
+# Common pieces
+class ColumnRef(BaseModel):
+ name: str = Field(
+ ...,
+ description="Column name",
+ min_length=1,
+ max_length=255,
+ pattern=r"^[a-zA-Z0-9_][a-zA-Z0-9_\s\-\.]*$",
+ )
+ label: str | None = Field(
+ None, description="Display label for the column", max_length=500
+ )
+ dtype: str | None = Field(None, description="Data type hint")
+ aggregate: (
+ Literal[
+ "SUM",
+ "COUNT",
+ "AVG",
+ "MIN",
+ "MAX",
+ "COUNT_DISTINCT",
+ "STDDEV",
+ "VAR",
+ "MEDIAN",
+ "PERCENTILE",
+ ]
+ | None
+ ) = Field(
+ None,
+ description="SQL aggregation function. Only these validated functions are "
+ "supported to prevent SQL errors.",
+ )
+
+ @field_validator("name")
+ @classmethod
+ def sanitize_name(cls, v: str) -> str:
+ """Sanitize column name to prevent XSS and SQL injection."""
+ if not v or not v.strip():
+ raise ValueError("Column name cannot be empty")
+
+ # Remove HTML tags and decode entities
+ sanitized = html.escape(v.strip())
+
+ # Check for script content
+ if re.search(r"<script[^>]*>.*?</script>", v, re.IGNORECASE | re.DOTALL):
+ raise ValueError(
+ "Column name contains potentially malicious script content"
+ )
+
Review Comment:
## Bad HTML filtering regexp
This regular expression does not match script end tags like </script >.
[Show more details](https://github.com/apache/superset/security/code-scanning/2044)
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -0,0 +1,1074 @@
+# 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.
+
+"""
+Pydantic schemas for chart-related responses
+"""
+
+from __future__ import annotations
+
+import html
+import re
+from datetime import datetime, timezone
+from typing import Annotated, Any, Dict, List, Literal, Protocol
+
+from pydantic import (
+ BaseModel,
+ ConfigDict,
+ Field,
+ field_validator,
+ model_validator,
+ PositiveInt,
+)
+
+from superset.daos.base import ColumnOperator, ColumnOperatorEnum
+from superset.mcp_service.common.cache_schemas import (
+ CacheStatus,
+ FormDataCacheControl,
+ MetadataCacheControl,
+ QueryCacheControl,
+)
+from superset.mcp_service.system.schemas import (
+ PaginationInfo,
+ TagInfo,
+ UserInfo,
+)
+
+
+class ChartLike(Protocol):
+ """Protocol for chart-like objects with expected attributes."""
+
+ id: int
+ slice_name: str | None
+ viz_type: str | None
+ datasource_name: str | None
+ datasource_type: str | None
+ url: str | None
+ description: str | None
+ cache_timeout: int | None
+ form_data: Dict[str, Any] | None
+ query_context: Any | None
+ changed_by: Any | None # User object
+ changed_by_name: str | None
+ changed_on: str | datetime | None
+ changed_on_humanized: str | None
+ created_by: Any | None # User object
+ created_by_name: str | None
+ created_on: str | datetime | None
+ created_on_humanized: str | None
+ uuid: str | None
+ tags: List[Any] | None
+ owners: List[Any] | None
+
+
+class ChartInfo(BaseModel):
+ """Full chart model with all possible attributes."""
+
+ id: int = Field(..., description="Chart ID")
+ slice_name: str = Field(..., description="Chart name")
+ viz_type: str | None = Field(None, description="Visualization type")
+ datasource_name: str | None = Field(None, description="Datasource name")
+ datasource_type: str | None = Field(None, description="Datasource type")
+ url: str | None = Field(None, description="Chart URL")
+ description: str | None = Field(None, description="Chart description")
+ cache_timeout: int | None = Field(None, description="Cache timeout")
+ form_data: Dict[str, Any] | None = Field(None, description="Chart form data")
+ query_context: Any | None = Field(None, description="Query context")
+ changed_by: str | None = Field(None, description="Last modifier (username)")
+ changed_by_name: str | None = Field(
+ None, description="Last modifier (display name)"
+ )
+ changed_on: str | datetime | None = Field(
+ None, description="Last modification timestamp"
+ )
+ changed_on_humanized: str | None = Field(
+ None, description="Humanized modification time"
+ )
+ created_by: str | None = Field(None, description="Chart creator (username)")
+ created_on: str | datetime | None = Field(None, description="Creation timestamp")
+ created_on_humanized: str | None = Field(
+ None, description="Humanized creation time"
+ )
+ uuid: str | None = Field(None, description="Chart UUID")
+ tags: List[TagInfo] = Field(default_factory=list, description="Chart tags")
+ owners: List[UserInfo] = Field(default_factory=list, description="Chart owners")
+ model_config = ConfigDict(from_attributes=True, ser_json_timedelta="iso8601")
+
+
+class GetChartAvailableFiltersRequest(BaseModel):
+ """
+ Request schema for get_chart_available_filters tool.
+
+ Currently has no parameters but provides consistent API for future extensibility.
+ """
+
+ model_config = ConfigDict(
+ extra="forbid",
+ str_strip_whitespace=True,
+ )
+
+
+class ChartAvailableFiltersResponse(BaseModel):
+ column_operators: Dict[str, Any] = Field(
+ ..., description="Available filter operators and metadata for each column"
+ )
+
+
+class ChartError(BaseModel):
+ error: str = Field(..., description="Error message")
+ error_type: str = Field(..., description="Type of error")
+ timestamp: datetime = Field(
+ default_factory=lambda: datetime.now(timezone.utc),
+ description="Error timestamp",
+ )
+ model_config = ConfigDict(ser_json_timedelta="iso8601")
+
+
+class ChartCapabilities(BaseModel):
+ """Describes what the chart can do for LLM understanding."""
+
+ supports_interaction: bool = Field(description="Chart supports user interaction")
+ supports_real_time: bool = Field(description="Chart supports live data updates")
+ supports_drill_down: bool = Field(
+ description="Chart supports drill-down navigation"
+ )
+ supports_export: bool = Field(description="Chart can be exported to other formats")
+ optimal_formats: List[str] = Field(description="Recommended preview formats")
+ data_types: List[str] = Field(
+ description="Types of data shown (time_series, categorical, etc)"
+ )
+
+
+class ChartSemantics(BaseModel):
+ """Semantic information for LLM reasoning."""
+
+ primary_insight: str = Field(
+ description="Main insight or pattern the chart reveals"
+ )
+ data_story: str = Field(description="Narrative description of what the data shows")
+ recommended_actions: List[str] = Field(
+ description="Suggested next steps based on data"
+ )
+ anomalies: List[str] = Field(description="Notable outliers or unusual patterns")
+ statistical_summary: Dict[str, Any] = Field(
+ description="Key statistics (mean, median, trends)"
+ )
+
+
+class PerformanceMetadata(BaseModel):
+ """Performance information for LLM cost understanding."""
+
+ query_duration_ms: int = Field(description="Query execution time")
+ estimated_cost: str | None = Field(None, description="Resource cost estimate")
+ cache_status: str = Field(description="Cache hit/miss status")
+ optimization_suggestions: List[str] = Field(
+ default_factory=list, description="Performance improvement tips"
+ )
+
+
+class AccessibilityMetadata(BaseModel):
+ """Accessibility information for inclusive visualization."""
+
+ color_blind_safe: bool = Field(description="Uses colorblind-safe palette")
+ alt_text: str = Field(description="Screen reader description")
+ high_contrast_available: bool = Field(description="High contrast version available")
+
+
+class VersionedResponse(BaseModel):
+ """Base class for versioned API responses."""
+
+ schema_version: str = Field("2.0", description="Response schema version")
+ api_version: str = Field("v1", description="MCP API version")
+
+
+class GetChartInfoRequest(BaseModel):
+ """Request schema for get_chart_info with support for ID or UUID."""
+
+ identifier: Annotated[
+ int | str,
+ Field(description="Chart identifier - can be numeric ID or UUID string"),
+ ]
+
+
+def serialize_chart_object(chart: ChartLike | None) -> ChartInfo | None:
+ if not chart:
+ return None
+
+ # TODO (Phase 3): Generate MCP service screenshot URL
+ # For now, use chart's native URL instead of screenshot URL
+ # Screenshot functionality will be added in Phase 3 PR
+
+ chart_id = getattr(chart, "id", None)
+ chart_url = getattr(chart, "url", None)
+
+ return ChartInfo(
+ id=chart_id,
+ slice_name=getattr(chart, "slice_name", None),
+ viz_type=getattr(chart, "viz_type", None),
+ datasource_name=getattr(chart, "datasource_name", None),
+ datasource_type=getattr(chart, "datasource_type", None),
+ url=chart_url,
+ description=getattr(chart, "description", None),
+ cache_timeout=getattr(chart, "cache_timeout", None),
+ form_data=getattr(chart, "form_data", None),
+ query_context=getattr(chart, "query_context", None),
+ changed_by=getattr(chart, "changed_by_name", None)
+ or (str(chart.changed_by) if getattr(chart, "changed_by", None) else None),
+ changed_by_name=getattr(chart, "changed_by_name", None),
+ changed_on=getattr(chart, "changed_on", None),
+ changed_on_humanized=getattr(chart, "changed_on_humanized", None),
+ created_by=getattr(chart, "created_by_name", None)
+ or (str(chart.created_by) if getattr(chart, "created_by", None) else None),
+ created_on=getattr(chart, "created_on", None),
+ created_on_humanized=getattr(chart, "created_on_humanized", None),
+ uuid=str(getattr(chart, "uuid", "")) if getattr(chart, "uuid", None) else None,
+ tags=[
+ TagInfo.model_validate(tag, from_attributes=True)
+ for tag in getattr(chart, "tags", [])
+ ]
+ if getattr(chart, "tags", None)
+ else [],
+ owners=[
+ UserInfo.model_validate(owner, from_attributes=True)
+ for owner in getattr(chart, "owners", [])
+ ]
+ if getattr(chart, "owners", None)
+ else [],
+ )
+
+
+class GenerateChartResponse(BaseModel):
+ """Comprehensive chart creation response with rich metadata."""
+
+ # Core chart information
+ chart: ChartInfo | None = Field(None, description="Complete chart metadata")
+
+ # Multiple preview formats available
+ previews: Dict[str, "ChartPreviewContent"] = Field(
+ default_factory=dict,
+ description="Available preview formats keyed by format type",
+ )
+
+ # LLM-friendly capabilities
+ capabilities: ChartCapabilities | None = Field(
+ None, description="Chart interaction capabilities"
+ )
+ semantics: ChartSemantics | None = Field(
+ None, description="Semantic chart understanding"
+ )
+
+ # Navigation and context
+ explore_url: str | None = Field(None, description="Edit chart in Superset")
+ embed_code: str | None = Field(None, description="HTML embed snippet")
+ api_endpoints: Dict[str, str] = Field(
+ default_factory=dict, description="Related API endpoints for data/updates"
+ )
+
+ # Performance and accessibility
+ performance: PerformanceMetadata | None = Field(
+ None, description="Performance metrics"
+ )
+ accessibility: AccessibilityMetadata | None = Field(
+ None, description="Accessibility info"
+ )
+
+ # Success/error handling
+ success: bool = Field(True, description="Whether chart creation succeeded")
+ error: ChartError | None = Field(
+ None, description="Error details if creation failed"
+ )
+ warnings: List[str] = Field(default_factory=list, description="Non-fatal warnings")
+
+ # Inherit versioning
+ schema_version: str = Field("2.0", description="Response schema version")
+ api_version: str = Field("v1", description="MCP API version")
+
+
+class ChartFilter(ColumnOperator):
+ """
+ Filter object for chart listing.
+ col: The column to filter on. Must be one of the allowed filter fields.
+ opr: The operator to use. Must be one of the supported operators.
+ value: The value to filter by (type depends on col and opr).
+ """
+
+ col: Literal[
+ "slice_name",
+ "viz_type",
+ "datasource_name",
+ ] = Field(
+ ...,
+ description="Column to filter on. See get_chart_available_filters for "
+ "allowed values.",
+ )
+ opr: ColumnOperatorEnum = Field(
+ ...,
+ description="Operator to use. See get_chart_available_filters for "
+ "allowed values.",
+ )
+ value: str | int | float | bool | List[str | int | float | bool] = Field(
+ ..., description="Value to filter by (type depends on col and opr)"
+ )
+
+
+class ChartList(BaseModel):
+ charts: List[ChartInfo]
+ count: int
+ total_count: int
+ page: int
+ page_size: int
+ total_pages: int
+ has_previous: bool
+ has_next: bool
+ columns_requested: List[str] | None = None
+ columns_loaded: List[str] | None = None
+ filters_applied: List[ChartFilter] = Field(
+ default_factory=list,
+ description="List of advanced filter dicts applied to the query.",
+ )
+ pagination: PaginationInfo | None = None
+ timestamp: datetime | None = None
+ model_config = ConfigDict(ser_json_timedelta="iso8601")
+
+
+# --- Simplified schemas for generate_chart tool ---
+
+
+# Common pieces
+class ColumnRef(BaseModel):
+ name: str = Field(
+ ...,
+ description="Column name",
+ min_length=1,
+ max_length=255,
+ pattern=r"^[a-zA-Z0-9_][a-zA-Z0-9_\s\-\.]*$",
+ )
+ label: str | None = Field(
+ None, description="Display label for the column", max_length=500
+ )
+ dtype: str | None = Field(None, description="Data type hint")
+ aggregate: (
+ Literal[
+ "SUM",
+ "COUNT",
+ "AVG",
+ "MIN",
+ "MAX",
+ "COUNT_DISTINCT",
+ "STDDEV",
+ "VAR",
+ "MEDIAN",
+ "PERCENTILE",
+ ]
+ | None
+ ) = Field(
+ None,
+ description="SQL aggregation function. Only these validated functions are "
+ "supported to prevent SQL errors.",
+ )
+
+ @field_validator("name")
+ @classmethod
+ def sanitize_name(cls, v: str) -> str:
+ """Sanitize column name to prevent XSS and SQL injection."""
+ if not v or not v.strip():
+ raise ValueError("Column name cannot be empty")
+
+ # Remove HTML tags and decode entities
+ sanitized = html.escape(v.strip())
+
+ # Check for script content
+ if re.search(r"<script[^>]*>.*?</script>", v, re.IGNORECASE | re.DOTALL):
+ raise ValueError(
+ "Column name contains potentially malicious script content"
+ )
+
+ # Basic SQL injection patterns (basic protection)
+ dangerous_patterns = [
+ r"(;|\||&|\$|`)",
+ r"\b(DROP|DELETE|INSERT|UPDATE|CREATE|ALTER|EXEC|EXECUTE)\b",
+ r"--",
+ r"/\*.*\*/",
+ ]
+
+ for pattern in dangerous_patterns:
+ if re.search(pattern, v, re.IGNORECASE):
+ raise ValueError(
+ "Column name contains potentially unsafe characters or SQL keywords"
+ )
+
+ return sanitized
+
+ @field_validator("label")
+ @classmethod
+ def sanitize_label(cls, v: str | None) -> str | None:
+ """Sanitize display label to prevent XSS attacks."""
+ if v is None:
+ return v
+
+ # Strip whitespace
+ v = v.strip()
+ if not v:
+ return None
+
+ # Check for dangerous HTML tags and JavaScript protocols BEFORE escaping
+ dangerous_patterns = [
+ r"<script[^>]*>.*?</script>", # Script tags
+ r"<iframe[^>]*>.*?</iframe>", # Iframe tags
+ r"<object[^>]*>.*?</object>", # Object tags
+ r"<embed[^>]*>.*?</embed>", # Embed tags
+ r"<link[^>]*>", # Link tags
+ r"<meta[^>]*>", # Meta tags
+ r"javascript:", # JavaScript protocol
+ r"vbscript:", # VBScript protocol
+ r"data:text/html", # Data URL HTML
+ r"on\w+\s*=", # Event handlers (onclick, onload, etc)
+ ]
+
+ for pattern in dangerous_patterns:
+ if re.search(pattern, v, re.IGNORECASE | re.DOTALL):
+ raise ValueError(
+ "Label contains potentially malicious content. "
+ "HTML tags, JavaScript, and event handlers are not allowed in "
+ "labels."
+ )
+
+ # Filter dangerous Unicode characters
+ v = re.sub(
+ r"[\u200B-\u200D\uFEFF\u0000-\u0008\u000B\u000C\u000E-\u001F]", "", v
+ )
+
+ # HTML escape the cleaned content
+ sanitized = html.escape(v)
+
+ return sanitized if sanitized else None
+
+
+class AxisConfig(BaseModel):
+ title: str | None = Field(None, description="Axis title", max_length=200)
+ scale: Literal["linear", "log"] | None = Field(
+ "linear", description="Axis scale type"
+ )
+ format: str | None = Field(
+ None, description="Format string (e.g. '$,.2f')", max_length=50
+ )
+
+
+class LegendConfig(BaseModel):
+ show: bool = Field(True, description="Whether to show legend")
+ position: Literal["top", "bottom", "left", "right"] | None = Field(
+ "right", description="Legend position"
+ )
+
+
+class FilterConfig(BaseModel):
+ column: str = Field(
+ ..., description="Column to filter on", min_length=1, max_length=255
+ )
+ op: Literal["=", ">", "<", ">=", "<=", "!="] = Field(
+ ..., description="Filter operator"
+ )
+ value: str | int | float | bool = Field(..., description="Filter value")
+
+ @field_validator("column")
+ @classmethod
+ def sanitize_column(cls, v: str) -> str:
+ """Sanitize filter column name to prevent injection attacks."""
+ if not v or not v.strip():
+ raise ValueError("Filter column name cannot be empty")
+
+ # Remove HTML tags and decode entities
+ sanitized = html.escape(v.strip())
+
+ # Check for dangerous patterns
+ if re.search(r"<script[^>]*>.*?</script>", v, re.IGNORECASE | re.DOTALL):
+ raise ValueError(
+ "Filter column contains potentially malicious script content"
+ )
+
Review Comment:
## Bad HTML filtering regexp
This regular expression does not match script end tags like </script >.
[Show more details](https://github.com/apache/superset/security/code-scanning/2045)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org