codeant-ai-for-open-source[bot] commented on code in PR #34880:
URL: https://github.com/apache/superset/pull/34880#discussion_r3425031845
##########
superset/dashboards/api.py:
##########
@@ -1959,6 +1963,9 @@ def import_(self) -> Response:
else None
)
overwrite = request.form.get("overwrite") == "true"
+ overwrite_all = parse_boolean_string(
+ request.form.get("overwrite_all", "true" if overwrite else "false")
+ )
Review Comment:
**Suggestion:** The fallback value for `overwrite_all` is inverted for
requests that send `overwrite=true` but omit `overwrite_all`: it currently
defaults to true and will overwrite charts/datasets/databases even when the
caller did not request that. Default this field to false when missing, so
existing API clients and scripts don't unexpectedly perform full asset
overwrites. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Legacy dashboard import scripts may overwrite datasets unexpectedly.
- ❌ Existing database definitions can be silently replaced on import.
- ⚠️ Automation relying on overwrite-only-dashboards semantics breaks.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the dashboard import endpoint `/api/v1/dashboard/import/`
implemented by `import_`
in `superset/dashboards/api.py:88-137`, sending multipart form data with
`overwrite=true`
in `request.form` but omitting the `overwrite_all` field, and upload a ZIP
containing
dashboards that reference existing databases/datasets/charts.
2. Inside `import_`, `overwrite` is computed as
`request.form.get("overwrite") == "true"`
at `superset/dashboards/api.py:106`, yielding `True` for this request.
3. `overwrite_all` is then computed by `overwrite_all =
parse_boolean_string(request.form.get("overwrite_all", "true" if overwrite
else "false"))`
at `superset/dashboards/api.py:107-109`; because `overwrite_all` is missing
and
`overwrite` is true, the default string `"true"` is used and
`parse_boolean_string`
(defined in `superset/utils/core.py:7-33`) returns `True`.
4. `ImportDashboardsCommand` is instantiated with `overwrite_all=True` (see
`superset/dashboards/api.py:127-135`), and its `_import` method computes
`overwrite_assets
= overwrite and kwargs.get("overwrite_all", False)` at
`superset/commands/dashboard/importers/v1/__init__.py:131-133`, causing
`import_database`,
`import_dataset`, and `import_chart` (same file:145-147, 152-159, 169-178)
to be called
with `overwrite=True`, so existing databases/datasets/charts are overwritten
even though
the caller never explicitly requested `overwrite_all`, changing behavior for
legacy
clients that only set `overwrite=true`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=471f6d7382904c4e8c71c9167c378fe4&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=471f6d7382904c4e8c71c9167c378fe4&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/dashboards/api.py
**Line:** 1966:1968
**Comment:**
*Incorrect Condition Logic: The fallback value for `overwrite_all` is
inverted for requests that send `overwrite=true` but omit `overwrite_all`: it
currently defaults to true and will overwrite charts/datasets/databases even
when the caller did not request that. Default this field to false when missing,
so existing API clients and scripts don't unexpectedly perform full asset
overwrites.
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%2F34880&comment_hash=3065cbc3b5c1ab1044fdb0c17c2260542818d788411080cfeff6089a7e5a337b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34880&comment_hash=3065cbc3b5c1ab1044fdb0c17c2260542818d788411080cfeff6089a7e5a337b&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]
