Skip to content

Commit 1d5556d

Browse files
xingyugCopilot
andauthored
fix(ssh): replace private _username with public get_extra_info API (#408)
Replace all usages of asyncssh's private `conn._username` attribute with the public `conn.get_extra_info('username')` API. This decouples us from asyncssh's internal implementation details and prevents potential breakage on future asyncssh upgrades. 5 occurrences replaced in ssh.py, corresponding test mocks updated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 518fab3 commit 1d5556d

4 files changed

Lines changed: 31 additions & 17 deletions

File tree

src/linux_mcp_server/connection/ssh.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,11 @@ async def get_connection(self, host: str) -> asyncssh.SSHClientConnection:
120120
logger.debug(f"SSH_REUSE: {key} | pool_size={len(self._connections)}")
121121
# Use audit log with connection reuse info
122122
log_ssh_connect(
123-
host, username=conn._username, status=Status.success, reused=True, key_path=self._ssh_key
123+
host,
124+
username=conn.get_extra_info("username"),
125+
status=Status.success,
126+
reused=True,
127+
key_path=self._ssh_key,
124128
)
125129
return conn
126130
else:
@@ -156,7 +160,13 @@ async def get_connection(self, host: str) -> asyncssh.SSHClientConnection:
156160
self._connections[key] = conn
157161

158162
# Log successful connection using audit function
159-
log_ssh_connect(host, username=conn._username, status=Status.success, reused=False, key_path=self._ssh_key)
163+
log_ssh_connect(
164+
host,
165+
username=conn.get_extra_info("username"),
166+
status=Status.success,
167+
reused=False,
168+
key_path=self._ssh_key,
169+
)
160170

161171
# DEBUG level: Log pool state
162172
logger.debug(f"SSH_POOL: add_connection | connections={len(self._connections)}")
@@ -234,7 +244,7 @@ async def execute_remote(
234244
},
235245
)
236246
raise ConnectionError(
237-
f"Command timed out after {timeout}s on {conn._username}@{host}: {cmd_str}"
247+
f"Command timed out after {timeout}s on {conn.get_extra_info('username')}@{host}: {cmd_str}"
238248
) from None
239249

240250
return_code = result.exit_status if result.exit_status is not None else 0
@@ -315,14 +325,14 @@ async def get_remote_bin_path(
315325
result = await connection.run(shlex.join(["command", "-v", command]), timeout=timeout)
316326
except asyncssh.Error as err:
317327
raise ConnectionError(
318-
f"Error when trying to locate command '{command}' on {connection._username}@{hostname}: {err}"
328+
f"Error when trying to locate command '{command}' on {connection.get_extra_info('username')}@{hostname}: {err}"
319329
)
320330

321331
if result.exit_status == 0 and result.stdout:
322332
stdout = result.stdout.decode() if isinstance(result.stdout, bytes) else result.stdout
323333
return stdout.strip()
324334

325-
raise FileNotFoundError(f"Unable to find command '{command}' on {connection._username}@{hostname}")
335+
raise FileNotFoundError(f"Unable to find command '{command}' on {connection.get_extra_info('username')}@{hostname}")
326336

327337

328338
async def execute_command(

tests/connection/ssh/test_connection_manager.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66

77
@pytest.fixture
88
def mock_connection(mocker):
9-
mock_connection = mocker.AsyncMock(asyncssh.SSHClientConnection, name="connection", _username="testuser")
9+
mock_connection = mocker.AsyncMock(asyncssh.SSHClientConnection, name="connection")
10+
mock_connection.get_extra_info.return_value = "testuser"
1011
mock_connection.run.return_value = mocker.Mock(exit_status=0, stdout="remote output", stderr="")
1112
mock_connection.is_closed.return_value = False
1213

@@ -55,8 +56,10 @@ async def test_get_connection_different_hosts(mocker, mock_asyncssh_connect):
5556
manager = SSHConnectionManager()
5657
manager._connections.clear()
5758

58-
mock_conn1 = mocker.AsyncMock(asyncssh.SSHClientConnection, return_value=False, _username="testuser")
59-
mock_conn2 = mocker.AsyncMock(asyncssh.SSHClientConnection, return_value=False, _username="testuser")
59+
mock_conn1 = mocker.AsyncMock(asyncssh.SSHClientConnection, return_value=False)
60+
mock_conn1.get_extra_info.return_value = "testuser"
61+
mock_conn2 = mocker.AsyncMock(asyncssh.SSHClientConnection, return_value=False)
62+
mock_conn2.get_extra_info.return_value = "testuser"
6063

6164
async def async_connect(*args, **kwargs):
6265
return mock_conn1 if kwargs.get("host") == "host1" else mock_conn2
@@ -135,12 +138,10 @@ async def test_close_connections(mocker, mock_asyncssh_connect):
135138
manager = SSHConnectionManager()
136139
manager._connections.clear()
137140

138-
mock_conn1 = mocker.AsyncMock(
139-
asyncssh.SSHClientConnection, return_value=False, wait_closed=mocker.AsyncMock(), _username="testuser"
140-
)
141-
mock_conn2 = mocker.AsyncMock(
142-
asyncssh.SSHClientConnection, return_value=False, wait_closed=mocker.AsyncMock(), _username="testuser"
143-
)
141+
mock_conn1 = mocker.AsyncMock(asyncssh.SSHClientConnection, return_value=False, wait_closed=mocker.AsyncMock())
142+
mock_conn1.get_extra_info.return_value = "testuser"
143+
mock_conn2 = mocker.AsyncMock(asyncssh.SSHClientConnection, return_value=False, wait_closed=mocker.AsyncMock())
144+
mock_conn2.get_extra_info.return_value = "testuser"
144145

145146
async def async_connect(*args, **kwargs):
146147
return mock_conn1 if kwargs.get("host") == "host1" else mock_conn2

tests/connection/ssh/test_get_bin_path.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,17 @@ def test_get_bin_path_not_found(mocker):
1212

1313

1414
async def test_get_remote_bin_path_error(mocker):
15-
connection = mocker.Mock(asyncssh.SSHClientConnection, _username="testuser")
15+
connection = mocker.Mock(asyncssh.SSHClientConnection)
16+
connection.get_extra_info.return_value = "testuser"
1617
connection.run = mocker.AsyncMock(side_effect=asyncssh.Error(1, "Raised intentionally"))
1718

1819
with pytest.raises(ConnectionError, match="Raised intentionally"):
1920
await get_remote_bin_path("ls", "host", connection)
2021

2122

2223
async def test_get_remote_bin_not_found(mocker):
23-
connection = mocker.Mock(asyncssh.SSHClientConnection, _username="testuser")
24+
connection = mocker.Mock(asyncssh.SSHClientConnection)
25+
connection.get_extra_info.return_value = "testuser"
2426
connection.run = mocker.AsyncMock(return_value=mocker.Mock(exit_status=0, stdout="", stderr=""))
2527

2628
with pytest.raises(FileNotFoundError, match="Unable to find command"):

tests/connection/ssh/test_timeout.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ def ssh_manager():
4242
@pytest.fixture
4343
def mock_ssh_connection(mocker):
4444
"""Provide a mock SSH connection with asyncssh.connect patched."""
45-
mock_conn = Mock(spec=SSHClientConnection, _username="testuser")
45+
mock_conn = Mock(spec=SSHClientConnection)
46+
mock_conn.get_extra_info.return_value = "testuser"
4647
mock_conn.is_closed.return_value = False
4748

4849
mock_connect = AsyncMock(spec=asyncssh.connect)

0 commit comments

Comments
 (0)