codeant-ai-for-open-source[bot] commented on code in PR #38827:
URL: https://github.com/apache/superset/pull/38827#discussion_r2981391318
##########
superset/mcp_service/dashboard/tool/generate_dashboard.py:
##########
@@ -235,65 +237,92 @@ def generate_dashboard(
else _generate_title_from_charts(chart_objects)
)
- # Prepare dashboard data and create dashboard
- with
event_logger.log_context(action="mcp.generate_dashboard.db_write"):
- dashboard_data: Dict[str, Any] = {
- "dashboard_title": dashboard_title,
- "json_metadata": json.dumps(
- {
- "filter_scopes": {},
- "expanded_slices": {},
- "refresh_frequency": 0,
- "timed_refresh_immune_slices": [],
- "color_scheme": None,
- "label_colors": {},
- "shared_label_colors": {},
- "color_scheme_domain": [],
- "cross_filters_enabled": False,
- "native_filter_configuration": [],
- "global_chart_configuration": {
- "scope": {
- "rootPath": ["ROOT_ID"],
- "excluded": [],
- }
- },
- "chart_configuration": {},
- }
- ),
- "position_json": json.dumps(layout),
- "published": request.published,
- "slices": chart_objects, # Pass ORM objects, not IDs
- }
+ # Create the dashboard directly with db.session instead of using
+ # CreateDashboardCommand. The command's @transaction decorator
+ # may operate in a different SQLAlchemy scoped-session than the
+ # one g.user and chart ORM objects are bound to in the MCP
+ # context, causing "Object is already attached to session X
+ # (this is Y)" errors. By re-querying all ORM objects in the
+ # tool's own db.session we keep everything in a single session.
+ from sqlalchemy.orm import subqueryload
- if request.description:
- dashboard_data["description"] = request.description
+ from superset.models.dashboard import Dashboard
+
+ with
event_logger.log_context(action="mcp.generate_dashboard.db_write"):
+ json_metadata = json.dumps(
+ {
+ "filter_scopes": {},
+ "expanded_slices": {},
+ "refresh_frequency": 0,
+ "timed_refresh_immune_slices": [],
+ "color_scheme": None,
+ "label_colors": {},
+ "shared_label_colors": {},
+ "color_scheme_domain": [],
+ "cross_filters_enabled": False,
+ "native_filter_configuration": [],
+ "global_chart_configuration": {
+ "scope": {
+ "rootPath": ["ROOT_ID"],
+ "excluded": [],
+ }
+ },
+ "chart_configuration": {},
+ }
+ )
- # Create the dashboard using Superset's command pattern
try:
- command = CreateDashboardCommand(dashboard_data)
- dashboard = command.run()
- except Exception as cmd_err:
- # Surface the root cause from @transaction's error wrapping
- root_cause = cmd_err.__cause__ or cmd_err
+ dashboard = Dashboard()
+ dashboard.dashboard_title = dashboard_title
+ dashboard.json_metadata = json_metadata
+ dashboard.position_json = json.dumps(layout)
+ dashboard.published = request.published
+
+ if request.description:
+ dashboard.description = request.description
+
+ # Re-query the current user and charts directly in the
+ # current db.session. g.user was loaded in a Flask
+ # app_context that has since been torn down (the
+ # middleware's ``with flask_app.app_context()`` exits
+ # before the tool function runs), so the User object
+ # is bound to a dead/different scoped session.
+ # Querying fresh avoids all cross-session errors.
+ from superset.extensions import security_manager
+
+ current_user = (
+ db.session.query(security_manager.user_model)
+ .filter_by(id=g.user.id)
+ .first()
+ )
+ if current_user:
+ dashboard.owners = [current_user]
+
+ fresh_charts = (
+ db.session.query(Slice)
+ .filter(Slice.id.in_(request.chart_ids))
+ .order_by(Slice.id)
+ .all()
+ )
+ dashboard.slices = fresh_charts
+
+ db.session.add(dashboard)
+ db.session.commit()
+ except SQLAlchemyError as db_err:
+ db.session.rollback()
logger.error(
- "CreateDashboardCommand failed: %s (cause: %s)",
- cmd_err,
- root_cause,
+ "Dashboard creation failed: %s",
+ db_err,
exc_info=True,
)
return GenerateDashboardResponse(
dashboard=None,
dashboard_url=None,
- error=f"Failed to create dashboard: {root_cause}",
+ error=f"Failed to create dashboard: {db_err}",
Review Comment:
**Suggestion:** Returning raw database exception text to clients leaks
internal SQLAlchemy/database details. Keep full details in logs, but return a
sanitized error message in the API response. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Raw SQLAlchemy details exposed in tool response payload.
- ⚠️ Internal database state leakage to MCP clients.
```
</details>
```suggestion
error="Failed to create dashboard due to a database
error.",
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Trigger DB write failure in `generate_dashboard()` during
`db.session.commit()`
(`superset/mcp_service/dashboard/tool/generate_dashboard.py:310`), causing
`SQLAlchemyError`.
2. Inner handler catches it at lines `311-317` and returns response error
string at line
321 containing raw `{db_err}`.
3. MCP client receives `GenerateDashboardResponse.error` directly from this
function (not
middleware sanitization), exposing backend exception details.
4. This can reveal SQLAlchemy/DB internals (table/transaction/state details)
to external
MCP consumers.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dashboard/tool/generate_dashboard.py
**Line:** 321:321
**Comment:**
*Security: Returning raw database exception text to clients leaks
internal SQLAlchemy/database details. Keep full details in logs, but return a
sanitized error message in the API response.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38827&comment_hash=06325a2b47cb1a3402db0f8bb071964c207ea4f8b7d49bf3f43017e76f836cf6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38827&comment_hash=06325a2b47cb1a3402db0f8bb071964c207ea4f8b7d49bf3f43017e76f836cf6&reaction=dislike'>👎</a>
--
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]
