fix(mcp): add permission checks to generate_dashboard and update_chart tools#38845
Conversation
…t tools - generate_dashboard: Add chart access check via security_manager.can_access_chart() after verifying charts exist but before creating the dashboard - update_chart: Add dataset access validation via validate_chart_dataset() after finding the chart but before applying configuration changes - Add tests for both permission checks
Code Review Agent Run #298c73Actionable 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 |
dosubot
Bot
added
authentication:access-control
codeant-ai-for-open-source
Bot
added
the
size:L
Sequence DiagramThis PR adds two pre-write authorization checks in MCP tools. Generate dashboard now blocks inaccessible charts, and update chart now blocks updates when the chart dataset is inaccessible or missing. sequenceDiagram
participant Client
participant MCP
participant SecurityManager
participant DatasetValidator
Client->>MCP: Call generate dashboard
MCP->>SecurityManager: Check access for requested charts
SecurityManager-->>MCP: Access result per chart
alt Any chart inaccessible
MCP-->>Client: Return access denied with chart ids
else All charts accessible
MCP-->>Client: Continue with dashboard creation flow
end
Client->>MCP: Call update chart
MCP->>DatasetValidator: Validate chart dataset access
DatasetValidator-->>MCP: Dataset validation result
alt Dataset inaccessible or missing
MCP-->>Client: Return DatasetNotAccessible error
else Dataset accessible
MCP-->>Client: Continue with chart update flow
end
Generated by CodeAnt AI |
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (99.85%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #38845 +/- ##
==========================================
- Coverage 65.54% 64.40% -1.14%
==========================================
Files 1820 2535 +715
Lines 72868 130469 +57601
Branches 23339 30220 +6881
==========================================
+ Hits 47758 84035 +36277
- Misses 25110 44968 +19858
- Partials 0 1466 +1466
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:
|
Remove parentheses from @pytest.mark.asyncio() and @pytest.fixture() decorators to comply with ruff 0.9.7 PT023 rule.
Code Review Agent Run #5534c5Actionable 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 |
- Replace security_manager.can_access_chart() with validate_chart_dataset() in generate_dashboard for consistency with other MCP tools - Add dataset access check to add_chart_to_existing_dashboard - Narrow broad except Exception to specific exceptions in update_chart - Add test for add_chart_to_existing_dashboard dataset access validation - Update test mocks to use validate_chart_dataset instead of can_access_chart
OSS ruff 0.9.7 enforces no-parentheses style for @pytest.mark.asyncio and @pytest.fixture decorators.
Antonio-RiveroMartnez
left a comment
Antonio-RiveroMartnez
left a comment
There was a problem hiding this comment.
Why not using the RBAC system introduced in #38407 ? or extending it?
![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 #57d487
Actionable Suggestions - 1
-
superset/mcp_service/dashboard/tool/generate_dashboard.py - 1
- Error handling regression in multi-chart validation · Line 236-245
Additional Suggestions - 1
-
superset/mcp_service/chart/tool/update_chart.py - 1
-
Narrow exception handling risks unhandled errors · Line 270-270The exception handling change narrows the catch from all Exception to specific types, which may allow unexpected exceptions to propagate instead of returning error responses. This differs from similar functions like generate_chart.py and changes observable behavior.
Code suggestion
@@ -267,2 +267,2 @@ - return GenerateChartResponse.model_validate(result) - except (CommandException, ValueError, KeyError, AttributeError) as e: + return GenerateChartResponse.model_validate(result) + except Exception as e:
-
Review Details
-
Files reviewed - 4 · Commit Range:
1876a3c..31945f1- superset/mcp_service/chart/tool/update_chart.py
- superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
- superset/mcp_service/dashboard/tool/generate_dashboard.py
- tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.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
| for chart in chart_objects: | ||
| validation = validate_chart_dataset(chart, check_access=True) | ||
| if not validation.is_valid: | ||
| return GenerateDashboardResponse( | ||
| dashboard=None, | ||
| dashboard_url=None, | ||
| error=( | ||
| f"Chart {chart.id} is not accessible: {validation.error}" | ||
| ), | ||
| ) |
There was a problem hiding this comment.
The new validation stops at the first inaccessible chart, but the original code collected all inaccessible charts for better UX. Since generate_dashboard handles multiple charts like generate_chart does, collect all errors to match that pattern and avoid iterative fixes.
Code suggestion
Check the AI-generated fix before applying
| for chart in chart_objects: | |
| validation = validate_chart_dataset(chart, check_access=True) | |
| if not validation.is_valid: | |
| return GenerateDashboardResponse( | |
| dashboard=None, | |
| dashboard_url=None, | |
| error=( | |
| f"Chart {chart.id} is not accessible: {validation.error}" | |
| ), | |
| ) | |
| errors = [] | |
| for chart in chart_objects: | |
| validation = validate_chart_dataset(chart, check_access=True) | |
| if not validation.is_valid: | |
| errors.append(f"Chart {chart.id} is not accessible: {validation.error}") | |
| if errors: | |
| return GenerateDashboardResponse( | |
| dashboard=None, | |
| dashboard_url=None, | |
| error="; ".join(errors), | |
| ) |
Code Review Run #57d487
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Move chart dataset access validation into auth.py as check_chart_data_access(), making it part of the auth layer alongside mcp_auth_hook (class-level RBAC) and has_dataset_access. Tools now import from auth instead of chart_utils directly. This addresses review feedback to use/extend the RBAC system from apache#38407 rather than adding standalone inline checks.
codeant-ai-for-open-source
Bot
added
size:L
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #eb1ed3Actionable 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 |
|
Thanks for the feedback @Antonio-RiveroMartnez! Good question. I moved the data-level check into The reason it's a separate function rather than part of |
github-actions
Bot
added
🍒 6.1.0
2 participants
User description
SUMMARY
Fix two permission gaps in MCP tools:
generate_dashboard: Previously checked chart existence but did NOT verify chart access permissions. Users could create dashboards containing charts they shouldn't have access to. Now callssecurity_manager.can_access_chart()after verifying charts exist, returning an error listing inaccessible chart IDs.update_chart: Previously did not validate dataset access before applying configuration changes. Users could update charts whose underlying dataset they cannot access. Now callsvalidate_chart_dataset(chart, check_access=True)after finding the chart, returning a structuredDatasetNotAccessibleerror before any DB writes.Both fixes reuse existing utilities (
security_manager.can_access_chartandvalidate_chart_dataset) that are already used in other MCP tools likeget_chart_info.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - backend-only changes
TESTING INSTRUCTIONS
test_generate_dashboard_inaccessible_charts- asserts error when user lacks chart accesstest_update_chart_dataset_access_denied- asserts error when dataset is inaccessibletest_update_chart_dataset_not_found- asserts error when dataset is deletedtest_generate_dashboard_basicand all other dashboard generation testsTestUpdateCharttestsADDITIONAL INFORMATION
CodeAnt-AI Description
Block dashboard and chart updates when the user cannot access the underlying dataset
What Changed
Impact
✅ Fewer unauthorized dashboard creations✅ Clearer dataset access errors✅ Safer chart updates💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.