codeant-ai-for-open-source[bot] commented on code in PR #40575:
URL: https://github.com/apache/superset/pull/40575#discussion_r3332268506
##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -135,13 +143,15 @@ def update_id_refs( # pylint: disable=too-many-locals #
noqa: C901
# fix position
position = fixed.get("position", {})
for child in position.values():
- if (
- isinstance(child, dict)
- and child["type"] == "CHART"
- and "uuid" in child["meta"]
- and child["meta"]["uuid"] in chart_ids
- ):
- child["meta"]["chartId"] = chart_ids[child["meta"]["uuid"]]
+ if not isinstance(child, dict):
+ continue
+ if child.get("type") != "CHART":
+ continue
+ meta = child.get("meta")
+ if not isinstance(meta, dict):
+ continue
+ if "uuid" in meta and meta["uuid"] in chart_ids:
+ meta["chartId"] = chart_ids[meta["uuid"]]
Review Comment:
**Suggestion:** This membership check can raise `TypeError` when
`meta["uuid"]` is an unhashable corrupt value (for example, a dict/list), so
the "defensive" path still crashes on malformed exports. Add a type/hashability
check before probing `chart_ids`. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Assets import API crashes remapping chart IDs on import.
- ⚠️ Dashboard imports with corrupt UUID metadata fail unpredictably.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the assets import endpoint `POST /api/v1/assets/import/` handled by
`ImportExportRestApi.import_` in `superset/importexport/api.py:95-103`,
which constructs
an `ImportAssetsCommand` at `superset/importexport/api.py:228-237` from the
uploaded ZIP
bundle.
2. Include in the bundle a `dashboards/...` YAML whose `position` field has
a CHART entry
with `meta["uuid"]` set to a YAML mapping (Python `dict`) and
`meta["chartId"]` either
missing or explicitly `null`. This configuration is loaded into the `config`
dict passed
to `ImportAssetsCommand._import` in
`superset/commands/importers/v1/assets.py:122-165`.
3. In `ImportAssetsCommand._import`, when processing `dashboards/...`
entries, the code
calls `config = update_id_refs(config, chart_ids, dataset_info)` at
`superset/commands/importers/v1/assets.py:161-165`, invoking
`update_id_refs` in
`superset/commands/dashboard/importers/v1/utils.py:85-99` with the corrupted
`position`.
4. Inside `update_id_refs`, after metadata fixes, the "fix position" loop at
`superset/commands/dashboard/importers/v1/utils.py:143-155` iterates over
`position.values()`. For the corrupted CHART child, `meta` is a dict and the
condition `if
"uuid" in meta and meta["uuid"] in chart_ids:` at line 153 executes. Since
`meta["uuid"]`
is a `dict`, evaluating `meta["uuid"] in chart_ids` raises `TypeError:
unhashable type:
'dict'` before the body runs, crashing the import flow even though earlier
guards were
intended to make the code more defensive.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ecc3302bf75b44d0a7711d8ccc9bf1c0&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=ecc3302bf75b44d0a7711d8ccc9bf1c0&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/commands/dashboard/importers/v1/utils.py
**Line:** 153:154
**Comment:**
*Type Error: This membership check can raise `TypeError` when
`meta["uuid"]` is an unhashable corrupt value (for example, a dict/list), so
the "defensive" path still crashes on malformed exports. Add a type/hashability
check before probing `chart_ids`.
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%2F40575&comment_hash=8179b74fdaf8ba400c407da567039e548204f99d11b9c9e98a224b5e78bf7613&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40575&comment_hash=8179b74fdaf8ba400c407da567039e548204f99d11b9c9e98a224b5e78bf7613&reaction=dislike'>👎</a>
##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -51,15 +51,20 @@ def find_native_filter_datasets(metadata: dict[str, Any])
-> set[str]:
def build_uuid_to_id_map(position: dict[str, Any]) -> dict[str, int]:
- return {
- child["meta"]["uuid"]: child["meta"]["chartId"]
- for child in position.values()
- if (
- isinstance(child, dict)
- and child["type"] == "CHART"
- and "uuid" in child["meta"]
- )
- }
+ result: dict[str, int] = {}
+ for child in position.values():
+ if not isinstance(child, dict):
+ continue
+ if child.get("type") != "CHART":
+ continue
+ meta = child.get("meta")
+ if not isinstance(meta, dict):
+ continue
+ uuid = meta.get("uuid")
+ chart_id = meta.get("chartId")
+ if uuid is not None and chart_id is not None:
+ result[uuid] = chart_id
Review Comment:
**Suggestion:** The new guard still accepts any non-`None` `uuid`, including
unhashable values like dict/list from corrupt payloads. Writing `result[uuid]`
will then raise `TypeError` and break dashboard import. Validate that `uuid` is
a string (or at least hashable) before using it as a dict key. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Assets import API crashes on malformed dashboard UUID meta.
- ⚠️ Dashboard bundles with corrupt chart UUIDs cannot be imported.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Trigger the assets import API endpoint `POST /api/v1/assets/import/`,
which is
implemented by `ImportExportRestApi.import_` in
`superset/importexport/api.py:95-103`.
This endpoint reads the uploaded ZIP bundle and calls
`ImportAssetsCommand(...)` at
`superset/importexport/api.py:228-237`.
2. Ensure the uploaded bundle contains a dashboard V1 YAML under
`dashboards/` whose
`position` field includes a CHART node with `meta["uuid"]` set to a YAML
mapping (parsed
as a Python `dict`) and `meta["chartId"]` set to a valid integer. The
resulting config
dict is passed unchanged into `ImportAssetsCommand._import` in
`superset/commands/importers/v1/assets.py:122-165`.
3. In `ImportAssetsCommand._import`, for each `dashboards/...` file, the
code executes
`config = update_id_refs(config, chart_ids, dataset_info)` at
`superset/commands/importers/v1/assets.py:161-165`. This calls
`update_id_refs` in
`superset/commands/dashboard/importers/v1/utils.py:85-99` with the corrupted
`position`
structure from the bundle.
4. Inside `update_id_refs`, `old_ids =
build_uuid_to_id_map(fixed["position"])` at
`superset/commands/dashboard/importers/v1/utils.py:94` calls
`build_uuid_to_id_map`. In
that function, the corrupted CHART node is processed, `uuid` is a `dict`,
`chart_id` is an
`int`, the condition at line 65 (`if uuid is not None and chart_id is not
None:`) passes,
and line 66 (`result[uuid] = chart_id`) raises `TypeError: unhashable type:
'dict'`,
causing the entire import request to fail with a server error instead of
skipping the
malformed entry.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e00220a2cdbe43928e9a18f8d69f3418&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=e00220a2cdbe43928e9a18f8d69f3418&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/commands/dashboard/importers/v1/utils.py
**Line:** 65:66
**Comment:**
*Type Error: The new guard still accepts any non-`None` `uuid`,
including unhashable values like dict/list from corrupt payloads. Writing
`result[uuid]` will then raise `TypeError` and break dashboard import. Validate
that `uuid` is a string (or at least hashable) before using it as a dict key.
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%2F40575&comment_hash=5de1f5c067519c6f016ca700218ebb82d3c1b81a2840892035acdd1c1f5f3643&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40575&comment_hash=5de1f5c067519c6f016ca700218ebb82d3c1b81a2840892035acdd1c1f5f3643&reaction=dislike'>👎</a>
##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -120,6 +127,7 @@ def update_id_refs( # pylint: disable=too-many-locals #
noqa: C901
metadata["expanded_slices"] = {
str(id_map[int(old_id)]): value
for old_id, value in metadata["expanded_slices"].items()
+ if int(old_id) in id_map
Review Comment:
**Suggestion:** Converting `old_id` with `int(old_id)` in the filter
condition will throw `ValueError` for malformed keys (for example `"foo"`), so
corrupt `expanded_slices` data still crashes import instead of being skipped.
Parse IDs defensively (try/except) and drop non-numeric entries. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Assets import API crashes on malformed expanded_slices metadata.
- ⚠️ Dashboards with corrupt expanded_slices JSON cannot be imported.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Use the assets import REST endpoint `POST /api/v1/assets/import/`
implemented by
`ImportExportRestApi.import_` in `superset/importexport/api.py:95-103`. This
endpoint
unwraps the uploaded ZIP and constructs an `ImportAssetsCommand` at
`superset/importexport/api.py:228-237` with the bundle contents.
2. Ensure the bundle includes a `dashboards/...` YAML whose `metadata` JSON
(stored under
`json_metadata`) contains an `expanded_slices` object with at least one key
that is not a
numeric string, for example `{"foo": true}`. When loaded, this becomes
`metadata["expanded_slices"]` in the `config` dict consumed by
`ImportAssetsCommand._import` in
`superset/commands/importers/v1/assets.py:134-143,161-165`.
3. In `ImportAssetsCommand._import`, for each dashboard file, the code calls
`config =
update_id_refs(config, chart_ids, dataset_info)` at
`superset/commands/importers/v1/assets.py:161-165`, invoking
`update_id_refs` in
`superset/commands/dashboard/importers/v1/utils.py:85-99` with the corrupted
`metadata`
dict and the generated `id_map`.
4. Inside `update_id_refs`, the `"expanded_slices"` block at
`superset/commands/dashboard/importers/v1/utils.py:126-131` executes when
that key is
present. During the dict comprehension, for the non-numeric key
`old_id="foo"`, the filter
condition `if int(old_id) in id_map` at line 130 calls `int("foo")` and
raises
`ValueError: invalid literal for int() with base 10: 'foo'`. This exception
is uncaught,
causing the entire dashboard import to fail instead of simply dropping the
malformed
`expanded_slices` entry.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b6d6c29648124a6f9e94f3ce137ff53d&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=b6d6c29648124a6f9e94f3ce137ff53d&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/commands/dashboard/importers/v1/utils.py
**Line:** 130:130
**Comment:**
*Type Error: Converting `old_id` with `int(old_id)` in the filter
condition will throw `ValueError` for malformed keys (for example `"foo"`), so
corrupt `expanded_slices` data still crashes import instead of being skipped.
Parse IDs defensively (try/except) and drop non-numeric entries.
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%2F40575&comment_hash=2f34855a90e4982f95d7d9d2476b79495f3d404cb365c6524c8e5e443bf1d74b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40575&comment_hash=2f34855a90e4982f95d7d9d2476b79495f3d404cb365c6524c8e5e443bf1d74b&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]
