search

ronf / asyncssh

AsyncSSH is a Python package which provides an asynchronous client and server implementation of the SSHv2 protocol on top of the Python asyncio framework.
Eclipse Public License 2.0
1.5k stars 144 forks source link

SFTP preserve always follows symlinks #666

Open eyalgolan1337 opened 6 days ago

eyalgolan1337 commented 6 days ago

When copying a symlink with preserve=True, the attributes of the target file are copied, instead of the link, even if follow_symlinks is False.

The attributes are also copied to the target on the destination, which causes copies to fail with SFTPNoSuchFile when the symlink is copied before the target.

ronf commented 5 days ago

Thanks for the report!

Unfortunately, the SFTP protocol doesn't provide the ability to do the equivalent of FXP_SETSTAT (used for doing a chown, chmod, or utime on a file) without following symlinks. So, even if I tried to make copy() aware of this, it would only work when copying data from a remote system to the local system, and not vice-versa. It also wouldn't work when running as a server and getting a copy request from a remote system, and even when copying to the local system it seems that os.chmod() not following links is not supported on all OSes. In particular, Linux explicitly forbids this for os.chmod(), and Python raises NotImplementedError there:

>>> os.chmod('file', 0o555, follow_symlinks=False)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NotImplementedError: chmod: follow_symlinks unavailable on this platform

The Linux chmod man page says the following:

chmod never changes the permissions of symbolic links; the chmod system call cannot change their permissions. This is not a problem since the permissions of symbolic links are never used. However, for each symbolic link listed on the command line, chmod changes the permissions of the pointed-to file. In contrast, chmod ignores symbolic links encoun‐ tered during recursive directory traversals.

So, there's really no way to make this work in all cases with follow_symlinks=False and preserve=True. I could make it work better when copying from remote to local, and potentially ignore the exception on Linux to avoid the other issue you mentioned about attempting to follow the link and getting an error that the file doesn't exist, but I don't see a good way in SFTP to set attributes on a remote symlink without following the link. I'd probably need to completely skip doing the preserve and not try to set attributes at all when copying symlinks to a remote system.

eyalgolan1337 commented 4 days ago

I think just skipping the call to setstat for symlinks makes the most sense, with a note in the documentation clarifying this behavior (and possibly an info log).

(OpenSSH does have the lsetstat@openssh.com extension, but I'm not sure that it makes sense to implement it.)

ronf commented 4 days ago

Ah, thanks - I hadn't noticed that OpenSSH added lsetstat@openssh.com. That actually makers me feel better about trying to support this. If this extension is added to AsyncSSH, both it and OpenSSH should have no problem fully implementing update of uid, gid, and timestamps on symlinks on all platforms, and even permissions on symlinks on BSD and macOS.

The only time we'd need to skip updating the attributes is when follow_symlinks=False and preserve=True are set and the peer doesn't support lsetstat. We'd effectively be ignoring preserve=True in this case, but only on symlinks.

If I get some time this weekend, I'll take a closer look.