feat(mcp): add list and get tools for row level security and plugins - #40347
Conversation
dosubot
Bot
added
api
geido
left a comment
geido
left a comment
There was a problem hiding this comment.
Reviewed carefully given the sensitivity of RLS data. class_permission_name="Row Level Security" is in ADMIN_ONLY_VIEW_MENUS, so non-admins cannot see filter clauses, tables, or associated roles. MCP_RBAC_ENABLED defaults to True and check_tool_permission enforces can_read at call time. The deliberate bypass of USER_DIRECTORY_FIELDS for the RLS roles column is correct — that field represents policy scope, not user directory metadata, and the bypass only applies after the Admin gate has passed. Plugin tools expose only non-sensitive metadata.
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #f6e20e
Actionable Suggestions - 1
-
superset/mcp_service/plugin/tool/list_plugins.py - 1
- Type mismatch in serializer signature · Line 78-78
Additional Suggestions - 1
-
superset/mcp_service/rls/tool/list_rls_filters.py - 1
-
Serializer naming inconsistency · Line 78-78Rename the `_serialize` function to `_serialize_rls_filter` and update the `item_serializer` argument accordingly to match naming conventions used by other list tools.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset/mcp_service/rls/tool/get_rls_filter_info.py - 1
- Async/sync context manager mismatch · Line 69-78
-
superset/mcp_service/plugin/schemas.py - 1
- Missing filterable columns in schema · Line 62-62
Review Details
-
Files reviewed - 19 · Commit Range:
10ea6da..6e3a325- superset/mcp_service/app.py
- superset/mcp_service/plugin/__init__.py
- superset/mcp_service/plugin/dao.py
- superset/mcp_service/plugin/schemas.py
- superset/mcp_service/plugin/tool/__init__.py
- superset/mcp_service/plugin/tool/get_plugin_info.py
- superset/mcp_service/plugin/tool/list_plugins.py
- superset/mcp_service/privacy.py
- superset/mcp_service/rls/__init__.py
- superset/mcp_service/rls/schemas.py
- superset/mcp_service/rls/tool/__init__.py
- superset/mcp_service/rls/tool/get_rls_filter_info.py
- superset/mcp_service/rls/tool/list_rls_filters.py
- tests/unit_tests/mcp_service/plugin/__init__.py
- tests/unit_tests/mcp_service/plugin/tool/__init__.py
- tests/unit_tests/mcp_service/plugin/tool/test_plugin_tools.py
- tests/unit_tests/mcp_service/rls/__init__.py
- tests/unit_tests/mcp_service/rls/tool/__init__.py
- tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
|
Addressed review feedback:
|
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #40347 +/- ##
========================================
Coverage 64.00% 64.00%
========================================
Files 2629 2638 +9
Lines 141299 141604 +305
Branches 32545 32560 +15
========================================
+ Hits 90432 90629 +197
- Misses 49314 49420 +106
- Partials 1553 1555 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Integration test results
In a multi-tenant deployment where workspace admins are mapped to a custom FAB role that does not include This likely needs a permission adjustment in the deployment's role config, or the plugin tools should use a less-restrictive permission if plugins are meant to be readable by any admin. |
|
Thanks for the detailed staging report. After investigating the permission setup: Root cause: The plugin tools use In OSS Superset, Comparison with RLS tools: No OSS change needed: Switching Fix: Add |
|
Bito Review Failed - Technical Failure |
|
A couple of schema issues worth addressing before merge:
|
|
Applied cross-PR feedback from Richard's thorough review on #40344 and #40348 — same patterns addressed here where applicable. Checked each of Richard's patterns against this PR:
|
Code Review Agent Run #34e01aActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
544df09 to
02e4137
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…er roles to be returned RLS filter `roles` (which roles a filter applies to) are core RLS data, not user-directory metadata. Including 'roles' in USER_DIRECTORY_FIELDS caused filter_user_directory_columns() to strip it from any requested select_columns list, making it impossible to retrieve via list_rls_filters. No dashboard/chart/dataset schema defines a 'roles' field, so removing it from the block set has no privacy impact on other tools. Fixes test_list_rls_filters_returns_tables_and_roles.
…in RLS list tool 'roles' on a dashboard/chart exposes who has access to the resource and should be stripped by the USER_DIRECTORY_FIELDS privacy filter. 'roles' in an RLS filter is which roles the filter applies to — it is core filter data, not user-directory metadata. The RLS list tool now derives its column selection directly from ALL_RLS_COLUMNS (bypassing ModelListCore's USER_DIRECTORY_FIELDS filtering) so that RLS roles are selectable while dashboard roles remain hidden. Fixes three failing unit tests: - test_list_dashboards_omits_requested_user_directory_fields - test_get_allowed_fields_always_denies_user_directory_fields - test_filter_sensitive_data_strips_user_directory_fields_even_if_allowed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… ValueError in list_rls_filters - Rename _serialize → _serialize_plugin in list_plugins.py and _serialize → _serialize_rls_filter in list_rls_filters.py to match naming convention used by all other list tools - Fix type annotation cols: list[str] | None → list[str] in both serializer signatures to match ModelListCore Callable expectation - Strip USER_DIRECTORY_FIELDS from select_columns before run_tool in list_rls_filters so a roles-only request no longer raises ValueError; the existing model_dump bypass restores roles in the output - Add regression test for roles-only select_columns edge case Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
02e4137 to
69d170c
Compare
There was a problem hiding this comment.
Code Review Agent Run #28c2d8
Actionable Suggestions - 1
-
superset/mcp_service/plugin/tool/get_plugin_info.py - 1
- Avoid catching blind Exception · Line 92-92
Additional Suggestions - 2
-
superset/mcp_service/rls/tool/get_rls_filter_info.py - 1
-
Inconsistent tag in related tools · Line 41-41Inconsistent tag assignment: `get_rls_filter_info` uses `tags=["discovery"]` but its sibling `list_rls_filters` (line 48) uses `tags=["core"]`. All 15 `list_*.py` tools in the codebase use `tags=["core"]`; the `get_rls_filter_info` tool should follow the same pattern for consistent categorization in MCP tooling.
Code suggestion
--- a/superset/mcp_service/rls/tool/get_rls_filter_info.py +++ b/superset/mcp_service/rls/tool/get_rls_filter_info.py @@ -38,7 +38,7 @@ @tool( - tags=["discovery"], + tags=["core"], class_permission_name="Row Level Security", annotations=ToolAnnotations( title="Get RLS filter info",
-
-
superset/mcp_service/plugin/tool/list_plugins.py - 1
-
Unused parameter cols · Line 78-79The `_serialize_plugin` closure accepts `cols: list[str]` but never uses it — `serialize_plugin_object` returns all fields regardless. Other serializers (e.g., `_serialize_user`) conditionally use `cols` for relationship loading. Consider passing `cols` through for consistency and future column-filtering support.
Code suggestion
--- a/superset/mcp_service/plugin/tool/list_plugins.py +++ b/superset/mcp_service/plugin/tool/list_plugins.py @@ -75,7 +75,10 @@ async def list_plugins( from superset.mcp_service.plugin.dao import DynamicPluginDAO def _serialize_plugin(obj: object, cols: list[str]) -> PluginInfo | None: - return serialize_plugin_object(obj) + # Note: cols parameter available for future column-based filtering + # Current serialize_plugin_object returns all fields. + # Use cols if selective field serialization is needed later. + return serialize_plugin_object(obj) list_tool = ModelListCore(
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset/mcp_service/rls/tool/list_rls_filters.py - 1
- Return type mismatch · Line 59-59
-
superset/mcp_service/rls/tool/get_rls_filter_info.py - 1
- Broad exception catch masking errors · Line 92-92
-
superset/mcp_service/privacy.py - 1
- Docstring Inaccuracy · Line 143-143
Review Details
-
Files reviewed - 19 · Commit Range:
5771bce..69d170c- superset/mcp_service/app.py
- superset/mcp_service/plugin/__init__.py
- superset/mcp_service/plugin/dao.py
- superset/mcp_service/plugin/schemas.py
- superset/mcp_service/plugin/tool/__init__.py
- superset/mcp_service/plugin/tool/get_plugin_info.py
- superset/mcp_service/plugin/tool/list_plugins.py
- superset/mcp_service/privacy.py
- superset/mcp_service/rls/__init__.py
- superset/mcp_service/rls/schemas.py
- superset/mcp_service/rls/tool/__init__.py
- superset/mcp_service/rls/tool/get_rls_filter_info.py
- superset/mcp_service/rls/tool/list_rls_filters.py
- tests/unit_tests/mcp_service/plugin/__init__.py
- tests/unit_tests/mcp_service/plugin/tool/__init__.py
- tests/unit_tests/mcp_service/plugin/tool/test_plugin_tools.py
- tests/unit_tests/mcp_service/rls/__init__.py
- tests/unit_tests/mcp_service/rls/tool/__init__.py
- tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
|
|
||
| return result | ||
|
|
||
| except Exception as e: |
There was a problem hiding this comment.
Replace the broad Exception catch with specific exception types (e.g., ValueError, KeyError, or custom exceptions) to improve error handling and debugging.
Code suggestion
Check the AI-generated fix before applying
| except Exception as e: | |
| except (ValueError, KeyError, AttributeError) as e: |
Code Review Run #28c2d8
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
2 participants
SUMMARY
Adds four new MCP (Model Context Protocol) tools to Apache Superset across two new domains:
Row Level Security (
superset/mcp_service/rls/):list_rls_filters— list RLS filters with filtering, search, column selection, sorting, and pagination. Requires admin access.get_rls_filter_info— get full RLS filter details by ID (name, type, tables, roles, clause, group key). Requires admin access.Dynamic Plugins (
superset/mcp_service/plugin/):list_plugins— list registered dynamic plugins with filtering, search, column selection, sorting, and pagination.get_plugin_info— get plugin details by ID (name, key, bundle_url, timestamps).Both domains follow the established MCP service patterns:
@tooldecorator withclass_permission_namefor RBAC ("Row Level Security","DynamicPlugin")ModelListCore/ModelGetInfoCorefrommcp_core.pyfor reusable list/get logicmodel_serializerfor column-filtered responsesColumnOperator/ColumnOperatorEnumfor structured filter objectsevent_loggerinstrumentation and FastMCPctxloggingThe
DynamicPluginDAOis co-located insuperset/mcp_service/plugin/dao.pysince no top-level DAO existed for theDynamicPluginmodel.The
DEFAULT_INSTRUCTIONSinapp.pyare updated to clarify thatget_schemacovers chart/dataset/dashboard/database resource types only; RLS and plugin tools document their filterable/sortable columns inline in their docstrings.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend MCP tools only.
TESTING INSTRUCTIONS
list_rls_filters— returns paginated RLS filters with id, name, filter_type, clause by defaultlist_rls_filterswithselect_columns: ["id", "name", "tables", "roles"]— includes relationship dataget_rls_filter_infowith a valid RLS filter ID — returns full filter detailslist_plugins— returns paginated plugin list with id, name, key, bundle_url by defaultget_plugin_infowith a valid plugin ID — returns full plugin detailsUnit tests:
pytest tests/unit_tests/mcp_service/rls/ tests/unit_tests/mcp_service/plugin/ADDITIONAL INFORMATION
Live Test Results
Tested on staging Preset workspace (build ID:
36a56752, MCP server:claude.ai preset stg).list_rls_filtersRequest:
{}Response:
{ "rls_filters": [ {"id": 1, "name": "QA Test - Country Filter", "filter_type": "Regular", "clause": "publisher = 'Nintendo'"} ], "count": 1, "total_count": 1, "page": 1, "page_size": 10, "total_pages": 1, "has_previous": false, "has_next": false, "columns_requested": ["id", "name", "filter_type", "clause"], "columns_loaded": ["id", "name", "filter_type", "clause"], "columns_available": ["id", "name", "description", "filter_type", "tables", "clause", "group_key", "created_on", "changed_on"], "sortable_columns": ["id", "name", "filter_type", "changed_on"], "filters_applied": [], "pagination": {"page": 1, "page_size": 10, "total_count": 1, "total_pages": 1, "has_next": false, "has_previous": false}, "timestamp": "2026-05-29T17:13:05.492602Z" }✅ 1 RLS filter returned, no error.
get_rls_filter_infoRequest:
{"identifier": 1}Response:
{ "id": 1, "name": "QA Test - Country Filter", "description": "QA test for ticket 100910", "filter_type": "Regular", "tables": [{"id": 2, "table_name": "Video Game Sales"}], "roles": [{"id": 17, "name": "new role"}], "clause": "publisher = 'Nintendo'", "group_key": "", "created_on": "2026-03-10T13:12:14.214592", "changed_on": "2026-03-10T17:20:59.654195" }✅ Full RLS filter detail returned including tables and roles, no error.
list_pluginsRequest:
{}Response:
{ "plugins": [], "count": 0, "total_count": 0, "page": 1, "page_size": 10, "total_pages": 0, "has_previous": false, "has_next": false, "columns_requested": ["id", "name", "key", "bundle_url"], "columns_loaded": ["id", "name", "key", "bundle_url"], "columns_available": ["id", "name", "key", "bundle_url", "changed_on", "created_on"], "sortable_columns": ["id", "name", "key", "changed_on", "created_on"], "filters_applied": [], "pagination": {"page": 1, "page_size": 10, "total_count": 0, "total_pages": 0, "has_next": false, "has_previous": false}, "timestamp": "2026-05-29T17:13:07.920617Z" }✅ Empty list, no permission error (this workspace has no custom plugins installed),
columns_availablepresent.get_plugin_infoRequest:
{"identifier": 9999}Response:
{ "error": "PluginInfo with identifier '9999' not found", "error_type": "not_found", "timestamp": "2026-05-29T17:13:09.104378Z" }✅ Structured
not_founderror, no permission error, no crash.