codeant-ai-for-open-source[bot] commented on code in PR #45890:
URL: https://github.com/apache/superset/pull/45890#discussion_r4423711945
##########
superset/databases/utils.py:
##########
@@ -160,3 +162,86 @@ def make_url_safe(raw_url: str | URL) -> URL:
else:
return raw_url
+
+
+def _is_present(value: Any) -> bool:
+ """Is a credential field actually filled in? Whitespace does not count."""
+ if isinstance(value, str):
+ return bool(value.strip())
+ return bool(value)
+
+
+def validated_embedded_credentials(credentials: Any) -> dict[str, str] | None:
+ """
+ Return a complete embedded fallback credential, or ``None``.
+
+ The canonical shape is ``{username, password}`` or ``{username,
auth_method:
+ "keypair", privatekey_body, privatekey_pass}``; a half-filled credential is
+ ignored rather than half-applied. Engines may override this default through
+ ``BaseEngineSpec.validate_embedded_credentials`` for both query-time and
+ authoring-time validation.
+ """
+ if not isinstance(credentials, dict):
+ return None
+
+ username = credentials.get("username")
+ if not isinstance(username, str) or not _is_present(username):
+ return None
+
+ auth_method = credentials.get("auth_method", "password")
+ if auth_method == "keypair":
+ privatekey_body = credentials.get("privatekey_body")
+ privatekey_pass = credentials.get("privatekey_pass", "")
+ if (
+ not isinstance(privatekey_body, str)
+ or not privatekey_body
+ or not isinstance(privatekey_pass, str)
+ ):
Review Comment:
**Suggestion:** Whitespace-only `privatekey_body` values pass as complete
credentials, so embedded queries attempt authentication with an invalid key
instead of treating the credential as incomplete.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Incorrect condition
logic`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0b8eb18ad76e44a89bda4b1ab8a6b8d4&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=0b8eb18ad76e44a89bda4b1ab8a6b8d4&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/databases/utils.py
**Line:** 195:199
**Comment:**
*Incorrect Condition Logic: Whitespace-only `privatekey_body` values
pass as complete credentials, so embedded queries attempt authentication with
an invalid key instead of treating the credential as incomplete.
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%2F45890&comment_hash=ac3d6a1f646240e1c5aacba1c891ee28657d8e669c160ff8e9fd776997f86790&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45890&comment_hash=ac3d6a1f646240e1c5aacba1c891ee28657d8e669c160ff8e9fd776997f86790&reaction=dislike'>๐</a>
##########
superset/db_engine_specs/base.py:
##########
@@ -2544,6 +2570,67 @@ def impersonate_user(
return url, engine_kwargs
+ @classmethod
+ def validate_embedded_credentials(cls, credentials: Any) -> dict[str, str]
| None:
+ """
+ Return a complete embedded fallback credential, or ``None``.
+
+ Called by ``Database.get_embedded_fallback_credentials`` once its gate
has
+ decided the request is one the fallback governs. The canonical shape is
+ ``{username, password}`` or ``{username, auth_method: "keypair",
+ privatekey_body, privatekey_pass}``; an engine whose driver needs a
+ different shape overrides this. The database API and
parameter-validation
+ command use the same hook when checking credentials before saving.
+ """
+ return validated_embedded_credentials(credentials)
+
+ @classmethod
+ def apply_embedded_credentials(
+ cls,
+ database: Database, # pylint: disable=unused-argument
+ credentials: dict[str, str],
+ url: URL,
+ engine_kwargs: dict[str, Any],
+ ) -> tuple[URL, dict[str, Any]]:
+ """
+ Apply an embedded fallback credential to the connection.
+
+ Called *instead of* :meth:`impersonate_user`, not alongside it -- see
the
+ comment in ``Database._get_sqla_engine`` for why skipping
impersonation is
+ load-bearing twice over.
+
+ The identity is replaced on the URL rather than added to
``connect_args``
+ because the per-process engine cache is keyed on the rendered URL and
the
+ engine kwargs: putting it on the URL is what keeps a guest engine and a
+ logged-in user's engine on separate entries. ``URL.create`` rather than
+ ``URL.set`` so that the base credential is *replaced* -- ``set``
ignores
+ ``password=None``, which would leave a key-pair credential carrying the
+ connection's own password.
+ """
+ url = URL.create(
+ drivername=url.drivername,
+ username=credentials["username"],
+ password=credentials.get("password"),
+ host=url.host,
+ port=url.port,
+ database=url.database,
+ query=url.query,
+ )
+ return url, engine_kwargs
+
+ @classmethod
+ def get_embedded_credential_cache_key(cls, credentials: dict[str, str]) ->
str:
+ """
+ Cache-key material for results produced under an embedded fallback
credential.
+
+ Keyed on the configured identity rather than on the requesting
principal: a
+ ``GuestUser`` carries whatever username the guest token claims, and
whoever
+ mints the token must not get to choose which cache entry they read. All
+ guests on one connection legitimately share an entry -- they do run as
one
+ database user -- and guest-token RLS already separates them.
+ """
+ return f"embedded:{credentials['username']}"
Review Comment:
**Suggestion:** The cache partition contains only the username, so a
replacement Snowflake token restricted to a less-privileged role can reuse
results cached under the previous token.
**Assessment:** ๐ด `Critical` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Cache`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=55915dac362a4359a836ff6f8d8b95eb&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=55915dac362a4359a836ff6f8d8b95eb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/base.py
**Line:** 2632:2632
**Comment:**
*Cache: The cache partition contains only the username, so a
replacement Snowflake token restricted to a less-privileged role can reuse
results cached under the previous token.
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%2F45890&comment_hash=564fa4a89225f9c38f3a16a17cb649e1dd4098dc6a945a76bf76204903fcad82&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45890&comment_hash=564fa4a89225f9c38f3a16a17cb649e1dd4098dc6a945a76bf76204903fcad82&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]
