codeant-ai-for-open-source[bot] commented on code in PR #41753:
URL: https://github.com/apache/superset/pull/41753#discussion_r3531334223
##########
superset/charts/filters.py:
##########
@@ -106,6 +109,10 @@ def apply(self, query: Query, value: Any) -> Query:
if security_manager.can_access_all_datasources():
return query
+ # Embedded guests: scope to charts on the dashboards in their token.
+ if (guest_dashboards := guest_embedded_dashboard_filter()) is not None:
+ return query.filter(self.model.dashboards.any(guest_dashboards))
+
Review Comment:
**Suggestion:** The guest chart-scope branch assumes
`guest_embedded_dashboard_filter()` always returns a safe criterion, but that
helper switches to UUID filtering when any dashboard resource looks like a UUID
and then applies that UUID `IN` clause to all resource IDs. A mixed guest token
(numeric dashboard IDs plus UUIDs) will push non-UUID values into a UUID column
comparison and can raise a runtime bind/conversion error when this filter
executes. Split guest resources by ID type (UUID vs numeric) and combine both
predicates, instead of routing all IDs through a single UUID branch. [type
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Embedded chart listing fails for mixed-id guest tokens.
- ⚠️ MCP chart-data reads fail for affected embedded guests.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Enable embedded dashboards with the EMBEDDED_SUPERSET feature flag so
`guest_embedded_dashboard_filter()` is active
(superset/utils/filters.py:60-61) and create
both a regular `Dashboard` with integer `id`
(superset/models/dashboard.py:154) and an
`EmbeddedDashboard` row with a UUID primary key
(superset/models/embedded_dashboard.py:39-41) linked to dashboards.
2. Issue an embedded guest token whose `resources` list (typed as
`GuestTokenResources` in
superset/security/guest_token.py:121-130) contains two dashboard entries:
one with
`type="dashboard"` and `id` set to the `EmbeddedDashboard.uuid` value, and
another with
`type="dashboard"` and `id` set to an integer `Dashboard.id`, then
authenticate so
`GuestUser.resources` (security/guest_token.py:188-197) holds this mixed
(UUID plus
numeric) set.
3. Call the chart REST list endpoint served by `ChartRestApi`
(superset/charts/api.py:117-249), for example `GET /api/v1/chart`, which
applies
`ChartFilter` as a base access filter via `base_filters = [["id",
ChartFilter, lambda:
[]]]` (superset/charts/api.py:238) when constructing the SQLAlchemy query
for charts.
4. During `ChartFilter.apply` (superset/charts/filters.py, hunk lines
114-115),
`guest_embedded_dashboard_filter()` (superset/utils/filters.py:46-75)
collects all `id`
values from `guest.resources`, detects at least one UUID via `is_uuid()`
(superset/models/dashboard.py:47-52), and builds the predicate
`EmbeddedDashboard.uuid.in_(ids)` against the UUIDType column
(superset/models/embedded_dashboard.py:41) using the entire mixed `ids`
list; when
SQLAlchemy executes this filter through the
`dashboards.any(guest_dashboards)` condition,
the database attempts to convert non-UUID values (such as "12" or 12) to
UUID, raising a
type-conversion/bind error and causing a 500 error instead of returning
chart data for the
embedded guest.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b7636ccee5784417b09344163a12f5c7&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=b7636ccee5784417b09344163a12f5c7&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/charts/filters.py
**Line:** 114:115
**Comment:**
*Type Error: The guest chart-scope branch assumes
`guest_embedded_dashboard_filter()` always returns a safe criterion, but that
helper switches to UUID filtering when any dashboard resource looks like a UUID
and then applies that UUID `IN` clause to all resource IDs. A mixed guest token
(numeric dashboard IDs plus UUIDs) will push non-UUID values into a UUID column
comparison and can raise a runtime bind/conversion error when this filter
executes. Split guest resources by ID type (UUID vs numeric) and combine both
predicates, instead of routing all IDs through a single UUID branch.
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=06d1ebb00432c9c1c7560375060a0938fe46e84e2963e87b4a24b1c28d7079a0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41753&comment_hash=06d1ebb00432c9c1c7560375060a0938fe46e84e2963e87b4a24b1c28d7079a0&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]
