From 7b60c5f34c096b3dd9f9d57547a14a6ced932d7b Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:44:58 +0000 Subject: [PATCH 01/16] fix(sftp): handle Bitvise strict SFTP compliance for directory navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bitvise SSH Server negotiates SFTP v4+ where file type is communicated via a separate `type` field rather than embedded in the Unix permission bits. The existing code only checked `attrs.permissions` with `stat.S_ISDIR()`, which returns False when the file type bits are absent — causing directories like .ssh to appear as regular files and making navigation silently fail. Changes: - list_dir: check attrs.type (SFTP v4+ type field) as fallback when permissions lack file type bits, for both directories and symlinks - chdir: validate target is a directory via stat() after realpath() instead of blindly trusting the canonicalised path - stat: use attrs.type fallback for is_dir detection Closes #46 Co-Authored-By: Claude Opus 4.6 --- src/portkeydrop/protocols.py | 32 +++++- tests/test_protocols.py | 5 + tests/test_sftp_client.py | 187 +++++++++++++++++++++++++++++++++++ 3 files changed, 223 insertions(+), 1 deletion(-) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index 4efa4fc..c5b6979 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -21,6 +21,10 @@ logger = logging.getLogger(__name__) +# asyncssh SFTP v4+ file type constants (avoids import at module level) +_SFTP_TYPE_DIRECTORY = 2 +_SFTP_TYPE_SYMLINK = 3 + ProgressCallback = Callable[[int, int], None] # (bytes_transferred, total_bytes) @@ -597,7 +601,13 @@ def list_dir(self, path: str = ".") -> list[RemoteFile]: continue is_dir = bool(mode is not None and stat.S_ISDIR(mode)) full_path = f"{target.rstrip('/')}/{name}" + # SFTP v4+ file type field (separate from permissions) — used by + # strict servers like Bitvise that may not embed the type in the + # permission bits. + sftp_type = getattr(attrs, "type", None) is_link = bool(mode is not None and stat.S_ISLNK(mode)) + if not is_link and sftp_type == _SFTP_TYPE_SYMLINK: + is_link = True if is_link: try: target_attrs = self._run(sftp.stat(full_path)) @@ -605,8 +615,13 @@ def list_dir(self, path: str = ".") -> list[RemoteFile]: target_attrs.permissions ): is_dir = True + elif getattr(target_attrs, "type", None) == _SFTP_TYPE_DIRECTORY: + is_dir = True except Exception: pass + # Fallback: use SFTP v4+ type field when permissions lack type bits + if not is_dir and sftp_type == _SFTP_TYPE_DIRECTORY: + is_dir = True longname = getattr(entry, "longname", "") if not is_dir and longname and longname.startswith("d"): is_dir = True @@ -639,8 +654,21 @@ def chdir(self, path: str) -> str: logger.debug("chdir: '%s'", path) sftp = self._ensure_connected() try: - self._cwd = self._run(sftp.realpath(path)) + resolved = self._run(sftp.realpath(path)) + # Validate the target is a directory (strict servers like Bitvise + # require an explicit stat check — realpath only canonicalises). + attrs = self._run(sftp.stat(resolved)) + is_dir = False + if attrs.permissions is not None and stat.S_ISDIR(attrs.permissions): + is_dir = True + elif getattr(attrs, "type", None) == _SFTP_TYPE_DIRECTORY: + is_dir = True + if not is_dir: + raise NotADirectoryError(f"Not a directory: '{path}'") + self._cwd = resolved logger.debug("chdir: done, cwd='%s'", self._cwd) + except (NotADirectoryError, PermissionError): + raise except OSError as e: import errno as _errno @@ -749,6 +777,8 @@ def stat(self, path: str) -> RemoteFile: attrs = self._run(sftp.stat(path)) mode = attrs.permissions is_dir = stat.S_ISDIR(mode) if mode else False + if not is_dir and getattr(attrs, "type", None) == _SFTP_TYPE_DIRECTORY: + is_dir = True modified = datetime.fromtimestamp(attrs.mtime) if attrs.mtime else None perms = stat.filemode(mode) if mode else "" name = PurePosixPath(path).name diff --git a/tests/test_protocols.py b/tests/test_protocols.py index 7be6d81..36a1486 100644 --- a/tests/test_protocols.py +++ b/tests/test_protocols.py @@ -445,6 +445,11 @@ def test_chdir_download_upload_and_file_ops(self, mock_connect): mock_conn = AsyncMock() mock_sftp = AsyncMock() mock_sftp.realpath.side_effect = ["/", "/uploads"] + # chdir now validates with stat — return directory attributes + chdir_stat_attrs = MagicMock() + chdir_stat_attrs.permissions = stat_mod.S_IFDIR | 0o755 + chdir_stat_attrs.type = None + mock_sftp.stat.return_value = chdir_stat_attrs mock_conn.start_sftp_client.return_value = mock_sftp mock_connect.return_value = mock_conn diff --git a/tests/test_sftp_client.py b/tests/test_sftp_client.py index de02671..c476577 100644 --- a/tests/test_sftp_client.py +++ b/tests/test_sftp_client.py @@ -385,6 +385,10 @@ def _make_connected(self, sftp_info: ConnectionInfo) -> tuple[SFTPClient, AsyncM client = SFTPClient(sftp_info) mock_sftp = AsyncMock() mock_sftp.realpath.return_value = "/home/user/subdir" + stat_attrs = MagicMock() + stat_attrs.permissions = stat_mod.S_IFDIR | 0o755 + stat_attrs.type = None + mock_sftp.stat.return_value = stat_attrs client._conn = AsyncMock() client._sftp = mock_sftp client._cwd = "/home/user" @@ -396,6 +400,22 @@ def test_chdir_updates_cwd(self, sftp_info: ConnectionInfo) -> None: assert result == "/home/user/subdir" assert client._cwd == "/home/user/subdir" + def test_chdir_validates_directory_with_stat(self, sftp_info: ConnectionInfo) -> None: + client, mock_sftp = self._make_connected(sftp_info) + client.chdir("/home/user/subdir") + mock_sftp.stat.assert_awaited_once_with("/home/user/subdir") + + def test_chdir_rejects_non_directory(self, sftp_info: ConnectionInfo) -> None: + client, mock_sftp = self._make_connected(sftp_info) + stat_attrs = MagicMock() + stat_attrs.permissions = stat_mod.S_IFREG | 0o644 + stat_attrs.type = None + mock_sftp.stat.return_value = stat_attrs + + with pytest.raises(NotADirectoryError, match="Not a directory"): + client.chdir("/home/user/file.txt") + assert client._cwd == "/home/user" # unchanged + def test_chdir_permission_error(self, sftp_info: ConnectionInfo) -> None: import errno @@ -452,3 +472,170 @@ def test_oserror_reraises(self, sftp_info: ConnectionInfo) -> None: with pytest.raises(IOError): client.list_dir("/gone") + + +# SFTP v4+ file type constant (matches protocols._SFTP_TYPE_DIRECTORY) +_SFTP_TYPE_DIRECTORY = 2 +_SFTP_TYPE_SYMLINK = 3 +_SFTP_TYPE_REGULAR = 1 +_SFTP_TYPE_UNKNOWN = 5 + + +class TestSFTPBitviseCompliance: + """Tests for strict SFTP servers (Bitvise) that send file type in the + SFTP v4+ ``type`` field rather than (or in addition to) permission bits.""" + + def _make_connected(self, sftp_info: ConnectionInfo) -> tuple[SFTPClient, AsyncMock]: + client = SFTPClient(sftp_info) + mock_sftp = AsyncMock() + mock_sftp.realpath.return_value = "/home/user" + client._conn = AsyncMock() + client._sftp = mock_sftp + client._cwd = "/home/user" + return client, mock_sftp + + # ---- list_dir: type field fallback ---- + + def test_list_dir_detects_dir_via_sftp_type_when_permissions_none( + self, sftp_info: ConnectionInfo + ) -> None: + """Bitvise may return type=DIRECTORY with permissions=None.""" + client, mock_sftp = self._make_connected(sftp_info) + entry = MagicMock() + entry.filename = ".ssh" + entry.attrs = MagicMock() + entry.attrs.permissions = None + entry.attrs.type = _SFTP_TYPE_DIRECTORY + entry.attrs.size = 0 + entry.attrs.mtime = 0 + entry.attrs.uid = 1000 + entry.attrs.gid = 1000 + entry.longname = "" + mock_sftp.readdir.return_value = [entry] + + files = client.list_dir() + assert len(files) == 1 + assert files[0].name == ".ssh" + assert files[0].is_dir is True + + def test_list_dir_detects_dir_via_sftp_type_when_permissions_lack_type_bits( + self, sftp_info: ConnectionInfo + ) -> None: + """Bitvise may return permissions=0o700 without S_IFDIR bits + type=DIRECTORY.""" + client, mock_sftp = self._make_connected(sftp_info) + entry = MagicMock() + entry.filename = ".ssh" + entry.attrs = MagicMock() + entry.attrs.permissions = 0o700 # No S_IFDIR prefix + entry.attrs.type = _SFTP_TYPE_DIRECTORY + entry.attrs.size = 0 + entry.attrs.mtime = 0 + entry.attrs.uid = 1000 + entry.attrs.gid = 1000 + entry.longname = "" + mock_sftp.readdir.return_value = [entry] + + files = client.list_dir() + assert len(files) == 1 + assert files[0].name == ".ssh" + assert files[0].is_dir is True + + def test_list_dir_file_with_type_regular_stays_file(self, sftp_info: ConnectionInfo) -> None: + """File with type=REGULAR and no permission type bits stays a file.""" + client, mock_sftp = self._make_connected(sftp_info) + entry = MagicMock() + entry.filename = "readme.txt" + entry.attrs = MagicMock() + entry.attrs.permissions = 0o644 + entry.attrs.type = _SFTP_TYPE_REGULAR + entry.attrs.size = 100 + entry.attrs.mtime = 0 + entry.attrs.uid = 1000 + entry.attrs.gid = 1000 + entry.longname = "" + mock_sftp.readdir.return_value = [entry] + + files = client.list_dir() + assert len(files) == 1 + assert files[0].is_dir is False + + def test_list_dir_symlink_to_dir_via_sftp_type(self, sftp_info: ConnectionInfo) -> None: + """Symlink with type=SYMLINK that resolves to a dir via stat type field.""" + client, mock_sftp = self._make_connected(sftp_info) + entry = MagicMock() + entry.filename = "link-to-dir" + entry.attrs = MagicMock() + entry.attrs.permissions = None + entry.attrs.type = _SFTP_TYPE_SYMLINK + entry.attrs.size = 0 + entry.attrs.mtime = 0 + entry.attrs.uid = 1000 + entry.attrs.gid = 1000 + entry.longname = "" + + target_attrs = MagicMock() + target_attrs.permissions = None + target_attrs.type = _SFTP_TYPE_DIRECTORY + mock_sftp.stat.return_value = target_attrs + mock_sftp.readdir.return_value = [entry] + + files = client.list_dir() + assert len(files) == 1 + assert files[0].is_dir is True + + # ---- chdir: directory validation ---- + + def test_chdir_accepts_dir_via_sftp_type(self, sftp_info: ConnectionInfo) -> None: + """chdir succeeds when stat returns type=DIRECTORY with permissions=None.""" + client, mock_sftp = self._make_connected(sftp_info) + mock_sftp.realpath.return_value = "/home/user/.ssh" + stat_attrs = MagicMock() + stat_attrs.permissions = None + stat_attrs.type = _SFTP_TYPE_DIRECTORY + mock_sftp.stat.return_value = stat_attrs + + result = client.chdir("/home/user/.ssh") + assert result == "/home/user/.ssh" + assert client._cwd == "/home/user/.ssh" + + def test_chdir_accepts_dir_with_permissions_only(self, sftp_info: ConnectionInfo) -> None: + """chdir succeeds when stat returns S_IFDIR in permissions (standard Linux).""" + client, mock_sftp = self._make_connected(sftp_info) + mock_sftp.realpath.return_value = "/home/user/docs" + stat_attrs = MagicMock() + stat_attrs.permissions = stat_mod.S_IFDIR | 0o755 + stat_attrs.type = _SFTP_TYPE_UNKNOWN + mock_sftp.stat.return_value = stat_attrs + + result = client.chdir("/home/user/docs") + assert result == "/home/user/docs" + + def test_chdir_stat_permission_error_surfaces(self, sftp_info: ConnectionInfo) -> None: + """chdir raises PermissionError when stat fails with EACCES.""" + import errno + + client, mock_sftp = self._make_connected(sftp_info) + mock_sftp.realpath.return_value = "/home/user/.ssh" + err = IOError() + err.errno = errno.EACCES + mock_sftp.stat.side_effect = err + + with pytest.raises(PermissionError, match="Permission denied"): + client.chdir("/home/user/.ssh") + assert client._cwd == "/home/user" # unchanged + + # ---- stat: type field fallback ---- + + def test_stat_detects_dir_via_sftp_type(self, sftp_info: ConnectionInfo) -> None: + """stat() returns is_dir=True when type=DIRECTORY and permissions=None.""" + client, mock_sftp = self._make_connected(sftp_info) + stat_attrs = MagicMock() + stat_attrs.permissions = None + stat_attrs.type = _SFTP_TYPE_DIRECTORY + stat_attrs.size = 0 + stat_attrs.mtime = 0 + mock_sftp.stat.return_value = stat_attrs + + remote = client.stat("/home/user/.ssh") + assert remote.is_dir is True + assert remote.name == ".ssh" From cb9c2eefa1e7f9a64d3509325fdc7f75a345f321 Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:50:47 +0000 Subject: [PATCH 02/16] fix(sftp): assume directory when server returns no type info (Bitvise .ssh) --- src/portkeydrop/protocols.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index c5b6979..08b8175 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -663,6 +663,10 @@ def chdir(self, path: str) -> str: is_dir = True elif getattr(attrs, "type", None) == _SFTP_TYPE_DIRECTORY: is_dir = True + elif attrs.permissions is None and getattr(attrs, "type", None) is None: + # Server returned no type info (e.g. Bitvise on .ssh) — + # assume directory and let the server reject if wrong. + is_dir = True if not is_dir: raise NotADirectoryError(f"Not a directory: '{path}'") self._cwd = resolved From 78711b3ef3366b3406398a14a79f42f45dd45057 Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:54:13 +0000 Subject: [PATCH 03/16] debug(sftp): add readdir/stat logging to diagnose Bitvise .ssh issue --- src/portkeydrop/protocols.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index 08b8175..ede7d30 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -575,6 +575,7 @@ def list_dir(self, path: str = ".") -> list[RemoteFile]: logger.debug("list_dir: requesting entries for '%s'", target) try: entries = self._run(sftp.readdir(target)) + logger.debug("list_dir: readdir returned %d raw entries for '%s'", len(entries) if entries is not None else -1, target) except PermissionError: raise except OSError as e: @@ -658,6 +659,7 @@ def chdir(self, path: str) -> str: # Validate the target is a directory (strict servers like Bitvise # require an explicit stat check — realpath only canonicalises). attrs = self._run(sftp.stat(resolved)) + logger.debug("chdir stat: path='%s' permissions=%s type=%s", resolved, attrs.permissions, getattr(attrs, "type", None)) is_dir = False if attrs.permissions is not None and stat.S_ISDIR(attrs.permissions): is_dir = True @@ -666,6 +668,7 @@ def chdir(self, path: str) -> str: elif attrs.permissions is None and getattr(attrs, "type", None) is None: # Server returned no type info (e.g. Bitvise on .ssh) — # assume directory and let the server reject if wrong. + logger.debug("chdir: no type info from server, assuming directory for '%s'", resolved) is_dir = True if not is_dir: raise NotADirectoryError(f"Not a directory: '{path}'") From 6442b014c666572e58c1ae4838ea71d80c5218de Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:56:27 +0000 Subject: [PATCH 04/16] fix(sftp): catch asyncssh.SFTPError in list_dir (Bitvise permission errors) --- src/portkeydrop/protocols.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index ede7d30..397ff69 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -584,6 +584,15 @@ def list_dir(self, path: str = ".") -> list[RemoteFile]: if e.errno in (_errno.EACCES, _errno.EPERM): raise PermissionError(f"Permission denied: cannot list '{target}'") from e raise + except Exception as e: + # asyncssh raises SFTPError (not OSError) for server-side errors. + # Map permission errors; surface everything else. + import asyncssh as _asyncssh + if isinstance(e, _asyncssh.SFTPError): + logger.warning("list_dir: SFTPError for '%s': code=%s msg=%s", target, getattr(e, "code", "?"), e) + if getattr(e, "code", None) in (3, 4): # FX_PERMISSION_DENIED=3, FX_FAILURE=4 + raise PermissionError(f"Permission denied: cannot list '{target}'") from e + raise logger.debug("list_dir: got %d entries for '%s'", len(entries), target) for entry in entries: name = entry.filename From aca15b495362aa0223c7251adefd6d72e4b70783 Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:00:59 +0000 Subject: [PATCH 05/16] fix(sftp): replace readdir with _readdir_safe to handle Bitvise count=0 EOF quirk --- src/portkeydrop/protocols.py | 37 +++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index 397ff69..0e14780 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -568,13 +568,48 @@ def disconnect(self) -> None: # Directory operations # ------------------------------------------------------------------ + @staticmethod + async def _readdir_safe(sftp, path: str): + """Readdir loop that treats consecutive empty responses as EOF. + + Some SFTP servers (e.g. Bitvise on the .ssh directory) return + FXP_NAME with count=0 indefinitely instead of FX_EOF. asyncssh's + default scandir() loops forever in this case. We break after + _MAX_EMPTY_READDIR consecutive empty batches — matching WinSCP behaviour. + """ + import asyncssh as _asyncssh + + _MAX_EMPTY = 3 + dirpath = sftp.compose_path(path) + handle = await sftp._handler.opendir(dirpath) + entries = [] + consecutive_empty = 0 + at_end = False + try: + while not at_end: + names, at_end = await sftp._handler.readdir(handle) + if not names: + consecutive_empty += 1 + logger.debug("_readdir_safe: empty batch %d for '%s'", consecutive_empty, path) + if consecutive_empty >= _MAX_EMPTY: + logger.warning("_readdir_safe: %d consecutive empty batches for '%s' — treating as EOF (Bitvise quirk)", _MAX_EMPTY, path) + break + else: + consecutive_empty = 0 + entries.extend(names) + except _asyncssh.SFTPEOFError: + pass + finally: + await sftp._handler.close(handle) + return entries + def list_dir(self, path: str = ".") -> list[RemoteFile]: sftp = self._ensure_connected() target = (path if path != "." else self._cwd).rstrip("/") or "/" files: list[RemoteFile] = [] logger.debug("list_dir: requesting entries for '%s'", target) try: - entries = self._run(sftp.readdir(target)) + entries = self._run(self._readdir_safe(sftp, target)) logger.debug("list_dir: readdir returned %d raw entries for '%s'", len(entries) if entries is not None else -1, target) except PermissionError: raise From d1fde5ac1ef6be41f6419ef6777512e00baf0a59 Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:05:05 +0000 Subject: [PATCH 06/16] test(sftp): update mocks for _readdir_safe and add Bitvise count=0 test Co-Authored-By: Claude Opus 4.6 --- tests/test_sftp_client.py | 88 ++++++++++++++++++++++++++++++++++----- 1 file changed, 78 insertions(+), 10 deletions(-) diff --git a/tests/test_sftp_client.py b/tests/test_sftp_client.py index c476577..fcad2e5 100644 --- a/tests/test_sftp_client.py +++ b/tests/test_sftp_client.py @@ -31,6 +31,28 @@ def _make_mock_conn() -> tuple[MagicMock, MagicMock]: return mock_conn, mock_sftp +def _setup_readdir(mock_sftp: AsyncMock, entries: list) -> None: + """Wire up _handler mocks so _readdir_safe returns *entries* in one batch. + + _readdir_safe uses sftp._handler.opendir / readdir / close instead of the + high-level sftp.readdir wrapper. + """ + mock_sftp.compose_path.side_effect = lambda p: p + mock_handler = AsyncMock() + mock_handler.opendir.return_value = "fake-handle" + mock_handler.readdir.return_value = (entries, True) # (names, at_end) + mock_handler.close.return_value = None + mock_sftp._handler = mock_handler + + +def _setup_readdir_error(mock_sftp: AsyncMock, error: Exception) -> None: + """Wire up _handler mocks so _readdir_safe raises *error* on opendir.""" + mock_sftp.compose_path.side_effect = lambda p: p + mock_handler = AsyncMock() + mock_handler.opendir.side_effect = error + mock_sftp._handler = mock_handler + + class TestSFTPClientInit: def test_creates_conn_attribute(self, sftp_info: ConnectionInfo) -> None: client = SFTPClient(sftp_info) @@ -350,7 +372,7 @@ def test_list_dir_returns_files(self, sftp_info: ConnectionInfo) -> None: entry.attrs.uid = 1000 entry.attrs.gid = 1000 entry.longname = "-rw-r--r-- 1 user group 100 Jan 1 file.txt" - mock_sftp.readdir.return_value = [entry] + _setup_readdir(mock_sftp, [entry]) files = client.list_dir() assert len(files) == 1 @@ -363,7 +385,7 @@ def test_list_dir_permission_error(self, sftp_info: ConnectionInfo) -> None: client, mock_sftp = self._make_connected(sftp_info) err = IOError() err.errno = errno.EACCES - mock_sftp.readdir.side_effect = err + _setup_readdir_error(mock_sftp, err) with pytest.raises(PermissionError, match="Permission denied"): client.list_dir("/restricted") @@ -374,7 +396,7 @@ def test_list_dir_reraises_other_oserror(self, sftp_info: ConnectionInfo) -> Non client, mock_sftp = self._make_connected(sftp_info) err = IOError() err.errno = errno.ENOENT - mock_sftp.readdir.side_effect = err + _setup_readdir_error(mock_sftp, err) with pytest.raises(IOError): client.list_dir("/gone") @@ -445,7 +467,7 @@ def test_socket_file_skipped(self, sftp_info: ConnectionInfo) -> None: entry.attrs = MagicMock() entry.attrs.permissions = stat_mod.S_IFSOCK | 0o600 entry.longname = "srw------- 1 user group 0 Jan 1 agent.sock" - mock_sftp.readdir.return_value = [entry] + _setup_readdir(mock_sftp, [entry]) files = client.list_dir() assert files == [] @@ -457,7 +479,7 @@ def test_fifo_file_skipped(self, sftp_info: ConnectionInfo) -> None: entry.attrs = MagicMock() entry.attrs.permissions = stat_mod.S_IFIFO | 0o644 entry.longname = "prw-r--r-- 1 user group 0 Jan 1 mypipe" - mock_sftp.readdir.return_value = [entry] + _setup_readdir(mock_sftp, [entry]) files = client.list_dir() assert files == [] @@ -468,7 +490,7 @@ def test_oserror_reraises(self, sftp_info: ConnectionInfo) -> None: client, mock_sftp = self._make_connected(sftp_info) err = IOError() err.errno = errno.ENOENT - mock_sftp.readdir.side_effect = err + _setup_readdir_error(mock_sftp, err) with pytest.raises(IOError): client.list_dir("/gone") @@ -511,7 +533,7 @@ def test_list_dir_detects_dir_via_sftp_type_when_permissions_none( entry.attrs.uid = 1000 entry.attrs.gid = 1000 entry.longname = "" - mock_sftp.readdir.return_value = [entry] + _setup_readdir(mock_sftp, [entry]) files = client.list_dir() assert len(files) == 1 @@ -533,7 +555,7 @@ def test_list_dir_detects_dir_via_sftp_type_when_permissions_lack_type_bits( entry.attrs.uid = 1000 entry.attrs.gid = 1000 entry.longname = "" - mock_sftp.readdir.return_value = [entry] + _setup_readdir(mock_sftp, [entry]) files = client.list_dir() assert len(files) == 1 @@ -553,7 +575,7 @@ def test_list_dir_file_with_type_regular_stays_file(self, sftp_info: ConnectionI entry.attrs.uid = 1000 entry.attrs.gid = 1000 entry.longname = "" - mock_sftp.readdir.return_value = [entry] + _setup_readdir(mock_sftp, [entry]) files = client.list_dir() assert len(files) == 1 @@ -577,7 +599,7 @@ def test_list_dir_symlink_to_dir_via_sftp_type(self, sftp_info: ConnectionInfo) target_attrs.permissions = None target_attrs.type = _SFTP_TYPE_DIRECTORY mock_sftp.stat.return_value = target_attrs - mock_sftp.readdir.return_value = [entry] + _setup_readdir(mock_sftp, [entry]) files = client.list_dir() assert len(files) == 1 @@ -639,3 +661,49 @@ def test_stat_detects_dir_via_sftp_type(self, sftp_info: ConnectionInfo) -> None remote = client.stat("/home/user/.ssh") assert remote.is_dir is True assert remote.name == ".ssh" + + # ---- _readdir_safe: Bitvise count=0 EOF quirk ---- + + def test_readdir_safe_treats_consecutive_empty_batches_as_eof( + self, sftp_info: ConnectionInfo + ) -> None: + """_readdir_safe stops after 3 consecutive empty readdir responses. + + Bitvise may return FXP_NAME with count=0 (empty list, at_end=False) + indefinitely instead of FX_EOF. _readdir_safe must break out after + _MAX_EMPTY (3) consecutive empty batches — matching WinSCP behaviour. + """ + client, mock_sftp = self._make_connected(sftp_info) + + entry = MagicMock() + entry.filename = "hello.txt" + entry.attrs = MagicMock() + entry.attrs.permissions = stat_mod.S_IFREG | 0o644 + entry.attrs.type = _SFTP_TYPE_REGULAR + entry.attrs.size = 42 + entry.attrs.mtime = 0 + entry.attrs.uid = 1000 + entry.attrs.gid = 1000 + entry.longname = "" + + mock_sftp.compose_path.side_effect = lambda p: p + mock_handler = AsyncMock() + mock_handler.opendir.return_value = "fake-handle" + # Batch 1: real entry, at_end=False + # Batches 2-4: empty, at_end=False (Bitvise quirk — never sends EOF) + mock_handler.readdir.side_effect = [ + ([entry], False), + ([], False), + ([], False), + ([], False), + ] + mock_handler.close.return_value = None + mock_sftp._handler = mock_handler + + files = client.list_dir() + + assert len(files) == 1 + assert files[0].name == "hello.txt" + # 1 real batch + 3 empty batches = 4 readdir calls + assert mock_handler.readdir.call_count == 4 + mock_handler.close.assert_awaited_once() From 0d51953d2f13b2d9a20f2c05f4baff5c82d93645 Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:06:35 +0000 Subject: [PATCH 07/16] fix(sftp): run _readdir_safe as inline coroutine closure to avoid nested event loop dispatch --- src/portkeydrop/protocols.py | 70 ++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index 0e14780..1796bb6 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -568,48 +568,46 @@ def disconnect(self) -> None: # Directory operations # ------------------------------------------------------------------ - @staticmethod - async def _readdir_safe(sftp, path: str): - """Readdir loop that treats consecutive empty responses as EOF. - - Some SFTP servers (e.g. Bitvise on the .ssh directory) return - FXP_NAME with count=0 indefinitely instead of FX_EOF. asyncssh's - default scandir() loops forever in this case. We break after - _MAX_EMPTY_READDIR consecutive empty batches — matching WinSCP behaviour. - """ - import asyncssh as _asyncssh - - _MAX_EMPTY = 3 - dirpath = sftp.compose_path(path) - handle = await sftp._handler.opendir(dirpath) - entries = [] - consecutive_empty = 0 - at_end = False - try: - while not at_end: - names, at_end = await sftp._handler.readdir(handle) - if not names: - consecutive_empty += 1 - logger.debug("_readdir_safe: empty batch %d for '%s'", consecutive_empty, path) - if consecutive_empty >= _MAX_EMPTY: - logger.warning("_readdir_safe: %d consecutive empty batches for '%s' — treating as EOF (Bitvise quirk)", _MAX_EMPTY, path) - break - else: - consecutive_empty = 0 - entries.extend(names) - except _asyncssh.SFTPEOFError: - pass - finally: - await sftp._handler.close(handle) - return entries - def list_dir(self, path: str = ".") -> list[RemoteFile]: sftp = self._ensure_connected() target = (path if path != "." else self._cwd).rstrip("/") or "/" files: list[RemoteFile] = [] logger.debug("list_dir: requesting entries for '%s'", target) + + async def _readdir_safe(): + """Readdir loop that treats consecutive empty responses as EOF. + + Some SFTP servers (e.g. Bitvise on the .ssh directory) return + FXP_NAME with count=0 indefinitely instead of FX_EOF. asyncssh + loops forever in this case; we break after 3 consecutive empty + batches — matching WinSCP behaviour. + """ + _MAX_EMPTY = 3 + dirpath = sftp.compose_path(target) + handle = await sftp._handler.opendir(dirpath) + result = [] + consecutive_empty = 0 + at_end = False + try: + while not at_end: + names, at_end = await sftp._handler.readdir(handle) + if not names: + consecutive_empty += 1 + logger.debug("_readdir_safe: empty batch %d for '%s'", consecutive_empty, target) + if consecutive_empty >= _MAX_EMPTY: + logger.warning("_readdir_safe: treating %d consecutive empty batches as EOF for '%s'", _MAX_EMPTY, target) + break + else: + consecutive_empty = 0 + result.extend(names) + except asyncssh.SFTPEOFError: + pass + finally: + await sftp._handler.close(handle) + return result + try: - entries = self._run(self._readdir_safe(sftp, target)) + entries = self._run(_readdir_safe()) logger.debug("list_dir: readdir returned %d raw entries for '%s'", len(entries) if entries is not None else -1, target) except PermissionError: raise From ba83661a542109158508ae404f03abc28e751c86 Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:08:16 +0000 Subject: [PATCH 08/16] fix(sftp): add missing runtime asyncssh import in _readdir_safe closure --- src/portkeydrop/protocols.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index 1796bb6..20b7221 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -582,6 +582,8 @@ async def _readdir_safe(): loops forever in this case; we break after 3 consecutive empty batches — matching WinSCP behaviour. """ + import asyncssh as _asyncssh + _MAX_EMPTY = 3 dirpath = sftp.compose_path(target) handle = await sftp._handler.opendir(dirpath) @@ -600,7 +602,7 @@ async def _readdir_safe(): else: consecutive_empty = 0 result.extend(names) - except asyncssh.SFTPEOFError: + except _asyncssh.SFTPEOFError: pass finally: await sftp._handler.close(handle) From ba2356441c427ecb570a2ad122a3de163062418d Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:10:11 +0000 Subject: [PATCH 09/16] fix(sftp): decode SFTPName filenames from bytes in _readdir_safe --- src/portkeydrop/protocols.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index 20b7221..44bfc5f 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -601,6 +601,13 @@ async def _readdir_safe(): break else: consecutive_empty = 0 + # Decode filenames from bytes → str (same as asyncssh scandir does + # internally when called with a str path). + for entry in names: + if entry.filename and isinstance(entry.filename, (bytes, bytearray)): + entry.filename = sftp.decode(entry.filename) + if entry.longname and isinstance(entry.longname, (bytes, bytearray)): + entry.longname = sftp.decode(entry.longname) result.extend(names) except _asyncssh.SFTPEOFError: pass From cc8566f3aed080d574a4b6014f9565a874649ed2 Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:11:58 +0000 Subject: [PATCH 10/16] style: ruff format protocols.py --- src/portkeydrop/protocols.py | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index 44bfc5f..15855c9 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -595,9 +595,15 @@ async def _readdir_safe(): names, at_end = await sftp._handler.readdir(handle) if not names: consecutive_empty += 1 - logger.debug("_readdir_safe: empty batch %d for '%s'", consecutive_empty, target) + logger.debug( + "_readdir_safe: empty batch %d for '%s'", consecutive_empty, target + ) if consecutive_empty >= _MAX_EMPTY: - logger.warning("_readdir_safe: treating %d consecutive empty batches as EOF for '%s'", _MAX_EMPTY, target) + logger.warning( + "_readdir_safe: treating %d consecutive empty batches as EOF for '%s'", + _MAX_EMPTY, + target, + ) break else: consecutive_empty = 0 @@ -617,7 +623,11 @@ async def _readdir_safe(): try: entries = self._run(_readdir_safe()) - logger.debug("list_dir: readdir returned %d raw entries for '%s'", len(entries) if entries is not None else -1, target) + logger.debug( + "list_dir: readdir returned %d raw entries for '%s'", + len(entries) if entries is not None else -1, + target, + ) except PermissionError: raise except OSError as e: @@ -630,8 +640,14 @@ async def _readdir_safe(): # asyncssh raises SFTPError (not OSError) for server-side errors. # Map permission errors; surface everything else. import asyncssh as _asyncssh + if isinstance(e, _asyncssh.SFTPError): - logger.warning("list_dir: SFTPError for '%s': code=%s msg=%s", target, getattr(e, "code", "?"), e) + logger.warning( + "list_dir: SFTPError for '%s': code=%s msg=%s", + target, + getattr(e, "code", "?"), + e, + ) if getattr(e, "code", None) in (3, 4): # FX_PERMISSION_DENIED=3, FX_FAILURE=4 raise PermissionError(f"Permission denied: cannot list '{target}'") from e raise @@ -710,7 +726,12 @@ def chdir(self, path: str) -> str: # Validate the target is a directory (strict servers like Bitvise # require an explicit stat check — realpath only canonicalises). attrs = self._run(sftp.stat(resolved)) - logger.debug("chdir stat: path='%s' permissions=%s type=%s", resolved, attrs.permissions, getattr(attrs, "type", None)) + logger.debug( + "chdir stat: path='%s' permissions=%s type=%s", + resolved, + attrs.permissions, + getattr(attrs, "type", None), + ) is_dir = False if attrs.permissions is not None and stat.S_ISDIR(attrs.permissions): is_dir = True @@ -719,7 +740,9 @@ def chdir(self, path: str) -> str: elif attrs.permissions is None and getattr(attrs, "type", None) is None: # Server returned no type info (e.g. Bitvise on .ssh) — # assume directory and let the server reject if wrong. - logger.debug("chdir: no type info from server, assuming directory for '%s'", resolved) + logger.debug( + "chdir: no type info from server, assuming directory for '%s'", resolved + ) is_dir = True if not is_dir: raise NotADirectoryError(f"Not a directory: '{path}'") From b6f90707513b60ba25a86b2bda870d4f105f364c Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:14:04 +0000 Subject: [PATCH 11/16] test: fix test_list_dir_maps_file_attributes mock for _readdir_safe --- tests/test_protocols.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_protocols.py b/tests/test_protocols.py index 36a1486..8acc1b7 100644 --- a/tests/test_protocols.py +++ b/tests/test_protocols.py @@ -427,7 +427,15 @@ def test_list_dir_maps_file_attributes(self, mock_connect): dot_entry = MagicMock() dot_entry.filename = "." - mock_sftp.readdir.return_value = [dot_entry, file_entry, dir_entry] + + # list_dir uses _readdir_safe which calls sftp._handler.opendir/readdir directly. + mock_handler = AsyncMock() + mock_sftp._handler = mock_handler + mock_sftp.compose_path.return_value = b"/home/user" + # First call returns entries, second call returns empty (EOF signal) + mock_handler.readdir.side_effect = [ + ([dot_entry, file_entry, dir_entry], True), + ] client = SFTPClient(ConnectionInfo(protocol=Protocol.SFTP, host="example.com")) client.connect() From e79a436e5926be573b07d6de2a8658fd673b3cbf Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:17:24 +0000 Subject: [PATCH 12/16] fix(sftp): resolve symlinks before stat to get correct file size for progress --- src/portkeydrop/protocols.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index 15855c9..4c22a5f 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -769,7 +769,13 @@ def download( async def _download(): async with sftp.open(remote_path, "rb") as rf: - total = (await sftp.stat(remote_path)).size or 0 + # Use realpath to resolve symlinks before stat so we get the + # actual file size rather than the symlink size (0). + try: + resolved = await sftp.realpath(remote_path) + total = (await sftp.stat(resolved)).size or 0 + except Exception: + total = (await sftp.stat(remote_path)).size or 0 transferred = 0 while True: chunk = await rf.read(8192) From d6b808c9cd0cf042e09be4a81f91da7739d9bb80 Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:20:40 +0000 Subject: [PATCH 13/16] fix(sftp): don't zero out total_bytes in callback when stat returns 0 (symlinks) --- src/portkeydrop/dialogs/transfer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/portkeydrop/dialogs/transfer.py b/src/portkeydrop/dialogs/transfer.py index 82bcc44..8b579ed 100644 --- a/src/portkeydrop/dialogs/transfer.py +++ b/src/portkeydrop/dialogs/transfer.py @@ -165,7 +165,7 @@ def callback(transferred: int, total: int) -> None: if item.cancel_event.is_set(): raise InterruptedError("Transfer cancelled") item.transferred_bytes = transferred - item.total_bytes = total + item.total_bytes = total if total > 0 else item.total_bytes self._notify() client.download(item.remote_path, f, callback=callback) @@ -187,7 +187,7 @@ def callback(transferred: int, total: int) -> None: if item.cancel_event.is_set(): raise InterruptedError("Transfer cancelled") item.transferred_bytes = transferred - item.total_bytes = total + item.total_bytes = total if total > 0 else item.total_bytes self._notify() client.upload(f, item.remote_path, callback=callback) From e6e8b6d21f57cf2a65a43b27b6f887045bce326f Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:21:27 +0000 Subject: [PATCH 14/16] test(sftp): add coverage for _readdir_safe edge cases Co-Authored-By: Claude Opus 4.6 --- tests/test_sftp_client.py | 114 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/test_sftp_client.py b/tests/test_sftp_client.py index fcad2e5..4aa26f9 100644 --- a/tests/test_sftp_client.py +++ b/tests/test_sftp_client.py @@ -707,3 +707,117 @@ def test_readdir_safe_treats_consecutive_empty_batches_as_eof( # 1 real batch + 3 empty batches = 4 readdir calls assert mock_handler.readdir.call_count == 4 mock_handler.close.assert_awaited_once() + + # ---- _readdir_safe: SFTPEOFError handling ---- + + def test_readdir_safe_handles_sftp_eof_error(self, sftp_info: ConnectionInfo) -> None: + """_readdir_safe catches SFTPEOFError and returns collected results.""" + import asyncssh + + client, mock_sftp = self._make_connected(sftp_info) + + entry = MagicMock() + entry.filename = "data.csv" + entry.longname = "" + entry.attrs = MagicMock() + entry.attrs.permissions = stat_mod.S_IFREG | 0o644 + entry.attrs.type = _SFTP_TYPE_REGULAR + entry.attrs.size = 99 + entry.attrs.mtime = 0 + entry.attrs.uid = 1000 + entry.attrs.gid = 1000 + + mock_sftp.compose_path.side_effect = lambda p: p + mock_handler = AsyncMock() + mock_handler.opendir.return_value = "fake-handle" + # First call returns an entry; second raises SFTPEOFError + mock_handler.readdir.side_effect = [ + ([entry], False), + asyncssh.SFTPEOFError(), + ] + mock_handler.close.return_value = None + mock_sftp._handler = mock_handler + + files = client.list_dir() + assert len(files) == 1 + assert files[0].name == "data.csv" + mock_handler.close.assert_awaited_once() + + # ---- _readdir_safe: bytes filename decode ---- + + def test_readdir_safe_decodes_bytes_filenames(self, sftp_info: ConnectionInfo) -> None: + """_readdir_safe decodes bytes filenames/longnames via sftp.decode.""" + client, mock_sftp = self._make_connected(sftp_info) + + entry = MagicMock() + entry.filename = b"report.txt" + entry.longname = b"-rw-r--r-- 1 user user 42 Jan 1 00:00 report.txt" + entry.attrs = MagicMock() + entry.attrs.permissions = stat_mod.S_IFREG | 0o644 + entry.attrs.type = _SFTP_TYPE_REGULAR + entry.attrs.size = 42 + entry.attrs.mtime = 0 + entry.attrs.uid = 1000 + entry.attrs.gid = 1000 + + mock_sftp.decode = MagicMock(side_effect=lambda b: b.decode("utf-8")) + mock_sftp.compose_path.side_effect = lambda p: p + mock_handler = AsyncMock() + mock_handler.opendir.return_value = "fake-handle" + mock_handler.readdir.return_value = ([entry], True) + mock_handler.close.return_value = None + mock_sftp._handler = mock_handler + + files = client.list_dir() + assert len(files) == 1 + assert files[0].name == "report.txt" + assert mock_sftp.decode.call_count == 2 + + # ---- list_dir: SFTPError exception mapping ---- + + def test_list_dir_sftp_error_permission_denied(self, sftp_info: ConnectionInfo) -> None: + """list_dir maps SFTPError code=3 (FX_PERMISSION_DENIED) to PermissionError.""" + import asyncssh + + client, mock_sftp = self._make_connected(sftp_info) + _setup_readdir_error(mock_sftp, asyncssh.SFTPError(3, "Permission denied")) + + with pytest.raises(PermissionError, match="Permission denied"): + client.list_dir("/secret") + + def test_list_dir_sftp_error_failure_mapped_to_permission( + self, sftp_info: ConnectionInfo + ) -> None: + """list_dir maps SFTPError code=4 (FX_FAILURE) to PermissionError.""" + import asyncssh + + client, mock_sftp = self._make_connected(sftp_info) + _setup_readdir_error(mock_sftp, asyncssh.SFTPError(4, "Failure")) + + with pytest.raises(PermissionError, match="Permission denied"): + client.list_dir("/secret") + + def test_list_dir_sftp_error_non_permission_reraises(self, sftp_info: ConnectionInfo) -> None: + """list_dir re-raises SFTPError with non-permission code unchanged.""" + import asyncssh + + client, mock_sftp = self._make_connected(sftp_info) + _setup_readdir_error(mock_sftp, asyncssh.SFTPError(7, "Connection lost")) + + with pytest.raises(asyncssh.SFTPError): + client.list_dir("/data") + + # ---- chdir: no type info fallback ---- + + def test_chdir_assumes_dir_when_no_type_info(self, sftp_info: ConnectionInfo) -> None: + """chdir assumes directory when stat returns permissions=None and type=None.""" + client, mock_sftp = self._make_connected(sftp_info) + mock_sftp.realpath.return_value = "/home/user/.ssh" + stat_attrs = MagicMock() + stat_attrs.permissions = None + stat_attrs.type = None + mock_sftp.stat.return_value = stat_attrs + + result = client.chdir("/home/user/.ssh") + assert result == "/home/user/.ssh" + assert client._cwd == "/home/user/.ssh" From 5afce4ce97f3093ee12dfae503d9684830277f0d Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:08:17 +0000 Subject: [PATCH 15/16] revert: remove symlink progress fixes (unrelated to PR #50 scope) --- src/portkeydrop/dialogs/transfer.py | 4 +- src/portkeydrop/protocols.py | 8 +- tests/test_sftp_client.py | 114 ---------------------------- 3 files changed, 3 insertions(+), 123 deletions(-) diff --git a/src/portkeydrop/dialogs/transfer.py b/src/portkeydrop/dialogs/transfer.py index 8b579ed..82bcc44 100644 --- a/src/portkeydrop/dialogs/transfer.py +++ b/src/portkeydrop/dialogs/transfer.py @@ -165,7 +165,7 @@ def callback(transferred: int, total: int) -> None: if item.cancel_event.is_set(): raise InterruptedError("Transfer cancelled") item.transferred_bytes = transferred - item.total_bytes = total if total > 0 else item.total_bytes + item.total_bytes = total self._notify() client.download(item.remote_path, f, callback=callback) @@ -187,7 +187,7 @@ def callback(transferred: int, total: int) -> None: if item.cancel_event.is_set(): raise InterruptedError("Transfer cancelled") item.transferred_bytes = transferred - item.total_bytes = total if total > 0 else item.total_bytes + item.total_bytes = total self._notify() client.upload(f, item.remote_path, callback=callback) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index 4c22a5f..15855c9 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -769,13 +769,7 @@ def download( async def _download(): async with sftp.open(remote_path, "rb") as rf: - # Use realpath to resolve symlinks before stat so we get the - # actual file size rather than the symlink size (0). - try: - resolved = await sftp.realpath(remote_path) - total = (await sftp.stat(resolved)).size or 0 - except Exception: - total = (await sftp.stat(remote_path)).size or 0 + total = (await sftp.stat(remote_path)).size or 0 transferred = 0 while True: chunk = await rf.read(8192) diff --git a/tests/test_sftp_client.py b/tests/test_sftp_client.py index 4aa26f9..fcad2e5 100644 --- a/tests/test_sftp_client.py +++ b/tests/test_sftp_client.py @@ -707,117 +707,3 @@ def test_readdir_safe_treats_consecutive_empty_batches_as_eof( # 1 real batch + 3 empty batches = 4 readdir calls assert mock_handler.readdir.call_count == 4 mock_handler.close.assert_awaited_once() - - # ---- _readdir_safe: SFTPEOFError handling ---- - - def test_readdir_safe_handles_sftp_eof_error(self, sftp_info: ConnectionInfo) -> None: - """_readdir_safe catches SFTPEOFError and returns collected results.""" - import asyncssh - - client, mock_sftp = self._make_connected(sftp_info) - - entry = MagicMock() - entry.filename = "data.csv" - entry.longname = "" - entry.attrs = MagicMock() - entry.attrs.permissions = stat_mod.S_IFREG | 0o644 - entry.attrs.type = _SFTP_TYPE_REGULAR - entry.attrs.size = 99 - entry.attrs.mtime = 0 - entry.attrs.uid = 1000 - entry.attrs.gid = 1000 - - mock_sftp.compose_path.side_effect = lambda p: p - mock_handler = AsyncMock() - mock_handler.opendir.return_value = "fake-handle" - # First call returns an entry; second raises SFTPEOFError - mock_handler.readdir.side_effect = [ - ([entry], False), - asyncssh.SFTPEOFError(), - ] - mock_handler.close.return_value = None - mock_sftp._handler = mock_handler - - files = client.list_dir() - assert len(files) == 1 - assert files[0].name == "data.csv" - mock_handler.close.assert_awaited_once() - - # ---- _readdir_safe: bytes filename decode ---- - - def test_readdir_safe_decodes_bytes_filenames(self, sftp_info: ConnectionInfo) -> None: - """_readdir_safe decodes bytes filenames/longnames via sftp.decode.""" - client, mock_sftp = self._make_connected(sftp_info) - - entry = MagicMock() - entry.filename = b"report.txt" - entry.longname = b"-rw-r--r-- 1 user user 42 Jan 1 00:00 report.txt" - entry.attrs = MagicMock() - entry.attrs.permissions = stat_mod.S_IFREG | 0o644 - entry.attrs.type = _SFTP_TYPE_REGULAR - entry.attrs.size = 42 - entry.attrs.mtime = 0 - entry.attrs.uid = 1000 - entry.attrs.gid = 1000 - - mock_sftp.decode = MagicMock(side_effect=lambda b: b.decode("utf-8")) - mock_sftp.compose_path.side_effect = lambda p: p - mock_handler = AsyncMock() - mock_handler.opendir.return_value = "fake-handle" - mock_handler.readdir.return_value = ([entry], True) - mock_handler.close.return_value = None - mock_sftp._handler = mock_handler - - files = client.list_dir() - assert len(files) == 1 - assert files[0].name == "report.txt" - assert mock_sftp.decode.call_count == 2 - - # ---- list_dir: SFTPError exception mapping ---- - - def test_list_dir_sftp_error_permission_denied(self, sftp_info: ConnectionInfo) -> None: - """list_dir maps SFTPError code=3 (FX_PERMISSION_DENIED) to PermissionError.""" - import asyncssh - - client, mock_sftp = self._make_connected(sftp_info) - _setup_readdir_error(mock_sftp, asyncssh.SFTPError(3, "Permission denied")) - - with pytest.raises(PermissionError, match="Permission denied"): - client.list_dir("/secret") - - def test_list_dir_sftp_error_failure_mapped_to_permission( - self, sftp_info: ConnectionInfo - ) -> None: - """list_dir maps SFTPError code=4 (FX_FAILURE) to PermissionError.""" - import asyncssh - - client, mock_sftp = self._make_connected(sftp_info) - _setup_readdir_error(mock_sftp, asyncssh.SFTPError(4, "Failure")) - - with pytest.raises(PermissionError, match="Permission denied"): - client.list_dir("/secret") - - def test_list_dir_sftp_error_non_permission_reraises(self, sftp_info: ConnectionInfo) -> None: - """list_dir re-raises SFTPError with non-permission code unchanged.""" - import asyncssh - - client, mock_sftp = self._make_connected(sftp_info) - _setup_readdir_error(mock_sftp, asyncssh.SFTPError(7, "Connection lost")) - - with pytest.raises(asyncssh.SFTPError): - client.list_dir("/data") - - # ---- chdir: no type info fallback ---- - - def test_chdir_assumes_dir_when_no_type_info(self, sftp_info: ConnectionInfo) -> None: - """chdir assumes directory when stat returns permissions=None and type=None.""" - client, mock_sftp = self._make_connected(sftp_info) - mock_sftp.realpath.return_value = "/home/user/.ssh" - stat_attrs = MagicMock() - stat_attrs.permissions = None - stat_attrs.type = None - mock_sftp.stat.return_value = stat_attrs - - result = client.chdir("/home/user/.ssh") - assert result == "/home/user/.ssh" - assert client._cwd == "/home/user/.ssh" From d7d7490129a3a9e346ec5fe737840a98fcbbd4bf Mon Sep 17 00:00:00 2001 From: Orinks <38449772+Orinks@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:21:27 +0000 Subject: [PATCH 16/16] test(sftp): add coverage for _readdir_safe edge cases Co-Authored-By: Claude Opus 4.6 --- tests/test_sftp_client.py | 114 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/test_sftp_client.py b/tests/test_sftp_client.py index fcad2e5..4aa26f9 100644 --- a/tests/test_sftp_client.py +++ b/tests/test_sftp_client.py @@ -707,3 +707,117 @@ def test_readdir_safe_treats_consecutive_empty_batches_as_eof( # 1 real batch + 3 empty batches = 4 readdir calls assert mock_handler.readdir.call_count == 4 mock_handler.close.assert_awaited_once() + + # ---- _readdir_safe: SFTPEOFError handling ---- + + def test_readdir_safe_handles_sftp_eof_error(self, sftp_info: ConnectionInfo) -> None: + """_readdir_safe catches SFTPEOFError and returns collected results.""" + import asyncssh + + client, mock_sftp = self._make_connected(sftp_info) + + entry = MagicMock() + entry.filename = "data.csv" + entry.longname = "" + entry.attrs = MagicMock() + entry.attrs.permissions = stat_mod.S_IFREG | 0o644 + entry.attrs.type = _SFTP_TYPE_REGULAR + entry.attrs.size = 99 + entry.attrs.mtime = 0 + entry.attrs.uid = 1000 + entry.attrs.gid = 1000 + + mock_sftp.compose_path.side_effect = lambda p: p + mock_handler = AsyncMock() + mock_handler.opendir.return_value = "fake-handle" + # First call returns an entry; second raises SFTPEOFError + mock_handler.readdir.side_effect = [ + ([entry], False), + asyncssh.SFTPEOFError(), + ] + mock_handler.close.return_value = None + mock_sftp._handler = mock_handler + + files = client.list_dir() + assert len(files) == 1 + assert files[0].name == "data.csv" + mock_handler.close.assert_awaited_once() + + # ---- _readdir_safe: bytes filename decode ---- + + def test_readdir_safe_decodes_bytes_filenames(self, sftp_info: ConnectionInfo) -> None: + """_readdir_safe decodes bytes filenames/longnames via sftp.decode.""" + client, mock_sftp = self._make_connected(sftp_info) + + entry = MagicMock() + entry.filename = b"report.txt" + entry.longname = b"-rw-r--r-- 1 user user 42 Jan 1 00:00 report.txt" + entry.attrs = MagicMock() + entry.attrs.permissions = stat_mod.S_IFREG | 0o644 + entry.attrs.type = _SFTP_TYPE_REGULAR + entry.attrs.size = 42 + entry.attrs.mtime = 0 + entry.attrs.uid = 1000 + entry.attrs.gid = 1000 + + mock_sftp.decode = MagicMock(side_effect=lambda b: b.decode("utf-8")) + mock_sftp.compose_path.side_effect = lambda p: p + mock_handler = AsyncMock() + mock_handler.opendir.return_value = "fake-handle" + mock_handler.readdir.return_value = ([entry], True) + mock_handler.close.return_value = None + mock_sftp._handler = mock_handler + + files = client.list_dir() + assert len(files) == 1 + assert files[0].name == "report.txt" + assert mock_sftp.decode.call_count == 2 + + # ---- list_dir: SFTPError exception mapping ---- + + def test_list_dir_sftp_error_permission_denied(self, sftp_info: ConnectionInfo) -> None: + """list_dir maps SFTPError code=3 (FX_PERMISSION_DENIED) to PermissionError.""" + import asyncssh + + client, mock_sftp = self._make_connected(sftp_info) + _setup_readdir_error(mock_sftp, asyncssh.SFTPError(3, "Permission denied")) + + with pytest.raises(PermissionError, match="Permission denied"): + client.list_dir("/secret") + + def test_list_dir_sftp_error_failure_mapped_to_permission( + self, sftp_info: ConnectionInfo + ) -> None: + """list_dir maps SFTPError code=4 (FX_FAILURE) to PermissionError.""" + import asyncssh + + client, mock_sftp = self._make_connected(sftp_info) + _setup_readdir_error(mock_sftp, asyncssh.SFTPError(4, "Failure")) + + with pytest.raises(PermissionError, match="Permission denied"): + client.list_dir("/secret") + + def test_list_dir_sftp_error_non_permission_reraises(self, sftp_info: ConnectionInfo) -> None: + """list_dir re-raises SFTPError with non-permission code unchanged.""" + import asyncssh + + client, mock_sftp = self._make_connected(sftp_info) + _setup_readdir_error(mock_sftp, asyncssh.SFTPError(7, "Connection lost")) + + with pytest.raises(asyncssh.SFTPError): + client.list_dir("/data") + + # ---- chdir: no type info fallback ---- + + def test_chdir_assumes_dir_when_no_type_info(self, sftp_info: ConnectionInfo) -> None: + """chdir assumes directory when stat returns permissions=None and type=None.""" + client, mock_sftp = self._make_connected(sftp_info) + mock_sftp.realpath.return_value = "/home/user/.ssh" + stat_attrs = MagicMock() + stat_attrs.permissions = None + stat_attrs.type = None + mock_sftp.stat.return_value = stat_attrs + + result = client.chdir("/home/user/.ssh") + assert result == "/home/user/.ssh" + assert client._cwd == "/home/user/.ssh"