codeant-ai-for-open-source[bot] commented on code in PR #41624:
URL: https://github.com/apache/superset/pull/41624#discussion_r3506026929
##########
superset/queries/saved_queries/schemas.py:
##########
@@ -14,9 +14,17 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from marshmallow import fields, Schema
+from flask_babel import lazy_gettext as _
+from marshmallow import fields, Schema, ValidationError
from marshmallow.validate import Length
+
+def validate_label(value: str) -> None:
+ """Reject blank or whitespace-only saved query labels."""
+ if value is None or not value.strip():
+ raise ValidationError(_("Label must not be empty."))
Review Comment:
**Suggestion:** The new blank-label validator is only defined here but not
applied to `ImportV1SavedQuerySchema.label`, so imported saved queries can
still use whitespace-only names and recreate the same broken nameless entries
through the import path. Reuse this validator (or equivalent
trimming/validation) in the import schema so all creation paths enforce the
same rule. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Imported saved queries can still use whitespace-only labels.
- ⚠️ Saved queries list shows nameless rows from imports.
- ⚠️ Inconsistent validation between create and import saved queries.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Prepare a v1 import bundle ZIP containing a `queries/<uuid>.yaml` file
whose contents
match `ImportV1SavedQuerySchema` (fields `catalog`, `schema`, `label`,
`description`,
`sql`, `uuid`, `version`, `database_uuid` as defined in
`superset/queries/saved_queries/schemas.py:49-57`), but set `label` to a
whitespace-only
string, e.g. `" "`.
2. Call the saved query import endpoint `POST /api/v1/saved_query/import/`
with the ZIP
file as multipart form-data; this endpoint is implemented by
`SavedQueryRestApi.import_`
in `superset/queries/saved_queries/api.py:201-210`, which builds `contents`
and
instantiates `ImportSavedQueriesCommand(contents, ...)`.
3. The dispatcher command in
`superset/commands/query/importers/dispatcher.py:35-52`
creates `v1.ImportSavedQueriesCommand`
(`superset/commands/query/importers/v1/__init__.py:33-42`), which wires
`schemas =
{"databases/": ImportV1DatabaseSchema(), "queries/":
ImportV1SavedQuerySchema()}`;
`ImportV1SavedQuerySchema.label` at
`superset/queries/saved_queries/schemas.py:52` uses
only `allow_none=True, validate=Length(0, 256)` and does not call
`validate_label`, so the
whitespace-only `label` passes marshmallow validation unchanged.
4. In `superset/commands/query/importers/v1/utils.py:24-31`,
`import_saved_query(config,
overwrite=...)` calls `SavedQuery.import_from_dict(config, recursive=False)`
without
trimming or validating `config["label"]`, persisting a `SavedQuery` row
whose `label`
column (model at `superset/models/sql_lab.py:32-41`) contains only spaces.
When listing
saved queries via `SavedQueryRestApi` list configuration (`list_columns`
including
`"label"` at `superset/queries/saved_queries/api.py:108-120`), the frontend
renders a
nameless, effectively blank Name cell for this imported query, recreating
the original
nameless-entry behavior through the import path even though direct
create/update now use
`validators_columns = {"label": validate_label}` and `pre_add` stripping at
`superset/queries/saved_queries/api.py:78-87`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1a96e220219d43d2a99a37632135de5f&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=1a96e220219d43d2a99a37632135de5f&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/queries/saved_queries/schemas.py
**Line:** 22:25
**Comment:**
*Incomplete Implementation: The new blank-label validator is only
defined here but not applied to `ImportV1SavedQuerySchema.label`, so imported
saved queries can still use whitespace-only names and recreate the same broken
nameless entries through the import path. Reuse this validator (or equivalent
trimming/validation) in the import schema so all creation paths enforce the
same rule.
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%2F41624&comment_hash=63dd2e0c2d079e32af5c56a89712aadad8b37de018742b2e722ed3caed4e9822&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41624&comment_hash=63dd2e0c2d079e32af5c56a89712aadad8b37de018742b2e722ed3caed4e9822&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]
