codeant-ai-for-open-source[bot] commented on code in PR #37616:
URL: https://github.com/apache/superset/pull/37616#discussion_r2755619542
##########
superset/extensions/ssh.py:
##########
@@ -38,6 +38,41 @@ def __init__(self, app: Flask) -> None:
sshtunnel.TUNNEL_TIMEOUT = app.config["SSH_TUNNEL_TIMEOUT_SEC"]
sshtunnel.SSH_TIMEOUT = app.config["SSH_TUNNEL_PACKET_TIMEOUT_SEC"]
+ @staticmethod
+ def _load_private_key(key_str: str, password: str | None = None) -> PKey:
+ """
+ Load a private key from a string, automatically detecting the key type.
+
+ Paramiko supports multiple key types (RSA, Ed25519, ECDSA, DSS).
+ This method tries each type until one succeeds, enabling support for
+ modern key formats without hardcoding specific key types.
+
+ :param key_str: Private key content as a string
+ :param password: Optional passphrase for encrypted keys
+ :return: Loaded PKey instance
+ :raises SSHException: If the key cannot be loaded with any supported
type
+ """
+ key_file = StringIO(key_str)
+ # Try key types in order of common usage
+ # RSA: Most common, legacy standard
+ # Ed25519: Modern, recommended by security best practices
+ # ECDSA: Modern elliptic curve
+ # DSS: Legacy DSA keys
+ key_classes = [RSAKey, Ed25519Key, ECDSAKey, DSSKey]
Review Comment:
**Suggestion:** When Paramiko is installed without support for some key
types (for example Ed25519 when `Ed25519Key` is set to None), iterating over
`[RSAKey, Ed25519Key, ECDSAKey, DSSKey]` and calling
`key_class.from_private_key(...)` will raise an AttributeError for the None
entries, aborting key auto-detection instead of simply skipping unsupported
types. Filtering out `None` key classes before the loop ensures that
environments without certain algorithms still work and that the function only
attempts to load actual key classes. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ SSH DB tunnel creation fails (superset/extensions/ssh.py:create_tunnel)
- ⚠️ Environments lacking Paramiko algorithms affected
- ❌ Any private-key-only SSH tunnels may error
- ⚠️ Admins with Ed25519 keys may be blocked
```
</details>
```suggestion
key_classes = [
key_class
for key_class in (RSAKey, Ed25519Key, ECDSAKey, DSSKey)
if key_class is not None
]
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Ensure Paramiko is installed in an environment where some algorithm
classes are
unavailable (e.g., Ed25519 support is absent such that Ed25519Key is None).
The import in
this file is at superset/extensions/ssh.py:24 (`from paramiko import ...
Ed25519Key ...`).
2. Configure a database to use an SSH tunnel with only a private key (no ssh
password).
The code path reaches SSHManager.create_tunnel at
superset/extensions/ssh.py:86 and enters
the branch at lines ~108-112 where `_load_private_key()` is called:
- Call site: superset/extensions/ssh.py:109 `private_key =
self._load_private_key(ssh_tunnel.private_key,
ssh_tunnel.private_key_password)`.
3. Execution enters SSHManager._load_private_key at
superset/extensions/ssh.py:55. The
function builds `key_classes = [RSAKey, Ed25519Key, ECDSAKey, DSSKey]` (line
~61) and
begins iterating over them at line ~63.
4. If one of those imported symbols is None (for example Ed25519Key is
None), the loop
attempts to call `key_class.from_private_key(...)` at
superset/extensions/ssh.py:66 where
`key_class` is None. Because the code only catches SSHException (not
AttributeError),
Python raises AttributeError "'NoneType' object has no attribute
'from_private_key'". This
exception propagates out of `_load_private_key()` causing SSH tunnel
creation to fail at
the call site (superset/extensions/ssh.py:109), blocking DB connection setup.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/extensions/ssh.py
**Line:** 61:61
**Comment:**
*Possible Bug: When Paramiko is installed without support for some key
types (for example Ed25519 when `Ed25519Key` is set to None), iterating over
`[RSAKey, Ed25519Key, ECDSAKey, DSSKey]` and calling
`key_class.from_private_key(...)` will raise an AttributeError for the None
entries, aborting key auto-detection instead of simply skipping unsupported
types. Filtering out `None` key classes before the loop ensures that
environments without certain algorithms still work and that the function only
attempts to load actual key classes.
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]
