codeant-ai-for-open-source[bot] commented on code in PR #36958:
URL: https://github.com/apache/superset/pull/36958#discussion_r2670201575
##########
superset/databases/schemas.py:
##########
@@ -888,6 +888,9 @@ def fix_allow_csv_upload(
is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)
external_url = fields.String(allow_none=True)
ssh_tunnel = fields.Nested(DatabaseSSHTunnel, allow_none=True)
+ configuration_method = fields.Enum(
+ ConfigurationMethod, by_value=True, required=False, allow_none=True
Review Comment:
**Suggestion:** Logic bug: the new Enum field has no load_default, so when
`configuration_method` is missing during import it will deserialize to None
instead of the default enum value expected elsewhere; this can cause downstream
logic that compares to enum members to behave incorrectly or miss the intended
default behavior. Add a load_default to ensure a sensible default enum is set
during deserialization. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
ConfigurationMethod,
by_value=True,
required=False,
allow_none=True,
load_default=ConfigurationMethod.SQLALCHEMY_FORM,
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Adding a load_default here is a harmless, sensible improvement: other parts
of the file (DatabaseParametersSchemaMixin) already set a load_default for the
same concept, so making the ImportV1 schema consistent avoids subtle downstream
logic relying on an enum member vs None when the field is absent in older
exports.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/databases/schemas.py
**Line:** 892:892
**Comment:**
*Logic Error: Logic bug: the new Enum field has no load_default, so
when `configuration_method` is missing during import it will deserialize to
None instead of the default enum value expected elsewhere; this can cause
downstream logic that compares to enum members to behave incorrectly or miss
the intended default behavior. Add a load_default to ensure a sensible default
enum is set during deserialization.
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.
```
</details>
##########
superset/models/core.py:
##########
@@ -205,6 +206,7 @@ class Database(CoreDatabase, AuditMixinNullable,
ImportExportMixin): # pylint:
"encrypted_extra",
"impersonate_user",
"ssh_tunnel",
+ "configuration_method",
Review Comment:
**Suggestion:** Allowing `configuration_method` in `extra_import_fields`
lets import payloads set arbitrary strings into the column, which can create
invalid enum-like values in the DB; remove it from `extra_import_fields` so
imports can't inject unvalidated values (or ensure import pipeline validates it
explicitly). [security]
**Severity Level:** Critical 🚨
```suggestion
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Allowing unvalidated values to be imported into a field that is effectively
an enum can lead to invalid DB state and surprising behavior. Removing it from
extra_import_fields or otherwise enforcing validation in the import pipeline
prevents uncontrolled values from being written. This suggestion addresses a
real data-integrity/security concern visible in the diff.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/core.py
**Line:** 209:209
**Comment:**
*Security: Allowing `configuration_method` in `extra_import_fields`
lets import payloads set arbitrary strings into the column, which can create
invalid enum-like values in the DB; remove it from `extra_import_fields` so
imports can't inject unvalidated values (or ensure import pipeline validates it
explicitly).
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.
```
</details>
--
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]
