bito-code-review[bot] commented on code in PR #40673:
URL: https://github.com/apache/superset/pull/40673#discussion_r3344776422
##########
tests/unit_tests/extensions/ssh_test.py:
##########
@@ -34,3 +64,130 @@ def test_ssh_tunnel_timeout_setting() -> None:
factory.init_app(app)
assert sshtunnel.TUNNEL_TIMEOUT == 123.0
assert sshtunnel.SSH_TIMEOUT == 321.0
+
+
+@patch("superset.extensions.ssh.socket.create_connection")
+@patch("superset.extensions.ssh.paramiko.Transport")
+def test_verify_host_key_match(
+ mock_transport_cls: Mock, mock_create_connection: Mock
+) -> None:
+ # The server presents the same key we expect: verification passes.
+ server_key = paramiko.RSAKey.generate(2048)
+ manager = _make_manager(strict=False)
+ tunnel = _ssh_tunnel(_authorized_key(server_key))
+
+ transport = mock_transport_cls.return_value
+ transport.get_remote_server_key.return_value = server_key
+
+ manager._verify_host_key(tunnel) # should not raise
+
+ # The TCP connect is bounded by an explicit timeout, and the resulting
+ # socket is handed to Transport.
+ mock_create_connection.assert_called_once_with(
+ ("ssh.example.com", 22), timeout=321.0
+ )
+
mock_transport_cls.assert_called_once_with(mock_create_connection.return_value)
+ transport.start_client.assert_called_once()
+ transport.close.assert_called_once()
+
+
+@patch("superset.extensions.ssh.socket.create_connection")
+@patch("superset.extensions.ssh.paramiko.Transport")
+def test_verify_host_key_mismatch_raises(
+ mock_transport_cls: Mock, mock_create_connection: Mock
+) -> None:
+ # The server presents a different key than expected: verification fails.
+ expected_key = paramiko.RSAKey.generate(2048)
+ presented_key = paramiko.RSAKey.generate(2048)
+ manager = _make_manager(strict=False)
+ tunnel = _ssh_tunnel(_authorized_key(expected_key))
+
+ transport = mock_transport_cls.return_value
+ transport.get_remote_server_key.return_value = presented_key
+
+ with pytest.raises(SSHTunnelHostKeyVerificationError):
+ manager._verify_host_key(tunnel)
+
+ transport.close.assert_called_once()
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete socket call assertion</b></div>
<div id="fix">
The test `_verify_host_key` on a key mismatch but does not assert
`socket.create_connection` was called. The sibling test
`test_verify_host_key_match` (line 86-88) verifies this call;
`test_verify_host_key_mismatch_raises` should do the same to ensure the socket
is created before the mismatch is detected.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- tests/unit_tests/extensions/ssh_test.py
+++ tests/unit_tests/extensions/ssh_test.py
@@ -107,6 +107,7 @@ def test_verify_host_key_mismatch_raises(
with pytest.raises(SSHTunnelHostKeyVerificationError):
manager._verify_host_key(tunnel)
+ mock_create_connection.assert_called_once()
transport.close.assert_called_once()
```
</div>
</details>
</div>
<small><i>Code Review Run #b513ca</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
