sftp::set_metadata is broken: https://github.com/wez/wezterm/issues/3784

distant-ssh2/src/api.rs

@@ -89,6 +89,9 @@ impl DistantApi for SshDistantApi {
         capabilities.take(CapabilityKind::Search);
         capabilities.take(CapabilityKind::CancelSearch);
 
+        // Broken via wezterm-ssh, so not supported right now
+        capabilities.take(CapabilityKind::SetPermissions);
+
         Ok(capabilities)
     }
 
@@ -686,6 +689,7 @@ impl DistantApi for SshDistantApi {
         })
     }
 
+    #[allow(unreachable_code)]
     async fn set_permissions(
         &self,
         ctx: DistantCtx<Self::LocalData>,
@@ -697,27 +701,19 @@ impl DistantApi for SshDistantApi {
             "[Conn {}] Setting permissions for {:?} {{permissions: {:?}, options: {:?}}}",
             ctx.connection_id, path, permissions, options
         );
-        let sftp = self.session.sftp();
 
-        macro_rules! set_permissions {
-            ($path:expr) => {{
-                let filename = if options.follow_symlinks {
-                    sftp.read_link($path)
-                        .compat()
-                        .await
-                        .map_err(to_other_error)?
-                } else {
-                    Utf8PathBuf::try_from($path).map_err(to_other_error)?
-                };
+        // Unsupported until issue resolved: https://github.com/wez/wezterm/issues/3784
+        return Err(io::Error::new(
+            io::ErrorKind::Unsupported,
+            "Unsupported until issue resolved: https://github.com/wez/wezterm/issues/3784",
+        ));
 
-                let mut metadata = sftp
-                    .symlink_metadata(&filename)
-                    .compat()
-                    .await
-                    .map_err(to_other_error)?;
+        let sftp = self.session.sftp();
 
+        macro_rules! set_permissions {
+            ($path:ident, $metadata:ident) => {{
                 let mut current = Permissions::from_unix_mode(
-                    metadata
+                    $metadata
                         .permissions
                         .ok_or_else(|| to_other_error("Unable to read file permissions"))?
                         .to_unix_mode(),
@@ -725,41 +721,99 @@ impl DistantApi for SshDistantApi {
 
                 current.apply_from(&permissions);
 
-                metadata.permissions =
+                $metadata.permissions =
                     Some(FilePermissions::from_unix_mode(current.to_unix_mode()));
 
-                sftp.set_metadata(filename.as_path(), metadata)
+                println!("set_metadata for {:?}", $path.as_path());
+                sftp.set_metadata($path.as_path(), $metadata)
                     .compat()
                     .await
                     .map_err(to_other_error)?;
 
-                if metadata.is_dir() {
-                    Some(filename)
+                if $metadata.is_dir() {
+                    Some($path)
                 } else {
                     None
                 }
             }};
+            ($path:ident) => {{
+                let mut path = Utf8PathBuf::try_from($path).map_err(to_other_error)?;
+
+                // Query metadata to determine if we are working with a symlink
+                println!("symlink_metadata for {:?}", path);
+                let mut metadata = sftp
+                    .symlink_metadata(&path)
+                    .compat()
+                    .await
+                    .map_err(to_other_error)?;
+
+                // If we are excluding symlinks and this is a symlink, then we're done
+                if options.exclude_symlinks && metadata.is_symlink() {
+                    None
+                } else {
+                    // If we are following symlinks and this is a symlink, then get the real path
+                    // and destination metadata
+                    if options.follow_symlinks && metadata.is_symlink() {
+                        println!("read_link for {:?}", path);
+                        path = sftp
+                            .read_link(path)
+                            .compat()
+                            .await
+                            .map_err(to_other_error)?;
+
+                        println!("metadata for {:?}", path);
+                        metadata = sftp
+                            .metadata(&path)
+                            .compat()
+                            .await
+                            .map_err(to_other_error)?;
+                    }
+
+                    set_permissions!(path, metadata)
+                }
+            }};
         }
 
-        if options.recursive {
-            let mut paths = VecDeque::new();
+        let mut paths = VecDeque::new();
 
-            // Queue up our path if it is a directory
-            if let Some(path) = set_permissions!(path) {
-                paths.push_back(path);
-            }
+        // Queue up our path if it is a directory
+        if let Some(path) = set_permissions!(path) {
+            paths.push_back(path);
+        }
 
+        if options.recursive {
             while let Some(path) = paths.pop_front() {
+                println!("read_dir for {:?}", path);
                 let paths_and_metadata =
                     sftp.read_dir(path).compat().await.map_err(to_other_error)?;
-                for (path, _) in paths_and_metadata {
-                    if let Some(path) = set_permissions!(path) {
+                for (mut path, mut metadata) in paths_and_metadata {
+                    if options.exclude_symlinks && metadata.is_symlink() {
+                        println!("skipping symlink for {:?}", path);
+                        continue;
+                    }
+
+                    // If we are following symlinks, then adjust our path and metadata
+                    if options.follow_symlinks && metadata.is_symlink() {
+                        println!("read_link for {:?}", path);
+                        path = sftp
+                            .read_link(path)
+                            .compat()
+                            .await
+                            .map_err(to_other_error)?;
+
+                        println!("metadata for {:?}", path);
+                        metadata = sftp
+                            .metadata(&path)
+                            .compat()
+                            .await
+                            .map_err(to_other_error)?;
+                    }
+
+                    if let Some(path) = set_permissions!(path, metadata) {
                         paths.push_back(path);
                     }
                 }
             }
-        } else {
-            let _ = set_permissions!(path);
         }
 
         Ok(())

distant-ssh2/tests/ssh2/client.rs

@@ -4,7 +4,9 @@ use std::time::Duration;
 
 use assert_fs::prelude::*;
 use assert_fs::TempDir;
-use distant_core::protocol::{ChangeKindSet, Environment, FileType, Metadata};
+use distant_core::protocol::{
+    ChangeKindSet, Environment, FileType, Metadata, Permissions, SetPermissionsOptions,
+};
 use distant_core::{DistantChannelExt, DistantClient};
 use once_cell::sync::Lazy;
 use predicates::prelude::*;
@@ -1207,6 +1209,476 @@ async fn metadata_should_resolve_file_type_of_symlink_if_flag_specified(
     );
 }
 
+#[rstest]
+#[test(tokio::test)]
+#[ignore]
+async fn set_permissions_should_set_readonly_flag_if_specified(
+    #[future] client: Ctx<DistantClient>,
+) {
+    let mut client = client.await;
+    let temp = assert_fs::TempDir::new().unwrap();
+    let file = temp.child("file");
+    file.write_str("some text").unwrap();
+
+    // Verify that not readonly by default
+    let permissions = tokio::fs::symlink_metadata(file.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(!permissions.readonly(), "File is already set to readonly");
+
+    // Change the file permissions
+    client
+        .set_permissions(
+            file.path().to_path_buf(),
+            Permissions::readonly(),
+            Default::default(),
+        )
+        .await
+        .unwrap();
+
+    // Retrieve permissions to verify set
+    let permissions = tokio::fs::symlink_metadata(file.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(permissions.readonly(), "File not set to readonly");
+}
+
+#[allow(unused_attributes)]
+#[rstest]
+#[test(tokio::test)]
+#[cfg_attr(not(unix), ignore)]
+#[ignore]
+async fn set_permissions_should_set_unix_permissions_if_on_unix_platform(
+    #[future] client: Ctx<DistantClient>,
+) {
+    #[cfg(unix)]
+    {
+        use std::os::unix::prelude::*;
+
+        let mut client = client.await;
+        let temp = assert_fs::TempDir::new().unwrap();
+        let file = temp.child("file");
+        file.write_str("some text").unwrap();
+
+        // Verify that permissions do not match our readonly state
+        let permissions = tokio::fs::symlink_metadata(file.path())
+            .await
+            .unwrap()
+            .permissions();
+        let mode = permissions.mode() & 0o777;
+        assert_ne!(mode, 0o400, "File is already set to 0o400");
+
+        // Change the file permissions
+        client
+            .set_permissions(
+                file.path().to_path_buf(),
+                Permissions::from_unix_mode(0o400),
+                Default::default(),
+            )
+            .await
+            .unwrap();
+
+        // Retrieve file permissions to verify set
+        let permissions = tokio::fs::symlink_metadata(file.path())
+            .await
+            .unwrap()
+            .permissions();
+
+        // Drop the upper bits that mode can have (only care about read/write/exec)
+        let mode = permissions.mode() & 0o777;
+
+        assert_eq!(mode, 0o400, "Wrong permissions on file: {:o}", mode);
+    }
+    #[cfg(not(unix))]
+    {
+        unreachable!();
+    }
+}
+
+#[allow(unused_attributes)]
+#[rstest]
+#[test(tokio::test)]
+#[cfg_attr(unix, ignore)]
+#[ignore]
+async fn set_permissions_should_set_readonly_flag_if_not_on_unix_platform(
+    #[future] client: Ctx<DistantClient>,
+) {
+    let mut client = client.await;
+    let temp = assert_fs::TempDir::new().unwrap();
+    let file = temp.child("file");
+    file.write_str("some text").unwrap();
+
+    // Verify that not readonly by default
+    let permissions = tokio::fs::symlink_metadata(file.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(!permissions.readonly(), "File is already set to readonly");
+
+    // Change the file permissions to be readonly (in general)
+    client
+        .set_permissions(
+            file.path().to_path_buf(),
+            Permissions::from_unix_mode(0o400),
+            Default::default(),
+        )
+        .await
+        .unwrap();
+
+    #[cfg(not(unix))]
+    {
+        // Retrieve file permissions to verify set
+        let permissions = tokio::fs::symlink_metadata(file.path())
+            .await
+            .unwrap()
+            .permissions();
+
+        assert!(permissions.readonly(), "File not marked as readonly");
+    }
+    #[cfg(unix)]
+    {
+        unreachable!();
+    }
+}
+
+#[rstest]
+#[test(tokio::test)]
+#[ignore]
+async fn set_permissions_should_not_recurse_if_option_false(#[future] client: Ctx<DistantClient>) {
+    let mut client = client.await;
+    let temp = assert_fs::TempDir::new().unwrap();
+    let file = temp.child("file");
+    file.write_str("some text").unwrap();
+
+    let symlink = temp.child("link");
+    symlink.symlink_to_file(file.path()).unwrap();
+
+    // Verify that dir is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(temp.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(
+        !permissions.readonly(),
+        "Temp dir is already set to readonly"
+    );
+
+    // Verify that file is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(file.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(!permissions.readonly(), "File is already set to readonly");
+
+    // Verify that symlink is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(symlink.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(
+        !permissions.readonly(),
+        "Symlink is already set to readonly"
+    );
+
+    // Change the permissions of the directory and not the contents underneath
+    client
+        .set_permissions(
+            temp.path().to_path_buf(),
+            Permissions::readonly(),
+            SetPermissionsOptions {
+                recursive: false,
+                ..Default::default()
+            },
+        )
+        .await
+        .unwrap();
+
+    // Retrieve permissions of the file, symlink, and directory to verify set
+    let permissions = tokio::fs::symlink_metadata(temp.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(permissions.readonly(), "Temp directory not set to readonly");
+
+    let permissions = tokio::fs::symlink_metadata(file.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(!permissions.readonly(), "File unexpectedly set to readonly");
+
+    let permissions = tokio::fs::symlink_metadata(symlink.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(
+        !permissions.readonly(),
+        "Symlink unexpectedly set to readonly"
+    );
+}
+
+#[rstest]
+#[test(tokio::test)]
+#[ignore]
+async fn set_permissions_should_traverse_symlinks_while_recursing_if_following_symlinks_enabled(
+    #[future] client: Ctx<DistantClient>,
+) {
+    let mut client = client.await;
+    let temp = assert_fs::TempDir::new().unwrap();
+    let file = temp.child("file");
+    file.write_str("some text").unwrap();
+
+    let temp2 = assert_fs::TempDir::new().unwrap();
+    let file2 = temp2.child("file");
+    file2.write_str("some text").unwrap();
+
+    let symlink = temp.child("link");
+    symlink.symlink_to_dir(temp2.path()).unwrap();
+
+    // Verify that symlink is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(file2.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(!permissions.readonly(), "File2 is already set to readonly");
+
+    // Change the main directory permissions
+    client
+        .set_permissions(
+            temp.path().to_path_buf(),
+            Permissions::readonly(),
+            SetPermissionsOptions {
+                follow_symlinks: true,
+                recursive: true,
+                ..Default::default()
+            },
+        )
+        .await
+        .unwrap();
+
+    // Retrieve permissions referenced by another directory
+    let permissions = tokio::fs::symlink_metadata(file2.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(permissions.readonly(), "File2 not set to readonly");
+}
+
+#[rstest]
+#[test(tokio::test)]
+#[ignore]
+async fn set_permissions_should_not_traverse_symlinks_while_recursing_if_following_symlinks_disabled(
+    #[future] client: Ctx<DistantClient>,
+) {
+    let mut client = client.await;
+    let temp = assert_fs::TempDir::new().unwrap();
+    let file = temp.child("file");
+    file.write_str("some text").unwrap();
+
+    let temp2 = assert_fs::TempDir::new().unwrap();
+    let file2 = temp2.child("file");
+    file2.write_str("some text").unwrap();
+
+    let symlink = temp.child("link");
+    symlink.symlink_to_dir(temp2.path()).unwrap();
+
+    // Verify that symlink is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(file2.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(!permissions.readonly(), "File2 is already set to readonly");
+
+    // Change the main directory permissions
+    client
+        .set_permissions(
+            temp.path().to_path_buf(),
+            Permissions::readonly(),
+            SetPermissionsOptions {
+                follow_symlinks: false,
+                recursive: true,
+                ..Default::default()
+            },
+        )
+        .await
+        .unwrap();
+
+    // Retrieve permissions referenced by another directory
+    let permissions = tokio::fs::symlink_metadata(file2.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(
+        !permissions.readonly(),
+        "File2 unexpectedly set to readonly"
+    );
+}
+
+#[rstest]
+#[test(tokio::test)]
+#[ignore]
+async fn set_permissions_should_skip_symlinks_if_exclude_symlinks_enabled(
+    #[future] client: Ctx<DistantClient>,
+) {
+    let mut client = client.await;
+    let temp = assert_fs::TempDir::new().unwrap();
+    let file = temp.child("file");
+    file.write_str("some text").unwrap();
+
+    let symlink = temp.child("link");
+    symlink.symlink_to_file(file.path()).unwrap();
+
+    // Verify that symlink is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(symlink.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(
+        !permissions.readonly(),
+        "Symlink is already set to readonly"
+    );
+
+    // Change the symlink permissions
+    client
+        .set_permissions(
+            symlink.path().to_path_buf(),
+            Permissions::readonly(),
+            SetPermissionsOptions {
+                exclude_symlinks: true,
+                ..Default::default()
+            },
+        )
+        .await
+        .unwrap();
+
+    // Retrieve permissions to verify not set
+    let permissions = tokio::fs::symlink_metadata(symlink.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(
+        !permissions.readonly(),
+        "Symlink (or file underneath) set to readonly"
+    );
+}
+
+#[rstest]
+#[test(tokio::test)]
+#[ignore]
+async fn set_permissions_should_support_recursive_if_option_specified(
+    #[future] client: Ctx<DistantClient>,
+) {
+    let mut client = client.await;
+    let temp = assert_fs::TempDir::new().unwrap();
+    let file = temp.child("file");
+    file.write_str("some text").unwrap();
+
+    // Verify that dir is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(temp.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(
+        !permissions.readonly(),
+        "Temp dir is already set to readonly"
+    );
+
+    // Verify that file is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(file.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(!permissions.readonly(), "File is already set to readonly");
+
+    // Change the permissions of the file pointed to by the symlink
+    client
+        .set_permissions(
+            temp.path().to_path_buf(),
+            Permissions::readonly(),
+            SetPermissionsOptions {
+                recursive: true,
+                ..Default::default()
+            },
+        )
+        .await
+        .unwrap();
+
+    // Retrieve permissions of the file, symlink, and directory to verify set
+    let permissions = tokio::fs::symlink_metadata(temp.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(permissions.readonly(), "Temp directory not set to readonly");
+
+    let permissions = tokio::fs::symlink_metadata(file.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(permissions.readonly(), "File not set to readonly");
+}
+
+#[rstest]
+#[test(tokio::test)]
+#[ignore]
+async fn set_permissions_should_support_following_symlinks_if_option_specified(
+    #[future] client: Ctx<DistantClient>,
+) {
+    let mut client = client.await;
+    let temp = assert_fs::TempDir::new().unwrap();
+    let file = temp.child("file");
+    file.write_str("some text").unwrap();
+
+    let symlink = temp.child("link");
+    symlink.symlink_to_file(file.path()).unwrap();
+
+    // Verify that file is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(file.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(!permissions.readonly(), "File is already set to readonly");
+
+    // Verify that symlink is not readonly by default
+    let permissions = tokio::fs::symlink_metadata(symlink.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(
+        !permissions.readonly(),
+        "Symlink is already set to readonly"
+    );
+
+    // Change the permissions of the file pointed to by the symlink
+    client
+        .set_permissions(
+            symlink.path().to_path_buf(),
+            Permissions::readonly(),
+            SetPermissionsOptions {
+                follow_symlinks: true,
+                ..Default::default()
+            },
+        )
+        .await
+        .unwrap();
+
+    // Retrieve permissions of the file and symlink to verify set
+    let permissions = tokio::fs::symlink_metadata(file.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(permissions.readonly(), "File not set to readonly");
+
+    let permissions = tokio::fs::symlink_metadata(symlink.path())
+        .await
+        .unwrap()
+        .permissions();
+    assert!(
+        !permissions.readonly(),
+        "Symlink unexpectedly set to readonly"
+    );
+}
+
 #[rstest]
 #[test(tokio::test)]
 async fn proc_spawn_should_not_fail_even_if_process_not_found(

distant-ssh2/tests/ssh2/ssh.rs

@@ -9,6 +9,9 @@ use crate::sshd::*;
 async fn detect_family_should_return_windows_if_sshd_on_windows(#[future] ssh: Ctx<Ssh>) {
     let ssh = ssh.await;
     let family = ssh.detect_family().await.expect("Failed to detect family");
+
+    // NOTE: We are testing against the local machine, so if Rust was compiled for Windows, then we
+    //       are also on a Windows machine remotely for this test!
     assert_eq!(
         family,
         if cfg!(windows) {