Add tests for set_permissions of local

distant-core/src/api/local.rs

@@ -2,6 +2,7 @@ use std::io;
 use std::path::{Path, PathBuf};
 
 use async_trait::async_trait;
+use ignore::{DirEntry as WalkDirEntry, WalkBuilder};
 use log::*;
 use tokio::io::AsyncWriteExt;
 use walkdir::WalkDir;
@@ -418,7 +419,116 @@ impl DistantApi for LocalDistantApi {
         permissions: Permissions,
         options: SetPermissionsOptions,
     ) -> io::Result<()> {
-        permissions.write(path, options).await
+        /// Builds permissions from the metadata of `entry`, failing if metadata was unavailable.
+        fn build_permissions(
+            entry: &WalkDirEntry,
+            permissions: &Permissions,
+        ) -> io::Result<std::fs::Permissions> {
+            // Load up our std permissions so we can modify them
+            let mut std_permissions = entry
+                .metadata()
+                .map_err(|x| match x.io_error() {
+                    Some(x) => io::Error::new(x.kind(), format!("(Read permissions failed) {x}")),
+                    None => io::Error::new(
+                        io::ErrorKind::Other,
+                        format!("(Read permissions failed) {x}"),
+                    ),
+                })?
+                .permissions();
+
+            // Create current permissions from std permissions
+            let mut current = Permissions::from(std_permissions.clone());
+
+            // Apply the readonly flag for all platforms
+            if let Some(readonly) = permissions.is_readonly() {
+                std_permissions.set_readonly(readonly);
+            }
+
+            // On Unix platforms, we can apply a bitset change
+            #[cfg(unix)]
+            {
+                use std::os::unix::prelude::*;
+                current.apply_from(permissions);
+                std_permissions.set_mode(current.to_unix_mode());
+            }
+
+            Ok(std_permissions)
+        }
+
+        async fn set_permissions_impl(
+            entry: &WalkDirEntry,
+            permissions: &Permissions,
+        ) -> io::Result<()> {
+            let permissions = match permissions.is_complete() {
+                // If we are on a Unix platform and we have a full permission set, we do not need
+                // to retrieve the permissions to modify them and can instead produce a new
+                // permission set purely from the permissions
+                #[cfg(unix)]
+                true => std::fs::Permissions::from(*permissions),
+
+                // Otherwise, we have to load in the permissions from metadata and merge with our
+                // changes
+                _ => build_permissions(entry, permissions)?,
+            };
+
+            if log_enabled!(Level::Trace) {
+                let mut output = String::new();
+                output.push_str("readonly = ");
+                output.push_str(if permissions.readonly() {
+                    "true"
+                } else {
+                    "false"
+                });
+
+                #[cfg(unix)]
+                {
+                    use std::os::unix::prelude::*;
+                    output.push_str(&format!(", mode = {:#o}", permissions.mode()));
+                }
+
+                trace!("Setting {:?} permissions to ({})", entry.path(), output);
+            }
+
+            tokio::fs::set_permissions(entry.path(), permissions)
+                .await
+                .map_err(|x| io::Error::new(x.kind(), format!("(Set permissions failed) {x}")))
+        }
+
+        let walk = WalkBuilder::new(path)
+            .follow_links(options.follow_symlinks)
+            .max_depth(if options.recursive { None } else { Some(0) })
+            .standard_filters(false)
+            .skip_stdout(true)
+            .build();
+
+        // Process as much as possible and then fail with an error
+        let mut errors = Vec::new();
+        for entry in walk {
+            match entry {
+                Ok(entry) if entry.path_is_symlink() && options.exclude_symlinks => {}
+                Ok(entry) => {
+                    if let Err(x) = set_permissions_impl(&entry, &permissions).await {
+                        errors.push(format!("{:?}: {x}", entry.path()));
+                    }
+                }
+                Err(x) => {
+                    errors.push(x.to_string());
+                }
+            }
+        }
+
+        if errors.is_empty() {
+            Ok(())
+        } else {
+            Err(io::Error::new(
+                io::ErrorKind::PermissionDenied,
+                errors
+                    .into_iter()
+                    .map(|x| format!("* {x}"))
+                    .collect::<Vec<_>>()
+                    .join("\n"),
+            ))
+        }
     }
 
     async fn search(
@@ -1820,6 +1930,442 @@ mod tests {
         );
     }
 
