You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Vitor-Avila (via GitHub)" <gi...@apache.org> on 2026/07/28 00:17:34 UTC
[PR] chore: Mask specific Druid/Presto/Trino fields [superset]
Vitor-Avila opened a new pull request, #42492:
URL: https://github.com/apache/superset/pull/42492
### SUMMARY
This PR declares sensitive fields directly in `encrypted_extra_sensitive_fields` for Druid, Presto and Trino. This helps improving Secure Extra to only mask real sensitive fields.
For example, currently if you save a Druid connection with Secure Extra set to:
``` json
{
"connect_args": {
"scheme": "http",
"jwt": "<JWT_TOKEN>"
}
}
```
When editing the connection, the field would show `{"connect_args" XXXXXXXXXX}`. This makes it very difficult to determine what the current configuration is.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No UI changes.
### TESTING INSTRUCTIONS
1. Create a Druid connection with a JWT token set via `{"connect_args": {"scheme": "http","jwt": "<JWT_TOKEN>"}}`.
2. Edit the connection, and confirm that only the `jwt` is escaped.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Mask specific Druid/Presto/Trino fields [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Mask specific Druid/Presto/Trino fields [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42492:
URL: https://github.com/apache/superset/pull/42492#issuecomment-5098351773
<!-- Bito Reply -->
The flagged security issue is correct. By defining `encrypted_extra_sensitive_fields` as a class attribute on `PrestoBaseEngineSpec`, it is inherited by all descendants, including `HiveEngineSpec`, `SparkEngineSpec`, and `DatabricksHiveEngineSpec`. This overrides the default `BaseEngineSpec` behavior, which masks all top-level fields, and restricts masking only to the three paths defined in the new attribute, potentially exposing other sensitive fields in Hive-family connections.
To resolve this, you should move the `encrypted_extra_sensitive_fields` definition from `PrestoBaseEngineSpec` to `PrestoEngineSpec` and `TrinoEngineSpec` individually, or ensure that `BaseEngineSpec` retains its catch-all masking behavior for descendants that do not explicitly override it.
Would you like me to implement this fix and check the rest of the comments on this PR?
**superset/db_engine_specs/presto.py**
```
class PrestoBaseEngineSpec(BaseEngineSpec, metaclass=ABCMeta):
supports_dynamic_schema = True
supports_catalog = supports_dynamic_catalog = supports_cross_catalog_queries = True
# Move encrypted_extra_sensitive_fields to concrete classes instead
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Mask specific Druid/Presto/Trino fields [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #42492:
URL: https://github.com/apache/superset/pull/42492
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Mask specific Druid/Presto/Trino fields [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42492:
URL: https://github.com/apache/superset/pull/42492#issuecomment-5098473642
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42492?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 64.44%. Comparing base ([`1d752a0`](https://app.codecov.io/gh/apache/superset/commit/1d752a0ced6925d0b9b73a303660de497bc30ea1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9eefb28`](https://app.codecov.io/gh/apache/superset/commit/9eefb287f349c32391f0cc628843fcaef0c42f87?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 5 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42492 +/- ##
==========================================
- Coverage 65.26% 64.44% -0.82%
==========================================
Files 2794 2794
Lines 157635 157646 +11
Branches 36049 36049
==========================================
- Hits 102875 101599 -1276
- Misses 52780 54074 +1294
+ Partials 1980 1973 -7
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42492/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/42492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.37% <100.00%> (+<0.01%)` | :arrow_up: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/42492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/42492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/42492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.29% <100.00%> (+<0.01%)` | :arrow_up: |
| [python](https://app.codecov.io/gh/apache/superset/pull/42492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.35% <100.00%> (-1.64%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/42492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.21% <100.00%> (-0.01%)` | :arrow_down: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/42492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42492?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Mask specific Druid/Presto/Trino fields [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42492:
URL: https://github.com/apache/superset/pull/42492#issuecomment-5098318306
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org