From f10fe75ef84cf23a3b925d93a05c1cc21195e704 Mon Sep 17 00:00:00 2001 From: Mikhail Ilchenko Date: Thu, 9 Jul 2026 19:19:29 +0400 Subject: [PATCH 1/4] fix: preserve root path in _strip_protocol so walk('/') works SSHFileSystem inherited the default root_marker of "", so _strip_protocol("/") collapsed the root to an empty string. walk("/") then listed the home directory and produced relative paths that were not considered to exist. Set root_marker = "/" to keep the root, and add walk tests covering a nested tree and the root case. --- sshfs/spec.py | 2 ++ tests/test_sshfs.py | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/sshfs/spec.py b/sshfs/spec.py index 5c1fc73..5478615 100644 --- a/sshfs/spec.py +++ b/sshfs/spec.py @@ -35,6 +35,8 @@ class SSHFileSystem(AsyncFileSystem): + root_marker = "/" + def __init__( self, host, diff --git a/tests/test_sshfs.py b/tests/test_sshfs.py index e55639c..26052aa 100644 --- a/tests/test_sshfs.py +++ b/tests/test_sshfs.py @@ -203,6 +203,33 @@ def test_ls(fs, remote_dir): assert dirs == expected +def test_walk(fs, remote_dir): + fs.mkdir(remote_dir + "/a") + fs.mkdir(remote_dir + "/a/b") + fs.touch(remote_dir + "/a/f1") + fs.touch(remote_dir + "/a/b/f2") + + result = { + root: (sorted(dirs), sorted(files)) + for root, dirs, files in fs.walk(remote_dir + "/a") + } + assert result == { + remote_dir + "/a": (["b"], ["f1"]), + remote_dir + "/a/b": ([], ["f2"]), + } + + +def test_walk_root(fs): + # Regression: SSHFileSystem._strip_protocol("/") used to collapse the + # root to "", so walk("/") listed the home directory and yielded + # relative paths that were not considered to exist. + root, dirs, files = next(iter(fs.walk("/", maxdepth=1, detail=True))) + assert root == "/" + for info in dirs.values(): + assert info["name"].startswith("/") + assert fs.exists(info["name"]) + + def test_mkdir(fs, remote_dir): fs.mkdir(remote_dir + "dir/") assert fs.isdir(remote_dir + "dir/") From d0bcbb3e6e05235c495bc617e73f6b4b17adcdce Mon Sep 17 00:00:00 2001 From: Mikhail Ilchenko Date: Fri, 24 Jul 2026 12:40:42 +0400 Subject: [PATCH 2/4] Address review --- sshfs/spec.py | 11 ++++++++--- tests/test_sshfs.py | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/sshfs/spec.py b/sshfs/spec.py index 5478615..2a2ef6b 100644 --- a/sshfs/spec.py +++ b/sshfs/spec.py @@ -35,8 +35,6 @@ class SSHFileSystem(AsyncFileSystem): - root_marker = "/" - def __init__( self, host, @@ -86,7 +84,14 @@ def __init__( def _strip_protocol(cls, path): # Remove components such as host and username from path. inferred_path = infer_storage_options(path)["path"] - return super()._strip_protocol(inferred_path) + stripped = super()._strip_protocol(inferred_path) + # super() collapses a bare "/" to the (empty) root_marker. Restore it + # only when the caller explicitly supplied the absolute root, so that + # walk("/") lists from the root while relative paths (and "") still + # resolve against the home directory. + if not stripped and inferred_path.startswith("/"): + return "/" + return stripped @staticmethod def _get_kwargs_from_urls(urlpath): diff --git a/tests/test_sshfs.py b/tests/test_sshfs.py index 26052aa..d9bc45e 100644 --- a/tests/test_sshfs.py +++ b/tests/test_sshfs.py @@ -230,6 +230,21 @@ def test_walk_root(fs): assert fs.exists(info["name"]) +def test_strip_protocol(): + strip = SSHFileSystem._strip_protocol + # An explicit absolute root is preserved so walk("/") lists from the root. + assert strip("/") == "/" + assert strip("ssh://host/") == "/" + # Empty and relative paths still resolve against the home directory + # instead of being redirected to the root. + assert strip("") == "" + assert strip("ssh://host") == "" + assert strip("foo/bar") == "foo/bar" + # Regular absolute paths are unaffected. + assert strip("/foo/bar") == "/foo/bar" + assert strip("ssh://host/foo/bar") == "/foo/bar" + + def test_mkdir(fs, remote_dir): fs.mkdir(remote_dir + "dir/") assert fs.isdir(remote_dir + "dir/") From 20e992409663e4e8123baa1fa3afaca730d42682 Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Sun, 26 Jul 2026 15:01:29 -1000 Subject: [PATCH 3/4] tests: make test_walk_root deterministic The mock server exposes the host's real filesystem, so assertions about the contents of "/" depend on machine state: mock-ssh-server stats every entry of a listing, so a single dangling symlink at the host root fails the whole readdir, and fsspec's walk(on_error="omit") swallows the error, silently turning the name/exists assertions into a no-op. Assert only the yielded root, which distinguishes the fix (root "/") from the regression (root "") on every machine. Also correct the failure-mode note: most servers (e.g. OpenSSH) reject an empty path with ENOENT outside REALPATH; servers implementing the SFTP draft's empty-path rule resolve it to the default directory instead. Co-Authored-By: Claude Fable 5 --- tests/test_sshfs.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/test_sshfs.py b/tests/test_sshfs.py index d9bc45e..2d1200f 100644 --- a/tests/test_sshfs.py +++ b/tests/test_sshfs.py @@ -220,14 +220,11 @@ def test_walk(fs, remote_dir): def test_walk_root(fs): - # Regression: SSHFileSystem._strip_protocol("/") used to collapse the - # root to "", so walk("/") listed the home directory and yielded - # relative paths that were not considered to exist. - root, dirs, files = next(iter(fs.walk("/", maxdepth=1, detail=True))) + # walk("/") must anchor at the root, not the user's home directory. + # Only the yielded root is asserted: the mock server exposes the + # host's real filesystem, so the contents of "/" are unpredictable. + root, _dirs, _files = next(fs.walk("/", maxdepth=1)) assert root == "/" - for info in dirs.values(): - assert info["name"].startswith("/") - assert fs.exists(info["name"]) def test_strip_protocol(): From 2d0321372a0112dfada80fed841028cfd51cb1b5 Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Tue, 4 Aug 2026 13:20:54 -0700 Subject: [PATCH 4/4] tests: replace walk-based root test with stat-based assertions test_walk_root listed the host's real root through the mock server: mock-ssh-server stats every entry of a listing, so a single dangling symlink at "/" fails the whole readdir. On older fsspec releases (e.g. 2025.10.0, which Python 3.9 resolves) walk() yields nothing when the listing fails, so next() raised StopIteration -- the macos-latest/3.9 CI failure. Assert root resolution via stat instead, which depends neither on the contents of "/" nor on the fsspec version. Co-Authored-By: Claude Fable 5 --- tests/test_sshfs.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_sshfs.py b/tests/test_sshfs.py index 2d1200f..ed39a98 100644 --- a/tests/test_sshfs.py +++ b/tests/test_sshfs.py @@ -219,12 +219,12 @@ def test_walk(fs, remote_dir): } -def test_walk_root(fs): - # walk("/") must anchor at the root, not the user's home directory. - # Only the yielded root is asserted: the mock server exposes the - # host's real filesystem, so the contents of "/" are unpredictable. - root, _dirs, _files = next(fs.walk("/", maxdepth=1)) - assert root == "/" +def test_root(fs): + # An explicit "/" must resolve to the filesystem root, not the + # user's home directory. + assert fs.exists("/") + assert fs.isdir("/") + assert fs.info("/")["name"] == "/" def test_strip_protocol():