+    #[test(tokio::test)]
+    async fn set_permissions_should_set_readonly_flag_if_specified() {
+        let (api, ctx, _rx) = setup(1).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
+        api.set_permissions(
+            ctx,
+            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");
+    }
+
+    #[test(tokio::test)]
+    #[cfg_attr(not(unix), ignore)]
+    async fn set_permissions_should_set_unix_permissions_if_on_unix_platform() {
+        #[cfg(unix)]
+        {
+            use std::os::unix::prelude::*;
+
+            let (api, ctx, _rx) = setup(1).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
+            api.set_permissions(
+                ctx,
+                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!();
+        }
+    }
+
+    #[test(tokio::test)]
+    #[cfg_attr(unix, ignore)]
+    async fn set_permissions_should_set_readonly_flag_if_not_on_unix_platform() {
+        let (api, ctx, _rx) = setup(1).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)
+        api.set_permissions(
+            ctx,
+            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!();
+        }
+    }
+
+    #[test(tokio::test)]
+    async fn set_permissions_should_not_recurse_if_option_false() {
+        let (api, ctx, _rx) = setup(1).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
+        api.set_permissions(
+            ctx,
+            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"
+        );
+    }
+
+    #[test(tokio::test)]
+    async fn set_permissions_should_traverse_symlinks_while_recursing_if_following_symlinks_enabled(
+    ) {
+        let (api, ctx, _rx) = setup(1).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
+        api.set_permissions(
+            ctx,
+            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");
+    }
+
+    #[test(tokio::test)]
+    async fn set_permissions_should_not_traverse_symlinks_while_recursing_if_following_symlinks_disabled(
+    ) {
+        let (api, ctx, _rx) = setup(1).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
+        api.set_permissions(
+            ctx,
+            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"
+        );
+    }
+
+    #[test(tokio::test)]
+    async fn set_permissions_should_skip_symlinks_if_exclude_symlinks_enabled() {
+        let (api, ctx, _rx) = setup(1).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
+        api.set_permissions(
+            ctx,
+            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"
+        );
+    }
+
+    #[test(tokio::test)]
+    async fn set_permissions_should_support_recursive_if_option_specified() {
+        let (api, ctx, _rx) = setup(1).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
+        api.set_permissions(
+            ctx,
+            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");
+    }
+
+    #[test(tokio::test)]
+    async fn set_permissions_should_support_following_symlinks_if_option_specified() {
+        let (api, ctx, _rx) = setup(1).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
+        api.set_permissions(
+            ctx,
+            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"
+        );
+    }
+
     // NOTE: Ignoring on windows because it's using WSL which wants a Linux path
     //       with / but thinks it's on windows and is providing \
     #[test(tokio::test)]

distant-core/src/protocol/permissions.rs

@@ -1,27 +1,23 @@
-use std::cmp;
-use std::io;
-use std::path::Path;
-
 use bitflags::bitflags;
-use ignore::types::TypesBuilder;
-use ignore::DirEntry;
-use ignore::WalkBuilder;
-use log::*;
 use serde::{Deserialize, Serialize};
 
-const MAXIMUM_THREADS: usize = 12;
-
 #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
 #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
 #[serde(default, deny_unknown_fields, rename_all = "snake_case")]
 pub struct SetPermissionsOptions {
+    /// Whether or not to exclude symlinks from traversal entirely, meaning that permissions will
+    /// not be set on symlinks (usually resolving the symlink and setting the permission of the
+    /// referenced file or directory) that are explicitly provided or show up during recursion.
+    pub exclude_symlinks: bool,
+
+    /// Whether or not to traverse symlinks when recursively setting permissions. Note that this
+    /// does NOT influence setting permissions when encountering a symlink as most platforms will
+    /// resolve the symlink before setting permissions.
+    pub follow_symlinks: bool,
+
     /// Whether or not to set the permissions of the file hierarchies rooted in the paths, instead
-    /// of just the paths themselves
+    /// of just the paths themselves.
     pub recursive: bool,
-
-    /// Whether or not to resolve the pathes to the underlying file/directory prior to setting the
-    /// permissions
-    pub resolve_symlink: bool,
 }
 
 #[cfg(feature = "schemars")]
@@ -35,197 +31,13 @@ impl SetPermissionsOptions {
 ///
 /// When used to set permissions on a file, directory, or symlink,
 /// only fields that are set (not `None`) will be applied.
-#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
-#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
-pub struct Permissions {
-    /// Whether or not the file/directory/symlink is marked as unwriteable
-    pub readonly: Option<bool>,
-
-    /// Represents permissions that are specific to a unix remote machine
-    pub unix: Option<UnixPermissions>,
-}
-
-impl Permissions {
-    pub async fn read(path: impl AsRef<Path>, resolve_symlink: bool) -> io::Result<Self> {
-        let std_permissions = Self::read_std_permissions(path, resolve_symlink).await?;
-
-        Ok(Self {
-            readonly: Some(std_permissions.readonly()),
-            #[cfg(unix)]
-            unix: Some(UnixPermissions::from(std_permissions)),
-            #[cfg(not(unix))]
-            unix: None,
-        })
-    }
-
-    /// Sets the permissions for the specified `path`.
-    ///
-    /// If `resolve_symlink` is true, will resolve the path to the underlying file/directory prior
-    /// to attempting to set the permissions.
-    ///
-    /// If `recursive` is true, will apply permissions to all
-    ///
-    /// When used to set permissions on a file, directory, or symlink, only fields that are set
-    /// (not `None`) will be applied.
-    pub async fn write(
-        &self,
-        path: impl AsRef<Path>,
-        options: SetPermissionsOptions,
-    ) -> io::Result<()> {
-        async fn set_permissions(this: &Permissions, entry: &DirEntry) -> io::Result<()> {
-            // If we are on a Unix platform and we have a full permission set, we do not need to
-            // retrieve the permissions to modify them and can instead produce a new permission set
-            // purely from the Unix permissions
-            let permissions = if cfg!(unix) && this.has_complete_unix_permissions() {
-                this.unix.unwrap().into()
-            } else {
-                let mut std_permissions = entry
-                    .metadata()
-                    .map_err(|x| match x.io_error() {
-                        Some(x) => {
-                            io::Error::new(x.kind(), format!("(Read permissions failed) {x}"))
-                        }
-                        None => io::Error::new(
-                            io::ErrorKind::Other,
-                            format!("(Read permissions failed) {x}"),
-                        ),
-                    })?
-                    .permissions();
-
-                // Apply the readonly flag if we are provided it
-                if let Some(readonly) = this.readonly {
-                    std_permissions.set_readonly(readonly);
-                }
-
-                // Update our unix permissions if we were given new permissions by loading
-                // in the current permissions and applying any changes on top of those
-                #[cfg(unix)]
-                if let Some(permissions) = this.unix {
-                    use std::os::unix::prelude::*;
-                    let mut current = UnixPermissions::from_unix_mode(std_permissions.mode());
-                    current.apply_from(&permissions);
-                    std_permissions.set_mode(current.to_unix_mode());
-                }
-
-                std_permissions
-            };
-
-            if log_enabled!(Level::Trace) {
-                let mut output = String::new();
-                output.push_str("readonly = ");
-                output.push_str(if permissions.readonly() {
-                    "true"
-                } else {
-                    "false"
-                });
-
-                #[cfg(unix)]
-                {
-                    use std::os::unix::prelude::*;
-                    output.push_str(&format!(", mode = {:#o}", permissions.mode()));
-                }
-
-                trace!("Setting {:?} permissions to ({})", entry.path(), output);
-            }
-
-            tokio::fs::set_permissions(entry.path(), permissions)
-                .await
-                .map_err(|x| io::Error::new(x.kind(), format!("(Set permissions failed) {x}")))
-        }
-
-        let walk = WalkBuilder::new(path)
-            .follow_links(options.resolve_symlink)
-            .max_depth(if options.recursive { None } else { Some(0) })
-            .threads(cmp::min(MAXIMUM_THREADS, num_cpus::get()))
-            .types(
-                TypesBuilder::new()
-                    .add_defaults()
-                    .build()
-                    .map_err(|x| io::Error::new(io::ErrorKind::Other, x))?,
-            )
-            .skip_stdout(true)
-            .build();
-
-        // Process as much as possible and then fail with an error
-        let mut errors = Vec::new();
-        for entry in walk {
-            match entry {
-                Ok(entry) => {
-                    if let Err(x) = set_permissions(self, &entry).await {
-                        errors.push(format!("{:?}: {x}", entry.path()));
-                    }
-                }
-                Err(x) => {
-                    errors.push(x.to_string());
-                }
-            }
-        }
-
-        if errors.is_empty() {
-            Ok(())
-        } else {
-            Err(io::Error::new(
-                io::ErrorKind::PermissionDenied,
-                errors
-                    .into_iter()
-                    .map(|x| format!("* {x}"))
-                    .collect::<Vec<_>>()
-                    .join("\n"),
-            ))
-        }
-    }
-
-    /// Reads [`std::fs::Permissions`] from `path`.
-    ///
-    /// If `resolve_symlink` is true, will resolve the path to the underlying file/directory prior
-    /// to attempting to read the permissions.
-    async fn read_std_permissions(
-        path: impl AsRef<Path>,
-        resolve_symlink: bool,
-    ) -> io::Result<std::fs::Permissions> {
-        Ok(if resolve_symlink {
-            tokio::fs::metadata(path.as_ref()).await?.permissions()
-        } else {
-            tokio::fs::symlink_metadata(path.as_ref())
-                .await?
-                .permissions()
-        })
-    }
-
-    /// Returns true if `unix` is populated with a complete permission set as defined by
-    /// [`UnixPermissions::is_complete`], otherwise returns false.
-    pub fn has_complete_unix_permissions(&self) -> bool {
-        if let Some(permissions) = self.unix.as_ref() {
-            permissions.is_complete()
-        } else {
-            false
-        }
-    }
-}
-
-#[cfg(feature = "schemars")]
-impl Permissions {
-    pub fn root_schema() -> schemars::schema::RootSchema {
-        schemars::schema_for!(Permissions)
-    }
-}
-
-impl From<std::fs::Permissions> for Permissions {
-    fn from(permissions: std::fs::Permissions) -> Self {
-        Self {
-            readonly: Some(permissions.readonly()),
-            #[cfg(unix)]
-            unix: Some(UnixPermissions::from(permissions)),
-            #[cfg(not(unix))]
-            unix: None,
-        }
-    }
-}
-
-/// Represents unix-specific permissions about some path on a remote machine
+///
+/// On `Unix` platforms, this translates directly into the mode that
+/// you would find with `chmod`. On all other platforms, this uses the
+/// write flags to determine whether or not to set the readonly status.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
-pub struct UnixPermissions {
+pub struct Permissions {
     /// Represents whether or not owner can read from the file
     pub owner_read: Option<bool>,
 
@@ -254,7 +66,56 @@ pub struct UnixPermissions {
     pub other_exec: Option<bool>,
 }
 
-impl UnixPermissions {
+impl Permissions {
+    /// Creates a set of [`Permissions`] that indicate readonly status.
+    ///
+    /// ```
+    /// use distant_core::protocol::Permissions;
+    ///
+    /// let permissions = Permissions::readonly();
+    /// assert_eq!(permissions.is_readonly(), Some(true));
+    /// assert_eq!(permissions.is_writable(), Some(false));
+    /// ```
+    pub fn readonly() -> Self {
+        Self {
+            owner_write: Some(false),
+            group_write: Some(false),
+            other_write: Some(false),
+
+            owner_read: Some(true),
+            group_read: Some(true),
+            other_read: Some(true),
+
+            owner_exec: None,
+            group_exec: None,
+            other_exec: None,
+        }
+    }
+    /// Creates a set of [`Permissions`] that indicate globally writable status.
+    ///
+    /// ```
+    /// use distant_core::protocol::Permissions;
+    ///
+    /// let permissions = Permissions::writable();
+    /// assert_eq!(permissions.is_readonly(), Some(false));
+    /// assert_eq!(permissions.is_writable(), Some(true));
+    /// ```
+    pub fn writable() -> Self {
+        Self {
+            owner_write: Some(true),
+            group_write: Some(true),
+            other_write: Some(true),
+
+            owner_read: Some(true),
+            group_read: Some(true),
+            other_read: Some(true),
+
+            owner_exec: None,
+            group_exec: None,
+            other_exec: None,
+        }
+    }
+
     /// Returns true if the permission set has a value specified for each permission (no `None`
     /// settings).
     pub fn is_complete(&self) -> bool {
@@ -269,6 +130,24 @@ impl UnixPermissions {
             && self.other_exec.is_some()
     }
 
+    /// Returns `true` if permissions represent readonly, `false` if permissions represent
+    /// writable, and `None` if no permissions have been set to indicate either status.
+    #[inline]
+    pub fn is_readonly(&self) -> Option<bool> {
+        // Negate the writable status to indicate whether or not readonly
+        self.is_writable().map(|x| !x)
+    }
+
+    /// Returns `true` if permissions represent ability to write, `false` if permissions represent
+    /// inability to write, and `None` if no permissions have been set to indicate either status.
+    #[inline]
+    pub fn is_writable(&self) -> Option<bool> {
+        self.owner_write
+            .zip(self.group_write)
+            .zip(self.other_write)
+            .map(|((owner, group), other)| owner || group || other)
+    }
+
     /// Applies `other` settings to `self`, overwriting any of the permissions in `self` with `other`.
     #[inline]
     pub fn apply_from(&mut self, other: &Self) {
@@ -359,25 +238,42 @@ impl UnixPermissions {
 }
 
 #[cfg(feature = "schemars")]
-impl UnixPermissions {
+impl Permissions {
     pub fn root_schema() -> schemars::schema::RootSchema {
-        schemars::schema_for!(UnixPermissions)
+        schemars::schema_for!(Permissions)
     }
 }
 
 #[cfg(unix)]
-impl From<std::fs::Permissions> for UnixPermissions {
-    /// Converts [`std::fs::Permissions`] into [`UnixPermissions`] using the `mode`.
+impl From<std::fs::Permissions> for Permissions {
+    /// Converts [`std::fs::Permissions`] into [`Permissions`] using
+    /// [`std::os::unix::fs::PermissionsExt::mode`] to supply the bitset.
     fn from(permissions: std::fs::Permissions) -> Self {
         use std::os::unix::prelude::*;
         Self::from_unix_mode(permissions.mode())
     }
 }
 
+#[cfg(not(unix))]
+impl From<std::fs::Permissions> for Permissions {
+    /// Converts [`std::fs::Permissions`] into [`Permissions`] using the `readonly` flag.
+    ///
+    /// This will not set executable flags, but will set all read and write flags with write flags
+    /// being `false` if `readonly`, otherwise set to `true`.
+    fn from(permissions: std::fs::Permissions) -> Self {
+        if permissions.readonly() {
+            Self::readonly()
+        } else {
+            Self::writable()
+        }
+    }
+}
+
 #[cfg(unix)]
-impl From<UnixPermissions> for std::fs::Permissions {
-    /// Converts [`UnixPermissions`] into [`std::fs::Permissions`] using the `mode`.
-    fn from(permissions: UnixPermissions) -> Self {
+impl From<Permissions> for std::fs::Permissions {
+    /// Converts [`Permissions`] into [`std::fs::Permissions`] using
+    /// [`std::os::unix::fs::PermissionsExt::from_mode`].
+    fn from(permissions: Permissions) -> Self {
         use std::os::unix::prelude::*;
         std::fs::Permissions::from_mode(permissions.to_unix_mode())
     }

distant-ssh2/src/api.rs

@@ -10,7 +10,7 @@ use async_trait::async_trait;
 use distant_core::net::server::ConnectionCtx;
 use distant_core::protocol::{
     Capabilities, CapabilityKind, DirEntry, Environment, FileType, Metadata, Permissions,
-    ProcessId, PtySize, SetPermissionsOptions, SystemInfo, UnixMetadata, UnixPermissions,
+    ProcessId, PtySize, SetPermissionsOptions, SystemInfo, UnixMetadata,
 };
 use distant_core::{DistantApi, DistantCtx};
 use log::*;
@@ -701,7 +701,7 @@ impl DistantApi for SshDistantApi {
 
         macro_rules! set_permissions {
             ($path:expr) => {{
-                let filename = if options.resolve_symlink {
+                let filename = if options.follow_symlinks {
                     sftp.read_link($path)
                         .compat()
                         .await
@@ -716,36 +716,17 @@ impl DistantApi for SshDistantApi {
                     .await
                     .map_err(to_other_error)?;
 
-                // As is with Rust using `set_readonly`, this will make world-writable if true!
-                if let Some(readonly) = permissions.readonly {
-                    let mut current = UnixPermissions::from_unix_mode(
-                        metadata
-                            .permissions
-                            .ok_or_else(|| to_other_error("Unable to read file permissions"))?
-                            .to_unix_mode(),
-                    );
-
-                    current.owner_write = Some(!readonly);
-                    current.group_write = Some(!readonly);
-                    current.other_write = Some(!readonly);
-
-                    metadata.permissions =
-                        Some(FilePermissions::from_unix_mode(current.to_unix_mode()));
-                }
-
-                if let Some(new_permissions) = permissions.unix.as_ref() {
-                    let mut current = UnixPermissions::from_unix_mode(
-                        metadata
-                            .permissions
-                            .ok_or_else(|| to_other_error("Unable to read file permissions"))?
-                            .to_unix_mode(),
-                    );
+                let mut current = Permissions::from_unix_mode(
+                    metadata
+                        .permissions
+                        .ok_or_else(|| to_other_error("Unable to read file permissions"))?
+                        .to_unix_mode(),
+                );
 
-                    current.apply_from(new_permissions);
+                current.apply_from(&permissions);
 
-                    metadata.permissions =
-                        Some(FilePermissions::from_unix_mode(current.to_unix_mode()));
-                }
+                metadata.permissions =
+                    Some(FilePermissions::from_unix_mode(current.to_unix_mode()));
 
                 sftp.set_metadata(filename.as_path(), metadata)
                     .compat()

src/cli/commands/client.rs

@@ -8,7 +8,6 @@ use distant_core::net::common::{ConnectionId, Host, Map, Request, Response};
 use distant_core::net::manager::ManagerClient;
 use distant_core::protocol::{
     self, ChangeKindSet, FileType, Permissions, SearchQuery, SetPermissionsOptions, SystemInfo,
-    UnixPermissions,
 };
 use distant_core::{DistantChannel, DistantChannelExt, RemoteCommand, Searcher, Watcher};
 use log::*;
@@ -1021,35 +1020,24 @@ async fn async_run(cmd: ClientSubcommand) -> CliResult {
         }) => {
             debug!("Parsing {mode:?} into a proper set of permissions");
             let permissions = {
-                let readonly = if mode.trim().eq_ignore_ascii_case("readonly") {
-                    Some(true)
+                if mode.trim().eq_ignore_ascii_case("readonly") {
+                    Permissions::readonly()
                 } else if mode.trim().eq_ignore_ascii_case("notreadonly") {
-                    Some(false)
+                    Permissions::writable()
                 } else {
-                    None
-                };
-
-                Permissions {
-                    readonly,
-                    unix: {
-                        if readonly.is_none() {
-                            // Attempt to parse an octal number (chmod absolute), falling back to
-                            // parsing the mode string similar to chmod's symbolic mode
-                            let mode = match u32::from_str_radix(&mode, 8) {
-                                Ok(absolute) => file_mode::Mode::from(absolute),
-                                Err(_) => {
-                                    let mut new_mode = file_mode::Mode::empty();
-                                    new_mode
-                                        .set_str(&mode)
-                                        .context("Failed to parse mode string")?;
-                                    new_mode
-                                }
-                            };
-                            Some(UnixPermissions::from_unix_mode(mode.mode()))
-                        } else {
-                            None
+                    // Attempt to parse an octal number (chmod absolute), falling back to
+                    // parsing the mode string similar to chmod's symbolic mode
+                    let mode = match u32::from_str_radix(&mode, 8) {
+                        Ok(absolute) => file_mode::Mode::from(absolute),
+                        Err(_) => {
+                            let mut new_mode = file_mode::Mode::empty();
+                            new_mode
+                                .set_str(&mode)
+                                .context("Failed to parse mode string")?;
+                            new_mode
                         }
-                    },
+                    };
+                    Permissions::from_unix_mode(mode.mode())
                 }
             };
 
@@ -1072,7 +1060,7 @@ async fn async_run(cmd: ClientSubcommand) -> CliResult {
 
             let options = SetPermissionsOptions {
                 recursive,
-                resolve_symlink: follow_symlinks,
+                follow_symlinks,
             };
             debug!("Setting permissions for {path:?} as (permissions = {permissions:?}, options = {options:?})");
             channel