Samin061 commented on code in PR #74012:
URL: https://github.com/apache/airflow/pull/74012#discussion_r4190064580
##########
task-sdk/tests/task_sdk/io/test_path.py:
##########
@@ -337,6 +337,49 @@ def test_iterdir_children_use_authenticated_fs(self,
fake_fs_with_conn):
assert all(c.__wrapped__._fs_cached is fake_fs_with_conn for c in
children)
+class TestRecursiveCopyToLocal:
+ """Recursive remote->local copy must not follow ``..`` in object keys
outside the destination."""
+
+ @pytest.fixture(autouse=True)
+ def restore_cache(self):
+ cache = _STORE_CACHE.copy()
+ yield
+ _STORE_CACHE.clear()
+ _STORE_CACHE.update(cache)
+
+ @pytest.fixture
+ def remote_fs(self):
+ fs = _FakeRemoteFileSystem(conn_id="my_conn")
+ attach(protocol="ffs2", conn_id="my_conn", fs=fs)
+ try:
+ yield fs
+ finally:
+ _FakeRemoteFileSystem.store.clear()
+ _FakeRemoteFileSystem.pseudo_dirs[:] = [""]
+
+ def test_rejects_key_escaping_destination(self, remote_fs, tmp_path):
+ remote_fs.pipe_file("bucket/srcdir/normal.txt", b"ok")
+ remote_fs.pipe_file("bucket/srcdir/../../escape/pwned.txt", b"pwned")
+ src = ObjectStoragePath("ffs2://my_conn@bucket/srcdir",
conn_id="my_conn")
+ dst = ObjectStoragePath(f"file://{tmp_path.as_posix()}/dest")
+
+ with pytest.raises(ValueError, match="resolves outside"):
+ src.copy(dst, recursive=True)
+
+ assert not (tmp_path / "escape" / "pwned.txt").exists()
+
+ def test_allows_contained_keys(self, remote_fs, tmp_path):
+ remote_fs.pipe_file("bucket/srcdir/a.txt", b"a")
+ remote_fs.pipe_file("bucket/srcdir/sub/b.txt", b"b")
+ src = ObjectStoragePath("ffs2://my_conn@bucket/srcdir",
conn_id="my_conn")
+ dst = ObjectStoragePath(f"file://{tmp_path.as_posix()}/dest")
+
+ src.copy(dst, recursive=True)
+
+ assert (tmp_path / "dest" / "a.txt").read_bytes() == b"a"
+ assert (tmp_path / "dest" / "sub" / "b.txt").read_bytes() == b"b"
+
Review Comment:
Good catch. The guard now forwards only `maxdepth` to `fs.find`, which is
what `fs.get` itself passes to the listing, so `callback` and the other
transfer kwargs no longer reach `_lsdir`.
One tweak to the test: with fsspec 2026.9.0 (what `uv.lock` pins)
`MemoryFileSystem.find` is a single pass over the store and never calls `ls`,
so patching `ls` didn't fail on this branch for me. I kept the test but patch
`find` with an s3fs-like strict signature instead; it fails with `TypeError:
unexpected keyword argument 'callback'` before the fix and passes after.
--
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]
