codeant-ai-for-open-source[bot] commented on code in PR #40959:
URL: https://github.com/apache/superset/pull/40959#discussion_r3410560569
##########
tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py:
##########
@@ -625,3 +627,63 @@ def
test_client_warnings_discarded_even_when_server_also_warns(self) -> None:
assert len(req.sanitization_warnings) == 1
assert "dashboard_title" in req.sanitization_warnings[0]
assert "injected" not in req.sanitization_warnings[0]
+
+
+class TestDuplicateDashboardRequestTitleSanitization:
+ """XSS / sanitization behavior for
DuplicateDashboardRequest.dashboard_title."""
+
+ def test_plain_title_passes_without_warning(self) -> None:
Review Comment:
**Suggestion:** Add a short docstring to this newly added test function to
comply with the rule requiring docstrings on new functions. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added test method in the PR and it has no docstring. The
rule requires new Python functions and classes to include docstrings, so the
suggestion correctly identifies a real violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d37f642b69364aa2a9a0ac09e44a0dad&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d37f642b69364aa2a9a0ac09e44a0dad&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:** tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
**Line:** 635:635
**Comment:**
*Custom Rule: Add a short docstring to this newly added test function
to comply with the rule requiring docstrings on new functions.
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%2F40959&comment_hash=db93e779950d776df2e9ed4d90e2bd63aac6f67a719a28147287404c64e8abef&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40959&comment_hash=db93e779950d776df2e9ed4d90e2bd63aac6f67a719a28147287404c64e8abef&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py:
##########
@@ -625,3 +627,63 @@ def
test_client_warnings_discarded_even_when_server_also_warns(self) -> None:
assert len(req.sanitization_warnings) == 1
assert "dashboard_title" in req.sanitization_warnings[0]
assert "injected" not in req.sanitization_warnings[0]
+
+
+class TestDuplicateDashboardRequestTitleSanitization:
+ """XSS / sanitization behavior for
DuplicateDashboardRequest.dashboard_title."""
+
+ def test_plain_title_passes_without_warning(self) -> None:
+ req = DuplicateDashboardRequest(dashboard_id=1,
dashboard_title="Regional Copy")
+ assert req.dashboard_title == "Regional Copy"
+ assert req.sanitization_warnings == []
+
+ def test_title_accepts_aliases(self) -> None:
+ req = DuplicateDashboardRequest(dashboard_id="my-slug", name="From
Name")
+ assert req.dashboard_title == "From Name"
+
+ def test_script_only_title_is_rejected(self) -> None:
+ with pytest.raises(ValidationError, match="removed entirely by
sanitization"):
+ DuplicateDashboardRequest(
+ dashboard_id=1, dashboard_title="<script>alert(1)</script>"
+ )
+
+ def test_empty_title_is_rejected(self) -> None:
+ with pytest.raises(ValidationError):
+ DuplicateDashboardRequest(dashboard_id=1, dashboard_title="")
+
+ def test_partial_strip_emits_warning(self) -> None:
+ req = DuplicateDashboardRequest(
+ dashboard_id=1, dashboard_title="Q1 <b>Review</b>"
+ )
+ assert req.dashboard_title == "Q1 Review"
+ assert len(req.sanitization_warnings) == 1
+ assert "dashboard_title" in req.sanitization_warnings[0]
+
+ def test_client_supplied_warnings_are_discarded(self) -> None:
+ """``sanitization_warnings`` is server-only; client input is
dropped."""
+ req = DuplicateDashboardRequest(
+ dashboard_id=1,
+ dashboard_title="Plain Title",
+ sanitization_warnings=["<script>fake notice</script>"],
+ )
+ assert req.sanitization_warnings == []
+
+
+class TestDuplicateDashboardResponse:
+ """Serialization and error sanitization for DuplicateDashboardResponse."""
+
+ def test_defaults(self) -> None:
Review Comment:
**Suggestion:** Add a brief docstring for this new test method so the
function is documented inline. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new test method lacks a docstring. The custom rule explicitly requires
docstrings on newly added Python functions, so the suggestion is verified.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f7fb4108d1ab45b7a38f895f6527d74c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f7fb4108d1ab45b7a38f895f6527d74c&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:** tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
**Line:** 675:675
**Comment:**
*Custom Rule: Add a brief docstring for this new test method so the
function is documented inline.
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%2F40959&comment_hash=89bf0f5a795d7e92cfb4c38615c53912a82d454a76e81df8d63f7a94e0ff8106&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40959&comment_hash=89bf0f5a795d7e92cfb4c38615c53912a82d454a76e81df8d63f7a94e0ff8106&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py:
##########
@@ -625,3 +627,63 @@ def
test_client_warnings_discarded_even_when_server_also_warns(self) -> None:
assert len(req.sanitization_warnings) == 1
assert "dashboard_title" in req.sanitization_warnings[0]
assert "injected" not in req.sanitization_warnings[0]
+
+
+class TestDuplicateDashboardRequestTitleSanitization:
+ """XSS / sanitization behavior for
DuplicateDashboardRequest.dashboard_title."""
+
+ def test_plain_title_passes_without_warning(self) -> None:
+ req = DuplicateDashboardRequest(dashboard_id=1,
dashboard_title="Regional Copy")
+ assert req.dashboard_title == "Regional Copy"
+ assert req.sanitization_warnings == []
+
+ def test_title_accepts_aliases(self) -> None:
Review Comment:
**Suggestion:** Add an inline docstring describing the alias behavior being
validated so the new function is documented. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added test method does not include a docstring. That matches the
stated rule requiring docstrings for new functions, so this is a verified
violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f20a4a92594f4e98a71caffd7a708cc8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f20a4a92594f4e98a71caffd7a708cc8&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:** tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
**Line:** 640:640
**Comment:**
*Custom Rule: Add an inline docstring describing the alias behavior
being validated so the new function is documented.
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%2F40959&comment_hash=e07d577628a2ddd30cd2bb56c9707988fd0f4ba2a3ebd72302e6fa9e9249c000&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40959&comment_hash=e07d577628a2ddd30cd2bb56c9707988fd0f4ba2a3ebd72302e6fa9e9249c000&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py:
##########
@@ -625,3 +627,63 @@ def
test_client_warnings_discarded_even_when_server_also_warns(self) -> None:
assert len(req.sanitization_warnings) == 1
assert "dashboard_title" in req.sanitization_warnings[0]
assert "injected" not in req.sanitization_warnings[0]
+
+
+class TestDuplicateDashboardRequestTitleSanitization:
+ """XSS / sanitization behavior for
DuplicateDashboardRequest.dashboard_title."""
+
+ def test_plain_title_passes_without_warning(self) -> None:
+ req = DuplicateDashboardRequest(dashboard_id=1,
dashboard_title="Regional Copy")
+ assert req.dashboard_title == "Regional Copy"
+ assert req.sanitization_warnings == []
+
+ def test_title_accepts_aliases(self) -> None:
+ req = DuplicateDashboardRequest(dashboard_id="my-slug", name="From
Name")
+ assert req.dashboard_title == "From Name"
+
+ def test_script_only_title_is_rejected(self) -> None:
Review Comment:
**Suggestion:** Add a concise docstring explaining the rejection case being
tested to satisfy the new-function docstring requirement. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The method is newly added and has no docstring. Since the rule says new
Python functions should be documented inline, this is a real violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9a605d3c226e4442972166aa84c7d794&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9a605d3c226e4442972166aa84c7d794&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:** tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
**Line:** 644:644
**Comment:**
*Custom Rule: Add a concise docstring explaining the rejection case
being tested to satisfy the new-function docstring requirement.
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%2F40959&comment_hash=fd2307080ad987e0ee28619554437bdcb816d5ec7d81923a0ad4abfa6d5dcf47&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40959&comment_hash=fd2307080ad987e0ee28619554437bdcb816d5ec7d81923a0ad4abfa6d5dcf47&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py:
##########
@@ -0,0 +1,361 @@
+# 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.
+
+"""
+Unit tests for the duplicate_dashboard MCP tool.
+
+Follows the same pattern used in test_add_chart_to_existing_dashboard.py:
+- Tests run through the async MCP Client (not direct function calls)
+- Patches applied at source locations (superset.daos.dashboard.*,
+ superset.commands.dashboard.copy.*)
+- auth is mocked via the autouse mock_auth fixture
+
+Covers:
+- Duplicate referencing the same charts (duplicate_slices=False, default)
+- Duplicate with deep-copied charts (duplicate_slices=True)
+- Source dashboard not found
+- Source dashboard access denied / copy forbidden
+- Title sanitization (XSS stripped, XSS-only title rejected)
+"""
+
+import logging
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+from superset.utils import json
+
+logging.basicConfig(level=logging.DEBUG)
+logger = logging.getLogger(__name__)
+
+
+# ---------------------------------------------------------------------------
+# Fixtures
+# ---------------------------------------------------------------------------
+
+
[email protected]
+def mcp_server() -> object:
+ """Return the FastMCP app instance for use in MCP client tests."""
+ return mcp
+
+
[email protected](autouse=True)
+def mock_auth():
Review Comment:
**Suggestion:** Add an explicit return type annotation to `mock_auth` so the
new fixture function is fully typed according to the rule for new Python code.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python function in a new test file, and it lacks a
return type annotation. The custom rule requires new Python code to be fully
typed, so the suggestion correctly identifies a real violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=af302f9f4f994da0a1bf6f5aff0e693a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=af302f9f4f994da0a1bf6f5aff0e693a&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:**
tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py
**Line:** 60:60
**Comment:**
*Custom Rule: Add an explicit return type annotation to `mock_auth` so
the new fixture function is fully typed according to the rule for new Python
code.
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%2F40959&comment_hash=f95257b63b03277396b3d8e24eee395a221ad290a2bc745addd260c0d1e2b338&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40959&comment_hash=f95257b63b03277396b3d8e24eee395a221ad290a2bc745addd260c0d1e2b338&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py:
##########
@@ -625,3 +627,63 @@ def
test_client_warnings_discarded_even_when_server_also_warns(self) -> None:
assert len(req.sanitization_warnings) == 1
assert "dashboard_title" in req.sanitization_warnings[0]
assert "injected" not in req.sanitization_warnings[0]
+
+
+class TestDuplicateDashboardRequestTitleSanitization:
+ """XSS / sanitization behavior for
DuplicateDashboardRequest.dashboard_title."""
+
+ def test_plain_title_passes_without_warning(self) -> None:
+ req = DuplicateDashboardRequest(dashboard_id=1,
dashboard_title="Regional Copy")
+ assert req.dashboard_title == "Regional Copy"
+ assert req.sanitization_warnings == []
+
+ def test_title_accepts_aliases(self) -> None:
+ req = DuplicateDashboardRequest(dashboard_id="my-slug", name="From
Name")
+ assert req.dashboard_title == "From Name"
+
+ def test_script_only_title_is_rejected(self) -> None:
+ with pytest.raises(ValidationError, match="removed entirely by
sanitization"):
+ DuplicateDashboardRequest(
+ dashboard_id=1, dashboard_title="<script>alert(1)</script>"
+ )
+
+ def test_empty_title_is_rejected(self) -> None:
+ with pytest.raises(ValidationError):
+ DuplicateDashboardRequest(dashboard_id=1, dashboard_title="")
+
+ def test_partial_strip_emits_warning(self) -> None:
+ req = DuplicateDashboardRequest(
+ dashboard_id=1, dashboard_title="Q1 <b>Review</b>"
+ )
+ assert req.dashboard_title == "Q1 Review"
+ assert len(req.sanitization_warnings) == 1
+ assert "dashboard_title" in req.sanitization_warnings[0]
+
+ def test_client_supplied_warnings_are_discarded(self) -> None:
+ """``sanitization_warnings`` is server-only; client input is
dropped."""
+ req = DuplicateDashboardRequest(
+ dashboard_id=1,
+ dashboard_title="Plain Title",
+ sanitization_warnings=["<script>fake notice</script>"],
+ )
+ assert req.sanitization_warnings == []
+
+
+class TestDuplicateDashboardResponse:
+ """Serialization and error sanitization for DuplicateDashboardResponse."""
+
+ def test_defaults(self) -> None:
+ resp = DuplicateDashboardResponse()
+ assert resp.dashboard is None
+ assert resp.dashboard_url is None
+ assert resp.duplicated_slices is False
+ assert resp.error is None
+ assert resp.warnings == []
+
+ def test_error_is_wrapped_for_llm_context(self) -> None:
Review Comment:
**Suggestion:** Add a docstring describing the error-wrapping assertion to
document this newly added function. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This added test method has no docstring in the final file state. That
directly violates the rule for newly added Python functions/classes to be
documented inline.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ae21d9a124e34c1e8741d12e544559a9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ae21d9a124e34c1e8741d12e544559a9&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:** tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
**Line:** 683:683
**Comment:**
*Custom Rule: Add a docstring describing the error-wrapping assertion
to document this newly added function.
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%2F40959&comment_hash=5d2780a77e3b8271fde52a96868da2f145e415ea4721fdf4be914660127a0a4a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40959&comment_hash=5d2780a77e3b8271fde52a96868da2f145e415ea4721fdf4be914660127a0a4a&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]
