diff --git a/dvc/tree/ssh/connection.py b/dvc/tree/ssh/connection.py index 6c8e25856..37a4d6d74 100644 --- a/dvc/tree/ssh/connection.py +++ b/dvc/tree/ssh/connection.py @@ -2,6 +2,7 @@ import errno import logging import os import posixpath +import shlex import stat from contextlib import suppress @@ -291,6 +292,7 @@ class SSHConnection: Example: MD5 (foo.txt) = f3d220a856b52aabbf294351e8a24300 """ + path = shlex.quote(path) if self.uname == "Linux": md5 = self.execute("md5sum " + path).split()[0] elif self.uname == "Darwin": @@ -304,6 +306,8 @@ class SSHConnection: return md5 def copy(self, src, dest): + dest = shlex.quote(dest) + src = shlex.quote(src) self.execute(f"cp {src} {dest}") def open_max_sftp_channels(self): @@ -325,6 +329,8 @@ class SSHConnection: self.sftp.symlink(src, dest) def reflink(self, src, dest): + dest = shlex.quote(dest) + src = shlex.quote(src) if self.uname == "Linux": return self.execute(f"cp --reflink {src} {dest}") @@ -334,4 +340,6 @@ class SSHConnection: raise DvcException(f"'{self.uname}' is not supported as a SSH remote") def hardlink(self, src, dest): + dest = shlex.quote(dest) + src = shlex.quote(src) self.execute(f"ln {src} {dest}") diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py index a84a22924..875f57b8c 100644 --- a/tests/unit/remote/ssh/test_connection.py +++ b/tests/unit/remote/ssh/test_connection.py @@ -8,9 +8,18 @@ import pytest from dvc.info import get_fs_type from dvc.system import System +from dvc.tree.ssh.connection import SSHConnection here = os.path.abspath(os.path.dirname(__file__)) +SRC_PATH_WITH_SPECIAL_CHARACTERS = "Escape me [' , ']" +ESCAPED_SRC_PATH_WITH_SPECIAL_CHARACTERS = "'Escape me ['\"'\"' , '\"'\"']'" + +DEST_PATH_WITH_SPECIAL_CHARACTERS = "Escape me too [' , ']" +ESCAPED_DEST_PATH_WITH_SPECIAL_CHARACTERS = ( + "'Escape me too ['\"'\"' , '\"'\"']'" +) + def test_isdir(ssh_connection): assert ssh_connection.isdir(here) @@ -131,3 +140,60 @@ def test_move(tmp_dir, ssh_connection): ssh_connection.move("foo", "copy") assert os.path.exists("copy") assert not os.path.exists("foo") + + +@pytest.mark.parametrize( + "uname,md5command", [("Linux", "md5sum"), ("Darwin", "md5")] +) +def test_escapes_filepaths_for_md5_calculation( + ssh_connection, uname, md5command, mocker +): + fake_md5 = "x" * 32 + uname_mock = mocker.PropertyMock(return_value=uname) + mocker.patch.object(SSHConnection, "uname", new_callable=uname_mock) + ssh_connection.execute = mocker.Mock(return_value=fake_md5) + ssh_connection.md5(SRC_PATH_WITH_SPECIAL_CHARACTERS) + ssh_connection.execute.assert_called_with( + f"{md5command} {ESCAPED_SRC_PATH_WITH_SPECIAL_CHARACTERS}" + ) + + +def test_escapes_filepaths_for_copy(ssh_connection, mocker): + ssh_connection.execute = mocker.Mock() + ssh_connection.copy( + SRC_PATH_WITH_SPECIAL_CHARACTERS, DEST_PATH_WITH_SPECIAL_CHARACTERS + ) + ssh_connection.execute.assert_called_with( + f"cp {ESCAPED_SRC_PATH_WITH_SPECIAL_CHARACTERS} " + + f"{ESCAPED_DEST_PATH_WITH_SPECIAL_CHARACTERS}" + ) + + +@pytest.mark.parametrize( + "uname,cp_command", [("Linux", "cp --reflink"), ("Darwin", "cp -c")] +) +def test_escapes_filepaths_for_reflink( + ssh_connection, uname, cp_command, mocker +): + uname_mock = mocker.PropertyMock(return_value=uname) + mocker.patch.object(SSHConnection, "uname", new_callable=uname_mock) + ssh_connection.execute = mocker.Mock() + ssh_connection.reflink( + SRC_PATH_WITH_SPECIAL_CHARACTERS, DEST_PATH_WITH_SPECIAL_CHARACTERS + ) + ssh_connection.execute.assert_called_with( + f"{cp_command} " + + f"{ESCAPED_SRC_PATH_WITH_SPECIAL_CHARACTERS} " + + f"{ESCAPED_DEST_PATH_WITH_SPECIAL_CHARACTERS}" + ) + + +def test_escapes_filepaths_for_hardlink(ssh_connection, mocker): + ssh_connection.execute = mocker.Mock() + ssh_connection.hardlink( + SRC_PATH_WITH_SPECIAL_CHARACTERS, DEST_PATH_WITH_SPECIAL_CHARACTERS + ) + ssh_connection.execute.assert_called_with( + f"ln {ESCAPED_SRC_PATH_WITH_SPECIAL_CHARACTERS} " + + f"{ESCAPED_DEST_PATH_WITH_SPECIAL_CHARACTERS}" + )