codeant-ai-for-open-source[bot] commented on code in PR #42492:
URL: https://github.com/apache/superset/pull/42492#discussion_r3661850810
##########
superset/db_engine_specs/presto.py:
##########
@@ -166,6 +166,12 @@ class PrestoBaseEngineSpec(BaseEngineSpec,
metaclass=ABCMeta):
supports_dynamic_schema = True
supports_catalog = supports_dynamic_catalog =
supports_cross_catalog_queries = True
+
+ encrypted_extra_sensitive_fields = {
+ "$.auth_params.password": "Password",
+ "$.auth_params.token": "JWT Token",
+ "$.connect_args.requests_kwargs.jwt": "JWT Token",
+ }
Review Comment:
**Suggestion:** The new class attribute is inherited by `HiveEngineSpec`,
`SparkEngineSpec`, and `DatabricksHiveEngineSpec`, not just Presto and Trino.
This replaces their previous inherited `BaseEngineSpec` default of masking `$.
*` top-level encrypted-extra fields with only these three paths, so unrelated
encrypted-extra values in Hive-family connections will be returned unmasked in
database edit responses. Define this mapping only on `PrestoEngineSpec` and
`TrinoEngineSpec`, or explicitly preserve the appropriate masking contract for
each descendant. [security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Hive-family edit responses can expose encrypted-extra secrets.
- ❌ Spark and Databricks connections lose catch-all masking.
- ⚠️ Existing sensitive values become visible to database editors.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a Hive, Spark, or legacy Databricks Interactive Cluster database
with an
encrypted-extra value outside the three newly declared paths; these engine
specs are
defined at `superset/db_engine_specs/hive.py:88`,
`superset/db_engine_specs/spark.py:44`,
and `superset/db_engine_specs/databricks.py:241`.
2. The inheritance chain makes each of those classes use the new mapping:
`HiveEngineSpec`
inherits `PrestoEngineSpec` at `superset/db_engine_specs/hive.py:88`, while
Spark and
Databricks inherit Hive at `superset/db_engine_specs/spark.py:44` and
`superset/db_engine_specs/databricks.py:241`.
3. Request or edit the database through the database API, which exposes
encrypted-extra
data through the database serialization path;
`Database.masked_encrypted_extra` calls
`self.db_engine_spec.mask_encrypted_extra()` at
`superset/models/core.py:339-341`.
4. `BaseEngineSpec.mask_encrypted_extra()` uses only the descendant's
`encrypted_extra_sensitive_field_paths()` at
`superset/db_engine_specs/base.py:2596-2617`;
because the inherited mapping contains only `$.auth_params.password`,
`$.auth_params.token`, and `$.connect_args.requests_kwargs.jwt`, unrelated
encrypted-extra
values are returned unchanged instead of being masked by the base class's
`$. *` catch-all
defined at `superset/db_engine_specs/base.py:561-563`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=31a2789da3a74f0f8db4fea78a70fae2&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=31a2789da3a74f0f8db4fea78a70fae2&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/db_engine_specs/presto.py
**Line:** 170:174
**Comment:**
*Security: The new class attribute is inherited by `HiveEngineSpec`,
`SparkEngineSpec`, and `DatabricksHiveEngineSpec`, not just Presto and Trino.
This replaces their previous inherited `BaseEngineSpec` default of masking `$.
*` top-level encrypted-extra fields with only these three paths, so unrelated
encrypted-extra values in Hive-family connections will be returned unmasked in
database edit responses. Define this mapping only on `PrestoEngineSpec` and
`TrinoEngineSpec`, or explicitly preserve the appropriate masking contract for
each descendant.
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%2F42492&comment_hash=2792edff3ad01a0a408a3a9d6d59db778fbb2e61d32fb6baec7e6f55dc991866&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42492&comment_hash=2792edff3ad01a0a408a3a9d6d59db778fbb2e61d32fb6baec7e6f55dc991866&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]
