codeant-ai-for-open-source[bot] commented on code in PR #41472:
URL: https://github.com/apache/superset/pull/41472#discussion_r3532849571
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1784,6 +1784,31 @@ def dashboard_layout_serializer(dashboard: "Dashboard")
-> DashboardLayout:
)
+class DeleteDashboardRequest(BaseModel):
+ """Request schema for delete_dashboard."""
+
+ identifier: int | str = Field(
+ ...,
+ description="Dashboard identifier - numeric ID, UUID string, or slug.",
+ )
Review Comment:
**Suggestion:** `identifier` currently accepts `bool` values because `bool`
is a subclass of `int` in Python, so a request like `{"identifier": true}` will
be coerced to dashboard ID `1` and can delete the wrong dashboard. Reject
booleans explicitly (for example via strict integer typing or a validator) so
only real numeric IDs and strings are accepted. [type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
❌ delete_dashboard may delete unintended dashboard when identifier bool.
⚠️ MCP clients risk deletion from malformed boolean identifiers.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start the MCP service and use a FastMCP client as shown in
`tests/unit_tests/mcp_service/dashboard/tool/test_delete_dashboard.py` lines
64-67, but
call `client.call_tool("delete_dashboard", {"request": {"identifier":
true}})` instead of
passing an integer identifier.
2. The request payload is parsed into `DeleteDashboardRequest` defined in
`superset/mcp_service/dashboard/schemas.py` lines 1787-1793, where
`identifier: int | str
= Field(...)` is declared with default (non-strict) Pydantic behaviour, so a
JSON boolean
`true`/`false` is coerced to an integer `1`/`0` because `bool` is a subclass
of `int`.
3. The `delete_dashboard` tool implementation in
`superset/mcp_service/dashboard/tool/delete_dashboard.py` lines 83-105
receives
`request.identifier` as the coerced integer (e.g., `1`) and calls
`_find_dashboard_by_identifier(request.identifier)` at line 105;
`_find_dashboard_by_identifier` (lines 45-52) checks `isinstance(identifier,
int)` and
then calls `DashboardDAO.find_by_id(int(identifier))`, resolving dashboard
ID `1`.
4. If a dashboard with `id=1` exists (mirroring the `_mock_dashboard` helper
in
`tests/unit_tests/mcp_service/dashboard/tool/test_delete_dashboard.py` lines
52-56 and
typical production data), `DeleteDashboardCommand([dashboard_id]).run()` at
lines 118-121
will permanently delete that dashboard, even though the caller only supplied
a boolean
identifier, demonstrating that a boolean can unintentionally target and
delete the wrong
dashboard.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f0f372684fe14eddbcb677f7ea0999b8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f0f372684fe14eddbcb677f7ea0999b8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dashboard/schemas.py
**Line:** 1790:1793
**Comment:**
*Type Error: `identifier` currently accepts `bool` values because
`bool` is a subclass of `int` in Python, so a request like `{"identifier":
true}` will be coerced to dashboard ID `1` and can delete the wrong dashboard.
Reject booleans explicitly (for example via strict integer typing or a
validator) so only real numeric IDs and strings are accepted.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41472&comment_hash=1017fddd3c8ef7ba41acbaf5f6bfd3679592636e2219539574c6ece1b1b60e42&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41472&comment_hash=1017fddd3c8ef7ba41acbaf5f6bfd3679592636e2219539574c6ece1b1b60e42&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]
