codeant-ai-for-open-source[bot] commented on code in PR #41753:
URL: https://github.com/apache/superset/pull/41753#discussion_r3537762382
##########
superset/mcp_service/chart/tool/get_chart_data.py:
##########
@@ -604,6 +623,11 @@ async def get_chart_data( # noqa: C901
)
)
+ # For an embedded guest, attach the dashboard context so
+ # raise_for_access authorizes the data query.
+ if guest_dashboard_id is not None:
+ guest_scope.authorize_query(query_context, guest_dashboard_id,
chart)
Review Comment:
**Suggestion:** The guest path currently fails open when embedded-dashboard
scope cannot be resolved: dataset RBAC pre-check is skipped for every guest,
and query authorization is only injected when `guest_dashboard_id` is present.
If a guest request reaches this point with `guest_dashboard_id` missing, the
query can run without the embedded dashboard guard and rely only on generic
datasource permissions. Treat “guest with no resolved dashboard scope” as an
access error and only skip dataset pre-check when the guest scope is actually
resolved. [security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
❌ Embedded guest assistant may read data beyond token dashboards.
⚠️ Dataset RBAC bypassed when guest scope resolution fails.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start Superset with embedded MCP tools enabled and configure a guest user
whose
security profile allows datasource access (via `get_dataset_access_filters`
in
`superset/utils/filters.py:24-43`) but whose guest token has no DASHBOARD
resources, so
`guest_embedded_dashboard_filter()` in `superset/utils/filters.py:46-83`
returns `None`.
2. As that guest user, resolve a chart directly by identifier through
`ChartDAO.find_by_id()` at `superset/daos/chart.py:349-381`; because
`guest_embedded_dashboard_filter()` returned `None`, `ChartFilter.apply()` at
`superset/charts/filters.py:107-137` falls back to dataset-based filters and
allows the
chart through based solely on datasource permissions.
3. From the same embedded guest context, invoke the MCP tool
`get_chart_data` (`async def
get_chart_data` in
`superset/mcp_service/chart/tool/get_chart_data.py:288-307`) with the
chart’s numeric ID; inside the function, `guest_scope.is_guest_read()`
(`superset/mcp_service/guest_scope.py:35-41`) returns `True`, so the dataset
RBAC
pre-check block at
`superset/mcp_service/chart/tool/get_chart_data.py:414-430` is entirely
skipped for this guest.
4. Still in `get_chart_data`, `guest_dashboard_id =
guest_scope.guest_dashboard_id(chart)`
at `superset/mcp_service/chart/tool/get_chart_data.py:391` returns `None`
because the
chart is not on any guest-authorized dashboard
(`guest_scope.guest_dashboard_id` in
`superset/mcp_service/guest_scope.py:44-54` finds no `has_guest_access`
matches), so the
authorization injection at
`superset/mcp_service/chart/tool/get_chart_data.py:626-629`
never runs; `ChartDataCommand.run()` executes the query without embedded
dashboard context
and without prior dataset validation, meaning the guest can read chart data
purely based
on generic datasource permissions instead of the intended embedded-dashboard
scope.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2a2dc6ee2c3e45bfb065db0a724f3250&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=2a2dc6ee2c3e45bfb065db0a724f3250&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/chart/tool/get_chart_data.py
**Line:** 415:629
**Comment:**
*Security: The guest path currently fails open when embedded-dashboard
scope cannot be resolved: dataset RBAC pre-check is skipped for every guest,
and query authorization is only injected when `guest_dashboard_id` is present.
If a guest request reaches this point with `guest_dashboard_id` missing, the
query can run without the embedded dashboard guard and rely only on generic
datasource permissions. Treat “guest with no resolved dashboard scope” as an
access error and only skip dataset pre-check when the guest scope is actually
resolved.
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%2F41753&comment_hash=7e95e0360462b42f91f0739445130ff15f9d1a731fe5b947a4e55e3f8ecde801&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41753&comment_hash=7e95e0360462b42f91f0739445130ff15f9d1a731fe5b947a4e55e3f8ecde801&reaction=dislike'>👎</a>
##########
superset/mcp_service/chart/tool/get_chart_info.py:
##########
@@ -106,9 +106,13 @@ async def _validate_chart_dataset_access(
Logs any non-fatal warnings (e.g., virtual dataset warnings) via ctx.
"""
from superset.daos.chart import ChartDAO
+ from superset.mcp_service import guest_scope
if not result.id:
return None
+ # Guests read via the dashboard context, not dataset RBAC; skip the
perm-check.
+ if guest_scope.is_guest_read():
+ return None
Review Comment:
**Suggestion:** Skipping dataset access validation for all guests is too
broad because it does not verify that the request is actually bound to a
guest-authorized embedded dashboard context. This creates a contract gap where
guest requests with incomplete/invalid dashboard scoping can still return chart
info. Gate this skip on a successfully resolved guest dashboard scope (or
reject when guest scope cannot be resolved) instead of on `is_guest_read()`
alone. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ Embedded guest chart-info tool exposes metadata beyond dashboards.
⚠️ Guest metadata access not tied to embedded dashboard scope.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure an embedded guest user in Superset whose guest token has either
no DASHBOARD
resources or resources that do not include the target chart’s dashboards, so
`guest_embedded_dashboard_filter()` in `superset/utils/filters.py:46-83`
returns `None`
and `ChartFilter.apply()` in `superset/charts/filters.py:107-137` falls back
to dataset
access filters rather than dashboard scoping.
2. Grant this guest user datasource-level access such that
`get_dataset_access_filters()`
(`superset/utils/filters.py:24-43`) returns a filter matching the chart’s
datasource; then
call `ChartDAO.find_by_id()` from MCP via `ChartInfo` resolution (used in
`ModelGetInfoCore` in `superset/mcp_service/mcp_core.py`, and invoked in
`get_chart_info`
at `superset/mcp_service/chart/tool/get_chart_info.py:319-331`), allowing
the chart to be
resolved based on datasource permissions alone.
3. From the embedded guest context, invoke the MCP tool `get_chart_info`
(`async def
get_chart_info` in
`superset/mcp_service/chart/tool/get_chart_info.py:234-280`) with the
chart’s identifier so that `ModelGetInfoCore.run_tool()` returns a
`ChartInfo` object
describing that chart.
4. After chart resolution, `_validate_chart_dataset_access()` is called at
`superset/mcp_service/chart/tool/get_chart_info.py:353-356`; inside this
helper
(`superset/mcp_service/chart/tool/get_chart_info.py:100-130`),
`guest_scope.is_guest_read()` (`superset/mcp_service/guest_scope.py:35-41`)
returns `True`
and the function immediately returns `None`, skipping the dataset access
validation
entirely, so the guest receives full chart metadata (including datasource
name/type) even
though the request is not proven to be bound to any guest-authorized
dashboard context.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2e2f54e9e2bf4f2fad183a15b78f89c4&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=2e2f54e9e2bf4f2fad183a15b78f89c4&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/chart/tool/get_chart_info.py
**Line:** 114:115
**Comment:**
*Security: Skipping dataset access validation for all guests is too
broad because it does not verify that the request is actually bound to a
guest-authorized embedded dashboard context. This creates a contract gap where
guest requests with incomplete/invalid dashboard scoping can still return chart
info. Gate this skip on a successfully resolved guest dashboard scope (or
reject when guest scope cannot be resolved) instead of on `is_guest_read()`
alone.
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%2F41753&comment_hash=e9352c6da8a1c84ad2263c9a8ce372224d3a7a8f02fd6759cacd5904e3c70673&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41753&comment_hash=e9352c6da8a1c84ad2263c9a8ce372224d3a7a8f02fd6759cacd5904e3c70673&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]
